diff --git a/.changeset/5795-related-list-inherit-list-view-sort.md b/.changeset/5795-related-list-inherit-list-view-sort.md new file mode 100644 index 0000000000..ba4b8789e3 --- /dev/null +++ b/.changeset/5795-related-list-inherit-list-view-sort.md @@ -0,0 +1,40 @@ +--- +'@object-ui/app-shell': minor +'@object-ui/plugin-detail': minor +--- + +An auto-derived related list now orders its rows by the CHILD object's default list view +`sort`, instead of falling to the server's primary-key order (objectui#5795). A task +version's "check items" tab whose child object declares `sort: [{ field: 'seq_no' }]` +renders 10/20/30/40; before this it rendered whatever order the ids happened to give — +20/30/10/40 in the reported case — while the child object's own list page obeyed the +declaration. + +**Declared as user-visible, deliberately, even though no key was added.** The contract +question ("where does a derived related list's sort declaration live?") was ruled on +objectstack#11345 (maintainer, 2026-08-23) as **direction 1**: inherit the child's list +view sort, and add **no** new spec key — the field-level `relatedListSort` the issue also +proposed was explicitly not approved. So there is nothing new to author, and +`record:related_list.sort` was already declared, parsed and consumed; this fills it. What a +host observes is nonetheless new: a derived related-list descriptor gains a populated +`sort` where it had none, and the query it issues gains an `$orderby`. An app whose child +objects declare a default list order will see those tabs re-order on upgrade — which is the +point of the change, and is why this is not a patch. + +Nothing is inherited where nothing was declared: a child object with no default list-view +sort produces the same descriptor, the same node and the same `$orderby`-free query as +before. + +The two `sort` surfaces declare the same union and mean different things by its string arm +— a `ListView` string is the legacy space-separated `'seq_no desc'`, while the related +list's own reader takes `'field'` / `'-field'` — so the inherited value is normalized to +the array arm once, at the derivation, through `@object-ui/core`'s +`convertSortToQueryParams` (the repo's single definition of both authored dialects). An +un-normalized inherit would have ordered by a field literally named `seq_no desc`. + +Known and unchanged: `$orderby` is only assembled while the related list is in windowed +(server-paged) mode, so a declared *or* inherited sort still disappears while the built-in +client text filter is active. That hole pre-dates this change and affects the authored prop +identically; it is now pinned as a recorded fact in +`plugin-detail/src/__tests__/RelatedList.sortDroppedOutsideWindowed.test.tsx` rather than +fixed here. diff --git a/packages/app-shell/src/utils/__tests__/deriveRelatedLists.inheritSort.test.ts b/packages/app-shell/src/utils/__tests__/deriveRelatedLists.inheritSort.test.ts new file mode 100644 index 0000000000..80c1ed9c36 --- /dev/null +++ b/packages/app-shell/src/utils/__tests__/deriveRelatedLists.inheritSort.test.ts @@ -0,0 +1,167 @@ +/** + * 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#5795 — a derived related list inherits the CHILD object's default + * list view `sort`. + * + * Ruled on objectstack#11345 (maintainer, 2026-08-23 15:02Z) as **direction + * 1**: inherit the child's list-view sort, and add **no** new spec key — the + * field-level `relatedListSort` the issue also offered was explicitly not + * approved. So there is nothing new to author: the ordering a child object + * already declares for its own list is the ordering its related lists use, + * exactly as `columns` already defaults to that list's columns. + * + * ## The dialect trap this file exists to pin + * + * `ListView.sort` and `record:related_list.sort` declare the SAME union + * (`string | Array<{field, order}>`) and mean DIFFERENT things by the string + * arm: + * + * - ListView's string is the legacy space-separated clause, `'seq_no desc'` + * (`@objectstack/spec` `ui/view.zod.ts`, annotated `Legacy "field desc"`); + * - the related list's own `normalizeSortSpec` (`plugin-detail/RelatedList + * .tsx`) reads the OData-ish `'field'` / `'-field'`. + * + * Inheriting the string verbatim therefore does not produce "a sort in another + * notation" — it produces `$orderby` on a field whose NAME is the seven + * characters `seq_no desc`, which no object has. The route taken (and pinned + * below) is to normalize at this boundary, always to the ARRAY arm, through + * `@object-ui/core`'s `convertSortToQueryParams` — the repo's one definition of + * both authored dialects — so no second parser of the legacy string exists to + * drift from it. + * + * `deriveRelatedLists` is the ONE place that knows it is reading a ListView and + * writing a related list, which is why the translation belongs here and not as + * a tolerant reader on the consuming end (AGENTS.md #0.1). + */ + +import { describe, it, expect } from 'vitest'; +import { deriveRelatedLists } from '../deriveRelatedLists'; + +const PARENT = { name: 'task_version', label: 'Task Version', fields: {} }; + +/** + * Shaped after the issue's downstream case: a "task version" owns "check + * items" that carry an explicit `seq_no` (10/20/30/40), which rendered in + * record-id order because the derivation emitted no `sort` at all. + */ +const childWithList = (list: unknown) => ({ + name: 'check_item', + label: 'Check Item', + ...(list === undefined ? {} : { list }), + fields: { + seq_no: { type: 'number', label: 'Seq No' }, + task_version: { type: 'master_detail', reference_to: 'task_version', label: 'Task Version' }, + }, +}); + +const derive = (child: unknown) => + deriveRelatedLists(PARENT, [PARENT, child as any])[0]; + +describe('deriveRelatedLists — inherited default list-view sort (objectui#5795)', () => { + it('SUBJECT — inherits the array arm of the child list view sort', () => { + const entry = derive(childWithList({ sort: [{ field: 'seq_no', order: 'asc' }] })); + expect(entry.childObject).toBe('check_item'); + expect(entry.sort).toEqual([{ field: 'seq_no', order: 'asc' }]); + }); + + it('preserves a multi-key authored order, in the order authored', () => { + // Key order matters and survives the map round-trip inside the + // normalizer because every ObjectStack field name matches + // `^[a-z_][a-z0-9_]*$` — none is an integer-like key JS would hoist. + const entry = derive( + childWithList({ + sort: [ + { field: 'stage', order: 'desc' }, + { field: 'seq_no', order: 'asc' }, + ], + }), + ); + expect(entry.sort).toEqual([ + { field: 'stage', order: 'desc' }, + { field: 'seq_no', order: 'asc' }, + ]); + }); + + it('THE DIALECT PIN — normalizes the legacy space-separated string arm', () => { + const entry = derive(childWithList({ sort: 'seq_no desc' })); + expect(entry.sort).toEqual([{ field: 'seq_no', order: 'desc' }]); + // Stated as its own assertion because it is the whole failure mode: an + // un-normalized inherit yields a FIELD literally named `seq_no desc`. + expect(entry.sort?.[0].field).toBe('seq_no'); + expect(entry.sort?.[0].field).not.toBe('seq_no desc'); + }); + + it('reads a bare legacy string as ascending', () => { + expect(derive(childWithList({ sort: 'seq_no' })).sort).toEqual([ + { field: 'seq_no', order: 'asc' }, + ]); + }); + + it('is case-insensitive about the legacy direction word', () => { + expect(derive(childWithList({ sort: 'seq_no DESC' })).sort).toEqual([ + { field: 'seq_no', order: 'desc' }, + ]); + }); + + it('COUNTER-PROBE — a child with no list-view sort gains no `sort` key at all', () => { + // Not `[]`, not `undefined`-valued: the key is ABSENT, so the synthesized + // node stays byte-identical to what it was before this inheritance + // existed. Were it present-and-empty, "inherited nothing" and "inherited + // an order" would be indistinguishable downstream — and inheritance would + // be satisfiable by inventing an order. + for (const list of [undefined, {}, { sort: undefined }, { sort: '' }, { sort: [] }]) { + const entry = derive(childWithList(list)); + expect(entry.childObject).toBe('check_item'); + expect('sort' in entry).toBe(false); + } + }); + + it('COUNTER-PROBE — an unusable `sort` is dropped, never guessed at', () => { + for (const sort of [42, { field: 'seq_no' }, ['seq_no'], [{ order: 'desc' }], null]) { + expect('sort' in derive(childWithList({ sort }))).toBe(false); + } + }); + + it('applies the same inherited order to EVERY related list of that child', () => { + // A child may point at one parent through several FKs; each surfaces as + // its own list, and all of them list the same object, so all inherit the + // same declared order. + const child = { + name: 'check_item', + label: 'Check Item', + list: { sort: [{ field: 'seq_no', order: 'asc' }] }, + fields: { + seq_no: { type: 'number', label: 'Seq No' }, + owner_version: { type: 'master_detail', reference_to: 'task_version', label: 'Owner' }, + review_version: { type: 'lookup', reference_to: 'task_version', label: 'Reviewer' }, + }, + }; + const entries = deriveRelatedLists(PARENT, [PARENT, child as any]); + expect(entries).toHaveLength(2); + for (const e of entries) { + expect(e.sort).toEqual([{ field: 'seq_no', order: 'asc' }]); + } + }); + + it('leaves an unrelated child list untouched when only one child declares a sort', () => { + const sorted = childWithList({ sort: [{ field: 'seq_no', order: 'asc' }] }); + const unsorted = { + name: 'attachment_note', + label: 'Note', + fields: { + task_version: { type: 'lookup', reference_to: 'task_version', label: 'Task Version' }, + }, + }; + const entries = deriveRelatedLists(PARENT, [PARENT, sorted as any, unsorted as any]); + const byObject = Object.fromEntries(entries.map((e) => [e.childObject, e])); + expect(byObject.check_item.sort).toEqual([{ field: 'seq_no', order: 'asc' }]); + expect('sort' in byObject.attachment_note).toBe(false); + }); +}); diff --git a/packages/app-shell/src/utils/deriveRelatedLists.ts b/packages/app-shell/src/utils/deriveRelatedLists.ts index fb7366757a..cb43404c9f 100644 --- a/packages/app-shell/src/utils/deriveRelatedLists.ts +++ b/packages/app-shell/src/utils/deriveRelatedLists.ts @@ -21,6 +21,13 @@ * - `relatedListTitle` / `relatedListColumns` on the FK field override the * derived title / columns (columns default to the child object's own list * columns when omitted — resolved by the renderer). + * - ROW ORDER is inherited from the child object's DEFAULT LIST VIEW `sort` + * (objectui#5795). There is deliberately no field-level `relatedListSort` + * to pair with the keys above: the contract question was ruled on + * objectstack#11345 (maintainer, 2026-08-23) as direction 1 — inherit the + * child's list-view sort, and add NO new spec key. A related list is just + * another surface that lists that object, so it orders the way that + * object's own list orders, exactly as `columns` already defaults. * - Audit FKs (`created_by` / `updated_by` / `owner_id`) are skipped — they * exist on virtually every object and would balloon the detail page into * dozens of duplicate cards. @@ -42,6 +49,8 @@ * server-side; this closes the UI/DX gap. */ +import { convertSortToQueryParams } from '@object-ui/core'; + /** Audit/ownership FKs that exist on nearly every object — never related lists. */ const AUDIT_FK_FIELDS = new Set(['created_by', 'updated_by', 'owner_id']); @@ -64,12 +73,65 @@ export interface DerivedRelatedList { * while non-primary lists collapse into a single "Related" tab. */ isPrimary: boolean; + /** + * Default row order, INHERITED from the child object's default list view + * `sort` (objectui#5795; ruled on objectstack#11345, maintainer 2026-08-23: + * direction 1 — inherit the child list view's sort, NO new spec key). + * + * Always the ARRAY arm of the `record:related_list.sort` union, never the + * string arm. Both surfaces declare `string | Array<{field, order}>`, but + * the two string arms are DIFFERENT dialects: a ListView string is the + * legacy space-separated `'seq_no desc'`, while the related list's own + * `normalizeSortSpec` reads `'field'` / `'-field'`. Passing the ListView + * string through verbatim would order by a field literally named + * `"seq_no desc"`. Translating at this boundary — the one place that knows + * it is reading a ListView and writing a related list — is the whole point; + * a tolerant reader on the consuming end would be the wrong fix (#0.1). + * + * Absent (never `[]`) when the child declares no default list view sort, so + * the related list's query stays byte-identical to what it sent before. + */ + sort?: Array<{ field: string; order: 'asc' | 'desc' }>; } interface ObjectLike { name?: string; label?: string; fields?: Record | any[]; + /** + * The object's DEFAULT list view, as merged onto the object def by + * `MetadataProvider.mergeViewsIntoObjects` (`merged.list = extra.primary`, + * where `primary` is the expanded view item flagged `isDefault`). Its `sort` + * is what a derived related list inherits. + * + * Optional on purpose: metadata of type `view` may arrive AFTER the objects + * do, in which case this is undefined on the first derivation pass and the + * descriptor carries no `sort`. That is not a silent hole — `objects` is a + * fresh array once the views merge, so the memo over this derivation + * recomputes and the sort appears. + */ + list?: { sort?: string | Array<{ field?: string; order?: 'asc' | 'desc' }> }; +} + +/** + * The child object's inherited default row order, normalized to the ARRAY arm. + * + * Both arms are lowered through `convertSortToQueryParams` — the repo's ONE + * definition of the authored-`sort` dialects (`@object-ui/core`) — so no second + * parser of the legacy `'field desc'` string can drift from it. Its return is a + * field→direction map; re-expanding it preserves the authored key order because + * every ObjectStack field name matches `^[a-z_][a-z0-9_]*$` (spec + * `field.zod.ts`), so none is an integer-like key that JS would hoist. + * + * Returns `undefined` — never `[]` — when nothing orderable was declared. + */ +function inheritedListViewSort( + list: ObjectLike['list'], +): Array<{ field: string; order: 'asc' | 'desc' }> | undefined { + const map = convertSortToQueryParams(list?.sort as any); + if (!map) return undefined; + const entries = Object.entries(map).map(([field, order]) => ({ field, order })); + return entries.length > 0 ? entries : undefined; } /** Normalize an object's `fields` (record or array) into `[name, def]` pairs. */ @@ -121,6 +183,10 @@ export function deriveRelatedLists( // it requires read access on the child — the FK's mere existence does not // grant the current user anything (objectui#2359). if (canRead && !canRead(child.name)) continue; + // objectui#5795: every related list derived from this child inherits the + // child's own default list-view order, so compute it once per child + // rather than once per FK. + const inheritedSort = inheritedListViewSort(child.list); for (const [fieldName, fieldDef] of fieldEntries(child.fields)) { if (!fieldDef) continue; const type = fieldDef.type; @@ -136,6 +202,7 @@ export function deriveRelatedLists( referenceField: fieldName, isOwned: type === 'master_detail', isPrimary: fieldDef.relatedList === 'primary', + ...(inheritedSort ? { sort: inheritedSort } : {}), _fkLabel: (typeof fieldDef.label === 'string' && fieldDef.label) || fieldName, ...(typeof fieldDef.relatedListTitle === 'string' && fieldDef.relatedListTitle ? { title: fieldDef.relatedListTitle } diff --git a/packages/app-shell/src/views/RecordDetailView.relatedListInheritedSort-5795.test.tsx b/packages/app-shell/src/views/RecordDetailView.relatedListInheritedSort-5795.test.tsx new file mode 100644 index 0000000000..f8b060bc22 --- /dev/null +++ b/packages/app-shell/src/views/RecordDetailView.relatedListInheritedSort-5795.test.tsx @@ -0,0 +1,274 @@ +/** + * 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#5795 — the `$orderby` a derived related list puts ON THE WIRE. + * + * Ruled on objectstack#11345 (maintainer, 2026-08-23 15:02Z), direction 1: + * derived related lists inherit the child object's default list view `sort`, + * with NO new spec key. The user-visible bug this closes: a task version's + * "check items" tab rendered 20/30/10/40 (the server's primary-key fallback) + * while the child object's own list, sorted by `seq_no`, rendered 10/20/30/40. + * + * ## Why this file renders the whole page instead of asserting the descriptor + * + * The descriptor `deriveRelatedLists` emits is THREE hops from the wire, and + * two of those hops re-drop it onto a fresh object literal that names each key + * it carries forward: + * + * 1. `RecordDetailView` maps the descriptor into `buildDefaultPageSchema`'s + * `related` shape (a fresh literal — a key it does not name is gone); + * 2. `buildDefaultTabs`' `relatedNode` maps THAT into the `record:related_list` + * component node (a second fresh literal, same property); + * 3. `RecordRelatedListRenderer` hands `schema.sort` to `RelatedList` as + * `defaultSort`, which `normalizeSortSpec` lowers into `$orderby`. + * + * An assertion on the descriptor is green while any of those three drops the + * key, which is exactly the shape the defect had — so the subject here is the + * argument `dataSource.find` is actually called with, and the harness is the + * real page over a fake backend. + * + * ## What each leg decides + * + * - SUBJECT: the declared order reaches `$orderby`. + * - DIALECT: a child declaring the LEGACY string form (`'seq_no desc'`, the + * space-separated `ListView` arm) still reaches the wire as a real field + * name. Un-normalized, this leg is the one that goes red — `$orderby` would + * name a field literally called `seq_no desc`. `record:related_list.sort` + * declares a string arm too, but it means `'field'`/`'-field'`, so the two + * string arms are NOT interchangeable and the inherit must translate. + * - COUNTER-PROBE: a child with NO list-view sort must still produce a + * WORKING related list that sends NO `$orderby`. Without it, "inheritance" + * would be satisfiable by inventing an order for everyone. + * + * ## The hole this inheritance inherits (attached deliberately, not fixed) + * + * `$orderby` is assembled inside `RelatedList`'s `windowed` branch only, and + * `windowed` goes false while the client text filter is active — so a declared + * `record:related_list.sort` is DROPPED whenever the list leaves windowed + * mode, and the client path returns rows unsorted unless a column was clicked. + * That is pre-existing on the authored prop and the inherited sort inherits it + * identically. It is pinned as a recorded fact next door, in + * `plugin-detail/src/__tests__/RelatedList.sortDroppedOutsideWindowed.test.tsx` + * — `RelatedList.tsx` is out of scope for this card, so the pin records the + * behaviour rather than changing it. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MetadataCtx } from '@object-ui/react'; + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', () => ({ + useRecordPresence: () => ({ viewers: [], others: [] }), + PresenceAvatars: () => null, +})); + +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(), + }), +})); + +// Orthogonal chrome — stubbed so the only asynchrony in this file is the +// related list's own fetch. +vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null })); +vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null })); +vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null })); +vi.mock('./FlowRunner', () => ({ FlowRunner: () => null })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); + +import { RecordDetailView } from './RecordDetailView'; + +const PARENT = 'task_version'; +const CHILD = 'check_item'; +const RECORD_ID = 'tv-1'; + +/** The parent object — a plain record page with one owned child collection. */ +const parentObject = { + name: PARENT, + label: 'Task Version', + managedBy: 'platform', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + }, +}; + +/** + * The child object. `list` is the DEFAULT list view as `MetadataProvider` + * merges it onto the object def (`merged.list = extra.primary`, where + * `primary` is the expanded view item flagged `isDefault`) — i.e. this is the + * post-merge shape the page really receives, which is what makes the read at + * derivation time non-empty. + */ +const childObject = (list?: unknown) => ({ + name: CHILD, + label: 'Check Item', + managedBy: 'platform', + ...(list === undefined ? {} : { list }), + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + seq_no: { type: 'number', label: 'Seq No' }, + [PARENT]: { type: 'master_detail', reference_to: PARENT, label: 'Task Version' }, + }, +}); + +/** Rows deliberately seeded OUT of `seq_no` order, as the issue's repro is. */ +const CHILD_ROWS = [ + { id: 'ci-b', name: 'Item B', seq_no: 20, [PARENT]: RECORD_ID }, + { id: 'ci-c', name: 'Item C', seq_no: 30, [PARENT]: RECORD_ID }, + { id: 'ci-a', name: 'Item A', seq_no: 10, [PARENT]: RECORD_ID }, + { id: 'ci-d', name: 'Item D', seq_no: 40, [PARENT]: RECORD_ID }, +]; + +function makeDataSource() { + return { + find: vi.fn(async (objectName: string) => ({ + data: objectName === CHILD ? CHILD_ROWS : [], + total: objectName === CHILD ? CHILD_ROWS.length : 0, + })), + create: vi.fn(async (_o: string, row: any) => row), + findOne: vi.fn(async (_o: string, recordId: string) => ({ + id: recordId, + name: `Version ${recordId}`, + })), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as any; +} + +function renderPage(objects: any[], dataSource: any) { + const metadata = { + objects, + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + } as any; + return render( + + + {}} + objectNameOverride={PARENT} + recordIdOverride={RECORD_ID} + embedded + /> + + , + ); +} + +/** + * Render the page with the Related tab already open and return the params the + * related list's own fetch went out with. + * + * The tab is selected through the URL (`?tab=related` — tabs are + * URL-addressable by a stable semantic value, ADR-0054 C3) rather than by + * clicking, so the list is mounted by the page's own routing rather than by a + * synthetic event this test would then also be asserting about. + * + * The child object is queried TWICE on this page and only one of the two is + * the subject: the tab strip auto-derives its count badge with a + * `$top: 1, $count: true` probe that carries no ordering by design. Picking + * the windowed page fetch explicitly keeps a counter-probe from passing + * because it read the count probe instead. + */ +const isListFetch = (params: any) => !params?.$count && typeof params?.$top === 'number'; + +async function childQueryParams(list?: unknown) { + const ds = makeDataSource(); + renderPage([parentObject, childObject(list)], ds); + // Fails loudly if the list never fetched, rather than returning "no + // `$orderby`" — which is what every counter-probe here would read as a pass. + await waitFor(() => { + expect( + ds.find.mock.calls.some((c: any[]) => c[0] === CHILD && isListFetch(c[1])), + ).toBe(true); + }); + const call = ds.find.mock.calls.find((c: any[]) => c[0] === CHILD && isListFetch(c[1]))!; + return call[1] as Record; +} + +beforeEach(() => { + cleanup(); + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('derived related list — inherited $orderby on the wire (objectui#5795)', () => { + it('SUBJECT — the child list view sort reaches $orderby', async () => { + const params = await childQueryParams({ sort: [{ field: 'seq_no', order: 'asc' }] }); + // The parent scope is the live control: it proves the query under + // inspection is the related list's own, not some other read. + expect(params.$filter).toEqual({ [PARENT]: RECORD_ID }); + expect(params.$orderby).toEqual([{ field: 'seq_no', order: 'asc' }]); + }); + + it('SUBJECT — a descending declared order arrives descending', async () => { + const params = await childQueryParams({ sort: [{ field: 'seq_no', order: 'desc' }] }); + expect(params.$orderby).toEqual([{ field: 'seq_no', order: 'desc' }]); + }); + + it('DIALECT — the legacy space-separated string arm reaches the wire normalized', async () => { + const params = await childQueryParams({ sort: 'seq_no desc' }); + expect(params.$orderby).toEqual([{ field: 'seq_no', order: 'desc' }]); + // The failure this leg exists for, stated so a regression reads plainly: + // an un-normalized inherit orders by a FIELD NAMED `seq_no desc`. + expect(params.$orderby[0].field).toBe('seq_no'); + expect(JSON.stringify(params.$orderby)).not.toContain('seq_no desc'); + }); + + it('COUNTER-PROBE — no declared sort sends NO $orderby, and the list still works', async () => { + const params = await childQueryParams(undefined); + expect('$orderby' in params).toBe(false); + // Live control: the list is a real, scoped, windowed query — so the + // missing `$orderby` above means "nothing was inherited", not "nothing + // was fetched". + expect(params.$filter).toEqual({ [PARENT]: RECORD_ID }); + expect(params.$top).toBeGreaterThan(0); + }); + + it('COUNTER-PROBE — a list view with an empty sort is the same as none', async () => { + expect('$orderby' in (await childQueryParams({ sort: [] }))).toBe(false); + }); +}); diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 15a99fe948..594047191a 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -1963,7 +1963,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // its tab is shown (nothing is preloaded here), and header/row // affordances (+ New / View All / row navigation) are wired // downstream by `RelatedRecordActionsBridge`. - const related = childRelations.map(({ childObject, childLabel, referenceField, title: titleOverride, columns: columnsOverride, isPrimary }) => { + const related = childRelations.map(({ childObject, childLabel, referenceField, title: titleOverride, columns: columnsOverride, isPrimary, sort: inheritedSort }) => { const childObjectDef = objects.find((o: any) => o.name === childObject); // A `relatedListTitle` on the relationship wins; else fall back to the // localized child-object label. @@ -1981,6 +1981,14 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri ...(Array.isArray(columnsOverride) && columnsOverride.length > 0 ? { columns: columnsOverride } : {}), + // Row order inherited from the child object's default list view + // (objectui#5795, ruled direction 1 on objectstack#11345). Already + // normalized to the array arm by `deriveRelatedLists` — the ListView + // legacy string dialect (`'seq_no desc'`) is NOT the one the related + // list's `normalizeSortSpec` reads, so it must never travel verbatim. + // Absent when the child declares no list-view sort, keeping the + // synthesized node byte-identical to what it was before. + ...(inheritedSort && inheritedSort.length > 0 ? { sort: inheritedSort } : {}), ...(childObjectDef?.icon ? { icon: childObjectDef.icon } : {}), // `relatedList: 'primary'` prominence flag (ADR-0085) — the list is // promoted to its own tab by `buildDefaultTabs`. diff --git a/packages/plugin-detail/src/__tests__/RelatedList.sortDroppedOutsideWindowed.test.tsx b/packages/plugin-detail/src/__tests__/RelatedList.sortDroppedOutsideWindowed.test.tsx new file mode 100644 index 0000000000..fe4e76fa0f --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RelatedList.sortDroppedOutsideWindowed.test.tsx @@ -0,0 +1,167 @@ +/** + * 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. + */ + +/** + * A declared related-list `sort` is DROPPED while the client text filter is + * active — recorded, not fixed (attached to objectui#5795). + * + * `$orderby` is assembled inside `RelatedList`'s WINDOWED branch only, and + * `windowed` is false whenever `filterActive` is true: the built-in + * contains-filter is a client-side sweep over every field, inexpressible as a + * server filter, so the component falls back to fetching the whole collection. + * On that path the rows are returned in the order the server chose (primary + * key), because client-side sorting only runs when the user has clicked a + * column (`sortField`), and a declared `sort` never sets `sortField`. + * + * So a list ordered by `seq_no` reverts to id order the moment someone types a + * letter into its filter box, and returns to `seq_no` order when they clear it. + * + * ## Why this file exists here, on this card + * + * objectui#5795 makes derived related lists INHERIT the child object's default + * list view `sort`. That inheritance lands on exactly this prop, so it lands on + * exactly this hole — and the hole becomes reachable without anyone authoring + * anything, on every derived list whose child object declares a list-view + * order. Fixing it means changing `RelatedList`'s fetch/sort split, which is + * out of scope for that card (`RelatedList.tsx` is not its file surface). + * + * Recording it is the part that is in scope: pinned, the disappearance is a + * known, deliberate, dated fact with a test that will go red when someone + * closes it — instead of a surprise found by a user whose sorted tab + * unsorts itself mid-search. + * + * ⚠️ This file asserts TODAY'S behaviour. When the hole is closed, these + * assertions SHOULD go red — the fix is to rewrite them to the new contract, + * not to delete the file. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor, screen, fireEvent } from '@testing-library/react'; +import * as React from 'react'; +import { RelatedList } from '../RelatedList'; + +// Capture the schema RelatedList hands to SchemaRenderer (the data-table), so +// the rows it renders can be read without the table in the way. +const h = vi.hoisted(() => ({ schema: null as any })); +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + SchemaRenderer: (props: any) => { + h.schema = props.schema; + return null; + }, + }; +}); + +/** Rows in the server's primary-key order, deliberately NOT in `seq_no` order. */ +const PK_ORDER = [ + { id: 'ci-b', name: 'Item B', seq_no: 20 }, + { id: 'ci-c', name: 'Item C', seq_no: 30 }, + { id: 'ci-a', name: 'Item A', seq_no: 10 }, + { id: 'ci-d', name: 'Item D', seq_no: 40 }, +]; + +const columns = [ + { accessorKey: 'name', header: 'Name' }, + { accessorKey: 'seq_no', header: 'Seq No' }, +]; + +/** + * A backend that HONOURS `$orderby` — so an unordered result in these tests + * means the query carried no ordering, not that the fake ignored one. + */ +const makeDS = () => ({ + find: vi.fn(async (_api: string, params: any) => { + const rows = [...PK_ORDER]; + const orderby = params?.$orderby; + if (Array.isArray(orderby) && orderby.length > 0) { + const { field, order } = orderby[0]; + rows.sort((a: any, b: any) => (a[field] - b[field]) * (order === 'desc' ? -1 : 1)); + } + const skip = params?.$skip ?? 0; + const top = params?.$top ?? rows.length; + return { data: rows.slice(skip, skip + top), total: rows.length }; + }), +}); + +const DECLARED_SORT = [{ field: 'seq_no', order: 'asc' as const }]; + +function renderList(ds: any) { + return render( + , + ); +} + +const seqOrder = () => (h.schema?.data ?? []).map((r: any) => r.seq_no); +const lastParams = (ds: any) => ds.find.mock.calls[ds.find.mock.calls.length - 1][1]; + +beforeEach(() => { + h.schema = null; +}); + +describe('RelatedList — a declared sort outside windowed mode (attached to objectui#5795)', () => { + it('CONTROL — windowed, the declared sort goes out as $orderby and rows arrive ordered', async () => { + const ds = makeDS(); + renderList(ds); + await waitFor(() => expect(h.schema?.data?.length).toBe(4)); + expect(lastParams(ds).$orderby).toEqual(DECLARED_SORT); + expect(seqOrder()).toEqual([10, 20, 30, 40]); + }); + + it('RECORDED HOLE — typing in the client filter drops $orderby from the query', async () => { + const ds = makeDS(); + renderList(ds); + await waitFor(() => expect(h.schema?.data?.length).toBe(4)); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Item' } }); + + // The refetch leaves windowed mode: no `$top`/`$skip` window, and with it + // no `$orderby` — the declared order is not expressed anywhere on the wire. + await waitFor(() => { + expect(lastParams(ds).$top).toBeUndefined(); + }); + expect(lastParams(ds).$orderby).toBeUndefined(); + // Live control: the query is still the scoped related-list query. + expect(lastParams(ds).$filter).toEqual({ task_version: 'tv-1' }); + }); + + it('RECORDED HOLE — and the rows the user sees revert to the server order', async () => { + const ds = makeDS(); + renderList(ds); + await waitFor(() => expect(seqOrder()).toEqual([10, 20, 30, 40])); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Item' } }); + + // Every row still matches the filter, so this is purely the ordering + // changing under the user: 10/20/30/40 becomes the primary-key order. + await waitFor(() => expect(seqOrder()).toEqual([20, 30, 10, 40])); + }); + + it('the order comes back when the filter is cleared', async () => { + const ds = makeDS(); + renderList(ds); + await waitFor(() => expect(seqOrder()).toEqual([10, 20, 30, 40])); + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Item' } }); + await waitFor(() => expect(seqOrder()).toEqual([20, 30, 10, 40])); + fireEvent.change(screen.getByRole('textbox'), { target: { value: '' } }); + await waitFor(() => expect(seqOrder()).toEqual([10, 20, 30, 40])); + }); +}); diff --git a/packages/plugin-detail/src/synth/__tests__/buildDefaultPageSchema.relatedSort.test.ts b/packages/plugin-detail/src/synth/__tests__/buildDefaultPageSchema.relatedSort.test.ts new file mode 100644 index 0000000000..10aa1b8235 --- /dev/null +++ b/packages/plugin-detail/src/synth/__tests__/buildDefaultPageSchema.relatedSort.test.ts @@ -0,0 +1,178 @@ +/** + * 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#5795 — the synthesizer carries a related list's `sort` through to + * the `record:related_list` node, and NOWHERE else. + * + * `buildDefaultTabs`' `relatedNode` builds a FRESH object literal per related + * entry, naming every key it forwards; a key it does not name is simply gone, + * which is the shape the original defect had one hop upstream. This is the + * second of the two re-drop sites between the derivation and the wire, so it + * gets its own pins rather than relying on the end-to-end test alone. + * + * ## The synthesizer is a CARRIER here, not a policy + * + * It neither derives an order nor overrides one: `RecordDetailView` decides + * what a derived list inherits (the child object's default list view `sort` — + * ruled direction 1 on objectstack#11345, maintainer 2026-08-23, with **no new + * spec key**) and hands it over already lowered to the array arm. So the pins + * below are about faithful carriage and about the key being ABSENT when + * nothing was supplied — never about the value's content. + * + * ## Precedence, stated because the card asked for it to be + * + * There is no contest to resolve. A hand-authored page carries its own + * `record:related_list` node with its own `sort` and never enters this + * synthesizer at all (`RecordDetailView` synthesizes only when no page is + * assigned), and within the synthesized path the ONE producer of + * `related[].sort` is the inheritance. An authored sort therefore behaves + * exactly as it did before this change — which is what the last leg records. + * + * ## The third site that must NOT gain the key + * + * The same `related[]` array also feeds `record:reference_rail`'s entries. + * `ReferenceRailEntrySchema` is `$strict` and declares no `sort`, so emitting + * one there would be refused at save with nothing reading it — exactly the + * class objectui#5494 removed when it stopped emitting `rel.icon` onto rail + * entries. The rail is a top-3 summary, not the list; it is deliberately left + * unordered by this card. + */ + +import { describe, it, expect } from 'vitest'; +import { PageSchema } from '@objectstack/spec/ui'; +import { + buildDefaultPageSchema, + buildDefaultTabs, + type ObjectDefLike, +} from '../buildDefaultPageSchema'; + +const checkItemDef: ObjectDefLike = { + name: 'task_version', + label: 'Task Version', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + }, +}; + +const SORT = [{ field: 'seq_no', order: 'asc' as const }]; + +/** Every `record:related_list` node in a synthesized tabs node. */ +function relatedNodes(tabs: any): any[] { + const out: any[] = []; + const walk = (n: any) => { + if (!n || typeof n !== 'object') return; + if (n.type === 'record:related_list') out.push(n); + for (const c of n.children ?? []) walk(c); + }; + for (const item of tabs.properties?.items ?? []) for (const c of item.children ?? []) walk(c); + return out; +} + +const propsOf = (node: any) => node.properties ?? node; + +function issuesOf(result: { success: boolean; error?: { issues: any[] } }): string[] { + if (result.success) return []; + return (result.error?.issues ?? []).map( + (i: any) => `${i.code} @ ${i.path.join('.') || ''}: ${String(i.message).split('\n')[0]}`, + ); +} + +describe('buildDefaultTabs — related-list sort carriage (objectui#5795)', () => { + it('SUBJECT — forwards a supplied array-arm sort onto the node', () => { + const tabs = buildDefaultTabs(checkItemDef, { + related: [{ objectName: 'check_item', relationshipField: 'task_version', sort: SORT }], + }); + const nodes = relatedNodes(tabs); + expect(nodes).toHaveLength(1); + expect(propsOf(nodes[0]).sort).toEqual(SORT); + }); + + it('forwards the value VERBATIM — the synthesizer is not a second translator', () => { + // The spec union's string arm, in the related list's own `'-field'` + // notation. The one translation this feature needs (from the ListView's + // legacy `'field desc'` dialect) happens once, at the derivation; a second + // conversion here is how two dialects start disagreeing. + const tabs = buildDefaultTabs(checkItemDef, { + related: [{ objectName: 'check_item', relationshipField: 'task_version', sort: '-seq_no' }], + }); + expect(propsOf(relatedNodes(tabs)[0]).sort).toBe('-seq_no'); + }); + + it('COUNTER-PROBE — omits the key entirely when nothing was supplied', () => { + const tabs = buildDefaultTabs(checkItemDef, { + related: [{ objectName: 'check_item', relationshipField: 'task_version' }], + }); + const props = propsOf(relatedNodes(tabs)[0]); + // Absent, not present-and-undefined: "the author said nothing" and "the + // author asked for the default" are different facts, and the second is the + // one a later liveness audit would read. + expect('sort' in props).toBe(false); + }); + + it('carries a per-list sort, not a page-wide one', () => { + const tabs = buildDefaultTabs(checkItemDef, { + relatedLayout: 'stack', + related: [ + { objectName: 'check_item', relationshipField: 'task_version', sort: SORT }, + { objectName: 'attachment_note', relationshipField: 'task_version' }, + ], + }); + const byObject = Object.fromEntries( + relatedNodes(tabs).map((n) => [propsOf(n).objectName, propsOf(n)]), + ); + expect(byObject.check_item.sort).toEqual(SORT); + expect('sort' in byObject.attachment_note).toBe(false); + }); + + it('the synthesized page still parses under the real PageSchema with a sort', () => { + // The ruling added NO spec key: `record:related_list.sort` was already + // declared-parsed-consumed, and this change only fills it. Measured + // against the vendored `@objectstack/spec` the server enforces (ADR-0089 + // D3a closed these nodes with `.strict()`), so an invented key would be a + // loud parse error here rather than a silent strip. + const synth = buildDefaultPageSchema(checkItemDef, { + related: [{ objectName: 'check_item', relationshipField: 'task_version', sort: SORT }], + }); + const body = { + name: 'task_version_record', + label: 'Task Version Record', + type: 'record', + object: checkItemDef.name, + ...(synth.template ? { template: synth.template } : {}), + ...(Array.isArray(synth.regions) && synth.regions.length ? { regions: synth.regions } : {}), + }; + expect(issuesOf(PageSchema.safeParse(body) as any)).toEqual([]); + }); + + it('THE THIRD SITE — reference-rail entries do NOT gain a sort', () => { + // The rail is opt-in and needs at least two related lists (it also + // suppresses the Related tab), so this is the branch where the SAME + // `related[]` array reaches the rail's own strict entry shape. + const synth = buildDefaultPageSchema(checkItemDef, { + showReferenceRail: true, + related: [ + { objectName: 'check_item', relationshipField: 'task_version', sort: SORT }, + { objectName: 'attachment_note', relationshipField: 'task_version', sort: SORT }, + ], + }); + const aside = (synth.regions as any[]).find((r) => r.name === 'aside'); + const rail = aside?.components?.find((c: any) => c.type === 'record:reference_rail'); + expect(rail).toBeTruthy(); + for (const entry of propsOf(rail).entries) { + expect('sort' in entry).toBe(false); + } + // Deliberately NOT a whole-page `PageSchema` parse here, unlike the leg + // above: a Reference Rail page is already unpersistable for an unrelated, + // pre-existing reason — the synthesized `aside` region carries a + // `className` that `PageRegionSchema` refuses (objectui#4286, open). + // Measured on this branch with NO `sort` anywhere, so it is not this + // change; asserting a clean parse here would fail on someone else's + // defect and asserting the failure would pin it in place. + }); +}); diff --git a/packages/plugin-detail/src/synth/buildDefaultPageSchema.ts b/packages/plugin-detail/src/synth/buildDefaultPageSchema.ts index 8a267660bc..83721a3c82 100644 --- a/packages/plugin-detail/src/synth/buildDefaultPageSchema.ts +++ b/packages/plugin-detail/src/synth/buildDefaultPageSchema.ts @@ -169,7 +169,7 @@ export interface BuildPageOptions { * * - `objectName` and `relationshipField` are required. * - `title` overrides the default child-object label. - * - `columns` / `limit` / `icon` are forwarded to the renderer. + * - `columns` / `limit` / `icon` / `sort` are forwarded to the renderer. */ related?: Array<{ title?: string; @@ -178,6 +178,19 @@ export interface BuildPageOptions { columns?: any[]; limit?: number; icon?: string; + /** + * Row order for this list — the spec `record:related_list.sort` union, + * forwarded verbatim onto the synthesized node. + * + * The host supplies it; this synthesizer neither derives nor overrides it. + * `RecordDetailView` fills it from the child object's DEFAULT LIST VIEW + * sort (objectui#5795 — ruled direction 1 on objectstack#11345, + * maintainer 2026-08-23: inherit the child list view's sort, NO new spec + * key), already lowered to the array arm. Omitted → the node carries no + * `sort` and the related list falls back to the server's PK order, exactly + * as before this key had a producer. + */ + sort?: string | Array<{ field: string; order: 'asc' | 'desc' }>; /** * `relatedList: 'primary'` — a CORE relationship. Under the default * layout this list is promoted to its OWN tab; non-primary lists collapse @@ -687,6 +700,11 @@ export function buildDefaultTabs( ...(rel.columns ? { columns: rel.columns } : {}), ...(rel.limit ? { limit: rel.limit } : {}), ...(rel.icon ? { icon: rel.icon } : {}), + // objectui#5795. Emitted only when the host actually supplied one, for + // the reason every sibling key here is: a `sort: undefined` written + // onto the node is a different fact from "the author said nothing", + // and it is the one a later liveness audit would read. + ...(rel.sort ? { sort: rel.sort } : {}), }); const asOwnTab = (rel: NonNullable[number]) => ({ label: rel.title || rel.objectName,