Skip to content

Commit f7be03f

Browse files
claude[bot]claude
andauthored
fix(metadata): getViewsByObject() expands aggregated view containers instead of answering empty (#14206)
* wip(metadata): getViewsByObject container expansion * fix(metadata): expand aggregated view containers in getViewsByObject * test(metadata): split the dedupe control so it is green in both directions * chore: changeset + record ablation #2 in the pin header --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9738c35 commit f7be03f

4 files changed

Lines changed: 480 additions & 10 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(metadata): `getViewsByObject()` expands aggregated view containers instead of answering empty (#13913)
6+
7+
`MetadataManager.getViewsByObject()` reads `this.list('view')` — the manager's
8+
own registry + loader store, which is a completely different store from the
9+
`sys_metadata` rows the REST route (`GET /meta/view?object=`) reads through
10+
`getMetaItems`. #13407 taught that route to expand a runtime-authored aggregated
11+
`defineView` container inline; this exit never called it and had no equivalent
12+
step, so a container the REST route now serves still answered **empty** here —
13+
for every internal/SDK caller that uses this entry point rather than the route.
14+
15+
Getting the container into the store was never enough on its own: the filter
16+
also requires `viewKind`, and a container has none. Relaxing that requirement
17+
would answer with the container itself as a view — the behaviour #7163 ruled
18+
wrong — so the repair adds the container's **expansion**, whose items each carry
19+
the `viewKind` + `object` pair this filter has always tested. The filter is
20+
untouched; it reads the top-level `object` exactly as `ViewSchema.object`
21+
declares.
22+
23+
The expansion is registry-free and per-read, mirroring #13407's choice at the
24+
other exit and for the same reason: the registry is process-wide, so a read must
25+
not graft rows into it. Already-present names win, so a container whose expanded
26+
ViewItems were registered by a source registrar (the ObjectQL boot loop, the
27+
artifact/HMR loader) still answers with those registered, fully-enriched items
28+
and gains nothing new.
29+
30+
The object-derivation chain (`object``list.data.object``form.data.object`
31+
→ the row's own `name`) now has one spelling for this package, in the new
32+
`view-container-expansion.ts`, rather than a third private copy to fall behind.
Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #13913 — `MetadataManager.getViewsByObject()` answered EMPTY for an
5+
* aggregated view container living in this manager's own backing store.
6+
*
7+
* ---------------------------------------------------------------------------
8+
* The defect, and why it survived #13407
9+
* ---------------------------------------------------------------------------
10+
* There are two independent object-bound readers. The REST route
11+
* (`GET /meta/view?object=`) reads through `ObjectStackProtocolImplementation.
12+
* getMetaItems` over `sys_metadata` rows; #13407 taught THAT reader to expand a
13+
* runtime-authored container inline. `getViewsByObject()` reads
14+
* `this.list('view')` — `MetadataManager`'s OWN registry + loader store, a
15+
* completely separate store — never calls `getMetaItems`, and had no equivalent
16+
* step. So the very container #13407 made visible on the wire still answered
17+
* empty through this entry point.
18+
*
19+
* The filter here was never the bug: it reads the top-level `object`, exactly
20+
* as `ViewSchema.object` declares. Two conditions have to hold at once, and the
21+
* second is the one that decides the shape of the fix — the filter ALSO
22+
* requires `viewKind`, and a container has none. Getting the container into the
23+
* store is therefore not enough; the container's EXPANSION has to be what the
24+
* read sees. Relaxing the `viewKind` requirement instead would answer with the
25+
* container itself as a view — the behaviour #7163 ruled wrong — which is why
26+
* `answers with EXPANDED items and never the container itself` below is a pin
27+
* against that regression and not a restatement of the fix.
28+
*
29+
* ---------------------------------------------------------------------------
30+
* What is driven, and why it is NOT the REST route
31+
* ---------------------------------------------------------------------------
32+
* Every assertion goes through `manager.getViewsByObject(...)` itself. A pin
33+
* written against `GET /meta/view?object=` would have gone green on `main`
34+
* without touching this bug at all — #13929 already repaired that exit.
35+
*
36+
* The container fixture is the card's own shape: a top-level `object` and NO
37+
* `list.data.object`. That combination is what #13407's corrected derivation
38+
* chain exists for, and it is why `container.object` has to be consulted first.
39+
*
40+
* ---------------------------------------------------------------------------
41+
* The controls, and what the first ablation corrected about them
42+
* ---------------------------------------------------------------------------
43+
* The two `CONTROL:` cases are green in BOTH directions on purpose: this change
44+
* must not move what the exit already answered, so a case going red there would
45+
* report a regression rather than this fix. Neither may therefore depend on the
46+
* expansion existing.
47+
*
48+
* The dedupe case was originally written as ONE control asserting both the full
49+
* answer AND the identity of the registered item. The first ablation falsified
50+
* that: it went RED, because pre-fix the answer is the registered item alone.
51+
* A control that goes red under ablation is not a control, so it is split here
52+
* — the SET assertion is an ordinary case (`contributes only the names the
53+
* store does not already hold`, red under ablation), and the both-directions
54+
* half is the object-IDENTITY assertion, which holds either way.
55+
*
56+
* Reverse verification, direction re-predicted after that split and recorded in
57+
* the PR body as measured: reverting `getViewsByObject` to its pre-#13913 body
58+
* turns the four container/expansion cases RED and leaves all three of the
59+
* remaining cases GREEN.
60+
*
61+
* `answers with EXPANDED items and never the container itself (#7163)` is one
62+
* of those three, and it is green under that reversion only VACUOUSLY — the
63+
* pre-fix answer is empty, so nothing can be a container. It is a guard against
64+
* a DIFFERENT mutation, and it was ablated separately against exactly that one:
65+
* dropping `v.viewKind &&` from the filter (the tempting one-line "fix", which
66+
* makes the container answer as a view) turns it RED. Measured, so the guard is
67+
* known to fire rather than assumed to.
68+
*/
69+
70+
import { describe, it, expect, vi } from 'vitest';
71+
import type { IDataDriver } from '@objectstack/spec/contracts';
72+
import { MetadataManager } from './metadata-manager.js';
73+
import { DatabaseLoader } from './loaders/database-loader.js';
74+
75+
// The manager logs on some paths; keep the run quiet and stable.
76+
const logger = vi.hoisted(() => ({
77+
info: vi.fn(),
78+
warn: vi.fn(),
79+
error: vi.fn(),
80+
debug: vi.fn(),
81+
}));
82+
83+
vi.mock('@objectstack/core', async (orig) => ({
84+
...((await orig()) as object),
85+
createLogger: () => logger,
86+
}));
87+
88+
/**
89+
* The card's container shape: the binding lives ONLY in the top-level `object`
90+
* field. `list.data` deliberately carries no `object`, so the pre-#13407
91+
* two-deep derivation chain (`list.data.object` -> `form.data.object`) cannot
92+
* find it and the fallback to the row's own name is not available either.
93+
*/
94+
const runtimeContainer = {
95+
object: 'crm_lead',
96+
list: {
97+
label: 'All Leads',
98+
type: 'grid',
99+
data: { provider: 'object' },
100+
columns: [{ field: 'name' }, { field: 'company' }],
101+
},
102+
listViews: {
103+
pipeline: {
104+
label: 'Lead Pipeline',
105+
type: 'kanban',
106+
data: { provider: 'object' },
107+
columns: ['name', 'company'],
108+
kanban: { groupByField: 'status' },
109+
},
110+
},
111+
// Deliberately NOT keyed `default`: the container's default `list` already
112+
// claims `<object>.default`, and a form competing for that name is renamed
113+
// to `…_2` with a diagnostic. That rename is real expansion behaviour, but
114+
// pinning it here would make this file a test of collision handling rather
115+
// than of the exit under repair.
116+
formViews: {
117+
edit: { type: 'simple', sections: [{ label: 'Info', fields: [{ field: 'name' }] }] },
118+
},
119+
};
120+
121+
/** What `runtimeContainer` expands to, sorted — the whole expected answer. */
122+
const EXPANDED = ['crm_lead.default', 'crm_lead.edit', 'crm_lead.pipeline'];
123+
124+
/** An already-independent ViewItem — the shape a source registrar produces. */
125+
const independentViewItem = {
126+
name: 'crm_lead.legacy',
127+
object: 'crm_lead',
128+
viewKind: 'list',
129+
config: { type: 'grid', columns: [{ field: 'name' }] },
130+
order: 0,
131+
scope: 'package',
132+
};
133+
134+
const names = (items: unknown[]): string[] =>
135+
(items as { name: string }[]).map((i) => i.name).sort();
136+
137+
/**
138+
* A `sys_metadata` store serving the rows it is handed. Minimal on purpose:
139+
* `DatabaseLoader.loadMany()` only reaches `syncSchema` and `find`.
140+
*/
141+
function storeServing(rows: Record<string, unknown>[]): IDataDriver {
142+
return {
143+
name: 'mock',
144+
version: '1.0.0',
145+
supports: {},
146+
connect: async (): Promise<void> => {},
147+
disconnect: async (): Promise<void> => {},
148+
syncSchema: async (): Promise<void> => {},
149+
find: async (): Promise<Record<string, unknown>[]> => rows,
150+
} as unknown as IDataDriver;
151+
}
152+
153+
function managerWithRegistryContainer(): MetadataManager {
154+
const manager = new MetadataManager({ formats: ['json'], loaders: [] });
155+
// How a container is keyed everywhere in this repo: under the bare object
156+
// name, with no top-level `name` of its own.
157+
manager.registerInMemory('view', 'crm_lead', runtimeContainer);
158+
return manager;
159+
}
160+
161+
/**
162+
* What every SOURCE registrar leaves behind: the container under the bare
163+
* object key, PLUS each expanded ViewItem under `<object>.<viewKey>`. Only one
164+
* of the three is registered here, so both halves are observable in one store —
165+
* the registered name must not be re-minted, and the two absent ones must be.
166+
*/
167+
function managerWithContainerAndRegisteredItem(): {
168+
manager: MetadataManager;
169+
registeredPipeline: Record<string, unknown>;
170+
} {
171+
const manager = managerWithRegistryContainer();
172+
const registeredPipeline = {
173+
name: 'crm_lead.pipeline',
174+
object: 'crm_lead',
175+
viewKind: 'list',
176+
config: { type: 'kanban' },
177+
order: 0,
178+
scope: 'package',
179+
_packageId: 'crm',
180+
};
181+
manager.registerInMemory('view', 'crm_lead.pipeline', registeredPipeline);
182+
return { manager, registeredPipeline };
183+
}
184+
185+
describe('#13913 getViewsByObject() expands aggregated view containers', () => {
186+
it('answers with the container EXPANSION, not empty', async () => {
187+
const manager = managerWithRegistryContainer();
188+
189+
const views = await manager.getViewsByObject('crm_lead');
190+
191+
// Pre-fix this was `[]`: the container carries no `viewKind`, so the filter
192+
// rejected the only row in the store.
193+
expect(views.length).toBeGreaterThan(0);
194+
expect(names(views)).toEqual(EXPANDED);
195+
});
196+
197+
it('answers with EXPANDED items and never the container itself (#7163)', async () => {
198+
const manager = managerWithRegistryContainer();
199+
200+
const views = (await manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
201+
202+
// Every answer is an independent ViewItem bound to the requested object.
203+
for (const v of views) {
204+
expect(v.viewKind === 'list' || v.viewKind === 'form').toBe(true);
205+
expect(v.object).toBe('crm_lead');
206+
}
207+
// The container is keyed `crm_lead` and has neither `viewKind` nor a
208+
// top-level `name`; loosening the filter to admit it is the regression this
209+
// pins against.
210+
expect(names(views)).not.toContain('crm_lead');
211+
expect(views.some((v) => v.list !== undefined || v.listViews !== undefined)).toBe(false);
212+
});
213+
214+
it('derives the binding from the top-level `object` when `list.data.object` is absent', async () => {
215+
const manager = managerWithRegistryContainer();
216+
217+
// Nothing binds to the container's registry KEY by accident: ask for an
218+
// object the container does not name and the answer stays empty.
219+
expect(await manager.getViewsByObject('crm_account')).toEqual([]);
220+
expect(names(await manager.getViewsByObject('crm_lead'))).toEqual(EXPANDED);
221+
});
222+
223+
it('reaches a container that arrived through a LOADER, not only the registry', async () => {
224+
const manager = new MetadataManager({ formats: ['json'], loaders: [] });
225+
manager.registerLoader(
226+
new DatabaseLoader({
227+
driver: storeServing([
228+
{
229+
id: 'r1',
230+
name: 'crm_lead',
231+
type: 'view',
232+
// A runtime-authored row: the stored body is the container itself.
233+
metadata: JSON.stringify({ name: 'crm_lead', ...runtimeContainer }),
234+
},
235+
]),
236+
cache: { enabled: false },
237+
}),
238+
);
239+
240+
expect(names(await manager.getViewsByObject('crm_lead'))).toEqual(EXPANDED);
241+
});
242+
243+
it('contributes only the names the store does not already hold', async () => {
244+
const { manager } = managerWithContainerAndRegisteredItem();
245+
246+
const views = (await manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
247+
248+
// One entry per named view — the registered `crm_lead.pipeline` is NOT
249+
// joined by a second, freshly-expanded copy of itself.
250+
expect(names(views)).toEqual(EXPANDED);
251+
expect(views.filter((v) => v.name === 'crm_lead.pipeline')).toHaveLength(1);
252+
});
253+
254+
// ------------------------------------------------------------------
255+
// Controls — green in BOTH directions, and measured so.
256+
//
257+
// These pin what this change must NOT move. A case going red here would be
258+
// reporting a regression, not this fix, so neither may depend on the
259+
// expansion existing.
260+
// ------------------------------------------------------------------
261+
262+
it('CONTROL: a non-container ViewItem still resolves unchanged', async () => {
263+
const manager = new MetadataManager({ formats: ['json'], loaders: [] });
264+
manager.registerInMemory('view', 'crm_lead.legacy', independentViewItem);
265+
266+
const views = await manager.getViewsByObject('crm_lead');
267+
268+
expect(views).toEqual([independentViewItem]);
269+
});
270+
271+
it('CONTROL: a registered expanded item is returned BY IDENTITY, never shadowed', async () => {
272+
const { manager, registeredPipeline } = managerWithContainerAndRegisteredItem();
273+
274+
const views = (await manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
275+
276+
// Object identity, not deep equality: the store's own fully-enriched copy
277+
// (it carries `_packageId`) is what comes back, not an expansion-minted
278+
// stand-in that happens to share a name.
279+
expect(views.find((v) => v.name === 'crm_lead.pipeline')).toBe(registeredPipeline);
280+
});
281+
});

0 commit comments

Comments
 (0)