Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .changeset/view-column-io-canonical-identity-5725.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
'@object-ui/app-shell': minor
---

The metadata designer's View **column list** now reads a column's identity —
row label and bound field name — in the ObjectStack canonical spelling only:
`field` and `label`. The legacy TanStack aliases `accessorKey` / `header` are no
longer consulted (objectui#5725). This is the editor-side half of the read
objectui#5344 retired in the column inspector one file over; the two surfaces
render inside the **same panel** and until now gave the author two different
answers about what a column is called.

Why this is a behaviour change and not a tidy-up: `ListColumn` refuses both
legacy keys by name (`unrecognized_keys`), so a column carrying them has no
field key and no label the spec recognises. Two consequences the list surface
had that the inspector did not:

- **The label chain was inverted, not merely tolerant.** It read
`label ?? header ?? field ?? accessorKey`, preferring the undeclared `header`
**over** the declared `field`. A perfectly canonical column that also carried
a stray `header` key displayed the alias' value *instead of* its own declared
identity.
- **The field-name read backs more than a label.** It feeds `usedFieldNames()`,
which the Add-field picker consults, so a spec-refused column reserved a field
name and the picker tagged that field "Added" — for a column no accepted
document actually binds.

**What an author sees:** a stored column shaped `{ accessorKey, header }` is now
named positionally in the list (`col 1`) rather than by its refused spelling,
and the row stays selectable exactly as before — the positional fallback already
existed at the end of that chain. The Add-field picker no longer reports such a
column's field name as taken. Canonical `{ field, label }` columns and
bare-string columns are untouched, and a declared `label` / `field` now outranks
any stray alias beside it.

**What is deliberately NOT changed:** the write path. These helpers still
reorder and splice the raw `columns` array without normalising it, so no stored
document is rewritten by the act of viewing or editing it — the same fence
objectui#5344 held.
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,6 +24,12 @@
* - A legacy column therefore stays unsaveable before AND after an edit.
* That closed loop is the RULED OUTCOME, not an oversight; what this change
* removes is its invisibility.
*
* The last case in this suite was written by objectui#5344 as a pin DOCUMENTED
* TO FLIP: the column list rendered beside these controls read the same retired
* aliases through `previews/view-column-io.ts`, which was outside that card's
* granted surface. objectui#5725 retired that read, and the pin is flipped
* accordingly — the panel now gives one answer instead of two.
*/

import '@testing-library/jest-dom/vitest';
Expand DownExpand Up@@ -154,22 +160,33 @@ describe('ViewColumnInspector — identity is read in the canonical spelling onl
expect(parses(['title'])).toBe(true);
});

it('records the residue this card is fenced out of: the column LIST still shows the legacy name', () => {
it('the whole panel agrees: no surface presents the refused spelling as an identity', () => {
mount([{ accessorKey: 'name', header: 'Name' }]);

// `FieldsListEditor` renders inside this same panel and reads its row
// labels through `previews/view-column-io.ts`, which still carries the very
// FLIPPED, as objectui#5344 wrote this pin to be. It used to assert
// `toHaveLength(1)` and carried the residue objectui#5344 was fenced out
// of: `FieldsListEditor` renders inside this same panel and read its row
// labels through `previews/view-column-io.ts`, which still carried the
// alias retired above (`o.label ?? o.header ?? o.field ?? o.accessorKey`).
// So the panel's identity controls stop presenting the refused spelling
// while the list one line above still does. That file is outside this
// card's granted surface; filed as objectui#5725. When that lands, this
// expectation flips — deliberately, so the boundary is visible rather than
// silently forgotten.
// So the panel's identity controls stopped presenting the refused spelling
// while the list one line above still did — one panel, two answers.
// objectui#5725 retired that read too, so the count goes 2 → 1 → 0 across
// the two cards and the disagreement is closed.
//
// Counted, not merely asserted present: post-change EXACTLY ONE element
// says `Name`, the list row. Against the pre-change source there were two
// (the panel title read `header` as the column's label as well), which is
// why this reverse-verifies as a "found multiple elements" failure.
expect(screen.getAllByText('Name')).toHaveLength(1);
// This is the observable the objectui#5344 ruling required and did not get:
// asserted as a COUNT over the whole panel, so a surface re-acquiring the
// alias reverse-verifies as a "found N elements" failure rather than
// passing unnoticed.
expect(screen.queryAllByText('Name')).toHaveLength(0);

// …and the list row is not nameless: it names the column positionally, so
// the author still has a row to click. Measured end-to-end (including that
// the row selects) in FieldsListEditor.retiredAliases.test.tsx.
expect(screen.getByText('col 1')).toBeInTheDocument();

// The identity controls are unchanged by objectui#5725 — pinned here so the
// "agree" claim is two-sided rather than a claim about the list alone.
expect(shownFieldKey()).toEqual({ widget: 'input', text: '' });
expect(shownHeader()).toBe('');
});
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* What the column manager SHOWS and RESERVES after the identity read is
* retired to the canonical spelling (objectui#5725).
*
* The unit pins live in `view-column-io.retiredAliases.test.ts`; this suite
* mounts the real `FieldsListEditor` because two of the claims are only true
* end-to-end:
*
* - the positional fallback `col N` actually produces a row the author can
* CLICK. That was the open question objectui#5725 declined to take — an
* empty field-key box invites re-authoring, an empty list row would leave
* nothing to click — and it is answered here by measurement rather than by
* reading the fallback and assuming a row comes out of it.
* - `usedFieldNames()` reaches the Add-field picker. Measured through the
* real popover, so the reachability claim is not taken on trust.
*
* NOTE on the size of defect 2: the picker does NOT hide a used field, it tags
* it "Added" and still lets the author click it. So a spec-refused column
* mislabelled a field as taken; it never blocked adding it. Recorded here so
* the pin says what the mechanism does, not what it was assumed to do.
*/

import '@testing-library/jest-dom/vitest';
import * as React from 'react';
import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, fireEvent, cleanup, within } from '@testing-library/react';

// `useObjectFields` short-circuits the fetch when handed a catalog, but still
// constructs the shared metadata client — stub it so no test reaches the
// network. Same mechanism as ViewColumnInspector.identityRead.test.tsx.
const state = vi.hoisted(() => ({
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
}));
vi.mock('../useMetadata', () => ({
useMetadataClient: () => state.metadataClient,
}));

import { FieldsListEditor } from './FieldsListEditor';

afterEach(cleanup);

const CATALOG = [
{ name: 'name', label: 'Name', type: 'text', hidden: false },
{ name: 'amount', label: 'Amount', type: 'number', hidden: false },
];

/** The stored shape the whole retirement family is about. */
const LEGACY = { accessorKey: 'name', header: 'Name' };

function mount(columns: unknown[], onSelectionChange = vi.fn()) {
render(
<FieldsListEditor
variantKey="list"
schema={{ type: 'grid', columns }}
columns={columns}
allStrings={false}
objectName="invoices"
objectFieldsOverride={CATALOG}
selectedIndex={null}
onPatch={vi.fn()}
onSelectionChange={onSelectionChange}
/>,
);
return onSelectionChange;
}

/**
* Open the Add-field picker and return its content element.
*
* Scoped to the popover rather than the whole render: the column list rows are
* `role="button"` too, so an unscoped query for an option matches the row of a
* canonical column as well and the test dies on ambiguity instead of measuring
* anything.
*/
function openPicker(): HTMLElement {
fireEvent.click(screen.getByRole('button', { name: /Add field/i }));
return screen.getByRole('dialog');
}

/** One field's row inside the open picker. */
const option = (picker: HTMLElement, label: string) =>
within(picker).getByRole('button', { name: new RegExp(`^${label}`) });

describe('FieldsListEditor — a spec-refused column names itself positionally', () => {
it('renders a CLICKABLE row for a legacy column instead of its retired alias', () => {
const onSelectionChange = mount([LEGACY]);

// The retired alias is gone from the list…
expect(screen.queryByText('Name')).not.toBeInTheDocument();
// …and the row is not nameless: it says `col 1`.
expect(screen.getByText('col 1')).toBeInTheDocument();

// The measurement the open question turns on: the row still selects.
const row = screen.getByText('col 1').closest('[role="button"]') as HTMLElement;
fireEvent.click(row);
expect(onSelectionChange).toHaveBeenCalledWith({
kind: 'column',
id: 'list.columns[0]',
label: 'col 1',
});
});

it('counter-probe: a canonical column still shows its declared label', () => {
mount([{ field: 'amount', label: 'Amount' }]);

expect(screen.getByText('Amount')).toBeInTheDocument();
expect(screen.queryByText('col 1')).not.toBeInTheDocument();
});

it('lets a declared identity outrank a stray undeclared alias', () => {
// The column carries NO `label`, which is the shape the inverted chain
// actually bit on: `label ?? header ?? field` reached `header` before
// `field`, so this row read `STRAY` instead of its own declared field.
// Measured: with `label` also present the old chain returns `label` too,
// so a `{field, label, header}` row cannot reverse-verify this leg at all
// — it is green against both sources and would have been a phantom pin.
mount([{ field: 'amount', header: 'STRAY' }]);

expect(screen.getByText('amount')).toBeInTheDocument();
expect(screen.queryByText('STRAY')).not.toBeInTheDocument();
});
});

describe('FieldsListEditor — a spec-refused column reserves no field name', () => {
it('does not tag `name` as Added for a legacy {accessorKey, header} column', () => {
mount([LEGACY]);
const picker = openPicker();

// Guard against a vacuous pass: the picker really opened and really lists
// the field, so the absent badge below is a measurement, not an empty DOM.
expect(option(picker, 'Name')).toBeInTheDocument();
expect(within(picker).queryByText('Added')).not.toBeInTheDocument();
});

it('counter-probe: a canonical column DOES still reserve its name', () => {
mount([{ field: 'name', label: 'Name' }]);
const picker = openPicker();

expect(within(option(picker, 'Name')).getByText('Added')).toBeInTheDocument();
// Falsification: the badge is keyed to the bound name, not shown on every row.
expect(within(option(picker, 'Amount')).queryByText('Added')).not.toBeInTheDocument();
});
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `view-column-io` reads a column's identity in the CANONICAL spelling only
* (objectui#5725) — the editor-side half of the retirement `ViewColumnInspector`
* landed one file over in objectui#5344.
*
* `ListColumn` refuses `accessorKey` / `header` by name (`unrecognized_keys`),
* so a column carrying them has no field key and no label the spec recognises.
* These helpers nevertheless read both, which produced two distinct defects —
* pinned separately below because they are separate legs, not one:
*
* 1. `colLabel`'s chain was `label ?? header ?? field ?? accessorKey`:
* INVERTED, not merely tolerant. `header` was preferred OVER `field`, so a
* canonical column carrying a stray `header` displayed the undeclared
* alias INSTEAD of its own declared identity.
* 2. `colFieldName` backs `usedFieldNames()`, which the Add-field picker
* consults, so a spec-refused column RESERVED a field name — a display
* alias leaking into a non-display decision.
*
* The counter-probe rows are load-bearing: "the aliases are gone" is otherwise
* satisfiable by breaking the label and the reservation outright.
*/

import { describe, it, expect } from 'vitest';
import { colFieldName, colLabel, usedFieldNames } from './view-column-io';

/** The stored shape the whole retirement family is about. */
const LEGACY = { accessorKey: 'name', header: 'Name' };

describe('view-column-io · colLabel reads the canonical spelling only', () => {
it('names a spec-refused column positionally, never by its retired alias', () => {
// Not `''`: the positional fallback is what keeps the row clickable, and
// it is the only reason dropping the alias is safe for the LIST surface
// (the asymmetry objectui#5725 named — an empty field-key box invites
// re-authoring, an empty list row would leave nothing to click).
expect(colLabel(LEGACY, 0)).toBe('col 1');
expect(colLabel({ header: 'Name' }, 3)).toBe('col 4');
expect(colLabel({ accessorKey: 'name' }, 0)).toBe('col 1');
});

it('lets a DECLARED identity outrank a stray undeclared alias', () => {
// The inverted-precedence leg. Pre-change these read 'STRAY'/'STRAY'.
expect(colLabel({ field: 'name', label: 'Name', header: 'STRAY' }, 0)).toBe('Name');
expect(colLabel({ field: 'name', header: 'STRAY' }, 0)).toBe('name');
});

it('counter-probe: canonical and bare-string columns are untouched', () => {
expect(colLabel({ field: 'amount', label: 'Amount' }, 0)).toBe('Amount');
expect(colLabel({ field: 'amount' }, 0)).toBe('amount');
expect(colLabel('amount', 0)).toBe('amount');
expect(colLabel('', 0)).toBe('col 1');
expect(colLabel(null, 1)).toBe('col 2');
});
});

describe('view-column-io · colFieldName / usedFieldNames bind the canonical key only', () => {
it('does not let a spec-refused column bind a field name', () => {
expect(colFieldName(LEGACY)).toBeUndefined();
expect(colFieldName({ accessorKey: 'name' })).toBeUndefined();
});

it('does not let a spec-refused column RESERVE a name in the picker', () => {
// The reachability claim: this Set is what the Add-field picker reads.
// Pre-change it contained 'name', so the picker reported the field as
// already taken for a column no accepted document actually binds.
expect([...usedFieldNames([LEGACY])]).toEqual([]);
});

it('counter-probe: canonical and bare-string columns still reserve their names', () => {
expect(colFieldName({ field: 'amount', label: 'Amount' })).toBe('amount');
expect(colFieldName('amount')).toBe('amount');
expect([...usedFieldNames([{ field: 'amount' }, 'status', LEGACY])].sort()).toEqual([
'amount',
'status',
]);
});
});
Loading
Loading