Skip to content

Commit cde7502

Browse files
os-zhuangclaude
andauthored
fix(form): create/edit record modal honors the object's default form view (#2007)
The "New <object>" modal (and the modal edit form) rendered every field from the raw object schema, in schema order — ignoring the curated sections + field selection/order defined in the object's default FORM VIEW. Only `tabbed` views were partially honored; a `simple` view with curated sections had no effect. Add `resolveFormViewLayout(objectDef)` (utils/recordFormNavigation), which resolves the default form view (`objectDef.form ?? formViews.default`) into the modal's layout props (curated `sections`, `contentLayout: 'tabbed'`, and master-detail `subforms`) — mirroring the full-screen RecordFormPage. Form views live on the object definition (merged by MetadataProvider), not on getObjectSchema, so resolution reads useMetadata().objects. Wired into both create-modal paths: - AppContent's global New/Edit ModalForm (replaces the tabbed-only inline logic so `simple` sectioned views are honored, keeping create + edit consistent), and - useActionModal (action-opened forms), which previously passed no fields and fell back to the whole schema. When the object has no form view (or one without sections) the modal keeps its prior flat-field behavior. Frontend-only. Tests: 11 resolver unit tests + an end-to-end ModalForm render test (curated fields only vs. full-schema fallback). Full app-shell suite green (927). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d9af1e3 commit cde7502

7 files changed

Lines changed: 329 additions & 16 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
fix(form): create/edit record modal now honors the object's default form view
6+
7+
The "New <object>" modal (and the modal edit form) rendered every field from
8+
the raw object schema, in schema order — ignoring the curated sections + field
9+
selection/order defined in the object's default FORM VIEW. Customizing the form
10+
view (section grouping, field selection/order) had no effect on the create
11+
modal; only `tabbed` views were partially honored, while a `simple` view with
12+
curated sections was dropped entirely.
13+
14+
New `resolveFormViewLayout(objectDef)` helper resolves the default form view
15+
(`objectDef.form ?? formViews.default`) into the modal's layout props (curated
16+
`sections`, `contentLayout: 'tabbed'`, and master-detail `subforms`), mirroring
17+
the full-screen `RecordFormPage`. It is wired into:
18+
19+
- the global New/Edit `ModalForm` in `AppContent` (replacing the tabbed-only
20+
inline logic so `simple` sectioned views are honored too), and
21+
- `useActionModal` (action-opened forms), which previously passed no
22+
`fields`/`sections` and so fell back to the whole object schema.
23+
24+
When the object declares no form view — or one without sections — the modal
25+
keeps its prior flat-field behavior. Frontend-only.

packages/app-shell/src/console/AppContent.tsx

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { usePreviewDrafts } from '../preview/PreviewModeContext';
2424
import { PreviewDraftEmptyState } from '../preview/PreviewDraftEmptyState';
2525
import { ExpressionProvider, evaluateVisibility } from '../providers/ExpressionProvider';
2626
import { useTrackRouteAsRecent } from '../hooks/useTrackRouteAsRecent';
27-
import { resolveRecordFormTarget, resolveNavigateCreateUrl, resolveNavigateEditUrl } from '../utils/recordFormNavigation';
27+
import { resolveRecordFormTarget, resolveFormViewLayout, resolveNavigateCreateUrl, resolveNavigateEditUrl } from '../utils/recordFormNavigation';
2828
import { matchAppBySegment } from '../utils/appRoute';
2929
import { resolveHref, type NavTemplateContext } from '@object-ui/layout';
3030
import { ExpressionEvaluator } from '@object-ui/core';
@@ -608,20 +608,14 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
608608
objectName: currentObjectDef.name,
609609
mode: editingRecord ? 'edit' : 'create',
610610
recordId: editingRecord?.id,
611-
// Master-detail by config: if the object's form view declares
612-
// inline child collections, the standard New/Edit modal renders
613-
// them as an atomic master-detail form (no bespoke page).
614-
subforms: (currentObjectDef as any).form?.subforms
615-
?? (currentObjectDef as any).formViews?.default?.subforms,
616-
// ADR-0050 (#1890): forward the default form view's layout so the
617-
// New/Edit modal can be tabbed (not just a flat stack). `formType`
618-
// stays 'modal' (the container); `contentLayout` carries the layout.
619-
...(() => {
620-
const fd: any = (currentObjectDef as any).form ?? (currentObjectDef as any).formViews?.default;
621-
return fd?.type === 'tabbed' && Array.isArray(fd?.sections)
622-
? { contentLayout: 'tabbed' as const, sections: fd.sections }
623-
: {};
624-
})(),
611+
// Honor the object's DEFAULT FORM VIEW: curated sections (field
612+
// selection + order + grouping), `contentLayout: 'tabbed'` when the
613+
// view is tabbed, and inline child collections (master-detail).
614+
// When the view declares sections they drive the modal layout and
615+
// win over the flat `fields` list below; otherwise this resolves to
616+
// {} and `fields` (every field, raw schema order) is used as before.
617+
// `formType` stays 'modal' (the container). (#1890 / ADR-0050.)
618+
...resolveFormViewLayout(currentObjectDef as any),
625619
title: editingRecord
626620
? t('form.editTitle', { object: objectLabel(currentObjectDef as any) })
627621
: t('form.createTitle', { object: objectLabel(currentObjectDef as any) }),
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* Regression (create modal ignored the object's form view): the "New <object>"
11+
* modal rendered every field from the raw object schema, in schema order,
12+
* ignoring the curated sections + field selection/order defined in the object's
13+
* default FORM VIEW.
14+
*
15+
* This pins the END-TO-END contract the fix relies on: resolve the form-view
16+
* layout from the object definition (exactly as `AppContent`'s global New/Edit
17+
* modal and `useActionModal` now do) and feed it to the real `<ModalForm>`. The
18+
* modal must then render ONLY the curated fields — proving the view drives the
19+
* create form, not the raw schema. The resolver's own branch coverage lives in
20+
* `utils/__tests__/recordFormNavigation.test.ts`; this test guards the wiring +
21+
* rendering so the two can't silently drift back to "show every field".
22+
*/
23+
24+
import { describe, it, expect, vi, beforeEach } from 'vitest';
25+
import { render, screen, waitFor } from '@testing-library/react';
26+
import React from 'react';
27+
import { registerAllFields } from '@object-ui/fields';
28+
import { ModalForm } from '@object-ui/plugin-form';
29+
import { resolveFormViewLayout } from '../../utils/recordFormNavigation';
30+
31+
registerAllFields();
32+
33+
// An object whose RAW schema (what `getObjectSchema` returns) has three fields…
34+
const makeDataSource = (): any => ({
35+
getObjectSchema: vi.fn().mockResolvedValue({
36+
name: 'project',
37+
fields: {
38+
title: { type: 'text', label: 'Title' },
39+
summary: { type: 'textarea', label: 'Summary' },
40+
// …plus a field the form view deliberately leaves OUT.
41+
secret_internal: { type: 'text', label: 'Secret Internal' },
42+
},
43+
}),
44+
create: vi.fn().mockResolvedValue({ id: '1' }),
45+
findOne: vi.fn(),
46+
});
47+
48+
// …but whose DEFAULT FORM VIEW curates only two of them, grouped under a section
49+
// and re-ordered (summary before title) relative to the schema.
50+
const objectDef = {
51+
name: 'project',
52+
form: {
53+
type: 'simple',
54+
sections: [
55+
{ name: 'basics', label: 'Basics', fields: [{ field: 'summary' }, { field: 'title' }] },
56+
],
57+
},
58+
};
59+
60+
beforeEach(() => vi.clearAllMocks());
61+
62+
describe('create modal honors the object form view (sections + field selection)', () => {
63+
it('renders only the curated fields, not the whole object schema', async () => {
64+
// Exactly what AppContent / useActionModal now do: resolve the form-view
65+
// layout from the object definition and spread it into the modal schema.
66+
const layout = resolveFormViewLayout(objectDef as any);
67+
expect(layout.sections).toHaveLength(1);
68+
69+
render(
70+
<ModalForm
71+
schema={{
72+
objectName: 'project',
73+
mode: 'create',
74+
title: 'New Project',
75+
open: true,
76+
onOpenChange: vi.fn(),
77+
...layout,
78+
} as any}
79+
dataSource={makeDataSource()}
80+
/>,
81+
);
82+
83+
// The curated section header + both selected fields render…
84+
await waitFor(() => expect(screen.getByText('Basics')).toBeTruthy());
85+
expect(screen.getByText('Title')).toBeTruthy();
86+
expect(screen.getByText('Summary')).toBeTruthy();
87+
88+
// …and the field the form view omitted is absent. Before the fix the modal
89+
// fell back to the raw object schema and rendered all three (incl. this one).
90+
expect(screen.queryByText('Secret Internal')).toBeNull();
91+
});
92+
93+
it('falls back to the full schema when the object declares no form view', async () => {
94+
// No `form` / `formViews` → resolver returns {}, the modal keeps its prior
95+
// behavior (flat list from the raw schema). This is the un-curated baseline
96+
// the fix must NOT change.
97+
const layout = resolveFormViewLayout({ name: 'project' } as any);
98+
expect(layout).toEqual({});
99+
100+
render(
101+
<ModalForm
102+
schema={{
103+
objectName: 'project',
104+
mode: 'create',
105+
title: 'New Project',
106+
open: true,
107+
onOpenChange: vi.fn(),
108+
...layout,
109+
} as any}
110+
dataSource={makeDataSource()}
111+
/>,
112+
);
113+
114+
// Every schema field is present, including the one a form view would curate out.
115+
await waitFor(() => expect(screen.getByText('Title')).toBeTruthy());
116+
expect(screen.getByText('Summary')).toBeTruthy();
117+
expect(screen.getByText('Secret Internal')).toBeTruthy();
118+
});
119+
});

packages/app-shell/src/hooks/useActionModal.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ import {
4040
SheetTitle,
4141
cn,
4242
} from '@object-ui/components';
43-
import { SchemaRenderer } from '@object-ui/react';
43+
import { SchemaRenderer, useMetadata } from '@object-ui/react';
4444
import { ModalForm } from '@object-ui/plugin-form';
45+
import { resolveFormViewLayout } from '../utils/recordFormNavigation';
4546

4647
type Placement = 'center' | 'side' | 'bottom' | 'fullscreen';
4748
type ModalSize = 'sm' | 'default' | 'lg' | 'xl' | 'full';
@@ -95,6 +96,10 @@ export function normalizeModalSchema(schema: any): ModalDescriptor {
9596

9697
export function useActionModal(dataSource?: any) {
9798
const [state, setState] = useState<{ d: ModalDescriptor; resolve: (r: ActionResult) => void } | null>(null);
99+
// Object metadata — degrades to an empty list outside a MetadataProvider
100+
// (see useMetadata). Used to resolve the object's default form view so the
101+
// create/edit modal honors its curated sections + field selection/order.
102+
const { objects } = useMetadata();
98103

99104
const close = useCallback((r: ActionResult) => {
100105
setState((s) => {
@@ -119,6 +124,14 @@ export function useActionModal(dataSource?: any) {
119124
};
120125

121126
if (d.objectName && !d.content) {
127+
// Honor the object's default FORM VIEW (curated sections + field
128+
// selection/order + master-detail subforms) unless the action descriptor
129+
// passed an explicit field list. Without this the modal falls back to the
130+
// raw object schema — every field, in schema order. Mirrors the global
131+
// New/Edit modal in AppContent so action-opened forms stay consistent.
132+
const viewLayout = (d.fields || (d as any).sections)
133+
? {}
134+
: resolveFormViewLayout(objects.find((o: any) => o?.name === d.objectName));
122135
modalElement = (
123136
<ModalForm
124137
schema={{
@@ -130,6 +143,7 @@ export function useActionModal(dataSource?: any) {
130143
title: d.title,
131144
description: d.description,
132145
fields: d.fields,
146+
...viewLayout,
133147
modalSize: d.size,
134148
open: true,
135149
onOpenChange,

packages/app-shell/src/utils/__tests__/recordFormNavigation.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import { describe, it, expect } from 'vitest';
1515
import {
1616
resolveRecordFormTarget,
17+
resolveFormViewLayout,
1718
resolveNavigateCreateUrl,
1819
resolveNavigateEditUrl,
1920
} from '../recordFormNavigation';
@@ -260,3 +261,77 @@ describe('resolveNavigateEditUrl', () => {
260261
});
261262
});
262263
});
264+
265+
266+
describe('resolveFormViewLayout', () => {
267+
it('returns {} when objectDef is missing', () => {
268+
expect(resolveFormViewLayout(undefined)).toEqual({});
269+
expect(resolveFormViewLayout(null)).toEqual({});
270+
});
271+
272+
it('returns {} when the object declares no form view', () => {
273+
expect(resolveFormViewLayout({})).toEqual({});
274+
expect(resolveFormViewLayout({ formViews: {} })).toEqual({});
275+
});
276+
277+
it('returns the curated sections for a simple form view (no contentLayout)', () => {
278+
const sections = [{ label: 'Details', fields: [{ field: 'subject' }] }];
279+
expect(resolveFormViewLayout({ form: { type: 'simple', sections } })).toEqual({ sections });
280+
});
281+
282+
it('adds contentLayout:"tabbed" for a tabbed form view with sections', () => {
283+
const sections = [
284+
{ label: 'A', fields: [{ field: 'x' }] },
285+
{ label: 'B', fields: [{ field: 'y' }] },
286+
];
287+
expect(resolveFormViewLayout({ form: { type: 'tabbed', sections } })).toEqual({
288+
sections,
289+
contentLayout: 'tabbed',
290+
});
291+
});
292+
293+
it('stacks wizard/split sections (no contentLayout — no modal equivalent)', () => {
294+
const sections = [{ label: 'Step 1', fields: ['a'] }];
295+
expect(resolveFormViewLayout({ form: { type: 'wizard', sections } })).toEqual({ sections });
296+
expect(resolveFormViewLayout({ form: { type: 'split', sections } })).toEqual({ sections });
297+
});
298+
299+
it('treats an empty sections array as "no curation" (omits the key)', () => {
300+
expect(resolveFormViewLayout({ form: { type: 'simple', sections: [] } })).toEqual({});
301+
// …even when tabbed: contentLayout only rides along with real sections.
302+
expect(resolveFormViewLayout({ form: { type: 'tabbed', sections: [] } })).toEqual({});
303+
});
304+
305+
it('forwards non-empty subforms (master-detail) and omits empty ones', () => {
306+
const subforms = [{ childObject: 'invoice_line' }];
307+
expect(resolveFormViewLayout({ form: { type: 'simple', subforms } })).toEqual({ subforms });
308+
expect(resolveFormViewLayout({ form: { type: 'simple', subforms: [] } })).toEqual({});
309+
});
310+
311+
it('combines sections + subforms', () => {
312+
const sections = [{ label: 'Details', fields: ['subject'] }];
313+
const subforms = [{ childObject: 'invoice_line' }];
314+
expect(resolveFormViewLayout({ form: { type: 'simple', sections, subforms } })).toEqual({
315+
sections,
316+
subforms,
317+
});
318+
});
319+
320+
it('falls back to formViews.default when `form` is absent (legacy container)', () => {
321+
const sections = [{ label: 'Details', fields: ['subject'] }];
322+
expect(
323+
resolveFormViewLayout({ formViews: { default: { type: 'simple', sections } } }),
324+
).toEqual({ sections });
325+
});
326+
327+
it('prefers the default `form` ViewItem over formViews.default', () => {
328+
const formSections = [{ label: 'Primary', fields: ['a'] }];
329+
const legacySections = [{ label: 'Legacy', fields: ['b'] }];
330+
expect(
331+
resolveFormViewLayout({
332+
form: { type: 'simple', sections: formSections },
333+
formViews: { default: { type: 'simple', sections: legacySections } },
334+
}),
335+
).toEqual({ sections: formSections });
336+
});
337+
});

packages/app-shell/src/utils/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44

55
export {
66
resolveRecordFormTarget,
7+
resolveFormViewLayout,
78
} from './recordFormNavigation';
89
export type {
910
ObjectDefinitionForNavigation,
1011
RecordFormTarget,
12+
ObjectDefinitionForFormView,
13+
FormViewDefinition,
14+
FormViewModalLayout,
1115
} from './recordFormNavigation';
1216

1317
export { deriveRelatedLists } from './deriveRelatedLists';

0 commit comments

Comments
 (0)