fix(react/plugins): the record-page URL follows the record source, not the top-level key

.changeset/7638-navigation-url-follows-record-source.md

@@ -0,0 +1,48 @@
+---
+'@object-ui/plugin-calendar': patch
+'@object-ui/plugin-tree': patch
+'@object-ui/react': patch
+---
+
+A record-page URL now names the object the clicked rows actually came from, in
+`ObjectTree` and `ObjectCalendar` (objectui#7638).
+
+`useNavigationOverlay` builds `/{objectName}/record/{id}` out of whatever it is handed,
+and both components handed it the bare top-level `schema.objectName` while resolving
+their own rows through the objectui#6939 record-source ladder (`data`, then
+`staticData`, then `objectName`). objectui#6939 published `objectName` as that ladder's
+THIRD RUNG and not as a parallel "page object" concept, so a block has exactly one
+record source — and a row fetched through `data.object` whose click built
+`/{schema.objectName}/record/{id}` named a record that the URL's own object does not
+contain.
+
+Two shapes change, both toward the object the rows came from:
+
+- a block carrying **both** bindings navigated to the top-level key and now navigates to
+  `data.object`;
+- a **data-only** block had no name to build a URL from at all, so the hook took its
+  `/{id}` leg — an unrouted path that paints a blank page — and now builds the routed
+  record URL.
+
+`ObjectCalendar` is where the divergence was plainest: on one click it resolved the
+detail drawer through the ladder and the navigation URL through the top-level key. The
+URL now reuses the very `schemaObjectName` that already keys the calendar's record query
+and its `$expand` derivation, so query, drawer and URL agree by construction.
+
+**Nothing else moves.** Both converted sites keep a site-local `?? schema.objectName`
+tail for the off-contract `data: { provider: 'object' }` that carries no `object`
+(`ViewDataSchema` declares it required) — the same tail `ObjectTree`'s `headerObjectName`
+already carries, and the same conservatism objectui#7627 applied when it published the
+shared reader. `useNavigationOverlay`'s own signature is unchanged: it still takes an
+`objectName`, and only what callers hand it has changed.
+
+The hook's `@example` stops prescribing `objectName: schema.objectName`. That prose is
+why there were copies to convert at all — component authors copied the divergence out of
+the documentation, correctly, as written — so it now points at
+`resolveRecordSourceObjectName` and says explicitly that a caller with no data config
+has nothing above rung three and should keep passing `schema.objectName`.
+
+`ObjectKanban` is deliberately **not** converted: it has no data config, no
+`getDataConfig`, and its `data` is a raw row array rather than a `ViewData` binding, so
+`schema.objectName` already IS its record source and its board, drawer and URL already
+agree.

packages/plugin-calendar/src/ObjectCalendar.tsx

@@ -622,7 +622,21 @@ export const ObjectCalendar: React.FC<ObjectCalendarComponentProps> = ({
   const navIsOverlay = navConfig.mode === 'drawer' || navConfig.mode === 'modal' || navConfig.mode === 'split' || navConfig.mode === 'popover';
   const navigation = useNavigationOverlay({
     navigation: navConfig,
-    objectName: schema.objectName,
+    // The record-page URL follows the RECORD SOURCE (objectui#7638): the very
+    // `schemaObjectName` resolved above, which already keys this calendar's
+    // record query and which the detail drawer at the bottom of this file
+    // resolves the same way. Before this it read the bare `schema.objectName`,
+    // so ONE click resolved the drawer through the objectui#6939 ladder and the
+    // navigation URL through the top-level key — two receivers, one gesture,
+    // two different objects.
+    //
+    // The `?? schema.objectName` tail is NOT the shared rung repeated: it is
+    // this site's own coercion of the OFF-CONTRACT `data: { provider: 'object' }`
+    // that carries no `object` (`ViewDataSchema` declares it required), and it
+    // is here so this conversion changes nothing this component navigates to
+    // today EXCEPT the divergence it closes. `ObjectTree`'s converted site and
+    // `headerObjectName` both keep the same tail for the same reason.
+    objectName: schemaObjectName ?? schema.objectName,
     onRowClick: navIsOverlay ? undefined : onRowClick,
   });
 

packages/plugin-calendar/src/__tests__/ObjectCalendar.navigationRecordSource-7638.test.tsx

@@ -0,0 +1,141 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#7638 — `ObjectCalendar`'s record-page URL follows the RECORD SOURCE.
+ *
+ * ## The divergence this closes, which THIS component proves was accidental
+ *
+ * On a single click this calendar used to resolve two receivers two different
+ * ways: the detail drawer's `objectName` through the objectui#6939
+ * record-source ladder, and the navigation URL through the bare top-level
+ * `schema.objectName`. Two receivers, one gesture, two different objects —
+ * which is the evidence that the divergence was a copy, not a design.
+ *
+ * The same `schemaObjectName` that keys this calendar's record query and its
+ * `$expand` derivation now also builds the URL, so all three agree by
+ * construction rather than by coincidence.
+ *
+ * ## What is asserted, and why not a spy on the resolver
+ *
+ * The observable is the URL a click actually navigates to, so every case drives
+ * a real event click through the real `new_window` branch and reads
+ * `window.open`'s first argument. Asserting `resolveRecordSourceObjectName` was
+ * CALLED would pass equally well with its result discarded.
+ *
+ * `new_window` is the mode under test because it is the branch that builds the
+ * URL in-process — the overlay modes open the drawer and build none, and the
+ * `page` branch delegates to an `onNavigate` this call site never passes.
+ *
+ * ## The lit control
+ *
+ * The first case carries no `data` block, so rung three IS its record source
+ * and `/appointments/record/e1` is both the old answer and the new one. It is
+ * the instrument check and it must read NON-ZERO: a `window.open` that never
+ * fires would make every "did not navigate to the decoy" assertion below
+ * vacuously true, and this file would be a dark instrument reporting green.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
+import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ObjectCalendar } from '../ObjectCalendar';
+
+/** The object the rows really came from — what the URL must name. */
+const RECORD_SOURCE = 'clinic_visit';
+/** The top-level key, rung three — the decoy the URL must stop naming. */
+const DECOY = 'appointments';
+
+/**
+ * One event on TODAY, so it lands in the default month view without the test
+ * having to drive the calendar's navigation controls first.
+ */
+const TODAY = new Date();
+const ROWS = [{ id: 'e1', name: 'Follow-up', starts_at: TODAY.toISOString() }];
+
+function renderCalendar(schema: Record<string, unknown>) {
+  return render(
+    <ObjectCalendar
+      schema={
+        {
+          type: 'object-calendar',
+          calendar: { startDateField: 'starts_at', titleField: 'name' },
+          navigation: { mode: 'new_window' },
+          ...schema,
+        } as never
+      }
+      data={ROWS as never}
+    />,
+  );
+}
+
+/** Click the event and hand back the URL `window.open` was given. */
+async function clickEventAndReadUrl(
+  open: ReturnType<typeof vi.spyOn>,
+): Promise<string | undefined> {
+  const event = await screen.findByText('Follow-up');
+  fireEvent.click(event);
+  await waitFor(() => expect(open).toHaveBeenCalled());
+  return open.mock.calls[0]?.[0] as string | undefined;
+}
+
+let open: ReturnType<typeof vi.spyOn>;
+
+beforeEach(() => {
+  open = vi.spyOn(window, 'open').mockImplementation(() => null);
+});
+
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+});
+
+describe('ObjectCalendar navigation URL follows the record source (objectui#7638)', () => {
+  it('LIT CONTROL: with no `data` block, rung three IS the record source and still builds the URL', async () => {
+    renderCalendar({ objectName: DECOY });
+
+    // Reads non-zero, or every assertion below is vacuous.
+    expect(await clickEventAndReadUrl(open)).toBe(`/${DECOY}/record/e1`);
+    expect(open).toHaveBeenCalledWith(`/${DECOY}/record/e1`, '_blank');
+  });
+
+  it('navigates to the object the ROWS came from, not the top-level key', async () => {
+    renderCalendar({
+      objectName: DECOY,
+      data: { provider: 'object', object: RECORD_SOURCE },
+    });
+
+    const url = await clickEventAndReadUrl(open);
+    expect(url).toBe(`/${RECORD_SOURCE}/record/e1`);
+    // The whole finding in one line: before objectui#7638 this was the answer,
+    // while the drawer on the very same click resolved `clinic_visit`.
+    expect(url).not.toBe(`/${DECOY}/record/e1`);
+  });
+
+  it('builds a routed URL for a data-only block, which previously had no name to use', async () => {
+    // No top-level `objectName` at all, so `schema.objectName` was `undefined`
+    // and the hook took its `/${encodedId}` leg — an unrouted path that paints
+    // a blank page.
+    renderCalendar({ data: { provider: 'object', object: RECORD_SOURCE } });
+
+    const url = await clickEventAndReadUrl(open);
+    expect(url).toBe(`/${RECORD_SOURCE}/record/e1`);
+    expect(url).not.toBe('/e1');
+  });
+
+  it('keeps the `?? schema.objectName` tail for the OFF-CONTRACT `{ provider: "object" }`', async () => {
+    // `ViewDataSchema` declares `object` REQUIRED on the `object` provider, so
+    // this shape is off-contract and the shared reader returns `undefined` for
+    // it rather than coercing. The site keeps its own tail, so this conversion
+    // changes nothing this component navigates to today.
+    renderCalendar({ objectName: DECOY, data: { provider: 'object' } });
+
+    expect(await clickEventAndReadUrl(open)).toBe(`/${DECOY}/record/e1`);
+  });
+});

packages/plugin-tree/src/ObjectTree.navigationRecordSource-7638.test.tsx

@@ -0,0 +1,153 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#7638 — `ObjectTree`'s record-page URL follows the RECORD SOURCE.
+ *
+ * ## The divergence this closes
+ *
+ * `useNavigationOverlay` builds `/{objectName}/record/{id}` out of whatever it
+ * is handed. This component used to hand it the bare top-level
+ * `schema.objectName` while resolving its own rows — and its column labels
+ * (`headerObjectName`) — through the objectui#6939 record-source ladder
+ * (`data`, then `staticData`, then `objectName`). objectui#6939 published
+ * `objectName` as that ladder's THIRD RUNG and not as a parallel "page object"
+ * concept, so a block has exactly ONE record source: a row fetched through
+ * `data.object` whose click built `/{schema.objectName}/record/{id}` named a
+ * record that the URL's own object does not contain.
+ *
+ * ## Why these assertions and not a spy on the resolver
+ *
+ * The observable under test is the URL a user's click actually navigates to, so
+ * every case here drives a real click through the real `new_window` branch and
+ * reads `window.open`'s first argument. Asserting that
+ * `resolveRecordSourceObjectName` was CALLED would pass just as well with its
+ * result thrown away.
+ *
+ * `new_window` is the mode chosen because it is the branch that builds the URL
+ * in-process; the `page` branch delegates to an `onNavigate` this component
+ * never passes, and the overlay modes never build a URL at all.
+ *
+ * ## The lit control
+ *
+ * The first case carries NO `data` block, so rung three is the record source
+ * and `/business_unit/record/1` is both the old and the new answer. It is here
+ * as the instrument check: it is the case that must read NON-ZERO — a
+ * `window.open` that is never called at all would make every "did not navigate
+ * to the decoy" assertion below vacuously true. If that case ever goes silent,
+ * the rest of this file is a dark instrument and its greens mean nothing.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
+import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ObjectTree } from './ObjectTree';
+
+/**
+ * Rows reach the component through the `data` PROP, so no `dataSource` is
+ * needed and the fetch effect falls through to them for every schema shape
+ * below. That keeps the only variable across these cases the thing under test:
+ * which object the schema NAMES.
+ */
+const ROWS = [
+  { id: '1', name: 'Acme', parent_id: null },
+  { id: '2', name: 'Engineering', parent_id: '1' },
+];
+
+/** The object the rows really came from — what the URL must name. */
+const RECORD_SOURCE = 'org_chart_node';
+/** The top-level key, rung three — the decoy the URL must stop naming. */
+const DECOY = 'business_unit';
+
+function renderTree(schema: Record<string, unknown>) {
+  return render(
+    <ObjectTree
+      schema={
+        {
+          type: 'object-tree',
+          parentField: 'parent_id',
+          labelField: 'name',
+          fields: ['name'],
+          navigation: { mode: 'new_window' },
+          ...schema,
+        } as never
+      }
+      data={ROWS}
+    />,
+  );
+}
+
+/** Click the first row and hand back the URL `window.open` was given. */
+async function clickRowAndReadUrl(open: ReturnType<typeof vi.spyOn>): Promise<string | undefined> {
+  const cell = await screen.findByText('Acme');
+  fireEvent.click(cell);
+  await waitFor(() => expect(open).toHaveBeenCalled());
+  return open.mock.calls[0]?.[0] as string | undefined;
+}
+
+let open: ReturnType<typeof vi.spyOn>;
+
+beforeEach(() => {
+  open = vi.spyOn(window, 'open').mockImplementation(() => null);
+});
+
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+});
+
+describe('ObjectTree navigation URL follows the record source (objectui#7638)', () => {
+  it('LIT CONTROL: with no `data` block, rung three IS the record source and still builds the URL', async () => {
+    renderTree({ objectName: DECOY });
+
+    // Reads non-zero, or every assertion below is vacuous.
+    expect(await clickRowAndReadUrl(open)).toBe(`/${DECOY}/record/1`);
+    expect(open).toHaveBeenCalledWith(`/${DECOY}/record/1`, '_blank');
+  });
+
+  it('navigates to the object the ROWS came from, not the top-level key', async () => {
+    renderTree({
+      objectName: DECOY,
+      data: { provider: 'object', object: RECORD_SOURCE },
+    });
+
+    const url = await clickRowAndReadUrl(open);
+    expect(url).toBe(`/${RECORD_SOURCE}/record/1`);
+    // The whole finding in one line: before objectui#7638 this was the answer.
+    expect(url).not.toBe(`/${DECOY}/record/1`);
+  });
+
+  it('builds a routed URL for a data-only block, which previously had no name to use', async () => {
+    // No top-level `objectName` at all. `schema.objectName` was `undefined`
+    // here, so the hook took its `: `/${encodedId}`` leg and produced `/1` — an
+    // unrouted path that paints a blank page.
+    renderTree({ data: { provider: 'object', object: RECORD_SOURCE } });
+
+    const url = await clickRowAndReadUrl(open);
+    expect(url).toBe(`/${RECORD_SOURCE}/record/1`);
+    expect(url).not.toBe('/1');
+  });
+
+  it('keeps the `?? schema.objectName` tail for the OFF-CONTRACT `{ provider: "object" }`', async () => {
+    // `ViewDataSchema` declares `object` REQUIRED on the `object` provider, so
+    // this shape is off-contract and the shared reader deliberately returns
+    // `undefined` for it rather than coercing. This site keeps its own tail —
+    // exactly as `headerObjectName` above it does — so the conversion changes
+    // nothing this component resolves today except the divergence it closes.
+    renderTree({ objectName: DECOY, data: { provider: 'object' } });
+
+    expect(await clickRowAndReadUrl(open)).toBe(`/${DECOY}/record/1`);
+  });
+
+  it('falls back to rung three for a `value`-provider block, which names no object', async () => {
+    renderTree({ objectName: DECOY, data: { provider: 'value', items: ROWS } });
+
+    expect(await clickRowAndReadUrl(open)).toBe(`/${DECOY}/record/1`);
+  });
+});

packages/plugin-tree/src/ObjectTree.tsx

@@ -595,7 +595,20 @@ export const ObjectTree: React.FC<ObjectTreeProps> = ({
 
   const navigation = useNavigationOverlay({
     navigation: (schema as any).navigation,
-    objectName: schema.objectName,
+    // The record-page URL names the object the ROWS came from, not the block's
+    // bare top-level key (objectui#7638). objectui#6939 published `objectName`
+    // as the THIRD RUNG of ONE record-source ladder (`data`, then `staticData`,
+    // then `objectName`) rather than as a parallel "page object" concept, so a
+    // block has exactly one record source. A row fetched through
+    // `data.object` whose click built `/{schema.objectName}/record/{id}` named
+    // a record that the URL's own object does not contain.
+    //
+    // The `?? schema.objectName` tail is NOT the shared rung repeated: it is
+    // this site's own coercion of the OFF-CONTRACT `data: { provider: 'object' }`
+    // that carries no `object` (`ViewDataSchema` declares it required), kept for
+    // exactly the reason `headerObjectName` above keeps it — so this conversion
+    // changes nothing this site resolves today EXCEPT the divergence it closes.
+    objectName: resolveRecordSourceObjectName(schema, dataConfig) ?? schema.objectName,
     onRowClick,
   });