From 66b10ee7b57a478f64d642ecc0b1bfae7b887086 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 18:09:08 +0000 Subject: [PATCH 1/4] fix(app-shell): hold a new field validation rule until it has a field and a message A new state_machine / format / json_schema rule was written to the object draft at once, on the object's first listed field and with no message. It now waits in the Validations panel, the hold guarded rules already take, with no field picked for the author. "Add transition" keeps its blank row local until it has a from-state instead of writing the key "". A conditional rule's `when` gets its own caption (en, zh) instead of the script rule's fail-condition one. Claude-Session: https://claude.ai/code/session_01JQwxbaA4X9WGDi4ZgwZ4PZ Co-authored-by: Claude --- .../src/views/metadata-admin/i18n.ts | 13 + ...ationsPanel.newRuleComplete-26731.test.tsx | 267 ++++++++++++++++++ ...lidationsPanel.newRuleWaits-11820.test.tsx | 13 +- ...jectValidationsPanel.runsOn-11923.test.tsx | 9 +- .../ObjectValidationsPanel.test.tsx | 15 +- .../studio-design/ObjectValidationsPanel.tsx | 228 ++++++++++----- .../ObjectValidationsPanel.whenKey.test.tsx | 27 +- 7 files changed, 481 insertions(+), 91 deletions(-) create mode 100644 ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleComplete-26731.test.tsx diff --git a/ui/packages/app-shell/src/views/metadata-admin/i18n.ts b/ui/packages/app-shell/src/views/metadata-admin/i18n.ts index 97f0d3ad3fe..ab7c781b13c 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/ui/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -2063,6 +2063,8 @@ const ENGINE_STRINGS_EN: Record = { 'engine.studio.held.needsInput': '{input} on the step “{step}” needs a value', 'engine.studio.held.needsCondition': 'the rule “{rule}” needs a condition', 'engine.studio.held.needsThenCondition': 'the “Then” rule of “{rule}” needs a condition', + 'engine.studio.held.needsRuleField': 'the rule “{rule}” needs a field', + 'engine.studio.held.needsRuleMessage': 'the rule “{rule}” needs an error message', // objectui#11910 — a dashboard widget held until it is bound; `input` is the // widget inspector's own label for it (Dataset, Values (measures)). 'engine.studio.held.widgetNeedsInput': '{input} on the widget “{widget}” needs a value', @@ -2305,6 +2307,8 @@ const ENGINE_STRINGS_EN: Record = { 'engine.studio.rules.newButton': 'New validation rule', 'engine.studio.rules.noMessage': '(no message)', 'engine.studio.rules.notSaved': 'Not saved — needs a condition', + 'engine.studio.rules.notSavedField': 'Not saved — needs a field', + 'engine.studio.rules.notSavedMessage': 'Not saved — needs a message', 'engine.studio.rules.structured': '“{type}” rules carry structured configuration (state-machine transitions / format constraints etc.) and cannot be edited here yet — maintain them in the code package. Message: {message}', 'engine.studio.rules.none2': '(none)', 'engine.studio.rules.deleteRule': 'Delete rule', @@ -2314,6 +2318,9 @@ const ENGINE_STRINGS_EN: Record = { 'engine.studio.rules.celPre': 'Fail condition (CEL) — when the condition is ', 'engine.studio.rules.celTrue': 'true', 'engine.studio.rules.celMid': ', the save is rejected with the message above.', + // A conditional rule's `when`: it says when the `then` rule applies. + 'engine.studio.rules.whenPre': 'Guard (CEL) — when the condition is ', + 'engine.studio.rules.whenMid': ', the “Then” rule below applies; otherwise the “Otherwise” rule, if one is set.', 'engine.studio.rules.severity': 'Severity', 'engine.studio.rules.severityError': 'error (rejects the save)', 'engine.studio.rules.enabled': 'Enabled', @@ -4685,6 +4692,8 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.studio.held.needsInput': '步骤「{step}」的「{input}」需要填写', 'engine.studio.held.needsCondition': '规则「{rule}」需要一个条件', 'engine.studio.held.needsThenCondition': '规则「{rule}」的「Then」规则需要一个条件', + 'engine.studio.held.needsRuleField': '规则「{rule}」需要选择一个字段', + 'engine.studio.held.needsRuleMessage': '规则「{rule}」需要一条错误消息', 'engine.studio.held.widgetNeedsInput': '组件「{widget}」的「{input}」需要填写', 'engine.studio.held.line': '尚未保存：{clause}。更改已保留在此处，补全后会自动保存。', 'engine.studio.held.publish': '未发布：{clause}，该更改尚未保存。请先补全，再发布。', @@ -4906,6 +4915,8 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.studio.rules.newButton': '新建验证规则', 'engine.studio.rules.noMessage': '(无消息)', 'engine.studio.rules.notSaved': '未保存 —— 需要条件', + 'engine.studio.rules.notSavedField': '未保存 —— 需要字段', + 'engine.studio.rules.notSavedMessage': '未保存 —— 需要消息', 'engine.studio.rules.structured': '「{type}」类型的规则带有结构化配置(状态机转移表 / 格式约束等),暂不支持在此编辑 —— 请在代码包中维护。消息:{message}', 'engine.studio.rules.none2': '(无)', 'engine.studio.rules.deleteRule': '删除规则', @@ -4915,6 +4926,8 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.studio.rules.celPre': '失败条件(CEL)—— 条件为 ', 'engine.studio.rules.celTrue': '真', 'engine.studio.rules.celMid': ' 时,拒绝保存并显示上面的消息。', + 'engine.studio.rules.whenPre': '前提条件(CEL)—— 条件为 ', + 'engine.studio.rules.whenMid': ' 时,应用下面的「Then」规则;否则应用「Otherwise」规则(如已设置)。', 'engine.studio.rules.severity': '严重度', 'engine.studio.rules.severityError': 'error(拒绝保存)', 'engine.studio.rules.enabled': '启用', diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleComplete-26731.test.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleComplete-26731.test.tsx new file mode 100644 index 00000000000..34371770b6a --- /dev/null +++ b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleComplete-26731.test.tsx @@ -0,0 +1,267 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectstack#26731 — a new field rule is held until it is complete, an empty + * transition row is never stored, and a conditional's `when` is captioned as + * what it is. + * + * Choosing State machine, Format or JSON schema in the New menu used to write + * the rule to the object draft at once, on the object's first listed field + * (`created_at` on a served object) and with no message, and the Data pillar's + * autosave stored it. Now the new rule waits in the panel, as a guarded one + * waits for its condition (objectui#11820): no field is picked for the author, + * and the rule is written once, when it has a field and an error message. + * + * "Add transition" used to write the key `""` straight into the rule; the + * blank row now stays in the editor until it has a from-state. + * + * The `conditional` rule's `when` input carried the script rule's caption + * ("Fail condition … the save is rejected"), the opposite of what `when` does: + * it says when the `then` rule applies. + * + * The harness feeds every `onPatch` back into `draft`, as the Data pillar does. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, within } from '@testing-library/react'; +import { I18nProvider, createI18n } from '@object-ui/i18n'; +import { ObjectSchema } from '@objectstack/spec/data'; + +import { ObjectValidationsPanel } from './ObjectValidationsPanel'; +import { t } from '../metadata-admin/i18n'; + +afterEach(() => cleanup()); + +// The served object's audit column first, as the card measured it: the field a +// new rule must not be seeded with. +const baseDraft: Record = { + name: 'invoice', + label: 'Invoice', + fields: { + created_at: { type: 'datetime', label: 'Created At', system: true }, + status: { type: 'select', label: 'Status' }, + payload: { type: 'json', label: 'Payload' }, + }, + validations: [ + { type: 'script', name: 'no_negative', message: 'no', condition: 'record.amount < 0', severity: 'error' }, + ], +}; + +const MESSAGE_BOX = 'e.g. Completion date is required when status is Done'; + +function Harness({ onPatch, initial = baseDraft }: { onPatch: (p: Record) => void; initial?: Record }) { + const [draft, setDraft] = React.useState(initial); + return ( + { + onPatch(p); + setDraft((d) => ({ ...d, ...p })); + }} + /> + ); +} + +function addFromMenu(label: string) { + fireEvent.click(screen.getByText('New')); + fireEvent.click(screen.getByRole('button', { name: 'Advanced' })); + fireEvent.click(screen.getByRole('button', { name: label })); +} + +/** Pick `label` in the open rule's field picker, the shared `Select` (objectui#11865). */ +async function pickField(label: string): Promise { + fireEvent.keyDown(screen.getByTestId('rule-field'), { key: 'ArrowDown' }); + fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: label })); +} + +function lastValidations(onPatch: ReturnType): Array> { + const calls = onPatch.mock.calls; + return calls[calls.length - 1][0].validations as Array>; +} + +const FIELD_TYPES: ReadonlyArray<[type: string, menu: string, field: string]> = [ + ['state_machine', 'State machine — allowed transitions', 'Status (status)'], + ['format', 'Format — regex / built-in format', 'Status (status)'], + ['json_schema', 'JSON schema — validate a JSON field', 'Payload (payload)'], +]; + +describe('a new field rule waits for its field and message (objectstack#26731)', () => { + it.each(FIELD_TYPES)('a new %s rule is held with no field until it has a field and a message, then written once', async (type, menu, fieldLabel) => { + const onPatch = vi.fn(); + render(); + addFromMenu(menu); + + // Held: listed, open, marked, and no field picked for the author. + expect(onPatch).not.toHaveBeenCalled(); + expect(screen.getByText('validation_2')).toBeInTheDocument(); + expect(screen.getByTestId('rule-unsaved')).toHaveTextContent('Not saved — needs a field'); + expect(screen.getByTestId('rule-held')).toHaveTextContent( + 'Not saved yet: the rule “validation_2” needs a field. Your changes are kept here and saved once it is filled in.', + ); + expect(screen.getByTestId('rule-field')).toHaveTextContent('— pick a field —'); + expect(screen.getByTestId('rule-held-hint')).toBeInTheDocument(); + + await pickField(fieldLabel); + // A field alone is not enough: the rule still has nothing to tell the user. + expect(onPatch).not.toHaveBeenCalled(); + expect(screen.getByTestId('rule-unsaved')).toHaveTextContent('Not saved — needs a message'); + expect(screen.getByTestId('rule-held')).toHaveTextContent('the rule “validation_2” needs an error message'); + expect(screen.getAllByTestId('rule-held-hint')).toHaveLength(1); + + fireEvent.change(screen.getByPlaceholderText(MESSAGE_BOX), { target: { value: 'Bad value' } }); + expect(onPatch).toHaveBeenCalledTimes(1); + const written = lastValidations(onPatch); + expect(written).toHaveLength(2); + expect(written[0]).toEqual((baseDraft.validations as unknown[])[0]); + const fieldName = /\((\w+)\)$/.exec(fieldLabel)![1]; + expect(written[1]).toMatchObject({ + type, + name: 'validation_2', + field: fieldName, + message: 'Bad value', + events: ['insert', 'update'], + }); + expect(written[1].field).not.toBe('created_at'); + expect(ObjectSchema.safeParse({ ...baseDraft, validations: written }).success).toBe(true); + + // Written, it is an ordinary rule. + expect(screen.queryByTestId('rule-unsaved')).toBeNull(); + expect(screen.queryByTestId('rule-held')).toBeNull(); + expect(screen.queryByTestId('rule-held-hint')).toBeNull(); + }); + + it('a message typed first waits for the field, and the field writes it', async () => { + const onPatch = vi.fn(); + render(); + addFromMenu('Format — regex / built-in format'); + fireEvent.change(screen.getByPlaceholderText(MESSAGE_BOX), { target: { value: 'Bad code' } }); + expect(onPatch).not.toHaveBeenCalled(); + expect(screen.getByTestId('rule-held')).toHaveTextContent('needs a field'); + + await pickField('Status (status)'); + expect(onPatch).toHaveBeenCalledTimes(1); + expect(lastValidations(onPatch)[1]).toMatchObject({ type: 'format', field: 'status', message: 'Bad code' }); + }); + + it('a blank message does not complete the rule', async () => { + const onPatch = vi.fn(); + render(); + addFromMenu('JSON schema — validate a JSON field'); + await pickField('Payload (payload)'); + fireEvent.change(screen.getByPlaceholderText(MESSAGE_BOX), { target: { value: ' ' } }); + expect(onPatch).not.toHaveBeenCalled(); + expect(screen.getByTestId('rule-held')).toHaveTextContent('needs an error message'); + }); + + it('control — an existing rule switched to a field type keeps the valid first-field skeleton', async () => { + const onPatch = vi.fn(); + render(); + fireEvent.keyDown(screen.getByTestId('rule-type'), { key: 'ArrowDown' }); + fireEvent.click( + within(await screen.findByRole('listbox')).getByRole('option', { name: 'Format — regex / built-in format' }), + ); + expect(onPatch).toHaveBeenCalledTimes(1); + expect(lastValidations(onPatch)[0]).toMatchObject({ type: 'format', name: 'no_negative', field: 'created_at' }); + expect(screen.queryByTestId('rule-unsaved')).toBeNull(); + }); +}); + +describe('an empty transition row is not stored (objectstack#26731)', () => { + const machine = { + ...baseDraft, + validations: [ + { type: 'state_machine', name: 'status_flow', message: 'Bad move', field: 'status', transitions: { open: ['closed'] } }, + ], + }; + + it('“Add transition” shows a blank row and writes nothing until it has a from-state', () => { + const onPatch = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Add transition' })); + expect(onPatch).not.toHaveBeenCalled(); + const froms = screen.getAllByPlaceholderText(t('engine.studio.rules.transitionFrom', 'en-US')); + expect(froms).toHaveLength(2); + expect(froms[1]).toHaveValue(''); + + // Next states typed first stay in the row, unsent. + const tos = screen.getAllByPlaceholderText(t('engine.studio.rules.transitionTo', 'en-US')); + fireEvent.change(tos[1], { target: { value: 'open' } }); + expect(onPatch).not.toHaveBeenCalled(); + + fireEvent.change(froms[1], { target: { value: 'closed' } }); + expect(onPatch).toHaveBeenCalledTimes(1); + const written = lastValidations(onPatch)[0]; + expect(written.transitions).toEqual({ open: ['closed'], closed: ['open'] }); + expect(written.transitions).not.toHaveProperty(''); + // The row it became is the one the author is typing in. + expect(screen.getAllByPlaceholderText(t('engine.studio.rules.transitionFrom', 'en-US'))[1]).toHaveValue('closed'); + }); + + it('deleting the blank row writes nothing', () => { + const onPatch = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Add transition' })); + const deletes = screen.getAllByRole('button', { name: 'Delete' }); + // The row's own delete, not the rule's. + fireEvent.click(deletes.filter((b) => b.getAttribute('data-testid') !== 'rule-delete')[1]); + expect(onPatch).not.toHaveBeenCalled(); + expect(screen.getAllByPlaceholderText(t('engine.studio.rules.transitionFrom', 'en-US'))).toHaveLength(1); + }); +}); + +describe('the conditional rule’s `when` has its own caption (objectstack#26731)', () => { + const both = { + ...baseDraft, + validations: [ + { type: 'script', name: 'no_negative', message: 'no', condition: 'record.amount < 0', severity: 'error' }, + { + type: 'conditional', + name: 'enterprise_only', + message: 'x', + when: "record.status == 'open'", + then: { type: 'script', name: 'enterprise_only_then', message: 'y', condition: 'record.amount > 1' }, + }, + ], + }; + + function captionOf(ruleName: string, lang: 'en' | 'zh'): string { + const panel = {}} />; + if (lang === 'en') render(panel); + else { + const zh = createI18n({ defaultLanguage: 'zh', detectBrowserLanguage: false, resources: { zh: {} } }); + render({panel}); + } + fireEvent.click(screen.getByText(ruleName)); + const group = screen.getAllByRole('group').find((g) => g.hasAttribute('aria-labelledby') && g.tabIndex === -1)!; + const text = document.getElementById(group.getAttribute('aria-labelledby')!)!.textContent ?? ''; + cleanup(); + return text; + } + + it.each([ + ['en', 'en-US'], + ['zh', 'zh-CN'], + ] as const)('in %s the conditional caption is not the script one, and says the Then rule applies', (lang, locale) => { + const script = captionOf('no_negative', lang); + const conditional = captionOf('enterprise_only', lang); + expect(script).toBe( + t('engine.studio.rules.celPre', locale) + t('engine.studio.rules.celTrue', locale) + t('engine.studio.rules.celMid', locale), + ); + expect(conditional).toBe( + t('engine.studio.rules.whenPre', locale) + t('engine.studio.rules.celTrue', locale) + t('engine.studio.rules.whenMid', locale), + ); + expect(conditional).not.toBe(script); + // Real rows, not the key echoed back. + expect(t('engine.studio.rules.whenPre', locale)).not.toBe('engine.studio.rules.whenPre'); + expect(t('engine.studio.rules.whenMid', locale)).not.toBe('engine.studio.rules.whenMid'); + expect(conditional).toContain('Then'); + }); +}); diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx index 7b791d43818..b0cae09be07 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx @@ -23,7 +23,8 @@ * * Controls: an existing rule is edited exactly as before (a type switch still * seeds the valid `'false'` placeholder, since that rule is already in the - * draft), and a type with no guard is written at once. + * draft), and a type with no guard waits for no condition — only for its field + * and message (objectstack#26731, pinned in `newRuleComplete-26731`). * * The harness feeds every `onPatch` back into `draft`, as the Data pillar does, * so what the panel shows after a write is what the pillar would show. @@ -181,10 +182,18 @@ describe('a new validation rule waits for its condition (objectui#11820)', () => expect(screen.queryByText('validation_2')).toBeNull(); }); - it('control — a type with no guard is written at once, on Create + Update', () => { + it('control — a type with no guard waits for no condition: its field and message write it, on Create + Update', async () => { const onPatch = vi.fn(); render(); addFromMenu('Format — regex / built-in format'); + // objectstack#26731 — held until it has a field and a message, never a condition. + expect(onPatch).not.toHaveBeenCalled(); + expect(screen.queryByRole('button', { name: /Expression/ })).toBeNull(); + fireEvent.keyDown(screen.getByTestId('rule-field'), { key: 'ArrowDown' }); + fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: 'Name (name)' })); + fireEvent.change(screen.getByPlaceholderText('e.g. Completion date is required when status is Done'), { + target: { value: 'Bad name' }, + }); expect(onPatch).toHaveBeenCalledTimes(1); const written = lastValidations(onPatch)[1]; expect(written).toMatchObject({ type: 'format', name: 'validation_2', events: ['insert', 'update'] }); diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx index 56d2c0957ca..b646e6cc5cf 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx @@ -218,7 +218,7 @@ describe('the "Runs on" row reads the spec\'s events contract (objectui#11923)', }); }); - it('a new rule starts on the spec\'s default (objectui#11820)', () => { + it('a new rule starts on the spec\'s default (objectui#11820)', async () => { const onPatch = vi.fn(); render(); fireEvent.click(screen.getByText('New')); @@ -226,8 +226,13 @@ describe('the "Runs on" row reads the spec\'s events contract (objectui#11923)', // about where a new rule starts, not about the menu's layout. const advanced = screen.queryByRole('button', { name: 'Advanced' }); if (advanced) fireEvent.click(advanced); - // A type with no guard is written at once. fireEvent.click(screen.getByRole('button', { name: t('engine.studio.rules.typeFormat', EN) })); + // Written once it has a field and a message (objectstack#26731). + fireEvent.keyDown(screen.getByTestId('rule-field'), { key: 'ArrowDown' }); + fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: 'Name (name)' })); + fireEvent.change(screen.getByPlaceholderText(t('engine.studio.rules.messagePlaceholder', EN)), { + target: { value: 'Bad name' }, + }); const rule = lastRule(onPatch); expect(rule.type).toBe('format'); expect(rule.events).toEqual(SPEC_DEFAULT); diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.test.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.test.tsx index ffbc6be98f5..eec0d0709c7 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.test.tsx @@ -54,17 +54,24 @@ describe('ObjectValidationsPanel', () => { expect(screen.getByTestId('rule-unsaved')).toBeTruthy(); }); - it('adds a non-script rule type from the New menu with a valid skeleton', () => { + it('adds a non-script rule type from the New menu with a valid skeleton', async () => { const onPatch = vi.fn(); render(); fireEvent.click(screen.getByText('New')); fireEvent.click(screen.getByRole('button', { name: 'Advanced' })); fireEvent.click(screen.getByRole('button', { name: 'State machine — allowed transitions' })); + // objectstack#26731 — held until the author picks its field and writes its message. + expect(onPatch).not.toHaveBeenCalled(); + fireEvent.keyDown(screen.getByTestId('rule-field'), { key: 'ArrowDown' }); + fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: /\(name\)$/ })); + fireEvent.change(screen.getByPlaceholderText('e.g. Completion date is required when status is Done'), { + target: { value: 'Bad move' }, + }); + expect(onPatch).toHaveBeenCalledTimes(1); const patch = onPatch.mock.calls[0][0]; const added = patch.validations[patch.validations.length - 1]; - // Seeded valid: a required `field` (first field) + an (empty) transitions map. - // A type with no guard has nothing to wait for, so it is written at once. - expect(added).toMatchObject({ type: 'state_machine', name: 'validation_3', field: 'name' }); + // Seeded valid: the picked `field` + an (empty) transitions map. + expect(added).toMatchObject({ type: 'state_machine', name: 'validation_3', field: 'name', message: 'Bad move' }); expect(added.transitions).toEqual({}); // objectui#11820 — and it starts on Create + Update. expect(added.events).toEqual(['insert', 'update']); diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx index 4722e2f4c80..8bfe689bb39 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx @@ -23,11 +23,13 @@ * Adding a rule starts from the "New" menu, which opens on a few common rules in * plain words (objectui#11861, `validationPresets.ts`) and keeps every spec type * under "Advanced" — see "Starting points" below. A new rule starts on - * Create + Update (objectui#11820). A type whose rule carries a CEL guard - * (`script`, `cross_field`, `conditional`) is NOT written to the object draft - * until the author gives it a condition — see "A new rule waits for its - * condition" below. The other types are written at once, each seeded with a - * VALID skeleton so the object-draft save never 422s. The common fields + * Create + Update (objectui#11820). A new rule is NOT written to the object + * draft until the author has filled in what its type needs: a type whose rule + * carries a CEL guard (`script`, `cross_field`, `conditional`) waits for a + * condition, and a type whose rule names one field (`state_machine`, `format`, + * `json_schema`) waits for that field and an error message — see "A new rule + * waits until it is complete" below. Each is seeded with a skeleton the spec + * accepts, so the object-draft save never 422s. The common fields * (name / label / message / severity / events / priority / active) are shared * by all types; the type-specific fields render below them. CEL conditions * reuse the metadata-admin `ConditionBuilder`, fed the DRAFT field list so @@ -37,7 +39,7 @@ * calls `onPatch({ validations })` and the Data pillar's Save draft owns the * write. Nothing here fetches or saves on its own. * - * ## A new rule waits for its condition (objectui#11820) + * ## A new rule waits until it is complete (objectui#11820, objectstack#26731) * * A guard-bearing rule used to be written at once with the placeholder guard * `'false'`: the draft autosave stored a rule that can never fire, and nothing @@ -49,6 +51,13 @@ * first edit that gives it a guard writes it to the draft; from then on it is an * ordinary rule. * + * The field-bearing types (`state_machine`, `format`, `json_schema`) were + * still written at once, seeded with the object's first field and no message, + * so the autosave stored a live rule on a field the author never chose + * (objectstack#26731 saw `created_at`). They now take the same hold: a new one + * starts with no field, and waits for a field and an error message, with the + * hint under whichever input it waits on. See {@link missingPart}. + * * Why the panel and not the pillar's hold: the pillar holds a draft its write * guard would refuse (`objectHeldEdit` in `metadataError.ts`) and routes "Show * me" to a field. A rule that has never been written is not an edit of that @@ -58,8 +67,8 @@ * in the draft, and the line says it is not saved). * * An EXISTING rule is edited exactly as before, including a type switch, which - * still seeds the `'false'` placeholder: a rule already in the draft must stay - * saveable while it is being reshaped. + * still seeds the `'false'` placeholder and the first field: a rule already in + * the draft must stay saveable while it is being reshaped. * * ## Starting points (objectui#11861) * @@ -311,6 +320,23 @@ const BUILTIN_FORMATS = ['', 'url', 'email', 'phone', 'json'] as const; /** The rule types whose rule carries a CEL guard (`guardKey` names its key). */ const GUARDED_TYPES: ReadonlySet = new Set(['script', 'cross_field', 'conditional']); +/** + * The rule types whose rule names one `field` and carries no guard. A new one + * waits for that field and an error message (objectstack#26731). + */ +const FIELD_TYPES: ReadonlySet = new Set(['state_machine', 'format', 'json_schema']); + +/** What a new, not-yet-saved rule still lacks ({@link missingPart}). */ +type MissingPart = 'rule' | 'then' | 'field' | 'message'; + +/** The held line's clause and the rule list's not-saved mark, per {@link MissingPart}. */ +const MISSING_WORDING: Readonly> = { + rule: { clause: 'engine.studio.held.needsCondition', mark: 'engine.studio.rules.notSaved' }, + then: { clause: 'engine.studio.held.needsThenCondition', mark: 'engine.studio.rules.notSaved' }, + field: { clause: 'engine.studio.held.needsRuleField', mark: 'engine.studio.rules.notSavedField' }, + message: { clause: 'engine.studio.held.needsRuleMessage', mark: 'engine.studio.rules.notSavedMessage' }, +}; + function readRules(input: unknown): ValidationRuleDraft[] { if (!Array.isArray(input)) return []; return input.filter((r): r is ValidationRuleDraft => !!r && typeof r === 'object'); @@ -343,20 +369,22 @@ function guardKey(type: unknown): 'when' | 'condition' { } /** - * A minimal skeleton for a rule of `type`; required `field`/`fields` seed from - * the first field. The guard's KEY is per-type (`guardKey`) — `conditional` - * spells it `when`. + * A minimal skeleton for a rule of `type`. The guard's KEY is per-type + * (`guardKey`) — `conditional` spells it `when`. * - * `guard` is what the guard-bearing types are seeded with: + * `guard` says whose skeleton it is, and so what it is seeded with: * * - `'false'` — a VALID skeleton (an empty guard is rejected by the spec's * ExpressionInputSchema), used when an EXISTING rule switches type: it is * already in the draft, so the next save must not 422. `'false'` never - * fires, which the editor's guard caption says. + * fires, which the editor's guard caption says. A required `field` seeds + * from the first field. * - `''` — a NEW rule (objectui#11820): the guard is left for the author, * and the panel keeps the rule out of the draft until it is filled - * ({@link missingGuard}). The conditional's nested `then` gets the same - * empty `condition`, so its JSON shows where the condition goes. + * ({@link missingPart}). The conditional's nested `then` gets the same + * empty `condition`, so its JSON shows where the condition goes. A + * required `field` is left for the author too (objectstack#26731): the + * first field is whatever the object lists first, often a system column. * * "Valid" is a claim about a foreign schema, so it is pinned against that * schema rather than asserted in prose: `whenKeyPin` parses what this panel @@ -368,17 +396,18 @@ function makeSkeleton(type: RuleType, name: string, firstField: string | undefin const base = { name, message: '', severity: 'error' as const, active: true }; // A new rule leaves its guard key out; `missingGuard` reads absent as empty. const own = guard ? { [guardKey(type)]: guard } : {}; + const field = guard ? (firstField ?? '') : ''; switch (type) { case 'script': return { ...base, type, ...own }; case 'cross_field': return { ...base, type, ...own, fields: firstField ? [firstField] : [] }; case 'state_machine': - return { ...base, type, field: firstField ?? '', transitions: {} }; + return { ...base, type, field, transitions: {} }; case 'format': - return { ...base, type, field: firstField ?? '' }; + return { ...base, type, field }; case 'json_schema': - return { ...base, type, field: firstField ?? '', schema: {} }; + return { ...base, type, field, schema: {} }; case 'conditional': return { ...base, @@ -411,6 +440,22 @@ function missingGuard(rule: ValidationRuleDraft): 'rule' | 'then' | null { return null; } +/** + * What a new rule still lacks before the panel writes it, or `null` when it is + * complete for its type: a guarded type's guard ({@link missingGuard}), or a + * field type's `'field'`, then its `'message'` (objectstack#26731) — each + * lacking while blank. The spec accepts both blank; the author has simply not + * said yet which field the rule checks, or what it tells the user. + */ +function missingPart(rule: ValidationRuleDraft): MissingPart | null { + if (typeof rule.type === 'string' && FIELD_TYPES.has(rule.type)) { + if (!(typeof rule.field === 'string' && rule.field.trim())) return 'field'; + if (!(typeof rule.message === 'string' && rule.message.trim())) return 'message'; + return null; + } + return missingGuard(rule); +} + /** A JSON