fix(plugin-detail): 相关列表的 Add 门改问 add.picker.object,漏写 picker 不再把整块换成红卡 (#3838)

.changeset/related-list-add-picker-guard-3838.md

@@ -0,0 +1,7 @@
+---
+'@object-ui/plugin-detail': patch
+---
+
+`record:related_list`: an `add` without `add.picker` no longer takes the whole related list down.
+
+The Add-picker gate compared only `add` for truthiness and then read `add.picker.object` bare, so page metadata declaring `add` but omitting the (spec-required) `picker` threw during render and `SchemaRenderer` replaced the entire related list with a "Component failed to render" card whose message never mentioned `picker`. Both the Add button and the picker dialog now gate on the resolved `add.picker.object` — the list body renders as usual, only the unconfigured Add affordance is withheld, and a console hint names the missing key. Off-spec `add` still does nothing, so no lenient second dialect is introduced; producing-side validation of page metadata is tracked separately.

apps/console/src/__tests__/public-block-binding-reach.test.tsx

@@ -235,8 +235,9 @@ const sampleFor = (input: any): unknown => {
   if (input.name === 'objectName') return PROBE_OBJECT;
   // `record:related_list.add` — the generic `object` sample below is `{}`, and
   // `{}` is not a valid `add`: the spec makes `picker` required. An invalid one
-  // does not merely under-configure this block, it CRASHES it
-  // (`RelatedList.tsx:1299` dereferences `add.picker.object`, objectui#3838) —
+  // did not merely under-configure this block, it CRASHED it
+  // (`RelatedList.tsx:1299` dereferenced `add.picker.object`, objectui#3838,
+  // whose fix now gates the Add affordance on the resolved picker target) —
   // and a crashed block makes no data calls, which is indistinguishable from the
   // "declines to fetch" verdict this block is ledgered for below. That is a green
   // for the wrong reason, so the sample is spec-valid at the source instead.
@@ -381,11 +382,12 @@ describe('public blocks — a declared objectName reaches the data layer (object
         //
         // Added with objectui#3808, and DEFENSIVE rather than load-bearing today:
         // that change made an invalid `add` sample crash `record:related_list`
-        // (objectui#3838), which is what it does in the sibling probe, but not
+        // (objectui#3838), which is what it did in the sibling probe, but not
         // here — `renderers/record-related-list.tsx:185` passes
         // `dataSource={ctx?.dataSource}`, this probe mounts with no RecordContext,
-        // so `RelatedList`'s `add && dataSource` guard short-circuits before the
-        // unguarded read. Checked, not assumed: reverting the sample to `{}` keeps
+        // so `RelatedList`'s picker gate (`add && pickerObject && dataSource`,
+        // truthiness-only on `add` before #3838) short-circuits before the read
+        // either way. Checked, not assumed: reverting the sample to `{}` keeps
         // all 16 green. The predicate itself is known to work — applied to both
         // branches it reports the two crashes in objectui#3840 — so this is a
         // cheap standing guard on the one branch where a crash IS the pass

apps/console/src/__tests__/record-block-record-reach.test.tsx

@@ -205,10 +205,14 @@ const DATA_SOURCE_METHODS = [
  * sample, which would put an unspecified bag on every future `object` input.
  *
  * That `{}` did not merely under-exercise the block, it CRASHED it —
- * `RelatedList.tsx:1299` dereferences `add.picker.object` where `:378` / `:390`
- * optional-chain the same path — and the crash is filed as objectui#3838 rather
+ * `RelatedList.tsx:1299` dereferenced `add.picker.object` where `:378` / `:390`
+ * optional-chain the same path — and the crash was filed as objectui#3838 rather
  * than papered over here: this fixture's job is to be spec-valid, not to steer
- * clear of the renderer's unguarded reads.
+ * clear of the renderer's unguarded reads. #3838 has since tightened that gate
+ * to require the resolved `add.picker.object`, so the same `{}` now withholds
+ * the Add affordance behind a named console hint instead of taking the block
+ * down; the sample below stays spec-valid on its own merit, not as crash
+ * avoidance.
  */
 const SAMPLE_BY_INPUT: Readonly<Record<string, unknown>> = {
   // On `record:*` this names the RELATED object, not the page's object —

packages/plugin-detail/src/RelatedList.tsx

@@ -393,6 +393,27 @@ export const RelatedList: React.FC<RelatedListProps> = ({
     return derived.length > 0 ? derived : undefined;
   }, [pickerSchema, pickerDisplayField]);
 
+  // Developer hint for an `add` that cannot be honoured (#3838). `picker` is
+  // REQUIRED on `add` by the spec (`RecordRelatedListProps.add`), so getting
+  // here means the page metadata is off-spec — but nothing on the render path
+  // parses it (the sdui-parser manifest gate compares top-level key names and
+  // coarse types only), so the renderer is the first place able to say so. It
+  // says WHICH key is missing, because the failure it replaces — a bare
+  // `add.picker.object` read that threw and made SchemaRenderer swap the whole
+  // list for a "failed to render" card — never mentioned `picker` at all.
+  // Console-only, matching the in-file hint for the other partial
+  // misconfiguration (`no referenceField/parentId` below): the block-level
+  // dashed placeholder precedent in `renderers/record-related-list.tsx` is for
+  // blocks that can render NOTHING (missing objectName), whereas here only the
+  // Add affordance is unconfigured and the list body is perfectly fine.
+  React.useEffect(() => {
+    if (!add || pickerObject) return;
+    // eslint-disable-next-line no-console
+    console.warn(
+      `[RelatedList] "${api || objectName || 'related list'}" declares add without add.picker.object — the Add affordance is not rendered. add.picker is required by the spec (RecordRelatedListProps.add): set add.picker.object to the object the picker should list.`,
+    );
+  }, [add, pickerObject, api, objectName]);
+
   React.useEffect(() => {
     // Stale-response guard: page flips re-run this effect while an earlier
     // window may still be in flight — a slow page-2 response must not
@@ -1118,7 +1139,12 @@ export const RelatedList: React.FC<RelatedListProps> = ({
                 onToolbarAction={onToolbarAction}
               />
             ))}
-            {add && (
+            {/* Gated on the RESOLVED picker target, not merely on `add` being
+                truthy: an `add` without `picker` is metadata the spec rejects,
+                and offering a button that could never open a picker is worse
+                than withholding it (#3838 — the console hint above names the
+                missing key). */}
+            {add && pickerObject && (
               <Button
                 variant={isEmpty ? 'ghost' : 'outline'}
                 size="sm"
@@ -1290,13 +1316,19 @@ export const RelatedList: React.FC<RelatedListProps> = ({
           {addError}
         </div>
       )}
-      {add && dataSource && (
+      {/* Same gate as the Add button above — `pickerObject` (i.e.
+          `add?.picker?.object`, computed once near the picker-schema fetch) is
+          what the dialog needs, so requiring it here removes the last
+          render-path bare read of `add.picker` rather than optional-chaining
+          it: off-spec `add` still does nothing at all, so no second dialect
+          appears (AGENTS.md #0.1). #3838. */}
+      {add && pickerObject && dataSource && (
         <RecordPickerDialog
           open={pickerOpen}
           onOpenChange={(o) => setPickerOpen(o)}
           multiple
           dataSource={dataSource as any}
-          objectName={add.picker.object}
+          objectName={pickerObject}
           title={add.label || t('detail.add', { defaultValue: 'Add' })}
           displayField={pickerDisplayField}
           columns={pickerColumns}

packages/plugin-detail/src/__tests__/RelatedList.addPickerGuard.test.tsx

@@ -0,0 +1,159 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#3838 — an `add` without `add.picker` must not take the whole
+ * related list down.
+ *
+ * `add.picker` is REQUIRED by the spec (`RecordRelatedListProps.add`:
+ * `safeParse({ add: { label: 'Add' } })` reports `invalid_type` on
+ * `add.picker`), and `RelatedListProps` types it as required too — which is
+ * exactly why the bare `add.picker.object` read behind a truthiness-only
+ * `{add && dataSource && (` gate type-checked. Nothing on the render path
+ * parses the schema (the sdui-parser manifest gate compares top-level key
+ * names and coarse types only, `validateComponentProps` is advisory, and this
+ * renderer did not check), so off-spec page metadata reached the read and threw
+ * `TypeError: Cannot read properties of undefined (reading 'object')`, which
+ * `SchemaRenderer` turned into a "Component 'record:related_list' failed to
+ * render" card over the ENTIRE list — with no mention of `picker` anywhere in
+ * the message.
+ *
+ * The fix tightens the gates to the resolved picker target (the same
+ * `add?.picker?.object` the file already computes for the picker schema fetch),
+ * so the bare read is gone rather than optional-chained: an `add` missing
+ * `picker` withholds the Add affordance and names the missing key in the
+ * console, while the list body renders as usual. It is not consumer-side
+ * leniency (AGENTS.md #0.1) — off-spec `add` still does nothing, so no second
+ * dialect appears; the producer-side fix is #3838's route (c), out of scope
+ * here.
+ */
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, screen, waitFor, fireEvent } from '@testing-library/react';
+import * as React from 'react';
+import { RelatedList } from '../RelatedList';
+
+const junctionRows = [
+  { id: 'ups_1', permission_set_id: 'ps_1' },
+  { id: 'ups_2', permission_set_id: 'ps_2' },
+];
+
+const makeDataSource = () => ({
+  getObjectSchema: vi.fn(async () => ({
+    name: 'sys_user_permission_set',
+    fields: { permission_set_id: { type: 'text', label: 'Permission Set' } },
+  })),
+  find: vi.fn(async (api: string) =>
+    api === 'sys_user_permission_set'
+      ? { data: junctionRows, total: junctionRows.length }
+      : { data: [], total: 0 },
+  ),
+});
+
+/**
+ * `add` is the only thing that varies across these cases; everything else is
+ * the configuration a working assignment list uses (referenceField + parentId
+ * present, so the unrelated "refusing to fetch all rows" hint stays silent and
+ * the console spy sees only the diagnostic under test).
+ */
+const renderList = (ds: any, add?: any) =>
+  render(
+    <RelatedList
+      title="Permission Sets"
+      type="table"
+      api="sys_user_permission_set"
+      objectName="sys_user_permission_set"
+      referenceField="user_id"
+      parentId="u_1"
+      columns={[{ accessorKey: 'permission_set_id', header: 'Permission Set' }]}
+      dataSource={ds}
+      add={add}
+    />,
+  );
+
+/** The Add affordance, whatever label the metadata gave it. */
+const queryAddButton = () =>
+  screen.queryByRole('button', { name: /Assign position|Grant permission set|^Add$/ });
+
+let warn: ReturnType<typeof vi.spyOn>;
+
+beforeEach(() => {
+  warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+});
+
+afterEach(() => {
+  warn.mockRestore();
+});
+
+const warnings = () => warn.mock.calls.map((c) => String(c[0])).join('\n');
+
+describe('RelatedList — add without add.picker (#3838)', () => {
+  it('renders the list body, withholds the Add affordance, and names add.picker.object', async () => {
+    const ds = makeDataSource();
+
+    // Pre-fix this render THREW the TypeError above (in the app: the red card
+    // replacing the whole block). Reaching the assertions at all is the
+    // no-crash half of the pin.
+    renderList(ds, { label: 'Assign position' });
+
+    // The list body is untouched: the parent-scoped fetch still runs and its
+    // rows still reach the view (the count badge is fed by the same
+    // `relatedData` the table renders from — the table itself is a `data-table`
+    // delegated to SchemaRenderer/plugin-grid, out of this unit's scope).
+    await waitFor(() =>
+      expect(ds.find).toHaveBeenCalledWith('sys_user_permission_set', expect.objectContaining({
+        $filter: { user_id: 'u_1' },
+      })),
+    );
+    expect(screen.getByText('Permission Sets')).toBeInTheDocument();
+    await waitFor(() =>
+      expect(screen.getByLabelText(`${junctionRows.length} records`)).toBeInTheDocument(),
+    );
+
+    // The unconfigured Add affordance does not render — clicking a button that
+    // can never open a picker is worse than not offering it.
+    expect(queryAddButton()).toBeNull();
+
+    // …and the author is told WHICH key is missing, which the crash never did.
+    await waitFor(() => expect(warn).toHaveBeenCalled());
+    expect(warnings()).toContain('add.picker.object');
+    expect(warnings()).toContain('sys_user_permission_set');
+  });
+
+  it('still renders the Add affordance and opens the picker for a spec-valid add', async () => {
+    const ds = makeDataSource();
+    renderList(ds, {
+      picker: { object: 'sys_permission_set', labelField: 'label' },
+      linkField: 'permission_set_id',
+      label: 'Grant permission set',
+    });
+
+    const button = await waitFor(() => {
+      const b = queryAddButton();
+      expect(b).not.toBeNull();
+      return b!;
+    });
+    fireEvent.click(button);
+
+    // Opening the dialog fetches the picker TARGET's schema — proof the gate
+    // still passes the resolved `add.picker.object` through.
+    await waitFor(() =>
+      expect(ds.getObjectSchema).toHaveBeenCalledWith('sys_permission_set'),
+    );
+    expect(warnings()).not.toContain('add.picker.object');
+  });
+
+  it('leaves a list with no add untouched and silent', async () => {
+    const ds = makeDataSource();
+    renderList(ds, undefined);
+
+    await waitFor(() => expect(ds.find).toHaveBeenCalled());
+    expect(queryAddButton()).toBeNull();
+    // No `add` is a legitimate configuration, not a misconfiguration.
+    expect(warnings()).not.toContain('add.picker.object');
+  });
+});