From 09d4968100df176d025ce6331b4468bd74b03020 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 11:12:47 +0000 Subject: [PATCH] fix(app-shell): relay a list view's own fieldOrder to ListView renderListView's relay carried the active view's hiddenFields onto the schema it hands ListView but had no rung for fieldOrder, the other per-view half of the columns x hiddenFields x fieldOrder composition, so a per-view order was accepted, served and then dropped. Add the rung beside hiddenFields with the same precedence (view over list node), pin it at the relay, and replace the census ledger's known-gap row with a named check for the rung. Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN Co-authored-by: Claude --- .changeset/7516-fieldorder-relay-rung.md | 18 ++ .../ObjectView.fieldOrderRelay-7516.test.tsx | 257 ++++++++++++++++++ .../ObjectView.relayRungCensus-7559.test.ts | 41 ++- packages/app-shell/src/views/ObjectView.tsx | 20 +- 4 files changed, 323 insertions(+), 13 deletions(-) create mode 100644 .changeset/7516-fieldorder-relay-rung.md create mode 100644 packages/app-shell/src/views/ObjectView.fieldOrderRelay-7516.test.tsx diff --git a/.changeset/7516-fieldorder-relay-rung.md b/.changeset/7516-fieldorder-relay-rung.md new file mode 100644 index 0000000000..d7d7e7c4fe --- /dev/null +++ b/.changeset/7516-fieldorder-relay-rung.md @@ -0,0 +1,18 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(app-shell): a list view's own `fieldOrder` now reaches the list + +A `fieldOrder` authored on a per-list-view entry (`listViews.VIEW_NAME.fieldOrder`) +was accepted and served, then silently dropped: `ObjectView`'s `renderListView` +relay carried the view's `hiddenFields` onto the schema it hands `ListView` but +had no rung for `fieldOrder`, so the list could only ever be ordered by a +`fieldOrder` already on the host's list schema. It is relayed now, beside +`hiddenFields` and with the same precedence: the view's value wins, and a view +that authors none keeps the list's own. + +`ListView` composes it exactly as it always has — `columns` projects, +`hiddenFields` subtracts, `fieldOrder` sorts what survives, and a surviving +column it does not list sorts last in its `columns` order — the composition +objectstack#15184 wrote into the protocol. diff --git a/packages/app-shell/src/views/ObjectView.fieldOrderRelay-7516.test.tsx b/packages/app-shell/src/views/ObjectView.fieldOrderRelay-7516.test.tsx new file mode 100644 index 0000000000..7f04ddd55b --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.fieldOrderRelay-7516.test.tsx @@ -0,0 +1,257 @@ +/** + * 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#7516 — the object page relays a per-view `fieldOrder`. + * + * ## The defect this pins + * + * `renderListView` builds `fullSchema` by spreading the host's `listSchema` + * and then relaying keys off the active `viewDef`. `hiddenFields` had a rung; + * `fieldOrder`, the other per-view half of the same field composition, had + * NONE. So `schema.fieldOrder` at the `ListView` end could only ever be what + * the host put on the list schema, and a per-view order was unreachable — + * declared by the protocol, accepted and served, then dropped here. The + * relay-rung census (`ObjectView.relayRungCensus-7559.test.ts`) carried it as a + * `known-gap` until this card. + * + * ## What the rung carries, and what it does not decide + * + * objectstack#15184 ruling B kept `fieldOrder` and wrote the composition into + * the contract: `columns` projects, `hiddenFields` subtracts from the + * projection, `fieldOrder` sorts what survives (a surviving column it does not + * list sorts last, keeping its `columns`-relative order). `ListView`'s + * `effectiveFields` memo already runs those steps in that order on whatever + * `schema.fieldOrder` it receives. The rung therefore only DELIVERS the view's + * value into that slot — the same slot the list-node spelling fills — so the + * two spellings compose identically by construction. Where both are present, + * the view wins, the precedence the `hiddenFields` rung beside it uses. + * + * ## The value DOES arrive here — the relay is where it died + * + * `buildViewTabs` composes each entry through `viewEntry`, which is + * `Object.assign` over the authored body and stamps only `id` afterwards, so an + * authored `fieldOrder` is present on `viewDef`. The in-tree host + * (`plugin-view`'s `ObjectView`) fills its own `fieldOrder` slot from a NAMED + * list view only, and this page hands it no `listViews`, so on this route the + * spread carries nothing for the key. + * + * ## Why the schema is captured rather than rendered + * + * The claim is about what THIS file hands down, so `ListView` is stubbed and + * its `schema` prop recorded — the same posture as + * `ObjectView.rowColorRelay-7218.test.tsx`. How the captured order then sorts + * the columns is `plugin-list`'s half, and is not re-pinned here. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + usePermissions: () => ({ + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + isLoaded: false, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }), + useFieldPermissions: () => ({ canRead: () => true, canWrite: () => true, permissions: [] }), + }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRealtimeSubscription: () => ({ lastMessage: null }), + useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }), +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +/** The list schema this page hands down — captured, not rendered. */ +let captured: any = null; +vi.mock('@object-ui/plugin-list', async (importOriginal) => ({ + ...(await importOriginal()), + ListView: (props: any) => { + captured = props.schema; + return null; + }, +})); + +/** + * What the HOST puts on the list schema before this page's relay runs — the + * `listSchema` the `...listSchema` spread carries in. It stands in for the + * list-node spelling (the in-tree host fills it from a named list view), and + * it is the only way to exercise the rung's second limb from here. + */ +let hostList: Record = {}; + +vi.mock('@object-ui/plugin-view', async (importOriginal) => ({ + ...(await importOriginal>()), + ObjectView: (props: any) => + props.renderListView?.({ + schema: { ...(props.schema ?? {}), ...hostList }, + dataSource: props.dataSource, + onEdit: props.onEdit, + className: '', + refreshKey: 0, + }) ?? null, + ViewTabBar: () => null, + ManageViewsDialog: () => null, +})); + +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); + +import { ObjectView } from './ObjectView'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; + +const OBJECT_NAME = 'duly_task'; + +/** The per-view order — the value this card is about. */ +const VIEW_FIELD_ORDER = ['stage', 'name']; +/** A list-node one. Distinct so a crossed wire fails instead of passing. */ +const LIST_FIELD_ORDER = ['owner', 'stage', 'name']; + +function objectsWith(view: Record) { + return [ + { + name: OBJECT_NAME, + label: 'Task', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + stage: { type: 'text', label: 'Stage' }, + owner: { type: 'text', label: 'Owner' }, + }, + listViews: { + by_stage: { label: 'By stage', type: 'grid', columns: ['name', 'stage', 'owner'], ...view }, + }, + }, + ]; +} + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as any; +} + +/** Render the object list and return the whole schema the relay handed down. */ +async function relayed(objects: any[]): Promise { + captured = null; + render( + + + + {}} />} + /> + + + , + ); + // `options` is built unconditionally by the same object literal as the rung + // under test, so its arrival is the signal that the relay actually ran — + // waiting on `fieldOrder` itself would hang rather than fail on a regression. + await waitFor(() => { + expect(captured?.options).toBeTruthy(); + }); + return captured; +} + +beforeEach(() => { + cleanup(); + captured = null; + hostList = {}; + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +describe("ObjectView relays the active view's own fieldOrder (objectui#7516)", () => { + it('THE FIX: a per-view `fieldOrder` reaches the renderer, verbatim', async () => { + // Before the rung existed this was `undefined` for every object on this + // route, which is the whole of the defect. Verbatim, because the order IS + // the value: the relay sorts nothing and filters nothing — composing it + // with `columns` and `hiddenFields` is `ListView`'s job. + const schema = await relayed(objectsWith({ fieldOrder: VIEW_FIELD_ORDER })); + expect(schema.fieldOrder).toEqual(VIEW_FIELD_ORDER); + }); + + it('THE FIX: the per-view value OVERRIDES a list-node `fieldOrder`', async () => { + hostList = { fieldOrder: LIST_FIELD_ORDER }; + const schema = await relayed(objectsWith({ fieldOrder: VIEW_FIELD_ORDER })); + expect(schema.fieldOrder).toEqual(VIEW_FIELD_ORDER); + }); + + it('THE FIX: `fieldOrder` resolves by the same precedence as the `hiddenFields` rung beside it', async () => { + // The two per-view halves of one composition. Both are supplied by the + // list node AND by the view; both must come from the view, or `ListView` + // would sort the view's subtraction by the list node's order. + hostList = { hiddenFields: ['owner'], fieldOrder: LIST_FIELD_ORDER }; + const schema = await relayed( + objectsWith({ hiddenFields: ['name'], fieldOrder: VIEW_FIELD_ORDER }), + ); + expect(schema.hiddenFields).toEqual(['name']); + expect(schema.fieldOrder).toEqual(VIEW_FIELD_ORDER); + }); + + it("CONTROL: the list node's own `fieldOrder` is unchanged when the view authors none", async () => { + // The rung is a fallback, not a replacement: a fix that stomped the + // incoming value with `undefined` fails here. + hostList = { fieldOrder: LIST_FIELD_ORDER }; + const schema = await relayed(objectsWith({})); + expect(schema.fieldOrder).toEqual(LIST_FIELD_ORDER); + }); + + it('CONTROL: no `fieldOrder` anywhere stays absent', async () => { + const schema = await relayed(objectsWith({})); + expect(schema.fieldOrder).toBeUndefined(); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.relayRungCensus-7559.test.ts b/packages/app-shell/src/views/ObjectView.relayRungCensus-7559.test.ts index d101e1bb72..877619253c 100644 --- a/packages/app-shell/src/views/ObjectView.relayRungCensus-7559.test.ts +++ b/packages/app-shell/src/views/ObjectView.relayRungCensus-7559.test.ts @@ -19,7 +19,8 @@ * not a type error, not a lint finding and not a test failure. It is silence, * and the silence has now produced the same defect three times — * objectui#7199 (`description`), objectui#7218 (`rowColor`), objectui#7516 - * (`fieldOrder`, still open). + * (`fieldOrder`). Each of the three now has its rung, and each rung is named + * by a check below. * * This file is the ruling on that card: option (b). The census is not a list * somebody typed once — it is RE-DERIVED here, at test time, from four sources @@ -84,8 +85,9 @@ * ## What this file does NOT do * * ⛔ It does not fill in a missing rung. Each absent rung is its own card - * (`fieldOrder` is objectui#7516, which carries `needs-user-decision`); - * this file only makes absences visible and keeps them declared. + * (`fieldOrder` was objectui#7516, and that card added the rung together + * with its check below); this file only makes absences visible and keeps + * them declared. * ⛔ It does not touch `plugin-view`'s `ObjectView` — that host is the * objectui#5043 family, and its own pin * (`plugin-view/src/__tests__/objectViewHostSurface.test.tsx`) is the @@ -476,9 +478,6 @@ const ABSENCES: Record = { // ── Authored on the NODE, resolved by the host ──────────────────────────── operations: { kind: 'node-authored', reason: "Legacy CRUD affordance authored on the object-view node (`examples/.../object-view-record-surface.json`), not on a view record; the host resolves it upstream (`schema.operations || schema.table?.operations || …`). ListView does read `schema.operations?.export`, but what would feed it here is the NODE's value, and forwarding it is the caller's composition to make — objectui#5097's surface, not a per-view rung." }, - // ── Real absences, owned elsewhere ──────────────────────────────────────── - fieldOrder: { kind: 'known-gap', card: 'objectui#7516', reason: "ListView reads `schema.fieldOrder` and orders its columns by it; no rung carries the view's value here. That is this defect class's third instance and it is OPEN with `needs-user-decision` — ⛔ do not close it by adding a rung in passing." }, - // ── The runtime-only half of the intersection ───────────────────────────── onNavigate: { kind: 'host-runtime', reason: 'Host callback. This host wires record navigation through the `onRowClick` prop on the `` element instead; a view record cannot carry a function.' }, refreshTrigger: { kind: 'host-runtime', reason: 'Host refresh counter, supplied by the caller; not view metadata.' }, @@ -595,6 +594,21 @@ describe('objectui#7559 — every `ListViewSchema` member has a rung or a declar ).toBe(true); }); + it('objectui#7516 — the per-view `fieldOrder` rung is present', () => { + // The third instance, carried in `ABSENCES` as a `known-gap` until its card + // added the rung. The ledger row is gone, so without this rung the key + // would be neither relayed nor declared and the property above would name + // it — but the instance is named here too, as the other two are. + expect( + readsView(writtenAt('fieldOrder')), + 'The `fieldOrder` rung is gone — objectui#7516, the third instance of the same class.\n' + + '`ListView` still orders its columns by `schema.fieldOrder`, so a per-view order is\n' + + 'authored, served and then silently dropped. Restore\n' + + '`fieldOrder: viewDef.fieldOrder ?? listSchema.fieldOrder` beside the `hiddenFields` rung.', + ).toBe(true); + expect(listViewReads('fieldOrder')).toBe(true); + }); + it('a member with a rung that does NOT read the active view is not counted as relayed', () => { // The reading, asserted rather than described: `...listSchema` carries the // host's value and cannot stand in for the view's. This is what made all @@ -763,13 +777,16 @@ describe('objectui#7559 — a declared absence keeps its evidence', () => { // literal. A control that reddens whenever anything else reddens says // nothing — and the first person to see it red would delete it. // - // `fieldOrder` is a real, card-owned, reader-having absence (objectui#7516) - // and `quickFilters` is the deliberate suppression the card itself named. - // Neither is a failure, and neither may become one. + // `operations` is a real, reader-having absence (`ListView` reads it, and + // the relay deliberately does not carry it — `node-authored`, objectui#5097's + // surface) and `quickFilters` is the deliberate suppression the card itself + // named. Neither is a failure, and neither may become one. (This control + // used `fieldOrder` until objectui#7516 gave that key its rung.) const findings = MEMBERS.filter((m) => !RELAYED.includes(m) && !(m in ABSENCES)); - expect(MEMBERS).toContain('fieldOrder'); - expect(RELAYED).not.toContain('fieldOrder'); - expect(findings).not.toContain('fieldOrder'); + expect(MEMBERS).toContain('operations'); + expect(RELAYED).not.toContain('operations'); + expect(listViewReads('operations')).toBe(true); + expect(findings).not.toContain('operations'); expect(MEMBERS).not.toContain('quickFilters'); expect(WRITTEN_TOP).toContain('quickFilters'); diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index aa05200cc4..51a823a8ab 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -2608,7 +2608,8 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an * lint and to the tests — `viewDef` is `Record`, so * nothing REQUIRES a key to be written. That silence shipped the same * defect three times (objectui#7199 `description`, objectui#7218 - * `rowColor`, objectui#7516 `fieldOrder`), and objectui#7559 ended it: + * `rowColor`, objectui#7516 `fieldOrder` — all three carry their rung + * below now), and objectui#7559 ended it: * `ObjectView.relayRungCensus-7559.test.ts` re-derives the member set * from the zod mirror at test time and requires every member to have * either a rung here or a DECLARED absence with a reason. @@ -2684,6 +2685,23 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an return resolved; })(), hiddenFields: (viewDef as any).hiddenFields ?? listSchema.hiddenFields, + /** + * The per-view ORDERING of the field composition + * `columns` x `hiddenFields` x `fieldOrder` (objectui#7516) — + * objectstack#15184 ruling B kept the key and wrote the + * composition into the contract: `columns` projects, + * `hiddenFields` (the rung above) subtracts, `fieldOrder` sorts + * what survives, an unlisted survivor sorting last. `ListView`'s + * `effectiveFields` memo runs those steps on `schema.fieldOrder`; + * this rung only delivers the view's value into that slot, the + * one the list-node spelling fills, so the two compose alike. + * + * View over list node — the precedence of the `hiddenFields` rung + * above; a view that authors no order keeps the list node's. It + * was the one per-view half of the composition with no rung: + * authored, served, then dropped here. + */ + fieldOrder: viewDef.fieldOrder ?? listSchema.fieldOrder, columnState: (viewDef as any).columnState ?? (listSchema as any).columnState, onDensityChange: (mode) => { // Persist the spec-canonical `rowHeight` (#2890). Writing the