diff --git a/.changeset/objectview-canonical-table-key-forwarding.md b/.changeset/objectview-canonical-table-key-forwarding.md new file mode 100644 index 000000000..4d936496f --- /dev/null +++ b/.changeset/objectview-canonical-table-key-forwarding.md @@ -0,0 +1,40 @@ +--- +'@object-ui/plugin-view': patch +--- + +`ObjectView` forwards the canonical `table` keys — `pagination` / `selection` / `filter` / `sort` now take effect, and the deprecated spellings keep working as aliases. + +`ObjectViewSchema.table` is documented as inheriting from `ObjectGridSchema`, +but `ObjectView` does not spread it: it forwards a hand-written whitelist of +keys, and that whitelist carried only the **deprecated** half of four pairs. +`pageSize`, `selectable`, `defaultFilters` and `defaultSort` were forwarded; +their canonical successors `pagination`, `selection`, `filter` and `sort` had +**no read point at all** in the file. + +So an author who wrote the shape the type recommends — `table: { pagination: +{ pageSize: 25 } }`, having read `@deprecated Use pagination.pageSize instead` +on the key they were avoiding — got a view that compiled, read correctly, and +did nothing. There was no failure signal at any layer: the key is declared on +`ObjectGridSchema`, `ObjectGrid` already reads it, and only this forwarding hop +dropped it. That silent success is the defect being closed. + +All four canonical keys are now forwarded at every site that forwarded their +deprecated counterpart: the grid schema, the non-grid data fetch +(kanban / gallery / calendar / timeline / gantt / map), and the delegated +`renderListView` schema. When an author writes both spellings the **canonical +key wins** — it is read first in the chains `ObjectView` resolves itself, and on +the grid path both slots are forwarded so `ObjectGrid`'s existing canonical-first +resolution decides, keeping the two layers in agreement. + +Nothing that worked before changes. The deprecated spellings are still read and +are still the value used when they are the only one written; no canonical value +is synthesised from a deprecated one, so `ObjectGrid`'s `pagination`-keyed +behaviour is untouched for views that only ever wrote `pageSize`. The two +precedence segments ahead of `table` — a named `listViews` entry, then the +active view — are untouched, and a named view still outranks a `table` default. + +Declaration-surface note: `table` remains `Partial< Omit< ObjectGridSchema, … > >`, +which the `BaseSchema` index signature collapses to zero declared members, so +editor completion still offers no keys and a misspelling is still accepted +silently. That half is deferred to the structural track and is not addressed +here. diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index d6a7c9b58..0f077bb46 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -600,11 +600,16 @@ export const ObjectView: React.FC = ({ // correctly as a grid and returned everything as a calendar/kanban/ // gallery. It also keeps each source as its own child of the `and` // rather than spreading it, which is what a `ViewFilterRule[]` needs. + // The `table` segment reads the CANONICAL key first and the deprecated + // one only as its alias (objectui#5102). The two view segments ahead of + // it are untouched — this extends the last segment only. const finalFilter = mergeFilterNodes( - currentNamedViewConfig?.filter || activeView?.filter || schema.table?.defaultFilters, + currentNamedViewConfig?.filter || activeView?.filter + || schema.table?.filter || schema.table?.defaultFilters, ); - const sort = currentNamedViewConfig?.sort || activeView?.sort || schema.table?.defaultSort || undefined; + const sort = currentNamedViewConfig?.sort || activeView?.sort + || schema.table?.sort || schema.table?.defaultSort || undefined; // Auto-inject $expand for lookup/master_detail fields. // Use a ref instead of the state variable to avoid re-running this effect @@ -1016,23 +1021,61 @@ export const ObjectView: React.FC = ({ }, [schema.objectName, schema.table?.fields, currentNamedViewConfig, activeView]); // Build grid schema (default content renderer) - const gridSchema: ObjectGridSchema = useMemo(() => ({ - type: 'object-grid', - objectName: schema.objectName, - title: schema.table?.title, - description: schema.table?.description, - fields: currentNamedViewConfig?.columns || activeView?.columns || schema.table?.fields, - columns: currentNamedViewConfig?.columns || activeView?.columns || schema.table?.columns, - operations: { - ...operations, - create: false, // Create is handled by the view's create button - }, - defaultFilters: currentNamedViewConfig?.filter || activeView?.filter || schema.table?.defaultFilters, - defaultSort: currentNamedViewConfig?.sort || activeView?.sort || schema.table?.defaultSort, - pageSize: schema.table?.pageSize, - selectable: schema.table?.selectable, - className: schema.table?.className, - }), [schema, operations, currentNamedViewConfig, activeView]); + // + // objectui#5102: `table` is documented as "inherits from ObjectGridSchema", + // but this whitelist forwarded only the DEPRECATED half of four pairs — + // `pageSize` / `selectable` / `defaultFilters` / `defaultSort` — and dropped + // their canonical successors `pagination` / `selection` / `filter` / `sort` + // on the floor. An author who wrote the canonical shape the type recommends + // got a compile-clean, semantically correct, RUNTIME-INERT view. + // + // ObjectGrid already reads both spellings of all four and already resolves + // them canonical-first (`schema.pagination?.pageSize ?? schema.pageSize`; + // `if (schema.selection?.type) … else if (schema.selectable !== undefined)`; + // `schemaFilter !== undefined ? … : schema.defaultFilters`; + // `schemaSort ?? (schema.defaultSort ? [schema.defaultSort] : undefined)`). + // So the fix is forwarding, not translation — and the precedence is not a + // free choice here: emitting both slots lets ObjectGrid's existing + // canonical-wins rule decide, which is the only answer that keeps the two + // layers saying the same thing. + const gridSchema: ObjectGridSchema = useMemo(() => { + // The two segments ahead of the `table` one, resolved once. They keep + // riding the LEGACY slots they ride today: `filter`/`defaultFilters` are + // not interchangeable downstream — ObjectGrid lowers the canonical slot + // through `toFilterNode` and raw-assigns the legacy one — so moving a + // named-view filter across would change the wire shape of a path this + // card does not own. + const viewFilter = currentNamedViewConfig?.filter || activeView?.filter; + const viewSort = currentNamedViewConfig?.sort || activeView?.sort; + + return { + type: 'object-grid', + objectName: schema.objectName, + title: schema.table?.title, + description: schema.table?.description, + fields: currentNamedViewConfig?.columns || activeView?.columns || schema.table?.fields, + columns: currentNamedViewConfig?.columns || activeView?.columns || schema.table?.columns, + operations: { + ...operations, + create: false, // Create is handled by the view's create button + }, + defaultFilters: viewFilter || schema.table?.defaultFilters, + defaultSort: viewSort || schema.table?.defaultSort, + // Canonical `table` keys, at last forwarded. `filter`/`sort` carry the + // `table` segment ONLY: a view segment resolved above already occupies + // the legacy slot, and ObjectGrid prefers this slot over that one — so + // handing it `table.filter` while a named view is active would let the + // table default outrank the view, inverting the precedence the two + // untouched segments exist to express. + filter: viewFilter ? undefined : schema.table?.filter, + sort: viewSort ? undefined : schema.table?.sort, + pagination: schema.table?.pagination, + selection: schema.table?.selection, + pageSize: schema.table?.pageSize, + selectable: schema.table?.selectable, + className: schema.table?.className, + }; + }, [schema, operations, currentNamedViewConfig, activeView]); // Build form schema const buildFormSchema = (): ObjectFormSchema => { @@ -1150,12 +1193,19 @@ export const ObjectView: React.FC = ({ // written. It never was (see the note by the state declarations), so both // branches were dead. They are gone rather than corrected: the delegated // renderer owns the filter/sort UI and does its own combining. + // + // The `table` segment of both chains reads the canonical key first and the + // deprecated one as its alias (objectui#5102). Both land on `list-view`'s + // own `filter` / `sort` keys below, so a canonical value arrives in the slot + // that already matches its shape. const mergedFilters = currentNamedViewConfig?.filter || activeView?.filter + || schema.table?.filter || schema.table?.defaultFilters; const mergedSort = currentNamedViewConfig?.sort || activeView?.sort + || schema.table?.sort || schema.table?.defaultSort; // --- Content renderer --- diff --git a/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx new file mode 100644 index 000000000..d19e298a5 --- /dev/null +++ b/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx @@ -0,0 +1,358 @@ +/** + * 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. + */ + +/** + * objectui#5102 — the canonical `table` keys take effect. + * + * `ObjectViewSchema.table` is documented as "inherits from ObjectGridSchema", + * but `ObjectView` does not spread it: it forwards a hand-written whitelist of + * keys. That whitelist carried only the DEPRECATED half of four pairs, and + * dropped every canonical successor: + * + * forwarded (legacy) | canonical successor on ObjectGridSchema | forwarded? + * ---------------------|-----------------------------------------|----------- + * pageSize | pagination: PaginationConfig | NO + * selectable | selection: SelectionConfig | NO + * defaultFilters | filter | NO + * defaultSort | sort | NO + * + * So an author writing the shape the type recommends — `table: { pagination: + * { pageSize: 25 } }` — got a view that compiled, read correctly, and did + * NOTHING. There was no failure signal anywhere: the key is declared on + * `ObjectGridSchema`, `ObjectGrid` reads it, and only this forwarding hop lost + * it. That silent-success shape is the defect, not the missing feature. + * + * Maintainer ruling of 2026-08-18 (verbatim 「同意」): extend the whitelist so + * the canonical keys take effect, keeping the legacy spellings as WORKING + * aliases. Both halves are pinned here — a fix that quietly retired the legacy + * spelling would break every view authored against today's docs. + * + * PRECEDENCE, when an author writes both: the canonical key wins. Where + * `ObjectView` itself resolves the pair (the non-grid fetch, and the delegated + * `renderListView` schema) that choice is encoded in this file's `||` chains + * and pinned below. On the grid path `ObjectView` forwards BOTH slots and + * `ObjectGrid` arbitrates — it already resolves all four pairs canonical-first + * (`schema.pagination?.pageSize ?? schema.pageSize`; `if (schema.selection + * ?.type) … else if (schema.selectable !== undefined)`; `schemaFilter !== + * undefined ? … : schema.defaultFilters`; `schemaSort ?? (schema.defaultSort + * ? [schema.defaultSort] : undefined)`). Re-resolving the pair here instead + * would have made the two layers disagree, and synthesising a `pagination` + * object out of a legacy `pageSize` would flip `ObjectGrid`'s + * `paginationEnabled` (`schema.pagination !== undefined ? true : …`) for every + * view that only ever wrote the legacy key. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import { ObjectView } from '../ObjectView'; +import type { ObjectViewSchema } from '@object-ui/types'; + +vi.mock('@object-ui/react', async () => { + const React = await import('react'); + return { + SchemaRenderer: ({ schema }: any) =>
{schema?.type}
, + SchemaRendererContext: React.createContext(null), + subscribeDataChanges: () => () => {}, + notifyDataChanged: () => {}, + }; +}); + +/** + * The grid the view delegates to, replaced by a probe that records the schema + * it was handed. What reaches this object IS the forwarding whitelist. + */ +const gridSchemas: any[] = []; +vi.mock('@object-ui/plugin-grid', () => ({ + ObjectGrid: ({ schema }: any) => { + gridSchemas.push(schema); + return
; + }, +})); +vi.mock('@object-ui/plugin-form', () => ({ ObjectForm: () =>
})); + +const mockDataSource = () => ({ + find: vi.fn().mockResolvedValue({ data: [], total: 0 }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'task', fields: {} }), +}); + +/** Render on the grid path and return the schema ObjectGrid was handed. */ +function forwardedGridSchema(schema: Partial): any { + gridSchemas.length = 0; + render( + , + ); + return gridSchemas[0]; +} + +/** Render on a NON-grid path (ObjectView fetches for itself) and return `find`. */ +function nonGridFind(schema: Partial) { + const ds = mockDataSource(); + render( + , + ); + return ds.find; +} + +async function queryParams(find: ReturnType) { + await waitFor(() => expect(find).toHaveBeenCalled()); + return find.mock.calls[0][1]; +} + +/** Render through the delegated `renderListView` slot and return its schema. */ +function delegatedSchema(schema: Partial): any { + const seen: any[] = []; + render( + { seen.push(s); return
; }} + />, + ); + return seen[0]; +} + +beforeEach(() => { + vi.clearAllMocks(); + gridSchemas.length = 0; +}); + +describe('grid path: the canonical table keys reach ObjectGrid', () => { + // Each of these four was `undefined` on the forwarded schema before + // objectui#5102 — the value the author wrote never left ObjectView. + it('forwards table.pagination', () => { + const grid = forwardedGridSchema({ table: { pagination: { pageSize: 25 } } as any }); + expect(grid.pagination).toEqual({ pageSize: 25 }); + }); + + it('forwards table.pagination.pageSizeOptions along with it', () => { + const grid = forwardedGridSchema({ + table: { pagination: { pageSize: 25, pageSizeOptions: [25, 50] } } as any, + }); + expect(grid.pagination).toEqual({ pageSize: 25, pageSizeOptions: [25, 50] }); + }); + + it('forwards table.selection', () => { + const grid = forwardedGridSchema({ table: { selection: { type: 'multiple' } } as any }); + expect(grid.selection).toEqual({ type: 'multiple' }); + }); + + it('forwards table.filter', () => { + const grid = forwardedGridSchema({ table: { filter: [['stage', '=', 'won']] } as any }); + expect(grid.filter).toEqual([['stage', '=', 'won']]); + }); + + it('forwards table.sort', () => { + const grid = forwardedGridSchema({ table: { sort: [{ field: 'name', order: 'desc' }] } as any }); + expect(grid.sort).toEqual([{ field: 'name', order: 'desc' }]); + }); + + it('forwards the string form of table.sort', () => { + // `ObjectGridSchema.sort` is `string | SortConfig[]`; the legacy + // `defaultSort` has no string form, so this arity is reachable only + // through the canonical key. + const grid = forwardedGridSchema({ table: { sort: 'name desc' } as any }); + expect(grid.sort).toBe('name desc'); + }); +}); + +describe('grid path: the legacy spellings still work', () => { + it('still forwards table.pageSize', () => { + expect(forwardedGridSchema({ table: { pageSize: 25 } as any }).pageSize).toBe(25); + }); + + it('still forwards table.selectable', () => { + expect(forwardedGridSchema({ table: { selectable: true } as any }).selectable).toBe(true); + }); + + it('still forwards table.defaultFilters', () => { + const grid = forwardedGridSchema({ table: { defaultFilters: { status: 'active' } } as any }); + expect(grid.defaultFilters).toEqual({ status: 'active' }); + }); + + it('still forwards table.defaultSort', () => { + const grid = forwardedGridSchema({ table: { defaultSort: { field: 'name', order: 'asc' } } as any }); + expect(grid.defaultSort).toEqual({ field: 'name', order: 'asc' }); + }); + + it('leaves a canonical slot empty when only the legacy key is written', () => { + // The alias must not be re-emitted into the canonical slot: `ObjectGrid` + // keys `paginationEnabled` off `schema.pagination !== undefined`, so a + // synthesised `pagination` would turn pagination on for legacy authors. + const grid = forwardedGridSchema({ + table: { pageSize: 25, selectable: true, defaultFilters: { a: 1 }, defaultSort: { field: 'n', order: 'asc' } } as any, + }); + expect(grid.pagination).toBeUndefined(); + expect(grid.selection).toBeUndefined(); + expect(grid.filter).toBeUndefined(); + expect(grid.sort).toBeUndefined(); + }); +}); + +describe('grid path: both spellings written — each rides its own slot', () => { + // ObjectView forwards both and ObjectGrid arbitrates canonical-first (see the + // file header for the four expressions). What is pinned here is ObjectView's + // half: the canonical value must arrive in the canonical slot, or the + // downstream rule has nothing to prefer. + it('puts the canonical value in the canonical slot and the alias in the legacy one', () => { + const grid = forwardedGridSchema({ + table: { + pagination: { pageSize: 25 }, + pageSize: 10, + selection: { type: 'multiple' }, + selectable: false, + filter: [['stage', '=', 'won']], + defaultFilters: { stage: 'lost' }, + sort: [{ field: 'name', order: 'desc' }], + defaultSort: { field: 'created', order: 'asc' }, + } as any, + }); + expect(grid.pagination).toEqual({ pageSize: 25 }); + expect(grid.pageSize).toBe(10); + expect(grid.selection).toEqual({ type: 'multiple' }); + expect(grid.selectable).toBe(false); + expect(grid.filter).toEqual([['stage', '=', 'won']]); + expect(grid.defaultFilters).toEqual({ stage: 'lost' }); + expect(grid.sort).toEqual([{ field: 'name', order: 'desc' }]); + expect(grid.defaultSort).toEqual({ field: 'created', order: 'asc' }); + }); +}); + +describe('grid path: a named view still outranks the table segment', () => { + // The two segments ahead of `table` (`listViews` entry, then `activeView`) + // are untouched by objectui#5102 — they keep riding the legacy slots. The + // canonical slot must therefore stay EMPTY while one of them is active: + // ObjectGrid prefers the canonical slot, so a `table.filter` forwarded + // unconditionally would outrank the view the user is looking at. + const namedView = { + listViews: { + won: { + label: 'Won', + filter: [['stage', '=', 'won']], + sort: [{ field: 'name', order: 'desc' }], + }, + }, + defaultListView: 'won', + }; + + it('keeps the named view filter in force over a table.filter', () => { + const grid = forwardedGridSchema({ + ...namedView, + table: { filter: [['stage', '=', 'lost']] } as any, + } as any); + expect(grid.filter).toBeUndefined(); + expect(grid.defaultFilters).toEqual([['stage', '=', 'won']]); + }); + + it('keeps the named view sort in force over a table.sort', () => { + const grid = forwardedGridSchema({ + ...namedView, + table: { sort: 'created asc' } as any, + } as any); + expect(grid.sort).toBeUndefined(); + expect(grid.defaultSort).toEqual([{ field: 'name', order: 'desc' }]); + }); +}); + +describe('non-grid path: ObjectView resolves the pair itself, canonical first', () => { + // Calendar / kanban / gallery / timeline fetch through ObjectView, so the + // precedence chosen for this card is directly observable on the wire here. + it('queries with a canonical table.filter', async () => { + const params = await queryParams(nonGridFind({ table: { filter: [['stage', '=', 'won']] } as any })); + expect(params.$filter).toEqual([['stage', '=', 'won']]); + }); + + it('lowers a ViewFilterRule[] table.filter through the shared sink', async () => { + // Same treatment `table.defaultFilters` already gets — the canonical key + // reaches `$filter` as AST, not as bare rule objects (objectui#3431). + const params = await queryParams( + nonGridFind({ table: { filter: [{ field: 'stage', operator: 'eq', value: 'won' }] } as any }), + ); + expect(params.$filter).toEqual([['stage', 'equals', 'won']]); + }); + + it('prefers table.filter over table.defaultFilters', async () => { + const params = await queryParams( + nonGridFind({ + table: { filter: [['stage', '=', 'won']], defaultFilters: { stage: 'lost' } } as any, + }), + ); + expect(params.$filter).toEqual([['stage', '=', 'won']]); + }); + + it('still queries with table.defaultFilters alone', async () => { + const params = await queryParams(nonGridFind({ table: { defaultFilters: { status: 'active' } } as any })); + expect(params.$filter).toEqual(['status', '=', 'active']); + }); + + it('orders by a canonical table.sort', async () => { + const params = await queryParams(nonGridFind({ table: { sort: [{ field: 'name', order: 'desc' }] } as any })); + expect(params.$orderby).toEqual([{ field: 'name', order: 'desc' }]); + }); + + it('prefers table.sort over table.defaultSort', async () => { + const params = await queryParams( + nonGridFind({ + table: { sort: [{ field: 'name', order: 'desc' }], defaultSort: { field: 'created', order: 'asc' } } as any, + }), + ); + expect(params.$orderby).toEqual([{ field: 'name', order: 'desc' }]); + }); + + it('still orders by table.defaultSort alone', async () => { + const params = await queryParams(nonGridFind({ table: { defaultSort: { field: 'created', order: 'asc' } } as any })); + expect(params.$orderby).toEqual({ field: 'created', order: 'asc' }); + }); +}); + +describe('delegated renderListView: canonical first, alias still working', () => { + it('hands over a canonical table.filter', () => { + expect(delegatedSchema({ table: { filter: [['stage', '=', 'won']] } as any }).filter) + .toEqual([['stage', '=', 'won']]); + }); + + it('prefers table.filter over table.defaultFilters', () => { + const s = delegatedSchema({ + table: { filter: [['stage', '=', 'won']], defaultFilters: { stage: 'lost' } } as any, + }); + expect(s.filter).toEqual([['stage', '=', 'won']]); + }); + + it('still hands over table.defaultFilters alone', () => { + expect(delegatedSchema({ table: { defaultFilters: { status: 'active' } } as any }).filter) + .toEqual({ status: 'active' }); + }); + + it('hands over a canonical table.sort and prefers it over table.defaultSort', () => { + expect(delegatedSchema({ table: { sort: 'name desc' } as any }).sort).toBe('name desc'); + const s = delegatedSchema({ + table: { sort: 'name desc', defaultSort: { field: 'created', order: 'asc' } } as any, + }); + expect(s.sort).toBe('name desc'); + }); + + it('still hands over table.defaultSort alone', () => { + expect(delegatedSchema({ table: { defaultSort: { field: 'created', order: 'asc' } } as any }).sort) + .toEqual({ field: 'created', order: 'asc' }); + }); +});