Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/5940-detail-collection-decline-to-fetch.md
Original file line numberDiff line numberDiff line change
@@ -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.
22 changes: 22 additions & 0 deletions apps/console/src/__tests__/public-block-binding-reach.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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':
Expand Down
Original file line numberDiff line numberDiff line change
@@ -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<string, unknown> = {};
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<string[]> {
const calls: string[] = [];
const schema: any = {
type: 'object-master-detail-form',
objectName: PROBE_OBJECT,
mode: 'create',
formType: 'simple',
details,
};
const view = render(
<SchemaRendererProvider dataSource={recordingDataSource(calls)}>
<SchemaRenderer schema={schema} />
</SchemaRendererProvider>,
);
// 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)');
});
});
19 changes: 19 additions & 0 deletions packages/plugin-form/src/MasterDetailForm.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -337,6 +337,25 @@ export const MasterDetailForm: React.FC<MasterDetailFormProps> = ({
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
Expand Down
Loading