Persist document list pagination across navigation

[Pallavikumarimdb]

Description

Resolves an issue where changing the page limit/pagination preference on the document list was lost when navigating away and returning to the page.

Changes

• Client pagination hook: Added a createEffect to createParamSynchronizedPagination that automatically synchronizes active URL parameters with non-default stored page limits/indices from localStorage on mount or navigation.
• Unit tests: Added a test suite to verify default parameter states, prioritization of URL parameters, and fallback/updating behavior.

Closes: #1085

Summary by CodeRabbit

• New Features
  • Pagination settings (especially page size) are now persisted and kept in sync between the URL and browser storage on key pages, so your preferences carry over when you return.
• Bug Fixes
  • Pagination parameter parsing is stricter: invalid values no longer override stored defaults, and missing URL values are automatically filled in.
• Tests
  • Added automated coverage for pagination syncing, storage fallback behavior, and URL parameter updates.

[coderabbitai]

📝 Walkthrough

Walkthrough

createParamSynchronizedPagination now accepts an optional localStorageKey, reads and writes pageSize through localStorage, and fills missing URL pagination params from derived state. Three pages now pass page-specific keys built with buildLocalStorageKey.

Changes

Pagination localStorage persistence

Layer / File(s)	Summary
createParamSynchronizedPagination localStorage support apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts, apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.test.ts	Adds localStorageKey?: string, updates pagination parsing and persistence, and normalizes missing URL params with createEffect; tests cover defaults, precedence, fallback, invalid values, and setter side effects.
Page-level wiring of localStorageKey apps/papra-client/src/modules/document-views/pages/document-view.page.tsx, apps/papra-client/src/modules/documents/pages/documents.page.tsx, apps/papra-client/src/modules/organizations/pages/organization.page.tsx	Each page imports buildLocalStorageKey and passes a page-specific key to createParamSynchronizedPagination.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A bunny stored page sizes in a snug little den,
Hopped back and the list remembered again.
URL and local cache now dance in a pair,
With keys for each page kept tidy and fair.
🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	DocumentViewPage and OrganizationPage pagination persistence were added even though the linked issue only requests document list persistence.	Remove or split out the unrelated page updates, or link them to separate requirements, and keep this PR focused on documents pagination plus the shared helper.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The documents page and shared pagination hook changes satisfy the requested persistence across navigation for the document list.
Title check	✅ Passed	The title clearly summarizes the main change: preserving document list pagination across navigation.
Description check	✅ Passed	The description directly explains the pagination persistence fix and the related tests.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.test.ts`:
- Around line 62-72: The pagination fallback test verifies localStorage loading,
but it does not assert the mount-time URL synchronization introduced by
`createParamSynchronizedPagination`. Update this test to also expect the initial
`setSearchParams(..., { replace: true })` call when `pageSize` is missing from
the URL, using the same `createRoot` setup and `getPagination` flow so the
regression is covered.

In
`@apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts`:
- Around line 26-32: Reject invalid persisted page sizes in
query-synchronized-pagination by tightening the localStorage restore logic: in
the pagination initialization path, only accept values that are positive
integers instead of any numeric string, and ignore/reset anything like 0,
negatives, fractions, or Infinity. Update the validation around the stored value
used to set initialPageSize so that getPagination and downstream callers such as
fetchOrganizationDocuments never receive a bad pageSize from stale storage. Also
apply the same validation anywhere the persisted page size is read or written so
the URL and storage stay in sync with only valid values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 47c262b0-4d92-473a-b9de-521ee3773c9b

📥 Commits

Reviewing files that changed from the base of the PR and between dae7d99 and 984076f.

📒 Files selected for processing (5)

• apps/papra-client/src/modules/document-views/pages/document-view.page.tsx
• apps/papra-client/src/modules/documents/pages/documents.page.tsx
• apps/papra-client/src/modules/organizations/pages/organization.page.tsx
• apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.test.ts
• apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts (1)

61-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize invalid query params too, not just missing ones.

When page or pageSize is present but invalid (for example ?pageSize=-100), getPagination() falls back to storage/defaults, but this effect will not repair the URL because it only checks === undefined. That leaves the address bar out of sync with the active pagination state and keeps the bad value around on refresh/share/back-navigation. Treat invalid params the same as missing ones when deciding whether to write nextParams.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts`
around lines 61 - 80, The pagination sync effect in
query-synchronized-pagination.ts only repairs missing query params, so invalid
values like negative or non-numeric page/pageSize can remain in the URL while
getPagination() falls back to defaults. Update the createEffect block to
validate the current searchParams values for pageIndexParamName and
pageSizeParamName, and treat invalid values the same as undefined when building
nextParams. Keep the fix localized to the createEffect/getPagination flow so
setSearchParams still rewrites the URL with the normalized pagination state.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In
`@apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts`:
- Around line 61-80: The pagination sync effect in
query-synchronized-pagination.ts only repairs missing query params, so invalid
values like negative or non-numeric page/pageSize can remain in the URL while
getPagination() falls back to defaults. Update the createEffect block to
validate the current searchParams values for pageIndexParamName and
pageSizeParamName, and treat invalid values the same as undefined when building
nextParams. Keep the fix localized to the createEffect/getPagination flow so
setSearchParams still rewrites the URL with the normalized pagination state.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 76306d02-6f01-471d-8563-1db969b75194

📥 Commits

Reviewing files that changed from the base of the PR and between 984076f and 7c4c6b8.

📒 Files selected for processing (2)

• apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.test.ts
• apps/papra-client/src/modules/shared/pagination/query-synchronized-pagination.ts

[cla-signer]

Contributor License Agreement

✅ Everyone who contributed to this pull request has signed the CLA for it.