fix(app-shell): switching a report to joined clears the container binding it hides (objectui#10746)

.changeset/10746-joined-report-clears-binding.md

@@ -0,0 +1,31 @@
+---
+'@object-ui/app-shell': patch
+---
+
+fix(app-shell): switching a report to `joined` clears the container binding it hides (objectui#10746)
+
+Studio's report inspector committed `{ type: 'joined' }` alone when the type picker moved
+to `joined`, and in the same render hid its dataset / values / rows / columns / chart
+controls, while the spec's own `reportForm` hid the whole "Dataset binding" section
+(`order` included) through `visibleWhen: "data.type != 'joined'"`. A report that was
+bound first and switched second kept every one of those keys invisibly. `ReportSchema`'s
+joined arm refuses a container `dataset` / `rows` / `columns` / `values` (objectstack PR
+#20160: "a `joined` report selects per block — move `KEY` onto `blocks[]`, or delete it")
+and has always refused a container `order`, so the save was refused at a path no control
+on the Properties tab could reach; only the JSON source tab could delete the key.
+
+The type picker now drops `dataset`, `values`, `rows`, `columns`, `chart` and `order`
+in the same patch that commits `type: 'joined'` — an `undefined`-valued key, the same
+spelling the inspector's own chart panel and its sibling inspectors already clear with,
+which the host's shallow spread turns into an own key holding `undefined` and
+`JSON.stringify` omits on the wire. Only keys the draft actually carries are named, so an
+unbound report's switch stays the one-key patch it always was. `runtimeFilter` and
+`drilldown` are kept: the joined branch reads both. Switching between two non-joined
+types keeps the binding; switching away from `joined` restores nothing — the author
+re-binds. `blocks` is never touched. The clear holds for the patch and for the save that
+follows it. The metadata-admin editor rebuilds its draft as the served draft spread over
+`layered.effective` (on load, after each save and after publish), and `effective` is the
+published layer, so a report whose PUBLISHED version was bound gets those keys back in the
+draft after the first draft save until it is published, and a report already saved `joined`
+with stale keys is not repaired on load. Both are the host's draft-over-baseline merge,
+objectui#10765.

packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.joinedClearsBinding-10746.test.tsx

@@ -0,0 +1,228 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * objectui#10746 — switching a bound report's type to `joined` CLEARS the
+ * container's selection keys in the same patch, instead of only hiding them.
+ *
+ * A joined report selects per block: each block binds its own `dataset` and
+ * picks its own `rows` / `columns` / `values`. `ReportSchema`'s joined arm
+ * therefore refuses those four keys on the container — objectstack PR #20160,
+ * `JOINED_CONTAINER_SELECTION_KEYS` in `packages/spec/src/ui/report.zod.ts`:
+ * "a `joined` report selects per block — move `KEY` onto `blocks[]`, or delete
+ * it; on the container it selects nothing." — and has always refused a
+ * container `order` ("a `joined` report orders per block — move `order` onto
+ * `blocks[]`."). `chart` is inert on a joined container (objectstack#20161).
+ *
+ * The type picker used to commit `{ type: 'joined' }` and nothing else. In the
+ * same render this inspector hides its dataset / values / rows / columns /
+ * chart controls (`datasetBound`), and the spec's own `reportForm` hides its
+ * whole "Dataset binding" section — `order` included — through
+ * `visibleWhen: "data.type != 'joined'"`. A report bound first and switched
+ * second kept every one of those keys INVISIBLY; its save was then refused at a
+ * path no control on the Properties tab could reach.
+ *
+ * What the installed `@objectstack/spec` can and cannot measure here: 17.4.0 at
+ * the time of writing predates PR #20160, so it already refuses a container
+ * `order` and still ACCEPTS the four selection keys. The parse leg below shows
+ * the `order` half of the refusal going away; for the four keys the pins assert
+ * ABSENCE, against the rule quoted above. ⛔ Never weaken those to "parses under
+ * the installed spec" — that read green on the defect too.
+ *
+ * ⚠️ Assertion spelling. `toHaveBeenCalledWith` and `toEqual` treat an
+ * `undefined`-valued key as absent, so `{ type: 'joined', dataset: undefined }`
+ * satisfies `toHaveBeenCalledWith({ type: 'joined' })` — the defect and the fix
+ * read alike under them. Every patch here is read with `toStrictEqual`, which
+ * tells an own key holding `undefined` from a missing one.
+ */
+
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, screen, cleanup } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { ReportSchema } from '@objectstack/spec/ui';
+import { ReportDefaultInspector } from './ReportDefaultInspector';
+import type { DatasetCatalogEntry } from '../previews/useDatasetCatalog';
+
+afterEach(cleanup);
+
+// One fetch double at MODULE scope, never torn down — the same reason as in
+// `ReportDefaultInspector.test.tsx` (objectui#7439 / #6640): `useDatasetSemantics`
+// fires a fire-and-forget read that no test body awaits.
+vi.stubGlobal(
+  'fetch',
+  vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })),
+);
+
+const catalog: DatasetCatalogEntry[] = [
+  {
+    name: 'sales_metrics',
+    label: 'Sales metrics',
+    dimensions: [
+      { name: 'stage', type: 'text' },
+      { name: 'close_quarter', type: 'date' },
+    ],
+    measures: [
+      { name: 'total_amount', aggregate: 'sum' },
+      { name: 'deal_count', aggregate: 'count' },
+    ],
+  },
+];
+
+const baseProps = {
+  type: 'report',
+  name: 'pipeline',
+  locale: 'en-US' as const,
+  onSelectionChange: vi.fn(),
+  datasetCatalogOverride: catalog,
+};
+
+/** The six keys the type switch clears, in the order the inspector names them. */
+const CLEARED = ['dataset', 'values', 'rows', 'columns', 'chart', 'order'] as const;
+/** The four the spec's joined arm refuses on the container (objectstack PR #20160). */
+const SPEC_REFUSED_SELECTION = ['dataset', 'rows', 'columns', 'values'] as const;
+
+/** One dataset-bound block: where a joined report's data actually lives. */
+const block = { name: 'won_deals', dataset: 'sales_metrics', rows: ['stage'], values: ['total_amount'] };
+
+/**
+ * A matrix report bound the way the Properties tab binds one: every curated
+ * control has written its key, the spec form has written `order`, and the two
+ * keys the joined branch READS (`runtimeFilter`, `drilldown`) are set so the
+ * pins can show them surviving the switch.
+ */
+const boundDraft = {
+  name: 'pipeline',
+  label: 'Pipeline',
+  type: 'matrix',
+  dataset: 'sales_metrics',
+  values: ['total_amount'],
+  rows: ['stage'],
+  columns: ['close_quarter'],
+  chart: { type: 'bar', xAxis: 'stage', yAxis: 'total_amount' },
+  order: [{ by: 'total_amount' }],
+  runtimeFilter: { stage: 'won' },
+  drilldown: false,
+};
+
+type Patch = Record<string, unknown>;
+
+/** Mount on `draft`, pick `optionLabel` in the Report type control, hand back the one patch. */
+async function switchType(draft: Record<string, unknown>, optionLabel: string): Promise<Patch> {
+  const onPatch = vi.fn<(patch: Patch) => void>();
+  render(<ReportDefaultInspector {...baseProps} draft={draft} onPatch={onPatch} readOnly={false} />);
+  await userEvent.click(screen.getByRole('combobox', { name: 'Report type' }));
+  await userEvent.click(await screen.findByRole('option', { name: optionLabel }));
+  expect(onPatch, 'the type picker commits exactly one patch').toHaveBeenCalledTimes(1);
+  return onPatch.mock.calls[0][0];
+}
+
+/**
+ * The host's merge, as all three hosts spell it (`ResourceEditPage`,
+ * `ReportConfigPanel`, `StudioDesignSurface`) — `ResourceEditPage` passes
+ * `handleDraftChange((d) => ({ ...d, ...patch }))` and `ReportConfigPanel`'s
+ * `handlePatch` computes `{ ...draftRef.current, ...patch }`: a shallow spread.
+ */
+const merge = (draft: Record<string, unknown>, patch: Patch): Record<string, unknown> => ({ ...draft, ...patch });
+
+/** What reaches the wire: the draft JSON-encoded, which is how `client.save` sends a body. */
+const serialised = (doc: Record<string, unknown>): Record<string, unknown> => JSON.parse(JSON.stringify(doc));
+
+describe('ReportDefaultInspector — switching to `joined` clears the container binding (objectui#10746)', () => {
+  it('THE DEFECT: a bound report switched to `joined` loses dataset / values / rows / columns / chart / order in the SAME patch', async () => {
+    const patch = await switchType(boundDraft, 'Joined');
+    expect(patch).toStrictEqual({
+      type: 'joined',
+      dataset: undefined,
+      values: undefined,
+      rows: undefined,
+      columns: undefined,
+      chart: undefined,
+      order: undefined,
+    });
+  });
+
+  it('a cleared key is an own key holding `undefined` on the draft — not `null`, not `""` — and is ABSENT once serialised', async () => {
+    const patch = await switchType(boundDraft, 'Joined');
+    const committed = merge(boundDraft, patch);
+    for (const key of CLEARED) {
+      expect(Object.hasOwn(committed, key), `${key} is an own key after the spread`).toBe(true);
+      expect(committed[key], `${key} holds undefined, the one value JSON omits`).toBeUndefined();
+    }
+    const document = serialised(committed);
+    for (const key of CLEARED) expect(document).not.toHaveProperty(key);
+    // The keys the joined branch READS travel through untouched.
+    expect(document).toStrictEqual({
+      name: 'pipeline',
+      label: 'Pipeline',
+      type: 'joined',
+      runtimeFilter: { stage: 'won' },
+      drilldown: false,
+    });
+  });
+
+  it('the parse leg the installed spec can measure: the container `order` refusal goes away; the four selection keys are asserted absent against the spec rule', async () => {
+    const draft = { ...boundDraft, blocks: [block] };
+    const patch = await switchType(draft, 'Joined');
+    // What the defect committed: the type alone, every stale key kept.
+    const before = ReportSchema.safeParse(serialised(merge(draft, { type: 'joined' })));
+    expect(
+      before.success,
+      'INSTRUMENT CONTROL: the installed spec refuses a container `order` on a joined report',
+    ).toBe(false);
+    if (!before.success) {
+      const orderIssue = before.error.issues.find((i) => i.path[0] === 'order');
+      expect(orderIssue?.message).toMatch(/^a `joined` report orders per block/);
+    }
+    const after = serialised(merge(draft, patch));
+    for (const key of SPEC_REFUSED_SELECTION) {
+      expect(
+        after,
+        `\`${key}\` — "a \`joined\` report selects per block — move \`${key}\` onto \`blocks[]\`, or delete it"`,
+      ).not.toHaveProperty(key);
+    }
+    expect(after).not.toHaveProperty('order');
+    const parsed = ReportSchema.safeParse(after);
+    expect(parsed.success, JSON.stringify(parsed.success ? null : parsed.error.issues)).toBe(true);
+  });
+
+  it('names only the keys the draft carries: a partially bound report yields exactly those', async () => {
+    const patch = await switchType(
+      { name: 'pipeline', label: 'Pipeline', type: 'summary', dataset: 'sales_metrics', values: ['total_amount'] },
+      'Joined',
+    );
+    expect(patch).toStrictEqual({ type: 'joined', dataset: undefined, values: undefined });
+  });
+
+  it('BOUNDARY: an unbound report switching to `joined` stays the one-key patch it always was', async () => {
+    const patch = await switchType({ name: 'pipeline', label: 'Pipeline', type: 'tabular' }, 'Joined');
+    expect(patch).toStrictEqual({ type: 'joined' });
+  });
+
+  it('CONTROL: switching between two non-joined types keeps the binding', async () => {
+    const patch = await switchType(boundDraft, 'Summary');
+    expect(patch).toStrictEqual({ type: 'summary' });
+    expect(serialised(merge(boundDraft, patch))).toMatchObject({
+      dataset: 'sales_metrics',
+      values: ['total_amount'],
+      rows: ['stage'],
+      columns: ['close_quarter'],
+      chart: { type: 'bar', xAxis: 'stage', yAxis: 'total_amount' },
+      order: [{ by: 'total_amount' }],
+    });
+  });
+
+  it('CONTROL: `blocks` is untouched when the type becomes `joined`', async () => {
+    const draft = { ...boundDraft, blocks: [block] };
+    const patch = await switchType(draft, 'Joined');
+    expect(patch).not.toHaveProperty('blocks');
+    expect(merge(draft, patch).blocks, 'the same array, not a copy').toBe(draft.blocks);
+  });
+
+  it('`joined` → non-joined restores nothing: the patch is the type alone, `blocks` stays, and the author re-binds', async () => {
+    const draft = { name: 'multi', label: 'Multi', type: 'joined', blocks: [block] };
+    const patch = await switchType(draft, 'Summary');
+    expect(patch).toStrictEqual({ type: 'summary' });
+    const committed = merge(draft, patch);
+    expect(committed.blocks).toBe(draft.blocks);
+    for (const key of CLEARED) expect(committed).not.toHaveProperty(key);
+  });
+});

packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx

@@ -71,6 +71,33 @@ const REPORT_CURATED_FIELDS = new Set([
   'chart', // dedicated Chart panel below (type + dataset-aware X/Y pickers)
 ]);
 
+/**
+ * Top-level keys dropped from the container in the same patch that commits
+ * `type: 'joined'` (objectui#10746). A joined report selects per block — each
+ * block binds its own `dataset` and picks its own `rows` / `columns` /
+ * `values` — so `ReportSchema`'s joined arm refuses the four selection keys on
+ * the container ("a `joined` report selects per block — move `KEY` onto
+ * `blocks[]`, or delete it", objectstack PR #20160) and has always refused a
+ * container `order` ("a `joined` report orders per block"). `chart` is inert
+ * on a joined container (objectstack#20161) and is hidden by the same switch.
+ *
+ * Why CLEAR rather than un-hide: the moment the type becomes `joined` this
+ * inspector hides its dataset / values / rows / columns / chart controls
+ * (`datasetBound` below) and the spec's own `reportForm` hides its whole
+ * "Dataset binding" section — `order` included — through
+ * `visibleWhen: "data.type != 'joined'"`. A report that was bound first and
+ * switched second kept those keys INVISIBLY, and its save was then refused at
+ * a path no control on the Properties tab could reach.
+ *
+ * The clear is an `undefined`-valued key in the shallow patch — the spelling
+ * `commitChart` below and the sibling inspectors already clear with. The host
+ * spreads the patch over the draft, so the key becomes an own property holding
+ * `undefined`, which `JSON.stringify` omits on the wire and which the spec's
+ * refinement skips. `runtimeFilter` and `drilldown` are deliberately NOT here:
+ * the joined branch reads both.
+ */
+const JOINED_CONTAINER_CLEARED_KEYS = ['dataset', 'values', 'rows', 'columns', 'chart', 'order'] as const;
+
 /**
  * Chart types offered in the curated Chart panel. A dataset-bound report plots
  * one measure (yAxis) across one dimension (xAxis), so we surface the families
@@ -327,6 +354,23 @@ export function ReportDefaultInspector({
   // the spec form's repeater) — the top-level binding only applies otherwise.
   const datasetBound = reportType !== 'joined';
 
+  // objectui#10746 — see `JOINED_CONTAINER_CLEARED_KEYS`. Only keys the draft
+  // carries are named, so a host that mirrors each patched key to a live
+  // preview (`ReportConfigPanel`'s `onFieldChange`) sees no phantom clears and
+  // an unbound report's switch stays the one-key patch it always was.
+  // Switching AWAY from `joined` restores nothing: the binding was dropped when
+  // the type left, `onPatch` has no undo stack behind it, and the author
+  // re-binds. `blocks` is never touched in either direction.
+  const commitType = (nextType: string) => {
+    const patch: Record<string, unknown> = { type: nextType };
+    if (nextType === 'joined') {
+      for (const key of JOINED_CONTAINER_CLEARED_KEYS) {
+        if (draft[key] !== undefined) patch[key] = undefined;
+      }
+    }
+    onPatch(patch);
+  };
+
   // Graft any server-only top-level fields onto the bundled-spec form so they
   // are directly editable here even when the bundled `@objectstack/spec` lags
   // the running server (skew root-cure).
@@ -388,7 +432,7 @@ export function ReportDefaultInspector({
         label={tr('engine.inspector.report.type')}
         value={reportType}
         options={typeOptions}
-        onCommit={(v) => onPatch({ type: v })}
+        onCommit={commitType}
         disabled={readOnly}
       />