fix(app-shell): a position decision output lists the position registry and commits position names (objectui#7611)

.changeset/7611-decision-output-position-registry.md

@@ -0,0 +1,11 @@
+---
+'@object-ui/app-shell': minor
+'@object-ui/console': minor
+---
+
+A `position` decision output lists the position registry and commits position names (objectui#7611, objectstack ADR-0131 D3/D4).
+
+- **What the approver sees.** In the Approve and Reject dialogs (the Approval Center and the record page both decide through `DeclaredActionsBar`), a `position` output is a select over the `position` items the registry serves, read through the console's metadata store (`ensureType('position')`), the same reader the sharing-rule recipient picker lists positions through. Each option is labelled by the position's `label`, else its name, and the list is sorted by name. It used to be a `sys_position` lookup, which offered nothing on a database without `sys_position` rows.
+- **What the flow receives.** The decide body carries the picked positions' machine names (an array when the output is `multiple`). That is what the spec says the output collects (objectstack#22710), and what the engine's `resolveAs: 'position'` matches against `sys_user_position.position`. The lookup committed `sys_position` row ids instead.
+- **Without a metadata store.** A surface rendered outside a `<MetadataProvider>` has no registry reader, so a `position` output there is the free-text box an untyped output gets, the same fallback the recipient picker takes. An empty registry is not that case: it renders the select with no options.
+- `department` and `team` outputs are unchanged: they are still record lookups and commit record ids.

apps/console/src/services/approvalsApi.ts

@@ -135,9 +135,10 @@ export interface ApprovalRequestRow {
   /**
    * framework#3447 follow-up: the normalized TYPED declarations behind
    * `decision_outputs` — `{ key, label?, type?, multiple? }` — so the decide
-   * dialog renders a record picker (user / department / position / team; id
-   * values, `multiple` → id array) instead of free text. Prefer this and fall
-   * back to the bare key list (older backend).
+   * dialog renders a picker instead of free text: user / department / team
+   * pick records and commit ids, position lists the position registry and
+   * commits machine names (objectui#7611); `multiple` → an array of them.
+   * Prefer this and fall back to the bare key list (older backend).
    */
   decision_output_defs?: Array<{
     key: string;

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

@@ -21,11 +21,13 @@
  * object looked right), so the round trip through both stages is the pin.
  */
 
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi } from 'vitest';
 import {
   decisionOutputDefs,
   decisionOutputParams,
+  decisionOutputsReadRegistry,
   foldDecisionOutputs,
+  readDecisionOutputRegistry,
 } from './decisionOutputParams';
 import { resolveActionParams } from '@object-ui/fields';
 import { paramToField } from './paramToField';
@@ -82,6 +84,7 @@ describe('decisionOutputParams — declaration → param', () => {
     const [param] = decisionOutputParams(
       [{ key: 'parallel_positions', label: '并行会审岗位', type: 'position', multiple: true }],
       t,
+      { registry: { position: [] } },
     );
     expect(param).toMatchObject({ label: '并行会审岗位', multiple: true });
   });
@@ -127,7 +130,6 @@ describe('decisionOutputParams — the params survive resolution (objectui#2955)
   // union, and a bare table widens it to `string` (objectui#4040).
   it.each([
     ['department', 'sys_business_unit'],
-    ['position', 'sys_position'],
     ['team', 'sys_team'],
   ] as const)('a %s output reaches the widget as a %s picker', (type, referenceTo) => {
     const [param] = decisionOutputParams([{ key: 'k', type, multiple: true }], t);
@@ -151,7 +153,7 @@ describe('decisionOutputParams — the params survive resolution (objectui#2955)
     // were dropped in resolution the dialog would submit a blank value and let
     // the server 400 instead.
     const [param] = decisionOutputParams(
-      [{ key: 'k', type: 'position', required: true }], t, { decision: 'approve' },
+      [{ key: 'k', type: 'team', required: true }], t, { decision: 'approve' },
     );
     expect(widgetFor(param)).toMatchObject({ type: 'lookup', required: true });
   });
@@ -164,13 +166,143 @@ describe('decisionOutputParams — the params survive resolution (objectui#2955)
   it('never degrades a typed picker to a record-id text box', () => {
     // The #2955 symptom, stated as the invariant: no declared record kind may
     // arrive at the widget as `text` (that is the "paste a UUID" fallback).
+    // `position` is a catalog kind, so it is judged with a registry read in
+    // hand — an empty one, the state a fresh environment can be in.
     for (const type of ['user', 'department', 'position', 'team'] as const) {
-      const [param] = decisionOutputParams([{ key: 'k', type }], t);
+      const [param] = decisionOutputParams([{ key: 'k', type }], t, { registry: { position: [] } });
       expect(widgetFor(param).type).not.toBe('text');
     }
   });
 });
 
+/**
+ * objectui#7611 (ADR-0131 D3/D4) — a `position` output lists the position
+ * REGISTRY and commits the position's machine name, which is what the spec
+ * says the output collects (objectstack#22710) and what the engine's
+ * `resolveAs: 'position'` matches against `sys_user_position.position`. It used
+ * to be a `sys_position` lookup: a fresh database holds no `sys_position` rows,
+ * so the picker offered nothing, and a row it did offer committed a row id.
+ */
+describe('decisionOutputParams — a position output lists the registry (objectui#7611)', () => {
+  const POSITIONS = [
+    { name: 'manager', label: 'Project Manager' },
+    { name: 'auditor', label: 'Auditor' },
+    { name: 'exec' },
+    { label: 'nameless — not a catalog item' },
+    { name: '', label: 'empty name — not a catalog item' },
+  ];
+
+  it('offers the registry items by name, sorted by name, labelled by `label`', () => {
+    const [param] = decisionOutputParams(
+      [{ key: 'parallel_positions', type: 'position', multiple: true }],
+      t,
+      { registry: { position: POSITIONS } },
+    );
+    expect(param).toMatchObject({ type: 'select', multiple: true });
+    expect(param.options).toEqual([
+      { value: 'auditor', label: 'Auditor' },
+      { value: 'exec', label: 'exec' },
+      { value: 'manager', label: 'Project Manager' },
+    ]);
+    // Nothing names the retired record source.
+    expect(param.reference).toBeUndefined();
+    expect(JSON.stringify(param)).not.toContain('sys_position');
+  });
+
+  it('reaches the widget as a select whose values are the machine names', () => {
+    const [param] = decisionOutputParams(
+      [{ key: 'parallel_positions', type: 'position', multiple: true }],
+      t,
+      { registry: { position: POSITIONS } },
+    );
+    const field = widgetFor(param);
+    expect(field).toMatchObject({ type: 'select', multiple: true });
+    expect((field.options as Array<{ value: string }>).map((o) => o.value)).toEqual([
+      'auditor',
+      'exec',
+      'manager',
+    ]);
+  });
+
+  it('an empty registry is still a picker, with nothing to offer', () => {
+    const [param] = decisionOutputParams([{ key: 'p', type: 'position' }], t, {
+      registry: { position: [] },
+    });
+    expect(widgetFor(param)).toMatchObject({ type: 'select', options: [] });
+  });
+
+  it('no registry reader degrades to the free-text box an untyped output gets', () => {
+    // An absent reader is a different fact from an empty registry — the same
+    // split `RecipientPickerField` draws on the same registry.
+    const [param] = decisionOutputParams([{ key: 'p', type: 'position' }], t);
+    expect(param).toMatchObject({ type: 'text', helpText: 't:actions.decisionOutput.helpMultiValue' });
+  });
+
+  it('keeps `required` on approve, so a blank registry pick blocks Confirm', () => {
+    const [param] = decisionOutputParams(
+      [{ key: 'p', type: 'position', required: true }],
+      t,
+      { decision: 'approve', registry: { position: POSITIONS } },
+    );
+    expect(widgetFor(param)).toMatchObject({ type: 'select', required: true });
+  });
+
+  it('folds the picked names into the decide body unchanged', () => {
+    expect(foldDecisionOutputs({ 'outputs.parallel_positions': ['auditor', 'manager'] })).toEqual({
+      rest: {},
+      outputs: { parallel_positions: ['auditor', 'manager'] },
+    });
+  });
+});
+
+describe('decisionOutputsReadRegistry (objectui#7611)', () => {
+  it('is true only when an output is a catalog kind', () => {
+    expect(decisionOutputsReadRegistry([{ key: 'a', type: 'team' }, { key: 'b', type: 'position' }])).toBe(true);
+    expect(
+      decisionOutputsReadRegistry([
+        { key: 'a', type: 'user' },
+        { key: 'b', type: 'department' },
+        { key: 'c', type: 'team' },
+        { key: 'd', type: 'text' },
+        { key: 'e' },
+      ]),
+    ).toBe(false);
+    expect(decisionOutputsReadRegistry([])).toBe(false);
+  });
+});
+
+describe('readDecisionOutputRegistry (objectui#7611)', () => {
+  it('reads each catalog type the outputs list, once, and nothing else', async () => {
+    const ensureType = vi.fn(async (type: string) => (type === 'position' ? [{ name: 'ops' }] : []));
+    const registry = await readDecisionOutputRegistry(
+      [
+        { key: 'a', type: 'position' },
+        { key: 'b', type: 'position', multiple: true },
+        { key: 'c', type: 'team' },
+        { key: 'd', type: 'user' },
+        { key: 'e' },
+      ],
+      { ensureType },
+    );
+    expect(ensureType.mock.calls).toEqual([['position']]);
+    expect(registry).toEqual({ position: [{ name: 'ops' }] });
+  });
+
+  it('reads nothing when no output is a catalog kind', async () => {
+    const ensureType = vi.fn(async () => []);
+    expect(await readDecisionOutputRegistry([{ key: 'c', type: 'team' }], { ensureType })).toEqual({});
+    expect(ensureType).not.toHaveBeenCalled();
+  });
+
+  it('answers `{}` without a reader, and `[]` for a read that fails', async () => {
+    expect(await readDecisionOutputRegistry([{ key: 'a', type: 'position' }], null)).toEqual({});
+    const failing = { ensureType: vi.fn(async () => { throw new Error('HTTP_500'); }) };
+    expect(await readDecisionOutputRegistry([{ key: 'a', type: 'position' }], failing)).toEqual({
+      position: [],
+    });
+  });
+});
+
 describe('foldDecisionOutputs', () => {
   it('folds the dotted params into the nested decide body', () => {
     expect(

packages/app-shell/src/utils/decisionOutputParams.ts

@@ -70,10 +70,108 @@ export const DECISION_OUTPUT_PARAM_PREFIX = 'outputs.';
  */
 const OUTPUT_LOOKUP_OBJECTS: Record<string, string> = {
   department: 'sys_business_unit',
-  position: 'sys_position',
   team: 'sys_team',
 };
 
+/**
+ * Catalog kinds: the output's candidates are the items of one metadata type in
+ * the environment registry, and the value handed to the flow is the item's
+ * machine name (objectui#7611).
+ *
+ * `position` lives here and not in {@link OUTPUT_LOOKUP_OBJECTS}: ADR-0131 D3
+ * gives positions one home, the registry, and D4 references a declared item by
+ * name. The spec says the same of this output (objectstack#22682, landed as
+ * objectstack#22710): a `position` decision output collects position machine
+ * names, which is what the engine's `resolveAs: 'position'` matches against
+ * `sys_user_position.position`. The `sys_position` lookup this entry replaces
+ * listed rows a fresh database no longer has, so the picker offered nothing.
+ *
+ * The read is the one `RecipientPickerField`'s registry kinds make: the
+ * console's metadata store, `ensureType(<type>)`, the same cached reader that
+ * serves apps and objects — see {@link readDecisionOutputRegistry}.
+ */
+const OUTPUT_REGISTRY_TYPES: Record<string, string> = {
+  position: 'position',
+};
+
+/**
+ * The registry items a decision surface read for the catalog kinds above,
+ * keyed by metadata type.
+ *
+ * A type that is ABSENT and a type that maps to `[]` are different facts, and
+ * {@link decisionOutputParams} keeps them apart: `[]` is an empty registry, so
+ * the output renders a picker with nothing to offer; an absent entry means no
+ * reader was available (no metadata store), and only that degrades the output
+ * to the free-text box an untyped output gets.
+ */
+export type DecisionOutputRegistry = Readonly<Record<string, readonly unknown[]>>;
+
+/** The one member of the metadata store this module reads. */
+export interface DecisionOutputRegistryReader {
+  ensureType: (type: string) => Promise<readonly unknown[]>;
+}
+
+/**
+ * Does any of these outputs list a registry?
+ *
+ * A decision surface asks this before it awaits
+ * {@link readDecisionOutputRegistry}, so a decision with no catalog output
+ * still dispatches in the click's own tick, as it did before catalog kinds
+ * existed; only a decision that has a registry to read waits for it.
+ */
+export function decisionOutputsReadRegistry(defs: DecisionOutputDef[]): boolean {
+  return defs.some((d) => !!(d?.type && OUTPUT_REGISTRY_TYPES[d.type]));
+}
+
+/**
+ * Read the registry items the given outputs list, once per metadata type.
+ *
+ * `null` reader (a surface outside a `<MetadataProvider>`) → `{}`: no type is
+ * read, and each catalog kind degrades as {@link DecisionOutputRegistry} says.
+ * A type whose read fails maps to `[]`, the same answer `RecipientPickerField`
+ * gives a failed registry read.
+ */
+export async function readDecisionOutputRegistry(
+  defs: DecisionOutputDef[],
+  reader: DecisionOutputRegistryReader | null | undefined,
+): Promise<DecisionOutputRegistry> {
+  if (!reader) return {};
+  const types = new Set<string>();
+  for (const d of defs) {
+    const type = d?.type ? OUTPUT_REGISTRY_TYPES[d.type] : undefined;
+    if (type) types.add(type);
+  }
+  const out: Record<string, readonly unknown[]> = {};
+  for (const type of types) {
+    try {
+      const items = await reader.ensureType(type);
+      out[type] = Array.isArray(items) ? items : [];
+    } catch {
+      out[type] = [];
+    }
+  }
+  return out;
+}
+
+/**
+ * Registry items → select options: the value is the item's machine name and
+ * the label its `label`, else its name. Items without a name are dropped, and
+ * the list is sorted by name — the same reading `RecipientPickerField` makes
+ * of the same registry.
+ */
+function registryOptions(items: readonly unknown[]): Array<{ value: string; label: string }> {
+  const named = items.filter(
+    (it): it is { name: string; label?: unknown } =>
+      it != null && typeof (it as { name?: unknown }).name === 'string' && !!(it as { name: string }).name,
+  );
+  return [...named]
+    .sort((a, b) => a.name.localeCompare(b.name))
+    .map((it) => ({
+      value: it.name,
+      label: typeof it.label === 'string' && it.label ? it.label : it.name,
+    }));
+}
+
 /**
  * Read the declared outputs off an approval request row. Prefers the typed
  * declarations; falls back to the bare key list an older backend sends.
@@ -115,11 +213,18 @@ function humanizeKey(key: string): string {
  * nothing reads the outputs, so demanding routing data to say "no" would trap
  * the rejection). Marking it here is what turns the server's 400 into a
  * blocked Confirm button with the field flagged, instead of a round trip.
+ *
+ * A catalog kind (`position`) renders a `select` over the registry items the
+ * caller read with {@link readDecisionOutputRegistry} and passes as
+ * `registry`; its option values are machine names, so the decide body carries
+ * the names the engine routes by. `select` is a declared param type whose
+ * options travel through `resolveActionParams()` untouched, so no new param
+ * key is needed to get them to the widget.
  */
 export function decisionOutputParams(
   defs: DecisionOutputDef[],
   t: (key: string) => string,
-  opts: { decision?: 'approve' | 'reject' } = {},
+  opts: { decision?: 'approve' | 'reject'; registry?: DecisionOutputRegistry } = {},
 ): Array<Record<string, unknown>> {
   return defs.filter((d) => d && d.key).map((d) => {
     const base = {
@@ -135,6 +240,17 @@ export function decisionOutputParams(
         helpText: t('actions.decisionOutput.help'),
       };
     }
+    const registryType = d.type ? OUTPUT_REGISTRY_TYPES[d.type] : undefined;
+    const registryItems = registryType ? opts.registry?.[registryType] : undefined;
+    if (registryItems) {
+      return {
+        ...base,
+        type: 'select',
+        options: registryOptions(registryItems),
+        multiple: d.multiple === true,
+        helpText: t('actions.decisionOutput.help'),
+      };
+    }
     const lookupObject = d.type ? OUTPUT_LOOKUP_OBJECTS[d.type] : undefined;
     if (lookupObject) {
       return {