diff --git a/.changeset/list-toolbar-filter-fold-to-spec-rules.md b/.changeset/list-toolbar-filter-fold-to-spec-rules.md new file mode 100644 index 0000000000..ec5eba06c0 --- /dev/null +++ b/.changeset/list-toolbar-filter-fold-to-spec-rules.md @@ -0,0 +1,10 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +The list toolbar's "Filter" now saves. Saving a filter from the runtime toolbar PUT the FilterBuilder's whole group object (`{ id, logic, conditions }`) into the view's `filter`, where `@objectstack/spec`'s `ListViewSchema.filter` declares `ViewFilterRule[]` — so every save came back `422 invalid_metadata` and the filter was silently never persisted (objectstack#5159). + +The producer now folds the builder's group to the spec's flat `{ field, operator, value }` rule list before persisting, sharing one transform with the Studio view inspector (which had the only copy). Operators normalize through the spec's own `normalizeFilterOperator`, so the four builder operators the Studio's local table had drifted behind — `startsWith`, `endsWith`, `isNull`, `isNotNull` — now persist correctly too. The builder's per-row `id` is no longer written: it is a React list key that the read path regenerates, so stored view bodies keep the declared vocabulary only. + +A filter whose shape cannot be represented losslessly as a flat rule list — `OR` across several conditions, or nested condition groups — is now refused with a translated message instead of being quietly saved as `AND`, which would have returned a different set of records than the one on screen. Such a filter still applies to the current list; it just does not become part of the saved view. diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index de53baab9d..2a0cf04c00 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -14,6 +14,7 @@ import { useParams, useSearchParams, useNavigate, useLocation } from 'react-rout import { resolveFilterPlaceholders, DENSITY_MODE_TO_ROW_HEIGHT, normalizeListViewSchema, type FilterTokenScope } from '@object-ui/core'; import { parseUserFilterParams, applyUserFilterParams } from './userFilterUrlState'; import { buildListFilterKey, readListFilterState, writeListFilterState } from './listFilterStorage'; +import { foldFilterGroupToSpecRules, FILTER_FOLD_REFUSAL_KEYS } from './viewFilterFold'; const ObjectChart = lazy(() => import('@object-ui/plugin-charts').then((m) => ({ default: m.ObjectChart })), ); @@ -265,6 +266,34 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an [dataSource, objectName] ); + /** + * Persist a toolbar filter change (objectstack#5159). + * + * The list toolbar's FilterBuilder emits its own grouped dialect + * (`{ id, logic, conditions }`); `ListViewSchema.filter` declares + * `ViewFilterRule[]`. Handing the group straight to `persistViewPatch` is + * what made every `Filter → Add filter` PUT come back 422 `invalid_union`. + * Fold at the producer (AGENTS.md #0.1) — the spec is not widened. + * + * A group that cannot fold LOSSLESSLY (`logic: 'or'` across several + * conditions, or a nested group) is refused out loud and NOT saved, rather + * than quietly written as AND: an AND rewrite would return a different + * record set than the one the user is looking at. The filter still applies + * to the live grid — `convertFilterGroupToAST` honours `logic` in-session — + * it just does not become part of the stored view. + */ + const persistViewFilter = useCallback( + (viewIdLocal: string, baseViewDef: Record, group: unknown) => { + const folded = foldFilterGroupToSpecRules(group); + if (!folded.ok) { + toast.error(t(FILTER_FOLD_REFUSAL_KEYS[folded.reason])); + return; + } + persistViewPatch(viewIdLocal, baseViewDef, { filter: folded.rules }); + }, + [persistViewPatch, t] + ); + const handleViewConfigSave = useCallback((draft: Record) => { setViewDraft(draft); setRefreshKey(k => k + 1); @@ -1335,7 +1364,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an persistViewPatch(viewDef.id, viewDef, { sort }); }, onFilterChange: (filter: any) => { - persistViewPatch(viewDef.id, viewDef, { filter }); + persistViewFilter(viewDef.id, viewDef, filter); }, onHiddenFieldsChange: (hidden: string[]) => { persistViewPatch(viewDef.id, viewDef, { hiddenFields: hidden }); @@ -1590,7 +1619,11 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an persistViewPatch(viewDef.id, viewDef, { sort }); }} onFilterChange={(filter: any) => { - persistViewPatch(viewDef.id, viewDef, { filter }); + persistViewFilter(viewDef.id, viewDef, filter); + // localStorage keeps the BUILDER's group verbatim — it is + // read back into `initialFilters`, which needs `conditions` + // (and the row ids) to rehydrate the toolbar. Only the + // spec-governed view body is folded. writeListFilterState(listFilterKey, { filters: filter }); }} onSearchChange={(search: string) => { @@ -1612,7 +1645,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an dataSource={ds} /> ); - }, [activeView, objectDef, objectName, refreshKey, navOverlay, actions, persistViewPatch, urlFilters, initialUfSelections, handleUserFilterSelectionsChange, user?.id]); + }, [activeView, objectDef, objectName, refreshKey, navOverlay, actions, persistViewPatch, persistViewFilter, urlFilters, initialUfSelections, handleUserFilterSelectionsChange, user?.id]); // Memoize the merged views array so PluginObjectView doesn't get a new // reference on every render (which would trigger unnecessary data refetches). diff --git a/packages/app-shell/src/views/metadata-admin/widgets.tsx b/packages/app-shell/src/views/metadata-admin/widgets.tsx index 69d687a1b3..2876107209 100644 --- a/packages/app-shell/src/views/metadata-admin/widgets.tsx +++ b/packages/app-shell/src/views/metadata-admin/widgets.tsx @@ -43,7 +43,10 @@ import { } from '@object-ui/components'; import { ChevronDown, ChevronsUpDown, ChevronUp, Eye, EyeOff, Plus, Search, Trash2 } from 'lucide-react'; import { iconNames } from 'lucide-react/dynamic.mjs'; +import { toast } from 'sonner'; +import { useObjectTranslation } from '@object-ui/i18n'; import { useMetadataLocale, t, tFormat } from './i18n'; +import { foldFilterGroupToSpecRules, FILTER_FOLD_REFUSAL_KEYS } from '../viewFilterFold'; import { ColorVariantPicker } from './color-variant-field'; import { ConditionBuilder } from './inspectors/ConditionBuilder'; import { expressionSource, writeExpressionSource } from './inspectors/expression-envelope'; @@ -1675,14 +1678,13 @@ function ActionMultiWidget({ id, value, onChange, readOnly, context }: WidgetPro /* by ViewFilterRuleSchema); reads also accept legacy shorthand/camelCase */ /* spellings still present in already-stored view metadata (gt / eq / isNull). */ /* -------------------------------------------------------------------------- */ -const FB_TO_SPEC: Record = { - equals: 'equals', notEquals: 'not_equals', contains: 'contains', notContains: 'not_contains', - isEmpty: 'is_empty', isNotEmpty: 'is_not_empty', - greaterThan: 'greater_than', lessThan: 'less_than', - greaterOrEqual: 'greater_than_or_equal', lessOrEqual: 'less_than_or_equal', - before: 'before', after: 'after', between: 'between', - in: 'in', notIn: 'not_in', -}; +/* The WRITE direction (builder group → spec `ViewFilterRule[]`) now lives in */ +/* `../viewFilterFold`, shared with the runtime list toolbar — this file used */ +/* to hold the only copy, which is why the toolbar had none and PUT its raw */ +/* FilterGroup (objectstack#5159). Its local `FB_TO_SPEC` table went with it: */ +/* the shared fold normalizes through the spec's own */ +/* `normalizeFilterOperator`, which covers the four builder operators this */ +/* table had drifted behind (startsWith / endsWith / isNull / isNotNull). */ /** Spec operator → FilterBuilder camelCase. Keys cover both the canonical * vocabulary and legacy spellings (shorthand + snake/camel) so stored view * metadata written before canonicalization still seeds the builder. */ @@ -1707,6 +1709,10 @@ function FilterBuilderField({ value, onChange, fields, readOnly }: { fields: Array<{ name: string; label?: string; type?: string }>; readOnly?: boolean; }) { + // The metadata-admin `t` above is a static engine-string table; refusal + // copy lives in the shared console locale packs, so it resolves through the + // platform translator (same one ObjectView's toolbar toasts use). + const { t: tr } = useObjectTranslation(); const rules = Array.isArray(value) ? value : []; const group = { id: 'g', @@ -1725,10 +1731,16 @@ function FilterBuilderField({ value, onChange, fields, readOnly }: { ? rules.map((r) => `${fields.find((f) => f.name === r.field)?.label || r.field}`).filter(Boolean).join(', ') : ''; const handle = (g: any) => { - const next = (g?.conditions ?? []) - .filter((c: any) => c?.field) - .map((c: any) => ({ field: c.field, operator: FB_TO_SPEC[c.operator] ?? c.operator, value: c.value })); - onChange(next); + // Shared with the runtime list toolbar (objectstack#5159). A group that + // cannot fold losslessly — `logic: 'or'` over several rows, or a nested + // group — is refused rather than silently written as AND, which is what + // this callback used to do by dropping `g.logic` on the floor. + const folded = foldFilterGroupToSpecRules(g); + if (!folded.ok) { + toast.error(tr(FILTER_FOLD_REFUSAL_KEYS[folded.reason])); + return; + } + onChange(folded.rules as FilterRuleLite[]); }; return ( diff --git a/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts b/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts new file mode 100644 index 0000000000..4417d8ea1a --- /dev/null +++ b/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts @@ -0,0 +1,104 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectstack#5159 ratchet — the FilterBuilder's `FilterGroup` must never + * reach a persisted view `filter` again. + * + * The bug: the runtime list toolbar handed `onFilterChange`'s payload straight + * to `persistViewPatch({ filter })`. That payload is the builder's grouped + * dialect (`{ id, logic, conditions }`); `ListViewSchema.filter` declares + * `z.array(ViewFilterRuleSchema)`. Every `Filter → Add filter → save` came back + * 422 `invalid_union` — and because objectstack#5014 flattens union errors to a + * bare "Invalid input" that never names `filter`, it lived on `main` unnoticed. + * + * `viewFilterFold.test.ts` pins what the fold PRODUCES. This pins that the + * persist path still GOES THROUGH it: a shape assertion on the transform is + * worth nothing if a future edit routes a raw group around it. The two together + * close the issue's replay matrix at the persist-call boundary — variant ① (the + * captured `FilterGroup`) is unreachable, variant ③ (declared keys only) is + * what the PUT carries. + * + * If this fails: do not re-add `persistViewPatch(…, { filter })`. Route the + * group through `persistViewFilter` (ObjectView) / `foldFilterGroupToSpecRules` + * (`./viewFilterFold`), which folds to `ViewFilterRule[]` and refuses — out + * loud — the shapes that cannot fold losslessly. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const objectViewPath = path.join(here, 'ObjectView.tsx'); +const widgetsPath = path.join(here, 'metadata-admin/widgets.tsx'); + +const objectViewSrc = readFileSync(objectViewPath, 'utf8'); +const widgetsSrc = readFileSync(widgetsPath, 'utf8'); + +/** + * Every `persistViewPatch(…, { filter … })` in the file — the call that shipped + * the group. Exactly ONE is legitimate: the one inside `persistViewFilter`, + * whose value is the fold's output. Any other is the bug returning. + */ +const FILTER_PERSIST_CALLS = /persistViewPatch\s*\([^;]*?\{\s*filter\s*(?::\s*([A-Za-z0-9_.]+))?/gs; + +describe('objectstack#5159 ratchet — no raw FilterGroup on the view persist path', () => { + it('the ratchet is reading real source, not an empty string', () => { + expect(objectViewSrc.length).toBeGreaterThan(10_000); + expect(widgetsSrc.length).toBeGreaterThan(10_000); + // Sanity: the guarded symbol still exists under these names. + expect(objectViewSrc).toContain('persistViewPatch'); + }); + + it('the only `filter` patch ObjectView persists is the FOLD’s output', () => { + // `m[1]` is undefined for the shorthand `{ filter }` — the exact form + // the bug shipped — so it can never satisfy the expectation below. + const values = [...objectViewSrc.matchAll(FILTER_PERSIST_CALLS)].map((m) => m[1] ?? '{ filter } shorthand'); + // Exactly one such call, and it writes the folded rules — never the raw + // `filter` argument the toolbar handed in. + expect(values, 'a `persistViewPatch(…, { filter })` call appeared that does not write the fold’s output') + .toEqual(['folded.rules']); + }); + + it('every toolbar filter change routes through the fold', () => { + // Look at the code immediately following each `onFilterChange` binding: + // it must reach `persistViewFilter`, not `persistViewPatch`. + const sites = [...objectViewSrc.matchAll(/onFilterChange[=:]/g)]; + expect(sites.length, 'expected the list-toolbar onFilterChange handlers to be found') + .toBeGreaterThanOrEqual(2); + for (const site of sites) { + // Window = this handler only. It ends where the NEXT `onXxx` + // binding starts, so the sibling handlers (which legitimately call + // `persistViewPatch` for sort / hiddenFields / columnState) stay + // out of the assertion. + const rest = objectViewSrc.slice(site.index! + 1, site.index! + 401); + const next = rest.search(/\bon[A-Z]\w*\s*[=:]/); + const body = next === -1 ? rest : rest.slice(0, next); + expect(body, `this onFilterChange bypasses the fold:\n${body}`).toContain('persistViewFilter'); + expect(body, `this onFilterChange persists a raw group:\n${body}`).not.toContain('persistViewPatch'); + } + }); + + it('ObjectView imports the shared fold rather than re-deriving one', () => { + expect(objectViewSrc).toMatch(/import\s*\{[^}]*foldFilterGroupToSpecRules[^}]*\}\s*from\s*'\.\/viewFilterFold'/); + }); + + it('the Studio inspector folds through the SAME module — one dialect, not two', () => { + expect(widgetsSrc).toMatch(/import\s*\{[^}]*foldFilterGroupToSpecRules[^}]*\}\s*from\s*'\.\.\/viewFilterFold'/); + // The local operator table it used to carry is gone: a second table is + // how the runtime toolbar ended up with no fold at all. (Matched on the + // DECLARATION so the comment explaining the removal doesn't trip it.) + expect(widgetsSrc).not.toMatch(/const\s+FB_TO_SPEC\b/); + }); + + it('the refusal reaches the user — both fold call sites surface it, none swallow it', () => { + for (const [name, src] of [['ObjectView.tsx', objectViewSrc], ['widgets.tsx', widgetsSrc]] as const) { + expect(src, `${name} calls the fold`).toContain('foldFilterGroupToSpecRules'); + expect(src, `${name} surfaces the refusal as a toast`).toMatch( + /toast\.error\(\s*tr?\(\s*FILTER_FOLD_REFUSAL_KEYS/, + ); + } + }); +}); diff --git a/packages/app-shell/src/views/viewFilterFold.test.ts b/packages/app-shell/src/views/viewFilterFold.test.ts new file mode 100644 index 0000000000..7585ee8f37 --- /dev/null +++ b/packages/app-shell/src/views/viewFilterFold.test.ts @@ -0,0 +1,300 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectstack#5159 — the list toolbar PUT a FilterBuilder `FilterGroup` into + * the view's `filter`, where `ListViewSchema.filter` declares + * `z.array(ViewFilterRuleSchema)`. The server answered 422 `invalid_union`. + * + * These tests pin the producer-side fold: the shape that reaches the wire, the + * loud refusal for shapes that cannot fold losslessly, and — with the spec's + * own schema — that the folded body is actually accepted. + */ +import { describe, it, expect } from 'vitest'; +import { ListViewSchema } from '@objectstack/spec/ui'; +import { foldFilterGroupToSpecRules, FILTER_FOLD_REFUSAL_KEYS } from './viewFilterFold'; + +/** + * The body objectstack#5159 captured off the wire, verbatim: one click of + * `Filter → Add filter` on `/_console/apps/showcase_app/showcase_task`. + * Replay variant ① — the shape that must never be emitted again. + */ +const CAPTURED_TOOLBAR_GROUP = { + id: 'root', + logic: 'and', + conditions: [ + { id: '712135fb-58c5-4be4-a611-1925181509b0', field: 'title', operator: 'equals', value: '' }, + ], +}; + +describe('foldFilterGroupToSpecRules — flat AND group → ViewFilterRule[]', () => { + it('#5159: folds the captured toolbar body to replay variant ③ (declared keys only)', () => { + const result = foldFilterGroupToSpecRules(CAPTURED_TOOLBAR_GROUP); + expect(result.ok).toBe(true); + expect(result.ok && result.rules).toEqual([ + { field: 'title', operator: 'equals', value: '' }, + ]); + }); + + it('#5159: what is emitted is an ARRAY — never the FilterGroup object (variant ①)', () => { + const result = foldFilterGroupToSpecRules(CAPTURED_TOOLBAR_GROUP); + expect(result.ok).toBe(true); + const rules = result.ok ? result.rules : null; + expect(Array.isArray(rules)).toBe(true); + // The three keys that made the group a group are gone from the wire. + expect(rules).not.toHaveProperty('logic'); + expect(rules).not.toHaveProperty('conditions'); + expect(JSON.stringify(rules)).not.toContain('"logic"'); + }); + + it('#5114/#5159: strips the builder-minted row `id` — the read path regenerates it', () => { + const result = foldFilterGroupToSpecRules(CAPTURED_TOOLBAR_GROUP); + expect(result.ok && result.rules[0]).not.toHaveProperty('id'); + // Declared vocabulary only. + expect(result.ok && Object.keys(result.rules[0])).toEqual(['field', 'operator', 'value']); + }); + + it('folds several conditions in order', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', + logic: 'and', + conditions: [ + { id: 'a', field: 'status', operator: 'equals', value: 'open' }, + { id: 'b', field: 'amount', operator: 'greaterThan', value: 100 }, + ], + }); + expect(result.ok && result.rules).toEqual([ + { field: 'status', operator: 'equals', value: 'open' }, + { field: 'amount', operator: 'greater_than', value: 100 }, + ]); + }); + + it('normalizes the builder camelCase vocabulary onto the spec canon', () => { + const builderOperators = [ + 'equals', 'notEquals', 'contains', 'notContains', + 'isEmpty', 'isNotEmpty', 'greaterThan', 'lessThan', + 'greaterOrEqual', 'lessOrEqual', 'before', 'after', 'between', + 'in', 'notIn', 'startsWith', 'endsWith', 'isNull', 'isNotNull', + ]; + const result = foldFilterGroupToSpecRules({ + id: 'root', + logic: 'and', + conditions: builderOperators.map((operator, i) => ({ + id: `c${i}`, field: 'f', operator, value: 'x', + })), + }); + expect(result.ok).toBe(true); + expect(result.ok && result.rules.map(r => r.operator)).toEqual([ + 'equals', 'not_equals', 'contains', 'not_contains', + 'is_empty', 'is_not_empty', 'greater_than', 'less_than', + 'greater_than_or_equal', 'less_than_or_equal', 'before', 'after', 'between', + 'in', 'not_in', 'starts_with', 'ends_with', 'is_null', 'is_not_null', + ]); + }); + + it('passes an operator the spec does not know through VERBATIM (server rejects, we do not coerce)', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'and', + conditions: [{ id: 'a', field: 'f', operator: 'sounds_like', value: 'x' }], + }); + // NOT silently rewritten to `equals` — the spec enum is the judge. + expect(result.ok && result.rules[0].operator).toBe('sounds_like'); + }); + + it('drops the blank row `Add filter` inserts before a column is picked', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'and', + conditions: [ + { id: 'a', field: '', operator: 'equals', value: '' }, + { id: 'b', field: 'title', operator: 'equals', value: 'x' }, + ], + }); + expect(result.ok && result.rules).toEqual([{ field: 'title', operator: 'equals', value: 'x' }]); + }); + + it('omits `value` entirely when the row carries none (unary operators)', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'and', + conditions: [{ id: 'a', field: 'archived_at', operator: 'isEmpty' }], + }); + expect(result.ok && result.rules).toEqual([{ field: 'archived_at', operator: 'is_empty' }]); + expect(result.ok && 'value' in result.rules[0]).toBe(false); + }); +}); + +describe('foldFilterGroupToSpecRules — empty conditions', () => { + // Current UX contract: clearing every row must CLEAR the stored filter, so + // the fold yields `[]` — which `z.array(ViewFilterRuleSchema).optional()` + // accepts and which reads back as "no filter". It is not a refusal. + it('an empty group folds to an empty rule list (clears the stored filter)', () => { + expect(foldFilterGroupToSpecRules({ id: 'root', logic: 'and', conditions: [] })) + .toEqual({ ok: true, rules: [] }); + }); + + it('a group whose only rows are blank folds to an empty rule list', () => { + expect(foldFilterGroupToSpecRules({ + id: 'root', logic: 'and', + conditions: [{ id: 'a', field: '', operator: 'equals', value: '' }], + })).toEqual({ ok: true, rules: [] }); + }); + + it('null / undefined fold to an empty rule list', () => { + expect(foldFilterGroupToSpecRules(null)).toEqual({ ok: true, rules: [] }); + expect(foldFilterGroupToSpecRules(undefined)).toEqual({ ok: true, rules: [] }); + }); +}); + +describe('foldFilterGroupToSpecRules — loud refusal, never a silent downgrade (A1)', () => { + it("refuses `logic: 'or'` over two conditions instead of writing them as AND", () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', + logic: 'or', + conditions: [ + { id: 'a', field: 'status', operator: 'equals', value: 'open' }, + { id: 'b', field: 'status', operator: 'equals', value: 'blocked' }, + ], + }); + expect(result).toEqual({ ok: false, reason: 'or_logic' }); + // The refusal reaches the user through a translated string, not a + // console throw: the toolbar surfaces this key as a toast. + expect(FILTER_FOLD_REFUSAL_KEYS.or_logic).toBe('console.objectView.filterOrNotSavable'); + }); + + it('refuses OR however many conditions there are beyond one', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'or', + conditions: [ + { id: 'a', field: 'a', operator: 'equals', value: 1 }, + { id: 'b', field: 'b', operator: 'equals', value: 2 }, + { id: 'c', field: 'c', operator: 'equals', value: 3 }, + ], + }); + expect(result.ok).toBe(false); + }); + + it("folds `logic: 'or'` over ONE condition — OR and AND select the same records there", () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'or', + conditions: [{ id: 'a', field: 'status', operator: 'equals', value: 'open' }], + }); + expect(result).toEqual({ ok: true, rules: [{ field: 'status', operator: 'equals', value: 'open' }] }); + }); + + it("folds `logic: 'or'` over blank rows only (nothing to downgrade)", () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'or', + conditions: [{ id: 'a', field: '', operator: 'equals', value: '' }], + }); + expect(result).toEqual({ ok: true, rules: [] }); + }); + + it('refuses a nested group — a flat rule array cannot express it', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', + logic: 'and', + conditions: [ + { id: 'a', field: 'status', operator: 'equals', value: 'open' }, + { id: 'g1', logic: 'or', conditions: [{ id: 'b', field: 'x', operator: 'equals', value: 1 }] }, + ], + }); + expect(result).toEqual({ ok: false, reason: 'nested_group' }); + expect(FILTER_FOLD_REFUSAL_KEYS.nested_group).toBe('console.objectView.filterNestedNotSavable'); + }); + + it('refuses a nested AND group too — nesting, not the logic word, is the problem', () => { + const result = foldFilterGroupToSpecRules({ + id: 'root', logic: 'and', + conditions: [{ id: 'g1', logic: 'and', conditions: [] }], + }); + expect(result).toEqual({ ok: false, reason: 'nested_group' }); + }); + + it('every refusal reason has a user-readable key — no reason can surface untranslated', () => { + const reasons: Array = ['or_logic', 'nested_group']; + for (const reason of reasons) { + expect(FILTER_FOLD_REFUSAL_KEYS[reason]).toMatch(/^console\.objectView\./); + } + }); +}); + +describe('the folded body is what the spec actually accepts (replay-matrix closure)', () => { + /** A minimal view body that is valid apart from whatever `filter` carries. */ + const viewBody = (filter: unknown) => ({ + name: 'default', label: 'All', type: 'grid', + columns: ['title'], + filter, + }); + + it('the fixture itself is valid without a filter — the assertions below isolate `filter`', () => { + const parsed = ListViewSchema.safeParse(viewBody(undefined)); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + }); + + // Replay variant ① — the FilterGroup object the toolbar used to PUT. + it('#5159 variant ①: the raw FilterGroup is REJECTED by ListViewSchema, on `filter`', () => { + const parsed = ListViewSchema.safeParse(viewBody(CAPTURED_TOOLBAR_GROUP)); + expect(parsed.success).toBe(false); + // The 422 the browser saw. Pin that it is `filter` that fails, so this + // cannot pass because some unrelated key drifted out of the fixture. + const paths = (parsed as any).error.issues.map((i: any) => i.path.join('.')); + expect(paths).toContain('filter'); + }); + + // Replay variant ② — flat rule list, but still carrying the builder's row + // `id`. The issue's matrix recorded this as ACCEPTED against a server + // running framework `main` (PR #5154 tolerates the undeclared key). It is + // NOT accepted by the spec objectui itself pins: `ViewFilterRuleSchema` is + // a `strictObject` over `{field, operator, value}` only. So stripping `id` + // is not merely the tidier at-rest shape — under this pin it is required, + // and a fold that kept the id would trade a 422 `invalid_union` for a 422 + // `unrecognized_keys`. This pins the measurement behind that decision; if a + // later spec bump declares `id`, this test is the one that says so. + it('#5159 variant ②: a rule keeping the builder `id` is REJECTED, on `filter.0`', () => { + const withId = { id: '712135fb-58c5-4be4-a611-1925181509b0', field: 'title', operator: 'equals', value: '' }; + const parsed = ListViewSchema.safeParse(viewBody([withId])); + expect(parsed.success).toBe(false); + const issues = (parsed as any).error.issues; + expect(issues.map((i: any) => i.path.join('.'))).toContain('filter.0'); + expect(issues.map((i: any) => i.code)).toContain('unrecognized_keys'); + }); + + // …and the fold is what keeps us out of variant ②. + it('the fold strips the `id` that variant ② proves the spec rejects', () => { + const folded = foldFilterGroupToSpecRules(CAPTURED_TOOLBAR_GROUP); + expect(folded.ok).toBe(true); + expect(folded.ok && folded.rules.every((r) => !('id' in r))).toBe(true); + }); + + // Replay variant ③ — what the fold now emits. + it('#5159 variant ③: the folded rule array is ACCEPTED by ListViewSchema', () => { + const folded = foldFilterGroupToSpecRules(CAPTURED_TOOLBAR_GROUP); + expect(folded.ok).toBe(true); + const parsed = ListViewSchema.safeParse(viewBody(folded.ok ? folded.rules : undefined)); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + }); + + it('an empty rule list (filters cleared) is ACCEPTED by ListViewSchema', () => { + const parsed = ListViewSchema.safeParse(viewBody([])); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + }); + + it('every builder operator the fold normalizes survives ListViewSchema', () => { + const folded = foldFilterGroupToSpecRules({ + id: 'root', logic: 'and', + conditions: [ + { id: '1', field: 'a', operator: 'startsWith', value: 'x' }, + { id: '2', field: 'b', operator: 'isNull' }, + { id: '3', field: 'c', operator: 'greaterOrEqual', value: 5 }, + { id: '4', field: 'd', operator: 'notIn', value: ['x', 'y'] }, + ], + }); + expect(folded.ok).toBe(true); + const parsed = ListViewSchema.safeParse(viewBody(folded.ok ? folded.rules : undefined)); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + }); +}); diff --git a/packages/app-shell/src/views/viewFilterFold.ts b/packages/app-shell/src/views/viewFilterFold.ts new file mode 100644 index 0000000000..ab6353f3c8 --- /dev/null +++ b/packages/app-shell/src/views/viewFilterFold.ts @@ -0,0 +1,138 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * FilterBuilder → `@objectstack/spec` view filter, the WRITE direction. + * + * The FilterBuilder (`@object-ui/components`) speaks a grouped dialect — + * `{ id, logic, conditions }` with a per-row `id` and camelCase operators. + * `@objectstack/spec`'s `ListViewSchema.filter` / `ViewTab.filter` declare + * `z.array(ViewFilterRuleSchema)`: a FLAT list of `{ field, operator, value }`. + * Persisting the builder's group verbatim is a type error the server rejects + * with a `invalid_union` 422 — objectstack#5159, reproduced in a real browser + * on the list toolbar's `Filter → Add filter → save`. + * + * Contract-first (AGENTS.md #0.1): the producer folds, the spec is untouched. + * Widening `ListViewSchema.filter` to `union([rule[], FilterGroup])` would put + * two shapes in storage and force every reader to accept both — the exact debt + * this repo keeps paying down. + * + * The READ direction lives in `@object-ui/plugin-view` + * (`config/view-config-utils.ts`: `parseSpecFilter` / `toFilterGroup`); this is + * its missing half, extracted from the Studio inspector's `FilterBuilderField` + * (`metadata-admin/widgets.tsx`) which had the only copy, so the runtime + * toolbar and Studio now fold through ONE function rather than two dialects. + */ + +import { normalizeFilterOperator } from '@objectstack/spec/ui'; +import type { ViewFilterRule } from '@objectstack/spec/ui'; + +/** Why a group could not be folded to a flat spec rule list. */ +export type FilterFoldRefusal = + /** `logic: 'or'` over 2+ conditions — a flat rule array is AND-only. */ + | 'or_logic' + /** A condition that is itself a group — the spec rule list has no nesting. */ + | 'nested_group'; + +export type FilterFoldResult = + | { ok: true; rules: ViewFilterRule[] } + | { ok: false; reason: FilterFoldRefusal }; + +/** Loose shape of what the FilterBuilder hands back through `onChange`. */ +interface FilterGroupLike { + id?: string; + logic?: string; + conditions?: unknown[]; +} + +function isGroupLike(value: unknown): value is FilterGroupLike { + if (typeof value !== 'object' || value === null || Array.isArray(value)) return false; + const v = value as Record; + // A nested GROUP is recognised by carrying a logic operator or its own + // condition list — NOT merely by lacking `field`, which is also true of the + // blank row `Add filter` inserts before the user picks a column. + return Array.isArray(v.conditions) || v.logic === 'and' || v.logic === 'or'; +} + +/** + * Fold a FilterBuilder group into the spec's `ViewFilterRule[]`. + * + * Folding rules: + * + * - **Operators** are normalized through the spec's OWN + * {@link normalizeFilterOperator}, so the builder's camelCase ids + * (`notEquals`, `greaterOrEqual`, `startsWith`, `isNull`, …) land on the + * canonical vocabulary `ViewFilterRuleSchema` enumerates. Using the spec's + * exported map rather than a hand-kept table is what keeps this from + * becoming a second dialect — the previous local table in + * `metadata-admin/widgets.tsx` had drifted four operators behind the + * builder's dropdown (`startsWith`/`endsWith`/`isNull`/`isNotNull`). + * An operator the spec does not know is passed through VERBATIM so the + * server's enum rejects it loudly, rather than being coerced to `equals`. + * - **Blank rows are dropped.** `Add filter` inserts a row with `field: ''`; + * it is not yet a filter. (Same predicate the Studio inspector used.) + * - **`id` is stripped.** It is a React list key, not spec vocabulary. Two + * independent reasons, both measured: + * 1. `ViewFilterRuleSchema` is a `strictObject` over `{field, operator, + * value}` — the spec version this repo pins REJECTS a rule carrying `id` + * with `unrecognized_keys` on `filter.0`. (The issue's replay matrix saw + * variant ② accepted against a server running framework `main`, where + * #5154 tolerates the extra key; objectui's own pin does not. Keeping the + * id would only trade one 422 for another.) + * 2. Nothing downstream needs it persisted: the read path regenerates it — + * `parseSpecFilter`'s `parseTriplet` always mints `crypto.randomUUID()`, + * and `parseSingleOrNested` / `toFilterGroup` fall back to one — so the + * builder round-trip is lossless without it. + * `viewFilterFold.test.ts` pins both. + * - **`value` is carried verbatim**, including `''` (the row the toolbar emits + * the moment a field is picked). `ViewFilterRuleSchema.value` accepts it and + * rewriting it here would silently change what the user saved. + * + * Refusals (objectstack#5159, maintainer adjudication A1): a shape that cannot + * fold LOSSLESSLY is refused, never downgraded. `logic: 'or'` has no at-rest + * representation in a flat rule array, so quietly writing those conditions as + * AND would return a different record set than the one on screen. + * + * `logic: 'or'` over FEWER than two effective rules is folded rather than + * refused: with 0 or 1 condition, OR and AND select exactly the same records, + * so there is nothing to lose and nothing to downgrade. The refusal starts + * where the semantics actually diverge. + */ +export function foldFilterGroupToSpecRules(group: unknown): FilterFoldResult { + if (group == null) return { ok: true, rules: [] }; + + const conditions = isGroupLike(group) && Array.isArray(group.conditions) + ? group.conditions + : []; + + const rules: ViewFilterRule[] = []; + for (const condition of conditions) { + if (isGroupLike(condition)) return { ok: false, reason: 'nested_group' }; + if (typeof condition !== 'object' || condition === null) continue; + const c = condition as Record; + // Blank row the builder inserts before a column is chosen. + if (typeof c.field !== 'string' || c.field === '') continue; + const rule: ViewFilterRule = { + field: c.field, + operator: normalizeFilterOperator(c.operator) as ViewFilterRule['operator'], + }; + if (c.value !== undefined) rule.value = c.value as ViewFilterRule['value']; + rules.push(rule); + } + + const logic = isGroupLike(group) ? group.logic : undefined; + if (logic === 'or' && rules.length > 1) return { ok: false, reason: 'or_logic' }; + + return { ok: true, rules }; +} + +/** i18n key carrying the user-readable refusal for each {@link FilterFoldRefusal}. */ +export const FILTER_FOLD_REFUSAL_KEYS: Record = { + or_logic: 'console.objectView.filterOrNotSavable', + nested_group: 'console.objectView.filterNestedNotSavable', +}; diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 039b54328d..94f3f4cbd8 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -1496,6 +1496,8 @@ const ar = { objectNotFound: "الكائن غير موجود", objectNotFoundDescription: "الكائن \"{{objectName}}\" غير موجود في الإعدادات الحالية.", objectNotFoundHint: "تحقق من إعدادات التنقل في تطبيقك أو اختر كائنًا آخر من الشريط الجانبي.", + filterOrNotSavable: "يستخدم هذا الفلتر «أو» بين الشروط، وهو ما لا يمكن للعرض المحفوظ تخزينه بعد. لا يزال مطبقًا على هذه القائمة — أزل تجميع «أو» لحفظه في العرض.", + filterNestedNotSavable: "يستخدم هذا الفلتر مجموعات شروط متداخلة لا يمكن للعرض المحفوظ تخزينها. حوّله إلى قائمة شروط واحدة لحفظه في العرض.", allRecords: "جميع السجلات", exitDesignMode: "الخروج من وضع التصميم", enterDesignMode: "الدخول إلى وضع التصميم", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 293d052497..ef6feb1bfe 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -1496,6 +1496,8 @@ const de = { objectNotFound: "Objekt nicht gefunden", objectNotFoundDescription: "Das Objekt „{{objectName}}\" existiert in der aktuellen Konfiguration nicht.", objectNotFoundHint: "Überprüfen Sie Ihre App-Navigationseinstellungen oder wählen Sie ein anderes Objekt aus der Seitenleiste.", + filterOrNotSavable: "Dieser Filter verknüpft Bedingungen mit ODER, was eine gespeicherte Ansicht noch nicht ablegen kann. Er gilt weiterhin für diese Liste – entfernen Sie die ODER-Gruppierung, um ihn in der Ansicht zu speichern.", + filterNestedNotSavable: "Dieser Filter verwendet verschachtelte Bedingungsgruppen, die eine gespeicherte Ansicht nicht ablegen kann. Reduzieren Sie ihn auf eine einzelne Bedingungsliste, um ihn in der Ansicht zu speichern.", allRecords: "Alle Datensätze", exitDesignMode: "Designmodus beenden", enterDesignMode: "Designmodus starten", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 6d9955d604..cc3e48d531 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -1677,6 +1677,8 @@ const en = { objectNotFound: 'Object Not Found', objectNotFoundDescription: 'The object "{{objectName}}" does not exist in the current configuration.', objectNotFoundHint: 'Check your app navigation settings or select a different object from the sidebar.', + filterOrNotSavable: 'This filter uses OR between conditions, which a saved view cannot store yet. It still applies to this list — remove the OR grouping to save it to the view.', + filterNestedNotSavable: 'This filter uses nested condition groups, which a saved view cannot store. Flatten it to a single list of conditions to save it to the view.', systemViewReadonly: 'System view defined in code — read-only.', expandToPage: 'Open as full page', allRecords: 'All Records', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index ba7fae6a88..57e16f8e3e 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -1496,6 +1496,8 @@ const es = { objectNotFound: "Objeto no encontrado", objectNotFoundDescription: "El objeto \"{{objectName}}\" no existe en la configuración actual.", objectNotFoundHint: "Verifique la configuración de navegación de su aplicación o seleccione un objeto diferente en la barra lateral.", + filterOrNotSavable: "Este filtro une las condiciones con O, algo que una vista guardada aún no puede almacenar. Se sigue aplicando a esta lista: quite la agrupación O para guardarlo en la vista.", + filterNestedNotSavable: "Este filtro usa grupos de condiciones anidados, que una vista guardada no puede almacenar. Conviértalo en una única lista de condiciones para guardarlo en la vista.", allRecords: "Todos los registros", exitDesignMode: "Salir del modo de diseño", enterDesignMode: "Entrar en modo de diseño", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index fe0d0ef304..0bb66e6155 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -1496,6 +1496,8 @@ const fr = { objectNotFound: "Objet introuvable", objectNotFoundDescription: "L'objet « {{objectName}} » n'existe pas dans la configuration actuelle.", objectNotFoundHint: "Vérifiez les paramètres de navigation de votre application ou sélectionnez un autre objet dans la barre latérale.", + filterOrNotSavable: "Ce filtre relie les conditions par OU, ce qu'une vue enregistrée ne peut pas encore stocker. Il s'applique toujours à cette liste — supprimez le groupement OU pour l'enregistrer dans la vue.", + filterNestedNotSavable: "Ce filtre utilise des groupes de conditions imbriqués, qu'une vue enregistrée ne peut pas stocker. Aplatissez-le en une seule liste de conditions pour l'enregistrer dans la vue.", allRecords: "Tous les enregistrements", exitDesignMode: "Quitter le mode conception", enterDesignMode: "Entrer en mode conception", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index f806884a4f..f409225f0e 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -1496,6 +1496,8 @@ const ja = { objectNotFound: "オブジェクトが見つかりません", objectNotFoundDescription: "オブジェクト「{{objectName}}」は現在の設定に存在しません。", objectNotFoundHint: "アプリのナビゲーション設定を確認するか、サイドバーから別のオブジェクトを選択してください。", + filterOrNotSavable: "このフィルターは条件間に OR を使用しており、保存済みビューにはまだ保存できません。現在のリストには適用されています。ビューに保存するには OR グループを解除してください。", + filterNestedNotSavable: "このフィルターはネストされた条件グループを使用しており、保存済みビューには保存できません。ビューに保存するには単一の条件リストに展開してください。", allRecords: "すべてのレコード", exitDesignMode: "デザインモードを終了", enterDesignMode: "デザインモードに入る", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 2f9830d048..333ff3757b 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -1496,6 +1496,8 @@ const ko = { objectNotFound: "객체를 찾을 수 없음", objectNotFoundDescription: "객체 \"{{objectName}}\"이(가) 현재 구성에 존재하지 않습니다.", objectNotFoundHint: "앱 탐색 설정을 확인하거나 사이드바에서 다른 객체를 선택하세요.", + filterOrNotSavable: "이 필터는 조건 사이에 OR을 사용하며, 저장된 뷰에는 아직 저장할 수 없습니다. 현재 목록에는 계속 적용됩니다. 뷰에 저장하려면 OR 그룹을 해제하세요.", + filterNestedNotSavable: "이 필터는 중첩된 조건 그룹을 사용하며, 저장된 뷰에는 저장할 수 없습니다. 뷰에 저장하려면 단일 조건 목록으로 펼치세요.", allRecords: "모든 레코드", exitDesignMode: "디자인 모드 종료", enterDesignMode: "디자인 모드 시작", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 549584703a..e20c4d4472 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -1496,6 +1496,8 @@ const pt = { objectNotFound: "Objeto não encontrado", objectNotFoundDescription: "O objeto \"{{objectName}}\" não existe na configuração atual.", objectNotFoundHint: "Verifique as configurações de navegação do seu aplicativo ou selecione um objeto diferente na barra lateral.", + filterOrNotSavable: "Este filtro une as condições com OU, algo que uma visão salva ainda não consegue armazenar. Ele continua aplicado a esta lista — remova o agrupamento OU para salvá-lo na visão.", + filterNestedNotSavable: "Este filtro usa grupos de condições aninhados, que uma visão salva não consegue armazenar. Transforme-o em uma única lista de condições para salvá-lo na visão.", allRecords: "Todos os registros", exitDesignMode: "Sair do modo de design", enterDesignMode: "Entrar no modo de design", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index f44b43af37..123de7970a 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -1496,6 +1496,8 @@ const ru = { objectNotFound: "Объект не найден", objectNotFoundDescription: "Объект «{{objectName}}» не существует в текущей конфигурации.", objectNotFoundHint: "Проверьте настройки навигации приложения или выберите другой объект на боковой панели.", + filterOrNotSavable: "Этот фильтр объединяет условия через ИЛИ, что сохранённое представление пока не может хранить. Он по-прежнему применяется к этому списку — уберите группировку ИЛИ, чтобы сохранить его в представлении.", + filterNestedNotSavable: "Этот фильтр использует вложенные группы условий, которые сохранённое представление не может хранить. Преобразуйте его в один список условий, чтобы сохранить в представлении.", allRecords: "Все записи", exitDesignMode: "Выйти из режима дизайна", enterDesignMode: "Войти в режим дизайна", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 7f8d0ea5e5..712b34c68e 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -1672,6 +1672,8 @@ const zh = { objectNotFound: '未找到对象', objectNotFoundDescription: '对象"{{objectName}}"在当前配置中不存在。', objectNotFoundHint: '请检查您的应用导航设置或从侧边栏选择其他对象。', + filterOrNotSavable: '该筛选条件之间使用「或」,保存到视图暂不支持这种结构。它仍会作用于当前列表 —— 取消「或」分组后才能保存到视图。', + filterNestedNotSavable: '该筛选使用了嵌套条件分组,保存到视图不支持这种结构。请改为单层条件列表后再保存到视图。', allRecords: '所有记录', exitDesignMode: '退出设计模式', enterDesignMode: '进入设计模式',