fix(metadata): getViewsByObject() expands aggregated view containers instead of answering empty

.changeset/getviewsbyobject-expands-view-containers.md

@@ -0,0 +1,32 @@
+---
+"@objectstack/metadata": patch
+---
+
+fix(metadata): `getViewsByObject()` expands aggregated view containers instead of answering empty (#13913)
+
+`MetadataManager.getViewsByObject()` reads `this.list('view')` — the manager's
+own registry + loader store, which is a completely different store from the
+`sys_metadata` rows the REST route (`GET /meta/view?object=`) reads through
+`getMetaItems`. #13407 taught that route to expand a runtime-authored aggregated
+`defineView` container inline; this exit never called it and had no equivalent
+step, so a container the REST route now serves still answered **empty** here —
+for every internal/SDK caller that uses this entry point rather than the route.
+
+Getting the container into the store was never enough on its own: the filter
+also requires `viewKind`, and a container has none. Relaxing that requirement
+would answer with the container itself as a view — the behaviour #7163 ruled
+wrong — so the repair adds the container's **expansion**, whose items each carry
+the `viewKind` + `object` pair this filter has always tested. The filter is
+untouched; it reads the top-level `object` exactly as `ViewSchema.object`
+declares.
+
+The expansion is registry-free and per-read, mirroring #13407's choice at the
+other exit and for the same reason: the registry is process-wide, so a read must
+not graft rows into it. Already-present names win, so a container whose expanded
+ViewItems were registered by a source registrar (the ObjectQL boot loop, the
+artifact/HMR loader) still answers with those registered, fully-enriched items
+and gains nothing new.
+
+The object-derivation chain (`object` → `list.data.object` → `form.data.object`
+→ the row's own `name`) now has one spelling for this package, in the new
+`view-container-expansion.ts`, rather than a third private copy to fall behind.

packages/metadata/src/metadata-manager-views-by-object-container.test.ts

@@ -0,0 +1,281 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #13913 — `MetadataManager.getViewsByObject()` answered EMPTY for an
+ * aggregated view container living in this manager's own backing store.
+ *
+ * ---------------------------------------------------------------------------
+ * The defect, and why it survived #13407
+ * ---------------------------------------------------------------------------
+ * There are two independent object-bound readers. The REST route
+ * (`GET /meta/view?object=`) reads through `ObjectStackProtocolImplementation.
+ * getMetaItems` over `sys_metadata` rows; #13407 taught THAT reader to expand a
+ * runtime-authored container inline. `getViewsByObject()` reads
+ * `this.list('view')` — `MetadataManager`'s OWN registry + loader store, a
+ * completely separate store — never calls `getMetaItems`, and had no equivalent
+ * step. So the very container #13407 made visible on the wire still answered
+ * empty through this entry point.
+ *
+ * The filter here was never the bug: it reads the top-level `object`, exactly
+ * as `ViewSchema.object` declares. Two conditions have to hold at once, and the
+ * second is the one that decides the shape of the fix — the filter ALSO
+ * requires `viewKind`, and a container has none. Getting the container into the
+ * store is therefore not enough; the container's EXPANSION has to be what the
+ * read sees. Relaxing the `viewKind` requirement instead would answer with the
+ * container itself as a view — the behaviour #7163 ruled wrong — which is why
+ * `answers with EXPANDED items and never the container itself` below is a pin
+ * against that regression and not a restatement of the fix.
+ *
+ * ---------------------------------------------------------------------------
+ * What is driven, and why it is NOT the REST route
+ * ---------------------------------------------------------------------------
+ * Every assertion goes through `manager.getViewsByObject(...)` itself. A pin
+ * written against `GET /meta/view?object=` would have gone green on `main`
+ * without touching this bug at all — #13929 already repaired that exit.
+ *
+ * The container fixture is the card's own shape: a top-level `object` and NO
+ * `list.data.object`. That combination is what #13407's corrected derivation
+ * chain exists for, and it is why `container.object` has to be consulted first.
+ *
+ * ---------------------------------------------------------------------------
+ * The controls, and what the first ablation corrected about them
+ * ---------------------------------------------------------------------------
+ * The two `CONTROL:` cases are green in BOTH directions on purpose: this change
+ * must not move what the exit already answered, so a case going red there would
+ * report a regression rather than this fix. Neither may therefore depend on the
+ * expansion existing.
+ *
+ * The dedupe case was originally written as ONE control asserting both the full
+ * answer AND the identity of the registered item. The first ablation falsified
+ * that: it went RED, because pre-fix the answer is the registered item alone.
+ * A control that goes red under ablation is not a control, so it is split here
+ * — the SET assertion is an ordinary case (`contributes only the names the
+ * store does not already hold`, red under ablation), and the both-directions
+ * half is the object-IDENTITY assertion, which holds either way.
+ *
+ * Reverse verification, direction re-predicted after that split and recorded in
+ * the PR body as measured: reverting `getViewsByObject` to its pre-#13913 body
+ * turns the four container/expansion cases RED and leaves all three of the
+ * remaining cases GREEN.
+ *
+ * `answers with EXPANDED items and never the container itself (#7163)` is one
+ * of those three, and it is green under that reversion only VACUOUSLY — the
+ * pre-fix answer is empty, so nothing can be a container. It is a guard against
+ * a DIFFERENT mutation, and it was ablated separately against exactly that one:
+ * dropping `v.viewKind &&` from the filter (the tempting one-line "fix", which
+ * makes the container answer as a view) turns it RED. Measured, so the guard is
+ * known to fire rather than assumed to.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import type { IDataDriver } from '@objectstack/spec/contracts';
+import { MetadataManager } from './metadata-manager.js';
+import { DatabaseLoader } from './loaders/database-loader.js';
+
+// The manager logs on some paths; keep the run quiet and stable.
+const logger = vi.hoisted(() => ({
+  info: vi.fn(),
+  warn: vi.fn(),
+  error: vi.fn(),
+  debug: vi.fn(),
+}));
+
+vi.mock('@objectstack/core', async (orig) => ({
+  ...((await orig()) as object),
+  createLogger: () => logger,
+}));
+
+/**
+ * The card's container shape: the binding lives ONLY in the top-level `object`
+ * field. `list.data` deliberately carries no `object`, so the pre-#13407
+ * two-deep derivation chain (`list.data.object` -> `form.data.object`) cannot
+ * find it and the fallback to the row's own name is not available either.
+ */
+const runtimeContainer = {
+  object: 'crm_lead',
+  list: {
+    label: 'All Leads',
+    type: 'grid',
+    data: { provider: 'object' },
+    columns: [{ field: 'name' }, { field: 'company' }],
+  },
+  listViews: {
+    pipeline: {
+      label: 'Lead Pipeline',
+      type: 'kanban',
+      data: { provider: 'object' },
+      columns: ['name', 'company'],
+      kanban: { groupByField: 'status' },
+    },
+  },
+  // Deliberately NOT keyed `default`: the container's default `list` already
+  // claims `<object>.default`, and a form competing for that name is renamed
+  // to `…_2` with a diagnostic. That rename is real expansion behaviour, but
+  // pinning it here would make this file a test of collision handling rather
+  // than of the exit under repair.
+  formViews: {
+    edit: { type: 'simple', sections: [{ label: 'Info', fields: [{ field: 'name' }] }] },
+  },
+};
+
+/** What `runtimeContainer` expands to, sorted — the whole expected answer. */
+const EXPANDED = ['crm_lead.default', 'crm_lead.edit', 'crm_lead.pipeline'];
+
+/** An already-independent ViewItem — the shape a source registrar produces. */
+const independentViewItem = {
+  name: 'crm_lead.legacy',
+  object: 'crm_lead',
+  viewKind: 'list',
+  config: { type: 'grid', columns: [{ field: 'name' }] },
+  order: 0,
+  scope: 'package',
+};
+
+const names = (items: unknown[]): string[] =>
+  (items as { name: string }[]).map((i) => i.name).sort();
+
+/**
+ * A `sys_metadata` store serving the rows it is handed. Minimal on purpose:
+ * `DatabaseLoader.loadMany()` only reaches `syncSchema` and `find`.
+ */
+function storeServing(rows: Record<string, unknown>[]): IDataDriver {
+  return {
+    name: 'mock',
+    version: '1.0.0',
+    supports: {},
+    connect: async (): Promise<void> => {},
+    disconnect: async (): Promise<void> => {},
+    syncSchema: async (): Promise<void> => {},
+    find: async (): Promise<Record<string, unknown>[]> => rows,
+  } as unknown as IDataDriver;
+}
+
+function managerWithRegistryContainer(): MetadataManager {
+  const manager = new MetadataManager({ formats: ['json'], loaders: [] });
+  // How a container is keyed everywhere in this repo: under the bare object
+  // name, with no top-level `name` of its own.
+  manager.registerInMemory('view', 'crm_lead', runtimeContainer);
+  return manager;
+}
+
+/**
+ * What every SOURCE registrar leaves behind: the container under the bare
+ * object key, PLUS each expanded ViewItem under `<object>.<viewKey>`. Only one
+ * of the three is registered here, so both halves are observable in one store —
+ * the registered name must not be re-minted, and the two absent ones must be.
+ */
+function managerWithContainerAndRegisteredItem(): {
+  manager: MetadataManager;
+  registeredPipeline: Record<string, unknown>;
+} {
+  const manager = managerWithRegistryContainer();
+  const registeredPipeline = {
+    name: 'crm_lead.pipeline',
+    object: 'crm_lead',
+    viewKind: 'list',
+    config: { type: 'kanban' },
+    order: 0,
+    scope: 'package',
+    _packageId: 'crm',
+  };
+  manager.registerInMemory('view', 'crm_lead.pipeline', registeredPipeline);
+  return { manager, registeredPipeline };
+}
+
+describe('#13913 getViewsByObject() expands aggregated view containers', () => {
+  it('answers with the container EXPANSION, not empty', async () => {
+    const manager = managerWithRegistryContainer();
+
+    const views = await manager.getViewsByObject('crm_lead');
+
+    // Pre-fix this was `[]`: the container carries no `viewKind`, so the filter
+    // rejected the only row in the store.
+    expect(views.length).toBeGreaterThan(0);
+    expect(names(views)).toEqual(EXPANDED);
+  });
+
+  it('answers with EXPANDED items and never the container itself (#7163)', async () => {
+    const manager = managerWithRegistryContainer();
+
+    const views = (await manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
+
+    // Every answer is an independent ViewItem bound to the requested object.
+    for (const v of views) {
+      expect(v.viewKind === 'list' || v.viewKind === 'form').toBe(true);
+      expect(v.object).toBe('crm_lead');
+    }
+    // The container is keyed `crm_lead` and has neither `viewKind` nor a
+    // top-level `name`; loosening the filter to admit it is the regression this
+    // pins against.
+    expect(names(views)).not.toContain('crm_lead');
+    expect(views.some((v) => v.list !== undefined || v.listViews !== undefined)).toBe(false);
+  });
+
+  it('derives the binding from the top-level `object` when `list.data.object` is absent', async () => {
+    const manager = managerWithRegistryContainer();
+
+    // Nothing binds to the container's registry KEY by accident: ask for an
+    // object the container does not name and the answer stays empty.
+    expect(await manager.getViewsByObject('crm_account')).toEqual([]);
+    expect(names(await manager.getViewsByObject('crm_lead'))).toEqual(EXPANDED);
+  });
+
+  it('reaches a container that arrived through a LOADER, not only the registry', async () => {
+    const manager = new MetadataManager({ formats: ['json'], loaders: [] });
+    manager.registerLoader(
+      new DatabaseLoader({
+        driver: storeServing([
+          {
+            id: 'r1',
+            name: 'crm_lead',
+            type: 'view',
+            // A runtime-authored row: the stored body is the container itself.
+            metadata: JSON.stringify({ name: 'crm_lead', ...runtimeContainer }),
+          },
+        ]),
+        cache: { enabled: false },
+      }),
+    );
+
+    expect(names(await manager.getViewsByObject('crm_lead'))).toEqual(EXPANDED);
+  });
+
+  it('contributes only the names the store does not already hold', async () => {
+    const { manager } = managerWithContainerAndRegisteredItem();
+
+    const views = (await manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
+
+    // One entry per named view — the registered `crm_lead.pipeline` is NOT
+    // joined by a second, freshly-expanded copy of itself.
+    expect(names(views)).toEqual(EXPANDED);
+    expect(views.filter((v) => v.name === 'crm_lead.pipeline')).toHaveLength(1);
+  });
+
+  // ------------------------------------------------------------------
+  // Controls — green in BOTH directions, and measured so.
+  //
+  // These pin what this change must NOT move. A case going red here would be
+  // reporting a regression, not this fix, so neither may depend on the
+  // expansion existing.
+  // ------------------------------------------------------------------
+
+  it('CONTROL: a non-container ViewItem still resolves unchanged', async () => {
+    const manager = new MetadataManager({ formats: ['json'], loaders: [] });
+    manager.registerInMemory('view', 'crm_lead.legacy', independentViewItem);
+
+    const views = await manager.getViewsByObject('crm_lead');
+
+    expect(views).toEqual([independentViewItem]);
+  });
+
+  it('CONTROL: a registered expanded item is returned BY IDENTITY, never shadowed', async () => {
+    const { manager, registeredPipeline } = managerWithContainerAndRegisteredItem();
+
+    const views = (await manager.getViewsByObject('crm_lead')) as Record<string, unknown>[];
+
+    // Object identity, not deep equality: the store's own fully-enriched copy
+    // (it carries `_packageId`) is what comes back, not an expansion-minted
+    // stand-in that happens to share a name.
+    expect(views.find((v) => v.name === 'crm_lead.pipeline')).toBe(registeredPipeline);
+  });
+});