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 000000000..2703d9555 --- /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 e4ad490e4..b7ea137a9 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 000000000..f3db043fd --- /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 81b4dc527..251a354c1 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