refactor: Migrate SeriesTable to the shared table state hook - #404
Conversation
PR 2 of the #366 cut (#394). SeriesTable now calls useTableState in place (it renders its own bulk bar, per #359) with the URL store injected from tableUrlStore.ts (#377), selectAllScope: "page" (#382), and pagination handed to TanStack's autoResetPageIndex (#360). - The page-clamp effect is deleted; the clamp is render-time and display-only, composed over the URL store in the caller's adapter. An out-of-range ?page=99 renders the last page and keeps its URL — the one assertion #381 pinned expressly so this change would surface here, inverted in this PR and nowhere else. - search folds into the shared params map under the same key, replacing the deprecated throttleMs: 300 with limitUrlUpdates: throttle(300). - Six hand-rolled setRowSelection({}) calls become clearSelection(); the hook owns selection pruning and the deep-link auto-reset arming. - confirmDelete derives from the selection state object it was armed against, replacing the reset that lived in onRowSelectionChange. - SeriesTable's line leaves the ESLint ratchet allowlist. Suite: 29 files / 173 tests green; SeriesTable.test.tsx untouched except the pinned inversion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCo8dyZq6YvkQmvK4zNjyg
🦋 Changeset detectedLatest commit: b406789 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesSeriesTable shared state migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🤖 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 `@frontend/src/components/series/SeriesTable.tsx`:
- Around line 130-137: Update the pagination clamp in SeriesTable’s
pageIndex/maxPage logic to calculate the maximum display page from the globally
search-filtered rows, using the same filtering behavior as useTableState. Keep
the raw URL pageIndex unchanged in urlStore, and preserve the existing state
override so deep links with a search term cannot render an empty page.
🪄 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: CHILL
Plan: Pro Plus
Run ID: e9119f2a-1327-4cfe-876e-c6b3d5bc5b2b
📒 Files selected for processing (4)
.changeset/library-out-of-range-page-url-kept.mdfrontend/eslint.config.jsfrontend/src/components/series/SeriesTable.tsxfrontend/tests/components/SeriesTable.test.tsx
💤 Files with no reviewable changes (1)
- frontend/eslint.config.js
CodeRabbit finding on #404: the display clamp counted only the domain-filtered rows, but the global search runs inside useTableState — so ?page=99&search=<single match> clamped to the last unsearched page and rendered an empty table. The clamp now reproduces TanStack's includesString over the same four accessor columns, and a new test pins the searched out-of-range deep link (verified red against the domain-only clamp). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCo8dyZq6YvkQmvK4zNjyg
1 participant
PR 2 of the four-PR migration cut by wayfinder #366. Closes #394; part of the #353 map.
SeriesTablemigrates touseTableStatein place — it renders its own bulk bar, so per #359 it calls the hook itself;SeriesListPageand the component's props are untouched.What changed
useTableUrlStorebackssorting/globalFilter/pageIndex; the component keeps a domain-only nuqs map (type,progress,status,view).searchfolds into the shared map under the same key — bookmarks survive — and the deprecatedthrottleMs: 300becomes the per-parserlimitUrlUpdates: throttle(300).store.state.pageIndexstraight to TanStack, which renders an empty page for an index past the end). Page resets are TanStack's ownautoResetPageIndex, armed one render after rows appear — Ship the deep-linked-page pre-fix on SeriesTable #381's line transfers verbatim.selectAllScope: "page"— the last page-scoped site (Decide what happens to the orphaned IssuesTable #382); the header checkbox goes through the hook's scope-aware helpers.setRowSelection({})→clearSelection(), including the two on the list/grid view toggle that Decide the hook's surface area and the selection ownership model #359's inventory missed. The hook owns selection pruning (Pre-fix SeriesTable's filtered-out selection #390's pre-fix behaviour, now centralised).confirmDeletederives from the selection state object it was armed against — any selection change disarms it, reproducing the reset that lived inonRowSelectionChange(which the hook now owns) without asetState-in-effect.SeriesTable.tsxleaves the allowlist; three files remain (Migrate WantedTable and UpcomingTable to useTableState #395, Migrate ImportTable to useTableState #396).Parity — the invariant, old and new side by side
SeriesTable.test.tsxrenders the same<SeriesTable data={…} />interface, so its four table-state tests and #390's selection tests pass verbatim — the file is untouched except the one assertion #381 pinned expressly so this PR would surface the change rather than hide it.Invariant: an out-of-range deep link never renders an empty table. That half is unchanged —
?page=99over 63 rows still renders the last real page. The URL half inverts, because the guarded clamp effect that rewrote the URL is deleted:// 63 rows at 20 a page is pages 0–3, so the last page is Series 61–63. expect(screen.getByText("Series 61")).toBeTruthy(); - expect(new URLSearchParams(window.location.search).get("page")).toBe("3"); + expect(new URLSearchParams(window.location.search).get("page")).toBe("99");The rewrite could not tell "rows have not arrived" from "genuinely out of range" — that indistinguishability was the deep-link bug (#372, #381) — so the URL now stays exactly as the user supplied it.
Changeset
patch— the migration's only one (#366, per #386's operator-observable rule): the out-of-range URL rewrite is operator-visible, and #381 shipped the equivalent change aspatch.Suite: 29 files / 173 tests green;
lint:modern,lint:backend,lint:generated,lint:frontendandtscall pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01PCo8dyZq6YvkQmvK4zNjyg
Summary by CodeRabbit
Bug Fixes
Tests