fix(app-shell): a position approver is read through the spec's registry binding (objectui#7611 Q3b)

.changeset/7611-approver-registry-read.md

@@ -0,0 +1,9 @@
+---
+'@object-ui/app-shell': patch
+---
+
+A position approver is read through the spec's registry binding, `{ source: 'registry', type }` (objectstack#22710): its readable label and its designer picker come from the position registry, and the picker stores the position's name (objectui#7611, ruled objectstack#22865 Q3b → B).
+
+- **The approval panel and the override dialog.** When the bundled spec binds `position` to the registry (the console objectstack ships injects that spec), a pending `position:<name>` approver now reads as the position's label, read through the console's metadata store (`ensureType('position')`), else as the prettified name. It used to fall through to the opaque-id truncation and read `positi…ager`. Who holds the seat is still read from `sys_user_position`.
+- **The flow designer's approver Value cell.** A `registry` entry in the approval node's published `xRef.sources` is no longer dropped. A kind bound that way renders a select over that metadata type's items (`GET /api/v1/meta/<type>`) and commits the item's name, with a manual-entry toggle; a stored name the registry does not list shows flagged `(not found)`, and an empty registry leaves the text box. It used to fall back to a lookup over `sys_position` rows, or to a plain text box on the console objectstack ships.
+- **Unchanged.** A `data` binding (`user`, `team`, `department`, and `position` under the installed spec 17.7.0) still renders the record lookup and still resolves its label from its directory object. No authoring surface changes, so `objectui validate` and the strict faces are unchanged. The type that admits the registry member before the installed spec declares it is marked for removal when objectui moves to spec 18.

content/docs/guide/flow-designer.md

@@ -122,6 +122,14 @@ strings, numbers, or objects) becomes a **nested repeater** — so an
 engine-published nested-array config is editable inline rather than dropping to
 the Advanced JSON block.
 
+An *Approval* node's approver **Value** cell takes its candidates from where the
+schema's published binding for the row's **Type** says they live (`xRef.sources`):
+a directory kind (`user`, `team`, `department`) is a lookup over its directory
+object's records, and a kind bound to the metadata registry
+(`{ source: 'registry', type }`, which is how `position` is bound on a server
+carrying objectstack#22710) is a select over that type's declared items that
+stores the item's name. Both offer a pencil toggle for typing a value by hand.
+
 A **Decision** node's Branches editor defines each branch's label, CEL
 expression, **and target node** in one table: the **Target** column picks the
 downstream node, wiring (creating, retargeting, or detaching) the branch's

packages/app-shell/README.md

@@ -1006,6 +1006,16 @@ the record's requests (comments and attachments included), and an inline
 **Send reminder** button for the submitter. Records without requests carry
 no tab at all.
 
+An approver the server leaves unnamed, such as a `position:<name>` seat, is
+named from where the spec's approver binding says its value lives: a
+directory kind (`team`, `department`) by its row's label, and a kind bound to
+the metadata registry (`{ source: 'registry', type }`, which is how a spec
+carrying objectstack#22710 binds `position`) by the registry item's label,
+read through the metadata store. When no label answers, a reference that
+carries a machine name (a position, under either binding) reads as the
+prettified name, never as the raw reference; an id-valued kind (`team`,
+`department`) keeps the caller's existing formatting.
+
 Visibility is gated by record READ access, not approver status — anyone who
 can open the record sees where its approval stands, without a trip to the
 Approval Center (a `setup`-app surface business roles typically cannot

packages/app-shell/src/hooks/useApproverDirectory.ts

@@ -24,10 +24,19 @@
  *
  *   - one `find` per distinct directory OBJECT (`sys_position`, `sys_team`,
  *     `sys_business_unit`, `sys_user`), batched with `$in` over that object's
- *     values — the label leg;
+ *     values — the label leg for a kind the spec binds to a DATA row;
+ *   - one registry read per metadata TYPE, for a kind the spec binds to the
+ *     registry instead (`{ source: 'registry', type }`, objectui#7611 Q3b):
+ *     the console's metadata store, `ensureType(<type>)` — the
+ *     `GET /api/v1/meta/<type>` read the Approval Center's position labels and
+ *     the decision outputs already make — matched by the item's `name`. That is
+ *     the label leg for a registry-bound kind; nothing is read from a mirror
+ *     table for it;
  *   - for `position` refs only, `sys_user_position` -> `sys_user` — the staffing
  *     leg, which answers "who actually sits in this seat" and, when it comes
- *     back empty, the unstaffed state `77f846a8b` surfaces.
+ *     back empty, the unstaffed state `77f846a8b` surfaces. The junction stores
+ *     the position's machine NAME under either binding, so this leg does not
+ *     depend on which one the spec declares.
  *
  * The two legs fail INDEPENDENTLY. A business-app reader may hold list access to
  * `sys_position` and none to `sys_user_position`; `find` rejects that with
@@ -48,11 +57,13 @@
  * tests, which must not inherit each other's answers.
  */
 
-import { useEffect, useMemo, useRef, useState } from 'react';
-import { useAdapter } from '@object-ui/react';
+import { useContext, useEffect, useMemo, useRef, useState } from 'react';
+import { MetadataCtx, useAdapter } from '@object-ui/react';
 import {
   APPROVER_DIRECTORY_BINDINGS,
+  APPROVER_REGISTRY_TYPES,
   PERSON_DISPLAY_FIELDS,
+  approverKindIsResolvable,
   parseApproverRef,
   rowLabel,
   type ResolvedApprover,
@@ -126,6 +137,37 @@ async function resolveLabels(
   }
 }
 
+/** The one member of the console's metadata store the registry leg reads. */
+export interface ApproverRegistryReader {
+  ensureType: (type: string) => Promise<readonly unknown[]>;
+}
+
+/**
+ * Resolve the label leg for every reference of one REGISTRY-bound kind
+ * (objectui#7611 Q3b): list the kind's metadata type once and label each
+ * reference by the item whose `name` it carries. An item with no string
+ * `label` adds nothing, so the prettified machine name still reads.
+ * Rejects propagate to {@link loadSlate}, which isolates each kind.
+ */
+async function resolveRegistryLabels(
+  reader: ApproverRegistryReader,
+  kind: string,
+  type: string,
+  values: string[],
+  into: Map<string, ResolvedApprover>,
+): Promise<void> {
+  const items = await reader.ensureType(type);
+  const wanted = new Set(values);
+  for (const item of Array.isArray(items) ? items : []) {
+    const name = (item as { name?: unknown } | null)?.name;
+    const label = (item as { label?: unknown } | null)?.label;
+    if (typeof name !== 'string' || !wanted.has(name)) continue;
+    if (typeof label !== 'string' || !label.trim()) continue;
+    const ref = `${kind}:${name}`;
+    into.set(ref, { ...into.get(ref), label: label.trim() });
+  }
+}
+
 /**
  * Resolve the staffing leg for `position:` references — who sits in each seat.
  * A seat that the junction never mentions is recorded as `[]` (probed, empty),
@@ -179,28 +221,46 @@ async function resolveStaffing(
  * position its label. Every reference asked about is written back even when
  * nothing resolved, so a second mount does not re-ask a question the directory
  * has already declined to answer.
+ *
+ * Which label leg a kind takes is the spec's published binding, read per kind:
+ * a `data` binding reads its directory object through the adapter, a
+ * `registry` binding reads its metadata type through the store. A leg whose
+ * reader is absent (no adapter, no `<MetadataProvider>`) is skipped, which is
+ * the same answer as a leg that failed.
  */
-async function loadSlate(adapter: any, refs: string[]): Promise<void> {
+async function loadSlate(
+  adapter: any,
+  registry: ApproverRegistryReader | null,
+  refs: string[],
+): Promise<void> {
   const answers = new Map<string, ResolvedApprover>();
   const valuesByKind = new Map<string, string[]>();
   for (const raw of refs) {
     const ref = parseApproverRef(raw);
-    if (!ref || !APPROVER_DIRECTORY_BINDINGS[ref.kind]) continue;
+    if (!ref || !approverKindIsResolvable(ref.kind)) continue;
     const bucket = valuesByKind.get(ref.kind) ?? [];
     if (!bucket.includes(ref.value)) bucket.push(ref.value);
     valuesByKind.set(ref.kind, bucket);
   }
 
   await Promise.all(
-    [...valuesByKind].map(([kind, values]) =>
-      resolveLabels(adapter, kind, values, answers).catch(() => {
+    [...valuesByKind].map(([kind, values]) => {
+      const registryType = APPROVER_REGISTRY_TYPES[kind];
+      const leg = registryType
+        ? registry
+          ? resolveRegistryLabels(registry, kind, registryType, values, answers)
+          : Promise.resolve()
+        : adapter
+          ? resolveLabels(adapter, kind, values, answers)
+          : Promise.resolve();
+      return leg.catch(() => {
         /* label leg unavailable — the prettified machine name still reads */
-      }),
-    ),
+      });
+    }),
   );
 
   const positions = valuesByKind.get('position');
-  if (positions && positions.length > 0) {
+  if (adapter && positions && positions.length > 0) {
     await resolveStaffing(adapter, positions, answers).catch(() => {
       /* staffing unknown — NOT the same claim as "the seat is empty" */
     });
@@ -216,6 +276,18 @@ async function loadSlate(adapter: any, refs: string[]): Promise<void> {
  */
 export function useApproverDirectory(refs: readonly string[]): ApproverDirectory {
   const adapter = useAdapter() as any;
+  // The console's metadata store, or `null` outside a `<MetadataProvider>` —
+  // the reader of the registry label leg. Read raw rather than through
+  // `useMetadata()`, whose no-provider fallback answers every type with `[]`,
+  // and held in a ref: the store's context value is rebuilt whenever any type
+  // it caches settles, and no effect may key on that identity (AGENTS.md #10).
+  const metadataStore = useContext(MetadataCtx);
+  const registryRef = useRef<ApproverRegistryReader | null>(metadataStore);
+  // Synced in an effect, not during render; declared before the load effect
+  // below, so it runs first in the same commit.
+  useEffect(() => {
+    registryRef.current = metadataStore;
+  });
   // Sorted + de-duplicated so a re-render that reorders the slate does not
   // re-run the effect, and so the in-flight key is stable across callers.
   const wanted = useMemo(() => [...new Set(refs)].filter(Boolean).sort(), [refs]);
@@ -229,13 +301,14 @@ export function useApproverDirectory(refs: readonly string[]): ApproverDirectory
   }, []);
 
   useEffect(() => {
-    if (!adapter || wanted.length === 0) return;
+    const registry = registryRef.current;
+    if ((!adapter && !registry) || wanted.length === 0) return;
     const missing = wanted.filter((r) => !CACHE.has(r));
     if (missing.length === 0) return;
     const batch = missing.join('|');
     let pending = INFLIGHT.get(batch);
     if (!pending) {
-      pending = loadSlate(adapter, missing).finally(() => INFLIGHT.delete(batch));
+      pending = loadSlate(adapter, registry, missing).finally(() => INFLIGHT.delete(batch));
       INFLIGHT.set(batch, pending);
     }
     void pending.then(() => { if (alive.current) bump((n) => n + 1); });

packages/app-shell/src/utils/approverIdentity.registryBinding-7611.test.tsx

@@ -0,0 +1,151 @@
+/**
+ * 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#7611 Q3b (ruled objectstack#22865 Q3b → B) — the record page's
+ * approval panel and the override dialog NAME a position approver when the
+ * bundled spec binds `position` to the registry, `{ source: 'registry', type }`
+ * (objectstack#22710).
+ *
+ * The console objectstack ships is bundled with objectstack's own spec
+ * injected, so its `APPROVER_VALUE_SOURCES.position` is that registry member.
+ * `approverIdentity` derived its bindings from the `data` member only, so
+ * `position` dropped out of every arm: `formatIdentity('position:manager')`
+ * fell through to the 14-character truncation and read `positi…ager`, and
+ * `useApproverDirectory` asked nothing about it. This file mocks the spec into
+ * the injected shape — the projection is the bundled spec's own object, so this
+ * is what the shipped bundle evaluates.
+ *
+ * Pinned, per arm:
+ *   - registry: the machine name is prettified, never truncated; the ref is
+ *     handed to the directory; its label comes from the registry item read
+ *     through the console's metadata store (`ensureType('position')`), with no
+ *     read of the `sys_position` mirror; staffing still comes from the junction;
+ *   - record: a `team:` ref still resolves against `sys_team` through the
+ *     adapter, and an opaque id still truncates.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { renderHook, waitFor, cleanup } from '@testing-library/react';
+
+vi.mock('@objectstack/spec/automation', async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>();
+  const sources = actual.APPROVER_VALUE_SOURCES as Record<string, unknown>;
+  return {
+    ...actual,
+    // objectstack#22710's projection of `position` — the injected spec's shape.
+    APPROVER_VALUE_SOURCES: { ...sources, position: { source: 'registry', type: 'position' } },
+  };
+});
+
+import { AdapterCtx, MetadataCtx } from '@object-ui/react';
+import {
+  APPROVER_DIRECTORY_BINDINGS,
+  APPROVER_REGISTRY_TYPES,
+  DEFAULT_APPROVER_COPY,
+  approverDisplay,
+  formatIdentity,
+  unresolvedApproverRefs,
+} from './approverIdentity';
+import { useApproverDirectory, resetApproverDirectoryCache } from '../hooks/useApproverDirectory';
+
+/** 32 hex characters — an opaque row id, as a `team:` approver carries. */
+const TEAM_ID = '0123456789abcdef0123456789abcdef';
+
+beforeEach(() => {
+  resetApproverDirectoryCache();
+});
+
+afterEach(() => {
+  cleanup();
+});
+
+describe('approverIdentity under a registry-binding spec (objectui#7611 Q3b)', () => {
+  it('reads position as registry-bound and keeps it off the record arm', () => {
+    expect(APPROVER_REGISTRY_TYPES).toEqual({ position: 'position' });
+    expect(APPROVER_DIRECTORY_BINDINGS.position).toBeUndefined();
+    expect(APPROVER_DIRECTORY_BINDINGS.team).toMatchObject({ object: 'sys_team', valueField: 'id' });
+  });
+
+  it('prettifies a position approver instead of truncating it', () => {
+    expect(formatIdentity('position:manager')).toBe('Manager');
+    expect(formatIdentity('position:sales_manager')).toBe('Sales Manager');
+  });
+
+  it('still truncates an opaque id on the record arm', () => {
+    expect(formatIdentity(`team:${TEAM_ID}`)).toBe('team:0…cdef');
+  });
+
+  it('hands a server-unresolved position approver to the directory', () => {
+    const record = {
+      pending_approvers: ['position:manager', `team:${TEAM_ID}`, 'u_1'],
+      pending_approver_names: { u_1: 'Ada' },
+    };
+    expect(unresolvedApproverRefs(record)).toEqual(['position:manager', `team:${TEAM_ID}`]);
+  });
+
+  it('composes the registry label first, the prettified name second', () => {
+    expect(
+      approverDisplay('position:manager', { resolved: { label: 'Line Manager' }, copy: DEFAULT_APPROVER_COPY }).label,
+    ).toBe('Line Manager');
+    expect(approverDisplay('position:manager', { copy: DEFAULT_APPROVER_COPY }).label).toBe('Manager');
+  });
+});
+
+describe('useApproverDirectory under a registry-binding spec (objectui#7611 Q3b)', () => {
+  function harness() {
+    const adapter = {
+      find: vi.fn(async (object: string) => {
+        if (object === 'sys_user_position') return { data: [{ position: 'manager', user_id: 'u_1' }] };
+        if (object === 'sys_user') return { data: [{ id: 'u_1', full_name: 'Ada Holder' }] };
+        if (object === 'sys_team') return { data: [{ id: TEAM_ID, label: 'Platform Team' }] };
+        return { data: [] };
+      }),
+    };
+    const ensureType = vi.fn(async (type: string) =>
+      type === 'position'
+        ? [
+            { name: 'manager', label: 'Line Manager' },
+            { name: 'exec', label: 'Executive' },
+          ]
+        : [],
+    );
+    const store = { ensureType } as unknown as React.ContextType<typeof MetadataCtx>;
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <MetadataCtx.Provider value={store}>
+        <AdapterCtx.Provider value={adapter as never}>{children}</AdapterCtx.Provider>
+      </MetadataCtx.Provider>
+    );
+    return { adapter, ensureType, wrapper };
+  }
+
+  it('labels a position from the registry item, never from the sys_position mirror', async () => {
+    const { adapter, ensureType, wrapper } = harness();
+    const { result } = renderHook(() => useApproverDirectory(['position:manager']), { wrapper });
+    await waitFor(() => expect(result.current['position:manager']).toBeDefined());
+
+    expect(result.current['position:manager']).toEqual({ label: 'Line Manager', holders: ['Ada Holder'] });
+    expect(ensureType).toHaveBeenCalledWith('position');
+    const objects = adapter.find.mock.calls.map(([object]) => object);
+    expect(objects).not.toContain('sys_position');
+    // Staffing still reads the junction, which stores the position's name.
+    expect(objects).toContain('sys_user_position');
+  });
+
+  it('keeps the record arm on the adapter', async () => {
+    const { adapter, ensureType, wrapper } = harness();
+    const ref = `team:${TEAM_ID}`;
+    const { result } = renderHook(() => useApproverDirectory([ref]), { wrapper });
+    await waitFor(() => expect(result.current[ref]).toBeDefined());
+
+    expect(result.current[ref]).toEqual({ label: 'Platform Team' });
+    expect(adapter.find.mock.calls.map(([object]) => object)).toEqual(['sys_team']);
+    expect(ensureType).not.toHaveBeenCalled();
+  });
+});