From fd8df8399641485567bd3657b31491ca7a7d5c79 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 23:45:01 +0000 Subject: [PATCH] fix(plugin-form): decline to fetch a detail collection with no child object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `object-master-detail-form`'s detail resolve effect called `getObjectSchema(d.childObject)` with no guard on the key being resolved. A malformed detail entry therefore made the block ask the data layer for an object literally named `undefined` — a real backend receives that query and whatever it returns becomes the console's problem. The effect now skips such an entry and warns, leaving it in place so the grid card shows its config hint and the row-state array stays index-matched. This is the choice `RelatedList` already makes for the same class of missing key, and the sibling child-schema-cache effect in this same component already spelled it `.filter(Boolean)`. The binding-reach probe's generic array sample left `details` as `['name']`, which the declared entry shape (`MasterDetailDetailConfig.childObject` is required) rules out as a publishable configuration; the sample is corrected on that reading, not on the bad call going away. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe --- ...5940-detail-collection-decline-to-fetch.md | 20 +++ .../public-block-binding-reach.test.tsx | 22 +++ ...tailForm.detailChildObjectDecline.test.tsx | 133 ++++++++++++++++++ packages/plugin-form/src/MasterDetailForm.tsx | 19 +++ 4 files changed, 194 insertions(+) create mode 100644 .changeset/5940-detail-collection-decline-to-fetch.md create mode 100644 packages/plugin-form/src/MasterDetailForm.detailChildObjectDecline.test.tsx diff --git a/.changeset/5940-detail-collection-decline-to-fetch.md b/.changeset/5940-detail-collection-decline-to-fetch.md new file mode 100644 index 0000000000..2703d95554 --- /dev/null +++ b/.changeset/5940-detail-collection-decline-to-fetch.md @@ -0,0 +1,20 @@ +--- +"@object-ui/plugin-form": patch +--- + +`object-master-detail-form` declines to fetch a detail collection whose child object it never resolved, instead of calling `getObjectSchema(undefined)`. + +`childObject` is REQUIRED on `MasterDetailDetailConfig` and is what every downstream read is keyed +on — `deriveDetail(d.childObject, …)`, the child-schema cache, and the FK scope of each child +fetch. But a detail entry reaches the renderer straight off an authored schema, so a malformed one +arrives with the key `undefined`, and the resolve effect asked the data layer for it anyway. +Measured: mounting the block with a detail entry that carries no `childObject` issued +`getObjectSchema(undefined)` — a real backend receives a query for an object literally named +`undefined`, and whatever it returns becomes the console's problem. + +The resolve effect now skips such an entry and warns, leaving it in place so the grid card shows +its config hint and the row-state array stays index-matched. This is the choice `RelatedList` +already makes for the same class of missing key (*"has no referenceField/parentId — refusing to +fetch all rows"*), and the sibling child-schema-cache effect in this same component already spelled +it `.filter(Boolean)`; the three now agree. A detail collection that names its child object fetches +exactly as before. diff --git a/apps/console/src/__tests__/public-block-binding-reach.test.tsx b/apps/console/src/__tests__/public-block-binding-reach.test.tsx index e4ad490e4c..b7ea137a9f 100644 --- a/apps/console/src/__tests__/public-block-binding-reach.test.tsx +++ b/apps/console/src/__tests__/public-block-binding-reach.test.tsx @@ -302,6 +302,28 @@ const sampleFor = (input: any): unknown => { return [{ name: 'probe_section', label: 'Probe Section', fields: ['name'] }]; } if (input.name === 'formType') return 'simple'; + // `object-master-detail-form.details` is the SEVENTH instance of the lesson, + // and it is keyed by NAME for the same reason `sections` is: the declared TYPE + // is `array`, which carries no information about the ENTRY. Decided on the + // DECLARED SHAPE, not on the bad call going away (the #3840 discriminator): + // an entry is `MasterDetailDetailConfig` (MasterDetailForm.tsx), whose + // `childObject: string` is REQUIRED and is what every downstream read is keyed + // on — `deriveDetail(d.childObject, …)`, the child-schema cache, and the FK + // scope of each child fetch. A bare `'name'` is therefore not a detail + // collection any author could publish: the generic sample left `childObject` + // `undefined`, and the renderer asked the data layer for an object literally + // named `undefined` (objectui#5940). That defect is fixed at the source — the + // renderer now declines to fetch, matching `RelatedList` — so this sample is + // spec-valid on its own merit, NOT as a way to stop the bad call. + // Only `childObject` is set: everything else on the entry is optional and + // derived from the child's metadata, so this is the minimal publishable + // configuration, and leaving it minimal keeps the derive path (a real data + // reach) exercised instead of short-circuited. `PROBE_OBJECT` as the child + // follows `add` above, which names it inside a nested object key for the same + // reason: it is the only object this fixture declares. + if (input.name === 'details') { + return [{ childObject: PROBE_OBJECT, title: 'Probe Detail' }]; + } if (input.defaultValue !== undefined) return input.defaultValue; switch (input.type) { case 'number': diff --git a/packages/plugin-form/src/MasterDetailForm.detailChildObjectDecline.test.tsx b/packages/plugin-form/src/MasterDetailForm.detailChildObjectDecline.test.tsx new file mode 100644 index 0000000000..f3db043fd4 --- /dev/null +++ b/packages/plugin-form/src/MasterDetailForm.detailChildObjectDecline.test.tsx @@ -0,0 +1,133 @@ +/** + * 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. + * + * `object-master-detail-form` must DECLINE TO FETCH a detail collection whose + * child object it never resolved — not call `getObjectSchema(undefined)` + * (objectui#5940). + * + * `childObject` is REQUIRED on `MasterDetailDetailConfig`, but a detail entry + * reaches the renderer straight off an authored schema, so a malformed one + * arrives with the key `undefined`. The renderer asked the data layer for it + * anyway: a real backend receives a query for an object literally named + * `undefined` and whatever it returns becomes the console's problem. + * `RelatedList` already takes the other choice for the same class of missing key + * ("has no referenceField/parentId — refusing to fetch all rows"), which is what + * makes this a defect in one component rather than an open question. + * + * ## Why these assertions read the FULL CALL LIST + * + * This call is INVISIBLE to the binding-reach probe + * (`apps/console/src/__tests__/public-block-binding-reach.test.tsx`), which asks + * whether *any* call carried the object name — the first, correct call already + * satisfies it. That probe was GREEN for as long as this defect was live, so a + * green probe is not evidence and neither is any assertion of the same shape. + * The defect surfaced only because the full list was read, and only an + * exact-list assertion can keep it from reopening exactly as it opened. + * + * ## Why the second test is not redundant + * + * ⭐ A "fix" that declined to fetch *everything* would also make the bad call + * disappear and would pass an absence-only assertion. Both directions are + * therefore pinned: the unresolvable detail is NOT fetched, and a well-formed + * one still IS. Measured against this file's own fixture before the guard + * landed: `['getObjectSchema("probe_object__c")', 'getObjectSchema(undefined)']`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, act } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +// Registers `object-master-detail-form`. +import './index'; + +const PROBE_OBJECT = 'probe_object__c'; + +/** + * The recording data source, deliberately the same shape as the binding-reach + * probe's (`dataCallsFor`): a Proxy, so ANY method the block reaches for is + * recorded rather than crashing it, and every call is stringified with its + * arguments so `undefined` is visible in the list rather than collapsing to an + * empty argument. + */ +function recordingDataSource(calls: string[]) { + const record = + (key: string) => + (...args: unknown[]) => { + calls.push(`${key}(${args.map((a) => JSON.stringify(a) ?? 'undefined').join(', ')})`); + return /^on[A-Z]/.test(key) || key === 'subscribe' ? () => {} : Promise.resolve([]); + }; + const seeded: Record = {}; + for (const m of ['find', 'findOne', 'create', 'update', 'delete', 'aggregate', 'getObjectSchema']) { + seeded[m] = record(m); + } + return new Proxy(seeded, { + get: (t, k: string) => (k in t ? (t as any)[k] : record(k)), + }) as any; +} + +async function callsFor(details: unknown): Promise { + const calls: string[] = []; + const schema: any = { + type: 'object-master-detail-form', + objectName: PROBE_OBJECT, + mode: 'create', + formType: 'simple', + details, + }; + const view = render( + + + , + ); + // Settle: the detail resolution runs in an effect, and a second pass follows + // once the object schema lands. + for (let i = 0; i < 10; i++) { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + } + try { + view.unmount(); + } catch { + /* teardown is not the subject */ + } + return calls; +} + +describe('object-master-detail-form — a detail collection with no child object (objectui#5940)', () => { + it('declines to fetch instead of calling getObjectSchema(undefined)', async () => { + // The #3840 binding-reach fixture's generic array sample: each entry is a + // bare string, so `childObject` is `undefined` on every one of them. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const calls = await callsFor(['name']); + + // The FULL LIST, not `.not.toContain(...)`: an absence-only assertion is the + // same shape that read green while the defect was live, and it would also + // pass for a renderer that stopped fetching altogether. Pinning the exact + // list states both halves at once — the parent binding is STILL made, and + // nothing else is. + expect(calls).toEqual([`getObjectSchema("${PROBE_OBJECT}")`]); + + // Stated separately so a failure names which half broke. + expect(calls).toContain(`getObjectSchema("${PROBE_OBJECT}")`); + expect(calls).not.toContain('getObjectSchema(undefined)'); + + // Declining silently would leave an author with an empty grid and no reason; + // `RelatedList` warns for the same case, so both components fail the same way. + expect(warn).toHaveBeenCalledWith(expect.stringContaining('childObject')); + warn.mockRestore(); + }); + + it('still fetches the schema of a detail collection that names its child object', async () => { + // ⭐ The other direction. Without this, a renderer that declined to fetch + // EVERY detail would pass the test above. + const calls = await callsFor([{ childObject: 'invoice_line', title: 'Invoice lines' }]); + + expect(calls).toContain('getObjectSchema("invoice_line")'); + expect(calls).not.toContain('getObjectSchema(undefined)'); + }); +}); diff --git a/packages/plugin-form/src/MasterDetailForm.tsx b/packages/plugin-form/src/MasterDetailForm.tsx index 81b4dc5273..251a354c1e 100644 --- a/packages/plugin-form/src/MasterDetailForm.tsx +++ b/packages/plugin-form/src/MasterDetailForm.tsx @@ -337,6 +337,25 @@ export const MasterDetailForm: React.FC = ({ const columnsTyped = d.columns?.length ? d.columns.every((c) => !!c.type) : false; // Fully configured (FK + every column typed) — nothing to resolve. if (d.relationshipField && columnsTyped) return d; + // Decline to fetch when the child object never resolved (objectui#5940). + // `childObject` is REQUIRED on `MasterDetailDetailConfig`, but a detail + // entry reaches this renderer straight off an authored schema, so a + // malformed one (or a bare string) arrives with it `undefined` — and the + // fetch below then asked the data layer for an object literally named + // `undefined`. A real backend receives that query and whatever it returns + // becomes the console's problem. `RelatedList` already takes the other + // choice for the same class of missing key ("has no referenceField/parentId + // — refusing to fetch all rows", RelatedList.tsx), and the sibling effect + // below already spells it `.filter(Boolean)`; this makes the three agree. + // Left as-is rather than dropped, exactly like the `catch` below — the + // grid card shows a config hint, and `details` stays length-matched to + // `rawDetails` (the row-state array is indexed against it). + if (!d.childObject) { + console.warn( + `[MasterDetailForm] a detail collection has no childObject — refusing to fetch its schema. Set childObject to the child object the collection lists.`, + ); + return d; + } try { const childSchema = await dataSource.getObjectSchema(d.childObject); // Author gave the FK + an explicit column set but left some columns