diff --git a/.changeset/29878-record-route-field-history.md b/.changeset/29878-record-route-field-history.md new file mode 100644 index 00000000000..496aa39179c --- /dev/null +++ b/.changeset/29878-record-route-field-history.md @@ -0,0 +1,12 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(app-shell): a record page with no declared record page shows a tracked record's field-change history again, in a packaged app and in an app created in the console + +Clause-②: no +Metadata-protocol: none + +A record route whose object has no declared record page reads its objects by name through the app's scope, not the whole object registry. That scope never held `sys_audit_log`, so the record page's history gate (an object declaring `enable.trackHistory`, and the audit log among the objects the page holds) read false. The page asked its payload for no field changes and never read the audit rows. Its timeline showed only the activity summaries, while the record drawer, which still reads the registry, showed the field changes. + +The route now also asks `sys_audit_log` by name, through the same scope, when the route's object declares `enable.trackHistory`, and waits for that answer before the record view first renders. If the read is refused, absent or failed, history stays off and the page still renders. The route reads no registry, and an object without `trackHistory` asks nothing more. diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.packagelessRecordRelated-29552.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.packagelessRecordRelated-29552.test.tsx index 88e1a0b156b..ec5d6d9028f 100644 --- a/ui/packages/app-shell/src/console/__tests__/AppContent.packagelessRecordRelated-29552.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.packagelessRecordRelated-29552.test.tsx @@ -68,10 +68,13 @@ vi.mock('../../views/ObjectView', () => ({
{objects.map((o) => o.name).join(',')}
), })); +/** objectstack#29878 — the object names each render of the record view was handed, in order. */ +const recordViewRenders = vi.hoisted(() => [] as string[][]); vi.mock('../../views/RecordDetailView', async () => { const { deriveRelatedLists: derive } = await import('../../utils/deriveRelatedLists'); return { RecordDetailView: ({ objects }: { objects: Array<{ name: string }> }) => { + recordViewRenders.push(objects.map((o) => o.name)); const { objectName } = useParams(); const def = objects.find((o) => o.name === objectName); const related = def ? derive(def as never, objects as never[]).map((r) => `${r.childObject}.${r.referenceField}`) : []; @@ -162,7 +165,14 @@ function serverBundle(app: Doc) { } type Peek = { status: 'none' } | { status: 'pending'; settled: Promise } | { status: 'ready'; items: ReturnType }; -const server = { calls: [] as string[], peek: null as Peek | null, objects: OBJECTS, pages: [] as Doc[] }; +const server = { + calls: [] as string[], + peek: null as Peek | null, + objects: OBJECTS, + pages: [] as Doc[], + /** By-name object reads the server answers with this error envelope instead (objectstack#29878). */ + refuse: {} as Record, +}; function makeAdapter() { const host: Record = { @@ -188,6 +198,8 @@ function makeAdapter() { const bundled = peek?.status === 'ready' ? peek.items.get(type)?.get(name) : undefined; if (bundled) return Promise.resolve(structuredClone(bundled.envelope)); server.calls.push(`${type}/${name}`); + const refusal = type === 'object' ? server.refuse[name] : undefined; + if (refusal) return Promise.reject(Object.assign(new Error(refusal.code), refusal)); const item = (type === 'object' ? server.objects : []).find((o) => o.name === name); return item ? Promise.resolve({ type, name, item: structuredClone(item) }) @@ -246,6 +258,8 @@ beforeEach(() => { server.calls = []; server.objects = OBJECTS; server.pages = []; + server.refuse = {}; + recordViewRenders.length = 0; server.peek = { status: 'ready', items: serverBundle(OPS_APP) }; localStorage.clear(); sessionStorage.clear(); @@ -466,3 +480,93 @@ describe('a packaged app’s record route with no declared record page reads its await waitFor(() => expect(objectCalls()).toContain('object')); }); }); + +/** + * objectstack#29878 — the record page draws a record's field-change history + * (`RecordDetailView`'s `historyEnabled`) only when the objects it is handed + * hold `sys_audit_log` and the record's object declares + * `enable.trackHistory`. The registry held it; the record route's scoped read + * (#29552, #29681) did not, so a packaged or package-less app's record page + * asked the page payload `fieldChanges: false` and never read the audit rows. + * The route now asks `sys_audit_log` by name for a tracked object, through + * the same scope, and waits for it, so the view's first render holds it. + */ +describe('a tracked object’s record route asks sys_audit_log by name (objectstack#29878)', () => { + const AUDIT: Doc = { name: 'sys_audit_log', _packageId: 'com.objectstack.audit', fields: { actor: { type: 'text' } } }; + const TRACKED: Doc[] = [ + ...OBJECTS.map((o) => (o.name === 'crm_account' ? { ...o, enable: { trackHistory: true } } : o)), + AUDIT, + ]; + const firstRender = () => recordViewRenders.find((names) => names.includes('crm_account')) ?? []; + const drawnObjects = () => (screen.getByTestId('record-view').textContent ?? '').split(','); + + beforeEach(() => { + server.objects = TRACKED; + // The bundle is built from the objects the server holds. + server.peek = { status: 'ready', items: serverBundle(OPS_APP) }; + }); + + it('a packaged app: the view’s first render holds sys_audit_log, read by name through the package scope', async () => { + server.peek = { status: 'ready', items: serverBundle(CRM_APP) }; + boot('/apps/crm/crm_account/record/r1'); + await waitFor(() => expect(screen.getByTestId('record-view')).toHaveTextContent('sys_user')); + await settle(); + + expect(firstRender()).toContain('sys_audit_log'); + expect(objectCalls()).not.toContain('object'); + expect(sorted(objectCalls())).toEqual(sorted(['object?package=app.crm', 'object/sys_user', 'object/sys_audit_log'])); + expect(drawnRelated()).toEqual(registryRelated('crm_account')); + }); + + it('a package-less app: the same, through the names-only scope', async () => { + boot('/apps/ops/crm_account/record/r1'); + await waitFor(() => expect(screen.getByTestId('record-view')).toHaveTextContent('sys_user')); + await settle(); + + expect(firstRender()).toContain('sys_audit_log'); + expect(objectCalls()).not.toContain('object'); + expect(sorted(objectCalls())).toEqual(sorted(['object/sys_user', 'object/sys_audit_log'])); + expect(drawnRelated()).toEqual(registryRelated('crm_account')); + }); + + it('CONTROL: an object without trackHistory asks nothing more', async () => { + server.peek = { status: 'ready', items: serverBundle(CRM_APP) }; + boot('/apps/crm/crm_contact/record/c1'); + await waitFor(() => expect(screen.getByTestId('record-view')).toHaveTextContent('crm_contact')); + await settle(); + + expect(objectCalls()).not.toContain('object/sys_audit_log'); + expect(drawnObjects()).not.toContain('sys_audit_log'); + }); + + it('a refused read keeps history off and the page drawn, with no registry read', async () => { + server.refuse = { sys_audit_log: { httpStatus: 403, code: 'PERMISSION_DENIED' } }; + server.peek = { status: 'ready', items: serverBundle(CRM_APP) }; + boot('/apps/crm/crm_account/record/r1'); + await waitFor(() => expect(screen.getByTestId('record-view')).toHaveTextContent('sys_user')); + await settle(); + + expect(objectCalls()).toContain('object/sys_audit_log'); + expect(objectCalls()).not.toContain('object'); + expect(drawnObjects()).not.toContain('sys_audit_log'); + expect(drawnRelated()).toEqual(registryRelated('crm_account')); + }); + + it('an absent sys_audit_log (404) keeps history off and the page drawn', async () => { + server.objects = TRACKED.filter((o) => o.name !== 'sys_audit_log'); + server.peek = { status: 'ready', items: serverBundle(OPS_APP) }; + boot('/apps/ops/crm_account/record/r1'); + await waitFor(() => expect(screen.getByTestId('record-view')).toHaveTextContent('sys_user')); + await settle(); + + expect(objectCalls()).toContain('object/sys_audit_log'); + expect(objectCalls()).not.toContain('object'); + expect(drawnObjects()).not.toContain('sys_audit_log'); + }); + + it('CONTROL: a record drawer over the route still reads the registry, which holds it (objectstack#27626)', async () => { + server.peek = { status: 'ready', items: serverBundle(CRM_APP) }; + boot('/apps/crm/crm_account/record/r1?recordId=r2'); + await waitFor(() => expect(objectCalls()).toContain('object')); + }); +}); diff --git a/ui/packages/app-shell/src/console/appObjectScope.ts b/ui/packages/app-shell/src/console/appObjectScope.ts index f4f1837bf27..a5f8942c06f 100644 --- a/ui/packages/app-shell/src/console/appObjectScope.ts +++ b/ui/packages/app-shell/src/console/appObjectScope.ts @@ -553,7 +553,10 @@ export function readAppObjectScope( const referencing = readObjectsReferencing?.(route.objectName); if (referencing?.state === 'pending') return { candidate, scope: PENDING_OBJECT_SCOPE, ready: false, ...NOT_READ }; if (referencing?.state !== 'hit') return { candidate, scope: null, ready: false, ...NOT_READ }; - return { candidate, ...readObjectRouteNames(readObjectScope, app, route.objectName, search, packageId, referencing.value) }; + return { + candidate, + ...readObjectRouteNames(readObjectScope, app, route.objectName, search, packageId, referencing.value, true), + }; } const extra = [route.objectName, ...appRequiredObjectNames(app)]; if (new URLSearchParams(search).has(COMMAND_PALETTE_PARAM)) { @@ -613,7 +616,10 @@ export function readAppObjectScope( * Only the route's object holds the route. `relatedNames` (objectstack#29552: * the objects that point at a record's object, whose related lists its * synthesized page draws) are asked with the route's names, and the route - * waits for them as it waits for its own object. + * waits for them as it waits for its own object. `synthesizedRecordPage` + * (objectstack#29878): the route is a record route with no declared record + * page, whose synthesized page also reads the objects its record streams need + * ({@link recordStreamObjectNames}); the route waits for those too. */ function readObjectRouteNames( readObjectScope: ReadObjectScope, @@ -622,6 +628,7 @@ function readObjectRouteNames( search: string, packageId: string | undefined, relatedNames: readonly string[] = [], + synthesizedRecordPage = false, ): Omit { // With no package and no name, nothing: the provider reads an empty // names-only ask as the whole registry (objectstack#29552, an app's root). @@ -646,16 +653,41 @@ function readObjectRouteNames( const first = relatedNames.length > 0 ? inRelatedOrder(asked, relatedNames) : asked; const routeDef = objectName ? first.objects.find((o: any) => o?.name === objectName) : undefined; const lookups = routeDef ? lookupTargetNames(routeDef) : []; - if (lookups.length === 0) return { scope: first, ready, ...routeOutcome }; + const streamNames = synthesizedRecordPage && routeDef ? recordStreamObjectNames(routeDef) : []; + if (lookups.length === 0 && streamNames.length === 0) return { scope: first, ready, ...routeOutcome }; // The lookup targets are asked without views, through the same scope as the // route's names: a lookup cell links and names its target and reads none of // its views, and a target outside the app bundle (`sys_user`) composed with // them read the whole `/meta/view` list (objectstack#29551). They join the // scope's objects as they land; the route's names keep their views. const readWithoutViews = readObjectScope as ReadNamesWithoutViews; - if (packageId) readWithoutViews({ packageId, names: lookups, views: false }); - else readWithoutViews({ names: lookups, views: false }); - return { scope: first, ready, ...routeOutcome }; + const readNames = (names: readonly string[]) => + packageId ? readWithoutViews({ packageId, names, views: false }) : readWithoutViews({ names, views: false }); + if (lookups.length > 0) readNames(lookups); + if (streamNames.length === 0) return { scope: first, ready, ...routeOutcome }; + // objectstack#29878 — the stream objects are asked the same way, once the + // route's object says it needs them, and the route waits for them: the + // record view reads them on its first render, where the page payload's ask + // goes out (`fieldChanges`). A refused, absent or failed read settles the + // wait without the object, so the stream it feeds stays off and the page is + // drawn; it is not the route's failure (`routeOutcome` reads the route's own + // object only). + const streamRead = readNames(streamNames); + return { scope: first, ready: ready && routeReadDone(streamRead), ...routeOutcome }; +} + +/** + * objectstack#29878 — the objects a synthesized record page's streams read + * off the route's object list, beyond the record's own object and its + * related ones: the audit log, whose field-change lines the record's timeline + * draws when the object declares `enable.trackHistory` (`RecordDetailView`'s + * `historyEnabled` asks the list whether it holds `sys_audit_log`). The + * registry held it; the route's scoped read holds only what is asked. + */ +const RECORD_HISTORY_OBJECT = 'sys_audit_log'; +function recordStreamObjectNames(routeDef: { name?: unknown; enable?: { trackHistory?: unknown } }): string[] { + if (routeDef.name === RECORD_HISTORY_OBJECT) return []; + return routeDef.enable?.trackHistory === true ? [RECORD_HISTORY_OBJECT] : []; } /** diff --git a/ui/packages/app-shell/src/views/RecordDetailView.timelineCount-28737.test.tsx b/ui/packages/app-shell/src/views/RecordDetailView.timelineCount-28737.test.tsx index e482dfe342a..7f15d79e418 100644 --- a/ui/packages/app-shell/src/views/RecordDetailView.timelineCount-28737.test.tsx +++ b/ui/packages/app-shell/src/views/RecordDetailView.timelineCount-28737.test.tsx @@ -184,13 +184,13 @@ beforeEach(() => { }); afterEach(() => { cleanup(); vi.unstubAllGlobals(); }); -async function boot(answers: Answers = {}, entry = `/apps/demo/${OBJECT}/record/c1`) { +async function boot(answers: Answers = {}, entry = `/apps/demo/${OBJECT}/record/c1`, objects: readonly unknown[] = ALL) { const { fetch, held } = heldTransport(answers); vi.stubGlobal('fetch', fetch); const ds = new ObjectStackAdapter({ baseUrl: BASE, fetch, autoReconnect: false }); await ds.connect(); const metadata = { - objects: ALL, pages: [], loading: false, error: null, + objects, pages: [], loading: false, error: null, refresh: async () => {}, invalidate: () => {}, ensureType: async () => [], getItem: async () => null, getItemsByType: () => [], } as never; @@ -198,7 +198,7 @@ async function boot(answers: Answers = {}, entry = `/apps/demo/${OBJECT}/record/ - {}} objectNameOverride={OBJECT} recordIdOverride="c1" /> + {}} objectNameOverride={OBJECT} recordIdOverride="c1" /> , @@ -272,3 +272,30 @@ describe('objectstack#28737 — the 动态 count is the server\'s, and the page expect(chipOf(view.container)).toBe('6'); }); }); + +/** + * objectstack#29878 — the page's `timeline` ask counts the field changes when + * the record's object declares `enable.trackHistory` and the objects the view + * is handed hold `sys_audit_log` (the record route asks it by name for such an + * object: `appObjectScope.ts`). Without the audit object in hand it asks none. + */ +describe('objectstack#29878 — the timeline ask counts the field changes of a tracked object', () => { + const tracked = { ...contactDef, enable: { ...contactDef.enable, trackHistory: true } }; + const auditDef = { name: 'sys_audit_log', label: 'Audit Log', fields: { id: { label: 'Id', type: 'text' } } }; + + it('with sys_audit_log in the objects, the page asks fieldChanges: true', async () => { + const { held, open } = await boot({ timeline: { total: 3 } }, undefined, [tracked, accountDef, childDef, auditDef]); + open(); + await settle(held); + const [page] = held.filter(isPage); + expect(page.body.timeline).toEqual({ discussion: true, activity: true, fieldChanges: true }); + }); + + it('CONTROL: without sys_audit_log in the objects, it asks fieldChanges: false', async () => { + const { held, open } = await boot({ timeline: { total: 3 } }, undefined, [tracked, accountDef, childDef]); + open(); + await settle(held); + const [page] = held.filter(isPage); + expect(page.body.timeline).toEqual({ discussion: true, activity: true, fieldChanges: false }); + }); +});