Skip to content

Conversation

@georgepyne
Copy link
Collaborator

@georgepyne georgepyne commented Nov 8, 2025

  • Added OffsetPaginationExtension that exposes validated GET/POST parameters so OpenAPI advertises offset and callers get consistent validation
  • Offset handling is achieved through request handling and paging state: GET handler now accepts the query argument and the POST model persists it. SearchHandler seeds QueryInfo with the caller-specified offsets and only returns “previous” links when the offset is greater than 0.

@georgepyne georgepyne changed the title feat(#194): add offset pagination support [#194]: Support offset in Search requests Nov 8, 2025
@georgepyne georgepyne marked this pull request as ready for review November 12, 2025 18:57
Copy link
Collaborator

@captaincoordinates captaincoordinates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks great, just a couple minor comments - one of which you're welcome to disregard.

assert next_result["features"][0]["id"] == _all_items[offset + limit]["id"]


def test_post_search_offset_exceeds_total() -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this test should be replicated in the GET search test module?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update the tests to replicate with GET search also

)

def _get_offset_from_request(self) -> Optional[int]:
offset_value = cast(Optional[int], getattr(self.search_request, "offset", None))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a slightly different approach could communicate intent more clearly, e.g.

offset_value = cast(POSTOffsetPagination, self.search_request).offset

but both approaches get the job done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants