diff --git a/.changeset/membership-tier-picker-reads-server-enum.md b/.changeset/membership-tier-picker-reads-server-enum.md new file mode 100644 index 000000000..6d81d656d --- /dev/null +++ b/.changeset/membership-tier-picker-reads-server-enum.md @@ -0,0 +1,30 @@ +--- +'@object-ui/app-shell': patch +--- + +The approver membership-tier picker offers the tiers the server accepts, and a stored `delegated_admin` is no longer labelled "(invalid)". + +The strict select for `org_membership_level` approvers carried a hand-spelled +`owner` / `admin` / `member` array, under a comment attributing the set to +better-auth. ADR-0105 D8 added `delegated_admin` to `sys_member.role`, and the +copy went stale in both directions at once: the tier could not be picked, and a +legitimately-saved `{ type: 'org_membership_level', value: 'delegated_admin' }` +approver rendered as `delegated_admin (invalid)` — a spec-valid, +runtime-resolvable value labelled invalid to the author's face. + +The picker now prefers the server-published enum (`xRef.sources[...]`, carried +through by `json-schema-to-fields`) and uses it verbatim, order included — the +same precedence rule this file already applies to record lookups, and the only +one that cannot drift from what the engine accepts. A tier the local pin has +never heard of renders as a humanized choice rather than a raw token, so the +next vocabulary addition reaches authors without an objectui release. + +The local list survives only as the fallback for a server predating the +annotation, and is now DERIVED from the spec's `BUILTIN_MEMBERSHIP_ROLE_OPTIONS` +— which that package documents as "the picker's vocabulary" and ships with +labels — so it is no longer a second source of truth that can go stale. A +published-but-empty enum falls back rather than rendering an empty strict +select, which would trap the author with no way to express a value at all. + +`(invalid)` keeps its meaning: a value outside the vocabulary the server +actually published is still flagged. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.lookup.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.lookup.test.tsx index a4f1c6d1f..b7bfcb659 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.lookup.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.lookup.test.tsx @@ -16,7 +16,9 @@ * 3. without an adapter (offline preview gallery) it degrades to free text * and does NOT fall back to the metadata list; * 4. `org-membership-level` is a STRICT select (free text is how - * `sales_manager` got stored into a three-value enum); + * `sales_manager` got stored into a closed enum). The tier VOCABULARY — + * published-enum-first, spec-derived fallback — is covered in + * `FlowReferenceField.membershipTier.test.tsx` (objectui#5309); * 5. `manager` is auto-resolved (disabled cell + explanation); * 6. `queue` warns that the runtime resolves it to nobody. */ diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.membershipTier.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.membershipTier.test.tsx new file mode 100644 index 000000000..abecbac38 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.membershipTier.test.tsx @@ -0,0 +1,203 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#5309 — the approver membership-tier select must offer the tiers the + * SERVER accepts, never a list hand-spelled in this package. + * + * The picker used to carry a literal `owner` / `admin` / `member` array under a + * comment attributing the set to better-auth. ADR-0105 D8 added + * `delegated_admin` to `sys_member.role`, and the copy went stale in the two + * ways a stale vocabulary always goes wrong: + * + * 1. the tier could not be PICKED — the control offered a quarter less than + * the column stores and the schema publishes; + * 2. a stored `{ type: 'org_membership_level', value: 'delegated_admin' }` + * row rendered as `delegated_admin (invalid)` — a spec-valid, + * runtime-resolvable approver labelled invalid to the author's face. + * + * Load-bearing behaviours, in the order the fix establishes them: + * + * 1. a SERVER-published enum (`xRef.sources[...]`, carried through by + * `json-schema-to-fields`) wins over the local fallback — the same + * precedence rule this file already applies to record lookups, and the + * only one that cannot drift from what the engine accepts; + * 2. the published vocabulary is used VERBATIM — order included — so the + * picker cannot quietly re-impose a local ordering; + * 3. a tier the local pin has never heard of still renders as a real, + * humanized choice rather than a raw token; + * 4. with NO published source (a server predating the annotation) the + * fallback is DERIVED from the spec's own published option list, so it is + * not a second source of truth and cannot go stale the way the literal + * did; + * 5. an empty / non-enum source falls back rather than rendering an EMPTY + * strict select — a select with nothing in it traps the author, the one + * thing this control must never do; + * 6. the reported symptom: a stored `delegated_admin` stops rendering + * "(invalid)" — while a genuinely out-of-vocabulary value still does. + * + * ## Pin state at the time of writing (deliberate, read before editing) + * + * This repo pins `@objectstack/spec@17.0.0`, which does NOT yet carry + * objectstack#9942 (`ORG_MEMBERSHIP_LEVELS` derived from + * `BUILTIN_MEMBERSHIP_ROLES`): its `APPROVER_VALUE_SOURCES.org_membership_level` + * projection still publishes three values. Nothing here asserts that + * projection — it would be red for reasons unrelated to this behaviour. + * + * What this pin DOES carry is `BUILTIN_MEMBERSHIP_ROLE_OPTIONS`, already the + * whole four-value vocabulary with labels, which the spec documents as "the + * picker's vocabulary". That is what the fallback derives from, so every + * assertion below holds under the current pin AND after the bump. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { BUILTIN_MEMBERSHIP_ROLE_OPTIONS } from '@objectstack/spec/identity'; + +const state = vi.hoisted(() => { + const metaList = vi.fn(async () => [] as unknown[]); + return { + metaList, + // STABLE identity, like the real memoized client — a fresh `{ list }` per + // render would setState → re-render → setState forever and hang the run. + metadataClient: { list: metaList }, + }; +}); + +vi.mock('@object-ui/react', () => ({ + useAdapter: () => null, + // @object-ui/components wires this at module scope (related-count-store). + subscribeDataChanges: () => () => {}, +})); +vi.mock('@object-ui/fields', () => ({ + LookupField: () =>
, +})); +vi.mock('../useMetadata', () => ({ + useMetadataClient: () => state.metadataClient, +})); +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [] }), +})); + +import { ReferenceCombobox, membershipLevelOptions } from './FlowReferenceField'; +import type { RefValueSource } from './flow-node-config'; + +afterEach(() => { + cleanup(); + state.metaList.mockClear(); +}); + +/** The four-value vocabulary as a server that carries objectstack#9942 publishes it. */ +const SERVER_FOUR: RefValueSource = { + source: 'enum', + values: ['member', 'delegated_admin', 'admin', 'owner'], +}; + +function renderTier(value: string, source?: RefValueSource) { + return render( + , + ); +} + +describe('membershipLevelOptions — the published enum wins (objectui#5309)', () => { + it('uses the server vocabulary VERBATIM, order included', () => { + // Deliberately NOT the spec's display order: if the picker re-imposed its + // own list this would come back owner-first, and a server that reorders + // (or narrows) its enum would be silently overridden. + expect(membershipLevelOptions(SERVER_FOUR).map((o) => o.value)).toEqual([ + 'member', + 'delegated_admin', + 'admin', + 'owner', + ]); + }); + + it('offers exactly what the server published, even when that is NARROWER', () => { + // A stack that restricts the tier vocabulary must not have the local list + // silently merged back in — that is the drift this whole card is about. + const narrowed = membershipLevelOptions({ source: 'enum', values: ['owner', 'member'] }); + expect(narrowed.map((o) => o.value)).toEqual(['owner', 'member']); + expect(narrowed.map((o) => o.label)).toEqual(['Owner', 'Member']); + }); + + it('humanizes a tier this pin has never heard of instead of showing a raw token', () => { + // The forward-compatibility half: the next vocabulary addition renders as + // a real choice on an un-bumped objectui, rather than as `regional_lead`. + const opts = membershipLevelOptions({ source: 'enum', values: ['owner', 'regional_lead'] }); + expect(opts).toEqual([ + { value: 'owner', label: 'Owner' }, + { value: 'regional_lead', label: 'Regional Lead' }, + ]); + }); +}); + +describe('membershipLevelOptions — the fallback is derived, not restated (objectui#5309)', () => { + it('falls back to the spec-published option list when the server publishes none', () => { + // The derivation pin: re-hardcoding a literal here goes red, whatever the + // literal says, because it can only match by copying the spec. + expect(membershipLevelOptions(undefined)).toEqual( + BUILTIN_MEMBERSHIP_ROLE_OPTIONS.map(({ value, label }) => ({ value, label })), + ); + }); + + it('offers delegated_admin on the fallback path too', () => { + // The reported defect, at the vocabulary level: this is what the literal + // `owner` / `admin` / `member` array could not do. Assertable on the + // CURRENT pin — `BUILTIN_MEMBERSHIP_ROLE_OPTIONS` is already four-valued. + const values = membershipLevelOptions(undefined).map((o) => o.value); + expect(values).toContain('delegated_admin'); + // Counter-probe for the assertion above: a term that is genuinely absent, + // so `toContain` is shown to be capable of failing here. + expect(values).not.toContain('sales_manager'); + expect(membershipLevelOptions(undefined).find((o) => o.value === 'delegated_admin')?.label) + .toBe('Delegated Admin'); + }); + + it('falls back rather than rendering an EMPTY strict select', () => { + // A published-but-empty enum is the one input that could leave the author + // with a select containing nothing at all. + expect(membershipLevelOptions({ source: 'enum', values: [] })).toEqual( + membershipLevelOptions(undefined), + ); + }); + + it('falls back for a source that is not an enum at all', () => { + for (const source of [{ source: 'auto' }, { source: 'unsupported' }] as RefValueSource[]) { + expect(membershipLevelOptions(source)).toEqual(membershipLevelOptions(undefined)); + } + }); +}); + +describe('ReferenceCombobox — a stored delegated_admin is not "(invalid)" (objectui#5309)', () => { + it('renders the stored tier as a real choice with no published source', () => { + // THE reported symptom, on the path an un-annotated server takes. + renderTier('delegated_admin'); + expect(screen.getByText('Delegated Admin')).toBeInTheDocument(); + expect(screen.queryByText('delegated_admin (invalid)')).not.toBeInTheDocument(); + }); + + it('renders the stored tier as a real choice when the server publishes the enum', () => { + renderTier('delegated_admin', SERVER_FOUR); + expect(screen.getByText('Delegated Admin')).toBeInTheDocument(); + expect(screen.queryByText('delegated_admin (invalid)')).not.toBeInTheDocument(); + }); + + it('still flags a value the SERVER says is outside its vocabulary', () => { + // The flag must keep meaning something: with a narrowed server enum, + // `delegated_admin` really is not storable, and hiding that would be the + // opposite error to the one being fixed. + renderTier('delegated_admin', { source: 'enum', values: ['owner', 'member'] }); + expect(screen.getByText('delegated_admin (invalid)')).toBeInTheDocument(); + }); + + it('still flags genuinely dirty legacy data', () => { + // NOTE: green both before and after this change — it pins the "(invalid)" + // affordance against a fix-by-deletion, not the fix itself. + renderTier('sales_manager'); + expect(screen.getByText('sales_manager (invalid)')).toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.tsx index f63a6f9f1..a96b050a3 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.tsx @@ -40,6 +40,7 @@ import { } from '@object-ui/components'; import { Pencil, Search } from 'lucide-react'; import { APPROVER_VALUE_SOURCES } from '@objectstack/spec/automation'; +import { BUILTIN_MEMBERSHIP_ROLE_OPTIONS } from '@objectstack/spec/identity'; import { useAdapter } from '@object-ui/react'; import { LookupField } from '@object-ui/fields'; import type { FlowReferenceSpec, ReferenceKind, RefValueSource } from './flow-node-config'; @@ -158,19 +159,68 @@ export const KIND_TO_RECORD_LOOKUP: Partial ({ value, label }), +); + +/** + * Label a tier the SERVER published. A value the spec already knows keeps the + * spec's own label; an unknown one is humanized (`delegated_admin` → + * "Delegated Admin") rather than shown as a raw token, so a vocabulary this + * package's pin predates still reads as a real choice. + */ +function membershipLevelLabel(value: string): string { + const known = ORG_MEMBERSHIP_LEVEL_OPTIONS.find((o) => o.value === value); + if (known) return known.label; + return value + .split('_') + .filter(Boolean) + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(' '); +} + +/** + * The tier options to offer, preferring the SERVER-published enum. + * + * The same precedence rule this file already states for record lookups (see + * {@link recordLookupFor}): the schema's own `xRef.sources` entry wins, + * because it cannot drift from the vocabulary the engine actually accepts. + * {@link ORG_MEMBERSHIP_LEVEL_OPTIONS} is the fallback for a server predating + * the annotation — and, being spec-derived, is not a second source of truth. + * + * An enum source publishing NO values falls back rather than rendering an + * empty strict select: a select with nothing in it traps the author with no + * way to express a value at all, the one thing this control must never do. */ -const ORG_MEMBERSHIP_LEVEL_OPTIONS: Option[] = [ - { value: 'owner', label: 'Owner' }, - { value: 'admin', label: 'Admin' }, - { value: 'member', label: 'Member' }, -]; +export function membershipLevelOptions(source: RefValueSource | undefined): Option[] { + if (source?.source === 'enum' && source.values.length > 0) { + return source.values.map((value) => ({ value, label: membershipLevelLabel(value) })); + } + return ORG_MEMBERSHIP_LEVEL_OPTIONS; +} /** A concrete (non-polymorphic) reference resolution. */ export interface ResolvedRef { @@ -640,14 +690,18 @@ export function ReferenceCombobox({ resolved, value, onCommit, onBlur, onSelect, } // Closed enum → strict select, never free text: `sales_manager` typed into - // a membership-tier box matches nobody at runtime. A stored value outside - // the enum (legacy dirty data) still renders, flagged, so editing an old - // row never silently blanks it — mirroring the repeater's select cells. + // a membership-tier box matches nobody at runtime. The vocabulary comes from + // the SERVER when it publishes one and from the spec-derived fallback + // otherwise — never from a list hand-spelled here (objectui#5309). A stored + // value outside the enum (legacy dirty data) still renders, flagged, so + // editing an old row never silently blanks it — mirroring the repeater's + // select cells. if (kind === 'org-membership-level') { const current = value != null ? String(value) : ''; - const shown = current && !ORG_MEMBERSHIP_LEVEL_OPTIONS.some((o) => o.value === current) - ? [...ORG_MEMBERSHIP_LEVEL_OPTIONS, { value: current, label: `${current} (invalid)` }] - : ORG_MEMBERSHIP_LEVEL_OPTIONS; + const tiers = membershipLevelOptions(resolved?.source); + const shown = current && !tiers.some((o) => o.value === current) + ? [...tiers, { value: current, label: `${current} (invalid)` }] + : tiers; return (