fix(app-shell): hold a new state_machine / format / json_schema validation rule until it has a field and a message

ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.environmentOverlay-12141.test.tsx

@@ -37,6 +37,11 @@ import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
 import { render, screen, cleanup, waitFor } from '@testing-library/react';
 import { MemoryRouter } from 'react-router-dom';
 import type { RichMetadataTypeEntry } from './useMetadata';
+// The lock banner's zh row lives in `@object-ui/i18n`'s `studio` namespace,
+// whose `zh` catalogue is fetched on demand (objectstack#25922). Resident
+// before any case, so the zh-CN banner reads its zh row on first render.
+import { loadStudioCatalogue } from '@object-ui/i18n/namespaces/studio';
+await loadStudioCatalogue('zh');
 
 const OVERLAY_TYPES = ['view', 'dashboard', 'report', 'translation', 'email_template'] as const;
 

ui/packages/app-shell/src/views/metadata-admin/i18n.ts

@@ -1393,6 +1393,8 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
   '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',
@@ -1635,6 +1637,8 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
   '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',
@@ -1644,6 +1648,9 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
   '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',
@@ -3390,6 +3397,8 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
   '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}，该更改尚未保存。请先补全，再发布。',
@@ -3611,6 +3620,8 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
   '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': '删除规则',
@@ -3620,6 +3631,8 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
   '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': '启用',

ui/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.newRuleComplete-26731.test.tsx

@@ -0,0 +1,268 @@
+/**
+ * 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 { ValidationRuleSchema } 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<string, unknown> = {
+  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<string, unknown>) => void; initial?: Record<string, unknown> }) {
+  const [draft, setDraft] = React.useState(initial);
+  return (
+    <ObjectValidationsPanel
+      draft={draft}
+      onPatch={(p) => {
+        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<void> {
+  fireEvent.keyDown(screen.getByTestId('rule-field'), { key: 'ArrowDown' });
+  fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: label }));
+}
+
+function lastValidations(onPatch: ReturnType<typeof vi.fn>): Array<Record<string, unknown>> {
+  const calls = onPatch.mock.calls;
+  return calls[calls.length - 1][0].validations as Array<Record<string, unknown>>;
+}
+
+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(<Harness onPatch={onPatch} />);
+    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');
+    // The rule the pillar would save parses through the spec's own rule schema.
+    expect(ValidationRuleSchema.safeParse(written[1]).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(<Harness onPatch={onPatch} />);
+    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(<Harness onPatch={onPatch} />);
+    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(<Harness onPatch={onPatch} />);
+    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(<Harness onPatch={onPatch} initial={machine} />);
+    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(Object.keys(written.transitions as object)).not.toContain('');
+    // 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(<Harness onPatch={onPatch} initial={machine} />);
+    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 = <ObjectValidationsPanel draft={both} onPatch={() => {}} />;
+    if (lang === 'en') render(panel);
+    else {
+      const zh = createI18n({ defaultLanguage: 'zh', detectBrowserLanguage: false, resources: { zh: {} } });
+      render(<I18nProvider instance={zh}>{panel}</I18nProvider>);
+    }
+    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');
+  });
+});

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(<Harness onPatch={onPatch} />);
     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'] });