fix(app-shell): keep a field's own option keys when an action param inherits them (#3559)

.changeset/quiet-moons-inherit.md

@@ -0,0 +1,26 @@
+---
+"@object-ui/app-shell": patch
+"@object-ui/core": patch
+---
+
+Action params that inherit a field's options now keep the keys that field declared
+
+A field-backed action param (`{ field: 'tier' }`) had its inherited option list
+rebuilt entry by entry as `{ label, value }`, which silently dropped every other
+key the field's options declared — most consequentially the per-option
+`visibleWhen` predicate (ADR-0058). A select field whose options narrow by
+predicate in an object form therefore offered the FULL list in an action dialog,
+including the entries the predicate exists to hide, with no diagnostic on either
+side; `color` / `icon` / `disabled` were lost the same way. Options authored
+inline on the param were never affected — they always passed through verbatim,
+which is the asymmetry this restores.
+
+The resolver now preserves each inherited entry and only does its two real jobs:
+expanding bare strings into label/value pairs and translating the label through
+`fieldOptionLabel`. The option widgets already filter on `visibleWhen`, so a
+role-gated option (`'admin' in current_user.positions`) inherited by a dialog
+param now narrows the offered set and clears a seeded value the predicate hides.
+
+`ActionParamDef.options` (`@object-ui/core`) and the resolver's `RawActionParam`
+are widened to match: `ActionParamOption` names the two keys the param layer
+reads and carries the rest of a field's option vocabulary through.

packages/app-shell/src/utils/resolveActionParams.optionVisibleWhen.test.tsx

@@ -0,0 +1,162 @@
+/**
+ * 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#3559, end to end: a field's per-option `visibleWhen` narrows the
+ * option list a dialog param actually renders.
+ *
+ * The unit half lives in `resolveActionParams.test.ts` (the resolved param keeps
+ * the keys, and `resolveVisibleOptions()` filters on them). What that cannot show
+ * is the CONSUMER half — whether the control the dialog builds evaluates the
+ * predicate at all, or merely receives it. So this file drives the real widget
+ * with the real field object, wired the way `ActionParamDialog` wires it:
+ *
+ *   field metadata → `resolveActionParams()` → `paramToField()` → `<SelectField>`
+ *
+ * `ActionParamDialog` renders `field={paramToField(param)}` and `id={param.name}`
+ * through `getLazyFieldWidget(field.type)`; `SelectField` is what that resolves
+ * to for a `select` param. It is imported here at MODULE scope rather than through
+ * the lazy registry on purpose (AGENTS.md §测试纪律): a first dynamic `import()`
+ * under a saturated transform pipeline can eat most of RTL's 1s budget, and the
+ * assertions below are synchronous effects.
+ *
+ * Deliberately NOT passed: `dependentValues`. The dialog does not pass it either —
+ * it keeps the in-progress param values in local state — so the predicate `record`
+ * a dialog option sees is whatever `SchemaRendererContext` supplies (the page's
+ * `formValues` / `data`), never the dialog's own values. The predicates exercised
+ * here are therefore the ones that work in a dialog today: scope-relative
+ * (`current_user`), the role-gating case ADR-0058 opens. RECORD-relative
+ * predicates in a dialog are measured, not asserted, in the last test — see the
+ * note there.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { PredicateScopeProvider } from '@object-ui/react';
+import { SelectField } from '@object-ui/fields';
+import { resolveActionParams, type ResolveActionParamsContext } from './resolveActionParams';
+import { paramToField } from './paramToField';
+
+/** An object whose `tier` field gates one option on the viewer's positions. */
+const accountCtx = (
+  options: Array<Record<string, unknown> | string>,
+): ResolveActionParamsContext => ({
+  objectName: 'account',
+  objects: [{ name: 'account', fields: { tier: { type: 'select', label: 'Tier', options } } }],
+  fieldLabel: (_o, _f, fallback) => fallback,
+});
+
+const ROLE_GATED = [
+  { label: 'Standard', value: 'standard' },
+  { label: 'Admin only', value: 'admin_only', visibleWhen: "'admin' in current_user.positions" },
+];
+
+/**
+ * Render the `tier` param exactly as `ActionParamDialog` renders a select param:
+ * resolve it from the field, adapt it with `paramToField()`, hand the result to
+ * the widget as `field` with `id={param.name}`.
+ */
+function renderInheritedSelect(
+  options: Array<Record<string, unknown> | string>,
+  positions: string[],
+  value?: string,
+) {
+  const onChange = vi.fn();
+  const param = resolveActionParams([{ field: 'tier' }], accountCtx(options))[0];
+  const field = paramToField(param);
+  // Same props the dialog passes a non-boolean param's widget. The cast is the
+  // dialog's own seam: `paramToField()` returns `Record< string, unknown >`-shaped
+  // field metadata and the widget is reached through a lazy `any` component there.
+  const props = { id: param.name, value: value ?? null, onChange, field } as unknown as
+    React.ComponentProps<typeof SelectField>;
+  render(
+    <PredicateScopeProvider scope={{ current_user: { positions } }}>
+      <SelectField {...props} />
+    </PredicateScopeProvider>,
+  );
+  return { onChange, field };
+}
+
+describe('field-inherited option predicates reach the dialog control (objectui#3559)', () => {
+  it('drops a role-gated option for a viewer who fails the predicate', () => {
+    // Every option gated → the offered set is empty, and the widget says so
+    // instead of rendering a dropdown of options the predicate excluded.
+    renderInheritedSelect(
+      [{ label: 'Admin only', value: 'admin_only', visibleWhen: "'admin' in current_user.positions" }],
+      ['sales'],
+    );
+    expect(screen.getByTestId('select-empty-tier')).toBeInTheDocument();
+    expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
+  });
+
+  it('offers the same list to a viewer who satisfies it', () => {
+    renderInheritedSelect(
+      [{ label: 'Admin only', value: 'admin_only', visibleWhen: "'admin' in current_user.positions" }],
+      ['admin'],
+    );
+    expect(screen.queryByTestId('select-empty-tier')).not.toBeInTheDocument();
+    expect(screen.getByRole('combobox')).toBeInTheDocument();
+  });
+
+  it('clears a pre-filled value the predicate hides (per-option, not whole-list)', () => {
+    // The per-option proof: `standard` survives (so the list is not gated as a
+    // whole — a combobox is still offered) while `admin_only` is not offered, so
+    // the widget's cascade-clear drops the seeded value.
+    const { onChange } = renderInheritedSelect(ROLE_GATED, ['sales'], 'admin_only');
+    expect(screen.getByRole('combobox')).toBeInTheDocument();
+    expect(onChange).toHaveBeenCalledWith(undefined);
+  });
+
+  it('keeps that value for a viewer the predicate admits', () => {
+    const { onChange } = renderInheritedSelect(ROLE_GATED, ['admin'], 'admin_only');
+    expect(onChange).not.toHaveBeenCalled();
+  });
+
+  it('leaves an unpredicated inherited list fully offered', () => {
+    const { onChange, field } = renderInheritedSelect(
+      [{ label: 'Standard', value: 'standard' }, 'won'],
+      [],
+      'standard',
+    );
+    expect(field.options).toEqual([
+      { label: 'Standard', value: 'standard' },
+      { label: 'won', value: 'won' },
+    ]);
+    expect(onChange).not.toHaveBeenCalled();
+    expect(screen.getByRole('combobox')).toBeInTheDocument();
+  });
+
+  it('MEASURES a record-relative predicate with no record in context: fails OPEN', () => {
+    // Not a claim about correct behaviour — a recorded measurement, because the
+    // fix delivers the keys and the widget honours them, but WHICH record a
+    // dialog evaluates them against is a separate question this PR does not
+    // settle.
+    //
+    // Measured, not assumed (the prediction going in was the opposite): with no
+    // `dependentValues` — the dialog passes none, it keeps param values in local
+    // state — and no `SchemaRendererContext` above, `record` is `{}`, and
+    // `record.country == 'cn'` is UNRESOLVABLE rather than false. Per
+    // `resolveVisibleOptions()`'s documented fail-open default the option is
+    // therefore KEPT, not hidden. Same list against a populated record filters
+    // as expected (`{ country: 'us' }` → `[]`), which is what the object form
+    // gets and what a dialog mounted under a page picks up from that page's
+    // `formValues`/`data`.
+    //
+    // So the residual gap is narrow and safe-by-default: a dialog cannot narrow
+    // an inherited list against its OWN in-progress params, and unresolvable
+    // predicates offer everything rather than hiding everything. Reported
+    // separately; nothing here should be read as endorsing it.
+    renderInheritedSelect(
+      [{ label: 'Zhejiang', value: 'zj', visibleWhen: "record.country == 'cn'" }],
+      ['admin'],
+    );
+    expect(screen.queryByTestId('select-empty-tier')).not.toBeInTheDocument();
+    expect(screen.getByRole('combobox')).toBeInTheDocument();
+  });
+});

packages/app-shell/src/utils/resolveActionParams.test.ts

@@ -17,6 +17,7 @@
  * forms, and across inline / field-backed / missing-field branches.
  */
 import { describe, it, expect, vi } from 'vitest';
+import { resolveVisibleOptions, type ActionParamOption } from '@object-ui/core';
 import type { ActionParam } from '@object-ui/types';
 import {
   RESOLVED_ONLY_PARAM_KEYS,
@@ -358,3 +359,156 @@ describe('resolveActionParams — authored through the public ActionParam type (
     expect(raw.reference).toBe('account');
   });
 });
+
+/**
+ * objectui#3559 — a field-inherited option list keeps the keys the FIELD declared.
+ *
+ * `normaliseOptions()` rebuilt every inherited entry as a fresh `{ label, value }`,
+ * so a select field's per-option `visibleWhen` (ADR-0058 / #2284, declared by
+ * `@objectstack/spec`'s `SelectOptionSchema` and filtered by
+ * `resolveVisibleOptions()` in `@object-ui/core`) survived in the object form and
+ * vanished the moment an action param inherited the field via `{ field: '…' }`.
+ * The dialog then offered the whole list — predicate-hidden entries included —
+ * with no diagnostic on either side. `color` / `icon` / `disabled` went the same
+ * way.
+ *
+ * The asymmetry is the tell: options authored INLINE on the param never came
+ * through `normaliseOptions()` at all (`param.options ?? normaliseOptions(…)`),
+ * so they always sank verbatim. One branch of one `??` behaved one way and the
+ * other the other, for the same authored key.
+ *
+ * These tests follow the resolver's two jobs (expand bare strings, translate the
+ * label) and pin that it does no third one, then carry one inherited list all the
+ * way to `resolveVisibleOptions()` — the reader the option widgets wrap — so the
+ * claim under test is the observable one: the predicate narrows the dialog's list.
+ */
+describe('resolveActionParams — field-inherited option keys (objectui#3559)', () => {
+  /**
+   * A `select` field whose options carry the full declared vocabulary. Authoring
+   * this literal is itself the type-level half of the fix: this file is compiled
+   * by `tsconfig.typetests.json`, so a `RuntimeField.options` re-narrowed to
+   * `{ label, value }` fails excess-property checking right here rather than
+   * silently reverting the behaviour below.
+   */
+  const optionCtx = () =>
+    ctx({
+      objectName: 'account',
+      objects: [
+        {
+          name: 'account',
+          fields: {
+            tier: {
+              type: 'select',
+              label: 'Tier',
+              options: [
+                { label: 'Standard', value: 'standard', color: 'gray' },
+                {
+                  label: 'Admin only',
+                  value: 'admin_only',
+                  visibleWhen: "'admin' in current_user.positions",
+                  color: 'red',
+                  icon: 'shield',
+                  disabled: false,
+                },
+              ],
+            },
+            stage: { type: 'select', label: 'Stage', options: ['open', 'won'] },
+          },
+        },
+      ],
+    });
+
+  it('keeps a per-option visibleWhen the field declared', () => {
+    const resolved = resolveActionParams([{ field: 'tier' }], optionCtx())[0];
+    expect(resolved.options).toEqual([
+      { label: 'Standard', value: 'standard', color: 'gray' },
+      {
+        label: 'Admin only',
+        value: 'admin_only',
+        visibleWhen: "'admin' in current_user.positions",
+        color: 'red',
+        icon: 'shield',
+        disabled: false,
+      },
+    ]);
+  });
+
+  it('still translates the label through fieldOptionLabel, keeping the other keys', () => {
+    const resolved = resolveActionParams(
+      [{ field: 'tier' }],
+      ctx({
+        ...optionCtx(),
+        fieldOptionLabel: (objectName, fieldName, value, fallback) =>
+          `${objectName}.${fieldName}.${value}:${fallback}`,
+      }),
+    )[0];
+    expect(resolved.options?.[1]).toEqual({
+      label: 'account.tier.admin_only:Admin only',
+      value: 'admin_only',
+      visibleWhen: "'admin' in current_user.positions",
+      color: 'red',
+      icon: 'shield',
+      disabled: false,
+    });
+  });
+
+  it('still expands bare strings into label/value pairs', () => {
+    expect(resolveActionParams([{ field: 'stage' }], optionCtx())[0].options).toEqual([
+      { label: 'open', value: 'open' },
+      { label: 'won', value: 'won' },
+    ]);
+  });
+
+  it('leaves options undefined when the field declares none', () => {
+    const resolved = resolveActionParams(
+      [{ field: 'tier' }],
+      ctx({ objectName: 'account', objects: [{ name: 'account', fields: { tier: { type: 'text' } } }] }),
+    )[0];
+    expect(resolved.options).toBeUndefined();
+  });
+
+  it('passes an option list authored INLINE on the param through verbatim', () => {
+    // The branch that was never broken — pinned so the fix is read as making the
+    // two branches agree, not as touching this one. Stays green either way.
+    const inline = [
+      { label: 'Yes', value: 'yes' },
+      { label: 'Admin only', value: 'admin_only', visibleWhen: "'admin' in current_user.positions" },
+    ];
+    const resolved = resolveActionParams(
+      [{ field: 'tier', options: inline }, { name: 'confirm', type: 'select', options: inline }],
+      optionCtx(),
+    );
+    // Field-backed with inline options: the inline list wins and the field's is
+    // not consulted (same object identity — nothing rebuilt it).
+    expect(resolved[0].options).toBe(inline);
+    expect(resolved[1].options).toBe(inline);
+  });
+
+  it('reaches resolveVisibleOptions through paramToField and narrows the offered set', () => {
+    // End to end over the real chain the dialog uses: field metadata →
+    // resolveActionParams → paramToField → the `field.options` a widget reads →
+    // `resolveVisibleOptions`, which is what `useCascadingOptions` wraps.
+    const field = paramToField(resolveActionParams([{ field: 'tier' }], optionCtx())[0]);
+
+    const forSales = resolveVisibleOptions(field.options, {}, { current_user: { positions: ['sales'] } });
+    expect(forSales.map((o) => o.value)).toEqual(['standard']);
+
+    const forAdmin = resolveVisibleOptions(field.options, {}, { current_user: { positions: ['admin'] } });
+    expect(forAdmin.map((o) => o.value)).toEqual(['standard', 'admin_only']);
+  });
+
+  it('keeps `label` and `value` required on a param option (type-level)', () => {
+    // The catch-all widens the vocabulary; it must not dissolve the two keys
+    // this layer itself reads. Compiled by `tsconfig.typetests.json`, so these
+    // assertions are checked — a re-widening to `Record< string, unknown >`
+    // turns the unused suppressions into errors.
+    const complete: ActionParamOption = { label: 'Standard', value: 'standard', color: 'gray' };
+    expect(complete.value).toBe('standard');
+
+    // @ts-expect-error `value` is not optional
+    const noValue: ActionParamOption = { label: 'Standard' };
+    // @ts-expect-error `label` is not optional
+    const noLabel: ActionParamOption = { value: 'standard' };
+    expect([noValue, noLabel]).toHaveLength(2);
+  });
+});