diff --git a/.changeset/7611-decision-output-position-registry.md b/.changeset/7611-decision-output-position-registry.md new file mode 100644 index 0000000000..ad934bc41d --- /dev/null +++ b/.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 `` 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. diff --git a/apps/console/src/services/approvalsApi.ts b/apps/console/src/services/approvalsApi.ts index 412a2ce00b..a74f6b8bb5 100644 --- a/apps/console/src/services/approvalsApi.ts +++ b/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; diff --git a/packages/app-shell/src/utils/decisionOutputParams.test.ts b/packages/app-shell/src/utils/decisionOutputParams.test.ts index 893a219f55..1b2ccd96c1 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.test.ts +++ b/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( diff --git a/packages/app-shell/src/utils/decisionOutputParams.ts b/packages/app-shell/src/utils/decisionOutputParams.ts index e728dee349..217b7f8a5f 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.ts +++ b/packages/app-shell/src/utils/decisionOutputParams.ts @@ -70,10 +70,108 @@ export const DECISION_OUTPUT_PARAM_PREFIX = 'outputs.'; */ const OUTPUT_LOOKUP_OBJECTS: Record = { 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()`, the same cached reader that + * serves apps and objects — see {@link readDecisionOutputRegistry}. + */ +const OUTPUT_REGISTRY_TYPES: Record = { + 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>; + +/** The one member of the metadata store this module reads. */ +export interface DecisionOutputRegistryReader { + ensureType: (type: string) => Promise; +} + +/** + * 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 ``) → `{}`: 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 { + if (!reader) return {}; + const types = new Set(); + for (const d of defs) { + const type = d?.type ? OUTPUT_REGISTRY_TYPES[d.type] : undefined; + if (type) types.add(type); + } + const out: Record = {}; + 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> { 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 { diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index 95b4cbbff9..be58bb51f9 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -43,6 +43,7 @@ import { } from '@object-ui/components'; import { ActionProvider, + MetadataCtx, useAction, useCapabilityGate, useCondition, @@ -66,7 +67,12 @@ import { useAdapter } from '../providers/AdapterProvider.js'; // 'authFetchSpy' before initialization`, the side effect running inside the // hoisted `@object-ui/auth` mock factory before the spy existed. import { useMetadataItem } from '@object-ui/react'; -import { decisionOutputDefs, decisionOutputParams } from '../utils/decisionOutputParams.js'; +import { + decisionOutputDefs, + decisionOutputParams, + decisionOutputsReadRegistry, + readDecisionOutputRegistry, +} from '../utils/decisionOutputParams.js'; import { getIcon } from '../utils/getIcon.js'; import { isOverrideDecision, bypassedApproverNames } from '../utils/approvalOverride.js'; import { @@ -142,6 +148,20 @@ const DeclaredActionButton: React.FC<{ // literal IS what renders, which is how English help text survived in a zh-CN // workspace (objectui#2762 P0-3). const { t } = useObjectTranslation(); + // The console's metadata store, or `null` outside a `` — + // the reader a catalog-kind decision output (`position`) lists the registry + // through (objectui#7611). Read raw rather than through `useMetadata()`, + // whose no-provider fallback answers every type with `[]`: an empty registry + // and an absent reader are different facts (`DecisionOutputRegistry`). Held + // in a ref and read at click time, so no callback keys on the store's + // context identity, which is rebuilt whenever any type it caches settles + // (AGENTS.md #10). The ref is synced after each commit rather than during + // render; a click always lands after one. + const metadataStore = React.useContext(MetadataCtx); + const metadataStoreRef = React.useRef(metadataStore); + React.useEffect(() => { + metadataStoreRef.current = metadataStore; + }); const recordData = record != null && typeof record === 'object' ? (record as Record) : {}; /** @@ -285,8 +305,19 @@ const DeclaredActionButton: React.FC<{ // Widget mapping (typed picker vs free text) lives in the shared helper, // so the record header's Approve/Reject renders the same controls // (objectui#2955). + // + // A catalog kind (`position`) lists the registry, so its items are read + // before the params are built — once per type, from the store's + // session cache after the first read (objectui#7611). Only a decision + // that HAS a catalog output awaits that read; every other decision + // dispatches in the click's own tick, as before. + const outputDefs = decision ? decisionOutputDefs(recordData) : []; + const metadataStoreNow = metadataStoreRef.current; + const registry = metadataStoreNow && decisionOutputsReadRegistry(outputDefs) + ? await readDecisionOutputRegistry(outputDefs, metadataStoreNow) + : undefined; const outputParams = decision - ? decisionOutputParams(decisionOutputDefs(recordData), t, { decision }) + ? decisionOutputParams(outputDefs, t, { decision, registry }) : []; // Typed as the SEAM's contract, not as `any` and not cast to `ActionDef` // on the way out (objectui#5611). `overrideNotice` below is host-composed diff --git a/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx b/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx index 8b80a00f39..f8d664e0b5 100644 --- a/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx +++ b/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx @@ -99,7 +99,7 @@ import { DeclaredActionsBar } from '../DeclaredActionsBar'; // The real provider — the `@object-ui/react` double above spreads `actual`, so // the ambient predicate scope the objectui#4080 no-row case needs is the // shipped one, not a stand-in for it. -import { PredicateScopeProvider } from '@object-ui/react'; +import { MetadataCtx, PredicateScopeProvider } from '@object-ui/react'; const REQUEST = { id: 'req_1', status: 'pending', record_id: 'proj_9' }; @@ -337,7 +337,9 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { // `reference`. Spelled `referenceTo` the target was dropped, and the // Approval Center rendered a "paste a record id" text box where a // sys_position multi-select belonged. The full round trip is pinned in - // `utils/decisionOutputParams.test.ts`; this pins what the BAR emits. + // `utils/decisionOutputParams.test.ts`; this pins what the BAR emits for a + // record kind (a `position` output lists the registry since objectui#7611 — + // pinned below). const decideAction = { name: 'approval_approve', type: 'api', @@ -351,7 +353,7 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { record={{ ...REQUEST, decision_output_defs: [ - { key: 'parallel_positions', label: '并行会审岗位', type: 'position', multiple: true }, + { key: 'owning_teams', label: '负责团队', type: 'team', multiple: true }, ], }} location="record_section" @@ -365,15 +367,92 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { const params = executeSpy.mock.calls[0][0].actionParams as Array>; expect(params).toHaveLength(1); expect(params[0]).toMatchObject({ - name: 'outputs.parallel_positions', - label: '并行会审岗位', + name: 'outputs.owning_teams', + label: '负责团队', type: 'lookup', - reference: 'sys_position', + reference: 'sys_team', multiple: true, }); expect(params[0].referenceTo).toBeUndefined(); }); + it('lists the position registry for a position output and offers names (objectui#7611)', async () => { + // ADR-0131 D3/D4: positions live in the environment registry and are + // referenced by machine name. The bar reads the registry through the + // console's metadata store — the reader `RecipientPickerField` lists the + // same registry through — and hands the dialog a select whose values are + // the names the engine routes by. A fresh database has no `sys_position` + // rows, so the lookup this replaces offered nothing. + const ensureType = vi.fn(async (type: string) => + type === 'position' + ? [{ name: 'manager', label: 'Project Manager' }, { name: 'auditor', label: 'Auditor' }] + : [], + ); + const store = { ensureType } as unknown as React.ContextType; + render( + + + , + ); + + fireEvent.click(screen.getByText('Approve')); + await waitFor(() => expect(executeSpy).toHaveBeenCalled()); + + expect(ensureType.mock.calls).toEqual([['position']]); + const params = executeSpy.mock.calls[0][0].actionParams as Array>; + expect(params).toHaveLength(1); + expect(params[0]).toMatchObject({ + name: 'outputs.parallel_positions', + label: '并行会审岗位', + type: 'select', + multiple: true, + options: [ + { value: 'auditor', label: 'Auditor' }, + { value: 'manager', label: 'Project Manager' }, + ], + }); + expect(params[0].reference).toBeUndefined(); + }); + + it('dispatches in the click\'s own tick when no output lists the registry (objectui#7611)', () => { + // Only a decision with a catalog output waits for the registry read; every + // other decision dispatches synchronously, as it did before — a store is + // mounted here, so it is the OUTPUTS that decide, not the store's absence. + const ensureType = vi.fn(async () => []); + const store = { ensureType } as unknown as React.ContextType; + render( + + + , + ); + + fireEvent.click(screen.getByText('Approve')); + + expect(executeSpy).toHaveBeenCalledTimes(1); + expect(ensureType).not.toHaveBeenCalled(); + }); + it('localizes the decision-output help text', async () => { const decideAction = { name: 'approval_approve',