fix(studio): show, keep and author a controlled-by-parent object's optional master

.changeset/30974-studio-optional-cbp-master.md

@@ -0,0 +1,16 @@
+---
+'@object-ui/app-shell': patch
+'@object-ui/i18n': patch
+---
+
+fix(studio): Studio shows, keeps and authors the optional master of a controlled-by-parent object (#30974)
+
+Clause-②: yes (widening)
+Metadata-protocol: none
+
+Since ADR-0055 §1 as amended (#30905, ruling A), a `controlled_by_parent` object's ONLY `master_detail` may be declared `required: false`: an optional master, whose records may name no master and are then visible only to their owner and to View All Records on the object. Studio still treated every master as required. It now follows the spec:
+
+- **Display.** The package OWD overview names the master the spec's `controlledByParentMasterOf` names, so an optional master is shown as the master instead of "no master". An omitted `required` on the object's only `master_detail` is shown as the master too, the way the builder and stored data read it.
+- **Save.** Switching an object to Controlled by parent (in the OWD overview or the object's Settings tab) still sets `required: true` on each `master_detail` that writes no `required`, and now keeps an explicit `required: false`. A Studio save no longer turns an optional master required.
+- **Authoring.** On the object's only `master_detail`, the field inspector's Required box is no longer locked on: clearing it writes `required: false`, and the help text under it explains the optional master and who can see a record that names none. Beside another `master_detail` the lock stands, since the spec admits no optional master there.
+- **New string.** `@object-ui/i18n`'s studio namespace gains `designer.field.requiredCbpOptionalMaster` (en, zh).

ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.cbpRequiredMaster-12326.test.tsx

@@ -8,11 +8,12 @@
  * Since `@objectstack/spec` 18 (objectstack#23100, ADR-0055 §1) such an object
  * derives its master from exactly one `master_detail` declared
  * `required: true`. Since ADR-0055 §1 as amended (objectstack#30905 ruling A)
- * the spec also reads an object's only `master_detail` that is not required as
- * an OPTIONAL master, whose rows may name none; the designer keeps writing
- * `required: true`, so the master it shows is one every row names. The
- * installed spec's `ObjectSchema` is the oracle for the object each edit
- * leaves behind.
+ * an object's ONLY `master_detail` may be declared `required: false`, an
+ * OPTIONAL master whose rows may name none. On that field the box stays free,
+ * clearing it writes `required: false`, and the help text explains the
+ * null-master owner rule (objectstack#30974). Beside another `master_detail`
+ * the lock stands. The installed spec's `ObjectSchema` is the oracle for the
+ * object each edit leaves behind.
  */
 
 import '@testing-library/jest-dom/vitest';
@@ -89,23 +90,51 @@ function mount(object: Row, selected: string) {
 const requiredBox = () => screen.getByRole('checkbox', { name: t('designer.field.required', 'en-US') });
 
 const REASON = t('designer.field.requiredCbpMaster', 'en-US');
+const OPTIONAL_HELP = t('designer.field.requiredCbpOptionalMaster', 'en-US');
 
 describe('ObjectFieldInspector — a controlled_by_parent master is required, locked (objectui#12326)', () => {
-  it('the Required box of the master is on and locked, with the reason', () => {
-    mount(
+  it('beside another master-detail, the Required box of the master is on and locked, with the reason', () => {
+    const { object } = mount(
       {
         name: 'crm_case',
         sharingModel: 'controlled_by_parent',
-        fields: { account: { type: 'master_detail', reference: 'crm_account', required: true } },
+        fields: {
+          account: { type: 'master_detail', reference: 'crm_account', required: true },
+          contact: { type: 'master_detail', reference: 'crm_contact', required: false },
+        },
       },
       'account',
     );
+    expect(ObjectSchema.safeParse(object()).success).toBe(true);
     expect(requiredBox()).toBeChecked();
     expect(requiredBox()).toBeDisabled();
     expect(screen.getByTestId('field-required-cbp-master')).toHaveTextContent(REASON);
   });
 
-  it('a stored master without required can be ticked, and then no longer cleared', () => {
+  it('the only master-detail can be cleared to an optional master, written required: false (objectstack#30974)', () => {
+    const { object, field } = mount(
+      {
+        name: 'crm_case',
+        sharingModel: 'controlled_by_parent',
+        fields: { account: { type: 'master_detail', reference: 'crm_account', required: true } },
+      },
+      'account',
+    );
+    expect(requiredBox()).toBeChecked();
+    expect(requiredBox()).toBeEnabled();
+    expect(screen.getByTestId('field-required-cbp-master')).toHaveTextContent(OPTIONAL_HELP);
+    fireEvent.click(requiredBox());
+    expect(field().required).toBe(false);
+    expect(requiredBox()).not.toBeChecked();
+    const parsed = ObjectSchema.safeParse(object());
+    expect(parsed.success).toBe(true);
+    expect((parsed.data?.fields as Record<string, { required?: boolean }>).account.required).toBe(false);
+    // And back: ticking it makes the master required again.
+    fireEvent.click(requiredBox());
+    expect(field().required).toBe(true);
+  });
+
+  it('a stored master without required can be ticked, and cleared again to an optional master', () => {
     const { object, field } = mount(
       {
         name: 'crm_case',
@@ -123,10 +152,12 @@ describe('ObjectFieldInspector — a controlled_by_parent master is required, lo
     expect(requiredBox()).toBeEnabled();
     fireEvent.click(requiredBox());
     expect(field().required).toBe(true);
-    expect(requiredBox()).toBeDisabled();
+    expect(requiredBox()).toBeEnabled();
     const after = ObjectSchema.safeParse(object());
     expect(after.success).toBe(true);
     expect((after.data?.fields as Record<string, { required?: boolean }>).account.required).toBe(true);
+    fireEvent.click(requiredBox());
+    expect(field().required).toBe(false);
   });
 
   it('outside controlled_by_parent the box stays free and no reason is shown', () => {
@@ -175,6 +206,6 @@ describe('ObjectFieldInspector — a controlled_by_parent master is required, lo
     await userEvent.click(within(list).getByRole('option', { name: 'Master-Detail' }));
     expect(field()).toEqual({ type: 'master_detail', label: 'Account', reference: 'crm_account', required: true });
     expect(ObjectSchema.safeParse(object()).success).toBe(true);
-    expect(requiredBox()).toBeDisabled();
+    expect(requiredBox()).toBeChecked();
   });
 });

ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx

@@ -1087,12 +1087,23 @@ export function ObjectFieldInspector({
 
   /**
    * objectui#12326 — on a `controlled_by_parent` object a `master_detail` field
-   * is `required: true` and its Required box is locked on: the object derives
-   * its master from exactly one required `master_detail` (ADR-0055 §1), and the
-   * save door refuses any other value. `ObjectSettingsPanel` sets it when the
-   * sharing model is chosen; this inspector keeps it from being cleared.
+   * is `required: true` and its Required box is locked on once ticked: the
+   * object derives its master from exactly one required `master_detail`
+   * (ADR-0055 §1). `ObjectSettingsPanel` sets it when the sharing model is
+   * chosen; this inspector keeps it from being cleared.
+   *
+   * The one exception is the object's ONLY `master_detail` (ADR-0055 §1 as
+   * amended, objectstack#30905 ruling A): it may be an optional master,
+   * declared `required: false`, whose rows may name no master and are then
+   * readable only by their owner and by View All on the object. There the box
+   * stays free, clearing it writes `required: false` (an omitted `required`
+   * would be defaulted back to required), and the help text says what an
+   * optional master means. Beside another `master_detail` the spec admits no
+   * optional master, so the lock stands.
    */
   const cbpMasterDetail = draft.sharingModel === 'controlled_by_parent' && type === 'master_detail';
+  const cbpOptionalMasterAllowed =
+    cbpMasterDetail && view.entries.filter((e) => e.def.type === 'master_detail').length === 1;
 
   /**
    * A retype keeps a picklist binding only on a type the spec lets name one.
@@ -1470,10 +1481,13 @@ export function ObjectFieldInspector({
             <InspectorCheckboxField
               label={tr('designer.field.required')}
               value={!!def.required}
-              onCommit={(v) => patchDef({ required: v || undefined })}
+              // An optional master is written as `required: false`, never by
+              // dropping the key (objectstack#30905 ruling A).
+              onCommit={(v) => patchDef({ required: cbpOptionalMasterAllowed ? v : v || undefined })}
               // Locked once it is on: the box can still be ticked on a stored
-              // master that lacks it, never cleared (objectui#12326).
-              disabled={locked || (cbpMasterDetail && def.required === true)}
+              // master that lacks it, never cleared (objectui#12326), except
+              // on the object's only `master_detail`, which may be optional.
+              disabled={locked || (cbpMasterDetail && !cbpOptionalMasterAllowed && def.required === true)}
             />
             <InspectorCheckboxField
               label={tr('designer.field.unique')}
@@ -1485,7 +1499,7 @@ export function ObjectFieldInspector({
         </DisplayPatchLock>
         {cbpMasterDetail && (
           <p data-testid="field-required-cbp-master" className="text-[11px] text-muted-foreground">
-            {tr('designer.field.requiredCbpMaster')}
+            {tr(cbpOptionalMasterAllowed ? 'designer.field.requiredCbpOptionalMaster' : 'designer.field.requiredCbpMaster')}
           </p>
         )}
         <TextareaField

ui/packages/app-shell/src/views/studio-design/ObjectSettingsPanel.tsx

@@ -207,9 +207,11 @@ export function ObjectSettingsPanel({
     { value: 'controlled_by_parent', label: t('engine.studio.settings.sharingControlledByParent', locale) },
   ];
   // objectui#12326 — choosing `controlled_by_parent` sets `required: true` on
-  // every `master_detail` field in the same patch: the spec admits no other
-  // value there (ADR-0055 §1), and the save door refuses an omitted one. The
-  // fields are written only when that changes them.
+  // each `master_detail` field that writes no `required`, in the same patch:
+  // the save door would read the omission as an optional master. An explicit
+  // `required: false` is an optional master the author chose (ADR-0055 §1 as
+  // amended, objectstack#30905 ruling A) and is kept. The fields are written
+  // only when that changes them.
   const pickSharingModel = (v: string) => {
     if (!v) {
       onPatch({ sharingModel: undefined });

ui/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.cbpRequiredMaster-12326.test.tsx

@@ -6,18 +6,13 @@
  *
  * Since `@objectstack/spec` 18 (objectstack#23100, ADR-0055 §1) the master is
  * the ONE `master_detail` declared `required: true`; zero or two or more derive
- * none. The row shows that master, or says there is none. Switching a row to
- * `controlled_by_parent` sets `required: true` on its `master_detail` fields in
- * the saved draft, as the per-object Settings tab does. The installed spec's
+ * none. ADR-0055 §1 as amended (objectstack#30905 ruling A) adds an OPTIONAL
+ * master: an object's only `master_detail` declared `required: false`. The row
+ * shows the master, required or optional, or says there is none. Switching a
+ * row to `controlled_by_parent` sets `required: true` on each `master_detail`
+ * that writes no `required`, as the per-object Settings tab does, and keeps an
+ * optional master as written (objectstack#30974). The installed spec's
  * `ObjectSchema` is the oracle for the saved body.
- *
- * ADR-0055 §1 as amended (objectstack#30905 ruling A) adds an OPTIONAL master:
- * an object's only `master_detail` declared `required: false`. The parse cannot
- * tell an omitted `required` from `false`, so it admits the omitted shape too.
- * This overview still names none for an omitted `required`, as the runtime
- * does for an object that reaches it unparsed (it reads `required: false`
- * literally, never its absence), and that row records the parse's answer
- * apart from the master shown.
  */
 
 import '@testing-library/jest-dom/vitest';
@@ -62,15 +57,14 @@ afterEach(cleanup);
 const cbp = (fields: Body): Body => ({ name: 'crm_case', label: 'Case', sharingModel: 'controlled_by_parent', fields });
 
 describe('PackageOwdOverviewPanel — the master shown is the one the runtime derives (objectui#12326)', () => {
-  // [label, fields, the master shown, whether the spec parse accepts it when
-  // that differs from "a master is shown"]
-  const SHAPES: Array<[string, Body, string | undefined, boolean?]> = [
+  // [label, fields, the master shown]
+  const SHAPES: Array<[string, Body, string | undefined]> = [
     ['one required master', { account: { type: 'master_detail', reference: 'crm_account', required: true } }, 'crm_account'],
+    ['one master without required', { account: { type: 'master_detail', reference: 'crm_account' } }, 'crm_account'],
     [
-      'one master without required (the parse reads it as an optional master; no master is shown)',
-      { account: { type: 'master_detail', reference: 'crm_account' } },
-      undefined,
-      true,
+      'one optional master (required: false)',
+      { account: { type: 'master_detail', reference: 'crm_account', required: false } },
+      'crm_account',
     ],
     [
       'two required masters',
@@ -82,10 +76,9 @@ describe('PackageOwdOverviewPanel — the master shown is the one the runtime de
     ],
   ];
 
-  it.each(SHAPES)('%s', async (_label, fields, master, parses) => {
-    // The oracle: the spec accepts the shapes a master is shown for, and the
-    // one row that says otherwise.
-    expect(ObjectSchema.safeParse(cbp(fields)).success).toBe(parses ?? master !== undefined);
+  it.each(SHAPES)('%s', async (_label, fields, master) => {
+    // The oracle: the spec accepts exactly the shapes a master is shown for.
+    expect(ObjectSchema.safeParse(cbp(fields)).success).toBe(master !== undefined);
     renderPanel({ crm_case: cbp(fields) });
     const cell = await screen.findByTestId('owd-master-crm_case');
     expect(cell).toHaveTextContent(
@@ -127,6 +120,30 @@ describe('PackageOwdOverviewPanel — switching a row to controlled_by_parent (o
     expect((parsed.data?.fields as Record<string, { required?: boolean }>).account.required).toBe(true);
   });
 
+  it('keeps an optional master as written: the save never turns required: false required (objectstack#30974)', async () => {
+    const body: Body = {
+      name: 'crm_case',
+      label: 'Case',
+      sharingModel: 'private',
+      fields: { account: { type: 'master_detail', reference: 'crm_account', required: false } },
+    };
+    const { saved } = renderPanel({ crm_case: body });
+    await screen.findByTestId('owd-row-crm_case');
+    fireEvent.keyDown(screen.getByTestId('owd-internal-crm_case'), { key: 'ArrowDown' });
+    fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: CBP_LABEL }));
+    expect(screen.getByTestId('owd-master-crm_case')).toHaveTextContent(
+      tFormat('engine.studio.owd.byParent', 'en-US', { master: 'crm_account' }),
+    );
+
+    fireEvent.click(screen.getByTestId('owd-save'));
+    await waitFor(() => expect(saved.length).toBe(1));
+    expect(saved[0].body.sharingModel).toBe('controlled_by_parent');
+    expect((saved[0].body.fields as Record<string, Body>).account.required).toBe(false);
+    const parsed = ObjectSchema.safeParse(saved[0].body);
+    expect(parsed.success).toBe(true);
+    expect((parsed.data?.fields as Record<string, { required?: boolean }>).account.required).toBe(false);
+  });
+
   it('a row with two master-detail fields shows no master before the save', async () => {
     renderPanel({
       crm_case: {

ui/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx

@@ -54,7 +54,7 @@ import {
 import { errorCodeIs } from '@object-ui/types';
 import { readFields } from '../metadata-admin/previews/object-fields-io.js';
 import { t, tFormat, type SupportedLocale } from '../metadata-admin/i18n.js';
-import { deriveMasterObject, requireCbpMasterDetails } from './owd-sharing.js';
+import { cbpMasterDetailDefaultsRequired, deriveMasterObject, requireCbpMasterDetails } from './owd-sharing.js';
 import { toast } from 'sonner';
 
 
@@ -69,13 +69,14 @@ interface OwdRow {
   internal: string;
   /**
    * The master (parent) object the row derives under `controlled_by_parent`:
-   * the reference of its one required `master_detail` (objectui#12326). For a
-   * row loaded as `controlled_by_parent` it is read from the body as stored,
-   * which is what the runtime derives. For any other row it is the master the
-   * row would derive once switched and saved here, since that save sets
-   * `required: true` on its `master_detail` fields. Undefined = no master: zero
-   * `master_detail` fields, or two or more, an authoring error the save door
-   * refuses.
+   * the reference of its one master `master_detail`, required or optional
+   * (objectui#12326, objectstack#30905 ruling A). For a row loaded as
+   * `controlled_by_parent` it is read from the body as stored, which is what
+   * the runtime derives. For any other row it is the master the row would
+   * derive once switched and saved here, since that save sets `required: true`
+   * on each `master_detail` that writes no `required`. Undefined = no master:
+   * zero `master_detail` fields, or two or more with no one master, an
+   * authoring error the save door refuses.
    */
   master?: string;
   /** True when this object already has an unpublished draft. */
@@ -138,7 +139,7 @@ function owdLabel(value: string, locale: SupportedLocale): string {
  */
 function cbpPrerequisite(fields: unknown): string[] {
   return readFields(fields)
-    .entries.filter((e) => e.def.type === 'master_detail' && e.def.required !== true)
+    .entries.filter((e) => cbpMasterDetailDefaultsRequired(e.def))
     .map((e) => `${e.name} → ${typeof e.def.reference === 'string' ? e.def.reference : '?'}`);
 }
 
@@ -399,8 +400,10 @@ export function PackageOwdOverviewPanel({
         if (s.edit.internal) next.sharingModel = s.edit.internal;
         else delete next.sharingModel;
         // objectui#12326 — switching to `controlled_by_parent` sets
-        // `required: true` on every `master_detail` field, as the per-object
-        // Settings tab does: the save door refuses an omitted one there.
+        // `required: true` on each `master_detail` field that writes no
+        // `required`, as the per-object Settings tab does: the save door would
+        // read the omission as an optional master. An explicit
+        // `required: false` is an optional master and is kept.
         let relations: string[] = [];
         if (s.edit.internal === 'controlled_by_parent' && next.fields !== undefined) {
           relations = cbpPrerequisite(next.fields);