fix: save staged edits for rows loaded via infinite scroll - #1545
Conversation
With infinite scroll enabled, the grid queries pageIndex 0 with a grown pageSize window (25-row batches), but the row mutation hooks resolved their TanStack DB collection from the paginated pageIndex/pageSize. The two query scopes diverge once more than one batch is loaded, so collection.update() targeted a collection missing rows beyond the first batch and threw UpdateKeyNotFoundError, making "Save n rows" fail silently (issue #1466). The mutation hooks (update, updateMany, delete, insert) now receive the view's exact query props and resolve their collection through the new useActiveTableQueryCollection hook, so mutations always target the same collection scope the grid displays — including the search term dimension that was previously omitted as well. Verified end-to-end in the ppg demo: editing and deleting rows beyond the first 25-row batch with infinite scroll enabled now persists and shows the success toast. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 26 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe active table view now creates one memoized query scope and passes it to fetching, selection, insertion, and row mutation hooks. A shared collection hook resolves the active table and collection from that scope, including infinite-scroll batch windows. Update, bulk update, delete, and insert operations use the resolved collection. Tests cover editing a row beyond the first 25-row batch, and documentation describes the mutation-scope rule. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
Compute preview deployed. Branch: |
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
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 `@ui/hooks/use-active-table-update.ts`:
- Around line 12-15: Update the useActiveTableUpdate flow, including its
collection.update transaction, to propagate params.options as
AdapterUpdateOptions when persistence is delegated to the collection. Ensure
callers’ update options remain effective throughout the transaction, or
explicitly remove options from the public API and migrate all callers
consistently.
In `@ui/studio/views/table/ActiveTableView.tsx`:
- Around line 212-240: Align mutation query props with the stable visibleData
during infinite-scroll load-more: do not let loadedInfinitePageCount immediately
redirect mutations to the expanded collection while the prior window is still
displayed. Either atomically update visibleData and mutation props after the
expanded query finishes, or disable save/delete mutations while that query is
fetching. Add coverage for saving during this transition, using
activeTableQueryProps, visibleData, and the affected mutation handlers.
🪄 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
Run ID: 4aa9bf77-df4b-4eaa-9cf6-0fc26da75952
📒 Files selected for processing (12)
.changeset/tidy-lions-tap.mdArchitecture/db-state.mdFEATURES.mdui/hooks/use-active-table-delete.tsui/hooks/use-active-table-insert.tsui/hooks/use-active-table-query.tsui/hooks/use-active-table-update-many.test.tsxui/hooks/use-active-table-update-many.tsui/hooks/use-active-table-update.tsui/hooks/use-selection.test.tsxui/hooks/use-selection.tsui/studio/views/table/ActiveTableView.tsx
- useActiveTableUpdate: drop the ignored AdapterUpdateOptions/table params from UseActiveTableUpdateParams. Persistence is delegated to the rows collection's onUpdate handler, so a per-call options channel could never reach the adapter; removing it makes that explicit instead of silently ignoring the values (API migration; the hook has no product callers). - ActiveTableView: keep row mutations targeting the visible window during infinite-scroll growth. While a grown window is fetching, the grid keeps showing the previous settled window; mutation query props now stay pinned to that settled window and swap to the grown scope atomically together with the rows (new resolveVisibleTableWindow helper + tests), closing the save/delete race during the transition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes #1466
Root cause
With infinite scroll enabled the grid queries
pageIndex: 0with a growing window (pageSize = 25 × loadedBatches), but the row-mutation hooks derived their TanStack DB collection scope independently fromusePagination()'s paginated page state. The scopes coincide for the first 25 rows — exactly why the reporter only saw failures for rows that "wouldn't show on the first page". Beyond that,collection.update()threwUpdateKeyNotFoundErrorwith no error surface, so saving silently did nothing.Fix
ActiveTableViewnow builds one memoized set of query props (infinite-scroll-aware page window, sort, filter, search) shared by the query hook and all mutation hooks (insert/update/updateMany/delete via selection), which resolve their collection through a newuseActiveTableQueryCollection(props)— guaranteeing mutations target exactly the collection the grid displays. Also removes a redundant duplicate table fetch the old hooks triggered. The contract is documented inArchitecture/db-state.md.Verification
🤖 Generated with Claude Code