diff --git a/.changeset/chilled-eels-shave.md b/.changeset/chilled-eels-shave.md new file mode 100644 index 0000000000..e98a251484 --- /dev/null +++ b/.changeset/chilled-eels-shave.md @@ -0,0 +1,16 @@ +--- +'@objectstack/objectql': patch +--- + +A `state_machine` rule's refusal now carries `constraint` and `value` alongside an **author-written** `message`, not only alongside the built-in one (#14311). + +`checkStateMachine` emitted the full field-error envelope — `field`, `code`, `message`, `label`, `constraint`, `value` — when the rule left its message empty, but dropped `constraint` and `value` the moment the rule declared one. Since `ValidationRuleSchema` **requires** `message` on every rule, the machine-readable half was in practice reachable only by declaring `message: ''`: every normally-authored state machine refused writes with no way for a client to learn *which* states are legal. + +A create form that wants to offer exactly the declared `initialStates`, or a detail page that wants to grey out illegal transitions, had to parse the author's prose or keep a second copy of the state machine. + +Now both paths emit the same envelope: + +- insert — `constraint: { allowed: 'planned' }`, `value: 'active'`, `code: 'invalid_initial_state'` +- update — `constraint: { from: 'draft', to: 'approved' }`, `code: 'invalid_transition'` + +The author still owns the wording; only the facts beside it are restored. Nothing about which writes are refused changes, and `constraint` / `value` are already declared on `FieldValidationError` (mirroring `FieldErrorSchema`), so no consumer contract widens — REST ships the same `400 VALIDATION_FAILED` envelope with the fields it always declared. diff --git a/examples/app-showcase/src/data/objects/project.object.ts b/examples/app-showcase/src/data/objects/project.object.ts index bc42b96936..0abe2631fd 100644 --- a/examples/app-showcase/src/data/objects/project.object.ts +++ b/examples/app-showcase/src/data/objects/project.object.ts @@ -135,7 +135,17 @@ export const Project = ObjectSchema.create({ // `insert` in `events` is what makes the initialStates check run on create. events: ['insert', 'update'] as const, initialStates: ['planned'], - message: 'Invalid project status transition.', + // ONE authored sentence answers BOTH refusals this rule can raise — + // `invalid_initial_state` on insert and `invalid_transition` on update — + // because `authoredRuleMessage` resolves one key per RULE, not per code. + // The old wording ("Invalid project status transition.") described only + // the update half, so a create rejected for being born `active` was told + // about a "transition" it had not attempted. It is translated at + // `objects.showcase_project._validations.project_status_flow.message` + // (#14253) — an authored message is emitted verbatim unless the bundle + // carries that key, which is why this one used to be the single English + // sentence on an otherwise zh-CN form. + message: 'Projects start as Planned, and then move only along the declared status flow.', transitions: { planned: ['active', 'cancelled'], active: ['on_hold', 'completed', 'cancelled'], diff --git a/examples/app-showcase/src/system/translations/index.ts b/examples/app-showcase/src/system/translations/index.ts index d924add710..86af31d1eb 100644 --- a/examples/app-showcase/src/system/translations/index.ts +++ b/examples/app-showcase/src/system/translations/index.ts @@ -34,6 +34,33 @@ export const ShowcaseTranslationBundle = { start_date: { label: 'Start Date' }, end_date: { label: 'End Date' }, }, + // An author-written `validations[].message` is emitted VERBATIM unless + // the bundle carries it here (#14253) — the built-in field catalog's + // own sentences have shipped zh-CN since #3957, so a rule that declares + // its own message is the one way a refusal escapes the caller's + // language. `project_status_flow` is the showcase's state machine and + // the only refusal a visitor reliably triggers (the New Project wizard + // used to offer four statuses the machine will not accept on create), + // so it read as the single English sentence on a zh-CN form. + // All FOUR of the object's rules, not just the state machines: the New + // Project wizard can trip `end_after_start` and `spent_within_budget` + // from its budget/schedule step, so translating only the status rule + // would move the single English sentence one step later rather than + // remove it. + _validations: { + project_status_flow: { + message: 'Projects start as Planned, and then move only along the declared status flow.', + }, + project_health_progression: { + message: 'Health changed by more than one step — confirm this is intentional.', + }, + end_after_start: { + message: 'Target End Date must be on or after the Start Date.', + }, + spent_within_budget: { + message: 'Spend exceeds 120% of budget — escalate before continuing.', + }, + }, }, showcase_task: { label: 'Task', @@ -270,6 +297,16 @@ export const ShowcaseTranslationBundle = { start_date: { label: '开始日期' }, end_date: { label: '结束日期' }, }, + // The zh-CN mirror of the `en` `_validations` block — see the note + // there. Without these two keys the write path's own refusals arrive in + // Chinese (built-in catalog, #3957) while these author-written ones + // arrive in English, inside one error envelope. + _validations: { + project_status_flow: { message: '项目的初始状态为“计划中”,此后只能按既定的状态流转变更。' }, + project_health_progression: { message: '健康度一次变更超过一级,请确认这是有意为之。' }, + end_after_start: { message: '结束日期不能早于开始日期。' }, + spent_within_budget: { message: '已花费超过预算的 120%,请先上报后再继续。' }, + }, // `default` — the container's DEFAULT list. `defineView({ list })` // declares it without a `name`, and the composer therefore registers it // as `.default`; `_views` keys are that bare runtime key diff --git a/examples/app-showcase/src/ui/pages/new-project-wizard.page.ts b/examples/app-showcase/src/ui/pages/new-project-wizard.page.ts index 2b9152c93e..68fee9b7b4 100644 --- a/examples/app-showcase/src/ui/pages/new-project-wizard.page.ts +++ b/examples/app-showcase/src/ui/pages/new-project-wizard.page.ts @@ -6,8 +6,10 @@ import { definePage } from '@objectstack/spec/ui'; * New Project Wizard — a multi-step (wizard) form surface. The showcase * defines wizard/tabbed/split form view *types* but had no page that actually * walks a user through a stepped create flow. This renders `object-form` with - * `formType: 'wizard'` directly: Basics → Status → Budget, with a step + * `formType: 'wizard'` directly: Basics → Health → Budget, with a step * indicator, over showcase_project. + * + * On `status`, and why it is not a step here, see the comment on `sections`. */ export const NewProjectWizardPage = definePage({ name: 'showcase_new_project_wizard', @@ -29,10 +31,35 @@ export const NewProjectWizardPage = definePage({ formType: 'wizard', showStepIndicator: true, title: 'Create a Project', - description: 'A three-step wizard — basics, status, then budget & schedule.', + description: 'A three-step wizard — basics, health, then budget & schedule.', + // `status` is deliberately ABSENT from this create wizard. + // + // `showcase_project`'s `project_status_flow` state machine declares + // `initialStates: ['planned']`, so `planned` is the only status a + // project may be CREATED in — the other four are reachable only by + // transition, after the record exists. The step offered all five + // (a `select` renders its whole option list; nothing in page + // metadata narrows it to the machine's entry points), so four of + // them were dead ends: the wizard accepted the pick, walked the + // author through a third step, and only then answered + // `400 VALIDATION_FAILED` from the create. A wizard demonstrating a + // state machine must not demo a dead end. + // + // With the field omitted, the option marked `default: true` + // (`planned`) supplies the value server-side — which is the same + // entry point the machine declares, so the two cannot drift. A + // one-option select would be the alternative and is strictly worse + // UI: it asks a question with exactly one answer. + // + // The GENERAL fix — a create form deriving its allowed values from + // the object's `stateMachine` — is a console (objectui) feature and + // is deliberately not built here; this app must be correct without + // it. `test/new-project-wizard-initial-status.test.ts` pins the + // invariant against the REAL metadata, so widening `initialStates` + // later re-opens the question instead of silently rotting. sections: [ { label: 'Basics', description: 'Name the project and bind its account.', fields: ['name', 'account', 'owner'] }, - { label: 'Status & Health', description: 'Where does it stand today?', fields: ['status', 'health'] }, + { label: 'Health', description: 'New projects start as Planned — how healthy is it today?', fields: ['health'] }, { label: 'Budget & Schedule', description: 'Money and dates.', fields: ['budget', 'spent', 'start_date', 'end_date'] }, ], // Without this, a successful submit left the filled step-3 form in diff --git a/examples/app-showcase/test/new-project-wizard-initial-status.test.ts b/examples/app-showcase/test/new-project-wizard-initial-status.test.ts new file mode 100644 index 0000000000..d6901a1c0a --- /dev/null +++ b/examples/app-showcase/test/new-project-wizard-initial-status.test.ts @@ -0,0 +1,203 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14311] The New Project wizard may not offer a status the state machine + * refuses on create. + * + * The wizard's second step listed `status`, and a `select` renders its whole + * option list — all five project statuses. `project_status_flow` declares + * `initialStates: ['planned']`, so four of those five were dead ends: the + * wizard accepted the pick, walked the author through a third step, and only + * then answered `400 VALIDATION_FAILED` from the create. A demo of + * "state machine + wizard" that demos a dead end teaches the wrong thing. + * + * These tests read the REAL page and the REAL object rather than a copy of + * either, so the invariant is checked against what the app actually ships: + * widening `initialStates`, re-adding the field, or adding a status option + * re-opens the question here instead of rotting silently. + * + * The last test is the end-to-end half, on the production harness (real + * `ObjectQL`, real `SqlDriver`, the app's REAL object): the create the wizard + * now performs succeeds, the one it used to allow is refused, and the refusal + * carries the field location and the legal initial states a form needs to act + * on it. Asserting only "it throws" would pass against a rejection for any + * other reason — including the `required` check, which is what a naive "just + * drop the field" fix would have tripped. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; + +import { Account, Project } from '../src/data/objects/index.js'; +import { NewProjectWizardPage } from '../src/ui/pages/new-project-wizard.page.js'; +import { ShowcaseTranslationBundle } from '../src/system/translations/index.js'; + +type Rule = { + type?: string; + name?: string; + field?: string; + initialStates?: string[]; + message?: string; +}; + +const APP_ID = 'com.objectstack.showcase'; +const PACKAGE_ID = `app:${APP_ID}`; +const ctx = { context: { userId: 'u_showcase', isSystem: true } }; + +const openEngines: ObjectQL[] = []; +afterEach(async () => { + while (openEngines.length) { + try { await openEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +/** + * The showcase's real objects on a real engine — same wiring as + * `hook-body-persisted-writes.test.ts`. `showcase_project.account` is a + * REQUIRED lookup, so `Account` is registered too and a real row is created: + * a rejection for a dangling reference would otherwise be indistinguishable + * from the state-machine refusal this test is about. + */ +async function bootShowcase(): Promise { + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.connect(); + + const engine = new ObjectQL(); + openEngines.push(engine); + engine.registerDriver(driver as never, true); + await engine.init(); + for (const def of [Account, Project]) { + engine.registry.registerObject(def as never, PACKAGE_ID, 'showcase'); + } + await engine.syncSchemas(); + return engine; +} + +/** The `project_status_flow` state machine, read off the real object. */ +const statusRule = ((Project as unknown as { validations?: Rule[] }).validations ?? []).find( + (r) => r?.type === 'state_machine' && r?.field === 'status', +)!; + +/** Every field the wizard's create form exposes, across all of its steps. */ +function wizardFields(): string[] { + const regions = (NewProjectWizardPage as unknown as { + regions?: Array<{ components?: Array<{ type?: string; properties?: Record }> }>; + }).regions ?? []; + const out: string[] = []; + for (const region of regions) { + for (const component of region.components ?? []) { + if (component?.type !== 'object-form') continue; + const sections = (component.properties?.sections ?? []) as Array<{ fields?: string[] }>; + for (const section of sections) out.push(...(section.fields ?? [])); + } + } + return out; +} + +/** The declared option values of a select field on the real object. */ +function optionValues(field: string): string[] { + const def = (Project as unknown as { + fields?: Record }>; + }).fields?.[field]; + return (def?.options ?? []).map((o) => (typeof o === 'object' && o !== null ? String(o.value) : String(o))); +} + +describe('#14311 — the New Project wizard and the status state machine', () => { + it('the premise: the object still constrains which status a project may be created in', () => { + // If this ever stops holding, the rest of this file is asserting nothing. + expect(statusRule?.name).toBe('project_status_flow'); + expect(statusRule?.initialStates).toEqual(['planned']); + expect((statusRule as { events?: string[] }).events).toContain('insert'); + }); + + it('the wizard does not offer a status the machine refuses on create', () => { + const offered = wizardFields(); + const initial = statusRule.initialStates ?? []; + const refusable = optionValues('status').filter((v) => !initial.includes(v)); + + // More than one legal initial state would make a narrowed select the right + // shape; with exactly one, the field must simply not be asked. + expect(refusable.length).toBeGreaterThan(0); + expect(initial).toHaveLength(1); + expect(offered).not.toContain('status'); + }); + + it('the value the wizard relies on is the machine entry point (a default, not a copy of it)', () => { + // Omitting the field only works because the object DEFAULTS it, and only + // stays correct because the default IS the declared initial state. + const def = (Project as unknown as { + fields?: Record }>; + }).fields?.status; + const defaulted = (def?.options ?? []).filter((o) => o?.default).map((o) => String(o.value)); + expect(defaulted).toEqual(statusRule.initialStates); + }); + + it('EVERY rule on the object is on the translation channel in both shipped locales', () => { + // An authored `validations[].message` is emitted VERBATIM unless the bundle + // carries `objects.._validations..message` (#14253). Scoped to the + // whole object rather than to the status rule on purpose: this one wizard + // can also trip `end_after_start` and `spent_within_budget` from its + // budget/schedule step, so pinning only the status rule would let the single + // English sentence move one step later instead of disappearing. + const rules = ((Project as unknown as { validations?: Rule[] }).validations ?? []) + .filter((r) => typeof r?.name === 'string'); + expect(rules.length).toBeGreaterThan(1); + + for (const rule of rules) { + const name = rule.name!; + for (const locale of ['en', 'zh-CN'] as const) { + const entry = (ShowcaseTranslationBundle as any)[locale] + ?.objects?.showcase_project?._validations?.[name]; + expect(entry?.message, `${locale} is missing a message for ${name}`).toBeTruthy(); + } + // The zh-CN entry must actually BE Chinese — an English copy satisfies + // "a key exists" while reproducing the defect exactly. + const zh = (ShowcaseTranslationBundle as any)['zh-CN'] + .objects.showcase_project._validations[name].message as string; + expect(zh, `${name}'s zh-CN message is not Chinese`).toMatch(/[一-龥]/); + expect(zh, `${name}'s zh-CN message is a copy of the authored one`).not.toBe(rule.message); + } + }); + + it('creates with the wizard payload and refuses the status it used to offer', async () => { + const engine = await bootShowcase(); + const account: any = await engine.insert( + 'showcase_account', { name: 'Northwind' }, ctx as never, + ); + + // What the wizard now sends: no `status` at all. + const created: any = await engine.insert( + 'showcase_project', + { name: 'Wizard smoke', account: String(account.id), health: 'green' }, + ctx as never, + ); + expect(created.status).toBe('planned'); + + // What it used to let an author send from step 2. + let thrown: any; + try { + await engine.insert( + 'showcase_project', + { name: 'Born active', account: String(account.id), status: 'active' }, + ctx as never, + ); + } catch (e) { thrown = e; } + + expect(thrown, 'expected the create to be refused').toBeDefined(); + // ADR-0112 envelope — REST maps this to `400 VALIDATION_FAILED` verbatim. + expect(thrown.code).toBe('VALIDATION_FAILED'); + const field = thrown.fields?.find((f: any) => f.field === 'status'); + // Field-located, so a multi-step form can jump to the step that owns it. + expect(field, 'the refusal must name the field it is about').toBeDefined(); + expect(field.code).toBe('invalid_initial_state'); + // #14311 — the facts ride along with the AUTHORED message, so a form can + // name the legal entry points without parsing the sentence. + expect(field.constraint).toEqual({ allowed: 'planned' }); + expect(field.value).toBe('active'); + }, 30000); +}); diff --git a/packages/objectql/src/validation/rule-validator.test.ts b/packages/objectql/src/validation/rule-validator.test.ts index 0eee35f134..57ba55257b 100644 --- a/packages/objectql/src/validation/rule-validator.test.ts +++ b/packages/objectql/src/validation/rule-validator.test.ts @@ -1591,6 +1591,81 @@ describe('state_machine initialStates enforcement on INSERT (#3165)', () => { ).not.toThrow(); }); + /** + * #14311 — an authored `message` costs the caller the WORDING, not the rest + * of the envelope. + * + * `constraint` and `value` are declared on `FieldValidationError` and are the + * half a client ACTS on: which states a record may be created in, and which + * one it tried. They were emitted only when the rule left its message empty — + * and `ValidationRuleSchema` REQUIRES `message` on every rule, so in practice + * the machine-readable half was reachable only by declaring `message: ''`. + * A create form that wants to offer exactly the legal initial states had to + * parse the author's prose, or hardcode a copy of the state machine. + * + * Both halves are asserted on the SAME rejection, because the defect was + * never "no error" — it was an error carrying less than it declares. + */ + it('carries constraint + value alongside an AUTHORED message (insert)', () => { + const [err] = (() => { + try { + evaluateValidationRules(approvalSchema, { approval_status: 'approved' }, 'insert'); + throw new Error('expected a ValidationError'); + } catch (e) { + return (e as ValidationError).fields; + } + })(); + // Envelope (ADR-0112): the refusal is a VALIDATION_FAILED naming the field. + expect(err.field).toBe('approval_status'); + expect(err.code).toBe('invalid_initial_state'); + // The author still owns the sentence. + expect(err.message).toBe('A request must start as draft.'); + // …and the facts ride along, exactly as they do without an authored message. + expect(err.constraint).toEqual({ allowed: 'draft' }); + expect(err.value).toBe('approved'); + }); + + it('carries constraint alongside an AUTHORED message (update transition)', () => { + const [err] = (() => { + try { + evaluateValidationRules( + approvalSchema, + { approval_status: 'approved' }, + 'update', + { previous: { approval_status: 'draft' } }, + ); + throw new Error('expected a ValidationError'); + } catch (e) { + return (e as ValidationError).fields; + } + })(); + expect(err.field).toBe('approval_status'); + expect(err.code).toBe('invalid_transition'); + expect(err.message).toBe('A request must start as draft.'); + expect(err.constraint).toEqual({ from: 'draft', to: 'approved' }); + }); + + /** + * The control for the pair above: the built-in path already carried both, and + * must keep doing so. If this ever diverges from the authored path again, the + * two assertions disagree instead of one silently shrinking. + */ + it('the built-in path (empty message) carries the same constraint + value', () => { + const catalogSchema = { + validations: [{ ...approvalSchema.validations[0], message: '' }], + }; + const [err] = (() => { + try { + evaluateValidationRules(catalogSchema, { approval_status: 'approved' }, 'insert'); + throw new Error('expected a ValidationError'); + } catch (e) { + return (e as ValidationError).fields; + } + })(); + expect(err.constraint).toEqual({ allowed: 'draft' }); + expect(err.value).toBe('approved'); + }); + it('does not affect UPDATE transitions (initialStates is insert-only)', () => { // draft → pending is a declared transition; initialStates must not interfere. expect(() => diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index a75f6c74d6..48fca3d9b6 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -2098,6 +2098,18 @@ function checkStateMachine( code, message: authoredRuleMessage(rule, ctx?.messages), label: resolveFieldLabel(rule.field, def, ctx?.messages), + // An authored message replaces the built-in WORDING — not the + // machine-readable facts beside it. `constraint` and `value` are + // declared on `FieldValidationError` (mirroring `FieldErrorSchema`) and + // are what a client ACTS on rather than displays: the legal + // `initialStates` a create form must offer, or the `from → to` pair a + // detail page greys out. Emitting them only on the built-in path meant + // that declaring a message — which the spec REQUIRES on every rule — + // silently cost the caller the rest of the envelope, so the richer + // envelope was reachable only by leaving the sentence empty. Nothing + // about the refusal changes; it gains the location it already declares. + ...(Object.keys(constraint).length > 0 ? { constraint } : {}), + ...(value !== undefined ? { value } : {}), }; } return buildFieldError({ field: rule.field, code, def, constraint, value }, ctx?.messages);