fix(app-shell): RecordDetailView modal 分派报告不可解析 target，删除死路 server 回退 (#3320)

.changeset/modal-dead-fallthrough-record-page.md

@@ -0,0 +1,5 @@
+---
+"@object-ui/app-shell": patch
+---
+
+RecordDetailView's `type:'modal'` dispatch no longer falls back to the server-side action handler when the target resolves to neither a page nor an object. That fallthrough could never succeed — the framework rejects `type:'modal'` over REST with a 400 (`headlessActionTypeError`) — so it only converted an authoring mistake into a confusing round-trip. The record page now reports the same descriptive authoring error as the shared console runtime (objectstack#3959), naming the action, the dud target, and the way out (`type:'script'` with `params`).

packages/app-shell/src/views/RecordDetailView.modalDispatch.test.tsx

@@ -0,0 +1,260 @@
+/**
+ * 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.
+ */
+
+/**
+ * RecordDetailView — `type: 'modal'` dispatch is CLIENT-SIDE ONLY
+ * (objectstack#3959 / objectui#3320).
+ *
+ * The record page wires its own `modal` handler into its `<ActionProvider>`,
+ * shadowing the shared console runtime for every action on the record surface.
+ * objectstack#3959 removed the "fall back to the server-side handler" branch
+ * from `useConsoleActionRuntime.modalActionHandler` — the framework's
+ * `headlessActionTypeError` rejects `type: 'modal'` over REST with a 400, so
+ * the fallthrough could never succeed and only converted an authoring mistake
+ * (a target naming no page) into a confusing round-trip. This copy kept the
+ * pre-#3959 shape, so the SAME dud modal action reported a descriptive
+ * authoring error on a list page and an opaque 400 on the record page.
+ *
+ * This file is the RecordDetailView-side counterpart of
+ * `useConsoleActionRuntime.test.tsx`'s "reports an unresolvable target instead
+ * of POSTing to /actions": it exercises the handler set the view REALLY hands
+ * to its ActionProvider (captured via a pass-through wrapper), so re-adding the
+ * fallthrough — `return serverActionHandler(action)` — turns it red.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, waitFor, act, cleanup } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
+
+const authFetchSpy = vi.fn(async () =>
+  new Response(JSON.stringify({ data: [] }), {
+    status: 200,
+    headers: { 'content-type': 'application/json' },
+  }),
+);
+vi.mock('@object-ui/auth', () => ({
+  useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }),
+  createAuthenticatedFetch: () => authFetchSpy,
+}));
+
+vi.mock('@object-ui/collaboration', () => ({
+  useRecordPresence: () => [],
+  PresenceAvatars: () => null,
+}));
+
+vi.mock('sonner', () => ({
+  toast: Object.assign(vi.fn(), {
+    success: vi.fn(),
+    error: vi.fn(),
+    info: vi.fn(),
+    warning: vi.fn(),
+    loading: vi.fn(),
+    dismiss: vi.fn(),
+  }),
+}));
+
+// The dialogs / flow runner are orthogonal chrome; stubbing them keeps this
+// file about the modal dispatch (same posture as the feed-signal tests).
+vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null }));
+vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null }));
+vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null }));
+vi.mock('./FlowRunner', () => ({ FlowRunner: () => null }));
+vi.mock('./MetadataInspector', () => ({
+  MetadataPanel: () => null,
+  useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }),
+}));
+
+// The client modal transport is stubbed for the same reason as in
+// useConsoleActionRuntime.test.tsx — importing it for real drags in
+// <ModalForm> and the whole plugin-form graph. `resolveTargetSpy` stands in
+// for the page/object lookup so each case below chooses whether the target
+// resolves; its real resolution rules are covered in
+// useActionModal.resolve.test.tsx.
+const modalHandlerSpy = vi.fn(async () => ({ success: true }));
+const resolveTargetSpy = vi.fn(async (_schema: any): Promise<any> => null);
+vi.mock('../hooks/useActionModal', () => ({
+  useActionModal: () => ({
+    modalHandler: modalHandlerSpy,
+    modalElement: null,
+    closeModal: () => {},
+    resolveModalTarget: resolveTargetSpy,
+  }),
+}));
+
+// The server-side dispatch the dead fallthrough used to reach. Returns
+// success so a re-added `return serverActionHandler(action)` flips BOTH the
+// not-called assertion AND the `success: false` assertion below.
+const serverActionSpy = vi.fn(async () => ({ success: true }));
+vi.mock('../utils/consoleServerAction', () => ({
+  createConsoleServerActionHandler: () => serverActionSpy,
+}));
+
+// Capture the handler set each <ActionProvider> receives while KEEPING the
+// real provider (children still get a working action context). The record
+// page's own set is the only one carrying `approval`, so the capture below
+// selects it even if a nested surface mounts a provider of its own. The page
+// body itself is orthogonal here — SchemaRenderer is stubbed so the file
+// stays about the wiring, not the render tree.
+const capturedHandlers: Array<Record<string, (action: any) => Promise<any>>> = [];
+vi.mock('@object-ui/react', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@object-ui/react')>();
+  return {
+    ...actual,
+    ActionProvider: (props: any) => {
+      capturedHandlers.push(props.handlers);
+      return React.createElement(actual.ActionProvider as any, props);
+    },
+    SchemaRenderer: () => null,
+  };
+});
+
+import { MetadataCtx } from '@object-ui/react';
+import { RecordDetailView } from './RecordDetailView';
+
+const OBJECT_NAME = 'crm_call';
+const RECORD_ID = 'rec-call-1';
+
+const OBJECTS = [
+  {
+    name: OBJECT_NAME,
+    label: 'Call',
+    fields: {
+      id: { type: 'text', label: 'Id' },
+      name: { type: 'text', label: 'Name' },
+    },
+  },
+];
+
+function makeDataSource() {
+  return {
+    find: vi.fn(async () => ({ data: [] })),
+    findOne: vi.fn(async () => ({ id: RECORD_ID, name: 'Intro call' })),
+    create: vi.fn(async () => ({})),
+    update: vi.fn(async () => ({})),
+    delete: vi.fn(async () => ({})),
+  } as any;
+}
+
+const METADATA = {
+  objects: OBJECTS,
+  pages: [],
+  loading: false,
+  error: null,
+  refresh: async () => {},
+  invalidate: () => {},
+  ensureType: async () => [],
+  getItem: async () => null,
+  getItemsByType: () => [],
+} as any;
+
+function renderDetail() {
+  return render(
+    <MemoryRouter initialEntries={[`/app/demo/${OBJECT_NAME}/${RECORD_ID}`]}>
+      <MetadataCtx.Provider value={METADATA}>
+        <RecordDetailView
+          dataSource={makeDataSource()}
+          objects={OBJECTS}
+          onEdit={() => {}}
+          objectNameOverride={OBJECT_NAME}
+          recordIdOverride={RECORD_ID}
+          embedded
+        />
+      </MetadataCtx.Provider>
+    </MemoryRouter>,
+  );
+}
+
+/** The record page's OWN handler set — the only provider carrying `approval`. */
+function recordPageHandlers() {
+  return [...capturedHandlers].reverse().find((h) => h && 'approval' in h);
+}
+
+/** Render the view and hand back its ActionProvider handlers, spies cleared. */
+async function mountAndCaptureHandlers() {
+  renderDetail();
+  await waitFor(() => expect(recordPageHandlers()).toBeTruthy());
+  const handlers = recordPageHandlers()!;
+  // Anything the mount itself fetched is not the dispatch under test.
+  authFetchSpy.mockClear();
+  serverActionSpy.mockClear();
+  resolveTargetSpy.mockClear();
+  modalHandlerSpy.mockClear();
+  return handlers;
+}
+
+beforeEach(() => {
+  cleanup();
+  capturedHandlers.length = 0;
+  authFetchSpy.mockClear();
+  serverActionSpy.mockClear();
+  modalHandlerSpy.mockClear();
+  resolveTargetSpy.mockReset();
+  resolveTargetSpy.mockResolvedValue(null);
+  // Unrelated chrome on this view (approvals, favourites, …) reaches for the
+  // platform API; in jsdom that is a real socket. Answer it locally so the
+  // only asynchrony left is the record load the capture waits on.
+  vi.stubGlobal(
+    'fetch',
+    vi.fn(async () =>
+      new Response(JSON.stringify({ data: [] }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      }),
+    ),
+  );
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+describe("RecordDetailView modal dispatch — CLIENT-SIDE ONLY (objectstack#3959 / objectui#3320)", () => {
+  it('opens the resolved target client-side and never POSTs to /actions', async () => {
+    const handlers = await mountAndCaptureHandlers();
+    resolveTargetSpy.mockResolvedValue({ content: { name: 'log_call', type: 'utility' } });
+
+    let r: any;
+    await act(async () => {
+      r = await handlers.modal({ name: 'log_call', type: 'modal', target: 'log_call' });
+    });
+
+    expect(resolveTargetSpy).toHaveBeenCalledWith('log_call');
+    expect(modalHandlerSpy).toHaveBeenCalledWith({ content: { name: 'log_call', type: 'utility' } });
+    expect(serverActionSpy).not.toHaveBeenCalled();
+    expect(authFetchSpy).not.toHaveBeenCalled();
+    expect(r).toMatchObject({ success: true });
+  });
+
+  // The RecordDetailView-side counterpart of useConsoleActionRuntime.test.tsx's
+  // case of the same name. Until objectui#3320 this handler fell back to
+  // `serverActionHandler(action)`, POSTing the modal to /actions — which the
+  // framework rejects with a 400 (headlessActionTypeError) — while the shared
+  // runtime already reported the authoring error (objectstack#3959).
+  it('reports an unresolvable target instead of POSTing to /actions', async () => {
+    const handlers = await mountAndCaptureHandlers();
+    resolveTargetSpy.mockResolvedValue(null);
+
+    let r: any;
+    await act(async () => {
+      r = await handlers.modal({
+        name: 'log_call', type: 'modal', target: 'log_call', params: { subject: 'Intro' },
+      });
+    });
+
+    expect(modalHandlerSpy).not.toHaveBeenCalled();
+    expect(serverActionSpy).not.toHaveBeenCalled();
+    expect(authFetchSpy).not.toHaveBeenCalled();
+    expect(r.success).toBe(false);
+    // The message must name the action, the dud target, and the way out —
+    // the same copy the shared runtime reports, so the two surfaces cannot
+    // drift apart silently again.
+    expect(String(r.error)).toContain('log_call');
+    expect(String(r.error)).toMatch(/type:'script' with params/);
+  });
+});

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

@@ -831,19 +831,35 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
 
   /**
    * `type: 'modal'` dispatch — same rule as the shared console runtime (see
-   * `useConsoleActionRuntime.modalActionHandler`): open `target` as a page (or
-   * an object form) when it names one, else fall through to the action's
-   * server-side handler. Registering it as a handler rather than relying on the
-   * runner's built-in `executeModal` is what gives the record page that server
-   * fallback, so a modal action bound to `engine.registerAction(...)` completes
-   * here exactly as it does on a list page.
+   * `useConsoleActionRuntime.modalActionHandler`): CLIENT-SIDE ONLY. The
+   * action's `target` names the page (or object form) to open; rendering it is
+   * the whole of what a modal action does.
+   *
+   * [objectstack#3959 / objectui#3320] This used to fall through to
+   * `serverActionHandler` when the target resolved to neither a page nor an
+   * object, "so a modal action bound to `engine.registerAction(...)` completes
+   * here exactly as it does on a list page". It never ran: the framework's
+   * `headlessActionTypeError` rejects `type: 'modal'` over REST with a 400,
+   * because a modal has no server dispatch — the fallthrough only converted an
+   * authoring mistake (a target naming no page) into a confusing round-trip.
+   * objectstack#3959 removed it from the shared runtime; this copy kept the
+   * pre-#3959 shape until objectui#3320. An unresolvable target is now
+   * reported as what it is. To collect input and then run server-side, declare
+   * `type: 'script'` with `params`: the runner collects the same dialog and
+   * the handler runs with those values.
    */
   const modalActionHandler = useCallback(async (action: ActionDef) => {
     const schema = (action as any).modal ?? action.target ?? (action as any).params?.schema;
     const descriptor = schema != null ? await resolveModalTarget(schema) : null;
     if (descriptor) return modalHandler(descriptor);
-    return serverActionHandler(action);
-  }, [resolveModalTarget, modalHandler, serverActionHandler]);
+    return {
+      success: false,
+      error:
+        `Action "${action.name}" is type:'modal' but its target ` +
+        `${schema != null ? `"${String(schema)}" ` : ''}names no page or object to open. ` +
+        `Point it at a page, or use type:'script' with params to collect input and run a handler.`,
+    };
+  }, [resolveModalTarget, modalHandler]);
 
   // ─── Approvals ─────────────────────────────────────────────────────
   // Since ADR-0019 an approval is a flow node: the flow opens the request,