fix(plugin-form): a detail that loaded but could not DERIVE gets a config hint naming relationshipField

.changeset/6394-master-detail-underivable-detail-config-hint.md

@@ -0,0 +1,31 @@
+---
+'@object-ui/plugin-form': patch
+---
+
+`MasterDetailForm` shows a config hint naming `relationshipField` for a detail collection
+whose child schema **loaded fine but could not be derived from**, instead of a permanent
+`Loading columns…` (objectui#6394).
+
+This is the third and last arm of the same resolver to be closed. `deriveDetail` throws
+when no lookup/`master_detail` field on the child object references the parent — a
+configuration error whose remedy is a key the author writes. The `catch` returned the
+entry unresolved, so it fell through to `!d.columns?.length ? <p>Loading columns…</p>`,
+and that message never ended: the derive is not retried, so those columns could never
+arrive. Same unbounded-wait-shown-as-a-spinner family as objectui#5940 / objectui#6188 /
+objectui#6194 / objectui#6360 / objectui#6372.
+
+The entry now carries `status: 'underivable'`, and the renderer gives it a branch of its
+own that names both ends of the relationship it could not find and the key to set:
+
+> Could not work out how `po_line` links to `purchase_order`: no lookup or master_detail
+> field on it references the parent. Set `relationshipField` on this collection to the
+> field that holds the parent record.
+
+⛔ Deliberately **not** objectui#6372's refusal placeholder, which states the schema could
+not be loaded — false for a schema that loaded fine. The two failures keep separate copy
+because they have different remedies: one is "check the object exists and reload", this one
+is "set this key". The thrown error is still logged with its stack (objectui#6372), since
+the placeholder shows the author the key rather than the raw message.
+
+Behaviour is unchanged for the other two arms and for a detail that is genuinely still
+fetching — that one keeps `Loading columns…`, where the message is true.

packages/plugin-form/src/MasterDetailForm.detailChildObjectDecline.test.tsx

@@ -145,8 +145,15 @@ describe('object-master-detail-form — a detail collection with no child object
  *
  * ⭐ Both directions are pinned here for the same reason the fetch half pins
  * both: an assertion that only says "no `Loading columns…`" would also pass for
- * a renderer that deleted the loading branch outright. So the second test holds
- * the loading branch alive for the detail that legitimately IS still resolving.
+ * a renderer that showed this hint for every detail. So the second test drives
+ * a detail that DOES name its child object and reads what that one gets
+ * instead.
+ *
+ * ⚠️ That second test used to hold the loading branch alive, on the belief that
+ * its detail was still resolving. It is not: this file's recording data source
+ * resolves `getObjectSchema` to `[]`, so the fetch succeeds and the DERIVE
+ * throws (objectui#6394). The loading branch's honest control — a fetch that
+ * never settles — lives in `MasterDetailForm.detailSchemaFetchFailure.test.tsx`.
  */
 
 async function renderDetails(details: unknown) {
@@ -200,14 +207,39 @@ describe('object-master-detail-form — what a declined detail RENDERS (objectui
     view.unmount();
   });
 
-  it('does NOT show the hint for a detail that names its child object, and still says "Loading columns…" while it resolves', async () => {
-    // ⭐ The other direction. Without this, deleting the loading branch
-    // altogether would pass the test above.
+  it('does NOT show the `childObject` hint for a detail that names its child object — whose fixture reaches the FAILED DERIVE, hinted at `relationshipField` (objectui#6394)', async () => {
+    // ⭐ The other direction, unchanged in purpose: without this, a renderer
+    // that showed the `childObject` hint for every unresolved detail would pass
+    // the test above.
+    //
+    // ⚠️ This assertion's PREMISE was rewritten under objectui#6394's triage
+    // ruling, which explicitly authorized moving this ONE objectui#6360
+    // assertion (and no other in this file). Its title used to claim the detail
+    // "still says `Loading columns…` while it resolves" — but this file's
+    // recording data source resolves `getObjectSchema` to `[]`, so for
+    // `invoice_line` the fetch SUCCEEDS and `deriveDetail` then throws: the
+    // entry is not resolving at all, it is permanently underivable, and the
+    // message it pinned could never end. The loading branch this believed it
+    // was holding alive now has an honest control that drives a never-settling
+    // promise (`MasterDetailForm.detailSchemaFetchFailure.test.tsx`, "DEGENERATE
+    // CONTROL: a detail that is still IN FLIGHT keeps "Loading columns…""),
+    // so naming what this fixture really hits loses no coverage.
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
     const { view } = await renderDetails([{ childObject: 'invoice_line', title: 'Invoice lines' }]);
 
+    // The original claim, kept verbatim: the `!childObject` hint belongs to the
+    // OTHER arm and must not appear for a detail that named its child object.
     expect(view.queryByTestId('md-detail-no-child-object')).toBeNull();
-    expect(view.container.textContent).toContain('Loading columns…');
 
+    // What this fixture actually reaches, now that the arm renders it: the
+    // config hint naming the key to set, in place of a "Loading columns…" that
+    // could never end (objectui#6394).
+    const hint = view.getByTestId('md-detail-no-relationship-field');
+    expect(hint.textContent).toContain('relationshipField');
+    expect(hint.textContent).toContain('invoice_line');
+    expect(view.container.textContent).not.toContain('Loading columns…');
+
+    warn.mockRestore();
     view.unmount();
   });
 });

packages/plugin-form/src/MasterDetailForm.detailDeriveFailure.test.tsx

@@ -0,0 +1,247 @@
+/**
+ * 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 not show a PERMANENT "Loading columns…" for a
+ * detail collection whose schema LOADED FINE but failed to DERIVE
+ * (objectui#6394).
+ *
+ * ## The arm this covers, and how it differs from the two already closed
+ *
+ * `MasterDetailForm`'s detail-resolution effect has THREE arms that return an
+ * entry with no usable columns:
+ *
+ *   1. `!d.childObject` — the objectui#5940 decline, rendered since
+ *      objectui#6360 as the `md-detail-no-child-object` config hint.
+ *   2. the FETCH threw — objectui#6372, rendered as the
+ *      `md-detail-schema-unavailable` refusal placeholder.
+ *   3. the fetch SUCCEEDED and `deriveDetail` threw — this card. The child
+ *      schema carries no lookup/master_detail field referencing the parent, so
+ *      no relationship field can be found.
+ *
+ * Arm 3 used to return the entry unresolved, so it fell through to
+ * `Loading columns…` — permanently, because the derive is not retried any more
+ * than the fetch is.
+ *
+ * ## Why it is neither of the other two placeholders
+ *
+ * ⭐ Reusing objectui#6372's refusal would state that the schema could not be
+ * loaded, which is FALSE here — the fetch resolved. Triage ruled option 2 for
+ * exactly that reason ("dishonest copy for a schema that loaded fine"): this is
+ * a CONFIGURATION error with a named remedy the code already holds, so it takes
+ * the config-hint shape and NAMES `relationshipField` as the key to set.
+ *
+ * ## Three arms, three outcomes
+ *
+ * ⭐ Every assertion about arm 3 is paired with a control, because a renderer
+ * that showed this hint for EVERY unresolved detail would satisfy the first
+ * test alone: a still-pending detail must keep "Loading columns…" (that message
+ * is TRUE there), a failed FETCH must keep objectui#6372's refusal, and a
+ * detail with no `childObject` must keep objectui#6360's hint.
+ */
+
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, waitFor } from '@testing-library/react';
+import React from 'react';
+import { registerAllFields } from '@object-ui/fields';
+import { MasterDetailForm } from './MasterDetailForm';
+
+registerAllFields();
+
+const PARENT = 'purchase_order';
+const parentSchema = { name: PARENT, fields: { ref: { type: 'text', label: 'Ref' } } };
+
+/**
+ * A child schema that LOADS FINE and cannot be derived from: it carries no
+ * lookup/master_detail field referencing the parent, which is exactly what
+ * `deriveDetail` throws on ("could not find a lookup/master_detail field on
+ * \"po_line\" referencing \"purchase_order\". Set relationshipField explicitly.").
+ */
+const underivableChildSchema = {
+  name: 'po_line',
+  fields: {
+    amount: { type: 'number', label: 'Amount' },
+    note: { type: 'text', label: 'Note' },
+  },
+};
+
+/** The same child, but carrying the FK — this one resolves all the way. */
+const resolvableChildSchema = {
+  name: 'po_line',
+  fields: {
+    po: { type: 'master_detail', reference: PARENT, label: 'PO' },
+    amount: { type: 'number', label: 'Amount' },
+  },
+};
+
+function dataSourceServing(childSchema: any, failing: string[] = []) {
+  return {
+    getObjectSchema: vi.fn(async (obj: string) => {
+      if (failing.includes(obj)) throw new Error(`SCHEMA_UNAVAILABLE: ${obj}`);
+      if (obj === PARENT) return parentSchema;
+      return childSchema;
+    }),
+    find: vi.fn().mockResolvedValue({ data: [] }),
+    create: vi.fn(),
+    update: vi.fn(),
+    delete: vi.fn(),
+    bulk: vi.fn(),
+  } as any;
+}
+
+function renderForm(details: any[], ds: any) {
+  return render(
+    <MasterDetailForm
+      schema={{ objectName: PARENT, mode: 'create', fields: ['ref'], details } as any}
+      dataSource={ds}
+    />,
+  );
+}
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe('object-master-detail-form — a detail whose schema LOADED but failed to DERIVE (objectui#6394)', () => {
+  it('renders a config hint naming `relationshipField` instead of a permanent "Loading columns…"', async () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const ds = dataSourceServing(underivableChildSchema);
+    const view = renderForm([{ childObject: 'po_line', title: 'PO lines' }], ds);
+
+    // The fetch was attempted and SUCCEEDED — read from the data source rather
+    // than assumed, because it is what separates this arm from objectui#6372.
+    await waitFor(() => expect(ds.getObjectSchema).toHaveBeenCalledWith('po_line'));
+    await expect(ds.getObjectSchema.mock.results[0].value).resolves.toBeTruthy();
+
+    const hint = await waitFor(() => view.getByTestId('md-detail-no-relationship-field'));
+
+    // ⭐ The hint must NAME THE KEY the author has to set. A generic "could not
+    // resolve this collection" placeholder would pass a presence check and
+    // leave the author exactly where "Loading columns…" left them — which is
+    // the whole complaint, and the reason triage ruled the config-hint shape.
+    expect(hint.textContent).toContain('relationshipField');
+    // Both ends of the relationship it could not find, so the author knows
+    // WHICH collection and WHICH parent are involved.
+    expect(hint.textContent).toContain('po_line');
+    expect(hint.textContent).toContain(PARENT);
+
+    // The spinner-shaped message must be gone for THIS detail: the derive has
+    // already failed and is not retried, so "loading" is not merely unhelpful,
+    // it is false.
+    expect(view.container.textContent).not.toContain('Loading columns…');
+
+    // ⛔ And it must NOT be objectui#6372's refusal: that placeholder says the
+    // schema could not be loaded, and this one loaded fine.
+    expect(view.queryByTestId('md-detail-schema-unavailable')).toBeNull();
+    // Nor objectui#6360's — this detail DID name its child object.
+    expect(view.queryByTestId('md-detail-no-child-object')).toBeNull();
+
+    // The section still renders, so the author sees which collection to fix.
+    expect(view.container.textContent).toContain('PO lines');
+
+    warn.mockRestore();
+    view.unmount();
+  });
+
+  it('still logs the derive error, carrying the thrown Error and not just a message about it', async () => {
+    // objectui#6372 stopped this arm discarding the error; rendering a hint must
+    // not re-swallow it. The thrown message holds the full diagnosis (which
+    // child, which parent, which key) — the placeholder shows the author the
+    // key, the log keeps the stack for whoever debugs it.
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const ds = dataSourceServing(underivableChildSchema);
+    const view = renderForm([{ childObject: 'po_line', title: 'PO lines' }], ds);
+
+    // Anchored on the state under test, so this cannot read green for an entry
+    // that never reached the failed derive at all.
+    await waitFor(() => expect(view.queryByTestId('md-detail-no-relationship-field')).not.toBeNull());
+
+    const call = warn.mock.calls.find((c) => String(c[0]).includes('relationshipField'));
+    expect(call, 'expected the derive failure to be logged, not discarded').toBeTruthy();
+    const carriesError = call!.some(
+      (a) => a instanceof Error && /could not find a lookup\/master_detail field/.test(a.message),
+    );
+    expect(carriesError, 'expected the thrown error object itself to reach the log').toBe(true);
+
+    warn.mockRestore();
+    view.unmount();
+  });
+
+  it('DEGENERATE CONTROL: a detail whose FETCH threw keeps objectui#6372’s refusal placeholder', async () => {
+    // ⭐ Without this, routing every unresolved detail to the new config hint
+    // would pass the tests above while erasing objectui#6372's placeholder —
+    // and telling an author to set `relationshipField` for a schema that could
+    // not be fetched is advice they cannot act on.
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const ds = dataSourceServing(underivableChildSchema, ['po_line']);
+    const view = renderForm([{ childObject: 'po_line', title: 'PO lines' }], ds);
+
+    const placeholder = await waitFor(() => view.getByTestId('md-detail-schema-unavailable'));
+    expect(placeholder.textContent).toContain('po_line');
+    expect(view.queryByTestId('md-detail-no-relationship-field')).toBeNull();
+
+    warn.mockRestore();
+    view.unmount();
+  });
+
+  it('DEGENERATE CONTROL: a detail still IN FLIGHT keeps "Loading columns…"', async () => {
+    // ⭐ The message is TRUE while the fetch has not settled, and a fix that
+    // simply deleted or repointed the loading branch would pass the first test.
+    let release: (v: any) => void = () => {};
+    const pending = new Promise((resolve) => { release = resolve; });
+    const ds = {
+      getObjectSchema: vi.fn(async (obj: string) => {
+        if (obj === PARENT) return parentSchema;
+        return pending;
+      }),
+      find: vi.fn().mockResolvedValue({ data: [] }),
+      create: vi.fn(), update: vi.fn(), delete: vi.fn(), bulk: vi.fn(),
+    } as any;
+
+    const view = renderForm([{ childObject: 'po_line', title: 'PO lines' }], ds);
+
+    await waitFor(() => expect(ds.getObjectSchema).toHaveBeenCalledWith('po_line'));
+    await waitFor(() => expect(view.container.textContent).toContain('Loading columns…'));
+    expect(view.queryByTestId('md-detail-no-relationship-field')).toBeNull();
+    expect(view.queryByTestId('md-detail-schema-unavailable')).toBeNull();
+
+    release(underivableChildSchema);
+    view.unmount();
+  });
+
+  it('DEGENERATE CONTROL: a detail that DERIVES renders its grid — no hint at all', async () => {
+    // ⭐ The behaviour has to be attributable to the FAILED DERIVE, not to
+    // "this renderer shows a hint for every detail".
+    const ds = dataSourceServing(resolvableChildSchema);
+    const view = renderForm([{ childObject: 'po_line', title: 'PO lines' }], ds);
+
+    await waitFor(() => expect(ds.getObjectSchema).toHaveBeenCalledWith('po_line'));
+    // Resolved all the way to a rendered grid — not merely "no hint", which a
+    // stuck-forever entry would also satisfy.
+    await waitFor(() => expect(view.queryByTestId('line-items')).not.toBeNull());
+
+    expect(view.queryByTestId('md-detail-no-relationship-field')).toBeNull();
+    expect(view.container.textContent).not.toContain('Loading columns…');
+
+    view.unmount();
+  });
+
+  it('DEGENERATE CONTROL: a detail with no `childObject` keeps objectui#6360’s hint', async () => {
+    // ⭐ The two config hints must stay distinguishable: this one has no child
+    // object to link to the parent at all, so `childObject` — not
+    // `relationshipField` — is the key its author has to set.
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const ds = dataSourceServing(underivableChildSchema);
+    const view = renderForm([{ title: 'Unconfigured' }], ds);
+
+    const hint = await waitFor(() => view.getByTestId('md-detail-no-child-object'));
+    expect(hint.textContent).toContain('childObject');
+    expect(view.queryByTestId('md-detail-no-relationship-field')).toBeNull();
+
+    warn.mockRestore();
+    view.unmount();
+  });
+});

packages/plugin-form/src/MasterDetailForm.detailSchemaFetchFailure.test.tsx

@@ -166,17 +166,26 @@ describe('object-master-detail-form — a detail whose schema fetch THREW (objec
     // relationship field" were indistinguishable.
     //
     // Routing both to the refusal placeholder would make it state something
-    // false — the schema loaded fine here. This arm's RENDER is deliberately
-    // unchanged (it still says "Loading columns…", pinned by objectui#6360);
-    // what this control holds is that it did not get swept into objectui#6372's
-    // placeholder. The error is nonetheless no longer discarded.
+    // false — the schema loaded fine here. What this control holds is that the
+    // derive failure did not get swept into objectui#6372's placeholder.
+    //
+    // ⚠️ Its WAIT was rewritten by objectui#6394, which is the card that gave
+    // this arm a render of its own: it waited on "Loading columns…", the
+    // permanent-spinner outcome objectui#6372 deliberately left standing here
+    // and triage has since ruled out. The subject of the control — this is NOT
+    // a load failure — is unchanged and still asserted below; the error is
+    // still no longer discarded.
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
     // Loads fine, but carries no field referencing the parent.
     const ds = dataSourceRejectingFor([], { name: 'po_line', fields: { amount: { type: 'number' } } });
     const view = renderForm([{ childObject: 'po_line', title: 'PO lines' }], ds);
 
     await waitFor(() => expect(ds.getObjectSchema).toHaveBeenCalledWith('po_line'));
-    await waitFor(() => expect(view.container.textContent).toContain('Loading columns…'));
+    // The derive failure's OWN placeholder (objectui#6394), naming
+    // `relationshipField` — not the "Loading columns…" this waited on before
+    // that card, and not the refusal below.
+    await waitFor(() => expect(view.queryByTestId('md-detail-no-relationship-field')).not.toBeNull());
+    expect(view.container.textContent).not.toContain('Loading columns…');
 
     // NOT the load-failure placeholder: the load succeeded.
     expect(view.queryByTestId('md-detail-schema-unavailable')).toBeNull();