diff --git a/.changeset/modal-form-section-group-11542.md b/.changeset/modal-form-section-group-11542.md new file mode 100644 index 0000000000..47cfe78610 --- /dev/null +++ b/.changeset/modal-form-section-group-11542.md @@ -0,0 +1,22 @@ +--- +'@object-ui/plugin-form': patch +--- + +The record dialog now draws a `form.sections[].group` section (objectui#11542). + +A form view section can declare its members by pointing `group` at one of the +object's `fieldGroups` (the reference form of objectstack#13855). `ObjectForm` +resolved that form, but `ModalForm` did not, and the console's More actions › +Edit / New dialog and action-opened modals mount `ModalForm` directly with the +form view's sections as authored. A `{ group }` section therefore reached the +dialog with no fields and was dropped: a tabbed form view showed no tab for the +group, a stacked one showed no header, and the fields only that group carries +could not be edited in the dialog. + +`ModalForm` now resolves its sections through `resolveSectionGroupReferences`, +the same resolver `ObjectForm` uses, against the object schema it already +loads. The group's section is drawn with the group's label and members in both +content layouts, its members pass the same field-level security gate as +enumerated fields, and an unknown group renders nothing and is reported once, +as it is on `ObjectForm`. A section list that uses no `group` reaches the +dialog unchanged. diff --git a/packages/plugin-form/README.md b/packages/plugin-form/README.md index 93bd6b493e..d1fb9a8a3c 100644 --- a/packages/plugin-form/README.md +++ b/packages/plugin-form/README.md @@ -385,7 +385,11 @@ it points `group` at one of the object's declared `fieldGroups` and inherits tha group's members **and** its presentation (objectstack#13855, ADR-0085 §5 — the spec range that carries it is the `@objectstack/spec` entry in this package's own `package.json`). `ObjectForm` resolves the reference once, above its routing -fork, so all six layouts inherit it. +fork, so all six layouts inherit it. `ModalForm` resolves its own `sections` +through the same call as well, because hosts mount it directly rather than +through `ObjectForm` — the console's record create / edit dialog and +action-opened modals do (objectui#11542). `DrawerForm` mounted directly does +not resolve them; reach it through `ObjectForm` with `formType: 'drawer'`. A host with its **own** section builder resolves it with the same function instead of deriving sections itself (objectui#8641): diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index 146186f541..470e6b4270 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -14,7 +14,7 @@ */ import React, { useState, useCallback, useEffect, useMemo, useId, useRef } from 'react'; -import type { FormField, FormSchema, DataSource, ObjectFormSchema } from '@object-ui/types'; +import type { FormField, FormSchema, DataSource, ObjectFormSchema, ObjectFormSection } from '@object-ui/types'; import { Dialog, MobileDialogContent, @@ -51,6 +51,7 @@ import { CONTAINER_GRID_COLS, } from './autoLayout'; import { deriveFieldGroupSections, projectSectionDivider, resolveSectionCollapse } from './fieldGroups'; +import { resolveSectionGroupReferences } from './sectionGroups'; import { sanitizeFormData, dirtyEditPayload, @@ -325,6 +326,29 @@ export const ModalForm: React.FC = ({ // Stable form id for linking the external submit button to the form element const formId = useId(); + // `form.sections[].group` (objectui#11542): a section that points `group` at + // one of the object's `fieldGroups` becomes the section that group declares, + // through the ONE resolver `ObjectForm`'s `withGroups` uses, so no assembly + // rule lives here. `ObjectForm` resolves above its routing fork, but the + // console's record dialog and an action-opened modal mount THIS component + // directly with the form view's sections as authored, so the dialog resolves + // its own. Resolved once, above both content layouts (tabbed and stacked), + // against the object schema this form already loads; while it loads the + // form shows its skeleton. Returns `schema.sections` itself when no section + // uses `group` — which includes every section list `ObjectForm` hands over, + // already resolved — so no other modal takes a new path. Every resolved + // member still goes through `gateFields` below, like an enumerated one. + const resolvedSections = useMemo( + () => + resolveSectionGroupReferences(schema.sections as ObjectFormSection[] | undefined, { + objectName: schema.objectName, + formType: schema.formType, + objectDef: objectSchema, + resolvable: typeof dataSource?.getObjectSchema === 'function', + }) as ModalFormSectionConfig[] | undefined, + [schema.sections, schema.objectName, schema.formType, objectSchema, dataSource], + ); + // Field-group fallback (object-designer metadata): when the caller passes no // explicit sections, honor the object's declared `fieldGroups` the same way // ObjectForm's simple path does — one section per group, with flat-path @@ -348,7 +372,7 @@ export const ModalForm: React.FC = ({ return sections.map((s) => ({ ...s, columns })) as ModalFormSectionConfig[]; }, [schema.sections, schema.columns, schema.mode, formFields, objectSchema]); - const effectiveSections = schema.sections?.length ? schema.sections : (derivedSections ?? undefined); + const effectiveSections = resolvedSections?.length ? resolvedSections : (derivedSections ?? undefined); // Compute auto-layout for flat fields (no sections) to determine inferred columns // (`customFields` does not switch it off: the members are merged into @@ -785,8 +809,8 @@ export const ModalForm: React.FC = ({ // 2+ silently dropped everything the user typed; and in the tabbed variant // Radix unmounted the inactive panel, destroying that tab's form state // outright. Same single-form pattern as ObjectForm / DrawerForm. - if (schema.sections?.length) { - const sections = schema.sections; + if (resolvedSections?.length) { + const sections = resolvedSections; const sectionKey = (sec: ModalFormSectionConfig, i: number) => sec.name || sec.label || String(i); // Section headers go through the same i18n hook ObjectForm uses, so a // translated group label wins over the raw metadata label. diff --git a/packages/plugin-form/src/__tests__/modalFormSectionGroupReference-11542.test.tsx b/packages/plugin-form/src/__tests__/modalFormSectionGroupReference-11542.test.tsx new file mode 100644 index 0000000000..dd8ab76f64 --- /dev/null +++ b/packages/plugin-form/src/__tests__/modalFormSectionGroupReference-11542.test.tsx @@ -0,0 +1,311 @@ +/** + * 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. + */ + +/** + * `form.sections[].group` on the record dialog — `ModalForm` rendered + * DIRECTLY by a host (objectui#11542). + * + * ## The path these pins hold + * + * The console's More actions › Edit / New dialog and an action-opened modal do + * not go through `ObjectForm`: `AppContent` and `useActionModal` in + * `@object-ui/app-shell` mount `ModalForm` themselves and spread the object's + * default form view into it through `resolveFormViewLayout`, which hands + * `formView.sections` over exactly as authored (with `contentLayout: 'tabbed'` + * for a tabbed view). `ObjectForm` resolves `{ group }` sections in + * `withGroups`, above its routing fork, so its own modal route + * (`formType: 'modal'`) was covered by objectui#7051 — but a host that skips + * `ObjectForm` skipped that resolution too. A `{ group }` section reached + * `ModalForm` with no `fields`, `buildSectionFields` built it an empty body, + * and the empty-body filter dropped it: no tab, no stacked header, and the + * group's fields had no editing surface in the dialog. + * + * `ModalForm` now resolves its own sections through the one resolver, + * `resolveSectionGroupReferences`, against the object schema it already loads. + * No assembly rule lives here or there; both reach `deriveFieldGroupLayout`. + * + * ## What discriminates + * + * The fixture declares TWO groups and references the SECOND one: a resolver + * that answered every reference with the first group (the caricature + * objectui#7051 measured) renders the wrong members, and the exclusivity + * assertion catches it. The hand-enumerated sections are the control — green + * before the fix and after it. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup, fireEvent } from '@testing-library/react'; +import { registerAllFields } from '@object-ui/fields'; +import { MePermissionsProvider } from '@object-ui/permissions'; +import { ModalForm } from '../ModalForm'; +import { resetSectionGroupReports } from '../sectionGroups'; + +registerAllFields(); + +// ─── Fixture: the hotcrm contact shape ──────────────────────────────────── + +/** + * `marketing` is declared FIRST and `buying_centre` SECOND, and the form + * references only the second. Distinct members per group, so a constant + * resolution cannot satisfy the member assertions. + */ +const CONTACT = { + name: 'contact', + fieldGroups: [ + { key: 'marketing', label: 'Marketing' }, + { key: 'buying_centre', label: 'Buying Centre' }, + ], + fields: { + first_name: { type: 'text', label: 'First Name' }, + last_name: { type: 'text', label: 'Last Name' }, + email: { type: 'text', label: 'Email' }, + phone: { type: 'text', label: 'Phone' }, + campaign_source: { type: 'text', label: 'Campaign Source', group: 'marketing' }, + buying_role: { type: 'text', label: 'Buying Role', group: 'buying_centre' }, + influence: { type: 'text', label: 'Influence', group: 'buying_centre' }, + budget_owner: { type: 'text', label: 'Budget Owner', group: 'buying_centre' }, + }, +}; + +const RECORD = { + id: 'c1', + first_name: 'Ada', + last_name: 'Lovelace', + email: 'ada@example.com', + phone: '555', + buying_role: 'Economic buyer', + influence: 'High', + budget_owner: 'Yes', +}; + +/** Hand-enumerated sections beside one `{ group }` reference — the authored form view. */ +const SECTIONS = [ + { name: 'identity', label: 'Identity', fields: ['first_name', 'last_name'] }, + { name: 'contact_info', label: 'Contact Info', fields: ['email', 'phone'] }, + { group: 'buying_centre', columns: 2 }, +]; + +const makeDS = () => ({ + getObjectSchema: vi.fn().mockResolvedValue(CONTACT), + findOne: vi.fn().mockResolvedValue({ ...RECORD }), + find: vi.fn().mockResolvedValue({ data: [] }), + create: vi.fn().mockResolvedValue({ id: 'c2' }), + update: vi.fn().mockResolvedValue({ ...RECORD }), +}); + +type Mode = 'create' | 'edit'; + +/** The dialog the way `AppContent` mounts it: `ModalForm` itself, no `ObjectForm` above it. */ +const modal = (mode: Mode, opts: { sections?: any[]; tabbed?: boolean; ds?: any } = {}) => ( + +); + +/** Wait until the form is drawn and, in edit mode, the record is on screen. */ +async function formDrawn(mode: Mode): Promise { + await waitFor(() => { + const el = document.body.querySelector('input[name="first_name"]') as HTMLInputElement | null; + if (!el) throw new Error('form not drawn yet'); + if (mode === 'edit' && el.value !== RECORD.first_name) throw new Error('record not on screen yet'); + }); + const forms = document.body.querySelectorAll('form'); + // ONE form for every section, the group's included (#2153 / #2959). + expect(forms).toHaveLength(1); + return forms[0] as HTMLFormElement; +} + +/** The tab strip, in order: the key of every tab trigger. */ +const tabKeys = () => + Array.from(document.body.querySelectorAll('[data-testid^="form-tab:"]')).map((el) => + (el.getAttribute('data-testid') as string).slice('form-tab:'.length), + ); + +/** The field inputs a tab panel draws, in order. */ +const panelInputs = (key: string) => { + const panel = document.body.querySelector(`[data-testid="form-tab-panel:${key}"]`); + if (!panel) return null; + return Array.from(panel.querySelectorAll('input[name]')).map((el) => el.getAttribute('name')); +}; + +/** Stacked layout, in document order: `H:LABEL` per section header, `F:NAME` per input. */ +function stackedOutline(form: HTMLElement): string[] { + const out: string[] = []; + form.querySelectorAll('input[name], .col-span-full').forEach((el) => { + if (el.tagName === 'INPUT') out.push(`F:${el.getAttribute('name')}`); + else if (el.classList.contains('border-b')) out.push(`H:${(el.textContent ?? '').trim()}`); + }); + return out; +} + +let uiErrors: string[] = []; +let errSpy: ReturnType; + +beforeEach(() => { + vi.clearAllMocks(); + resetSectionGroupReports(); + uiErrors = []; + // Only this repo's own diagnostics: React's act() advice also lands here. + errSpy = vi.spyOn(console, 'error').mockImplementation((...args: any[]) => { + const s = args.map(String).join(' '); + if (s.includes('[object-ui]')) uiErrors.push(s); + }); +}); + +afterEach(() => { + errSpy.mockRestore(); + cleanup(); +}); + +// ─── The tabbed dialog: New and Edit ────────────────────────────────────── + +describe.each(['create', 'edit'])( + 'objectui#11542 — the tabbed %s dialog (ModalForm mounted directly) draws a `{ group }` section', + (mode) => { + it("⭐ draws the group's tab, labelled by the group, holding exactly ITS members", async () => { + render(modal(mode)); + await formDrawn(mode); + + // The hand-enumerated tabs stay where the author put them, and the + // `{ group }` section is a tab of its own, in its authored position. + expect(tabKeys()).toEqual(['identity', 'contact_info', 'buying_centre']); + expect(screen.getByTestId('form-tab:buying_centre').textContent).toContain('Buying Centre'); + + // Exactly the referenced group's members, in declared order — and not + // the FIRST declared group's (`campaign_source`), which is what a + // constant resolution would draw. + expect(panelInputs('buying_centre')).toEqual(['buying_role', 'influence', 'budget_owner']); + expect(document.body.querySelectorAll('input[name="campaign_source"]')).toHaveLength(0); + expect(uiErrors).toEqual([]); + }); + + it('control: the hand-enumerated sections draw exactly their own members', async () => { + render(modal(mode)); + await formDrawn(mode); + expect(panelInputs('identity')).toEqual(['first_name', 'last_name']); + expect(panelInputs('contact_info')).toEqual(['email', 'phone']); + }); + }, +); + +describe('objectui#11542 — the group tab is a live part of the one form', () => { + it('edit: the group fields carry the record values', async () => { + render(modal('edit')); + await formDrawn('edit'); + const value = (name: string) => + (document.body.querySelector(`input[name="${name}"]`) as HTMLInputElement).value; + expect(value('buying_role')).toBe(RECORD.buying_role); + expect(value('budget_owner')).toBe(RECORD.budget_owner); + }); + + it('create: a value typed on the group tab reaches the one create() payload', async () => { + const ds = makeDS(); + render(modal('create', { ds })); + const form = await formDrawn('create'); + fireEvent.click(screen.getByTestId('form-tab:buying_centre')); + fireEvent.change(document.body.querySelector('input[name="buying_role"]') as HTMLInputElement, { + target: { value: 'Champion' }, + }); + fireEvent.change(document.body.querySelector('input[name="first_name"]') as HTMLInputElement, { + target: { value: 'Grace' }, + }); + fireEvent.submit(form); + await waitFor(() => expect(ds.create).toHaveBeenCalledTimes(1)); + expect(ds.create.mock.calls[0][1]).toMatchObject({ first_name: 'Grace', buying_role: 'Champion' }); + }); +}); + +// ─── The stacked arm: resolution is above the layout fork ───────────────── + +describe('objectui#11542 — the stacked dialog (no contentLayout) draws the group section too', () => { + it.each(['create', 'edit'])('%s: a header labelled by the group, then exactly its members', async (mode) => { + render(modal(mode, { tabbed: false })); + const form = await formDrawn(mode); + expect(tabKeys()).toEqual([]); + expect(stackedOutline(form)).toEqual([ + 'H:Identity', + 'F:first_name', + 'F:last_name', + 'H:Contact Info', + 'F:email', + 'F:phone', + 'H:Buying Centre', + 'F:buying_role', + 'F:influence', + 'F:budget_owner', + ]); + }); +}); + +// ─── Field-level security reaches the fields a group brings in ──────────── + +/** + * A group's members come from the object, not from the authored section, so + * the fix must not open a path around the one field gate (`gateFormFields`). + * `influence` is read-denied (never drawn); `budget_owner` is write-denied + * (drawn, locked); `buying_role` is the lit control (drawn, live). + */ +const principal = { + authenticated: true, + userId: 'u-1', + tenantId: null, + roles: ['sales_rep'], + permissionSets: ['sales_rep'], + objects: { contact: { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: false } }, + fields: { + 'contact.influence': { readable: false, editable: false }, + 'contact.budget_owner': { readable: true, editable: false }, + }, +}; + +/** Every drawn field in a container → whether it is locked (kept no enabled control). */ +function drawnIn(container: Element): Record { + const out: Record = {}; + for (const item of container.querySelectorAll('[data-field]')) { + out[item.getAttribute('data-field') as string] = + item.querySelector('input:not([disabled]), textarea:not([disabled])') === null; + } + return out; +} + +describe('objectui#11542 — FLS gates the fields a `{ group }` section brings in', () => { + it.each(['create', 'edit'])('%s: read-denied member is not drawn, write-denied member is locked', async (mode) => { + render({modal(mode)}); + await formDrawn(mode); + const panel = screen.getByTestId('form-tab-panel:buying_centre'); + expect(drawnIn(panel)).toEqual({ buying_role: false, budget_owner: true }); + expect(document.body.querySelectorAll('input[name="influence"]')).toHaveLength(0); + }); +}); + +// ─── The resolver's own verdicts reach this surface unchanged ───────────── + +describe('objectui#11542 — an unknown group renders nothing and is reported once', () => { + it('no tab, the siblings intact, one `form-section-group-unknown` report', async () => { + render(modal('create', { sections: [...SECTIONS.slice(0, 2), { group: 'no_such_group' }] })); + await formDrawn('create'); + expect(tabKeys()).toEqual(['identity', 'contact_info']); + // Reported because the object schema the dialog loaded proves the key + // resolves to nothing — a dialog that resolved with no object definition + // would stay silent here. + expect(uiErrors).toHaveLength(1); + expect(uiErrors[0]).toContain('no_such_group'); + expect(uiErrors[0]).toContain('form-section-group-unknown'); + }); +});