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