generate a fallback for footer.url if footer_pathname not set#17550
Conversation
278f12d to
7364e76
Compare
7364e76 to
ef34f33
Compare
bobsilverberg
left a comment
bobsilverberg
left a comment
There was a problem hiding this comment.
Looks good, nice work!
r+wc
| is_flat_url = request and is_gate_active(request, 'wrap-outgoing-parameter') | ||
|
|
||
| url = data.get('url') | ||
| if not url and obj.endpoint in ( |
There was a problem hiding this comment.
Nit: I'm not sure if it would actually be better because of nesting, but looking at this it feels like you could start with just if not url:, because that is part of both of the following if / elif clauses.
There was a problem hiding this comment.
yeah I was just avoid the extra layer of indentation but I guess it's more "correct" to have a two levels.
| return url | ||
| else: | ||
| return { | ||
| **data, |
There was a problem hiding this comment.
What would this **data be? If we don't need to worry about it when is_flat_url is True, why do we need to worry about it here? I guess it's just being safe in case there is something there?
There was a problem hiding this comment.
data will contain text too (and other fields if they're added to the serializer later) but I see now I forgot to do the same when flat_url is true above!
There was a problem hiding this comment.
fixed in 332ed63 - and changed the tests to better check that the output is what we expect (before ShelfSerializer was using CTAField so we partly relied on those tests to cover all the edge cases with the more unusual api gates)
2 participants
fixes mozilla/addons#8424