Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/26738-filter-builder-held-row.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@object-ui/app-shell': patch
---

fix(app-shell): "Add filter" in the Studio list-view inspector adds a row that can be edited

Clause-②: no
Metadata-protocol: none

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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectstack#26738 — the `filter-builder` widget HOLDS a condition row until
* it is complete, so "Add filter" adds a row that can be edited.
*
* The widget used to re-derive the builder's rows from the stored rules on
* every render. The fold (`foldFilterGroupToSpecRules`) drops a row that is
* not complete yet — the blank row "Add filter" inserts — and must keep
* dropping it, so the next render of the host erased the new row: the Studio
* list-view inspector could not author a filter at all.
*
* Pinned here, through a host that writes the way the list-view inspector
* does (a NEW body object on every `onChange`, so every write re-renders):
*
* - "Add filter" adds a row that stays on screen, and writes nothing while
* the row is incomplete;
* - completing the row writes exactly the complete rule, and the row stays;
* - a stored filter shows as its rows and round-trips unchanged: opening it
* writes nothing, and its rule reaches the host as stored;
* - CONTROL: a stored filter that changes from outside re-seeds the rows.
*/

import '@testing-library/jest-dom/vitest';
import * as React from 'react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
import { WIDGETS } from './widgets';
import { loaded } from './loadState';

// The builder lives inside a Radix popover, which does not open in jsdom.
// Rendering the popover parts inline is this directory's existing way of
// reaching it (`WidgetContext.catalogUnion.test.tsx`); the rest of the barrel
// stays real. No JSX in the factory: `vi.mock` is hoisted above the imports.
vi.mock('@object-ui/components', async () => {
const actual = await vi.importActual<Record<string, unknown>>('@object-ui/components');
const Pass = ({ children }: { children?: unknown }) => children as never;
return { ...actual, Popover: Pass, PopoverTrigger: Pass, PopoverContent: Pass };
});

afterEach(cleanup);

const FIELDS = loaded([
{ name: 'title', label: 'Title', type: 'text' },
{ name: 'status', label: 'Status', type: 'text' },
]);

type Rule = { field: string; operator: string; value?: unknown };
type Body = { type: string; filter?: Rule[] };

const FilterWidget = WIDGETS['filter-builder'];

/**
* Writes like `ObjectListViewInspector`'s Filter row: drop `filter` when the
* rules are empty, and store a NEW body either way.
*/
function Host({ initial, writes }: { initial: Body; writes: Array<Rule[] | undefined> }) {
const [body, setBody] = React.useState<Body>(initial);
return (
<>
<FilterWidget
schema={{}}
value={body.filter}
context={{ conditionScope: 'none', objectFields: FIELDS }}
onChange={(rules) => {
writes.push(rules as Rule[] | undefined);
const { filter: _dropped, ...rest } = body;
setBody(Array.isArray(rules) && rules.length > 0 ? { ...rest, filter: rules as Rule[] } : { ...rest });
}}
/>
<pre data-testid="stored-body">{JSON.stringify(body)}</pre>
<button type="button" data-testid="external-edit" onClick={() =>
setBody({ ...body, filter: [{ field: 'status', operator: 'equals', value: 'closed' }] })
} />
</>
);
}

const storedBody = (): Body => JSON.parse(screen.getByTestId('stored-body').textContent ?? 'null');
const valueInputs = () => screen.queryAllByPlaceholderText('Value') as HTMLInputElement[];

describe('the filter-builder widget holds a row until it is complete (objectstack#26738)', () => {
it('"Add filter" adds an editable row, writes nothing until it is complete, then writes the complete rule', () => {
const writes: Array<Rule[] | undefined> = [];
render(<Host initial={{ type: 'grid' }} writes={writes} />);
expect(valueInputs()).toHaveLength(0);

fireEvent.click(screen.getByRole('button', { name: 'Add filter' }));

// The blank row is on screen, and the view is untouched.
expect(valueInputs()).toHaveLength(1);
expect(writes).toEqual([]);
expect(storedBody()).toEqual({ type: 'grid' });

fireEvent.change(valueInputs()[0], { target: { value: 'open' } });

// Complete: exactly the rule is written, and the row stays with its value.
expect(writes).toEqual([[{ field: 'title', operator: 'equals', value: 'open' }]]);
expect(storedBody()).toEqual({ type: 'grid', filter: [{ field: 'title', operator: 'equals', value: 'open' }] });
expect(valueInputs()).toHaveLength(1);
expect(valueInputs()[0].value).toBe('open');
expect(screen.getByTestId('filter-builder-trigger')).toHaveTextContent('Title');
});

it('holds a second, incomplete row beside a stored rule without writing it', () => {
const writes: Array<Rule[] | undefined> = [];
const stored = [{ field: 'status', operator: 'equals', value: 'open' }];
render(<Host initial={{ type: 'grid', filter: stored }} writes={writes} />);

fireEvent.click(screen.getByRole('button', { name: 'Add filter' }));

expect(valueInputs()).toHaveLength(2);
expect(writes).toEqual([]);
expect(storedBody().filter).toEqual(stored);
});

it('shows a stored filter as its rows and round-trips it unchanged', () => {
const writes: Array<Rule[] | undefined> = [];
const stored = [{ field: 'status', operator: 'equals', value: 'open' }];
render(<Host initial={{ type: 'grid', filter: stored }} writes={writes} />);

// Opening it is a read: one row, its value, its field in the summary — no write.
expect(valueInputs()).toHaveLength(1);
expect(valueInputs()[0].value).toBe('open');
expect(screen.getByTestId('filter-builder-trigger')).toHaveTextContent('Status');
expect(writes).toEqual([]);

// An edit writes the rule in the stored shape, and the next edit back
// restores exactly what was stored.
fireEvent.change(valueInputs()[0], { target: { value: 'opened' } });
fireEvent.change(valueInputs()[0], { target: { value: 'open' } });
expect(writes.at(-1)).toEqual(stored);
expect(storedBody().filter).toEqual(stored);
});

it('CONTROL: a stored filter that changes from outside re-seeds the rows', () => {
const writes: Array<Rule[] | undefined> = [];
render(<Host initial={{ type: 'grid', filter: [{ field: 'status', operator: 'equals', value: 'open' }] }} writes={writes} />);
fireEvent.click(screen.getByRole('button', { name: 'Add filter' }));
expect(valueInputs()).toHaveLength(2);

fireEvent.click(screen.getByTestId('external-edit'));

// The held blank row belonged to the old filter; the new one is shown as stored.
expect(valueInputs()).toHaveLength(1);
expect(valueInputs()[0].value).toBe('closed');
expect(writes).toEqual([]);
});
});
56 changes: 42 additions & 14 deletions ui/packages/app-shell/src/views/metadata-admin/widgets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2730,6 +2730,23 @@ function ActionMultiWidget({ value, onChange, readOnly, context, ariaLabelledBy

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

/** The builder's row group for a stored rule list. */
function groupOfStoredRules(rules: FilterRuleLite[]) {
return {
id: 'g',
logic: 'and' as const,
conditions: rules.map((r, i) => ({
id: `c${i}`,
// Handed over as stored: the builder reads it through the spec's fold
// (objectui#9306), and a spelling nothing folds is kept verbatim so it
// round-trips on save instead of being rewritten.
operator: r.operator ?? 'equals',
field: r.field,
value: (r.value as any) ?? '',
})),
};
}

function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelledBy, loadError }: {
value?: FilterRuleLite[];
onChange: (rules: FilterRuleLite[]) => void;
Expand Down Expand Up @@ -2762,19 +2779,26 @@ function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelle
// platform translator (same one ObjectView's toolbar toasts use).
const { t: tr } = useObjectTranslation();
const rules = Array.isArray(value) ? value : [];
const group = {
id: 'g',
logic: 'and' as const,
conditions: rules.map((r, i) => ({
id: `c${i}`,
// Handed over as stored: the builder reads it through the spec's fold
// (objectui#9306), and a spelling nothing folds is kept verbatim so it
// round-trips on save instead of being rewritten.
operator: r.operator ?? 'equals',
field: r.field,
value: (r.value as any) ?? '',
})),
};
// The stored rules by CONTENT, so a re-render handing over an equal array is
// not a change (ui/AGENTS.md #10: never key on an identity).
const rulesKey = JSON.stringify(rules);
// The builder's rows are HELD here rather than re-derived from `value` on
// every render. The fold below drops a row that is not complete yet — the
// blank row "Add filter" inserts, a row with a column but no value — and
// must keep dropping it (what is not applied is not persisted). Re-deriving
// the rows from the stored rules therefore erased the new row on the next
// render, so no filter could be authored here at all. The held group keeps
// such a row on screen until it is complete; only the fold's rules leave.
const [draft, setDraft] = React.useState(() => groupOfStoredRules(rules));
const [seededFrom, setSeededFrom] = React.useState(rulesKey);
if (seededFrom !== rulesKey) {
// The stored rules moved. When they are the echo of this builder's own
// write, the held rows already fold to them; anything else (another
// editor, a reload, an undo) re-seeds the rows from what is stored.
setSeededFrom(rulesKey);
const held = foldFilterGroupToSpecRules(draft);
if (!held.ok || JSON.stringify(held.rules) !== rulesKey) setDraft(groupOfStoredRules(rules));
}
const fbFields = fields.map((f) => ({ value: f.name, label: f.label || f.name, type: f.type }));
const summary = rules.length
? rules.map((r) => `${fields.find((f) => f.name === r.field)?.label || r.field}`).filter(Boolean).join(', ')
Expand All @@ -2789,6 +2813,10 @@ function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelle
toast.error(tr(FILTER_FOLD_REFUSAL_KEYS[folded.reason]));
return;
}
setDraft(g);
// A row added or edited without completing it folds to the rules already
// stored: the view has not changed, so nothing is written.
if (JSON.stringify(folded.rules) === rulesKey) return;
onChange(folded.rules as FilterRuleLite[]);
};
return (
Expand All @@ -2810,7 +2838,7 @@ function FilterBuilderField({ value, onChange, fields, readOnly, id, ariaLabelle
) : fields.length === 0 ? (
<p className="text-xs text-muted-foreground">{t('engine.form.bindObjectForConditions', locale)}</p>
) : (
<FilterBuilder fields={fbFields} value={group as any} onChange={handle} />
<FilterBuilder fields={fbFields} value={draft as any} onChange={handle} />
)}
</PopoverContent>
</Popover>
Expand Down
Loading