diff --git a/.changeset/3719-settings-valuedomain-combobox.md b/.changeset/3719-settings-valuedomain-combobox.md new file mode 100644 index 0000000000..f39d01bbcc --- /dev/null +++ b/.changeset/3719-settings-valuedomain-combobox.md @@ -0,0 +1,36 @@ +--- +'@object-ui/console': patch +--- + +Setup's settings selects now follow the specifier's `valueDomain` declaration instead of +treating the curated `options` table as the domain (objectui#3719). + +Since objectstack#5712 / PR objectstack#6581 a settings specifier may declare +`valueDomain` (`iana_time_zone` | `iso_4217_currency` | `iso_3166_alpha2`), and when it +does the **standard's membership is the enforcement boundary** — the server accepts +`timezone: 'Europe/Zurich'` and `currency: 'CHF'`, neither of which is in the manifest's +list. The console kept drawing those keys as closed dropdowns, so an admin could author +only the 17 curated zones and 9 curated currencies while the contract took the whole +domain; every other legal value was reachable by API or `OS_LOCALIZATION_*` env only. The +keys' own descriptions had promised "IANA zone" / "ISO 4217 code" all along. + +`case 'select'` in `SettingsField` now keys the control off the declaration. Declared → +an editable combobox: the curated options stay on as suggestions (native ``, the +same suggest-but-allow-anything affordance `FlowReferenceField` uses — no new dependency), +free text is committed verbatim, and an out-of-domain value is refused by the server with +`invalid_value` + `constraint: { valueDomain }` into the field-error slot that already +exists. + +**Undeclared → the closed dropdown is untouched**, which is half the change rather than a +caveat. Those `options` are still exhaustive under objectstack#5131 (the sms/mail provider +selects), and `localization.locale` had its domain declaration deliberately **rejected** in +objectstack#6515 because its options *are* the shipped catalogs. Widening those to free +input would be a regression wearing this fix's clothes, so the two branches are pinned +against each other from the specifier data rather than from a list of key names — a key +that gains a domain server-side joins the right side of the pin with no edit here. + +Root cause, because it will recur: `Specifier` in `pages/settings/types.ts` is a +hand-written **local mirror** of the server's shape, not an import, so nothing tells it when +the schema grows — and TypeScript reports nothing, because a narrower mirror is a +structurally valid reading of a wider object. `valueDomain` is added there and the file +header now says to check the mirror first when a settings feature "doesn't render". diff --git a/apps/console/src/pages/settings/SettingsField.tsx b/apps/console/src/pages/settings/SettingsField.tsx index c8548d24be..09394a1000 100644 --- a/apps/console/src/pages/settings/SettingsField.tsx +++ b/apps/console/src/pages/settings/SettingsField.tsx @@ -159,6 +159,69 @@ function FieldError({ id, message }: { id: string; message: string }) { ); } +/** + * — the control a `select` gets when its specifier declares a + * `valueDomain` (objectstack#5712 / PR objectstack#6581). + * + * Such a key is judged against a STANDARD, not against the manifest's table: + * `PUT /api/settings/localization` takes any IANA zone or ISO 4217 code, so the + * 17 curated timezones and 9 currencies are a convenience list, not the domain. + * A closed dropdown therefore advertises a narrower contract than the server + * enforces, and leaves every legal value outside the table reachable only by + * API or env. + * + * Native `` gives exactly suggest-but-allow-anything, with zero extra + * dependencies and built-in accessibility — the same reason the flow designer's + * `FlowReferenceField` uses it. The curated `options` stay visible as + * suggestions; free text is committed verbatim, and an out-of-domain value is + * refused by the server with `invalid_value` + `constraint: { valueDomain }`, + * which lands in the field-error slot the wrapper already owns. + * + * Props beyond its own are forwarded to the `` so `wrapper`'s + * `aria-invalid` / `aria-describedby` reach the focusable control rather than a + * wrapping node (same seam as Combobox's trigger pass-through, objectui#3318). + */ +function DomainCombobox({ + id, + value, + options, + disabled, + onChange, + ...inputProps +}: { + id: string; + value: unknown; + options: Array<{ value: string; label: string }>; + disabled?: boolean; + onChange: (next: unknown) => void; +} & Omit, 'value' | 'onChange' | 'id'>) { + const listId = `${id}-domain`; + return ( + <> + + // renders a dead dropdown affordance on some browsers. + list={options.length > 0 ? listId : undefined} + value={value == null ? '' : String(value)} + disabled={disabled} + onChange={(e) => onChange(e.target.value)} + {...inputProps} + /> + {options.length > 0 ? ( + + {options.map((opt) => ( + + ))} + + ) : null} + + ); +} + export function SettingsField(props: SettingsFieldProps) { const { spec, resolved, value, onChange, onAction, locked, saving, labels, error } = props; const id = useId(); @@ -355,7 +418,33 @@ export function SettingsField(props: SettingsFieldProps) { /> ); - case 'select': + case 'select': { + // Keyed off the DECLARATION, never off the key: a key that gains a + // domain server-side gets the right control here with no edit. + // + // Declared → the standard is the enforcement boundary, so an editable + // combobox (objectstack#5712). ABSENT → ⛔ the closed dropdown stays + // exactly as it was: those `options` are still exhaustive (objectstack + // #5131 semantics — the sms/mail provider selects), and + // `localization.locale` had its domain declaration deliberately REJECTED + // in objectstack#6515 because its options ARE the shipped catalogs. + // Widening those to free input would be a regression wearing this fix's + // clothes, so the two branches are pinned against each other in + // `__tests__/SettingsField.valueDomain.test.tsx`. + if (spec.valueDomain) { + return wrapper( + ({ + value: String(opt.value), + label: renderOptionLabel(opt), + }))} + />, + ); + } return wrapper( , ); + } case 'radio': return wrapper( s.valueDomain); +const UNDECLARED = LOCALIZATION_SELECTS.filter((s) => !s.valueDomain); + +/** + * A legal member of each domain that is deliberately outside the curated + * `options` — the card's own repro values for the first two, both accepted by + * the server today. Keyed by DOMAIN, so a new key carrying a known domain needs + * no entry. + */ +const OUTSIDE_THE_CURATED_LIST: Record = { + iana_time_zone: 'Europe/Zurich', + iso_4217_currency: 'CHF', + iso_3166_alpha2: 'CH', +}; + +const optionValues = (spec: Specifier) => (spec.options ?? []).map((o) => String(o.value)); + +function renderField(spec: Specifier, extra: Partial[0]> = {}) { + const onChange = vi.fn(); + const view = render( + , + ); + return { ...view, onChange }; +} + +afterEach(cleanup); + +describe('SettingsField — `select` follows the specifier\'s valueDomain declaration', () => { + /** + * Guard first: both loops below are data-driven, so an empty group would let + * them pass while asserting nothing. This is the assertion that keeps the + * `.filter` honest. + */ + it('the fixture actually carries both kinds', () => { + expect(DECLARED.length).toBeGreaterThan(0); + expect(UNDECLARED.length).toBeGreaterThan(0); + expect(DECLARED.length + UNDECLARED.length).toBe(LOCALIZATION_SELECTS.length); + }); + + // ---- half 1: a declared domain accepts a value outside `options` ---------- + + for (const spec of DECLARED) { + it(`${spec.key} (${spec.valueDomain}) takes a value the curated options do not list`, () => { + const probe = OUTSIDE_THE_CURATED_LIST[spec.valueDomain!]; + // The probe is only evidence if it really is outside the table. + expect(probe).toBeTruthy(); + expect(optionValues(spec)).not.toContain(probe); + + const { container, onChange } = renderField(spec); + + const input = container.querySelector('input'); + expect(input, 'a domain-bearing select must render an editable control').not.toBeNull(); + + // The curated table survives — as SUGGESTIONS, not as the boundary. + const listId = input!.getAttribute('list'); + const datalist = container.querySelector('datalist'); + expect(datalist).not.toBeNull(); + expect(datalist!.id).toBe(listId); + expect( + Array.from(datalist!.querySelectorAll('option')).map((o) => o.getAttribute('value')), + ).toEqual(optionValues(spec)); + + // …and free text outside them is committed verbatim, not swallowed. + fireEvent.change(input!, { target: { value: probe } }); + expect(onChange).toHaveBeenCalledWith(probe); + }); + } + + // ---- half 2: no declaration ⇒ the closed dropdown is untouched ------------ + + for (const spec of UNDECLARED) { + it(`${spec.key} keeps the closed dropdown — its options are exhaustive`, () => { + const { container } = renderField(spec); + + // Nothing free-typable anywhere in the field. This IS the refusal, in DOM + // terms: Radix's Select renders a button trigger and no text entry. + expect(container.querySelector('input, textarea, [contenteditable="true"]')).toBeNull(); + expect(container.querySelector('datalist')).toBeNull(); + + // Still a dropdown, not merely "nothing rendered". + const trigger = screen.getByRole('combobox'); + expect(trigger.tagName).toBe('BUTTON'); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + }); + } + + // ---- the two halves, counted against each other -------------------------- + + it('renders exactly as many editable comboboxes as there are declarations', () => { + const { container } = render( +
+ {LOCALIZATION_SELECTS.map((spec) => ( + {}} /> + ))} +
, + ); + + // No dropdown was widened by accident, and none was left behind. + expect(container.querySelectorAll('input[list]')).toHaveLength(DECLARED.length); + expect(container.querySelectorAll('datalist')).toHaveLength(DECLARED.length); + // …and `input[list]` is the ONLY input the select branch produces, so the + // count above cannot be inflated by some other control. + expect(container.querySelectorAll('input')).toHaveLength(DECLARED.length); + expect( + screen.getAllByRole('combobox').filter((el) => el.tagName === 'BUTTON'), + ).toHaveLength(UNDECLARED.length); + }); + + // ---- the rejection the server sends still lands on the control ------------ + + it('a server rejection marks the combobox itself, not a wrapper', () => { + // The shape `service-settings` sends for an out-of-domain value: 400 + // SETTINGS_VALIDATION → details.fields[] → invalid_value with + // `constraint: { valueDomain: 'iana_time_zone' }`. SettingsView hands the + // message down as `error`; what is pinned here is that the wrapper's + // aria wiring reaches the new control. + const { container } = renderField(DECLARED[0], { + error: "'Mars/Olympus' is not a recognized IANA time zone.", + }); + + const input = container.querySelector('input')!; + expect(input).toHaveAttribute('aria-invalid', 'true'); + expect(input.getAttribute('aria-describedby')).toBe(screen.getByRole('alert').id); + expect(screen.getByRole('alert')).toHaveTextContent('not a recognized IANA time zone'); + }); +}); diff --git a/apps/console/src/pages/settings/types.ts b/apps/console/src/pages/settings/types.ts index e5d6c9aead..3b2213d709 100644 --- a/apps/console/src/pages/settings/types.ts +++ b/apps/console/src/pages/settings/types.ts @@ -5,6 +5,14 @@ * duplicate the minimal subset needed by the renderer. Source of * truth lives in `framework/packages/spec/src/system/settings-manifest.zod.ts` * (ADR-0007). + * + * ⚠️ Hand-written, not generated — so nothing tells this file when the server's + * shape GROWS. The payload keeps carrying the new member; only the renderer + * stops seeing it, and TypeScript reports nothing because a narrower mirror is + * a structurally valid reading of a wider object. That is exactly how + * `valueDomain` below went 18 days unread after the server started sending it + * (objectui#3719). When a settings feature "doesn't render", check this mirror + * against the zod schema first. */ export type SpecifierType = @@ -30,6 +38,19 @@ export type SpecifierType = export type SpecifierScope = 'global' | 'tenant' | 'user'; +/** + * Standard value domains a specifier's value may be judged against + * (objectstack#5933, PR objectstack#6515). Mirrors `SpecifierValueDomainSchema`. + * + * The vocabulary is closed server-side, so it is mirrored closed here. Note the + * renderer branches on PRESENCE, never on the particular member — a domain + * added upstream still gets the right control before this union catches up. + */ +export type SpecifierValueDomain = + | 'iana_time_zone' + | 'iso_4217_currency' + | 'iso_3166_alpha2'; + export interface SpecifierOption { value: string | number | boolean; label: string | { defaultValue?: string; key?: string }; @@ -57,6 +78,14 @@ export interface Specifier { deprecated?: boolean; replacedBy?: string; options?: SpecifierOption[]; + /** + * When declared, the STANDARD's membership is the enforcement boundary and + * `options` degrades to a UI suggestion list — the server accepts any member + * of the domain (objectstack#5712, PR objectstack#6581). When ABSENT, + * `options` is exhaustive (objectstack#5131) and the control must stay a + * closed dropdown. + */ + valueDomain?: SpecifierValueDomain; min?: number; max?: number; step?: number;