diff --git a/.changeset/12195-record-page-isdefault.md b/.changeset/12195-record-page-isdefault.md new file mode 100644 index 0000000000..13784880ab --- /dev/null +++ b/.changeset/12195-record-page-isdefault.md @@ -0,0 +1,11 @@ +--- +'@object-ui/react': patch +--- + +`usePageAssignment` opens an object's default record page: among the record pages served for the object, the one declared `isDefault: true` is returned, and declaration order decides only when none is (objectui#12195). + +The hook used to return the first matching record page in served order and never read `PageSchema.isDefault`, so the flag on a shipped page (the platform's `sys_user_detail`, `sys_organization_detail` and `sys_approval_request_detail`, and the showcase's record pages) did nothing. `@objectstack/spec` 18.0.0-next.1 states the rule this follows (maintainer ruling A on objectstack#23015): at most one default record page may exist for an object in a deployment, and a second is refused at `os validate`, at package registration and at metadata save, naming both pages. So the hook adds no tie-break of its own. It reads "the default record page of an object" through the spec's `defaultRecordPageObject`, the same reading those refusals use. + +**Who sees a different page.** An app or environment that authors its own record page for an object without `isDefault: true`, for an object that also has a served default record page, and whose page came first in the served list. That page used to open; the default opens now. Ruling A keeps two ways to replace a platform record page: an ADR-0005 overlay saved under that page's own name, and host composition (composing the stack without the platform's page). Shipping a second default is not one: the platform refuses it. With no default for an object, nothing changes: the first matching page still opens, and an explicit `pageName` still wins over the default. + +No type, option or export changes (`Clause-②: no`). diff --git a/packages/react/src/hooks/__tests__/usePageAssignment.declarationOrder-7298.test.tsx b/packages/react/src/hooks/__tests__/usePageAssignment.declarationOrder-7298.test.tsx index d5fca45e14..1902dbe5f6 100644 --- a/packages/react/src/hooks/__tests__/usePageAssignment.declarationOrder-7298.test.tsx +++ b/packages/react/src/hooks/__tests__/usePageAssignment.declarationOrder-7298.test.tsx @@ -16,6 +16,14 @@ * ordering rule is nailed down rather than implied — and the last block pins * the CONTRACT the removal rests on, so that if `priority` is ever declared * upstream (route A on that card) this file reds and points back here. + * + * Declaration order is the rule among pages that are NOT the object's default + * record page. Since objectui#12195 the declared `isDefault` decides first: + * maintainer ruling A on objectstack#23015 (record 6134056653) allows at most + * one default record page per object per deployment, and the renderer honours + * it with no tie-break. Three pins below used to say the flag was inert; they + * now pin the ruled behaviour, and `usePageAssignment.isDefault-12195.test.tsx` + * carries the rule's own pins and controls. */ import React from 'react'; @@ -81,7 +89,7 @@ const SYS_USER_DETAIL = { slots: { discussion: [] }, }; -describe('usePageAssignment — selection is by declaration order', () => { +describe('usePageAssignment — among pages that are not the default, selection is by declaration order', () => { it('returns the FIRST matching candidate, naming it', async () => { const result = await assign( [full('first_page', 'sys_user'), full('second_page', 'sys_user'), full('third_page', 'sys_user')], @@ -90,7 +98,7 @@ describe('usePageAssignment — selection is by declaration order', () => { expect(result.current.page?.name).toBe('first_page'); }); - it('reversing the declaration order reverses the winner — position is the whole rule', async () => { + it('reversing the declaration order reverses the winner — with no default, position is the whole rule', async () => { const a = full('alpha', 'sys_user'); const b = full('beta', 'sys_user'); @@ -98,15 +106,16 @@ describe('usePageAssignment — selection is by declaration order', () => { expect((await assign([b, a], 'sys_user')).current.page?.name).toBe('beta'); }); - it('`isDefault` on a later candidate does NOT promote it', async () => { + it('`isDefault` on a later candidate promotes it over an earlier page that is not the default (ruling A)', async () => { const result = await assign( [full('authored', 'sys_user'), full('marked_default', 'sys_user', { isDefault: true })], 'sys_user', ); - // Declared and writable, but this decision never reads it: inert while - // looking decisive. Pinned so a future reader does not mistake the flag - // for the thing that picks the page. - expect(result.current.page?.name).toBe('authored'); + // This pin used to read 'authored': the flag was declared and writable but + // nothing read it. Ruling A on objectstack#23015 (record 6134056653) makes + // the default record page the one that opens, so declaration order only + // decides among the pages that are not the default. + expect(result.current.page?.name).toBe('marked_default'); }); it('skips pages for other objects and other page types while ordering the rest', async () => { @@ -129,19 +138,25 @@ describe('usePageAssignment — selection is by declaration order', () => { describe('usePageAssignment — the collision every stock install has (objectui#7298)', () => { // `@objectstack/platform-objects` ships `sys_user_detail`, so ANY app that - // authors its own `sys_user` record page has exactly two candidates. + // authors its own `sys_user` record page has exactly two candidates. The + // platform's page is the object's default record page; the app's is not. - it('the app page wins when it is declared first', async () => { + it('the app page declared first no longer wins: the platform default does (ruling A)', async () => { const result = await assign( [full('app_user_detail', 'sys_user'), SYS_USER_DETAIL], 'sys_user', ); - expect(result.current.page?.name).toBe('app_user_detail'); - // The app page is `kind: 'full'`, so it populates `page`, never `slots`. - expect(result.current.slots).toBeNull(); + // This pin used to read 'app_user_detail': declaration order was the only + // override an app had. Ruling A on objectstack#23015 (record 6134056653) + // keeps two override paths, an ADR-0005 overlay by name and host + // composition; an earlier position in the served list is not one. + // `sys_user_detail` is `kind: 'slotted'`, so its win shows up as populated + // `slots` and a null `page`. + expect(result.current.page).toBeNull(); + expect(result.current.slots).toEqual({ discussion: [] }); }); - it('the platform page wins when IT is declared first — nothing authored breaks the tie', async () => { + it('the platform page wins when IT is declared first as well — the default wins from either position', async () => { const result = await assign( [SYS_USER_DETAIL, full('app_user_detail', 'sys_user')], 'sys_user', @@ -152,13 +167,23 @@ describe('usePageAssignment — the collision every stock install has (objectui# expect(result.current.slots).toEqual({ discussion: [] }); }); - it('`isDefault: true` on the platform page does not make it win from second place', async () => { - const result = await assign( - [full('app_user_detail', 'sys_user'), SYS_USER_DETAIL], + it('`isDefault: true` on the platform page makes it win from second place — the flag, not the position, decides', async () => { + const served = [full('app_user_detail', 'sys_user'), SYS_USER_DETAIL]; + expect(SYS_USER_DETAIL.isDefault).toBe(true); + // This pin used to read 'app_user_detail' (the flag was inert). Ruling A + // on objectstack#23015 (record 6134056653): the default opens. + const withFlag = await assign(served, 'sys_user'); + expect(withFlag.current.page).toBeNull(); + expect(withFlag.current.slots).toEqual({ discussion: [] }); + + // The same two pages in the same order, the platform page's flag the only + // change: declaration order decides again, so the flag is the variable. + const withoutFlag = await assign( + [full('app_user_detail', 'sys_user'), { ...SYS_USER_DETAIL, isDefault: false }], 'sys_user', ); - expect(SYS_USER_DETAIL.isDefault).toBe(true); - expect(result.current.page?.name).toBe('app_user_detail'); + expect(withoutFlag.current.page?.name).toBe('app_user_detail'); + expect(withoutFlag.current.slots).toBeNull(); }); }); diff --git a/packages/react/src/hooks/__tests__/usePageAssignment.isDefault-12195.test.tsx b/packages/react/src/hooks/__tests__/usePageAssignment.isDefault-12195.test.tsx new file mode 100644 index 0000000000..5687233f93 --- /dev/null +++ b/packages/react/src/hooks/__tests__/usePageAssignment.isDefault-12195.test.tsx @@ -0,0 +1,162 @@ +/** + * ObjectUI — usePageAssignment: the object's DEFAULT record page opens + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#12195. The record-page pick used to return the first matching page + * in served order and never read the declared `PageSchema.isDefault`, so the + * flag on a shipped page (the platform's `sys_user_detail`, for one) was inert + * while looking decisive. + * + * Maintainer ruling A on objectstack#23015 (record 6134056653) states the rule + * this hook now follows: at most one `isDefault: true` record page may exist for + * an object in a deployment, a second is refused at every door that can add one + * (naming both pages), and the renderer honours `isDefault` with no tie-break. + * Among pages that are not the default, declaration order stays the last + * resort (`usePageAssignment.declarationOrder-7298.test.tsx`). + * + * These pins assert WHICH PAGE is returned. Each rule pin has a control that + * holds the served list fixed and changes only the flag or only the order, so + * the flag is shown to be the variable. + */ + +import React from 'react'; +import { describe, it, expect } from 'vitest'; +import { renderHook, waitFor } from '@testing-library/react'; +import { defaultRecordPageObject } from '@objectstack/spec/ui'; +import { MetadataCtx, type MetadataContextValue } from '../../context/AppShellContext'; +import { usePageAssignment, type PageAssignmentOptions } from '../usePageAssignment'; + +/** A page as the metadata cache holds it: the body as served, unparsed. */ +type Page = Record; + +/** A metadata context carrying exactly the `pages` list under test. */ +function wrapperFor(pages: Page[]) { + const ctx: MetadataContextValue = { + apps: [], + objects: [], + dashboards: [], + reports: [], + pages, + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => pages, + getItem: async () => null, + getItemsByType: () => [], + getTypeStatus: () => 'ready' as const, + }; + return function Wrapper({ children }: { children: React.ReactNode }) { + return {children}; + }; +} + +async function assign(pages: Page[], objectName: string, opts?: PageAssignmentOptions) { + const { result } = renderHook(() => usePageAssignment(objectName, opts), { + wrapper: wrapperFor(pages), + }); + await waitFor(() => expect(result.current.loading).toBe(false)); + return result; +} + +const OBJECT = 'showcase_task'; + +/** A full record page for `object`; `extra` adds or overrides keys. */ +const record = (name: string, object: string = OBJECT, extra: Page = {}): Page => ({ + name, + label: name, + type: 'record', + object, + ...extra, +}); + +/** The same, declared the object's default record page. */ +const defaultRecord = (name: string, object: string = OBJECT, extra: Page = {}): Page => + record(name, object, { isDefault: true, ...extra }); + +describe('usePageAssignment — the object\'s default record page opens (objectui#12195, ruling A)', () => { + it('the fixtures are read the way the platform\'s doors read them (precondition)', () => { + // `defaultRecordPageObject` is the one reading every door that refuses a + // second default asks. Without this leg, a fixture the spec does not count + // as a default could make a pin below pass or fail for the wrong reason. + expect(defaultRecordPageObject(defaultRecord('d'))).toBe(OBJECT); + expect(defaultRecordPageObject(record('n'))).toBeUndefined(); + expect(defaultRecordPageObject(record('f', OBJECT, { isDefault: false }))).toBeUndefined(); + }); + + it('a default declared second wins over a page that is not the default declared first', async () => { + const result = await assign([record('authored_first'), defaultRecord('the_default')], OBJECT); + expect(result.current.page?.name).toBe('the_default'); + }); + + it('a default declared last wins from any depth in the served list', async () => { + const result = await assign( + [record('first'), record('second'), record('third'), defaultRecord('the_default')], + OBJECT, + ); + expect(result.current.page?.name).toBe('the_default'); + }); + + it('CONTROL: the same served list with the flag removed falls back to declaration order', async () => { + const result = await assign( + [record('authored_first'), record('the_default', OBJECT, { isDefault: false })], + OBJECT, + ); + expect(result.current.page?.name).toBe('authored_first'); + }); + + it('CONTROL: a single default with no other candidate is unchanged', async () => { + const result = await assign([defaultRecord('only_page')], OBJECT); + expect(result.current.page?.name).toBe('only_page'); + expect(result.current.slots).toBeNull(); + }); + + it('CONTROL: with no default, declaration order holds, `isDefault: false` included', async () => { + const pages = [ + record('alpha', OBJECT, { isDefault: false }), + record('beta'), + record('gamma', OBJECT, { isDefault: false }), + ]; + expect((await assign(pages, OBJECT)).current.page?.name).toBe('alpha'); + expect((await assign([...pages].reverse(), OBJECT)).current.page?.name).toBe('gamma'); + }); + + it('a slotted default wins the same way and populates `slots`, not `page`', async () => { + const result = await assign( + [record('authored_full'), defaultRecord('slotted_default', OBJECT, { kind: 'slotted', slots: { discussion: [] } })], + OBJECT, + ); + expect(result.current.page).toBeNull(); + expect(result.current.slots).toEqual({ discussion: [] }); + }); + + it('an explicit `pageName` still wins over the object\'s default', async () => { + const pages = [defaultRecord('the_default'), record('named_page')]; + const result = await assign(pages, OBJECT, { pageName: 'named_page' }); + expect(result.current.page?.name).toBe('named_page'); + }); + + it('the default is chosen AFTER the filter: another object\'s default, or a non-record default, does not displace the record page', async () => { + const result = await assign( + [ + defaultRecord('other_object_default', 'showcase_project'), + { ...defaultRecord('list_default'), type: 'list' }, + record('the_record_page'), + ], + OBJECT, + ); + expect(result.current.page?.name).toBe('the_record_page'); + }); + + it('two defaults (the cold-boot boundary the platform states): no tie-break is invented, served order decides', async () => { + // The platform refuses a second default at every door, but a stored + // default that hydrates after a package registered its own is not judged + // (objectstack#23213, closed not_planned). The hook adds no rung for that + // pair: the first default served wins, so reversing the order reverses the + // winner. Pinned so a precedence rule cannot be added here unannounced. + const a = defaultRecord('default_a'); + const b = defaultRecord('default_b'); + expect((await assign([record('not_default'), a, b], OBJECT)).current.page?.name).toBe('default_a'); + expect((await assign([record('not_default'), b, a], OBJECT)).current.page?.name).toBe('default_b'); + }); +}); diff --git a/packages/react/src/hooks/usePageAssignment.ts b/packages/react/src/hooks/usePageAssignment.ts index ebd6ff34d3..685e6ad301 100644 --- a/packages/react/src/hooks/usePageAssignment.ts +++ b/packages/react/src/hooks/usePageAssignment.ts @@ -6,9 +6,10 @@ * LICENSE file in the root directory of this source tree. * * `usePageAssignment` — resolve the record PageSchema that should be rendered - * for a given object. Walks the metadata cache exposed by `` - * and returns the first PageSchema whose `type === 'record'` and `object` - * matches the requested name. + * for a given object. Walks the metadata cache exposed by ``, + * keeps the PageSchemas whose `type === 'record'` and `object` matches the + * requested name, and returns the object's DEFAULT record page among them + * (`isDefault: true`), else the first of them. * * Returns a discriminated result by `PageSchema.kind`: * - `kind === 'full'` (default): the schema fully describes the page; @@ -18,16 +19,21 @@ * `buildDefaultPageSchema(objectDef, { slots })` so omitted slots * fall through to synthesized defaults. * - * Selection is by DECLARATION ORDER: the first matching page wins, so callers - * can deterministically fall back to the auto-generated DetailView when no - * record Page is authored. + * Selection is by the declared `PageSchema.isDefault`, then by DECLARATION + * ORDER among pages that are not the default: the spec allows one default + * record page per object, so the flag needs no tie-break here, and with no + * default the first matching page wins. No match returns null, so callers can + * deterministically fall back to the auto-generated DetailView when no record + * Page is authored. * * Future work (deferred): recordType / profile / app / formFactor filtering. - * Any author-driven ordering would have to be DECLARED on `PageSchema` in - * `@objectstack/spec` first — see the note on the selection itself below. + * Any further author-driven ordering would have to be DECLARED on + * `PageSchema` in `@objectstack/spec` first — see the note on the selection + * itself below. */ import { useEffect, useMemo, useState } from 'react'; +import { defaultRecordPageObject } from '@objectstack/spec/ui'; import { useMetadata } from '../context/AppShellContext.js'; export interface PageAssignmentOptions { @@ -143,22 +149,38 @@ export function usePageAssignment( if (!candidates.length) return null; - // Declaration order decides: the first match wins. + // The object's DEFAULT record page decides; with none, declaration order. // - // This used to sort on a `priority` key read off the candidate. No author - // could ever set it: `PageSchema` is a `strictObject` and does not declare - // `priority`, so a page carrying one is a HARD PARSE ERROR, not a page that - // sorts first. Every candidate therefore read `0`, the comparator returned - // `0` for every pair, and a stable sort left declaration order untouched — - // the sort documented an affordance the schema refuses (objectui#7298). + // At most one `isDefault: true` record page may exist for an object in a + // deployment. That is the spec's rule on `PageSchema.isDefault` (maintainer + // ruling A on objectstack#23015, record 6134056653), and the platform + // refuses a second default, naming both pages, at every door that can add + // one: `defineStack` / `composeStacks` (so `os validate` and a source boot), + // package registration, and metadata save. So this hook honours the flag + // with NO tie-break of its own. It asks `defaultRecordPageObject`, the one + // reading of "the default record page of an object" each of those doors + // asks, so the page the platform judges to be the default is the page that + // opens. Replacing a platform record page stays an ADR-0005 overlay by name + // (the same page, not a second default) or host composition; neither a + // second default nor an earlier position in the served list overrides one. // - // `isDefault` is not consulted here either. It IS declared on `PageSchema`, - // so unlike `priority` it is writable — but nothing in this decision reads - // it, which makes the flag on a shipped page (e.g. the platform's - // `sys_user_detail`) inert while looking decisive. Reading it would be new - // behaviour, not a repair; it is recorded here so the next reader does not - // mistake the flag for the thing that picks the page. - return candidates[0]; + // Boundary, stated rather than left implicit: at a cold boot every package + // registers before the environment's stored pages hydrate, so a stored + // default saved while no package held that object's default, and a package + // that adds one later, both reach the registry unrefused + // (objectstack#23213, closed not_planned). This hook invents no precedence + // for that pair (no layer, package or recency rung); served order stays the + // only order it has, so the first default served wins. + // + // Among pages that are not the default, declaration order is the last + // resort: the first match wins. This used to sort on a `priority` key read + // off the candidate. No author could ever set it: `PageSchema` is a + // `strictObject` and does not declare `priority`, so a page carrying one is + // a HARD PARSE ERROR, not a page that sorts first. Every candidate therefore + // read `0`, the comparator returned `0` for every pair, and a stable sort + // left declaration order untouched — the sort documented an affordance the + // schema refuses (objectui#7298). + return candidates.find(p => defaultRecordPageObject(p) === objectName) ?? candidates[0]; }, [meta.pages, objectName, opts.pageName, opts.recordType, opts.profile, opts.app, opts.formFactor]); // Discriminate by `kind`: full pages populate `page`, slotted pages