Skip to content

Commit 35ee2ad

Browse files
fix(app-shell): "Add filter" in the Studio list-view inspector adds a row that can be edited (#26821)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbU2U5PMGb7vt7eH4fiuXwNj)_ Fixes #26738 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:Studio 列表视图检查器(以及标签页预设)里的筛选器控件,现在自己保存「还没填完」的条件行。点「Add filter」会出现一行可编辑的条件;填完之后才把这条规则写进视图。 - **为什么改**:控件以前每次渲染都从已保存的规则重新生成行,而保存端的折叠(`foldFilterGroupToSpecRules`)会正确地丢掉未填完的行,于是新加的空行在下一次渲染就消失了,Studio 里根本无法给列表视图加筛选。 - **风险与回滚**:低。只改 app-shell 一个控件的状态持有方式;保存规则不变(未填完的条件仍然不会写进视图),不新增任何元数据键,spec 不动。回滚即 revert 本 PR。 - **先看**:`ui/packages/app-shell/src/views/metadata-admin/widgets.tsx` 里 `FilterBuilderField` 的 `draft` / `seededFrom` 两个状态和 `handle` 里新增的「与已存规则相同则不写」判断;新测试 `widgets.filterBuilderHeldRow-26738.test.tsx`。 - **两分钟验证**:`cd ui && pnpm exec vitest run packages/app-shell/src/views/metadata-admin/widgets.filterBuilderHeldRow-26738.test.tsx`,预期 4 个用例全绿;或在 Studio Interfaces 打开某对象的列表视图检查器,Filter →「Add filter」,出现一行,填入值后视图草稿里出现该 filter。 - **需要你做的**:无,席位落地。 ## What changes `FilterBuilderField` (the `filter-builder` widget the Studio list-view inspector's Filter row uses, and the per-tab filter of `filter-mode` tab presets) built the `FilterBuilder`'s row group from the stored rules on every render. The builder's "Add filter" inserts a blank row; the save-side fold drops a row that is not complete yet, as it must; the host stored the unchanged rules and re-rendered, and the re-derived group replaced the builder's rows, so the new row disappeared before it could be edited. The widget now holds the builder's row group in its own state: - every builder change is folded; a refused fold (OR over several rows, a nested group) still toasts and writes nothing; - an accepted fold updates the held rows, and writes only when the folded rules differ from what is stored. An incomplete row therefore stays on screen and writes nothing; completing it writes exactly the complete rule; - when the stored rules change and are not the echo of this builder's own write (another editor, a reload), the held rows are re-seeded from what is stored. The comparison keys on the rules' JSON, never on an array's identity. `viewFilterFold.ts` is unchanged: what is persisted is decided by the same fold as before. ## Tests New `widgets.filterBuilderHeldRow-26738.test.tsx` mounts the real `filter-builder` widget in a host that writes the way `ObjectListViewInspector` does (a new body object on every `onChange`, `filter` dropped when empty): - "Add filter" adds an editable row, writes nothing while it is incomplete, and completing it writes `[{ field: 'title', operator: 'equals', value: 'open' }]` with the row still on screen; - a second, incomplete row is held beside a stored rule without a write; - a stored filter shows as its row and summary, opening it writes nothing, and editing the value away and back writes the stored rule unchanged (round-trip); - CONTROL: a stored filter changed from outside re-seeds the rows. Reverse verification (one-off, not kept): with the fix committed, `widgets.tsx` restored to base `c8583f80` in the working tree only (`groupOfStoredRules` occurrences 3 → 0) made the new file red, Tests 3 failed | 1 passed (4); each red is the vanished row ("expected [] to have a length of 1 but got +0"). The round-trip case is green on base too, as a preservation pin should be. Restored with `git checkout HEAD -- …` (occurrences back to 3, `git diff HEAD` empty). ## Acceptance notes - The card suspected a regression from the filter-fold work of #26306 / #26348. Those changed the builder's value types, not how this widget feeds the builder; the re-derived group predates them in the code read here. History was not walked (shallow checkout), so when it started is not measured. - The inspector's write callback rebuilds the view body on every `onChange`, so before this change an incomplete row would also have written an unchanged body back to the draft. The widget now writes only when the folded rules change. - The popover's held rows live as long as the widget is mounted, so closing and reopening the popover keeps an incomplete row ("held until complete"). ## Verification All readings at head `5221c34d` (base `c8583f80`), under `scripts/pm/os-verify-lock.sh`. <details><summary>Touched package: @object-ui/app-shell</summary> - Dependency closure: `pnpm exec turbo run build --concurrency=2 --filter='@object-ui/app-shell^...'`, 63 tasks, VERDICT command-exit 0. - `pnpm --filter @object-ui/app-shell run type-check` (`tsc --noEmit`, then `tsc -p tsconfig.test.json`): VERDICT command-exit 0, zero TS errors. - New test file: Test Files 1 passed (1), Tests 4 passed (4). - Narrowed app-shell run: every app-shell test file naming the metadata-admin `widgets` module, `filter-builder-trigger`, `FilterBuilderWidget`, `viewFilterFold`, `ObjectListViewInspector`, `FilterModeWidget` or `filter-mode` (52 files): Test Files 52 passed (52), Tests 754 passed (754). The rest of app-shell is left to CI's Test UI shards. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 commands at `5221c34d`; each ran with its exit code captured before any pipe, plus `pnpm check:changeset-fixed` (its roster sits under `.changeset/`). `--ran` reconciliation: "34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED". - `node scripts/check-adr-0087-registration.mjs --base origin/main` → exit 0 - `node scripts/check-adr-0087-registration.mjs --self-test` → exit 0 - `node scripts/check-changeset-no-major.mjs --base origin/main` → exit 0 - `node scripts/check-closing-keyword-parity.mjs` → exit 0 - `node scripts/check-closing-keyword-parity.mjs --self-test` → exit 0 - `node scripts/check-comment-mask-corpus.mjs` → exit 0 - `node scripts/check-empty-changeset.mjs --base origin/main` → exit 0 - `node scripts/check-empty-changeset.mjs --self-test` → exit 0 - `node scripts/check-sdui-manifest.mjs` → exit 0 - `node scripts/check-sdui-manifest.mjs --self-test` → exit 0 - `node scripts/pm/release-rehearsal-clone.mjs --self-test` → exit 0 - `node scripts/release-pending-publish.mjs --self-test` → exit 0 - `pnpm --filter @objectstack/spec run check:duration-unit-keys` → exit 0 - `pnpm check:changeset-gate-self-tests` → exit 0 - `pnpm check:closed-ceremony-text` → exit 0 - `pnpm check:cross-package-test-inputs` → exit 0 - `pnpm check:dispatcher-error-vocabulary` → exit 0 - `pnpm check:driver-memory-census` → exit 0 - `pnpm check:engine-double-contract` → exit 0 - `pnpm check:gitlink-declared` → exit 0 - `pnpm check:nul-bytes` → exit 0 - `pnpm check:pm-changeset-deadline-census` → exit 0 - `pnpm check:published-files` → exit 0 - `pnpm check:query-options-erasure` → exit 0 - `pnpm check:refd-timer-probe` → exit 0 - `pnpm check:skill-identifier-liveness` → exit 0 - `pnpm check:test-source-alias` → exit 0 - `pnpm check:tier-file-adoption` → exit 0 - `pnpm check:type-check-coverage` → exit 0 - `pnpm check:type-check-debt` → exit 0 - `pnpm check:undeclared-dep-imports` → exit 0 - `pnpm check:watch-hint-literal` → exit 0 - `pnpm check:where-matcher` → exit 0 - `pnpm check:changeset-fixed` → exit 0 - `pnpm --dir ui exec vitest run scripts/` → exit 0 `pnpm --dir ui exec vitest run scripts/`: Test Files 178 passed | 2 skipped (180), Tests 5065 passed | 2 skipped (5067). `pnpm check:new-line-citations` (from `ui/`): 0 new citation(s), exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jV9H4DGK4phGzhzdZ4)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jV9H4DGK4phGzhzdZ4 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cdd9f0a commit 35ee2ad

3 files changed

Lines changed: 201 additions & 14 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): "Add filter" in the Studio list-view inspector adds a row that can be edited
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The `filter-builder` widget (the Filter row of the Studio list-view inspector and of each tab preset) now holds its condition rows until they are complete. It used to rebuild the rows from the stored rules on every render, and the save-side fold drops a row that is not complete yet, so the blank row "Add filter" inserts vanished on the next render and no list-view filter could be authored in Studio. An incomplete row now stays on screen and writes nothing; completing it writes exactly the complete rule. What is saved is unchanged: an incomplete rule still never reaches the view, and a stored filter shows as its rows and round-trips as stored.
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#26738 — the `filter-builder` widget HOLDS a condition row until
5+
* it is complete, so "Add filter" adds a row that can be edited.
6+
*
7+
* The widget used to re-derive the builder's rows from the stored rules on
8+
* every render. The fold (`foldFilterGroupToSpecRules`) drops a row that is
9+
* not complete yet — the blank row "Add filter" inserts — and must keep
10+
* dropping it, so the next render of the host erased the new row: the Studio
11+
* list-view inspector could not author a filter at all.
12+
*
13+
* Pinned here, through a host that writes the way the list-view inspector
14+
* does (a NEW body object on every `onChange`, so every write re-renders):
15+
*
16+
* - "Add filter" adds a row that stays on screen, and writes nothing while
17+
* the row is incomplete;
18+
* - completing the row writes exactly the complete rule, and the row stays;
19+
* - a stored filter shows as its rows and round-trips unchanged: opening it
20+
* writes nothing, and its rule reaches the host as stored;
21+
* - CONTROL: a stored filter that changes from outside re-seeds the rows.
22+
*/
23+
24+
import '@testing-library/jest-dom/vitest';
25+
import * as React from 'react';
26+
import { afterEach, describe, expect, it, vi } from 'vitest';
27+
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
28+
import { WIDGETS } from './widgets';
29+
import { loaded } from './loadState';
30+
31+
// The builder lives inside a Radix popover, which does not open in jsdom.
32+
// Rendering the popover parts inline is this directory's existing way of
33+
// reaching it (`WidgetContext.catalogUnion.test.tsx`); the rest of the barrel
34+
// stays real. No JSX in the factory: `vi.mock` is hoisted above the imports.
35+
vi.mock('@object-ui/components', async () => {
36+
const actual = await vi.importActual<Record<string, unknown>>('@object-ui/components');
37+
const Pass = ({ children }: { children?: unknown }) => children as never;
38+
return { ...actual, Popover: Pass, PopoverTrigger: Pass, PopoverContent: Pass };
39+
});
40+
41+
afterEach(cleanup);
42+
43+
const FIELDS = loaded([
44+
{ name: 'title', label: 'Title', type: 'text' },
45+
{ name: 'status', label: 'Status', type: 'text' },
46+
]);
47+
48+
type Rule = { field: string; operator: string; value?: unknown };
49+
type Body = { type: string; filter?: Rule[] };
50+
51+
const FilterWidget = WIDGETS['filter-builder'];
52+
53+
/**
54+
* Writes like `ObjectListViewInspector`'s Filter row: drop `filter` when the
55+
* rules are empty, and store a NEW body either way.
56+
*/
57+
function Host({ initial, writes }: { initial: Body; writes: Array<Rule[] | undefined> }) {
58+
const [body, setBody] = React.useState<Body>(initial);
59+
return (
60+
<>
61+
<FilterWidget
62+
schema={{}}
63+
value={body.filter}
64+
context={{ conditionScope: 'none', objectFields: FIELDS }}
65+
onChange={(rules) => {
66+
writes.push(rules as Rule[] | undefined);
67+
const { filter: _dropped, ...rest } = body;
68+
setBody(Array.isArray(rules) && rules.length > 0 ? { ...rest, filter: rules as Rule[] } : { ...rest });
69+
}}
70+
/>
71+
<pre data-testid="stored-body">{JSON.stringify(body)}</pre>
72+
<button type="button" data-testid="external-edit" onClick={() =>
73+
setBody({ ...body, filter: [{ field: 'status', operator: 'equals', value: 'closed' }] })
74+
} />
75+
</>
76+
);
77+
}
78+
79+
const storedBody = (): Body => JSON.parse(screen.getByTestId('stored-body').textContent ?? 'null');
80+
const valueInputs = () => screen.queryAllByPlaceholderText('Value') as HTMLInputElement[];
81+
82+
describe('the filter-builder widget holds a row until it is complete (objectstack#26738)', () => {
83+
it('"Add filter" adds an editable row, writes nothing until it is complete, then writes the complete rule', () => {
84+
const writes: Array<Rule[] | undefined> = [];
85+
render(<Host initial={{ type: 'grid' }} writes={writes} />);
86+
expect(valueInputs()).toHaveLength(0);
87+
88+
fireEvent.click(screen.getByRole('button', { name: 'Add filter' }));
89+
90+
// The blank row is on screen, and the view is untouched.
91+
expect(valueInputs()).toHaveLength(1);
92+
expect(writes).toEqual([]);
93+
expect(storedBody()).toEqual({ type: 'grid' });
94+
95+
fireEvent.change(valueInputs()[0], { target: { value: 'open' } });
96+
97+
// Complete: exactly the rule is written, and the row stays with its value.
98+
expect(writes).toEqual([[{ field: 'title', operator: 'equals', value: 'open' }]]);
99+
expect(storedBody()).toEqual({ type: 'grid', filter: [{ field: 'title', operator: 'equals', value: 'open' }] });
100+
expect(valueInputs()).toHaveLength(1);
101+
expect(valueInputs()[0].value).toBe('open');
102+
expect(screen.getByTestId('filter-builder-trigger')).toHaveTextContent('Title');
103+
});
104+
105+
it('holds a second, incomplete row beside a stored rule without writing it', () => {
106+
const writes: Array<Rule[] | undefined> = [];
107+
const stored = [{ field: 'status', operator: 'equals', value: 'open' }];
108+
render(<Host initial={{ type: 'grid', filter: stored }} writes={writes} />);
109+
110+
fireEvent.click(screen.getByRole('button', { name: 'Add filter' }));
111+
112+
expect(valueInputs()).toHaveLength(2);
113+
expect(writes).toEqual([]);
114+
expect(storedBody().filter).toEqual(stored);
115+
});
116+
117+
it('shows a stored filter as its rows and round-trips it unchanged', () => {
118+
const writes: Array<Rule[] | undefined> = [];
119+
const stored = [{ field: 'status', operator: 'equals', value: 'open' }];
120+
render(<Host initial={{ type: 'grid', filter: stored }} writes={writes} />);
121+
122+
// Opening it is a read: one row, its value, its field in the summary — no write.
123+
expect(valueInputs()).toHaveLength(1);
124+
expect(valueInputs()[0].value).toBe('open');
125+
expect(screen.getByTestId('filter-builder-trigger')).toHaveTextContent('Status');
126+
expect(writes).toEqual([]);
127+
128+
// An edit writes the rule in the stored shape, and the next edit back
129+
// restores exactly what was stored.
130+
fireEvent.change(valueInputs()[0], { target: { value: 'opened' } });
131+
fireEvent.change(valueInputs()[0], { target: { value: 'open' } });
132+
expect(writes.at(-1)).toEqual(stored);
133+
expect(storedBody().filter).toEqual(stored);
134+
});
135+
136+
it('CONTROL: a stored filter that changes from outside re-seeds the rows', () => {
137+
const writes: Array<Rule[] | undefined> = [];
138+
render(<Host initial={{ type: 'grid', filter: [{ field: 'status', operator: 'equals', value: 'open' }] }} writes={writes} />);
139+
fireEvent.click(screen.getByRole('button', { name: 'Add filter' }));
140+
expect(valueInputs()).toHaveLength(2);
141+
142+
fireEvent.click(screen.getByTestId('external-edit'));
143+
144+
// The held blank row belonged to the old filter; the new one is shown as stored.
145+
expect(valueInputs()).toHaveLength(1);
146+
expect(valueInputs()[0].value).toBe('closed');
147+
expect(writes).toEqual([]);
148+
});
149+
});

‎ui/packages/app-shell/src/views/metadata-admin/widgets.tsx‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2730,6 +2730,23 @@ function ActionMultiWidget({ value, onChange, readOnly, context, ariaLabelledBy
27302730

27312731
interface FilterRuleLite { field: string; operator: string; value?: unknown }
27322732

2733+
/** The builder's row group for a stored rule list. */
2734+
function groupOfStoredRules(rules: FilterRuleLite[]) {
2735+
return {
2736+
id: 'g',
2737+
logic: 'and' as const,
2738+
conditions: rules.map((r, i) => ({
2739+
id: `c${i}`,
2740+
// Handed over as stored: the builder reads it through the spec's fold
2741+
// (objectui#9306), and a spelling nothing folds is kept verbatim so it
2742+
// round-trips on save instead of being rewritten.
2743+
operator: r.operator ?? 'equals',
2744+
field: r.field,
2745+
value: (r.value as any) ?? '',
2746+
})),
2747+
};
2748+
}
2749+
27332750
function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelledBy, loadError }: {
27342751
value?: FilterRuleLite[];
27352752
onChange: (rules: FilterRuleLite[]) => void;
@@ -2762,19 +2779,26 @@ function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelle
27622779
// platform translator (same one ObjectView's toolbar toasts use).
27632780
const { t: tr } = useObjectTranslation();
27642781
const rules = Array.isArray(value) ? value : [];
2765-
const group = {
2766-
id: 'g',
2767-
logic: 'and' as const,
2768-
conditions: rules.map((r, i) => ({
2769-
id: `c${i}`,
2770-
// Handed over as stored: the builder reads it through the spec's fold
2771-
// (objectui#9306), and a spelling nothing folds is kept verbatim so it
2772-
// round-trips on save instead of being rewritten.
2773-
operator: r.operator ?? 'equals',
2774-
field: r.field,
2775-
value: (r.value as any) ?? '',
2776-
})),
2777-
};
2782+
// The stored rules by CONTENT, so a re-render handing over an equal array is
2783+
// not a change (ui/AGENTS.md #10: never key on an identity).
2784+
const rulesKey = JSON.stringify(rules);
2785+
// The builder's rows are HELD here rather than re-derived from `value` on
2786+
// every render. The fold below drops a row that is not complete yet — the
2787+
// blank row "Add filter" inserts, a row with a column but no value — and
2788+
// must keep dropping it (what is not applied is not persisted). Re-deriving
2789+
// the rows from the stored rules therefore erased the new row on the next
2790+
// render, so no filter could be authored here at all. The held group keeps
2791+
// such a row on screen until it is complete; only the fold's rules leave.
2792+
const [draft, setDraft] = React.useState(() => groupOfStoredRules(rules));
2793+
const [seededFrom, setSeededFrom] = React.useState(rulesKey);
2794+
if (seededFrom !== rulesKey) {
2795+
// The stored rules moved. When they are the echo of this builder's own
2796+
// write, the held rows already fold to them; anything else (another
2797+
// editor, a reload, an undo) re-seeds the rows from what is stored.
2798+
setSeededFrom(rulesKey);
2799+
const held = foldFilterGroupToSpecRules(draft);
2800+
if (!held.ok || JSON.stringify(held.rules) !== rulesKey) setDraft(groupOfStoredRules(rules));
2801+
}
27782802
const fbFields = fields.map((f) => ({ value: f.name, label: f.label || f.name, type: f.type }));
27792803
const summary = rules.length
27802804
? rules.map((r) => `${fields.find((f) => f.name === r.field)?.label || r.field}`).filter(Boolean).join(', ')
@@ -2789,6 +2813,10 @@ function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelle
27892813
toast.error(tr(FILTER_FOLD_REFUSAL_KEYS[folded.reason]));
27902814
return;
27912815
}
2816+
setDraft(g);
2817+
// A row added or edited without completing it folds to the rules already
2818+
// stored: the view has not changed, so nothing is written.
2819+
if (JSON.stringify(folded.rules) === rulesKey) return;
27922820
onChange(folded.rules as FilterRuleLite[]);
27932821
};
27942822
return (
@@ -2810,7 +2838,7 @@ function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelle
28102838
) : fields.length === 0 ? (
28112839
<p className="text-xs text-muted-foreground">{t('engine.form.bindObjectForConditions', locale)}</p>
28122840
) : (
2813-
<FilterBuilder fields={fbFields} value={group as any} onChange={handle} />
2841+
<FilterBuilder fields={fbFields} value={draft as any} onChange={handle} />
28142842
)}
28152843
</PopoverContent>
28162844
</Popover>

0 commit comments

Comments
 (0)