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

`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`).

Author: sagargg

datastore/services/read.py

@@ -226,19 +226,14 @@ def _build_pagination_links(
     Counter keys (added alongside the URL keys, 1-indexed):
       - ``page_size``   — rows per page = ``limit``; emitted whenever
         ``limit > 0`` (a UI can always render it, even on empty pages).
-      - ``page``        — current page = ``offset // limit + 1``.
-      - ``total_pages`` — ``ceil(total / limit)``; omitted when total
-        is unknown.
-
-    ``page`` and ``total_pages`` are dropped whenever the current page
-    has no rows — either because the resource is empty
-    (``total == 0``) or because the caller paged past the end
-    (``offset >= total > 0``). Reporting ``page=5 / total_pages=4``
-    would be incoherent (no such page exists); the absence + the
-    empty `records` array + `prev` are what let a UI recover. When
-    ``total`` is unknown (caller didn't request `include_total`) we
-    keep ``page`` since position is still meaningful for single-page
-    pickers.
+      - ``page``        — current page = ``offset // limit + 1``, or
+        ``null`` when the current page has no rows (empty resource or
+        ``offset >= total``). Reporting ``page=5 / total_pages=4``
+        would be incoherent, so we emit explicit `null` instead — UI
+        can distinguish "no current page" from "field missing".
+      - ``total_pages`` — ``ceil(total / limit)``, or ``null`` in the
+        same no-rows case. Omitted entirely when ``total`` is unknown
+        (``include_total=False``) so we don't fabricate a count.
 
     All non-`offset` query params ride along on every emitted URL.
 
@@ -269,16 +264,18 @@ def _qs(pairs: list[tuple[str, str]]) -> str:
         out["next"] = _qs(base_pairs + [("offset", str(offset + limit))])
     if limit > 0:
         out["page_size"] = limit
-    # Drop `page` / `total_pages` whenever the current page has no
-    # rows: empty resource or past-end pagination. `total is None`
-    # means "unknown → assume there might be rows" so we still emit
-    # `page`.
+    # `total is None` → unknown, assume there might be rows → real ints.
+    # Empty resource / past-end → emit explicit `null` so clients can
+    # distinguish "no current page exists" from "field forgotten".
     has_rows_on_page = total is None or (total > 0 and offset < total)
-    if limit > 0 and has_rows_on_page:
-        out["page"] = offset // limit + 1
-        if total is not None:
-            # ceil division without importing math
-            out["total_pages"] = (total + limit - 1) // limit
+    if limit > 0:
+        if has_rows_on_page:
+            out["page"] = offset // limit + 1
+            if total is not None:
+                out["total_pages"] = (total + limit - 1) // limit  # ceil div
+        elif total is not None:
+            out["page"] = None
+            out["total_pages"] = None
     return out
 
 
@@ -323,8 +320,12 @@ def _link_for(target_offset: int) -> str:
     if limit > 0:
         out["page_size"] = limit
     has_rows_on_page = total is None or (total > 0 and offset < total)
-    if limit > 0 and has_rows_on_page:
-        out["page"] = offset // limit + 1
-        if total is not None:
-            out["total_pages"] = (total + limit - 1) // limit
+    if limit > 0:
+        if has_rows_on_page:
+            out["page"] = offset // limit + 1
+            if total is not None:
+                out["total_pages"] = (total + limit - 1) // limit
+        elif total is not None:
+            out["page"] = None
+            out["total_pages"] = None
     return out

tests/test_datastore_search.py

@@ -492,19 +492,21 @@ def test_csv_quotes_values_with_special_chars(
 
 
 def test_search_objects_response_includes_links(client: TestClient) -> None:
-    """Empty-table case (placeholder engine, total=0): only `start` is
-    emitted — `next` / `prev` don't apply and the page counters are
-    suppressed when there's nothing to page through. Scheme + host
-    carried from the request (TestClient uses `http://testserver`)."""
+    """Empty-table case (placeholder engine, total=0): `start` and
+    `page_size` are emitted; `page` / `total_pages` come through as
+    explicit `null` so clients can tell "no current page" from "field
+    missing". `next` / `prev` don't apply."""
     response = client.get(SEARCH_URL, params=_params())
 
     assert response.status_code == 200
     links = response.json()["result"]["_links"]
-    assert set(links) == {"start", "page_size"}
+    assert set(links) == {"start", "page_size", "page", "total_pages"}
     assert links["start"].startswith("http://testserver/api/3/action/datastore_search")
     assert "offset" not in links["start"]
     assert f"resource_id={_RESOURCE_ID}" in links["start"]
     assert links["page_size"] == 100  # default limit
+    assert links["page"] is None
+    assert links["total_pages"] is None
 
 
 def test_search_links_prev_emitted_on_inner_page(client: TestClient) -> None:
@@ -537,17 +539,20 @@ def test_search_links_preserve_other_query_params(client: TestClient) -> None:
     links = response.json()["result"]["_links"]
     for v in links.values():
         if not isinstance(v, str):
-            continue  # `page` / `total_pages` are ints
+            continue  # `page_size` is int; `page`/`total_pages` may be null
         assert "filters=" in v
         assert "sort=" in v
         assert "fields=" in v
 
 
 def test_search_lists_format_also_includes_links(client: TestClient) -> None:
     """`records_format=lists` is still a JSON envelope, so `_links` is
-    present (placeholder engine, empty table: `start` + `page_size`)."""
+    present (placeholder engine, empty table: `start` + `page_size` +
+    null page counters)."""
     response = client.get(SEARCH_URL, params=_params(records_format="lists"))
 
     assert response.status_code == 200
     links = response.json()["result"]["_links"]
-    assert set(links) == {"start", "page_size"}
+    assert set(links) == {"start", "page_size", "page", "total_pages"}
+    assert links["page"] is None
+    assert links["total_pages"] is None

tests/test_read_service.py

@@ -153,14 +153,17 @@ def test_tsv_records_string_is_empty_when_engine_yields_no_rows() -> None:
 
 
 def test_links_present_on_every_format() -> None:
-    """Placeholder engine returns total=0 → no rows, so `page` and
-    `total_pages` are suppressed. `page_size` is always present when
-    `limit > 0` since a UI can render it even on an empty page."""
+    """Placeholder engine returns total=0 → no rows on the page.
+    `page` / `total_pages` come through as explicit `null` so clients
+    can distinguish "no current page" from "field missing".
+    `page_size` is always present when `limit > 0`."""
     for fmt in ("objects", "lists", "csv", "tsv"):
         body = _call(data_dict_overrides={"records_format": fmt})
         links = body["result"]["_links"]
-        assert set(links) == {"start", "page_size"}, fmt
+        assert set(links) == {"start", "page_size", "page", "total_pages"}, fmt
         assert links["page_size"] == 100  # default limit
+        assert links["page"] is None
+        assert links["total_pages"] is None
 
 
 # --- _build_pagination_links: URL surgery + presence rules -----------------
@@ -258,27 +261,27 @@ def test_links_omit_prev_at_first_page() -> None:
     assert "next" in links
 
 
-def test_links_drop_page_counters_on_empty_resource() -> None:
-    """No rows on the current page (empty resource) → suppress `page`
-    and `total_pages`. `page_size` rides along since a UI can still
-    render it; `start` is the only meaningful nav URL."""
+def test_links_null_page_counters_on_empty_resource() -> None:
+    """Empty resource → `page` / `total_pages` are explicit `null`
+    (not omitted). `page_size` and `start` are present as usual."""
     links = _build_pagination_links(
         "/path", limit=10, offset=0, total=0,
     )
-    assert set(links) == {"start", "page_size"}
+    assert set(links) == {"start", "page_size", "page", "total_pages"}
     assert links["page_size"] == 10
+    assert links["page"] is None
+    assert links["total_pages"] is None
 
 
-def test_links_drop_page_counters_when_offset_past_total() -> None:
-    """Caller paged past the end (`offset >= total`) → counters lie
-    about position, so they're dropped. `prev` remains so the UI can
-    walk back to a real page; the empty `records` array signals the
-    overshoot."""
+def test_links_null_page_counters_when_offset_past_total() -> None:
+    """Caller paged past the end (`offset >= total`) → counters would
+    lie about position, so they're emitted as explicit `null`. `prev`
+    remains so the UI can walk back to a real page."""
     links = _build_pagination_links(
         "/path", limit=100, offset=400, total=302,
     )
-    assert "page" not in links
-    assert "total_pages" not in links
+    assert links["page"] is None
+    assert links["total_pages"] is None
     assert "prev" in links  # offset > 0
     assert "next" not in links  # nothing past the end