diff --git a/.changeset/6188-line-items-child-object-decline.md b/.changeset/6188-line-items-child-object-decline.md new file mode 100644 index 000000000..13a4a04f6 --- /dev/null +++ b/.changeset/6188-line-items-child-object-decline.md @@ -0,0 +1,20 @@ +--- +"@object-ui/plugin-form": patch +--- + +`record:line_items` declines to fetch the child schema of a panel whose child object it never resolved, instead of calling `getObjectSchema(undefined)`. + +`childObject` is declared `required: true` on the block's registry entry and typed `string` on +`LineItemsPanelSchema`, but nothing enforces either — `inputs[].required` is designer metadata, and +the block has no spec schema — so a node reaches the renderer straight off an authored schema with +the key `undefined`, and the child-schema effect asked the data layer for it anyway. Measured: +mounting the block through the registry with `childObject` unset issued +`getObjectSchema(undefined)`, and a real backend receives a query for an object literally named +`undefined`. The effect's `.catch` then turned the answer into a null child schema, so the visible +outcome was a silently unsanitized child grid rather than an error. + +The effect now declines and warns, naming the key and what to set it to, and clears the cached child +schema so a later save is never sanitized against a previous object's fields. 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 one `object-master-detail-form` makes on this exact key. A +panel that names its child object fetches exactly as before. diff --git a/packages/plugin-form/src/LineItemsPanel.childObjectDecline.test.tsx b/packages/plugin-form/src/LineItemsPanel.childObjectDecline.test.tsx new file mode 100644 index 000000000..45fd4ae62 --- /dev/null +++ b/packages/plugin-form/src/LineItemsPanel.childObjectDecline.test.tsx @@ -0,0 +1,148 @@ +/** + * 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. + * + * `record:line_items` must DECLINE TO FETCH the child schema of a panel whose + * child object it never resolved — not call `getObjectSchema(undefined)` + * (objectui#6188). + * + * Same defect, same key name and same package as objectui#5940, which fixed the + * sibling site in `MasterDetailForm`. `childObject` is declared + * `required: true` on the registry entry for `record:line_items` + * (`index.tsx`) and is typed `string` on `LineItemsPanelSchema`, but NOTHING + * enforces either: `inputs[].required` is designer metadata (WidgetRegistry + * copies it onto the ComponentRegistry entry and no one parses a node against + * it), and the block has no spec schema at all — `@objectstack/spec` names + * `record:line_items` only as an example of a type authored in the wild outside + * its union. So a node reaches this renderer straight off an authored schema + * with the key `undefined`, and the effect asked the data layer for it anyway. + * + * ## Why the assertions read a CALL LIST + * + * The symptom here is quieter than #5940's: the effect's `.catch` turns + * whatever a real backend returns for an object literally named `undefined` + * into a NULL child schema, and a null child schema is exactly what the panel + * holds before any fetch resolves. So "it did not crash" and "the schema is + * null" were both TRUE while the defect was live — no assertion on resulting + * state could have caught it. Only the calls themselves distinguish the two + * worlds. + * + * ## 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 unresolved panel does NOT fetch its child schema, and a + * well-formed one still DOES. + */ + +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 `record:line_items`. +import './index'; + +const COLUMNS = [{ name: 'qty', label: 'Qty', type: 'number' as const }]; + +/** + * The recording data source, the same shape objectui#5940's test used: a Proxy, + * so ANY method the block reaches for is recorded rather than crashing it. + * + * Each call is recorded as `method(firstArgument)` — the OBJECT NAME argument, + * which is the whole subject here, and `undefined` is spelled out rather than + * collapsing to an empty argument. Deliberately NOT the full argument list: the + * child fetch also carries `$filter` / `$top`, and pinning those would make this + * file fail for changes that have nothing to do with the object name. + */ +function recordingDataSource(calls: string[]) { + const record = + (key: string) => + (...args: unknown[]) => { + calls.push(`${key}(${args.length === 0 ? '' : (JSON.stringify(args[0]) ?? 'undefined')})`); + return /^on[A-Z]/.test(key) || key === 'subscribe' ? () => {} : Promise.resolve({ data: [] }); + }; + 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(schemaExtra: Record): Promise { + const calls: string[] = []; + const schema: any = { + type: 'record:line_items', + relationshipField: 'invoice', + // Authored directly, as `LineItemsPanel.elementDataSource.test.tsx` does: + // this panel is bound to an EXISTING parent record, so a fixture without one + // would settle the question for a panel no author ships. + parentId: 'inv-1', + columns: COLUMNS, + ...schemaExtra, + }; + const view = render( + + + , + ); + // Settle: both reads run in effects, and the row load runs a second pass once + // the first response 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('record:line_items — a panel with no childObject (objectui#6188)', () => { + it('declines to fetch the child schema instead of calling getObjectSchema(undefined)', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const calls = await callsFor({}); + + // The FULL LIST, not `.not.toContain(...)`: an absence-only assertion would + // also pass for a panel that stopped fetching altogether, and the list is + // what makes the remaining entry visible instead of implied. + // + // ⚠️ `find(undefined)` IS STILL HERE AND IS STILL WRONG. It is the SIBLING + // site in this same component — `load()` guards `dataSource` and `parentId` + // but not `schema.childObject`, so the row fetch still queries an object + // literally named `undefined`. It is deliberately NOT fixed by objectui#6188, + // whose dispatch order scoped this card to the `getObjectSchema` call and + // said to FILE any further unguarded sub-key site rather than fix it; filed + // as objectui#6194. Pinned here rather than hidden behind a narrower fixture + // so the hole is recorded where the next reader will see it. When #6194 + // lands, this expectation becomes `toEqual([])` and the line below it goes. + expect(calls).toEqual(['find(undefined)']); + + // Stated separately so a failure names which half broke. + expect(calls).not.toContain('getObjectSchema(undefined)'); + + // Declining silently would leave an author with a grid that is quietly + // unsanitized and no reason why; `RelatedList` warns for the same class of + // missing key ("has no referenceField/parentId — refusing to fetch all + // rows"), and the warning names the key and what to set it to rather than + // reporting that something was undefined. + expect(warn).toHaveBeenCalledWith(expect.stringContaining('childObject')); + warn.mockRestore(); + }); + + it('still fetches the child schema of a panel that names its child object', async () => { + // ⭐ The other direction. Without this, a panel that declined to fetch + // EVERYTHING would pass the test above. + const calls = await callsFor({ childObject: 'invoice_line' }); + + expect(calls).toEqual(['getObjectSchema("invoice_line")', 'find("invoice_line")']); + expect(calls).not.toContain('getObjectSchema(undefined)'); + }); +}); diff --git a/packages/plugin-form/src/LineItemsPanel.tsx b/packages/plugin-form/src/LineItemsPanel.tsx index 3ddcbfb90..234898c05 100644 --- a/packages/plugin-form/src/LineItemsPanel.tsx +++ b/packages/plugin-form/src/LineItemsPanel.tsx @@ -111,6 +111,28 @@ export const LineItemsPanel: React.FC<{ schema: LineItemsPanelSchema }> = ({ sch useEffect(() => { const ds: any = dataSource; if (!ds || typeof ds.getObjectSchema !== 'function') return; + // Decline to fetch when the child object never resolved (objectui#6188). + // `childObject` is declared `required: true` on this block's registry entry + // and typed `string` above, but nothing enforces either — `inputs[].required` + // is designer metadata, and the block has no spec schema — so a node reaches + // this renderer straight off an authored schema with the key `undefined`, and + // the fetch below then asked the data layer for an object literally named + // `undefined`. `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 `MasterDetailForm` declines on this exact key + // (objectui#5940) with its child-schema cache spelling it `.filter(Boolean)`. + // + // Clearing the cache rather than just returning: an unresolvable panel HAS no + // child schema, and leaving a previous object's schema in place would sanitize + // the next save against the wrong object's fields. `null` is what the `.catch` + // below already produces, so the sanitize path needs no new case. + if (!schema.childObject) { + setChildSchema(null); + console.warn( + `[LineItemsPanel] a line-items panel has no childObject — refusing to fetch its child schema. Set childObject to the child object the panel lists.`, + ); + return; + } let cancelled = false; ds.getObjectSchema(schema.childObject) .then((s: any) => { if (!cancelled) setChildSchema(s ?? null); })