From dcb84af447fc7ad9b3693bec0f5d570eb2f57521 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 17 Jul 2026 23:17:49 +0800 Subject: [PATCH] fix(studio): approver Type dropdown drops deprecated `role`, membership-tier picker (framework #3133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two objectui-side gaps that let the ADR-0090 D3 approver `role` trap survive in the Studio flow designer, both found by dogfooding the running console. 1. `FlowObjectListField` had no `select` branch. An objectList column of kind `select` — which the approver `Type` is, derived from the spec enum — fell through to a plain ``. So the Type field was FREE TEXT: the options (and framework's ADR-0090 D3 `xEnumDeprecated` filtering that drops `role`) were computed and thrown away, and the author got no guidance at all. Added the branch: a real dropdown from `col.options`, honoring `xEnumDeprecated`. A STORED value no longer in the options (a 15.x `role` row) still renders, flagged `(deprecated)`, so editing a legacy row can't silently blank it — mirroring the framework runtime's keep-authored-spelling decision. 2. The `role`/`org-membership-level` reference picker called `client.list('role')` — a metadata type ADR-0090 D3 DELETED. It returned nothing, so the Value box degraded to free text. Replaced with a fixed owner/admin/member enum; `role` and `org_membership_level` both map to the new `org-membership-level` picker kind so a stored legacy row still renders. `json-schema-to-fields` now threads `xEnumDeprecated` from the schema into `enumOptions` (both the top-level and objectList-column paths). The hardcoded fallback config and the `ReferenceKind` union follow the rename. Verified end to end in the browser against a framework backend on the #3133 branch: the live approver Type dropdown shows [User, Org Membership Level, Position, Team, Department, Manager, Field, Queue] — no "Role". Co-Authored-By: Claude Opus 4.8 --- .../approver-type-membership-tier-picker.md | 24 ++++++ .../FlowObjectListField.select.test.tsx | 75 +++++++++++++++++++ .../inspectors/FlowObjectListField.tsx | 50 ++++++++++++- .../inspectors/FlowReferenceField.tsx | 16 +++- .../inspectors/flow-node-config.ts | 24 ++++-- .../inspectors/json-schema-to-fields.test.ts | 26 +++++-- .../inspectors/json-schema-to-fields.ts | 26 +++++-- 7 files changed, 223 insertions(+), 18 deletions(-) create mode 100644 .changeset/approver-type-membership-tier-picker.md create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.select.test.tsx diff --git a/.changeset/approver-type-membership-tier-picker.md b/.changeset/approver-type-membership-tier-picker.md new file mode 100644 index 000000000..3ccc9a4b7 --- /dev/null +++ b/.changeset/approver-type-membership-tier-picker.md @@ -0,0 +1,24 @@ +--- +"@object-ui/app-shell": patch +--- + +fix(studio): approver Type is a real dropdown that drops the deprecated `role` spelling (framework #3133) + +The flow designer's approver `Type` control silently rendered as free text: +`FlowObjectListField` had no `select` branch, so an objectList column of kind +`select` (which the approver type is, derived from the spec enum) fell through +to a plain `` and its computed options were never shown. Added the +missing branch — it renders a real dropdown from the column's `options`, and +keeps a **stored** value that is no longer offered (a deprecated enum member) +visible-but-flagged so editing a legacy row can't silently blank it. + +With the dropdown live, it honors framework's new `xEnumDeprecated` schema +annotation (ADR-0090 D3): the deprecated `role` approver type is dropped from +the options while `org_membership_level` is offered, so Studio no longer walks +authors into the trap of picking `role` (which resolves against the better-auth +membership tier and silently matches nobody). + +Also: the `org-membership-level` reference picker is a fixed three-value enum +(owner/admin/member) instead of the dead `client.list('role')` — the `role` +metadata type was removed by ADR-0090 D3, so that call returned nothing and the +Value box degraded to free text. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.select.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.select.test.tsx new file mode 100644 index 000000000..3753ae75c --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.select.test.tsx @@ -0,0 +1,75 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Regression: an objectList column of kind `select` must render a real + * dropdown from its `options`, not a free-text input. Before this branch + * existed the approver `Type` column (a select derived from the spec enum) + * fell through to a plain ``, so the computed options — including the + * ADR-0090 D3 filtering that drops the deprecated `role` spelling — were never + * shown, and Studio offered no guidance at all. + * + * Two behaviours are load-bearing: + * 1. the current option's label surfaces on the trigger, and + * 2. a STORED value that is no longer in `options` (a deprecated enum member) + * still renders — editing a legacy row must not silently blank it. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { FlowObjectListField } from './FlowObjectListField'; +import type { FlowConfigColumn } from './flow-node-config'; + +afterEach(cleanup); + +// Mirrors the approver objectList: a `select` Type column + a text Value. +const COLUMNS: FlowConfigColumn[] = [ + { + key: 'type', + label: 'Type', + kind: 'select', + options: [ + { value: 'user', label: 'User' }, + { value: 'org_membership_level', label: 'Org Membership Level' }, + { value: 'position', label: 'Position' }, + ], + }, + { key: 'value', label: 'Value', kind: 'text' }, +]; + +function renderList(rows: Array>) { + return render( + , + ); +} + +describe('FlowObjectListField — select column', () => { + it('renders a select column as a combobox (not free text)', () => { + renderList([{ type: 'position', value: 'finance' }]); + // The trigger surfaces the chosen option's LABEL, which a plain + // (value = the raw machine name) never would. + expect(screen.getByText('Position')).toBeInTheDocument(); + expect(screen.queryByText('Role')).not.toBeInTheDocument(); + }); + + it('offers only the non-deprecated options — `role` is absent', () => { + // The options a fresh row can pick from must match COLUMNS.options exactly; + // `role` was never an option here (it is filtered upstream by + // xEnumDeprecated), and the component must not reintroduce it. + expect(COLUMNS[0].options?.map((o) => o.value)).not.toContain('role'); + }); + + it('still renders a STORED deprecated value, flagged, so a legacy row is not blanked', () => { + // `role` is not among the offered options, but a flow authored on 15.x has + // it stored. It must remain visible (and marked) rather than vanish. + renderList([{ type: 'role', value: 'admin' }]); + expect(screen.getByText('role (deprecated)')).toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.tsx index d164d3a1c..f899df6e8 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.tsx @@ -13,7 +13,10 @@ import * as React from 'react'; import { Plus, X } from 'lucide-react'; -import { Button, Input, Label, Checkbox } from '@object-ui/components'; +import { + Button, Input, Label, Checkbox, + Select, SelectContent, SelectItem, SelectTrigger, SelectValue, +} from '@object-ui/components'; import { uniqueId } from './_shared'; import type { FlowConfigColumn } from './flow-node-config'; import { ReferenceCombobox, resolveRefKind, type FlowReferenceContext } from './FlowReferenceField'; @@ -192,6 +195,51 @@ export function FlowObjectListField({ showHint={false} /> + ) : col.kind === 'select' ? ( + (() => { + const current = + typeof row.values[col.key] === 'string' ? (row.values[col.key] as string) : ''; + const opts = col.options ?? []; + // A stored value dropped from the options (a deprecated + // enum member, e.g. the `role` approver type per + // ADR-0090 D3) must still render, or editing a legacy row + // would silently blank it. Surface it as selectable but + // flag it — it is not offered to fresh rows. + const shown = + current && !opts.some((o) => o.value === current) + ? [...opts, { value: current, label: `${current} (deprecated)` }] + : opts; + return ( +
+ +
+ ); + })() ) : col.kind === 'expression' ? (
> = { object: 'object', flow: 'flow', - role: 'role', position: 'position', user: 'user', team: 'team', @@ -64,6 +63,20 @@ const KIND_TO_META_TYPE: Partial> = { 'email-template': 'email_template', }; +/** + * 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 the options are supplied here instead. + */ +const ORG_MEMBERSHIP_LEVEL_OPTIONS: Option[] = [ + { value: 'owner', label: 'Owner' }, + { value: 'admin', label: 'Admin' }, + { value: 'member', label: 'Member' }, +]; + /** A concrete (non-polymorphic) reference resolution. */ export interface ResolvedRef { kind: ReferenceKind; @@ -320,6 +333,7 @@ export function ReferenceCombobox({ resolved, value, onCommit, onBlur, disabled, } if (kind === 'connector-action') return connectorActionOptions; if (kind === 'connector') return connectorListOptions; + if (kind === 'org-membership-level') return ORG_MEMBERSHIP_LEVEL_OPTIONS; if (kind === 'node') { const nodes = Array.isArray(ctx.draft.nodes) ? (ctx.draft.nodes as Array>) : []; const currentId = typeof ctx.node?.id === 'string' ? ctx.node.id : undefined; diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts index 883926200..653693d57 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts @@ -46,7 +46,12 @@ export type FlowConfigFieldKind = * • `object-field` → a field of some object; the object is resolved via * {@link FlowReferenceSpec.objectSource} * • `flow` → a flow, by name (`client.list('flow')`) - * • `role` → a better-auth org-membership tier (`client.list('role')`) + * • `org-membership-level` + * → a better-auth org-membership tier. A FIXED three-value + * enum (owner/admin/member), not a catalog: there is no + * `role` metadata type to list (ADR-0090 D3 renamed + * `sys_role` → `sys_position`), so this kind supplies its + * own options rather than calling `client.list`. * • `position` → a position / 岗位 by machine name (`client.list('position')`); * holders resolve via `sys_user_position` (ADR-0090 D3) * • `node` → another node in *this* flow, by id (read from the draft) @@ -62,7 +67,7 @@ export type ReferenceKind = | 'object' | 'object-field' | 'flow' - | 'role' + | 'org-membership-level' | 'position' | 'node' | 'user' @@ -416,10 +421,14 @@ const FLOW_NODE_CONFIG: Record = { key: 'type', label: 'Type', kind: 'select', + // `role` is deliberately absent: it is the deprecated spelling of + // `org_membership_level` (ADR-0090 D3) and reads as "the old name for + // position", which is the trap — a stored `role` row still renders + // and resolves, but the designer never authors a new one. options: [ { value: 'user', label: 'User' }, - { value: 'role', label: 'Role' }, { value: 'position', label: 'Position' }, + { value: 'org_membership_level', label: 'Organization membership (owner/admin/member)' }, { value: 'team', label: 'Team' }, { value: 'department', label: 'Department' }, { value: 'manager', label: 'Manager' }, @@ -434,14 +443,19 @@ const FLOW_NODE_CONFIG: Record = { key: 'value', label: 'Value', kind: 'reference', - placeholder: 'user id / role / position / field — per type', + placeholder: 'user id / position / membership tier / field — per type', ref: { kindFrom: 'type', objectSource: '$trigger', + // `role` maps to the same picker as `org_membership_level`: the + // designer no longer offers it, but a flow authored on 15.x still + // has stored rows, and they must keep rendering for the length of + // the deprecation window. map: { user: 'user', - role: 'role', position: 'position', + org_membership_level: 'org-membership-level', + role: 'org-membership-level', team: 'team', department: 'department', field: 'object-field', diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.test.ts index ef4f27fff..239f4cbf2 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.test.ts @@ -18,14 +18,23 @@ const APPROVAL_CONFIG_SCHEMA = { items: { type: 'object', properties: { - type: { type: 'string', enum: ['user', 'role', 'position', 'team', 'department', 'manager', 'field', 'queue'] }, + type: { + type: 'string', + enum: ['user', 'org_membership_level', 'role', 'position', 'team', 'department', 'manager', 'field', 'queue'], + // `role` still parses (a 15.x flow must keep loading) but is not + // offered for new authoring — ADR-0090 D3. + xEnumDeprecated: ['role'], + }, value: { description: 'User id / membership tier / position / team / department / field / queue — per `type`', type: 'string', xRef: { kindFrom: 'type', objectSource: '$trigger', - map: { user: 'user', role: 'role', position: 'position', team: 'team', department: 'department', field: 'object-field', queue: 'queue' }, + // Mirrors @objectstack/spec approval.zod.ts: both the canonical + // spelling and its deprecated `role` alias point at the same + // picker kind (ADR-0090 D3). + map: { user: 'user', org_membership_level: 'org-membership-level', role: 'org-membership-level', position: 'position', team: 'team', department: 'department', field: 'object-field', queue: 'queue' }, }, }, }, @@ -103,14 +112,21 @@ describe('jsonSchemaToFlowFields', () => { expect(colKeys).toEqual(['type', 'value']); const typeCol = approvers.columns!.find((c) => c.key === 'type')!; expect(typeCol.kind).toBe('select'); - expect(typeCol.options!.map((o) => o.value)).toEqual(['user', 'role', 'position', 'team', 'department', 'manager', 'field', 'queue']); + // `role` is dropped from the OPTIONS (xEnumDeprecated) while staying in the + // enum: the designer must not hand an author the deprecated spelling, which + // reads as "the old name for position" and silently routes to nobody. + expect(typeCol.options!.map((o) => o.value)).toEqual(['user', 'org_membership_level', 'position', 'team', 'department', 'manager', 'field', 'queue']); + expect(typeCol.options!.map((o) => o.value)).not.toContain('role'); const valueCol = approvers.columns!.find((c) => c.key === 'value')!; - // Polymorphic reference: the picker follows the row's `type`. + // Polymorphic reference: the picker follows the row's `type`. The + // deprecated `role` discriminator survives and resolves to the SAME picker + // kind as the canonical spelling — a flow authored on 15.x must keep + // rendering for the length of its deprecation window (ADR-0090 D3). expect(valueCol.kind).toBe('reference'); expect(valueCol.ref).toEqual({ kindFrom: 'type', objectSource: '$trigger', - map: { user: 'user', role: 'role', position: 'position', team: 'team', department: 'department', field: 'object-field', queue: 'queue' }, + map: { user: 'user', org_membership_level: 'org-membership-level', role: 'org-membership-level', position: 'position', team: 'team', department: 'department', field: 'object-field', queue: 'queue' }, }); expect(valueCol.placeholder).toBe('User id / membership tier / position / team / department / field / queue — per `type`'); }); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.ts b/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.ts index 198ee9ba5..3bbfa5109 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.ts @@ -37,6 +37,15 @@ import type { FlowConfigField, FlowConfigColumn, FlowConfigFieldKind, FlowRefere interface JsonSchemaNode { type?: string | string[]; enum?: unknown[]; + /** + * Enum members that still parse but must not be offered for new authoring + * (`@objectstack/spec` `.meta({ xEnumDeprecated })`). We drop them from the + * select's options while leaving them valid: a stored value keeps rendering, + * the designer just stops handing authors the deprecated spelling. Without + * this, a backward-compatible alias in the spec enum reappears in every + * picker derived from it — e.g. the approver `role` trap (ADR-0090 D3). + */ + xEnumDeprecated?: unknown[]; const?: unknown; default?: unknown; description?: string; @@ -59,7 +68,7 @@ const REFERENCE_KINDS: ReadonlySet = new Set([ 'object', 'object-field', 'flow', - 'role', + 'org-membership-level', 'position', 'node', 'user', @@ -126,10 +135,15 @@ function schemaType(node: JsonSchemaNode): string | undefined { return undefined; } -/** Build `{ value, label }` options from a string enum. */ -function enumOptions(values: unknown[]): Array<{ value: string; label: string }> { +/** + * Build `{ value, label }` options from a string enum, minus any member the + * schema marks deprecated ({@link JsonSchemaNode.xEnumDeprecated}). + */ +function enumOptions(values: unknown[], deprecated?: unknown[]): Array<{ value: string; label: string }> { + const skip = new Set((deprecated ?? []).filter((v): v is string => typeof v === 'string')); return values .filter((v): v is string => typeof v === 'string') + .filter((v) => !skip.has(v)) .map((v) => ({ value: v, label: humanizeKey(v) })); } @@ -168,7 +182,7 @@ function columnsFor(item: JsonSchemaNode): FlowConfigColumn[] { kind = 'reference'; } else if (Array.isArray(prop.enum)) { kind = 'select'; - options = enumOptions(prop.enum); + options = enumOptions(prop.enum, prop.xEnumDeprecated); } else if (t === 'boolean') { kind = 'boolean'; } else { @@ -245,7 +259,7 @@ export function jsonSchemaToFlowFields(schema: unknown): FlowConfigField[] | nul // The gate adopts the parent group's label; siblings keep their own. ...(isGate ? { label: prop.title || humanizeKey(key), ...(prop.description ? { help: prop.description } : {}), ...(defaultString(sp) ? { defaultValue: defaultString(sp) } : {}) } : meta(sp, subKey)), ...(subRef ? { ref: subRef } : {}), - ...(kind === 'select' && Array.isArray(sp.enum) ? { options: enumOptions(sp.enum) } : {}), + ...(kind === 'select' && Array.isArray(sp.enum) ? { options: enumOptions(sp.enum, sp.xEnumDeprecated) } : {}), ...(hasEnabled && !isGate ? { showWhen: { field: `${key}.enabled`, equals: ['true'] } } : {}), }; fields.push(field); @@ -268,7 +282,7 @@ export function jsonSchemaToFlowFields(schema: unknown): FlowConfigField[] | nul path: ['config', key], kind, ...meta(prop, key), - ...(kind === 'select' && Array.isArray(prop.enum) ? { options: enumOptions(prop.enum) } : {}), + ...(kind === 'select' && Array.isArray(prop.enum) ? { options: enumOptions(prop.enum, prop.xEnumDeprecated) } : {}), }); }