fix(helpers): keep a pagination zero in as_page - #113
CaptainAni187 wants to merge 1 commit into
Conversation
The total_count and total_pages candidates were chained with `or`, so a real 0 was skipped for the next candidate and an honestly empty page came back with total_count None, which reads as "the server did not say". Pick the first candidate that is not None instead, which is what the has_more line beside it already did.
|
crim doesn't review pull requests automatically here. Comment |
|
crim review |
|
crim is reviewing this pull request. Findings will be posted shortly. |
Good to mergeThe bug where an honestly empty page pretended it had no idea how empty it was is now fixed, and |
There was a problem hiding this comment.
LGTM
One-sentence assessment: A correct and well-scoped fix that stops as_page from discarding legitimate zero/false pagination values.
What this PR does
Replaces the a or b or ... truthiness chains in as_page with a _first_present helper that selects the first non-None value, so a server that honestly reports total_count: 0, total_pages: 0, or has_more: false no longer has those collapsed to None. The has_more rewrite is behaviorally equivalent to the prior is not None ternary, and precedence (flat fields before nested pagination) is preserved. Tests cover empty-page zeros, nested-pagination zeros, and flat-over-nested precedence.
Findings
No issues found.
as_pagepicks the pagination fields out of whichever key the endpoint used, and the candidates are chained withor:orskips a falsy value, and0is a perfectly real answer here. A page the server honestly reports as empty loses its zeros:total_count=Nonemeans "the server did not tell us", so a caller that pages until it has seentotal_countitems, or that renders "0 of N", cannot tell an empty result from a missing field.It is also inconsistent depending on where the zero sits in the chain, because the last term of an
oris returned whether or not it is truthy:Same payload, same kind of zero, one survives and one does not.
has_moreis already correct, and it is the line directly underneath:That explicit
is not Noneis there becauseFalsewould hit the same trap. The numeric fields just never got the same treatment.The change
A small
_first_presenthelper that returns the first candidate which is not None, used for all three fields. Precedence is unchanged, so a flat field still wins over the nestedpaginationobject whether or not it happens to be truthy.has_moremoves onto the helper too, which is the same behaviour it had, just no longer spelled out inline.Tests
Three added to
tests/custom/test_as_page_envelope.py: the flat zeros, the nested zeros, and one pinning that a flat0still beats a nested7so the fix cannot quietly reorder precedence. All three fail onmain. The existing seven are untouched and still pass._envelope.pyis.fernignored, so a regen keeps this.