Feature/cursor based pagination #28
Conversation
WalkthroughSwitches pagination from numeric page parameters to cursor-based pagination across documentation, route handlers, schemas, and tests, adding sqlakeyset-backed cursor handling, a new Cursor schema field, and updating endpoints/tests to accept and return cursor tokens. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
rect rgba(200,230,255,0.35)
Note over Client,API: Initial fetch (no cursor)
Client->>API: GET /products
API->>DB: Query (ordered, limit N)
DB-->>API: Rows[0..N-1] + bookmark_next
API-->>Client: { products: [...], cursor: { next: "b64_next", prev: null } }
end
rect rgba(220,255,220,0.35)
Note over Client,API: Subsequent fetch using cursor
Client->>API: GET /products?cursor=b64_next
API->>API: Decode cursor -> bookmark
API->>DB: Query(from bookmark, limit N)
DB-->>API: Rows[N..2N-1] + bookmark_next/prev
API-->>Client: { products: [...], cursor: { next: "b64_next2", prev: "b64_prev" } }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
piyush-jaiswal
force-pushed
the
feature/cursor-based-pagination
branch
from
b3da074 to
02dca06
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(3 hunks)app/routes/category.py(2 hunks)app/routes/product.py(2 hunks)app/routes/subcategory.py(2 hunks)app/schemas.py(3 hunks)tests/test_product.py(1 hunks)tests/test_relationships.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/routes/subcategory.py (3)
app/routes/category.py (4)
get(47-48)get(89-90)get(145-147)get(158-168)app/routes/product.py (3)
get(55-62)get(102-103)get(160-162)app/models.py (2)
Subcategory(93-110)Product(113-131)
tests/test_relationships.py (1)
tests/conftest.py (1)
client(26-27)
tests/test_product.py (1)
tests/conftest.py (1)
client(26-27)
app/routes/category.py (3)
app/routes/product.py (3)
get(55-62)get(102-103)get(160-162)app/routes/subcategory.py (4)
get(48-49)get(95-96)get(160-162)get(173-178)app/models.py (3)
Category(74-90)Product(113-131)Subcategory(93-110)
app/routes/product.py (3)
app/routes/category.py (4)
get(47-48)get(89-90)get(145-147)get(158-168)app/routes/subcategory.py (4)
get(48-49)get(95-96)get(160-162)get(173-178)app/models.py (1)
Product(113-131)
🪛 LanguageTool
README.md
[grammar] ~13-~13: Use a hyphen to join words.
Context: ...
Paginates result using cursor based pagination when products are fetch...
(QB_NEW_EN_HYPHEN)
🪛 Ruff (0.14.3)
app/schemas.py
11-11: Unused method argument: attr
(ARG002)
11-11: Unused method argument: obj
(ARG002)
11-11: Unused method argument: kwargs
(ARG002)
24-24: Unused method argument: attr
(ARG002)
24-24: Unused method argument: data
(ARG002)
24-24: Unused method argument: kwargs
(ARG002)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
app/routes/subcategory.py (1)
175-178: Nice use of deterministic ordering
Locking the subcategory products query toProduct.id.asc()before callingget_pagekeeps the keyset stable, which is exactly what sqlakeyset expects for consistent paging (dokk.org).README.md (1)
67-70: Doc update aligns with the new cursor contract
Calling out focus-page behaviour and the cursor query param in the README makes it much easier for API consumers to follow the same hop pattern the tests exercise.app/schemas.py (1)
10-34: Cursor field handles both directions cleanly
Serializing the sqlakeyset bookmarks through url-safe Base64 keeps the payload URL-friendly, and surfacing a marshmallowValidationErroron decode failures guarantees bad cursors short-circuit with a 422 response (sqlakeyset.readthedocs.io).app/routes/category.py (1)
163-168: Category products share the same stable ordering
Mirroring the subcategory implementation with an existence check plusProduct.id.asc()ensures this endpoint produces reproducible window slices across requests, matching sqlakeyset’s guidance on unique ordering for keysets (dokk.org).tests/test_product.py (1)
225-226: Test now walks the “next” cursor
Driving the second request withdata1["cursor"]["next"]exercises the entire response contract, so we’ll catch regressions if either side stops emitting/accepting the expected bookmark.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/test_relationships.py (1)
256-256: Remove the stale?page=1query parameter.Line 256 still uses the legacy
?page=1parameter. Since the migration to cursor-based pagination is complete, this parameter is no longer supported and should be removed. The first page should be fetched without any paging query string, consistent with the category test above (Line 221).Apply this diff:
- page1 = self.client.get(f"/subcategories/{subcategory['id']}/products?page=1").get_json() + page1 = self.client.get(f"/subcategories/{subcategory['id']}/products").get_json()
🧹 Nitpick comments (1)
README.md (1)
13-13: Fix hyphenation for compound adjective.Use a hyphen in "cursor-based pagination" as it modifies "pagination".
Apply this diff:
-Paginates result using cursor based pagination when products are fetched by categories, subcategories or themselves. +Paginates result using cursor-based pagination when products are fetched by categories, subcategories or themselves.As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(3 hunks)app/routes/category.py(2 hunks)app/routes/product.py(2 hunks)app/routes/subcategory.py(2 hunks)app/schemas.py(3 hunks)requirements.txt(1 hunks)tests/test_product.py(1 hunks)tests/test_relationships.py(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~13-~13: Use a hyphen to join words.
Context: ...
Paginates result using cursor based pagination when products are fetch...
(QB_NEW_EN_HYPHEN)
🪛 Ruff (0.14.3)
app/schemas.py
11-11: Unused method argument: attr
(ARG002)
11-11: Unused method argument: obj
(ARG002)
11-11: Unused method argument: kwargs
(ARG002)
24-24: Unused method argument: attr
(ARG002)
24-24: Unused method argument: data
(ARG002)
24-24: Unused method argument: kwargs
(ARG002)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (14)
tests/test_product.py (1)
225-226: LGTM!The cursor-based pagination is correctly implemented: the first page is fetched without a cursor parameter, and the second page uses the
nextcursor from the first page's response.app/routes/subcategory.py (2)
5-5: LGTM!The sqlakeyset import enables cursor-based pagination across the codebase.
173-178: LGTM!The cursor-based pagination implementation is correct:
- Orders by
Product.id.asc()for deterministic pagination- Uses
get_pagewith the cursor parameter- Returns both the page object and cursor metadata for client navigation
tests/test_relationships.py (2)
221-223: LGTM!The category products pagination correctly fetches the first page without any paging parameter and then uses the cursor for subsequent pages.
257-258: LGTM!The cursor extraction and subsequent page fetch are correctly implemented.
app/routes/product.py (2)
5-5: LGTM!The sqlakeyset import enables cursor-based pagination for product listings.
55-62: No changes needed — the code correctly serializes Query objects.The test
test_get_product_by_nameexercises exactly this code path and passes. Marshmallow-sqlalchemy'sSQLAlchemyAutoSchemaautomatically handles SQLAlchemy Query objects through iteration when used with aListfield containing aNestedschema. No explicit.all()call is required; line 58's direct return of the Query object is correct and consistent with the framework's design.app/routes/category.py (2)
5-5: LGTM!The sqlakeyset import enables cursor-based pagination for category products.
158-168: LGTM!The cursor-based pagination is correctly implemented:
- Validates category existence before pagination
- Orders by
Product.id.asc()for deterministic results- Uses
get_pagewith the cursor parameter- Returns both the page object and cursor metadata
app/schemas.py (4)
1-5: LGTM!The imports are correctly added to support cursor-based pagination with base64 encoding and sqlakeyset bookmarks.
10-34: LGTM!The Cursor field implementation is well-designed:
- Properly serializes cursors as base64-encoded next/prev tokens
- Deserializes and validates cursor strings with appropriate error handling
- Integrates cleanly with sqlakeyset bookmarks
Note: The static analysis warnings about unused parameters (
attr,obj,kwargs) are false positives—these parameters are required by the marshmallowFieldinterface.
102-102: LGTM!The cursor field is correctly added to
ProductsOutwithrequired=Falseto accommodate the name-based product lookup flow that doesn't use pagination.
133-133: LGTM!The
PaginationArgsschema correctly replaces the page-based parameter with the cursor-based parameter, completing the migration to cursor-based pagination.requirements.txt (1)
12-12: Verification confirms version is valid with no security advisories.The version
2.0.1746777265is a legitimate, published release on PyPI. The large build number follows a timestamp-based versioning scheme used consistently by this package. The security advisory check returned no vulnerabilities for sqlakeyset.No action needed on this dependency.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores