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
26 changes: 26 additions & 0 deletions .changeset/dashboard-expandable-family-convergence-5692.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/plugin-dashboard': patch
---

`plugin-dashboard`'s two private copies of the reference-bearing field family now
read `@object-ui/core`'s published `EXPANDABLE_FIELD_TYPES` instead of restating
it — `LOOKUP_TYPES` in `recordFields.tsx` and the inline disjunction inside
`computeLookupExpand` in `ObjectDataTable.tsx`. Both carry the identity pin the
already-converged consumers carry, so a member-identical private copy fails
rather than quietly re-forking the table.

Two behaviour changes on the dashboard table's `$expand`, in opposite directions:

- **A `tree` column is now expanded.** A self-referencing hierarchy field is
reference-bearing and a member of the shared family, so its cell renders the
parent record's display name instead of a bare id — the same treatment the form
and grid roads already gave it.
- **A `reference`-typed column is no longer expanded.** Measured before removing
it: `reference` is absent from `@objectstack/spec`'s closed `FieldType`
vocabulary and is refused by `FieldSchema.safeParse`, so no spec-compliant
object schema can declare a field whose stored type is `reference`. Dropping it
is a no-op on real data; the spelling is a legacy dialect alias on the
action-param surface, folded to `lookup` before any field-type data is read.

`EXPANDABLE_FIELD_TYPES` itself is unchanged — the measurement did not license
widening a published shared set.
47 changes: 44 additions & 3 deletions packages/core/src/utils/expand-fields.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,10 +23,19 @@ import { columnIdentity } from './column-identity.js';
* belongs in this set; whether the backend materialises the expanded object
* for it is a server concern — requesting it is harmless and forward-compatible.
*
* ## One family, four consumers
* ## One family, many consumers — and NO reliable count of them
*
* This section used to open "One family, four consumers" and enumerate four.
* That count was hand-kept, and hand-kept counts of this table are exactly what
* keeps going wrong: it was already stale by two when objectui#5692 measured it
* (`paramToField` in `app-shell` had joined with objectui#5312, and
* `ListView`'s relational-sort rule reads the set directly), and objectui#5312's
* claim to have converted "the LAST private copy" was false by two MORE — see
* the falsification note at the end. Read the list below as the lineage of the
* conversions, not as a census; the mechanical fact is the identity pins.
*
* This is the reference-bearing FAMILY, not the `$expand` builder's private
* list, and four concerns already read it under three different words:
* list, and these concerns read it under several different words:
*
* - `$expand` construction — `buildExpandFields` below ("expandable");
* - predicate-record projection — `predicate-record.ts` ("relational");
Expand All@@ -37,7 +46,16 @@ import { columnIdentity } from './column-identity.js';
* - the grid's bulk-action dialog — `widgetNeedsDataSource` in
* `packages/plugin-grid/src/components/bulkParamToField.ts`, which decides
* which param widget is handed the grid's `DataSource` and which param field
* shape carries `reference_to` / `display_field`.
* shape carries `reference_to` / `display_field`;
* - the action-param dialog — `paramToField` in
* `packages/app-shell/src/utils/paramToField.ts`, which decides which param
* carries a reference target (objectui#5312);
* - the list view's relational-sort rule — `ListView.tsx`, which will not offer
* a server-side sort on a field whose stored value is a foreign key;
* - the dashboard table's `$expand` whitelist — `computeLookupExpand` in
* `packages/plugin-dashboard/src/ObjectDataTable.tsx` (objectui#5692);
* - the dashboard's relation/link test — `isLookupType` in
* `packages/plugin-dashboard/src/recordFields.tsx` (objectui#5692).
*
* The third one used to be a second hand-maintained copy, and this comment used
* to claim the set "mirrors the form layer's `DATA_SOURCE_FIELD_TYPES`
Expand DownExpand Up@@ -69,6 +87,29 @@ import { columnIdentity } from './column-identity.js';
* carry an identity pin (a spy on this `has`) so a member-identical private copy
* fails rather than quietly re-forking the table.
*
* ## The "LAST private copy" claim was false — objectui#5692
*
* objectui#5312 converted `paramToField` (`app-shell`) and recorded it as the
* fourth and LAST private copy of this rule. `packages/plugin-dashboard` held
* two more the whole time — `LOOKUP_TYPES` in `recordFields.tsx` and an inline
* disjunction inside `computeLookupExpand` in `ObjectDataTable.tsx` — which
* predate that sweep and were outside its file surface, so nothing contradicted
* the claim. Both now derive from this set with NO extension and carry the same
* identity pin.
*
* Those two were NOT member-identical to this set, in either direction: they
* lacked `tree` and carried a fifth spelling, `reference`. Converging them was
* therefore a behaviour change in two directions, and the direction that could
* have widened THIS set was settled by measurement rather than by preference:
* `reference` is absent from `@objectstack/spec`'s closed `FieldType`
* vocabulary and is refused by `FieldSchema.safeParse` — measured with `lookup`
* / `master_detail` / `user` / `tree` as live controls and the retired `owner`
* plus a nonsense spelling as dead ones — so no spec-compliant object schema can
* declare a field whose stored type is `reference`, and the dashboard's copies
* were carrying a dead spelling, not a member this set was missing. This set is
* unchanged by objectui#5692; the two dashboard faces simply stopped answering
* for a type no producer can emit.
*
* Stated so it reads as a decision rather than a surprise: **adding a member
* here also grants that type the form's data-source wiring.** That is the
* intended coupling — a type whose stored value is a foreign key needs a
Expand Down
35 changes: 32 additions & 3 deletions packages/plugin-dashboard/src/ObjectDataTable.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,9 @@ import {
// re-exports the same function object; read here from its home.
isRetiredFieldType,
reportRetiredFieldType,
// The reference-bearing field family (objectui#5692). Read, never copied —
// see the convergence note on `computeLookupExpand`.
EXPANDABLE_FIELD_TYPES,
} from '@object-ui/core';
import type { DrillDownConfig } from '@object-ui/types';
import { Skeleton, RefreshIndicator, cn } from '@object-ui/components';
Expand DownExpand Up@@ -173,8 +176,9 @@ export function normalizeColumns(columns: (string | Record<string, any>)[]): Nor
/**
* Compute the list of lookup-typed accessors that should be expanded when
* fetching rows. Returns column accessors whose object schema field type is
* a relation (lookup/reference/master_detail/user). Used by the
* dashboard table widget to ask the data adapter to populate referenced
* a relation. Which types those are is NOT restated here: it is
* {@link EXPANDABLE_FIELD_TYPES}, the family `@object-ui/core` publishes. Used
* by the dashboard table widget to ask the data adapter to populate referenced
* records (e.g. `account: { id, name }`) so cells don't show raw FK ids.
*
* THE GATE (objectui#4914, ruling B) runs ahead of the relation test. Measured
Expand All@@ -185,6 +189,28 @@ export function normalizeColumns(columns: (string | Record<string, any>)[]): Nor
* show once the spelling stopped being a relation. That the author is TOLD is
* the whole difference between this and the mechanical deletion the
* measurement rejected.
*
* ## The relation test is core's object, not a private copy (objectui#5692)
*
* It used to be the inline literal
* `t === 'lookup' || t === 'reference' || t === 'master_detail' || t === 'user'`
* — one of TWO copies this package held (the other `LOOKUP_TYPES` in
* `recordFields.tsx`), neither deriving from nor pinned against the family core
* publishes. objectui#5312's claim to have converted "the LAST private copy"
* was false by these two; they predate that sweep and were outside its file
* surface.
*
* This is the LIVE half of that convergence — `computeLookupExpand` drives a
* real `$expand` on every dashboard table fetch — so both membership deltas are
* observable here, and both were decided by measurement (see `isLookupType` in
* `recordFields.tsx` for the full record):
*
* - a `tree` column now GETS `$expand`-ed, the same treatment the form / grid
* road already gives it;
* - a `reference` column no longer does, and that is a no-op on spec-compliant
* data: the spelling is absent from `@objectstack/spec`'s closed `FieldType`
* and refused by `FieldSchema.safeParse`, so no object schema can declare a
* field whose stored type is `reference`.
*/
export function computeLookupExpand(
schema: { columns?: any[]; objectName?: string },
Expand All@@ -202,7 +228,10 @@ export function computeLookupExpand(
reportRetiredFieldType(t);
return false;
}
return t === 'lookup' || t === 'reference' || t === 'master_detail' || t === 'user';
// Never `new Set([...EXPANDABLE_FIELD_TYPES, …])` and never a re-listing of
// its members: a copy re-forks the table, which is the defect this removed,
// and the identity pin fails on it by design.
return EXPANDABLE_FIELD_TYPES.has(t as string);
};

const cols = Array.isArray(schema.columns) ? schema.columns : [];
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
/**
* 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#5692 — this package's TWO private copies of the reference-bearing
* field family converge onto `@object-ui/core`'s `EXPANDABLE_FIELD_TYPES`.
*
* The copies were `LOOKUP_TYPES` in `recordFields.tsx` and an inline
* disjunction inside `computeLookupExpand` in `ObjectDataTable.tsx`. Neither
* derived from nor pinned against the shared set, and objectui#5312 had recorded
* `paramToField` as the LAST private copy — false by these two, which predate
* that sweep.
*
* ## Why the load-bearing pin is IDENTITY, not membership
*
* Every membership assertion below is satisfied by a private
* `new Set(['lookup', 'master_detail', 'tree', 'user'])` holding the same
* strings — i.e. by a re-fork of exactly the kind this change removed. So the
* pins that decide the convergence spy on the `has` of the object core exports:
* a call is recorded only if the face under test consulted THAT object, so a
* member-identical copy leaves the spy empty and fails here, where a value check
* would pass ON the defect. Same shape as objectui#4770 / #4790 / #4815 / #5312.
*
* ## The two membership deltas, and how each was decided
*
* The private copies were not in a subset relation with the shared set in either
* direction: they lacked `tree` and carried a fifth spelling, `reference`.
*
* - `tree` GAINED (accepted): a member of the spec's closed `FieldType` that
* the form / grid road already expands.
* - `reference` DROPPED (measured, not preferred): it is not a declarable field
* type at all, so no producer can emit a field whose stored type is
* `reference`. `describe('the reference drop is a no-op...')` below carries
* that measurement — with live and dead controls — so the day the spec adds
* the spelling, this file goes RED and the membership question reopens
* instead of the drop staying silently correct-by-accident.
*
* Ablation direction, predicted before running: restore either private copy and
* that face's identity pin goes RED (the spy records no call) while its `tree`
* pin goes red too and its `reference` pin flips; the ordinary-relation
* regression controls stay GREEN in both directions, which is what makes them
* controls rather than duplicates of the pins.
*/
import { describe, it, expect, vi } from 'vitest';
import { EXPANDABLE_FIELD_TYPES } from '@object-ui/core';
import { FieldType } from '@objectstack/spec/data';
import { isLookupType } from '../recordFields';
import { computeLookupExpand } from '../ObjectDataTable';

const SPEC_FIELD_TYPES: readonly string[] = [
...(FieldType as unknown as { options: readonly string[] }).options,
];

/** The relations an ordinary dashboard table shows — the regression control. */
const ORDINARY_RELATIONS = ['lookup', 'master_detail', 'user'] as const;

const objectSchema = () => ({
fields: {
id: { type: 'text' },
title: { type: 'text' },
account: { type: 'lookup', reference: 'accounts' },
parent_case: { type: 'master_detail', reference: 'cases' },
assignee: { type: 'user' },
parent_node: { type: 'tree', reference: 'nodes' },
legacy_ref: { type: 'reference', reference: 'accounts' },
},
});

const ALL_COLUMNS = [
'title',
'account',
'parent_case',
'assignee',
'parent_node',
'legacy_ref',
];

describe("the dashboard's relation rule is core's object, not a copy (objectui#5692)", () => {
it('`isLookupType` asks `@object-ui/core` EXPANDABLE_FIELD_TYPES', () => {
const spy = vi.spyOn(EXPANDABLE_FIELD_TYPES, 'has');
try {
expect(isLookupType('lookup')).toBe(true);
expect(spy.mock.calls.map(([k]) => k)).toContain('lookup');
} finally {
spy.mockRestore();
}
});

it('`computeLookupExpand` asks it too — in BOTH column modes', () => {
// The explicit-whitelist mode and the auto-derive mode are two separate
// code paths through the predicate, so a convergence that reconnected one
// would leave the other forked. Each is spied separately.
const modes: [string, () => unknown][] = [
['explicit whitelist', () =>
computeLookupExpand({ columns: ALL_COLUMNS }, objectSchema())],
['auto-derive', () => computeLookupExpand({}, objectSchema())],
];
for (const [label, exercise] of modes) {
const spy = vi.spyOn(EXPANDABLE_FIELD_TYPES, 'has');
try {
exercise();
expect(
spy.mock.calls.map(([k]) => k),
`${label} never consulted the shared set`,
).toContain('lookup');
} finally {
spy.mockRestore();
}
}
});
});

describe('the ordinary relations are untouched — regression control', () => {
// These must stay green through BOTH ablation legs. If they move, the
// convergence took the whole whitelist with it and the pins above are
// reporting on rubble rather than on a re-homed rule.
it('`isLookupType` still answers true for every ordinary relation', () => {
for (const type of ORDINARY_RELATIONS) {
expect(isLookupType(type), type).toBe(true);
}
expect(isLookupType('text')).toBe(false);
});

it('`$expand` still carries the ordinary relation columns, in both modes', () => {
const explicit = computeLookupExpand({ columns: ALL_COLUMNS }, objectSchema());
const auto = computeLookupExpand({}, objectSchema());
for (const expanded of [explicit, auto]) {
expect(expanded).toEqual(
expect.arrayContaining(['account', 'parent_case', 'assignee']),
);
expect(expanded).not.toContain('title');
expect(expanded).not.toContain('id');
}
});
});

describe('`tree` gains expansion on the dashboard road — the accepted direction', () => {
// A self-referencing hierarchy column is reference-bearing, so the form and
// grid roads already `$expand` it. The dashboard's private copies did not,
// which is the divergence this convergence closes; the column's cell shows
// the parent record's display name instead of a bare id.
it('is a member of the shared family', () => {
expect(EXPANDABLE_FIELD_TYPES.has('tree')).toBe(true);
});

it('`isLookupType` now answers true for it', () => {
expect(isLookupType('tree')).toBe(true);
});

it('a `tree` column is now requested for `$expand`, in both modes', () => {
expect(
computeLookupExpand({ columns: ALL_COLUMNS }, objectSchema()),
).toContain('parent_node');
expect(computeLookupExpand({}, objectSchema())).toContain('parent_node');
});
});

describe('the `reference` drop is a no-op on real data — the measured direction', () => {
/**
* The measurement, kept as an executable pin rather than as prose in a PR.
* Controls run on the same read as the subject, so a probe that had lost hold
* of the vocabulary (an empty list, the wrong export) fails as a broken probe
* instead of reporting the subject absent.
*/
it('every LIVE control IS a spec `FieldType`, and every DEAD one is not', () => {
// Live controls: the four members of the shared family.
for (const type of EXPANDABLE_FIELD_TYPES) {
expect(SPEC_FIELD_TYPES, `'${type}' is not a spec FieldType`).toContain(type);
}
// Dead controls: a spelling this renderer retired, and pure nonsense.
// If either turns up "present", the read is broken and the subject reading
// below means nothing.
expect(SPEC_FIELD_TYPES).not.toContain('owner');
expect(SPEC_FIELD_TYPES).not.toContain('zzz_not_a_field_type');
});

it('SUBJECT — `reference` is not a declarable field type', () => {
// The whole licence for dropping it. If the spec ever adds the spelling,
// this goes red and the "should the shared family gain `reference`?"
// question reopens — deliberately, rather than the drop remaining correct
// only by accident.
expect(SPEC_FIELD_TYPES).not.toContain('reference');
});

it('so the dashboard no longer answers for it', () => {
expect(isLookupType('reference')).toBe(false);
expect(
computeLookupExpand({ columns: ALL_COLUMNS }, objectSchema()),
).not.toContain('legacy_ref');
expect(computeLookupExpand({}, objectSchema())).not.toContain('legacy_ref');
});
});
Loading
Loading