From e2d74cbd75dd5c9c4ea3564f49cfe19b11cd705e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 11:44:49 +0000 Subject: [PATCH] fix(list,grid,detail,tree,core): every column resolver reads one key (#3104 PR2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR1 (#3119) put a canonicalizing fold at ListView's ingestion boundary. This converges the 22 read sites themselves onto `columnIdentity()`, so a surface that is NOT downstream of that fold resolves the same identity anyway. That distinction is the user-visible part. A standalone `object-grid` node — authored directly on a page, with no `list-view` above it — never passed through `normalizeListViewSchema`. Its `getSelectFields` read `c.field` alone while the `ensureId` probe one line above read `f?.name || f?.field`, so a legacy `{name:'account'}` column reached `$select` as a literal `undefined` hole: the server never returned the field and every cell in that column came back empty. Same for ObjectTree, RelatedList and the record:details / record:related_list renderers. Converged: ListView ×9 + its 2 request builders -> columnIdentity() RelatedList ×8 -> accessorKey || columnIdentity() ObjectGrid (probe + projection) -> columnIdentity() ObjectTree -> columnIdentity() || key buildExpandFields -> columnIdentity() record-details / record-related-list -> columnIdentity() (|| key) `accessorKey` keeps its precedence in RelatedList — it is TanStack Table's column key, not ObjectStack metadata identity, and only the `field || name` tail was converged. `key` stays a tail fallback in ObjectTree and record-related-list for the same reason: it is a generic entry key. Two incidental fixes TypeScript surfaced once the resolver stopped returning `any`: ListView's filter-field options and its hide-fields popover both built entries keyed `undefined` for a column with no resolvable identity. Those entries could never match a column; they are now dropped. Inventory re-triage: PR1 recorded 24 family members. Two were mis-classified and are reclassified rather than converged — reading what they actually feed shows they are not column reads at all. ViewPreview adapts a ViewItem FORM section to what object-form selects by (#3090's two-layer join); SchemaForm renders an arbitrary metadata ARRAY into a popover summary and guesses at a display key. So the family was 22, and it is now 0. The ratchet asserts that, asserts each converged surface actually routes through the shared reader (a surface that dropped identity resolution instead of converging it goes red), and pins accessorKey's precedence in RelatedList. Refs: objectstack#4115, #3090 (playbook), #3119 (PR1) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01C2pdPmf2yZSd4wFDs1NHY5 --- .../column-identity-consumers-read-one-key.md | 59 +++++++ content/docs/guide/troubleshooting.md | 19 +- .../__tests__/column-identity.ratchet.test.ts | 163 +++++++++--------- packages/core/src/utils/expand-fields.ts | 14 +- packages/plugin-detail/src/RelatedList.tsx | 17 +- .../src/renderers/record-details.tsx | 8 +- .../src/renderers/record-related-list.tsx | 14 +- packages/plugin-grid/src/ObjectGrid.tsx | 12 +- .../src/__tests__/columnIdentity.test.tsx | 88 ++++++++++ packages/plugin-list/src/ListView.tsx | 60 ++++--- packages/plugin-tree/src/ObjectTree.tsx | 8 +- 11 files changed, 314 insertions(+), 148 deletions(-) create mode 100644 .changeset/column-identity-consumers-read-one-key.md create mode 100644 packages/plugin-grid/src/__tests__/columnIdentity.test.tsx diff --git a/.changeset/column-identity-consumers-read-one-key.md b/.changeset/column-identity-consumers-read-one-key.md new file mode 100644 index 0000000000..5870efe76c --- /dev/null +++ b/.changeset/column-identity-consumers-read-one-key.md @@ -0,0 +1,59 @@ +--- +"@object-ui/core": patch +"@object-ui/plugin-list": patch +"@object-ui/plugin-grid": patch +"@object-ui/plugin-detail": patch +"@object-ui/plugin-tree": patch +--- + +fix(list,grid,detail,tree,core): every column resolver reads one key (#3104 PR2) + +PR1 (#3119) put a canonicalizing fold at ListView's ingestion boundary. This +converges the 22 read sites themselves onto `columnIdentity()` from +`@object-ui/core`, so a surface that is NOT downstream of that fold resolves +the same identity anyway. + +That distinction is the user-visible part. A standalone `object-grid` node — +authored directly on a page, with no `list-view` above it — never passed +through `normalizeListViewSchema`. Its `getSelectFields` read `c.field` alone +while the `ensureId` probe one line above read `f?.name || f?.field`, so a +legacy `{ name: 'account' }` column reached `$select` as a literal `undefined` +hole: the server never returned the field and every cell in that column came +back empty. Same for `ObjectTree`, `RelatedList` and the `record:details` / +`record:related_list` renderers. + +Converged: + +| Surface | Was | Now | +|---|---|---| +| `ListView` ×9 + its 2 request builders | `name \|\| fieldName \|\| field` vs `f?.field` | `columnIdentity()` | +| `RelatedList` ×8 | `accessorKey \|\| field \|\| name` | `accessorKey \|\| columnIdentity()` | +| `ObjectGrid` | name-first probe vs `c.field` projection | `columnIdentity()` | +| `ObjectTree` | `name \|\| fieldName \|\| field \|\| key` | `columnIdentity() \|\| key` | +| `buildExpandFields` | `field ?? name ?? fieldName` | `columnIdentity()` | +| `record-details` / `record-related-list` | `field \|\| name (\|\| key)` | `columnIdentity() (\|\| key)` | + +`accessorKey` keeps its precedence in `RelatedList` — it is TanStack Table's +column key, not ObjectStack metadata identity, and only the `field || name` +tail was converged. `key` stays a tail fallback in `ObjectTree` and +`record-related-list` for the same reason: it is a generic entry key. + +Two incidental fixes that TypeScript surfaced once the resolver stopped +returning `any`: ListView's filter-field options and its hide-fields popover +both built entries keyed `undefined` for a column with no resolvable identity. +Those entries could never match a column; they are now dropped. + +**Inventory re-triage.** PR1 recorded 24 family members. Two were mis-classified +and are reclassified here rather than converged — reading what they actually +feed shows they are not column reads at all: + +- `ViewPreview.tsx` adapts a ViewItem **form** section to what `object-form` + selects by (`field` → `name`) — the #3090 two-layer join. +- `SchemaForm.tsx` renders an arbitrary metadata **array** into a popover + summary and guesses at a display key; the entries are validations, actions, + or whatever the JSON schema declares. + +So the family was 22, and it is now **0**. The ratchet asserts that, asserts +each converged surface actually routes through the shared reader (a surface +that dropped identity resolution instead of converging it goes red), and pins +`accessorKey`'s precedence in `RelatedList`. diff --git a/content/docs/guide/troubleshooting.md b/content/docs/guide/troubleshooting.md index b26575c3ee..cf1be0151e 100644 --- a/content/docs/guide/troubleshooting.md +++ b/content/docs/guide/troubleshooting.md @@ -309,12 +309,19 @@ never expanded. { "field": "account" } ``` -**Fix:** Author columns with `field`. Metadata reaching a `list-view` is -canonicalized at the component boundary by `normalizeListViewSchema`, which -stamps `field` from whichever spelling is present and makes any legacy key it -already carries agree — so mixed metadata resolves to one field everywhere -instead of two. Legacy keys are still accepted, but they are a migration -bridge, not a second contract: fix the producer. +**Fix:** Author columns with `field`. Two mechanisms keep legacy metadata +working, and both resolve the canonical key first: + +- Metadata reaching a `list-view` is canonicalized at the component boundary by + `normalizeListViewSchema`, which stamps `field` from whichever spelling is + present and makes any legacy key it already carries agree. +- Every renderer that resolves a column — list, grid, tree, related lists, the + `$expand` / `$select` builders — reads that identity through one shared + function, so a surface rendered outside the fold (a standalone `object-grid` + node, for instance) still resolves the same field the request asked for. + +Legacy keys are still accepted, but they are a migration bridge, not a second +contract: fix the producer. If you read column identity in your own code, use the one reader rather than spelling out a fallback chain — it resolves canonical-first, so it agrees with diff --git a/packages/core/src/utils/__tests__/column-identity.ratchet.test.ts b/packages/core/src/utils/__tests__/column-identity.ratchet.test.ts index 8d5328cde4..ac110cb2c9 100644 --- a/packages/core/src/utils/__tests__/column-identity.ratchet.test.ts +++ b/packages/core/src/utils/__tests__/column-identity.ratchet.test.ts @@ -4,11 +4,17 @@ * * Ratchet — the column-identity dual read (`field` ?? `name`) may only shrink. * - * objectui#3104. A column's identity is read in two incompatible precedences - * across the repo, and the two halves land on different fields for the same - * column (see `ListView.columnIdentity.test.tsx` for the live repro). This file - * freezes the inventory so the family cannot grow while it is being retired: - * a new dual read in a new file fails, and an extra one in a listed file fails. + * objectui#3104. A column's identity used to be read in two incompatible + * precedences across the repo, and the two halves landed on different fields + * for the same column (see `ListView.columnIdentity.test.tsx` for the repro). + * + * **The family is now at zero** (PR2): every column-identity read goes through + * `columnIdentity()`. What remains in the inventory below is the residue the + * scanner still matches — reads that share the key names but not the concept. + * They are listed, not converged, and each says why. + * + * This file freezes that state: a new dual read in a new file fails, and an + * extra one in a listed file fails. * * Shrinking also fails, deliberately — lower the number here in the same commit * that removes the read, so the count in the tree and the count on record never @@ -78,71 +84,25 @@ interface Entry { } /** - * The inventory, as of objectui#3104 PR1. + * The inventory, as of objectui#3104 PR2. + * + * The column-identity family is EMPTY: all 22 reads now go through + * `columnIdentity()`. What is left below matched the scanner but was never the + * same concept — two-layer joins where both precedences are correct, the form + * cluster that #3090 settled the other way, and generic display fallbacks that + * merely share the key names. They stay listed so the scanner cannot quietly + * grow a real dual read inside a file we already decided about. * - * The headline is not that two precedences exist — it is that ListView and - * ObjectGrid each disagree with THEMSELVES. Both build their `$expand` / - * `$select` from `f?.field` alone (a single-key read the scanner cannot see, - * because it is not an alternation) while every render-side read in the same - * file is name-first. That is the mechanism behind "relation column shows a - * bare id": the request asks for one field, the renderer shows another. + * PR1 recorded 24 family members; two of those were mis-triaged and are + * reclassified here after reading what they actually feed: + * - `ViewPreview.tsx` converts a ViewItem FORM section into `object-form`'s + * runtime shape (`field` -> `name`) — the #3090 two-layer join, not a column. + * - `SchemaForm.tsx` renders an arbitrary metadata ARRAY into a summary string + * and guesses at a display key; the entries are validations/actions/whatever + * the JSON schema declares, so there is no column vocabulary to converge. */ const INVENTORY: Record = { - // ── the column-identity family (24) ────────────────────────────────────── - 'plugin-list/src/ListView.tsx': { - count: 9, - order: 'name-first', - verdict: 'column-identity', - why: 'FLS gate, hidden-field filter, fieldOrder, export ×2, hide-fields list — all name-first, while $expand/$select in the same file read `f?.field` only.', - }, - 'plugin-detail/src/RelatedList.tsx': { - count: 8, - order: 'adapter-first', - verdict: 'column-identity', - why: '`accessorKey || field || name` — merges TanStack\'s column key with metadata identity. The adapter key is out of scope; the `field || name` tail is not.', - }, - 'core/src/utils/expand-fields.ts': { - count: 1, - order: 'field-first', - verdict: 'column-identity', - why: 'The request path. Canonical-first, so it is the side the renderers must converge ONTO.', - }, - 'plugin-grid/src/ObjectGrid.tsx': { - count: 1, - order: 'name-first', - verdict: 'column-identity', - why: 'The `ensureId` probe is name-first while `getSelectFields` right below it reads `c.field` only — the same self-disagreement as ListView.', - }, - 'plugin-tree/src/ObjectTree.tsx': { - count: 1, - order: 'name-first', - verdict: 'column-identity', - why: 'Tree column identity.', - }, - 'plugin-detail/src/renderers/record-details.tsx': { - count: 1, - order: 'field-first', - verdict: 'column-identity', - why: 'Detail field entry identity.', - }, - 'plugin-detail/src/renderers/record-related-list.tsx': { - count: 1, - order: 'field-first', - verdict: 'column-identity', - why: 'Related-list column identity.', - }, - 'app-shell/src/views/metadata-admin/previews/ViewPreview.tsx': { - count: 1, - order: 'field-first', - verdict: 'column-identity', - why: 'Studio preview column identity.', - }, - 'app-shell/src/views/metadata-admin/SchemaForm.tsx': { - count: 1, - order: 'field-first', - verdict: 'column-identity', - why: 'Renders a column array into a summary string.', - }, + // ── the column-identity family: EMPTY (was 24 in PR1; 22 after re-triage) ── // ── two layers, not two spellings (6) ──────────────────────────────────── 'app-shell/src/utils/resolveActionParams.ts': { @@ -166,7 +126,7 @@ const INVENTORY: Record = { why: 'Action param name, same layering as resolveActionParams.', }, - // ── the form cluster — settled the other way (#3090) (2) ───────────────── + // ── the form cluster — settled the other way (#3090) (3) ───────────────── 'plugin-form/src/ObjectForm.tsx': { count: 1, verdict: 'form-cluster', @@ -177,8 +137,13 @@ const INVENTORY: Record = { verdict: 'form-cluster', why: 'The #3090 hub itself.', }, + 'app-shell/src/views/metadata-admin/previews/ViewPreview.tsx': { + count: 1, + verdict: 'form-cluster', + why: 'Re-triaged in PR2: `toFormFieldEntry` adapts a ViewItem FORM section to what `object-form` selects by (`name`), which is the #3090 join — not a list column. Converging it onto `columnIdentity` would be a category error.', + }, - // ── not identity reads (2) ─────────────────────────────────────────────── + // ── not identity reads (3) ─────────────────────────────────────────────── 'app-shell/src/views/metadata-admin/widgets.tsx': { count: 1, verdict: 'unrelated', @@ -189,6 +154,11 @@ const INVENTORY: Record = { verdict: 'unrelated', why: '`c.object ?? c.name` — a citation label fallback that happens to sit on a line mentioning `c.field`.', }, + 'app-shell/src/views/metadata-admin/SchemaForm.tsx': { + count: 1, + verdict: 'unrelated', + why: 'Re-triaged in PR2: `summariseComposite` renders ANY composite/array metadata value into a popover summary and guesses at a display key. The array items are whatever the JSON schema declares, so this is a display fallback over arbitrary objects, not a column read.', + }, }; function collectSourceFiles(): string[] { @@ -265,28 +235,55 @@ describe('column identity dual-read ratchet (#3104)', () => { expect(drift).toEqual([]); }); - it('keeps the family total from growing', () => { - // The number that has to reach zero. The other verdicts are inventory, not - // worklist: `two-layer` and `form-cluster` are settled decisions, and - // `unrelated` is scanner noise recorded so it cannot hide a real one. - expect(sum((e) => e.verdict === 'column-identity')).toBe(24); - expect(sum(() => true)).toBe(34); + it('holds the family at zero', () => { + // This is the number the battle was about, and it is done. The remaining + // verdicts are inventory, not worklist: `two-layer` and `form-cluster` are + // settled decisions, and `unrelated` is scanner residue recorded so it + // cannot hide a real one. + expect(sum((e) => e.verdict === 'column-identity')).toBe(0); + expect(sum(() => true)).toBe(12); }); - it('records a precedence for every read in the family', () => { + it('records a precedence for every read left in the family', () => { const missing = Object.entries(INVENTORY) .filter(([, e]) => e.verdict === 'column-identity' && !e.order) .map(([file]) => file); expect(missing).toEqual([]); }); - it('still shows both precedences in the family — the pollution this closes', () => { - const orders = new Set( - Object.values(INVENTORY) - .filter((e) => e.verdict === 'column-identity') - .map((e) => e.order), + it('routes every converged surface through the one reader', () => { + // The counterpart to "the family is zero": the reads did not vanish, they + // moved onto `columnIdentity`. If a surface silently dropped its identity + // resolution instead of converging it, this goes red. + const CONVERGED = [ + 'core/src/utils/expand-fields.ts', + 'plugin-list/src/ListView.tsx', + 'plugin-grid/src/ObjectGrid.tsx', + 'plugin-detail/src/RelatedList.tsx', + 'plugin-detail/src/renderers/record-details.tsx', + 'plugin-detail/src/renderers/record-related-list.tsx', + 'plugin-tree/src/ObjectTree.tsx', + ]; + const missing = CONVERGED.filter((rel) => { + const src = readFileSync(path.join(packagesRoot, rel), 'utf8'); + return !/\bcolumnIdentity\s*\(/.test(src); + }); + expect(missing).toEqual([]); + }); + + it('keeps the table-adapter key ahead of metadata identity in RelatedList', () => { + // A source-level pin, and labelled as one: RelatedList's columns can arrive + // in TanStack's shape (`accessorKey`) or ObjectStack's (`field`). The + // convergence replaced only the `field || name` tail — if `accessorKey` + // ever loses its precedence here, imported TanStack columns stop resolving + // and every cell in them goes blank. + const src = readFileSync( + path.join(packagesRoot, 'plugin-detail/src/RelatedList.tsx'), + 'utf8', ); - // When PR2 lands this becomes a single order and this assertion inverts. - expect(orders.size).toBeGreaterThan(1); + const identityReads = [...src.matchAll(/\bcolumnIdentity\s*\(/g)].length; + const adapterFirst = [...src.matchAll(/accessorKey\s*\|\|\s*columnIdentity\s*\(/g)].length; + expect(identityReads).toBeGreaterThan(0); + expect(adapterFirst).toBe(identityReads); }); }); diff --git a/packages/core/src/utils/expand-fields.ts b/packages/core/src/utils/expand-fields.ts index 7ba16841a7..6f4d61b008 100644 --- a/packages/core/src/utils/expand-fields.ts +++ b/packages/core/src/utils/expand-fields.ts @@ -6,6 +6,8 @@ * LICENSE file in the root directory of this source tree. */ +import { columnIdentity } from './column-identity.js'; + /** * Relational ("reference-bearing") field types whose stored value is a foreign * key into another object — and which therefore benefit from `$expand` so a @@ -101,12 +103,12 @@ export function buildExpandFields( if (columns && Array.isArray(columns) && columns.length > 0) { const columnFieldNames = new Set(); for (const col of columns) { - if (typeof col === 'string') { - columnFieldNames.add(col); - } else if (col && typeof col === 'object') { - const name = col.field ?? col.name ?? col.fieldName; - if (name) columnFieldNames.add(name); - } + // `columnIdentity` handles all three entry shapes (bare string, spec + // `{field}`, legacy `{name}`/`{fieldName}`) and resolves canonical-first + // — the same precedence every renderer now uses, so what gets expanded + // and what gets rendered can no longer name two different fields (#3104). + const name = columnIdentity(col); + if (name) columnFieldNames.add(name); } return referenceFieldNames.filter((f) => columnFieldNames.has(f)); } diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index 52c947fd56..a8951bd950 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -41,6 +41,7 @@ import type { LucideIcon } from 'lucide-react'; import type { DataSource, FieldMetadata } from '@object-ui/types'; import { getCellRenderer, resolveCellRendererType, RecordPickerDialog } from '@object-ui/fields'; import { + columnIdentity, compareSortValues, getSortValue, isExpandableFieldType, @@ -695,7 +696,7 @@ export const RelatedList: React.FC = ({ const filterFLS = (cols: any[]): any[] => { if (!perms?.isLoaded || !relatedObjectName) return cols; return cols.filter((c) => { - const key = c?.accessorKey || c?.field || c?.name; + const key = c?.accessorKey || columnIdentity(c); if (!key) return true; return perms.checkField(relatedObjectName, String(key), 'read'); }); @@ -703,7 +704,7 @@ export const RelatedList: React.FC = ({ const filterFK = (cols: any[]): any[] => referenceField ? cols.filter((c) => { - const key = c?.accessorKey || c?.field || c?.name; + const key = c?.accessorKey || columnIdentity(c); return key !== referenceField; }) : cols; @@ -717,7 +718,7 @@ export const RelatedList: React.FC = ({ const pruneEmpty = (cols: any[]): any[] => { if (!relatedData.length) return cols; return cols.filter((c) => { - const key = c?.accessorKey || c?.field || c?.name; + const key = c?.accessorKey || columnIdentity(c); if (!key) return true; return relatedData.some((r) => !isValueEmpty(r?.[key])); }); @@ -768,7 +769,7 @@ export const RelatedList: React.FC = ({ if (typeof c !== 'string') { // Object column: attach a cell renderer when it lacks one and we can // resolve the field def — preserves any author-supplied cell/render. - const key = c?.accessorKey || c?.field || c?.name; + const key = c?.accessorKey || columnIdentity(c); if (c && !c.cell && !c.render && key) { const def = (objectSchema?.fields as any)?.[key]; const cell = def ? makeCell(String(key), def) : undefined; @@ -895,7 +896,7 @@ export const RelatedList: React.FC = ({ const sortableColumns = React.useMemo(() => { if (!windowed) return effectiveColumns; return effectiveColumns.map((col: any) => { - const field = col.accessorKey || col.field || col.name; + const field = col.accessorKey || columnIdentity(col); return field && isExpandableFieldType((objectSchema?.fields as any)?.[field]) ? { ...col, sortable: false } : col; @@ -921,10 +922,10 @@ export const RelatedList: React.FC = ({ // duplicate renderer. Falls back to the data-table on desktop and // when explicit `type='list'` is requested (legacy path). if (isMobile && (type === 'grid' || type === 'table')) { - const titleField = effectiveColumns[0]?.accessorKey || effectiveColumns[0]?.field || effectiveColumns[0]?.name; + const titleField = effectiveColumns[0]?.accessorKey || columnIdentity(effectiveColumns[0]); const visibleFields = effectiveColumns .slice(1, 4) - .map((c: any) => c.accessorKey || c.field || c.name) + .map((c: any) => c.accessorKey || columnIdentity(c)) .filter(Boolean); return { type: 'object-gallery', @@ -1104,7 +1105,7 @@ export const RelatedList: React.FC = ({ {sortable && !hasSortableHeaders && effectiveColumns && effectiveColumns.length > 0 && relatedData.length > 0 && (
{effectiveColumns.map((col: any) => { - const field = col.accessorKey || col.field || col.name; + const field = col.accessorKey || columnIdentity(col); if (!field) return null; // A windowed sort goes out as a server `$orderby` on the flat // field name, so a relational column would order the collection by diff --git a/packages/plugin-detail/src/renderers/record-details.tsx b/packages/plugin-detail/src/renderers/record-details.tsx index 77e8ecbe5b..05a818020c 100644 --- a/packages/plugin-detail/src/renderers/record-details.tsx +++ b/packages/plugin-detail/src/renderers/record-details.tsx @@ -15,15 +15,11 @@ import { useRecordContext, useHighlightFieldNames, useSafeFieldLabel } from '@ob import { useFieldPermissions, usePermissions } from '@object-ui/permissions'; import { useObjectTranslation, pickLocalized } from '@object-ui/i18n'; import type { RecordDetailsComponentProps } from '@object-ui/types'; -import { isObjectInlineEditable } from '@object-ui/core'; +import { columnIdentity, isObjectInlineEditable } from '@object-ui/core'; import { DetailView } from '../DetailView'; /** Normalize a field entry (string | {field} | {name}) to its machine name. */ -const fieldName = (entry: any): string | null => { - if (typeof entry === 'string') return entry; - if (entry && typeof entry === 'object') return entry.field || entry.name || null; - return null; -}; +const fieldName = (entry: any): string | null => columnIdentity(entry) ?? null; const splitDesigner = (props: Record) => { const { 'data-obj-id': id, 'data-obj-type': type, style, ...rest } = props || {}; diff --git a/packages/plugin-detail/src/renderers/record-related-list.tsx b/packages/plugin-detail/src/renderers/record-related-list.tsx index cd5570b26e..7b0e2cc572 100644 --- a/packages/plugin-detail/src/renderers/record-related-list.tsx +++ b/packages/plugin-detail/src/renderers/record-related-list.tsx @@ -16,15 +16,17 @@ import { useRecordContext, useSafeFieldLabel, useRelatedRecordActions } from '@o import { useFieldPermissions, usePermissions } from '@object-ui/permissions'; import { useObjectTranslation, pickLocalized } from '@object-ui/i18n'; import { humanizeLabel } from '@object-ui/fields'; +import { columnIdentity } from '@object-ui/core'; import type { RecordRelatedListComponentProps } from '@object-ui/types'; import { RelatedList } from '../RelatedList'; -/** Normalize a column entry (string | {field} | {name} | {key}) to its name. */ -const colName = (entry: any): string | null => { - if (typeof entry === 'string') return entry; - if (entry && typeof entry === 'object') return entry.field || entry.name || entry.key || null; - return null; -}; +/** + * Normalize a column entry (string | {field} | {name} | {key}) to its name. + * `key` is kept as a tail fallback rather than folded into `columnIdentity`: + * it is a generic entry key, not ObjectStack metadata identity (#3104). + */ +const colName = (entry: any): string | null => + columnIdentity(entry) || (entry && typeof entry === 'object' ? entry.key : null) || null; /** Extract a record's primary key, tolerating the `id` / `_id` split. */ const rowId = (row: any): string | number | null => row?.id ?? row?._id ?? null; diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 2988082443..fd4541f26f 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, isExpandableFieldType } from '@object-ui/core'; +import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, columnIdentity, isExpandableFieldType } 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'; @@ -542,13 +542,19 @@ export const ObjectGrid: React.FC = ({ // Always include 'id' so row click / navigation handlers can resolve // the record key — without it `record.id` is undefined and the // primary-field link silently no-ops. + // Both halves used to resolve identity differently — the probe was + // name-first while the projection below read `c.field` alone — so a + // legacy `{name}` column was projected as `undefined` while the + // probe happily saw its name (#3104). One reader now, both halves. const ensureId = (list: any[]): any[] => { - const names = list.map((f: any) => typeof f === 'string' ? f : (f?.name || f?.field)); + const names = list.map((f: any) => columnIdentity(f)); return names.includes('id') ? list : ['id', ...list]; }; if (schemaFields) return ensureId(schemaFields as any[]); if (schemaColumns && Array.isArray(schemaColumns)) { - const fields = schemaColumns.map((c: any) => typeof c === 'string' ? c : c.field); + const fields = schemaColumns + .map((c: any) => columnIdentity(c)) + .filter((v): v is string => !!v); return ensureId(fields); } return undefined; diff --git a/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx b/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx new file mode 100644 index 0000000000..a4abca2d56 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx @@ -0,0 +1,88 @@ +/** + * 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. + * + * ObjectGrid column identity (objectui#3104 PR2). + * + * ObjectGrid is NOT downstream of `normalizeListViewSchema` when it is rendered + * on its own — a page can author an `object-grid` node directly, and then no + * ingestion fold has run over its `columns`. So the chokepoint from PR1 does + * not cover this surface; converging the reads onto `columnIdentity()` is what + * does. + * + * The bug: `getSelectFields` resolved `c.field` alone while the `ensureId` + * probe one line above resolved `f?.name || f?.field`. A legacy `{ name }` + * column was therefore projected as `undefined` — it reached `$select` as a + * hole, so the server never returned the field and every cell in that column + * came back empty. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render } from '@testing-library/react'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { ActionProvider } from '@object-ui/react'; + +const makeDataSource = () => ({ + find: vi.fn().mockResolvedValue({ value: [], '@odata.count': 0 }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'contacts', + fields: { + name: { type: 'text' }, + account: { type: 'lookup', reference: 'accounts' }, + }, + }), +}); + +const selectFor = async (columns: unknown[]) => { + const ds = makeDataSource(); + const schema: any = { + type: 'object-grid', + objectName: 'contacts', + columns, + }; + render( + + + , + ); + await vi.waitFor(() => expect(ds.find).toHaveBeenCalled()); + return (ds.find.mock.calls.at(-1)?.[1]?.$select ?? []) as string[]; +}; + +describe('ObjectGrid — $select resolves one column identity (#3104)', () => { + beforeEach(() => vi.clearAllMocks()); + + it('projects a spec-canonical `field` column', async () => { + expect(await selectFor([{ field: 'account' }])).toContain('account'); + }); + + it('projects a legacy `name`-only column instead of a hole', async () => { + // Before the convergence this landed in `$select` as `undefined`. + const select = await selectFor([{ name: 'account' }]); + expect(select).toContain('account'); + expect(select).not.toContain(undefined); + }); + + it('projects the canonical key when a column carries both', async () => { + const select = await selectFor([{ field: 'account', name: 'account_name' }]); + expect(select).toContain('account'); + expect(select).not.toContain('account_name'); + }); + + it('still projects bare string columns', async () => { + expect(await selectFor(['account'])).toContain('account'); + }); + + it('always includes `id` so row navigation can resolve the record key', async () => { + expect(await selectFor([{ name: 'account' }])).toContain('id'); + }); +}); diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 15fba7c4e4..60de744656 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -19,7 +19,7 @@ import { useDensityMode } from '@object-ui/react'; import type { ListViewSchema } from '@object-ui/types'; import { detectStatusField } from '@object-ui/types'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema, rowHeightToDensityMode, mergeFilterNodes, EXPANDABLE_FIELD_TYPES } from '@object-ui/core'; +import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema, rowHeightToDensityMode, mergeFilterNodes, columnIdentity, EXPANDABLE_FIELD_TYPES } from '@object-ui/core'; import { useObjectTranslation, useObjectLabel, useSafeFieldLabel, createSafeTranslation } from '@object-ui/i18n'; import { usePermissions } from '@object-ui/permissions'; @@ -954,7 +954,7 @@ export const ListView = React.forwardRef(({ const expandFields = React.useMemo(() => { const baseColumns = Array.isArray(schema.columns) ? (schema.columns as any[]) - .map((f) => (typeof f === 'string' ? f : f?.field)) + .map((f) => columnIdentity(f)) .filter((v): v is string => typeof v === 'string' && v.length > 0) : []; const collected = new Set(baseColumns); @@ -1107,7 +1107,7 @@ export const ListView = React.forwardRef(({ const selectFields = (() => { const rawCols = Array.isArray(schema.columns) ? (schema.columns as any[]) - .map(f => (typeof f === 'string' ? f : f?.field)) + .map(f => columnIdentity(f)) .filter((v): v is string => typeof v === 'string' && v.length > 0) : []; const cols = (perms?.isLoaded && schema.objectName) @@ -1422,7 +1422,7 @@ export const ListView = React.forwardRef(({ // FLS: drop columns the current user cannot read. if (perms?.isLoaded && schema.objectName) { fields = fields.filter((f: any) => { - const fieldName = typeof f === 'string' ? f : (f?.name || f?.fieldName || f?.field); + const fieldName = columnIdentity(f); if (!fieldName) return true; return perms.checkField(schema.objectName!, fieldName, 'read'); }); @@ -1431,19 +1431,17 @@ export const ListView = React.forwardRef(({ // Remove hidden fields if (hiddenFields.size > 0) { fields = fields.filter((f: any) => { - const fieldName = typeof f === 'string' ? f : (f?.name || f?.fieldName || f?.field); + const fieldName = columnIdentity(f); return fieldName != null && !hiddenFields.has(fieldName); }); } - + // Apply field order if (schema.fieldOrder && schema.fieldOrder.length > 0) { const orderMap = new Map(schema.fieldOrder.map((f: any, i: number) => [f as string, i])); fields = [...fields].sort((a: any, b: any) => { - const nameA = typeof a === 'string' ? a : (a?.name || a?.fieldName || a?.field); - const nameB = typeof b === 'string' ? b : (b?.name || b?.fieldName || b?.field); - const orderA: number = orderMap.get(nameA) ?? Infinity; - const orderB: number = orderMap.get(nameB) ?? Infinity; + const orderA: number = orderMap.get(columnIdentity(a) as string) ?? Infinity; + const orderB: number = orderMap.get(columnIdentity(b) as string) ?? Infinity; return orderA - orderB; }); } @@ -1710,20 +1708,25 @@ export const ListView = React.forwardRef(({ if (!objectDef?.fields) { // Fallback to the declared columns if objectDef not loaded yet - fields = (Array.isArray(schema.columns) ? (schema.columns as any[]) : []).map((f: any) => { - if (typeof f === 'string') return { value: f, label: f, type: 'text' }; - // `field` is the spec's ListColumn key; `name`/`fieldName` are the - // shapes hosts pass through from stored metadata. - const fieldName = f.name || f.fieldName || f.field; - return { + fields = (Array.isArray(schema.columns) ? (schema.columns as any[]) : []) + .flatMap((f: any) => { + if (typeof f === 'string') return [{ value: f, label: f, type: 'text' }]; + // A column with no resolvable identity cannot be filtered or sorted + // on — it used to become an option keyed `undefined`. Drop it. + const fieldName = columnIdentity(f); + if (!fieldName) return []; + return [{ value: fieldName, - label: tFieldLabel(fieldName, f.label || f.name), + // The label falls back to the identity, not to the raw `name`: + // after #3104 a legacy `name` mirrors the identity anyway, and + // reading it here would resurrect the second spelling. + label: tFieldLabel(fieldName, f.label || fieldName), type: f.type || 'text', options: buildOptions(fieldName, f.options), referenceTo: f.reference_to || f.reference, displayField: f.display_field || f.reference_field, idField: f.id_field, - }; + }]; }); } else { fields = Object.entries(objectDef.fields).map(([key, field]: [string, any]) => ({ @@ -1843,8 +1846,8 @@ export const ListView = React.forwardRef(({ && (exportConfig as any)?.streaming !== false; if (serverEligible) { const fields = effectiveFields - .map((f: any) => typeof f === 'string' ? f : (f.name || f.fieldName || f.field)) - .filter(Boolean); + .map((f: any) => columnIdentity(f)) + .filter(Boolean) as string[]; // The same three filter sources as the data fetch, from the same function. const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions); @@ -1903,7 +1906,7 @@ export const ListView = React.forwardRef(({ const exportData = maxRecords > 0 ? data.slice(0, maxRecords) : data; if (format === 'csv') { - const fields = effectiveFields.map((f: any) => typeof f === 'string' ? f : (f.name || f.fieldName || f.field)); + const fields = effectiveFields.map((f: any) => columnIdentity(f)).filter(Boolean) as string[]; const rows: string[] = []; if (includeHeaders) { rows.push(fields.join(',')); @@ -1953,13 +1956,18 @@ export const ListView = React.forwardRef(({ // All available fields for hide/show (with i18n) const allFields = React.useMemo(() => { - return (Array.isArray(schema.columns) ? (schema.columns as any[]) : []).map((f: any) => { + return (Array.isArray(schema.columns) ? (schema.columns as any[]) : []).flatMap((f: any) => { if (typeof f === 'string') { - return { name: f, label: tFieldLabel(f, f) }; + return [{ name: f, label: tFieldLabel(f, f) }]; } - const name = f.name || f.fieldName || f.field; - const rawLabel = f.label || f.name || f.field; - return { name, label: tFieldLabel(name, rawLabel) }; + // `name` here is this popover's OWN key (it drives `hiddenFields`), which + // is why it keeps that shape — but the value is the column identity, so + // hiding a column and projecting it now agree (#3104). + const name = columnIdentity(f); + // No resolvable identity → nothing to hide or show; it used to render a + // checkbox keyed `undefined` that could never match a column. + if (!name) return []; + return [{ name, label: tFieldLabel(name, f.label || name) }]; }); }, [schema.columns, tFieldLabel]); diff --git a/packages/plugin-tree/src/ObjectTree.tsx b/packages/plugin-tree/src/ObjectTree.tsx index c451262f1a..333f87e071 100644 --- a/packages/plugin-tree/src/ObjectTree.tsx +++ b/packages/plugin-tree/src/ObjectTree.tsx @@ -23,7 +23,7 @@ import React, { useEffect, useMemo, useState } from 'react'; import type { DataSource, ViewData } from '@object-ui/types'; import { useNavigationOverlay, useSafeFieldLabel } from '@object-ui/react'; import { NavigationOverlay, cn } from '@object-ui/components'; -import { extractRecords, buildExpandFields } from '@object-ui/core'; +import { extractRecords, buildExpandFields, columnIdentity } from '@object-ui/core'; import { ChevronRight, ChevronDown } from 'lucide-react'; export interface ObjectTreeProps { @@ -65,9 +65,9 @@ function getDataConfig(schema: any): ViewData | null { * resilient regardless of caller. */ function fieldKey(f: any): string | undefined { - if (typeof f === 'string') return f; - if (f && typeof f === 'object') return f.name || f.fieldName || f.field || f.key; - return undefined; + // `key` stays a tail fallback — it is a generic entry key, not ObjectStack + // metadata identity, so it is not part of `columnIdentity` (#3104). + return columnIdentity(f) || (f && typeof f === 'object' ? f.key : undefined) || undefined; } function getTreeConfig(schema: any): TreeConfig {