diff --git a/.changeset/grid-filter-input-spelling-4041.md b/.changeset/grid-filter-input-spelling-4041.md new file mode 100644 index 0000000000..4572265ed5 --- /dev/null +++ b/.changeset/grid-filter-input-spelling-4041.md @@ -0,0 +1,54 @@ +--- +"@object-ui/plugin-grid": patch +--- + +`object-grid` publishes the filter key it actually reads: `filter`, singular (objectui#4041) + +The registration declared plural `filters` while `ObjectGrid` reads singular +`schema.filter`, and `schema.filters` had **zero** read points anywhere in the +renderer. Both halves of that mismatch were silent, in opposite directions: + +- An author following the published vocabulary wrote `filters: [...]`. The save + gate accepted it — `sdui-parser/src/validate.ts` walks a node's props against + the block's `inputs`, and `filters` was there — the renderer never read it, and + the grid answered with **the whole table**. No error at authoring time, none at + runtime, and a wider answer is not visibly wrong. +- The spelling that actually worked, `filter`, was undeclared, so writing it was + reported as `unknown-prop`. + +Published word and runtime read pointed at opposite keys, on a shipped authoring +surface. `list-view` — the sibling block, same family — has always declared the +singular and read the singular. + +**The plural is removed, not taught to the renderer** (maintainer ruling +2026-08-10, option A). It has no read point on any ref, so no working grid can +depend on it: this deletes a key with no users rather than a contract. Teaching +the renderer to read `filters` too was the rejected alternative — it would have +hardened a misspelling into a second de-facto contract for the same concept. + +`patch` rather than `minor`/`major` on that same fact. The removed key never +reached the query on any released version, so nothing that worked stops working; +what changes is that a filter written under the published name now takes effect. + +**The read point now lowers through `toFilterNode`**, which is what makes the +newly-reachable key honest rather than merely reachable. Until now the only value +that could arrive at `schema.filter` was an ObjectQL AST synthesized by +`ElementDataSourceGate`, and copying that onto `$filter` verbatim was correct. An +author writes the spec's view vocabulary instead — `ViewFilterRule[]`, +`[{ field, operator, value }]` — and that shape byte-copied onto `$filter` is +refused on the wire: `isFilterAST` is false for an array of objects and the data +API answers `400 INVALID_FILTER` (measured against a real backend in +objectui#3431). Declaring the key without this hop would have traded a silent +wrong answer for a guaranteed failure, which is not a fix. `toFilterNode` is the +repo's single lowering hop before the wire and every other consumer on this chain +already went through it — `plugin-list`'s `buildEffectiveFilter`, `plugin-view`'s +`ObjectView`, `plugin-detail`'s `RelatedList`; this read point was the last one +that did not. + +Two behaviour changes ride along at that read point, both narrow and both toward +the shared sink's documented contract: a MongoDB-style object `filter` is now +converted instead of silently dropped (the old `Array.isArray` guard read false +for it, and the grid returned every record — the same defect `buildEffectiveFilter` +fixed one package over), and a declared-but-empty `filter: []` now skips `$filter` +rather than sending an empty one. The fetch and the server-side export read the +same lowered value, so the downloaded file cannot disagree with the screen. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index b5fa0ab1c8..932356b0d6 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -36,7 +36,7 @@ import { RefreshIndicator, } from '@object-ui/components'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, listViewPredicates, isProjectableField, isExpandableFieldType } from '@object-ui/core'; +import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, listViewPredicates, isProjectableField, isExpandableFieldType, toFilterNode } from '@object-ui/core'; import { usePermissions } from '@object-ui/permissions'; import { ChevronRight, ChevronDown, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react'; import { useRowColor } from './useRowColor'; @@ -437,7 +437,31 @@ export const ObjectGrid: React.FC = ({ const effectiveApiOps = objectName ? getObjectApiOperations(objectName) : undefined; const schemaFields = schema.fields; const schemaColumns = schema.columns; - const schemaFilter = schema.filter; + // The view's declared filter, lowered ONCE through the repo's single filter + // sink for both consumers below (the fetch and the server-side export). + // + // The lowering is what makes the authoring surface honest rather than merely + // reachable (objectui#4041). Until this block published `filter`, the only + // thing that could arrive here was an ObjectQL AST synthesized by + // `ElementDataSourceGate`, and passing that through verbatim was right. An + // AUTHOR writes the spec's view vocabulary instead — `ViewFilterRule[]`, + // `[{ field, operator, value }]` — and that shape byte-copied onto `$filter` + // is refused on the wire: `isFilterAST` is false for an array of objects and + // the data API answers `400 INVALID_FILTER` (measured against a real backend + // in objectui#3431). Declaring the key without this hop would have traded a + // silent wrong answer for a guaranteed failure, which is not a fix. + // + // `toFilterNode` is that hop by design — the last stop before the wire, where + // a value legitimately leaves the spec's view vocabulary and becomes an AST + // (see its doc for why the fold cannot live at the producer). It passes AST + // nodes through untouched, so the `ElementDataSourceGate` path is unchanged; + // it also collects the MongoDB-style object shape, and returns `undefined` + // for an absent or empty source so we skip `$filter` rather than sending an + // empty array. `plugin-list`'s `buildEffectiveFilter` and `plugin-view`'s + // `ObjectView` already reach the wire through this same sink; this read point + // was the last consumer on the chain that did not. + const schemaFilterSource = schema.filter; + const schemaFilter = useMemo(() => toFilterNode(schemaFilterSource), [schemaFilterSource]); const schemaSort = schema.sort; const schemaPagination = schema.pagination; const schemaPageSize = schema.pageSize; @@ -610,8 +634,14 @@ export const ObjectGrid: React.FC = ({ $skip: (serverPage - 1) * serverPageSize, }; - // Support new filter format - if (schemaFilter && Array.isArray(schemaFilter)) { + // The block's declared `filter` input, already lowered to an AST at + // the read point above. `undefined` means "no filter declared" — + // including a declared-but-empty array, which `toFilterNode` folds + // away so an empty `$filter` never goes out. The `Array.isArray` guard + // this replaces predates the lowering and was itself a silent drop: + // it read false for the MongoDB-style object shape, so such a filter + // went missing and the grid answered with every record. + if (schemaFilter !== undefined) { params.$filter = schemaFilter; } else if (schema.defaultFilters) { // Legacy support @@ -1414,6 +1444,10 @@ export const ObjectGrid: React.FC = ({ const cols = generateColumns().filter((c: any) => c.accessorKey !== '_actions'); const fields = cols.map((c: any) => c.accessorKey).filter(Boolean); + // Same lowered value the fetch above sends, which is what keeps the + // downloaded file agreeing with the screen: both read `schemaFilter` + // AFTER `toFilterNode`, so an authored `ViewFilterRule[]` cannot reach + // the wire as bare rule objects on one path and as AST on the other. const filter = Array.isArray(schemaFilter) ? schemaFilter : undefined; const sort = Array.isArray(schemaSort) ? schemaSort diff --git a/packages/plugin-grid/src/__tests__/gridFilterInputSpelling.test.tsx b/packages/plugin-grid/src/__tests__/gridFilterInputSpelling.test.tsx new file mode 100644 index 0000000000..24d4371c59 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/gridFilterInputSpelling.test.tsx @@ -0,0 +1,211 @@ +/** + * 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. + * + * `object-grid` — the filter written under the DECLARED input name must reach + * the query (objectui#4041, source thread objectstack#7119). + * + * ## What was wrong + * + * The registration published plural `filters` while `ObjectGrid` read singular + * `schema.filter`, and `schema.filters` had zero read points anywhere. Both + * halves failed quietly, in opposite directions: + * + * - an author following the published vocabulary wrote `filters: [...]`, the + * save gate accepted it (`sdui-parser/src/validate.ts` walks a node's props + * against `inputs`, and `filters` was there), the renderer never read it, + * and the grid answered with the WHOLE TABLE — no error anywhere; + * - the spelling that actually worked, `filter`, was not declared, so writing + * it was reported as `unknown-prop`. + * + * Published vocabulary and runtime read pointed at opposite keys. That is + * objectstack#4413's shape in the "declaration ≠ read point" variant + * (objectui#3407). + * + * ## What these tests pin + * + * The card's pin, and it is deliberately written to read the name OUT of the + * registry rather than hard-coding `'filter'`: a test that spells the key + * itself would keep passing if the declaration drifted again, which is exactly + * the defect. So `declaredFilterInput()` below is the subject, and every + * behavioural assertion writes its schema under that name. + * + * The second pin is the constraint recorded on the source thread: reaching the + * read point is not enough. An author writes the spec's view vocabulary + * (`ViewFilterRule[]` — `[{ field, operator, value }]`), and that shape copied + * byte-for-byte onto `$filter` is refused on the wire (`isFilterAST` is false + * for an array of objects; the data API answers `400 INVALID_FILTER`, measured + * against a real backend in objectui#3431). Declaring the key without lowering + * it through `toFilterNode` would have swapped a silent wrong answer for a + * guaranteed failure. So the pin is `$filter` carrying LOWERED AST, not merely + * `$filter` being present. + * + * The last one is the historical record the card asked for: the old plural is + * gone from the declaration, and — asserted behaviourally, not just by absence + * — a schema written the way the old declaration invited never filtered + * anything. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +// Registers `object-grid` and its `view:grid` alias. +import '../index'; + +/** The two tags this one renderer is published under. */ +const GRID_TAGS = [ + { label: 'object-grid', type: 'object-grid', namespace: undefined }, + { label: 'view:grid', type: 'grid', namespace: 'view' }, +] as const; + +/** + * The block's declared filter input, read from the registration. + * + * The whole defect was a declaration that disagreed with the renderer, so the + * declaration is the input to these tests, never a constant restated here. + */ +function declaredFilterInput(type: string, namespace?: string) { + const inputs = (ComponentRegistry.getConfig(type, namespace) as any)?.inputs ?? []; + return inputs.find((i: any) => /^filters?$/.test(i?.name)); +} + +function makeAdapter() { + return { + find: vi.fn().mockResolvedValue({ data: [{ id: '1', name: 'Acme', stage: 'won' }], total: 1 }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'account', + fields: { id: { type: 'text' }, name: { type: 'text' }, stage: { type: 'text' } }, + }), + }; +} + +async function findParamsFor(schema: Record) { + const adapter = makeAdapter(); + render( + + + , + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + return (adapter.find.mock.calls[0] as [string, any])[1]; +} + +describe('object-grid filter input — declaration matches the read point (objectui#4041)', () => { + it.each(GRID_TAGS)('$label declares the filter input in the SINGULAR', ({ type, namespace }) => { + const input = declaredFilterInput(type, namespace); + // Not `toBeDefined()` on a hard-coded `'filter'` lookup: the regex above + // accepts either spelling so this assertion reports WHICH one is published. + expect(input?.name).toBe('filter'); + expect(input?.type).toBe('array'); + }); + + it('declares one spelling across both tags, so the alias cannot drift', () => { + // The alias is the same renderer, so a second declaration is a second + // chance to disagree with it. (The third alignment the ruling names — with + // the sibling `list-view` block — is not asserted from here: `plugin-list` + // is not a dependency of this package, and adding one for a test would buy + // the pin with a package edge. It is verified in the PR: + // `packages/plugin-list/src/index.tsx:52,:93` declare `filter` too.) + const [a, b] = GRID_TAGS.map(({ type, namespace }) => declaredFilterInput(type, namespace)); + expect(a?.name).toBe(b?.name); + }); + + it('no longer declares the plural `filters` on either tag', () => { + for (const { type, namespace } of GRID_TAGS) { + const names = ((ComponentRegistry.getConfig(type, namespace) as any)?.inputs ?? []) + .map((i: any) => i?.name); + expect(names).not.toContain('filters'); + } + }); +}); + +describe('object-grid — the declared name reaches `$filter` (objectui#4041)', () => { + it.each(GRID_TAGS)( + '$label sends the filter written under its declared input name', + async ({ type, namespace }) => { + const key = declaredFilterInput(type, namespace)!.name; + const tag = namespace ? `${namespace}:${type}` : type; + const params = await findParamsFor({ + type: tag, + objectName: 'account', + columns: [{ field: 'name' }], + [key]: [['stage', '=', 'won']], + }); + // The card's pin, stated exactly: write a filter under the published + // word, get `$filter` in the query. + expect(params.$filter).toEqual([['stage', '=', 'won']]); + }, + ); + + it('lowers the spec view vocabulary to AST instead of sending rule objects', async () => { + // `ViewFilterRule[]` is what a saved view and a spec-following author both + // write. Sent verbatim the wire face answers `400 INVALID_FILTER` + // (objectui#3431), so "it arrived" is not the assertion — "it arrived in a + // shape the server accepts" is. + const key = declaredFilterInput('object-grid')!.name; + const params = await findParamsFor({ + type: 'object-grid', + objectName: 'account', + columns: [{ field: 'name' }], + [key]: [{ field: 'stage', operator: 'equals', value: 'won' }], + }); + expect(params.$filter).toEqual([['stage', 'equals', 'won']]); + // Stated the other way too, because this is the half that would rot + // silently: no bare rule OBJECT may survive into the outgoing filter. + for (const node of params.$filter as unknown[]) { + expect(Array.isArray(node)).toBe(true); + } + }); + + it('leaves an already-composed AST untouched', async () => { + // The `ElementDataSourceGate` path (and any URL triple) arrives here as AST + // already. `toFilterNode` must be a no-op for it — a second lowering pass + // would be the mirror-image defect. + const key = declaredFilterInput('object-grid')!.name; + const params = await findParamsFor({ + type: 'object-grid', + objectName: 'account', + columns: [{ field: 'name' }], + [key]: ['and', ['stage', '=', 'won'], ['amount', '>', 100]], + }); + expect(params.$filter).toEqual(['and', ['stage', '=', 'won'], ['amount', '>', 100]]); + }); + + it('skips `$filter` entirely for a declared-but-empty filter', async () => { + // Rather than sending `$filter: []`, which asks the server a question with + // no content. `toFilterNode`'s documented contract, now honoured here too. + const key = declaredFilterInput('object-grid')!.name; + const params = await findParamsFor({ + type: 'object-grid', + objectName: 'account', + columns: [{ field: 'name' }], + [key]: [], + }); + expect(params.$filter).toBeUndefined(); + }); +}); + +describe('object-grid — the retired plural spelling (historical record, objectui#4041)', () => { + it('never reached the query, which is why deleting it breaks no working usage', async () => { + // The state of the world BEFORE this change, pinned so the ruling's premise + // stays checkable: `filters` was published, accepted by the save gate, and + // dropped on the floor. Nobody could have a working grid that depends on + // it, so option A removes a key with no users rather than a contract. + const params = await findParamsFor({ + type: 'object-grid', + objectName: 'account', + columns: [{ field: 'name' }], + filters: [['stage', '=', 'won']], + }); + expect(params.$filter).toBeUndefined(); + }); +}); diff --git a/packages/plugin-grid/src/index.tsx b/packages/plugin-grid/src/index.tsx index 677e2b490f..2c53900327 100644 --- a/packages/plugin-grid/src/index.tsx +++ b/packages/plugin-grid/src/index.tsx @@ -54,8 +54,10 @@ export type { SplitPaneGridProps } from './SplitPaneGrid'; * The keys `ObjectGrid` reads for its own query — every one of them, which makes * this the only block where the spec's binding maps across without a gap. * `columns` is a FIELD list here (so a saved view's columns belong on it), the - * filter and sort go straight to `$filter` / `$orderby`, and the row cap is read - * as `pagination.pageSize` (`ObjectGrid.tsx`, `serverPageSize`). + * filter and sort reach `$filter` / `$orderby` (the filter via `toFilterNode`, + * the repo's single lowering hop before the wire — it passes the AST this gate + * composes through untouched), and the row cap is read as `pagination.pageSize` + * (`ObjectGrid.tsx`, `serverPageSize`). */ const OBJECT_GRID_DATA_SOURCE: ElementDataSourceMapping = { columns: true, @@ -86,15 +88,40 @@ export const ObjectGridRenderer: React.FC<{ schema: any; [key: string]: any }> = ); }; +/** + * The authoring surface for this block's query filter, in ONE spelling. + * + * It used to be published as plural `filters` while `ObjectGrid` read singular + * `schema.filter` and nothing anywhere read `schema.filters` (objectui#4041). + * Both halves were silent: `sdui-parser`'s save gate walks a node's props + * against these `inputs`, so `filters` was accepted and then dropped by the + * renderer — an author (very often an AI author) writing the published word got + * an unfiltered full-table answer with no error — while the spelling that + * actually works was reported as `unknown-prop`. The published vocabulary and + * the runtime read pointed at opposite keys. + * + * Resolved toward the implementation (maintainer ruling 2026-08-10, option A): + * the declaration is singular `filter`, aligned three ways — with the renderer's + * read point (`ObjectGrid.tsx`, `schema.filter`), with the sibling `list-view` + * block (`plugin-list/src/index.tsx`, `{ name: 'filter', … }`), and with the + * spec's own `filter` vocabulary. The plural is DELETED rather than taught to + * the renderer: it has zero read points on any ref, so no working in-the-wild + * usage depends on it, and reading it too would harden a misspelling into a + * second de-facto contract (AGENTS.md #0.1). + * + * Shared by both registrations below so the alias cannot drift from the block. + */ +const GRID_QUERY_INPUTS = [ + { name: 'objectName', type: 'string', label: 'Object Name', required: true }, + { name: 'columns', type: 'array', label: 'Columns' }, + { name: 'filter', type: 'array', label: 'Filter' }, +] as const; + ComponentRegistry.register('object-grid', ObjectGridRenderer, { namespace: 'plugin-grid', label: 'Object Grid', category: 'plugin', - inputs: [ - { name: 'objectName', type: 'string', label: 'Object Name', required: true }, - { name: 'columns', type: 'array', label: 'Columns' }, - { name: 'filters', type: 'array', label: 'Filters' }, - ] + inputs: GRID_QUERY_INPUTS.map((i) => ({ ...i })), }); // Alias for view namespace - this allows using { type: 'view:grid' } in schemas @@ -107,11 +134,8 @@ ComponentRegistry.register('grid', ObjectGridRenderer, { skipFallback: true, label: 'Data Grid', category: 'view', - inputs: [ - { name: 'objectName', type: 'string', label: 'Object Name', required: true }, - { name: 'columns', type: 'array', label: 'Columns' }, - { name: 'filters', type: 'array', label: 'Filters' }, - ] + // Same renderer, therefore the same declared surface — see GRID_QUERY_INPUTS. + inputs: GRID_QUERY_INPUTS.map((i) => ({ ...i })), }); // Register import-wizard component