Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/membership-tier-picker-reads-server-enum.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
*/
Expand Down
Original file line numberDiff line numberDiff line change
@@ -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: () => <div data-testid="record-lookup" />,
}));
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(
<ReferenceCombobox
resolved={{ kind: 'org-membership-level', source }}
value={value}
onCommit={vi.fn()}
/>,
);
}

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();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
Expand DownExpand Up@@ -158,19 +159,68 @@ export const KIND_TO_RECORD_LOOKUP: Partial<Record<ReferenceKind, RecordLookupBi
);

/**
* better-auth org-membership tiers. `org-membership-level` is intentionally NOT
* in {@link KIND_TO_META_TYPE}: it used to map to `client.list('role')`, but
* ADR-0090 D3 removed the `role` metadata type, so that call returned nothing
* and the picker silently degraded to a free-text box — which is how
* `sales_manager` got typed into a field that only ever accepts these three.
* The tier is a closed enum, so it renders as a STRICT select over these
* options (framework #3508) — free text would re-open the same trap.
* The membership-tier vocabulary, DERIVED from the spec — never restated here.
*
* `org-membership-level` is intentionally NOT in {@link KIND_TO_META_TYPE}: it
* used to map to `client.list('role')`, but ADR-0090 D3 removed the `role`
* metadata type, so that call returned nothing and the picker silently
* degraded to a free-text box — which is how `sales_manager` got typed into a
* closed enum. The tier renders as a STRICT select (framework #3508); free
* text would re-open the same trap.
*
* This list used to be hand-spelled `owner` / `admin` / `member` under a
* comment attributing the set to better-auth ("only ever accepts these
* three"). That copy went stale the moment ADR-0105 D8 added
* `delegated_admin` to `sys_member.role`: the picker offered a quarter less
* than the column stores, and a legitimately-saved `delegated_admin` approver
* rendered as `delegated_admin (invalid)` — a spec-valid, runtime-resolvable
* value labelled invalid to the author's face (objectui#5309).
*
* It now reads `BUILTIN_MEMBERSHIP_ROLE_OPTIONS`, which the spec publishes as
* the complete option list for `sys_member.role` and calls "the picker's
* vocabulary" in as many words — values AND labels — so the same drift cannot
* recur by construction. Same move, and the same reason, as
* {@link KIND_TO_RECORD_LOOKUP}.
*/
const ORG_MEMBERSHIP_LEVEL_OPTIONS: Option[] = BUILTIN_MEMBERSHIP_ROLE_OPTIONS.map(
({ value, label }) => ({ 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 {
Expand DownExpand Up@@ -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 (
<Select value={current || undefined} onValueChange={commitSelection} disabled={disabled}>
<SelectTrigger className="h-8 w-full text-sm">
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -55,11 +55,14 @@ export type FlowConfigFieldKind =
* {@link FlowReferenceSpec.objectSource}
* • `flow` → a flow, by name (`client.list('flow')`)
* • `org-membership-level`
* → a better-auth org-membership tier. A FIXED three-value
* enum (owner/admin/member) rendered as a STRICT select
* (framework #3508): there is no `role` metadata type to
* list (ADR-0090 D3), and free text is how dirty values
* like `sales_manager` got stored.
* → an org-membership tier. A CLOSED enum rendered as a
* STRICT select (framework #3508): there is no `role`
* metadata type to list (ADR-0090 D3), and free text is
* how dirty values like `sales_manager` got stored. The
* vocabulary is the spec's membership-role list, taken
* from the server-published `sources` entry when there
* is one — never spelled out here, which is how it went
* stale before (objectui#5309).
* • `user` / `team` / `department` / `position` → a DATA-record lookup on
* the matching directory object (`sys_user` / `sys_team`
* / `sys_business_unit` / `sys_position`) via the
Expand DownExpand Up@@ -631,7 +634,7 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
// #3447: CEL over current.* / trigger.* / vars.*, resolved at node
// entry — the value cell switches to the expression input.
{ value: 'expression', label: 'Expression (CEL)' },
{ value: 'org_membership_level', label: 'Organization membership (owner/admin/member)' },
{ value: 'org_membership_level', label: 'Organization membership tier' },
{ value: 'user', label: 'User' },
],
},
Expand Down
Loading