fix(list): the filter panel no longer persists a view overlay over the source-defined filter - #4214
Merged
Merged
Conversation
…e source filter (#4155) Opening a Console list's filter panel and clicking "Add filter" wrote a view-customization overlay into sys_metadata. The row that button inserts is incomplete by construction ({ field, operator: 'equals', value: '' }), and an override is merged key-wise over the source view, so that one stray condition became the view's ENTIRE filter — total: 0 for every user of the view, with the panel's own Clear all unable to undo it (it wrote filter: [], still an override that deletes the declaration). Write half: persistViewFilter and both onFilterChange persist bindings are removed. Filter-panel state is transient — session plus writeListFilterState's per-browser restore. Overlay writes belong to an explicit save only (handleViewConfigSave, ObjectDataPage's "Save as view"); ObjectDataPage had already made this call for the sibling surface (#2251). The fold (foldFilterGroupToSpecRules, objectstack#5159) survives for those explicit paths — a caller was removed, not the fold. Read half: sanitizeViewOverride runs on both loadViewOverrides branches, strips empty-valued conditions in both the spec-rule and legacy-triple shapes, and drops the filter KEY when nothing effective survives, so the source filter wins again. Poisoned installs self-heal on read — no sys_metadata delete, no restart. Empty-state copy: the view's own filter now counts as an active query, so a filtered-to-empty list says "no records match" rather than inviting the user to create their first record — the misread that sent this report's triage to the data and permission layers. The objectstack#5159 ratchet is retargeted, not deleted: no filter reaches the persist path, the persistViewFilter seam does not exist, no onFilterChange handler reaches any persist call, with the explicit-save path as a control. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4155
Diagnosis
The reported symptom — a list permanently empty, the filter panel showing no conditions, recovery only via a
sys_metadatadelete plus a service restart — comes from one automatic write meeting one merge rule.The write.
ObjectView's list toolbar boundonFilterChangetopersistViewFilter, which folded the panel's group and handed it topersistViewPatch. So every emission of the filter panel wrote a view-customization overlay.Add filterinserts a row that is incomplete by construction — a real field,operator: 'equals', andvalue: ''— and it emits immediately, before the user types a value. Merely opening the panel and clicking the button was therefore enough to write.The merge rule. A view override is applied over the source-declared view key-wise (
{ ...source, ...override }). An override carrying afilterkey does not add to the declared filter — it replaces it wholesale. So the one stray condition became the view's entire filter.Why the live screen never warned anyone.
ListView.convertFilterGroupToASTskips conditions whose value isnull/''/[], because[field, '=', '']is a silently-wrong filter (it matches only the empty string) rather than "no filter". The grid therefore ignored the very row that was being persisted. The one condition the screen refused to apply was the one condition that reached storage — which is why nothing looked wrong until the next load, when it came back as the whole filter and returnedtotal: 0.Why Clear all could not undo it. Clearing emitted an empty group, which folded to
[]and persistedfilter: []. That is not a poisoned filter, but it is still an override with an opinion aboutfilter, so it kept winning the spread and kept the declaration deleted. The panel showed no conditions while the request still carried the stale filter — exactly as reported.Why the empty state pointed triage away from the view layer.
ListViewdistinguishes "filtered to empty" from "truly empty (first run)", but the view's own declaredfilterdid not count toward that decision. A list emptied by its filter rendered the first-run copy, so the report read as data loss or a permission problem.What changed, per ruling
Ruling 1 — an end-user filter-panel interaction must not persist an overlay.
persistViewFilteris deleted, along with bothonFilterChangepersist bindings. Filter-panel state is transient: it belongs to the session and towriteListFilterState's per-browser restore. Overlay writes now require an explicit save —handleViewConfigSave, andObjectDataPage's "Save as view".ObjectDataPagehad already made this exact call for the sibling surface ("Deliberately NO onSortChange/onFilterChange persistence hooks", #2251); this surface was the outlier.foldFilterGroupToSpecRules(objectstack#5159) is untouched and still the one dialect for the explicit paths, including the Studio inspector — a caller was removed, not the fold.Ruling 2 — empty-value conditions are never persisted nor applied, and dropping them lets the source filter win again. Two layers. In
foldFilterGroupToSpecRules, a row that wants a value and has none is now dropped, matching the predicate the live query already used — what is not applied is not persisted. On the read path,sanitizeViewOverrideruns on both branches ofloadViewOverridesand strips the same conditions from a stored overlay, in both at-rest shapes (specViewFilterRuleobjects and the legacy runtime triple). When nothing effective survives it drops thefilterkey rather than writing[], which is what lets the merge fall through to the source declaration. Poisoned installs self-heal on the next read — nosys_metadatasurgery, no restart.A value-less operator (
isEmpty/isNulland friends) is complete without a value and is kept. The set is pinned against the FilterBuilder's ownneedsValueInputlist so the two cannot drift apart unnoticed.Ruling 3 — Clear all clears any panel-written overlay. Reached from both ends. Going forward the panel writes no overlay at all, so there is nothing for Clear all to leave behind; its clear is session state, which it still writes. For installs already carrying one, the
filter: []that Clear all previously wrote is now dropped on read by the same sanitizer, so the source filter returns without the user having to do anything.Ruling 4 — filtered-empty copy. The view's own
filternow counts as an active query, in both the array and Mongo-style object shapes, so a filtered-to-empty list says "no records match" instead of inviting the user to create a first record. No new strings: this routes tolist.noMatches/list.noMatchesMessage, which already exist in all ten locales. An author-suppliedemptyState.title/emptyState.messagestill wins over both branches.A deliberate consequence
An overlay can no longer express "this view has NO filter" over a source view that declares one. That is the strictly safer side of the trade: the shape that expressed it is the same shape that silently erased source declarations. An author who genuinely wants no filter edits the view, which writes the view body rather than an overlay.
Verification
Reverse verification (red-first). With the three source files reverted to
origin/mainand the tests kept: 24 failed, 13 passed. Restored: 63 passed. Representative signatures, each naming a value rather than a crash:One prediction was wrong, and the correction is committed. The suite header predicted that the
what must NOT changecontrol block would stay green in both worlds. It does not: reverting removes thesanitizeViewOverrideexport, so the module fails at import and every case in that file goes red, controls included. That bounds what the file can prove againstorigin/main— a missing export cannot tell "kept the real conditions" apart from "module did not load" — so those cases are controls in the forward direction only. The header now records the measured result instead of the prediction, and names the value-quoting failures as the genuinely discriminating ones.Gates (repo root, scoped):
vitest run packages/app-shell/ packages/plugin-list/type-check(both packages)lint(both packages)check-control-bytesBuild closure (
'@object-ui/app-shell^...','@object-ui/plugin-list^...') built first, so the type-check read fresh.d.tsrather than stale artifacts.Changesets: patch for
@object-ui/app-shelland@object-ui/plugin-list.Not fixed here — the cross-user half is platform-side
The card's cross-user angle is real and is not closed by this PR.
updateViewConfigwritesmeta.saveItem('view', viewId, merged)— a single org-wide metadata item keyed by view id, with no per-user scoping. Every key the toolbar still persists by design (sort,hiddenFields,columnState,rowHeight) is therefore written org-wide and applies to every user of the view, even thoughObjectViewdescribes them as "Airtable-style per-view personal config". Removing the filter write takes the destructive key out of that path, but the scoping gap remains for the others, and the client has nowhere to put a per-user overlay. Handed to the seat as evidence per the dispatch lane split; this lane lands no platform code.Generated by Claude Code