feat(tables): saved views with filter, sort, and column presets - #5961
feat(tables): saved views with filter, sort, and column presets#5961TheodoreSpeaks wants to merge 44 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview API & data: Table UI: Views dropdown (switch, rename, delete, new view), Columns show/hide menu (including workflow group toggles), Save / Save as view when state diverges from the active view, and Rollout: New Reviewed by Cursor Bugbot for commit 5e5fd7f. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR adds feature-gated saved table views.
Confidence Score: 3/5The PR is not yet safe to merge because a resize or drag completed after a view switch can still persist layout into the destination view. The grid rebinds its persistence sink whenever the active view changes, while resize and drag completion handlers dereference that sink without verifying the owner under which the interaction began, leaving the previously reported cross-view corruption path reachable. Files Needing Attention: apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx | Coordinates active-view selection, live layout snapshots, partial view updates, and create/save workflows. |
| apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx | Adds view-owned layout seeding and persistence across grid interactions. |
| apps/sim/hooks/use-table-undo.ts | Stamps layout-bearing undo entries with view ownership and gates asynchronous layout restoration. |
| apps/sim/lib/table/views/service.ts | Implements validated CRUD operations and database-side partial configuration merging. |
| packages/db/migrations/0273_table_views.sql | Introduces persisted table views with table and workspace relationships. |
Sequence Diagram
sequenceDiagram
participant U as User
participant T as Table UI
participant API as Table Views API
participant DB as table_views
U->>T: Select or edit saved view
T->>API: Create or patch view configuration
API->>DB: Validate and persist configuration
DB-->>API: Return saved view
API-->>T: Refresh active view
T-->>U: Apply filter, sort, visibility, and layout
Reviews (32): Last reviewed commit: "merge: staging into feat/table-views" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
… the persist The undo success callbacks applied the recorded view's order/widths/pinning to the grid unconditionally and only gated the PATCH, so switching views before a column create/delete mutation resolved left the destination displaying the origin view's layout until switched away and back. Each callback now checks ownership where its layout work begins: three are purely layout and return at the top; delete-column undo restores cell data first (row data, runs everywhere) and gates only the layout block below it. In a non-owning view the restored column still appears via the grid's append effect — at the end, leaving that view's layout untouched.
|
@cursor review |
…concile order at seed Two holes closed: The sink binding toggled on activeView, leaving a render-frame gap after the views query settled but before the resolve effect adopted a default — writes in that gap fell through to shared metadata. The sink is now always bound while views are enabled and handlePersistLayout is the single router, reading the owner at call time: unresolved buffers, a view patches the view, All writes metadata. The error branch stamps the owner so post-error writes stop buffering. The append effect only fires when the schema changes, so a column that arrived while another source owned the layout rendered via the displayColumns fallback but was never written into the adopted owner's stored order until a drag happened to heal it. Seeding now reconciles the incoming order against the schema and persists the appended tail through the current sink.
|
@cursor review |
Insert-column and the delete chain persist layout from mutation callbacks through updateMetadataRef, which always targets the current sink — so a view switch mid-flight wrote the origin view's order/widths/pins into the destination. Undo got this guard already; the live paths never did. Both now capture viewLayoutKey at dispatch and compare at the callback. On a mismatch the layout work is skipped: the destination re-seeded its own layout, the new column lands there via the append effect when the refetch arrives, and the deleted column's dangling keys are pruned on read. pushUndo also takes the captured owner as an override — stamped at callback time it would record the destination, letting a later undo apply the origin's layout to it.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1576c41. Configure here.
A failed background refetch flips isError while the cached list stays usable, and every view mutation invalidates the views query — so one blip made the resolve effect treat views as terminally failed and stop applying switches until the next successful refetch. The axis is now whether a list exists: error with no list ever fetched settles to All; error with a cached list resolves normally against the cache; owner is unknown only while the initial fetch is in flight.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b49a061. Configure here.
Route-count baseline conflict: staging moved to 984 on its own; merged truth with this branch's 2 view routes is 986, confirmed by running the audit.
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5e5fd7f. Configure here.
| layoutTouchedWhileUnownedRef.current = false | ||
| if (adoptedView || !userPermissions.canEdit) return | ||
| updateMetadataMutation.mutate(readLayout()) | ||
| }, |
There was a problem hiding this comment.
Load-window flush wipes pins
Medium Severity
resolvePendingLayout persists a full readLayout() snapshot when views settle to All. That snapshot always includes pinnedColumns, which stays [] until the grid seeds from table metadata. If views finish before the detail query and the user resized during the unowned window, the flush merges empty pins (and a partial columnWidths map) over the stored metadata and drops existing pins/widths.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5e5fd7f. Configure here.
| updateMetadataMutation.mutate(patch) | ||
| }, | ||
| [activeView, userPermissions.canEdit, viewOwnerUnknown] | ||
| ) |
There was a problem hiding this comment.
Layout writes misrouted after view adoption
Medium Severity
handlePersistLayout routes by activeView, but buffering only covers seededViewIdRef === undefined. After default adoption or create success stamps seededViewIdRef and writes the view id through the URL, activeView still lags a render — so layout patches fall through to shared table metadata instead of the adopted view. The comment already notes that lag; the guard does not cover it.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 5e5fd7f. Configure here.
1 participant
Summary
Columnsmenu to show/hide columns, with workflow outputs nested under their group (hiding all of a group's children drops its spanning header)table_viewstable; "All" is the absence of a view, so no backfill and the table is always reachable unfilteredtable-viewsfeature flag (UI only — routes and migration ship ungated so a view saved during rollout survives the flag being turned off)Type of Change
Testing
Tested manually.
bun run type-check,bun run lint, all 11 CI audits, andcheck:migrations origin/stagingpass. 29 unit tests across the views service, API contracts, and feature flags.Checklist