fix(console): resolve a modal action's target as a page, not an object (#3530) (#2826)

The console read a `type: 'modal'` action's `target` as an OBJECT name and
opened a create form for it, so a target naming a page issued
`GET /meta/object/<page>` — which 400s — and the dialog rendered ModalForm's
"Error loading form — Bad Request" instead of the page.

- `normalizeModalSchema` no longer guesses "object" for a string target; the new
  `useActionModal.resolveModalTarget` resolves it against metadata: page first
  (what the spec says the name means), then object for back-compat. Resolution
  uses `getItem(type, name)`, a single-item fetch, so it never eagerly loads the
  lazy page/object lists.
- The `create_x` / `edit_x` prefix convention is now only a fallback: a page
  actually named `create_opportunity` wins over the object `opportunity`.
- An unresolvable target reports that, instead of surfacing an HTTP error.
- `useConsoleActionRuntime` and `RecordDetailView` now share one modal dispatch
  rule, so the same button no longer behaves differently per surface.

Author: os-zhuang

.changeset/modal-action-target-page-resolution.md

@@ -0,0 +1,34 @@
+---
+"@object-ui/app-shell": patch
+---
+
+fix(console): resolve a modal action's `target` as a page, not an object (#3530)
+
+Submitting a `type: 'modal'` action failed with "Error loading form — Bad
+Request". The console read the action's `target` as an OBJECT name and opened a
+create form for it, so a target naming a page issued `GET
+/meta/object/<page>` — which 400s — and the dialog rendered `<ModalForm>`'s
+error state instead of the page. Every modal action in an app hit this; the only
+workaround was re-authoring each one as a screen flow.
+
+The spec is explicit that for `type: 'modal'`, `target` is "the modal/page name
+to open".
+
+- `normalizeModalSchema` no longer guesses "object" for a string target. It
+  records the raw name and `useActionModal.resolveModalTarget` (new) resolves it
+  against metadata: **page first**, then object for back-compat. Resolution uses
+  `getItem(type, name)`, a single-item fetch, so it never eagerly loads the lazy
+  page/object lists — this hook is mounted at the console root.
+- The `create_x` / `edit_x` prefix convention still yields an object form, but
+  now only as a fallback: a page actually named `create_opportunity` wins over
+  the object `opportunity` the name would otherwise be parsed into.
+- A target that names neither reports what is wrong ("Modal target "x" matches
+  no page or object") instead of surfacing a downstream HTTP error.
+
+Modal dispatch is also now the same on every console surface. `type: 'modal'`
+was wired straight to the server-action POST in `useConsoleActionRuntime` (list
+pages, SDUI pages, the declared-actions bar) while `RecordDetailView` opened
+modals client-side — the same button did two different things depending on where
+it was mounted. Both now run one rule: render `target` when it names a page or
+object, otherwise complete the action through its server-side handler, so a
+modal action bound to `engine.registerAction(...)` keeps working.

packages/app-shell/src/hooks/__tests__/useActionModal.resolve.test.tsx

@@ -0,0 +1,145 @@
+/**
+ * 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.
+ */
+
+/**
+ * Regression (framework#3530 — "Console: modal-typed actions resolve `target` as
+ * an object"): a `type: 'modal'` action's `target` names the PAGE to open, but
+ * the console read it as an OBJECT name and opened a create form for it. That
+ * issued `GET /meta/object/<page>`, which 400s, so the modal body was replaced
+ * with ModalForm's "Error loading form — Bad Request" and the action never
+ * completed.
+ *
+ * These pin the resolution CONTRACT: page first (what the spec says the name
+ * means), object second (back-compat), `null` last so the console runtimes can
+ * fall through to the action's server-side handler. The pure normalization
+ * step is covered separately in `useActionModal.test.ts`.
+ */
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { renderHook, act } from '@testing-library/react';
+import React from 'react';
+import { MetadataCtx } from '@object-ui/react';
+import { useActionModal } from '../useActionModal';
+
+const LOG_CALL_PAGE = { name: 'log_call', type: 'utility', label: 'Log a Call' };
+const CONTACT_OBJECT = { name: 'contact', label: 'Contact', fields: {} };
+
+/**
+ * Stand-in metadata context. `getItem(type, name)` is the ONLY lookup the
+ * resolver may use — reading the whole `pages` list would eagerly load a lazy
+ * metadata type at the console root, which is what `getItem` exists to avoid.
+ */
+function makeWrapper(items: Record<string, any[]>) {
+  const getItem = vi.fn(async (type: string, name: string) =>
+    (items[type] ?? []).find((i) => i.name === name) ?? null);
+  const value: any = {
+    apps: [], objects: items.object ?? [], dashboards: [], reports: [], pages: [],
+    loading: false, error: null,
+    refresh: async () => {}, invalidate: () => {}, ensureType: async () => [],
+    getItem,
+    getItemsByType: () => [],
+    getTypeStatus: () => 'ready',
+  };
+  const wrapper = ({ children }: { children: React.ReactNode }) => (
+    <MetadataCtx.Provider value={value}>{children}</MetadataCtx.Provider>
+  );
+  return { wrapper, getItem };
+}
+
+beforeEach(() => vi.clearAllMocks());
+
+describe('useActionModal — modal target resolution (framework#3530)', () => {
+  it('resolves a string target to the PAGE of that name, never GET /meta/object', async () => {
+    const { wrapper, getItem } = makeWrapper({ page: [LOG_CALL_PAGE] });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let d: any;
+    await act(async () => { d = await result.current.resolveModalTarget('log_call'); });
+
+    // Rendered as page CONTENT (same shape PageView feeds SchemaRenderer),
+    // not as an object form.
+    expect(d.content).toMatchObject({ name: 'log_call', type: 'utility' });
+    expect(d.objectName).toBeUndefined();
+    expect(d.title).toBe('Log a Call');
+
+    // The object lookup that produced the 400 must never happen.
+    expect(getItem).toHaveBeenCalledWith('page', 'log_call');
+    expect(getItem).not.toHaveBeenCalledWith('object', 'log_call');
+  });
+
+  it('falls back to an object create form when no page owns the name', async () => {
+    const { wrapper } = makeWrapper({ page: [], object: [CONTACT_OBJECT] });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let d: any;
+    await act(async () => { d = await result.current.resolveModalTarget('contact'); });
+
+    expect(d).toMatchObject({ objectName: 'contact', mode: 'create' });
+    expect(d.content).toBeUndefined();
+  });
+
+  it('prefers a page over the object a create_ prefix would parse into', async () => {
+    const { wrapper } = makeWrapper({
+      page: [{ name: 'create_opportunity', type: 'utility', label: 'New Opportunity' }],
+      object: [{ name: 'opportunity' }],
+    });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let d: any;
+    await act(async () => { d = await result.current.resolveModalTarget('create_opportunity'); });
+
+    expect(d.content).toMatchObject({ name: 'create_opportunity' });
+    expect(d.objectName).toBeUndefined();
+  });
+
+  it('still honors the create_ prefix when no page owns the name', async () => {
+    const { wrapper } = makeWrapper({ page: [], object: [{ name: 'opportunity' }] });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let d: any;
+    await act(async () => { d = await result.current.resolveModalTarget('create_opportunity'); });
+
+    expect(d).toMatchObject({ objectName: 'opportunity', mode: 'create' });
+  });
+
+  it('returns null when the target names neither a page nor an object', async () => {
+    // Not an error on its own — the console runtimes read null as "not a
+    // client-rendered modal" and run the action server-side instead.
+    const { wrapper } = makeWrapper({ page: [], object: [] });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let d: any;
+    await act(async () => { d = await result.current.resolveModalTarget('schedule_followup'); });
+    expect(d).toBeNull();
+  });
+
+  it('passes an explicit { objectName, mode } descriptor through without lookups', async () => {
+    // The lookup field's inline "create the referenced record" path.
+    const { wrapper, getItem } = makeWrapper({ page: [], object: [] });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let d: any;
+    await act(async () => {
+      d = await result.current.resolveModalTarget({ objectName: 'customers', mode: 'create' });
+    });
+
+    expect(d).toMatchObject({ objectName: 'customers', mode: 'create' });
+    expect(getItem).not.toHaveBeenCalled();
+  });
+
+  it('modalHandler reports an unresolvable target instead of opening a broken form', async () => {
+    const { wrapper } = makeWrapper({ page: [], object: [] });
+    const { result } = renderHook(() => useActionModal(), { wrapper });
+
+    let r: any;
+    await act(async () => { r = await result.current.modalHandler('schedule_followup'); });
+
+    expect(r.success).toBe(false);
+    expect(r.error).toContain('schedule_followup');
+  });
+});

packages/app-shell/src/hooks/__tests__/useActionModal.test.ts

@@ -2,19 +2,34 @@ import { describe, it, expect } from 'vitest';
 import { normalizeModalSchema } from '../useActionModal';
 
 describe('normalizeModalSchema', () => {
-  it('maps create_/new_/add_ string targets to a create object-form', () => {
-    expect(normalizeModalSchema('create_opportunity')).toEqual({ objectName: 'opportunity', mode: 'create' });
-    expect(normalizeModalSchema('new_task')).toEqual({ objectName: 'task', mode: 'create' });
-    expect(normalizeModalSchema('add_note')).toEqual({ objectName: 'note', mode: 'create' });
+  it('keeps a string target UNRESOLVED — page-vs-object is a metadata question', () => {
+    // framework#3530: assuming "object" here sent every page-targeting modal
+    // action to GET /meta/object/<page>, which 400s and rendered ModalForm's
+    // "Error loading form — Bad Request". `resolveModalTarget` asks the
+    // metadata service instead (see useActionModal.resolve.test.tsx).
+    expect(normalizeModalSchema('log_call')).toEqual({ targetName: 'log_call' });
+    expect(normalizeModalSchema('contact')).toEqual({ targetName: 'contact' });
   });
 
-  it('maps edit_/update_ string targets to an edit object-form', () => {
-    expect(normalizeModalSchema('edit_account')).toEqual({ objectName: 'account', mode: 'edit' });
-    expect(normalizeModalSchema('update_lead')).toEqual({ objectName: 'lead', mode: 'edit' });
+  it('carries a create_/new_/add_ prefix guess ALONGSIDE the raw target', () => {
+    expect(normalizeModalSchema('create_opportunity')).toEqual({
+      targetName: 'create_opportunity', objectName: 'opportunity', mode: 'create',
+    });
+    expect(normalizeModalSchema('new_task')).toEqual({
+      targetName: 'new_task', objectName: 'task', mode: 'create',
+    });
+    expect(normalizeModalSchema('add_note')).toEqual({
+      targetName: 'add_note', objectName: 'note', mode: 'create',
+    });
   });
 
-  it('treats a bare string as a create form for that object', () => {
-    expect(normalizeModalSchema('contact')).toEqual({ objectName: 'contact', mode: 'create' });
+  it('carries an edit_/update_ prefix guess as an edit form', () => {
+    expect(normalizeModalSchema('edit_account')).toEqual({
+      targetName: 'edit_account', objectName: 'account', mode: 'edit',
+    });
+    expect(normalizeModalSchema('update_lead')).toEqual({
+      targetName: 'update_lead', objectName: 'lead', mode: 'edit',
+    });
   });
 
   it('treats a bare SchemaNode (has type, no descriptor keys) as content', () => {

packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx

@@ -37,6 +37,22 @@ vi.mock('@object-ui/i18n', () => ({
   }),
 }));
 
+// The client modal transport is stubbed for the same reason — importing it for
+// real drags in <ModalForm> and the whole plugin-form graph. `resolveTargetSpy`
+// stands in for the page/object lookup so the modal-dispatch tests below can
+// choose whether a 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('../useActionModal', () => ({
+  useActionModal: () => ({
+    modalHandler: modalHandlerSpy,
+    modalElement: null,
+    closeModal: () => {},
+    resolveModalTarget: resolveTargetSpy,
+  }),
+}));
+
 // The dialogs/flow-runner are not exercised here — keep them as inert stubs so
 // the hook module imports cheaply.
 vi.mock('../../views/ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null }));
@@ -61,6 +77,9 @@ import { useAction, usePageVariables, PageVariablesProvider, PageVariableActionB
 beforeEach(() => {
   authFetchSpy.mockReset();
   navigateSpy.mockReset();
+  modalHandlerSpy.mockClear();
+  resolveTargetSpy.mockReset();
+  resolveTargetSpy.mockResolvedValue(null);
   (toast as any).mockClear?.();
   (toast as any).error.mockClear();
   (toast as any).success.mockClear();
@@ -295,6 +314,68 @@ describe('useConsoleActionRuntime — authenticated handlers', () => {
     expect(Object.keys(props.handlers).sort()).toEqual(['api', 'flow', 'modal', 'script']);
     expect(typeof props.onConfirm).toBe('function');
     expect(typeof props.onParamCollection).toBe('function');
+    expect(typeof props.onModal).toBe('function');
+  });
+});
+
+/**
+ * framework#3530 — `type: 'modal'` used to be wired straight to
+ * `serverActionHandler` here, while RecordDetailView opened modals client-side.
+ * The same button therefore did two different things depending on which surface
+ * mounted it. Both now run this rule: render `target` when it names a page (or
+ * object), else complete the action server-side.
+ */
+describe('modalActionHandler — open the target, else run it server-side', () => {
+  it('opens the resolved target client-side and never POSTs to /actions', async () => {
+    resolveTargetSpy.mockResolvedValue({ content: { name: 'log_call', type: 'utility' } });
+    const { result } = renderHook(() =>
+      useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'crm_call' }),
+    );
+
+    await act(async () => {
+      await result.current.modalActionHandler({ name: 'log_call', type: 'modal', target: 'log_call' } as any);
+    });
+
+    expect(resolveTargetSpy).toHaveBeenCalledWith('log_call');
+    expect(modalHandlerSpy).toHaveBeenCalledWith({ content: { name: 'log_call', type: 'utility' } });
+    expect(authFetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('falls back to the server action when the target names no page or object', async () => {
+    // The modal was the param dialog the runner already collected — the action
+    // still has to run, so it goes to its registered server handler.
+    resolveTargetSpy.mockResolvedValue(null);
+    authFetchSpy.mockResolvedValue({ ok: true, json: async () => ({ success: true, data: { ok: 1 } }) });
+    const { result } = renderHook(() =>
+      useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'crm_call' }),
+    );
+
+    let r: any;
+    await act(async () => {
+      r = await result.current.modalActionHandler({
+        name: 'log_call', type: 'modal', target: 'log_call', params: { subject: 'Intro' },
+      } as any);
+    });
+
+    expect(modalHandlerSpy).not.toHaveBeenCalled();
+    expect(authFetchSpy).toHaveBeenCalledTimes(1);
+    expect(authFetchSpy.mock.calls[0][0]).toContain('/api/v1/actions/crm_call/log_call');
+    expect(r.success).toBe(true);
+  });
+
+  it('prefers an inline `modal` descriptor over `target`', async () => {
+    resolveTargetSpy.mockResolvedValue({ objectName: 'customers', mode: 'create' });
+    const { result } = renderHook(() =>
+      useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'crm_call' }),
+    );
+
+    await act(async () => {
+      await result.current.modalActionHandler({
+        name: 'x', type: 'modal', target: 'ignored', modal: { objectName: 'customers', mode: 'create' },
+      } as any);
+    });
+
+    expect(resolveTargetSpy).toHaveBeenCalledWith({ objectName: 'customers', mode: 'create' });
   });
 });