diff --git a/.changeset/object-data-page-save-as-view-filter-fold.md b/.changeset/object-data-page-save-as-view-filter-fold.md new file mode 100644 index 0000000000..f25237965c --- /dev/null +++ b/.changeset/object-data-page-save-as-view-filter-fold.md @@ -0,0 +1,5 @@ +--- +'@object-ui/app-shell': patch +--- + +`ObjectDataPage`'s "Save as view" now folds the active URL drill conditions into `@objectstack/spec` `ViewFilterRule`s before persisting them, instead of writing the runtime filter triples verbatim into the view body. The page renders from a filter AST (`['stage', '=', 'open']`), but a saved view is a ViewItem whose `ListViewSchema.filter` declares `z.array(ViewFilterRuleSchema)` — `{ field, operator, value }` over the canonical operator words. Saving a drilled list therefore produced an off-spec ViewItem that the record gate rejects on `config.filter.0` ("expected object, received array"), so a view saved with drill conditions was already invalid the moment it was written (#3419). Operators are canonicalised through the spec's own `normalizeFilterOperator` — the same exit `viewFilterFold` uses for the FilterBuilder — so `=` becomes `equals` and `>=` becomes `greater_than_or_equal`, and `field` / `value` are carried through untouched. Contract-first: the fold is at the producer; no consumer was taught to accept triples. A condition whose operator has no canonical spelling is dropped from the persisted view with a debug-level note rather than written off-spec (the URL contract emits none such today). Saving a view with no drill conditions active is unchanged — no `filter` key is written, exactly as before. diff --git a/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts b/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts new file mode 100644 index 0000000000..159a534a09 --- /dev/null +++ b/packages/app-shell/src/views/ObjectDataPage.saveAsViewFilterFold.test.ts @@ -0,0 +1,167 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +// "Save as view" folds URL drill triples into spec ViewFilterRules +// (objectui#3419, tail of objectui#3375's viewEnvelope spec pin). +// +// `ObjectDataPage` renders from a runtime filter AST — `FilterTriple[]`, i.e. +// `[field, '=', value]`, the shape `parseUrlFilterTriples` produces and +// `ListView` sends as `$filter`. What "Save as view" PERSISTS is a ViewItem, +// and `ListViewSchema.filter` declares `z.array(ViewFilterRuleSchema)`: +// `{ field, operator, value }` over the canonical operator words. The two are +// different vocabularies for one key; writing the triples through verbatim +// produced an off-spec ViewItem that the record gate rejects with +// `config.filter.0 → "expected object, received array"`. +// +// This file pins the producer-side fold (AGENTS.md #0.1 — the consumer is NOT +// taught to accept triples). It asserts on `buildSaveAsViewSpec`, the whole +// spec-assembly step the callback delegates to, rather than on the fold helper +// alone: a test of the fold in isolation stays green if the call site goes back +// to `filter: urlFilters`, which is exactly the regression being pinned. +// +// Suite direction, decided before running: every case below is RED against the +// pre-#3419 producer (the raw-triple spec) and GREEN after — except +// `documents the pre-fix rejection`, which pins the SPEC's verdict on a raw +// triple and is green in both worlds by construction. + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { ViewItemSchema } from '@objectstack/spec/ui'; +import { URL_FILTER_OPS, type FilterTriple } from './drillUrlFilters'; +import { viewEnvelope } from './runtime-metadata-persistence'; +import { buildSaveAsViewSpec } from './ObjectDataPage'; + +/** The page's auto-derived column list (already field-security trimmed). */ +const COLUMNS = ['name', 'stage', 'amount']; + +/** What CreateViewDialog hands `handleSaveAsView`. */ +const DIALOG_CONFIG = { type: 'grid', label: 'Open deals', name: 'open_deals' }; + +/** Build the envelope exactly as `handleSaveAsView` does, then gate it. */ +function saveAsView(urlFilters: FilterTriple[]) { + const spec = buildSaveAsViewSpec(DIALOG_CONFIG, COLUMNS, urlFilters); + const env = viewEnvelope('crm_deal', spec, { + name: DIALOG_CONFIG.name, + label: DIALOG_CONFIG.label, + }); + return { spec, env, gate: ViewItemSchema.safeParse(env) }; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('Save as view folds URL drill triples to spec rules (objectui#3419)', () => { + it('documents the pre-fix rejection: a raw triple is not a ViewFilterRule', () => { + // The exact verdict from the issue's repro, kept as executable evidence of + // WHY the fold exists. This asserts the spec's behaviour, not ours, so it + // is green before and after — the regression pin is the next test. + const env = viewEnvelope( + 'crm_deal', + { type: 'grid', columns: COLUMNS, filter: [['stage', '=', 'open']] }, + { name: 'raw', label: 'Raw' }, + ); + const res = ViewItemSchema.safeParse(env); + expect(res.success).toBe(false); + expect(res.error?.issues).toContainEqual( + expect.objectContaining({ + code: 'invalid_type', + expected: 'object', + path: ['config', 'filter', 0], + }), + ); + }); + + it('folds an equality drill and the envelope passes the ViewItem gate', () => { + const { spec, gate } = saveAsView([['stage', '=', 'open']]); + expect(spec.filter).toEqual([{ field: 'stage', operator: 'equals', value: 'open' }]); + expect( + gate.success, + `ViewItem rejected by spec: ${JSON.stringify(gate.error?.issues)}`, + ).toBe(true); + }); + + it('folds a date-bucket drill (two range triples on one field)', () => { + // What a chart/date drill emits: `filter[close_date][gte]` + `[lt]`. + const { spec, gate } = saveAsView([ + ['close_date', '>=', '2026-01-01'], + ['close_date', '<', '2026-02-01'], + ]); + expect(spec.filter).toEqual([ + { field: 'close_date', operator: 'greater_than_or_equal', value: '2026-01-01' }, + { field: 'close_date', operator: 'less_than', value: '2026-02-01' }, + ]); + expect(gate.success).toBe(true); + }); + + it('folds EVERY operator the URL contract can emit to a canonical spelling', () => { + // Derived from `URL_FILTER_OPS` (plus `=`, which has no `[op]` suffix form) + // so a range operator added to the URL contract fails HERE rather than at + // publish time. `parseUrlFilterTriples` emits nothing outside this set. + const emittable = ['=', ...Object.values(URL_FILTER_OPS)]; + expect(emittable).toEqual(['=', '>=', '<=', '>', '<']); + + const { spec, gate } = saveAsView( + emittable.map((op, i) => ['f' + i, op, String(i)] as FilterTriple), + ); + expect(spec.filter.map((r: { operator: string }) => r.operator)).toEqual([ + 'equals', + 'greater_than_or_equal', + 'less_than_or_equal', + 'greater_than', + 'less_than', + ]); + expect(gate.success).toBe(true); + }); + + it('carries field and value through untouched', () => { + // Placeholder resolution has already run upstream; the fold must not + // re-interpret what it produced (`''` included — the spec accepts it). + const { spec } = saveAsView([ + ['owner_id', '=', 'usr_00042'], + ['note', '=', ''], + ]); + expect(spec.filter).toEqual([ + { field: 'owner_id', operator: 'equals', value: 'usr_00042' }, + { field: 'note', operator: 'equals', value: '' }, + ]); + }); + + it('drops a triple with no canonical operator instead of persisting it off-spec', () => { + // Defence in depth: `parseUrlFilterTriples` cannot emit `~=` today. If the + // URL contract ever grows an operator the spec has no word for, the saved + // view loses that one condition (with a debug note) rather than becoming a + // body the record gate rejects whole. + const debug = vi.spyOn(console, 'debug').mockImplementation(() => {}); + const { spec, gate } = saveAsView([ + ['stage', '=', 'open'], + ['score', '~=', '10'], + ]); + expect(spec.filter).toEqual([{ field: 'stage', operator: 'equals', value: 'open' }]); + expect(gate.success).toBe(true); + expect(debug).toHaveBeenCalledWith(expect.stringContaining('score')); + }); + + it('omits `filter` entirely when every triple was dropped', () => { + vi.spyOn(console, 'debug').mockImplementation(() => {}); + const { spec, gate } = saveAsView([['score', '~=', '10']]); + expect('filter' in spec).toBe(false); + expect(gate.success).toBe(true); + }); +}); + +describe('Save as view without drill filters is unchanged (objectui#3419)', () => { + it('writes no `filter` key and keeps the dialog payload byte-identical', () => { + const { spec, gate } = saveAsView([]); + expect(spec).toEqual({ ...DIALOG_CONFIG, columns: COLUMNS }); + expect(gate.success).toBe(true); + }); + + it('prefers the dialog columns over the page fallback, as before', () => { + const spec = buildSaveAsViewSpec({ ...DIALOG_CONFIG, columns: ['name'] }, COLUMNS, []); + expect(spec.columns).toEqual(['name']); + }); + + it('falls back to the page columns when the dialog carried an empty list', () => { + const spec = buildSaveAsViewSpec({ ...DIALOG_CONFIG, columns: [] }, COLUMNS, []); + expect(spec.columns).toEqual(COLUMNS); + }); +}); diff --git a/packages/app-shell/src/views/ObjectDataPage.tsx b/packages/app-shell/src/views/ObjectDataPage.tsx index 2e891f7563..25dc9613e5 100644 --- a/packages/app-shell/src/views/ObjectDataPage.tsx +++ b/packages/app-shell/src/views/ObjectDataPage.tsx @@ -42,11 +42,14 @@ import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; import { usePermissions, useFieldPermissions } from '@object-ui/permissions'; import { useAuth, useIsWorkspaceAdmin } from '@object-ui/auth'; import { resolveFilterPlaceholders } from '@object-ui/core'; +import { normalizeFilterOperator, ViewFilterRuleSchema } from '@objectstack/spec/ui'; +import type { ViewFilterRule } from '@objectstack/spec/ui'; import { parseUserFilterParams, applyUserFilterParams } from './userFilterUrlState'; import { parseUrlFilterTriples, groupFilterChips, deleteFieldFilterParams, + URL_FILTER_OPS, type FilterTriple, } from './drillUrlFilters'; import { @@ -72,6 +75,112 @@ import { useTenancyPosture } from '../hooks/useTenancyPosture'; const USER_FILTER_TYPES = new Set(['select', 'multiselect', 'radio', 'enum', 'boolean']); const MAX_USER_FILTERS = 4; +/** + * URL drill triple operator → the spec's OWN alias spelling. + * + * `parseUrlFilterTriples` speaks ObjectQL **symbols** (`=`, `>=`, `<=`, `>`, + * `<`) because a triple is what the runtime filter AST consumes. + * `ViewFilterRuleSchema.operator` enumerates a different vocabulary — the + * canonical words (`equals`, `greater_than_or_equal`, …). `normalizeFilterOperator`, + * the single canonicaliser this repo is allowed to use (it is also + * `viewFilterFold`'s exit), knows the spec's *word* aliases (`eq`, `gte`, `lt`, + * …) but not the symbols: hand it `'='` and it returns `'='` verbatim, which the + * enum then rejects. + * + * So this table is a **symbol → alias bridge, not a second canonical map**. + * The range half is derived by inverting `URL_FILTER_OPS`, whose suffixes + * (`gte`/`lte`/`gt`/`lt`) are already spec alias keys — a range operator added + * to the URL contract is therefore bridged here automatically. `'='` is the one + * hand-written entry, because equality has no `[op]` suffix form to invert. + * Canonicalisation itself stays in `normalizeFilterOperator`, exactly once. + */ +const TRIPLE_OP_TO_SPEC_ALIAS: Record = { + '=': 'eq', + ...Object.fromEntries( + Object.entries(URL_FILTER_OPS).map(([suffix, symbol]) => [symbol, suffix]), + ), +}; + +/** + * Fold URL drill triples into the spec's `ViewFilterRule[]` (objectui#3419). + * + * "Save as view" persists a **ViewItem**, and `ListViewSchema.filter` declares + * `z.array(ViewFilterRuleSchema)` — a flat list of `{ field, operator, value }` + * over the canonical operator vocabulary. The drill triples this page renders + * from the URL are the *runtime AST* shape; writing them into the view body + * verbatim produced an off-spec ViewItem (`config.filter.0` → "expected object, + * received array"). + * + * Contract-first (AGENTS.md #0.1): the fold happens **here, at the producer**. + * The alternative — teaching `ViewItemSchema`'s consumers to also accept + * triples — would put two filter dialects at rest, the exact debt this repo + * keeps paying down. Same reasoning, and the same `normalizeFilterOperator` + * exit, as `viewFilterFold.foldFilterGroupToSpecRules` (the FilterBuilder's + * half of this problem, objectstack#5159). + * + * `field` and `value` are carried verbatim. A triple whose operator has no + * canonical spelling — or whose value the rule schema refuses — is **dropped** + * from the persisted view with a debug-level note, never written off-spec: + * declared = enforced, and a view body that fails the record gate is rejected + * whole at publish time, which would lose the user's other conditions too. + * (`parseUrlFilterTriples` only ever emits the five operators bridged above, so + * the drop path is defence in depth against the URL contract growing an + * operator the spec has no word for.) + */ +function foldUrlFilterTriplesToSpecRules(triples: FilterTriple[]): ViewFilterRule[] { + const rules: ViewFilterRule[] = []; + for (const [field, op, value] of triples) { + const rule: Record = { + field, + operator: normalizeFilterOperator(TRIPLE_OP_TO_SPEC_ALIAS[op] ?? op), + }; + // `viewFilterFold` carries `''` through as a real value; only a genuinely + // absent one is omitted (unary operators take none). + if (value !== undefined) rule.value = value; + const parsed = ViewFilterRuleSchema.safeParse(rule); + if (!parsed.success) { + console.debug( + `[ObjectDataPage] Dropped URL filter on "${field}" from the saved view:` + + ` operator "${op}" has no canonical ViewFilterRule form.`, + ); + continue; + } + rules.push(parsed.data); + } + return rules; +} + +/** + * Assemble the list-view `spec` that "Save as view" hands to `viewEnvelope`. + * + * The whole producer step lives here rather than inline in the callback so it + * can be pinned against the real record gate (`ViewItemSchema`) without + * mounting the page and its provider stack — the fold above is only worth + * having if the code that PERSISTS actually goes through it, and a test on the + * fold alone would stay green if the call site went back to raw triples. + * + * `config` is the CreateViewDialog payload (`{ type, label, name, [type]: + * subConfig }`); `fallbackColumns` is this page's auto-derived, field-security + * trimmed column list, used only when the dialog carried none. + * + * Exported for `ObjectDataPage.saveAsViewFilterFold.test.ts`. @internal + */ +export function buildSaveAsViewSpec( + config: Record, + fallbackColumns: string[], + urlFilters: FilterTriple[], +): Record { + const filterRules = foldUrlFilterTriplesToSpecRules(urlFilters); + return { + ...config, + columns: + Array.isArray(config.columns) && config.columns.length > 0 ? config.columns : fallbackColumns, + // An all-dropped fold writes no `filter` key at all, byte-identical to a + // save with no drill conditions active. + ...(filterRules.length ? { filter: filterRules } : {}), + }; +} + export function ObjectDataPage({ dataSource, objects }: any) { const { appName, objectName } = useParams(); const { t } = useObjectTranslation(); @@ -247,11 +356,10 @@ export function ObjectDataPage({ dataSource, objects }: any) { const handleSaveAsView = React.useCallback( async (config: Record & { type: string; label: string }) => { try { - const spec: Record = { - ...config, - columns: Array.isArray(config.columns) && config.columns.length > 0 ? config.columns : columns, - ...(urlFilters.length ? { filter: urlFilters } : {}), - }; + // The URL conditions are folded into spec `ViewFilterRule`s on the way + // out (#3419). What renders this page is a runtime filter AST (triples); + // what gets PERSISTED must satisfy `ListViewSchema.filter`. + const spec: Record = buildSaveAsViewSpec(config, columns, urlFilters); // #2767 P1: unified identity — the qualified `.` name is the // URL segment AND the body identity. #2767 P4: land on the new draft in // preview mode so it's visible and one click from Publish.