Skip to content

Feat (search): emit null page / total_pages when no rows on current page#7

Merged
sagargg merged 1 commit into
mainfrom
feat/search-null-page-counters
May 28, 2026
Merged

Feat (search): emit null page / total_pages when no rows on current page#7
sagargg merged 1 commit into
mainfrom
feat/search-null-page-counters

Conversation

@sagargg
Copy link
Copy Markdown
Member

@sagargg sagargg commented May 28, 2026

Summary

  • datastore_search / datastore_search_sql previously omitted page and total_pages from _links whenever the current page had no rows (empty resource, or offset >= total).
  • Clients couldn't distinguish "field forgotten" from "no current page exists" — every consumer had to special-case the missing key.
  • Now both keys are emitted as explicit JSON null in those cases. Ints when the page has rows. Key stays omitted only when total is genuinely unknown (include_total=false) so we don't fabricate a count.

Response shape

// Past-end (offset=400, total=302) or empty resource (total=0)
"_links": {
  "start": "...",
  "page_size": 100,
  "page": null,        // was missing
  "total_pages": null  // was missing
}

// Real page — unchanged
"_links": {
  "start": "...", "prev": "...", "next": "...",
  "page_size": 20, "page": 2, "total_pages": 5
}

Test plan

  • Updated _build_pagination_links + _build_sql_pagination_links in datastore/services/read.py.
  • Updated unit tests in tests/test_read_service.py (renamed *_drop_page_counters_**_null_page_counters_*).
  • Updated end-to-end tests in tests/test_datastore_search.py.
  • Full suite passes: 329 tests, ruff clean.

Summary by CodeRabbit

  • Bug Fixes
    • Improved pagination metadata consistency: API responses now explicitly include page and total_pages fields in pagination metadata. These fields are returned as null when unavailable (empty resources or past-end pagination) rather than being omitted, ensuring more predictable response structures.

Review Change Stack

`datastore_search` / `datastore_search_sql` previously omitted `page`
and `total_pages` from `_links` whenever the current page had no rows
(empty resource or `offset >= total`). Clients can't distinguish a
forgotten field from "no current page exists". Emit explicit `null`
in those cases; ints when there are rows; key still omitted only
when `total` is genuinely unknown (`include_total=false`).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1a1fdcec-4c41-49e8-8f6a-257e58d04cb2

📥 Commits

Reviewing files that changed from the base of the PR and between 75e4838 and c498c28.

📒 Files selected for processing (3)
  • datastore/services/read.py
  • tests/test_datastore_search.py
  • tests/test_read_service.py

📝 Walkthrough

Walkthrough

The PR updates pagination metadata handling in the datastore read service. When a page is empty and the total count is known, page and total_pages fields are now explicitly emitted as null values instead of being omitted from the _links envelope. This change applies to both generic (_build_pagination_links) and SQL-specific (_build_sql_pagination_links) pagination builders, with corresponding updates to unit and E2E test expectations.

Changes

Pagination Metadata Consistency

Layer / File(s) Summary
Pagination counter logic and documentation
datastore/services/read.py
_build_pagination_links and _build_sql_pagination_links now emit page and total_pages as explicit null when the current page has no rows and total is known, replacing previous omission behavior. Documentation is updated to reflect the new semantics.
Unit test expectations for pagination behavior
tests/test_read_service.py
test_links_present_on_every_format and pagination link tests are updated to assert page and total_pages are always present in _links as explicit None values for empty resources and overshot pagination, rather than being suppressed.
E2E test expectations for datastore search links
tests/test_datastore_search.py
test_search_objects_response_includes_links, test_search_links_preserve_other_query_params, and test_search_lists_format_also_includes_links are updated to expect page/total_pages with null values in empty-table scenarios and to handle non-string link field values.

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 Counters now shine so bright,
Even empty pages show null light,
Links complete, no fields hidden away,
Pagination speaks truth every day!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/search-null-page-counters

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sagargg sagargg merged commit 5f70412 into main May 28, 2026
1 of 2 checks passed
@sagargg sagargg deleted the feat/search-null-page-counters branch May 28, 2026 11:38
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.

1 participant