From fcd2b30496efd3400e7830a2813e3285ada14dd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 08:55:51 +0000 Subject: [PATCH 1/2] fix(app-shell): Studio's validation "Runs on" row reads the spec's events contract (objectui#11923) The row offered Create, Update and Delete from a local list, and showed a rule with no `events` key as running on nothing. It now offers the spec's events (read off `ScriptValidationSchema.shape.events`), shows an absent key as the spec's default, and a tick writes the full resulting list. The Delete label row, en and zh, goes with the box: nothing else reads it. Claude-Session: https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU Co-authored-by: Claude --- .../src/views/metadata-admin/i18n.ts | 2 - .../ObjectValidationsPanel.celGate.test.tsx | 4 +- ...lidationsPanel.newRuleWaits-11820.test.tsx | 3 +- ...jectValidationsPanel.runsOn-11923.test.tsx | 236 ++++++++++++++++++ .../studio-design/ObjectValidationsPanel.tsx | 86 +++++-- 5 files changed, 306 insertions(+), 25 deletions(-) create mode 100644 packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index 20bb309133..f5ab478918 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -3086,7 +3086,6 @@ const ENGINE_STRINGS_EN: Record = { 'engine.studio.rules.events': 'Runs on', 'engine.studio.rules.event.insert': 'Create', 'engine.studio.rules.event.update': 'Update', - 'engine.studio.rules.event.delete': 'Delete', 'engine.studio.rules.priority': 'Priority', // objectui#11861 — the New menu's starting points, and the type list under Advanced. 'engine.studio.rules.presets': 'Common rules', @@ -6231,7 +6230,6 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.studio.rules.events': '触发时机', 'engine.studio.rules.event.insert': '新建', 'engine.studio.rules.event.update': '更新', - 'engine.studio.rules.event.delete': '删除', 'engine.studio.rules.priority': '优先级', 'engine.studio.rules.presets': '常用规则', 'engine.studio.rules.advanced': '高级', diff --git a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.celGate.test.tsx b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.celGate.test.tsx index b88d51397a..0a8eb142cf 100644 --- a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.celGate.test.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.celGate.test.tsx @@ -153,8 +153,8 @@ describe('ObjectValidationsPanel — blocking CEL issues reach the Data pillar ( fireEvent.change(rawEditor(), { target: { value: 'record.status ==' } }); await waitFor(() => expect(current()).toBe(1), { timeout: 3000 }); - // `getByText('Delete')` is ambiguous — a lifecycle-event checkbox carries - // the same word; the rule's own delete affordance is the BUTTON. + // The rule's own delete affordance is the BUTTON: queried by role, so no + // other text on the page that says Delete can be clicked in its place. fireEvent.click(screen.getByRole('button', { name: /Delete/ })); await waitFor(() => expect(current()).toBe(0), { timeout: 3000 }); }); diff --git a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx index 419b955374..ddaf09dfda 100644 --- a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleWaits-11820.test.tsx @@ -108,7 +108,8 @@ describe('a new validation rule waits for its condition (objectui#11820)', () => addFromMenu('Script — CEL fail condition'); expect(screen.getByRole('checkbox', { name: 'Create' })).toBeChecked(); expect(screen.getByRole('checkbox', { name: 'Update' })).toBeChecked(); - expect(screen.getByRole('checkbox', { name: 'Delete' })).not.toBeChecked(); + // objectui#11923 — the row offers only the spec's events: no Delete box. + expect(screen.queryByRole('checkbox', { name: 'Delete' })).toBeNull(); }); it('edits before the condition stay unsent and ride along when it is written', () => { diff --git a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx new file mode 100644 index 0000000000..56d2c0957c --- /dev/null +++ b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.runsOn-11923.test.tsx @@ -0,0 +1,236 @@ +/** + * 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. + */ + +/** + * objectui#11923 — the "Runs on" row reads the spec's `events` contract. + * + * The row offered Create, Update and Delete from a local list, and showed a + * rule with no `events` key as running on nothing. The spec's `events` (in + * `BASE_VALIDATION_SHAPE`) admits only `insert` and `update` and defaults an + * absent key to both, and the server runs such a rule on both. Pinned here: + * + * - the row offers exactly the spec's events, read off the spec here too, and + * every rule type's schema agrees on them; + * - an absent key shows as the spec's default, both boxes checked; + * - a tick or an untick writes the full resulting list, and what the row + * writes parses through the spec's own `ObjectSchema`, the parse the draft + * save applies; + * - a stored `delete` (written before this fix) does not crash the panel, is + * kept by an unrelated edit, and is left out by the row's next write, which + * then parses; + * - a new rule starts on the spec's default (objectui#11820). + * + * The harness feeds every `onPatch` back into `draft`, as the Data pillar does, + * so what the row shows after a write is what the pillar would show. + */ + +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 { + ObjectSchema, + ValidationRuleSchema, + ScriptValidationSchema, + CrossFieldValidationSchema, + StateMachineValidationSchema, + FormatValidationSchema, + JSONValidationSchema, + ConditionalValidationSchema, +} from '@objectstack/spec/data'; + +import { ObjectValidationsPanel } from './ObjectValidationsPanel'; +import { t } from '../metadata-admin/i18n'; + +afterEach(() => cleanup()); + +const EN = 'en-US'; + +/** Every rule type's schema: each spreads `BASE_VALIDATION_SHAPE`, `events` included. */ +const RULE_SCHEMAS = { + script: ScriptValidationSchema, + cross_field: CrossFieldValidationSchema, + state_machine: StateMachineValidationSchema, + format: FormatValidationSchema, + json_schema: JSONValidationSchema, + conditional: ConditionalValidationSchema, +}; + +/** The spec's events, as the row must offer them, in the spec's order. */ +const SPEC_EVENTS: readonly string[] = ScriptValidationSchema.shape.events.unwrap().element.options; + +const RULE = { type: 'script', name: 'no_negative', message: 'no', condition: 'record.amount < 0', severity: 'error' }; + +/** What the spec makes of a rule that names no events. */ +const SPEC_DEFAULT: readonly string[] = ValidationRuleSchema.parse(RULE).events as string[]; + +const label = (ev: string) => t(`engine.studio.rules.event.${ev}`, EN); + +function draftWith(rule: Record): Record { + return { + name: 'invoice', + label: 'Invoice', + fields: { + name: { type: 'text', label: 'Name' }, + amount: { type: 'number', label: 'Amount' }, + }, + validations: [rule], + }; +} + +function Harness({ onPatch, initial }: { onPatch: (p: Record) => void; initial: Record }) { + const [draft, setDraft] = React.useState(initial); + return ( + { + onPatch(p); + setDraft((d) => ({ ...d, ...p })); + }} + /> + ); +} + +/** The checkboxes of the "Runs on" row, and nothing else on the page. */ +function runsOnBoxes(): HTMLInputElement[] { + const row = screen.getByText('Runs on').parentElement as HTMLElement; + return within(row).getAllByRole('checkbox') as HTMLInputElement[]; +} + +function box(ev: string): HTMLInputElement { + return within(screen.getByText('Runs on').parentElement as HTMLElement).getByRole('checkbox', { + name: label(ev), + }) as HTMLInputElement; +} + +function lastRule(onPatch: ReturnType): Record { + const calls = onPatch.mock.calls; + const validations = calls[calls.length - 1][0].validations as Array>; + return validations[validations.length - 1]; +} + +/** The draft the pillar would save, with `rule` written, parses through the spec's `ObjectSchema`. */ +function expectSaveParses(rule: Record) { + const parsed = ObjectSchema.safeParse(draftWith(rule)); + expect(parsed.success, JSON.stringify(parsed.success ? null : parsed.error.issues)).toBe(true); + return parsed; +} + +function renderRule(rule: Record) { + const onPatch = vi.fn(); + render(); + return onPatch; +} + +describe('the "Runs on" row reads the spec\'s events contract (objectui#11923)', () => { + it('every rule type carries the same events contract, so reading it off one type is reading it off all', () => { + for (const [type, schema] of Object.entries(RULE_SCHEMAS)) { + const events = schema.shape.events; + expect(events.unwrap().element.options, type).toEqual(SPEC_EVENTS); + expect(events.parse(undefined), type).toEqual(SPEC_DEFAULT); + } + }); + + it('offers exactly the spec\'s events, and no Delete', () => { + renderRule({ ...RULE, events: ['insert'] }); + expect(runsOnBoxes().map((b) => b.closest('label')?.textContent)).toEqual(SPEC_EVENTS.map(label)); + expect(screen.queryByRole('checkbox', { name: 'Delete' })).toBeNull(); + }); + + it('shows a rule with no events key as the spec\'s default: both boxes checked', () => { + renderRule(RULE); + expect(SPEC_DEFAULT).toEqual(SPEC_EVENTS); + for (const ev of SPEC_EVENTS) expect(box(ev)).toBeChecked(); + expect(screen.getByRole('checkbox', { name: 'Create' })).toBeChecked(); + expect(screen.getByRole('checkbox', { name: 'Update' })).toBeChecked(); + }); + + it.each([ + ['insert', ['update']], + ['update', ['insert']], + ])('one untick of %s from the absent key writes the other event alone, and it parses', (unticked, written) => { + const onPatch = renderRule(RULE); + fireEvent.click(box(unticked)); + expect(onPatch).toHaveBeenCalledTimes(1); + const rule = lastRule(onPatch); + expect(rule.events).toEqual(written); + const parsed = expectSaveParses(rule); + expect(parsed.success && parsed.data.validations?.[0]?.events).toEqual(written); + // The row now shows what was written, not the default. + expect(box(unticked)).not.toBeChecked(); + expect(box(written[0])).toBeChecked(); + }); + + it('a tick writes the full resulting list, in the spec\'s order', () => { + const onPatch = renderRule({ ...RULE, events: ['update'] }); + expect(box('insert')).not.toBeChecked(); + fireEvent.click(box('insert')); + const rule = lastRule(onPatch); + expect(rule.events).toEqual(SPEC_EVENTS); + expectSaveParses(rule); + }); + + it('unticking the last box writes [], which the spec accepts, and shows no box checked', () => { + const onPatch = renderRule({ ...RULE, events: ['insert'] }); + fireEvent.click(box('insert')); + const rule = lastRule(onPatch); + expect(rule.events).toEqual([]); + const parsed = expectSaveParses(rule); + // `[]` is kept as written, not defaulted: the rule runs on nothing. + expect(parsed.success && parsed.data.validations?.[0]?.events).toEqual([]); + for (const ev of SPEC_EVENTS) expect(box(ev)).not.toBeChecked(); + }); + + describe('a stored delete, written before this fix', () => { + const STORED = { ...RULE, events: ['insert', 'delete'] }; + + it('control: the spec refuses the stored rule as it is', () => { + expect(ObjectSchema.safeParse(draftWith(STORED)).success).toBe(false); + }); + + it('renders: the boxes show the events the server runs it on, and there is no Delete box', () => { + renderRule(STORED); + expect(box('insert')).toBeChecked(); + expect(box('update')).not.toBeChecked(); + expect(runsOnBoxes()).toHaveLength(SPEC_EVENTS.length); + expect(screen.queryByRole('checkbox', { name: 'Delete' })).toBeNull(); + }); + + it('an unrelated edit keeps it', () => { + const onPatch = renderRule(STORED); + fireEvent.change(screen.getByDisplayValue('no'), { target: { value: 'No negatives' } }); + const rule = lastRule(onPatch); + expect(rule.message).toBe('No negatives'); + expect(rule.events).toEqual(['insert', 'delete']); + }); + + it('the row\'s next write leaves it out, so that write parses', () => { + const onPatch = renderRule(STORED); + fireEvent.click(box('update')); + const rule = lastRule(onPatch); + expect(rule.events).toEqual(['insert', 'update']); + expectSaveParses(rule); + }); + }); + + it('a new rule starts on the spec\'s default (objectui#11820)', () => { + const onPatch = vi.fn(); + render(); + fireEvent.click(screen.getByText('New')); + // The per-type list sits under Advanced since objectui#11861; this pin is + // 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) })); + const rule = lastRule(onPatch); + expect(rule.type).toBe('format'); + expect(rule.events).toEqual(SPEC_DEFAULT); + for (const ev of SPEC_EVENTS) expect(box(ev)).toBeChecked(); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx index 380c6a21d6..0462e644e0 100644 --- a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx @@ -81,6 +81,7 @@ import { expressionSource, writeExpressionSource } from '../metadata-admin/inspe import { readFields } from '../metadata-admin/previews/object-fields-io.js'; import { t, tFormat, useMetadataLocale } from '../metadata-admin/i18n.js'; import type { ExpressionInput } from '@objectstack/spec/shared'; +import { ScriptValidationSchema, type ScriptValidationParsed } from '@objectstack/spec/data'; import { VALIDATION_PRESETS, type PresetPlan, type ValidationPreset } from './validationPresets.js'; /** @@ -153,7 +154,59 @@ const RULE_TYPES: ReadonlyArray<{ value: RuleType; labelKey: string }> = [ { value: 'conditional', labelKey: 'engine.studio.rules.typeConditional' }, ]; -const EVENTS = ['insert', 'update', 'delete'] as const; +/** An event a validation rule may run on: the spec's enum, not a local list. */ +type RuleEvent = ScriptValidationParsed['events'][number]; + +/** + * objectui#11923 — the "Runs on" row reads the spec's `events` contract + * (`events` in `BASE_VALIDATION_SHAPE`, which every rule type spreads). A local + * list here offered `delete`, which the spec refuses (the server's rule + * validator runs only on insert and update), and showed a rule with no `events` + * key as running on nothing, while the spec defaults it to both and the server + * runs it on both. + * + * Both halves come off the spec's own schema: the enum's options are the boxes + * the row offers, and what the schema makes of an absent key is what the row + * shows for one. Read on first use rather than at import, as the spec builds + * its schemas lazily. `ScriptValidationSchema` is read because every rule type + * spreads the same key; `ObjectValidationsPanel.runsOn-11923.test.tsx` pins + * that every type's schema agrees. + */ +let runsOnSpec: { offered: readonly RuleEvent[]; absent: readonly RuleEvent[] } | undefined; +function runsOnContract(): { offered: readonly RuleEvent[]; absent: readonly RuleEvent[] } { + if (!runsOnSpec) { + const events = ScriptValidationSchema.shape.events; + runsOnSpec = { offered: events.unwrap().element.options, absent: events.parse(undefined) }; + } + return runsOnSpec; +} + +/** + * The events a rule runs on, as the "Runs on" boxes show them: its own list, or + * the spec's default when it names none. The one place the row reads `events`, + * so no box falls back on its own. + * + * A stored value the spec does not offer (a `delete` written before + * objectui#11923) has no box. The server never ran a rule on it, so the boxes + * still show what the rule runs on; an unrelated edit keeps it, and the next + * write from this row leaves it out (see `writeRunsOn`). + */ +function ruleRunsOn(rule: ValidationRuleDraft): readonly string[] { + if (rule.events === undefined) return runsOnContract().absent; + return Array.isArray(rule.events) ? rule.events : []; +} + +/** + * The full list a tick or an untick on the "Runs on" row writes: every offered + * event whose box is checked afterwards, in the spec's order. Unticking the last + * box writes `[]`, which the spec accepts and the server reads as running on + * nothing; it is not the absent key, so the row then shows no box checked. + */ +function writeRunsOn(rule: ValidationRuleDraft, event: RuleEvent, on: boolean): RuleEvent[] { + const current = ruleRunsOn(rule); + return runsOnContract().offered.filter((ev) => (ev === event ? on : current.includes(ev))); +} + /** * The events a NEW rule starts on: Create + Update (objectui#11820). They are * also the spec's own default for a rule that names none (`events` in @@ -1054,28 +1107,21 @@ export function ObjectValidationsPanel({ guardRef={guardRef} /> - {/* runs-on events */} + {/* runs-on events: the spec's events, an absent key shown as its default (objectui#11923) */}
{t('engine.studio.rules.events', locale)}
- {EVENTS.map((ev) => { - const on = Array.isArray(sel.events) ? sel.events.includes(ev) : false; - return ( - - ); - })} + {runsOnContract().offered.map((ev) => ( + + ))}
From 0b71e26e0b1e866fedd5c2e2d347c3ae4d88e99d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 08:56:08 +0000 Subject: [PATCH 2/2] chore(changeset): app-shell patch for the validation "Runs on" row (objectui#11923) Claude-Session: https://claude.ai/code/session_01MgfduSkFrfM3eorB3UGfAU Co-authored-by: Claude --- .changeset/11923-validation-runs-on.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/11923-validation-runs-on.md diff --git a/.changeset/11923-validation-runs-on.md b/.changeset/11923-validation-runs-on.md new file mode 100644 index 0000000000..401818f6ae --- /dev/null +++ b/.changeset/11923-validation-runs-on.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +--- + +Studio's validation-rule "Runs on" boxes follow the spec's events (objectui#11923). + +In an object's Validations view, the "Runs on" row offered Create, Update and Delete. The spec admits only `insert` and `update` for a rule's `events`, so ticking Delete wrote a rule the object's save refused. The row now offers exactly the events the spec declares, read from `@objectstack/spec` rather than from a list kept in Studio, so it is Create and Update today. + +A rule with no `events` key (authored in code or by AI) opened with no box checked, though the spec defaults it to Create and Update and the server runs it on both. Ticking Create then wrote `['insert']` and stopped the rule running on updates. Such a rule now shows both boxes checked, and each tick or untick writes the whole resulting list: unticking Create writes `['update']`. Unticking the last box writes an empty list, which the spec accepts and the server reads as running on nothing. + +A rule that still carries `delete` from before this fix opens without a Delete box; its boxes show the events the server runs it on. Editing its message or another setting keeps the stored list as it is. The next tick or untick on the row writes only the spec's events, so that save goes through. + +Nothing is added to the package entry: no export, prop, type member or language-pack key. The Delete label row (en and zh) is removed from the metadata-admin designer's own string table, as nothing reads it any more.