Skip to content

Commit b322b5d

Browse files
fix(console): a record page leaves out related lists and rail entries of an object the install does not hold (#27180)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8GUdk9SEEZXQX6B5hTkWx4K)_ Fixes #26988 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:记录页上,绑定到当前安装里不存在的对象的相关列表和右栏条目不再绘制,也不再为它发 `/meta/object`、`/data` 请求。 - 为什么改:ADR-0141 之后模块对象可以真实缺席,页面却照画「Crm Quote 0 — No related records found」这类拿原始名拼出的假标题,并打出一串 404。 - 风险与回滚:只在已注册对象列表非空时裁剪,与仪表盘 `requiresObject` 同一规则;回滚即撤掉 `RecordDetailView.tsx` 里两处 `pruneUnregisteredRecordPageObjects` 调用。 - 先看:`recordPageChildObjects.ts`(裁剪函数)、`RecordDetailView.tsx`(`assignedPage` / `assignedSlots` 两个 memo)、新测试文件。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/RecordDetailView.unregisteredObjectLists-26988.test.tsx`,预期 2 个用例通过。 - 需要你做的:无,席位落地。 ## What changes - `recordPageChildObjects.ts` gains `pruneUnregisteredRecordPageObjects(page, registered)`. It reads the same two arms as `recordPageChildObjects` (now shared helpers): a `record:related_list` block whose `properties.objectName` or `dataSource.object` names an object the registered list does not hold is removed; a `record:reference_rail` entry whose `objectName` does is removed, and a rail left with no entries goes with it. Only a string names an object. While the registered list is empty it returns the page untouched (the deferral `DashboardView` makes), and it is identity-preserving: a page or subtree with nothing to leave out is returned itself. - `RecordDetailView` prunes the authored page and a slotted page's slots right where `assignedPage` / `assignedSlots` are resolved, so the effective page, the synthesized page, the composed page and the history page all read one pruned page. - The registered list is the view's own `objects` prop, the list `deriveRelatedLists` already reads. It is not `useMetadata().objects`: that getter loads the whole registry, which the scoped record route (objectui#13046) exists to avoid. On that route `objects` is the scope: the package's objects plus every object the page binds, each asked by name. - The child-write subscription from #26890 reads the pruned page. An object the install does not hold cannot be written, so there is nothing to watch on it. - Derived related lists (`deriveRelatedLists`) were already safe: they come from the registered objects only. ## Tests - `RecordDetailView.unregisteredObjectLists-26988.test.tsx` uses the real page. It has a related list bound by `objectName`, one bound by `dataSource.object`, and a rail entry, all three for absent objects, beside a registered contact list and rail entry. - Absent arm: no humanized name ("Crm Quote" and the others), no rows, and no data-source call or `fetch` URL names any of the three objects. The contact list and rail entry still draw and read. - Present arm: the same page draws all three and reads every object. - `recordPageChildObjects.test.ts`: pins what is left out and what is kept, the deferral (an empty set prunes nothing), identity preservation, that the authored page is never mutated, and that non-string names are kept. - Ablation: `scripts/ablation-replace.mjs` replaced the guard line `if (registered.size === 0) return page;` with an unconditional `return page;`. The absent-arm test went red (`expected '…' not to contain 'Crm Quote'`) and the present arm stayed green. The tool restored the file (blob == HEAD, `git diff HEAD` empty). ## Acceptance notes - On a scoped record route (a full declared record page in a packaged app), the scope still asks `/meta/object/NAME` once for each name the page binds, absent ones included. That read is how the console learns the object is absent; it is the one existence check, and this PR adds none. Everything the page itself asked (each block's schema read and `/data` read) is gone. - A tab or container whose only content was a pruned list stays, now empty. Pruning containers is not part of this card. - `record:line_items` (`childObject`) is not pruned. It is outside this card's two arms. ## Verification Gate readings and the head they were taken on are in the os-dev report comment on #26988. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (Local: eslint on the four changed files, 0 errors; the repo-wide run is CI's.) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (Nothing publishes here, so no changeset.) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (Not touched.) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. `@object-ui/app-shell` is `private: true` and nothing else changed, so this PR takes `skip-changeset`. --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbaQJGyCfqGqBduLgbq)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c26d643 commit b322b5d

4 files changed

Lines changed: 516 additions & 9 deletions

File tree

‎ui/packages/app-shell/src/views/RecordDetailView.tsx‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { SkeletonDetail } from '../skeletons/index.js';
2525
import { ManagedByBadge, useManagedByNote } from '../components/ManagedByBadge.js';
2626
import { isSetupAppSegment } from '../console/setupApp.js';
2727
import { deriveRelatedLists } from '../utils/deriveRelatedLists.js';
28-
import { recordPageChildObjects } from './recordPageChildObjects.js';
28+
import { pruneUnregisteredRecordPageObjects, recordPageChildObjects } from './recordPageChildObjects.js';
2929
import { stripDiscussionNodes, hasExplicitAttachments, hasExplicitApprovals } from '../utils/pageSchemaIntrospect.js';
3030
import { ActionConfirmDialog, type ConfirmDialogState } from './ActionConfirmDialog.js';
3131
import { ActionParamDialog, type ParamDialogState } from './ActionParamDialog.js';
@@ -795,8 +795,29 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
795795
return isRecordPageOf(named, objectName) ? 'accepted' : 'refused';
796796
}, [requestedPageName, pageAssignmentLoading, metadataPages, objectName]);
797797
const namedPageBlocked = namedPageState === 'pending' || namedPageState === 'refused';
798-
const assignedPage = namedPageBlocked ? null : resolvedPage;
799-
const assignedSlots = namedPageBlocked ? null : resolvedSlots;
798+
// objectstack#26988 — a related list or rail entry the page lists for an
799+
// object this install does not hold is left out, by the rule `DashboardView`
800+
// prunes widgets with: the registered object list. That list is this view's
801+
// `objects` — the one `deriveRelatedLists` reads below, and on a scoped
802+
// record route the objects the page binds, each asked by name
803+
// (objectui#13046). ⛔ Never `useMetadata().objects` here: reading it loads
804+
// the whole registry the scope exists to spare (objectui#12811). Both the
805+
// authored page and a slotted page's slots are pruned, so every reader
806+
// downstream — the effective page, the composed page, and the child-write
807+
// subscription (objectstack#26890), which has nothing to watch on an object
808+
// that cannot be written — sees one page. Deferred while the list is empty.
809+
const registeredObjectNames = useMemo(
810+
() => new Set<string>(objects.map((o: any) => o?.name).filter(Boolean)),
811+
[objects],
812+
);
813+
const assignedPage = useMemo(
814+
() => (namedPageBlocked ? null : pruneUnregisteredRecordPageObjects(resolvedPage, registeredObjectNames)),
815+
[namedPageBlocked, resolvedPage, registeredObjectNames],
816+
);
817+
const assignedSlots = useMemo(
818+
() => (namedPageBlocked ? null : pruneUnregisteredRecordPageObjects(resolvedSlots, registeredObjectNames)),
819+
[namedPageBlocked, resolvedSlots, registeredObjectNames],
820+
);
800821
const synthesizedPage = useMemo(() => {
801822
// Synthesizer drives two cases:
802823
// 1) no assignedPage at all → pure default detail page
Lines changed: 312 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,312 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectstack#26988 — a record page leaves out a related list or rail entry
11+
* whose object this install does not hold.
12+
*
13+
* The reported reach (HotCRM on 18.2.0, a module's objects absent): an
14+
* opportunity's authored record page lists quotes; with the quote module not
15+
* installed the page drew "Crm Quote 0 — No related records found", the rail
16+
* read "+3 empty (Crm Quote · …)", and the page logged 404s on
17+
* `/api/v1/meta/object/…` and `/api/v1/data/…`. Since ADR-0141 a module's
18+
* objects can really be absent, so any app record page naming a dependent
19+
* module's object showed this.
20+
*
21+
* The rule is `DashboardView`'s `requiresObject` / `widget.object` filter: an
22+
* object-bound entry the registered object list does not hold is not drawn,
23+
* and nothing is asked about it. What this file pins, through the real page:
24+
*
25+
* 1. With the object absent, neither its related list (bound by
26+
* `properties.objectName` or by `dataSource.object`) nor its rail entry is
27+
* drawn, and no data or schema read names it — through the data source
28+
* or `fetch`. The lists and rail entries of registered objects still draw.
29+
* 2. With the object present, the same page draws it and reads it.
30+
*
31+
* The deferral (an empty list prunes nothing) and the arms are pinned on the
32+
* helper, in `recordPageChildObjects.test.ts`.
33+
*/
34+
35+
import * as React from 'react';
36+
import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest';
37+
import { render, screen, waitFor, cleanup, act } from '@testing-library/react';
38+
import { MemoryRouter, Route, Routes } from 'react-router-dom';
39+
import { MetadataCtx } from '@object-ui/react';
40+
import { RelatedCountStore } from '@object-ui/components';
41+
42+
vi.mock('@object-ui/auth', async (importOriginal) => ({
43+
...(await importOriginal<Record<string, unknown>>()),
44+
useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }),
45+
createAuthenticatedFetch: () => vi.fn(),
46+
}));
47+
48+
vi.mock('@object-ui/collaboration', async (importOriginal) => ({
49+
...(await importOriginal<Record<string, unknown>>()),
50+
useRecordPresence: () => [],
51+
PresenceAvatars: () => null,
52+
}));
53+
54+
vi.mock('sonner', () => ({
55+
toast: Object.assign(vi.fn(), {
56+
success: vi.fn(),
57+
error: vi.fn(),
58+
info: vi.fn(),
59+
warning: vi.fn(),
60+
loading: vi.fn(),
61+
dismiss: vi.fn(),
62+
}),
63+
}));
64+
65+
// Orthogonal chrome — same posture as RecordDetailView.authoredChildWriteRefreshesParent-26890.
66+
vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null }));
67+
vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null }));
68+
vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null }));
69+
vi.mock('./FlowRunner', () => ({ FlowRunner: () => null }));
70+
vi.mock('./MetadataInspector', () => ({
71+
MetadataPanel: () => null,
72+
useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }),
73+
}));
74+
75+
import { RecordDetailView } from './RecordDetailView';
76+
77+
beforeAll(() => {
78+
Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 });
79+
});
80+
81+
const APP = 'crm';
82+
const PARENT = 'os_26988_opportunity';
83+
/** Registered: listed in a related list and in the rail; always drawn. */
84+
const CONTACT = 'os_26988_contact';
85+
/** The dependent module's objects: listed by the page, absent from the install unless a case installs them. */
86+
const QUOTE = 'os_26988_crm_quote';
87+
const LINE_ITEM = 'os_26988_crm_line_item';
88+
const PRODUCT = 'os_26988_crm_product';
89+
const RECORD_ID = 'opp-1';
90+
91+
const parentObject = {
92+
name: PARENT,
93+
label: 'Opportunity',
94+
fields: {
95+
id: { type: 'text', label: 'Id' },
96+
name: { type: 'text', label: 'Name' },
97+
},
98+
};
99+
100+
const childOf = (name: string, label: string) => ({
101+
name,
102+
label,
103+
fields: {
104+
id: { type: 'text', label: 'Id' },
105+
name: { type: 'text', label: 'Name' },
106+
// Opted out of the derived lists: the authored page is the only place it shows.
107+
opportunity: { type: 'lookup', reference: PARENT, label: 'Opportunity', relatedList: false },
108+
},
109+
});
110+
111+
const CONTACT_OBJECT = childOf(CONTACT, 'Contact');
112+
const MODULE_OBJECTS = [childOf(QUOTE, 'Quote'), childOf(LINE_ITEM, 'Line Item'), childOf(PRODUCT, 'Product')];
113+
114+
/** The opportunity's authored record page, reduced to the blocks this card is about. */
115+
const RECORD_PAGE = {
116+
name: 'os_26988_opportunity_detail',
117+
label: 'Opportunity',
118+
type: 'record',
119+
object: PARENT,
120+
kind: 'full',
121+
template: 'default',
122+
isDefault: true,
123+
regions: [
124+
{
125+
name: 'main',
126+
width: 'large',
127+
components: [
128+
{ type: 'page:header', properties: { title: '{name}' } },
129+
{
130+
type: 'record:related_list',
131+
id: 'contacts',
132+
properties: { objectName: CONTACT, relationshipField: 'opportunity', columns: ['name'], limit: 5 },
133+
},
134+
{
135+
type: 'record:related_list',
136+
id: 'quotes',
137+
properties: { objectName: QUOTE, relationshipField: 'opportunity', columns: ['name'], limit: 5 },
138+
},
139+
{
140+
// Bound through the element data source, which the block's gate maps onto `objectName`.
141+
type: 'record:related_list',
142+
id: 'line_items',
143+
dataSource: { object: LINE_ITEM },
144+
properties: { relationshipField: 'opportunity', columns: ['name'], limit: 5 },
145+
},
146+
],
147+
},
148+
{
149+
name: 'aside',
150+
width: 'small',
151+
components: [
152+
{
153+
type: 'record:reference_rail',
154+
id: 'rail',
155+
properties: {
156+
entries: [
157+
{ objectName: CONTACT, relationshipField: 'opportunity', limit: 3 },
158+
{ objectName: PRODUCT, relationshipField: 'opportunity', limit: 3 },
159+
],
160+
},
161+
},
162+
],
163+
},
164+
],
165+
};
166+
167+
const ROWS: Record<string, Array<{ id: string; name: string; opportunity: string }>> = {
168+
[CONTACT]: [{ id: 'c-1', name: 'Grace Hopper', opportunity: RECORD_ID }],
169+
[QUOTE]: [{ id: 'q-1', name: 'QTE-0007', opportunity: RECORD_ID }],
170+
[LINE_ITEM]: [{ id: 'li-1', name: 'Seat licence', opportunity: RECORD_ID }],
171+
[PRODUCT]: [{ id: 'p-1', name: 'Widget Pro', opportunity: RECORD_ID }],
172+
};
173+
174+
function makeDataSource() {
175+
return {
176+
find: vi.fn(async (objectName: string) => {
177+
const rows = ROWS[objectName] ?? [];
178+
return { data: rows.map((r) => ({ ...r })), total: rows.length };
179+
}),
180+
findOne: vi.fn(async (objectName: string, id: string) => {
181+
if (objectName !== PARENT || id !== RECORD_ID) return null;
182+
return { id: RECORD_ID, name: 'Big Deal' };
183+
}),
184+
getObjectSchema: vi.fn(async (objectName: string) => {
185+
const all = [parentObject, CONTACT_OBJECT, ...MODULE_OBJECTS];
186+
return all.find((o) => o.name === objectName) ?? null;
187+
}),
188+
};
189+
}
190+
191+
type FakeDataSource = ReturnType<typeof makeDataSource>;
192+
193+
function renderPage(ds: FakeDataSource, objects: unknown[]) {
194+
const metadata = {
195+
objects,
196+
pages: [RECORD_PAGE],
197+
loading: false,
198+
error: null,
199+
refresh: async () => {},
200+
invalidate: () => {},
201+
ensureType: async () => [],
202+
getItem: async () => null,
203+
getItemsByType: () => [],
204+
} as unknown as React.ContextType<typeof MetadataCtx>;
205+
return render(
206+
<MemoryRouter initialEntries={[`/apps/${APP}/${PARENT}/record/${RECORD_ID}`]}>
207+
<MetadataCtx.Provider value={metadata}>
208+
<Routes>
209+
<Route
210+
path="/apps/:appName/:objectName/record/:recordId"
211+
element={<RecordDetailView dataSource={ds} objects={objects as any[]} onEdit={() => {}} />}
212+
/>
213+
</Routes>
214+
</MetadataCtx.Provider>
215+
</MemoryRouter>,
216+
);
217+
}
218+
219+
/** Every object name any data-source call was made for. */
220+
const dataSourceObjects = (ds: FakeDataSource) =>
221+
new Set([...ds.find.mock.calls, ...ds.findOne.mock.calls, ...ds.getObjectSchema.mock.calls].map(([name]) => name));
222+
223+
/** Every URL handed to `fetch`. */
224+
const fetchedUrls = () =>
225+
(globalThis.fetch as unknown as ReturnType<typeof vi.fn>).mock.calls.map(([input]) =>
226+
String(typeof input === 'object' && input && 'url' in input ? (input as Request).url : input),
227+
);
228+
229+
/** Let any read a mounted block could have scheduled land before reading. */
230+
const settle = () =>
231+
act(async () => {
232+
await new Promise((r) => setTimeout(r, 100));
233+
});
234+
235+
async function settledPage(objects: unknown[]) {
236+
const ds = makeDataSource();
237+
renderPage(ds, objects);
238+
await waitFor(() => expect(ds.findOne.mock.calls.some(([name, id]) => name === PARENT && id === RECORD_ID)).toBe(true));
239+
// The registered list draws in both arms: the page is past its first render.
240+
await screen.findAllByText('Grace Hopper');
241+
await settle();
242+
return ds;
243+
}
244+
245+
/**
246+
* Every element on screen: the rail reads its entries once it is seen, so an
247+
* observer that never fires would leave the rail asking nothing in either arm.
248+
*/
249+
class VisibleObserver {
250+
constructor(private readonly cb: (entries: Array<{ isIntersecting: boolean }>) => void) {}
251+
observe() {
252+
queueMicrotask(() => this.cb([{ isIntersecting: true }]));
253+
}
254+
unobserve() {}
255+
disconnect() {}
256+
}
257+
258+
beforeEach(() => {
259+
cleanup();
260+
RelatedCountStore._reset();
261+
vi.stubGlobal('IntersectionObserver', VisibleObserver);
262+
vi.stubGlobal(
263+
'fetch',
264+
vi.fn(async () =>
265+
new Response(JSON.stringify({ data: [] }), {
266+
status: 200,
267+
headers: { 'content-type': 'application/json' },
268+
}),
269+
),
270+
);
271+
});
272+
273+
afterEach(() => {
274+
vi.unstubAllGlobals();
275+
vi.restoreAllMocks();
276+
});
277+
278+
describe('RecordDetailView — a related list or rail entry whose object is not installed is left out (objectstack#26988)', () => {
279+
it('with the module absent, its lists and rail entry are not drawn and nothing is asked about their objects', async () => {
280+
const ds = await settledPage([parentObject, CONTACT_OBJECT]);
281+
282+
// No humanized raw name, no empty list, no folded rail chip for any of the three.
283+
const text = document.body.textContent ?? '';
284+
for (const humanized of ['Crm Quote', 'Crm Line Item', 'Crm Product']) expect(text).not.toContain(humanized);
285+
expect(screen.queryByText('QTE-0007')).toBeNull();
286+
expect(screen.queryByText('Seat licence')).toBeNull();
287+
expect(screen.queryByText('Widget Pro')).toBeNull();
288+
289+
// No `/meta/object` or `/data` round trip for them, through either door.
290+
const asked = dataSourceObjects(ds);
291+
for (const absent of [QUOTE, LINE_ITEM, PRODUCT]) {
292+
expect(asked.has(absent)).toBe(false);
293+
expect(fetchedUrls().filter((url) => url.includes(absent))).toEqual([]);
294+
}
295+
296+
// The registered object's list and rail entry still draw and read.
297+
expect(asked.has(CONTACT)).toBe(true);
298+
expect(screen.getAllByText('Grace Hopper').length).toBeGreaterThan(0);
299+
});
300+
301+
it('with the module installed, the same page draws and reads every list and rail entry', async () => {
302+
const ds = await settledPage([parentObject, CONTACT_OBJECT, ...MODULE_OBJECTS]);
303+
304+
await screen.findAllByText('QTE-0007');
305+
await screen.findAllByText('Seat licence');
306+
await screen.findAllByText('Widget Pro');
307+
await waitFor(() => {
308+
const asked = dataSourceObjects(ds);
309+
for (const present of [CONTACT, QUOTE, LINE_ITEM, PRODUCT]) expect(asked.has(present)).toBe(true);
310+
});
311+
});
312+
});

0 commit comments

Comments
 (0)