Skip to content

Add pagination to /bookmarks#602

Merged
PGijsbers merged 2 commits intodevelopfrom
pagination/bookmarks
Sep 3, 2025
Merged

Add pagination to /bookmarks#602
PGijsbers merged 2 commits intodevelopfrom
pagination/bookmarks

Conversation

@PGijsbers
Copy link
Contributor

@PGijsbers PGijsbers commented Sep 2, 2025

Change(s)

Change Type: Added

Change Category: Interface

Changelog Entry:
Add pagination to /bookmarks

Pagination limits and offsets can now be specified when browsing a user's bookmarks.
Note that the defaults have different behavior, and requesting all bookmarks in v2 now also limits to 10 by default.
Unlike the change to user/resources, we add this to v2 since the endpoint was not in use yet.

How to Test

Covered by tests.

Checklist

  • Tests have been added or updated to reflect the changes, or their absence is explicitly explained.
  • Documentation has been added or updated to reflect the changes, or their absence is explicitly explained.
  • A self-review has been conducted checking:
    • No unintended changes have been committed.
    • The changes in isolation seem reasonable.
    • Anything that may be odd or unintuitive is provided with a GitHub comment explaining it (but consider if this should not be a code comment or in the documentation instead).
  • All CI checks pass before pinging a reviewer, or provide an explanation if they do not.
  • The PR title matches the changelog entry's one-line description.

Related Issues

Closes #599

- id: pytest-check
name: pytest-check
entry: pytest src/tests --versions ''
entry: pytest src/tests --versions 'latest'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also made a few small internal changes to allow us to specify versions in the same was as Version when restricting tests, so we can use fewer magical strings and better parse user input.

Copy link
Contributor Author

@PGijsbers PGijsbers Sep 2, 2025

Choose a reason for hiding this comment

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

It also fixes a bug where the parametrization wouldn't work for any version other than latest (see below).

@PGijsbers PGijsbers requested a review from Taniya-Das September 2, 2025 14:33
client = TestClient(
app,
base_url=f"http://localhost/{request.param.prefix}", # param is Version
base_url=f"http://localhost{request.param.prefix}/", # param is Version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since Version.prefix was e.g., /v3 this lead to localhost//v3 instead of localhost/v3. This wasn't caught by pre-commit since it only runs against latest. I am not sure why CI/CD in the PR (#596) didn't catch it, but in the merge commit of develop it was broken.

Copy link
Collaborator

@Taniya-Das Taniya-Das left a comment

Choose a reason for hiding this comment

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

lgtm

@PGijsbers PGijsbers merged commit 4d6aee7 into develop Sep 3, 2025
4 checks passed
@PGijsbers PGijsbers deleted the pagination/bookmarks branch September 3, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add pagination to user/bookmarks

2 participants