Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/6900-required-in-gated-section-refusal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

The metadata designer's view inspector now refuses a form view that puts an object-required field
inside a section gated on identity, feature, host or page state (objectui#6900, ruling 5749269225).

When a form section's `visibleWhen` reads `current_user` (or its aliases `user`, `ctx.user`,
`os.user`), `features`, `app` or `page`, and the section holds a field the bound object declares
`required`, the inspector reports one blocking issue per such field through its existing
blocking-issues channel. The editor's Save stays disabled while any is reported, on both the
selected-variant inspector and the no-selection view panel. The inspector shows the refusal: it
names the field and says it is required on the object, names the section and its predicate, and
lists the three fixes (move the field out of the section, drop the predicate, or make the field
not required on the object).

Because the inspector's blocking channel now carries more than CEL faults, the editor's disabled
Save button reads "Fix the issues shown in the inspector before saving." whenever an inspector issue
holds it, a CEL fault included, instead of "Fix the CEL syntax errors before saving." The
permission-matrix and object-hooks Save buttons keep the CEL wording.

The server never evaluates a form section's predicate, so without this the view saves and the form
cannot be completed whenever the section is hidden. A section gated only on `record`, `previous`,
`parent`, `data` or a bare field draws no issue; those predicate shapes stay with the build-time
lint's `requiredWhen` teaching. The live schema check of the draft stays advisory and does not gate
Save. Nothing changes on the server.
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ afterEach(() => {
});

/** The Save button is an icon button identified by its title. */
// Title flipped to the neutral inspector copy by ruling 5831744213 (objectui#6900).
const saveButton = () =>
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the CEL syntax errors before saving\./ });
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the issues shown in the inspector before saving\./ });

/** Open the editor, select the block, and switch its visibility control to raw CEL. */
async function openBlockCel() {
Expand All @@ -138,7 +139,7 @@ describe('MetadataResourceEditPage — Save is gated on the block inspector’s

fireEvent.change(box, { target: { value: 'record.amount >' } });
await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
expect(saveButton()).toHaveAttribute('title', 'Fix the CEL syntax errors before saving.');
expect(saveButton()).toHaveAttribute('title', 'Fix the issues shown in the inspector before saving.');
});

it('re-enables Save once the predicate parses again', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ afterEach(() => {
});

/** The Save icon button, identified by its title in either state. */
// Title flipped to the neutral inspector copy by ruling 5831744213 (objectui#6900).
const saveButton = () =>
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the CEL syntax errors before saving\./ });
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the issues shown in the inspector before saving\./ });

const ruleBox = () =>
screen.getByTestId('cf-rule-0').querySelector('[role="combobox"]') as HTMLTextAreaElement;
Expand All @@ -138,7 +139,7 @@ describe('MetadataResourceEditPage — Save is gated on the DEFAULT inspector’

fireEvent.change(box, { target: { value: 'record.amount >' } });
await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
expect(saveButton()).toHaveAttribute('title', 'Fix the CEL syntax errors before saving.');
expect(saveButton()).toHaveAttribute('title', 'Fix the issues shown in the inspector before saving.');
});

it('re-enables Save once the condition parses again', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Save refuses a form view that puts an OBJECT-required field inside a
* predicate-gated section — on BOTH hosts of the view inspector
* (objectui#6900, landed by ruling 5749269225, letter c).
*
* ## The hazard
*
* The server validates a record against the object's field definitions and
* never reads a form view (ruled objectstack#13252, option C rejected). So a
* form whose section is shown only when `'sales_manager' in
* current_user.positions` and which holds the object-required `salary` saves
* fine as metadata, and then cannot be completed by anyone the predicate hides
* the section from: the record save is refused naming a field they cannot see.
*
* ## Why the refusal rides the inspector's blocking channel
*
* The ruling puts it in the channel ALREADY licensed to gate Save — the
* inspector's blocking issues — and keeps the live Zod pass
* (`validateMetadataDraft`) advisory. `ResourceEditPage.schemaAdvisory.test.tsx`
* (objectui#6980) pins that asymmetry and is untouched by this card.
*
* ## Both hosts
*
* `ResourceEditPage` mounts the view inspector two ways: the DEFAULT inspector
* when nothing is selected (`ViewDefaultInspector`) and the SCOPED one when the
* canvas emits a variant selection (`ViewInspector`). Each is pinned here, so
* the refusal cannot be wired on one of the two paths an author reaches the
* same editor by.
*
* Only the canvas is stubbed (so a preview exists, and so the scoped leg can
* emit its selection). The object catalog arrives through the real
* `useObjectFields` read of `client.get('object', …)`, and the predicate's
* roots are read by the real `@objectstack/formula` parser.
*/

import '@testing-library/jest-dom/vitest';
import * as React from 'react';
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
import { render, screen, fireEvent, cleanup, waitFor, act } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Canonical wire shape — the same spelling as the objectui#6237 fixtures. */
const cel = (source: string) => ({ dialect: 'cel', source });
const IDENTITY_GATE = cel("'sales_manager' in current_user.positions");
const RECORD_GATE = cel("record.status == 'sent'");

const state = vi.hoisted(() => ({
salaryRequired: true,
gate: null as unknown,
}));

function employeeObject(): Record<string, unknown> {
return {
name: 'employee',
label: 'Employee',
fields: {
subject: { type: 'text', label: 'Subject' },
salary: { type: 'number', label: 'Salary', ...(state.salaryRequired ? { required: true } : {}) },
},
};
}

function formView(): Record<string, unknown> {
return {
name: 'employee_form',
label: 'Employee form',
object: 'employee',
viewKind: 'form',
config: {
type: 'simple',
sections: [
{ name: 'basics', label: 'Basics', fields: ['subject'] },
{ name: 'pay', label: 'Compensation', visibleWhen: state.gate, fields: ['salary'] },
],
},
};
}

const mockClient = {
list: vi.fn(async () => []),
listDrafts: vi.fn(async () => []),
layered: vi.fn(async () => {
const view = formView();
return { effective: view, code: view, editable: true };
}),
getDraft: vi.fn(async () => null),
get: vi.fn(async (type: string, name: string) =>
type === 'object' && name === 'employee' ? employeeObject() : null,
),
saveDraft: vi.fn(async () => ({})),
};

vi.mock('./useMetadata', async (importOriginal) => {
const mod = await importOriginal<typeof import('./useMetadata')>();
return {
...mod,
useMetadataClient: () => mockClient,
useMetadataTypes: () => ({
entries: [{ type: 'view', name: 'view', label: 'View', allowOrgOverride: true }],
}),
};
});

import { MetadataResourceEditPage } from './ResourceEditPage';
import { registerBuiltinInspectors } from './inspectors';
import { registerMetadataPreview, getMetadataPreview, type MetadataSelection } from './preview-registry';

registerBuiltinInspectors();

/**
* Canvas stand-in. It exists so the host takes its split-editor branch; its one
* button emits the variant selection the real tab strip emits, which is what
* swaps the DEFAULT inspector for the SCOPED one.
*/
function StubViewCanvas({
onSelectionChange,
}: {
onSelectionChange?: (next: MetadataSelection | null) => void;
}) {
return (
<button type="button" onClick={() => onSelectionChange?.({ kind: 'view', id: 'config' })}>
select the form variant
</button>
);
}

const realViewPreview = getMetadataPreview('view');

beforeEach(() => {
state.salaryRequired = true;
state.gate = IDENTITY_GATE;
registerMetadataPreview('view', StubViewCanvas as never);
});

afterEach(() => {
cleanup();
if (realViewPreview) registerMetadataPreview('view', realViewPreview);
});

/** The Save icon button, identified by its title in either state. */
// Title flipped to the neutral inspector copy by ruling 5831744213 (objectui#6900).
const saveButton = () =>
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the issues shown in the inspector before saving\./ });

/**
* Let the lazily imported CEL parser and the object-catalog read settle, so a
* negative leg's green is observed AFTER the verdict it would have flipped.
*/
async function settle() {
await act(async () => {
await import('@objectstack/formula');
await new Promise((r) => setTimeout(r, 0));
});
}

async function openEditor(host: 'default' | 'scoped') {
render(
<MemoryRouter initialEntries={['/metadata/view/employee_form']}>
<MetadataResourceEditPage type="view" name="employee_form" />
</MemoryRouter>,
);
const select = await screen.findByRole('button', { name: 'select the form variant' });
if (host === 'scoped') fireEvent.click(select);
// Wait for the inspector (either host) to mount its curated Label field —
// found by its placeholder, since the variant's spec form below it carries
// "Label" inputs of its own (one per section).
return screen.findByPlaceholderText('e.g. All Leads');
}

/** Dirty the draft, so Save is live at all and only the gate can hold it. */
function dirty(label: HTMLElement) {
fireEvent.change(label, { target: { value: 'Employee form (edited)' } });
}

describe.each([['default'], ['scoped']] as const)(
'MetadataResourceEditPage — the %s view inspector refuses a required field in a gated section (objectui#6900)',
(host) => {
it('Save stays disabled on an identity-gated section holding an object-required field', async () => {
const label = await openEditor(host);
// Dirty FIRST: an unedited draft keeps Save disabled on its own, so only
// an edited one can show that the refusal is what holds it shut.
dirty(label);
await settle();
await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
// And the refusal the author reads is on screen beside it.
expect(await screen.findByTestId('view-gated-required-issues')).toHaveTextContent('Salary');
});

it('the Save title held by this refusal is the neutral inspector copy, not the CEL one (ruling 5831744213)', async () => {
const label = await openEditor(host);
dirty(label);
await settle();
await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
expect(saveButton()).toHaveAttribute('title', 'Fix the issues shown in the inspector before saving.');
});

it('control: the same form with the field NOT required on the object saves', async () => {
state.salaryRequired = false;
const label = await openEditor(host);
dirty(label);
await settle();
await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 });
expect(screen.queryByTestId('view-gated-required-issues')).toBeNull();
});

it('control: the same section gated on a RECORD predicate saves', async () => {
state.gate = RECORD_GATE;
const label = await openEditor(host);
dirty(label);
await settle();
await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 });
expect(screen.queryByTestId('view-gated-required-issues')).toBeNull();
});
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ afterEach(() => {
});

/** The Save button is an icon button identified by its title. */
// Title flipped to the neutral inspector copy by ruling 5831744213 (objectui#6900).
const saveButton = () =>
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the CEL syntax errors before saving\./ });
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the issues shown in the inspector before saving\./ });

/**
* Dirty the draft with a predicate that parses, then WAIT FOR THE BANNER.
Expand Down Expand Up @@ -224,7 +225,7 @@ describe('MetadataResourceEditPage — client Zod issues are ADVISORY, inspector
// differently on purpose.
fireEvent.change(box, { target: { value: 'record.amount >' } });
await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
expect(saveButton()).toHaveAttribute('title', 'Fix the CEL syntax errors before saving.');
expect(saveButton()).toHaveAttribute('title', 'Fix the issues shown in the inspector before saving.');
expect(screen.getByTestId('metadata-validation-banner')).toHaveTextContent(SERVER_ONLY_KEY);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,10 @@ afterEach(() => {
});

/** The Save button, found by its title in every state this suite reaches. */
// Title flipped to the neutral inspector copy by ruling 5831744213 (objectui#6900).
const saveButton = () =>
screen.getByRole('button', {
name: /Save \(⌘S\)|The server refused this draft|Fix the CEL syntax errors before saving\.|No changes to save/,
name: /Save \(⌘S\)|The server refused this draft|Fix the issues shown in the inspector before saving\.|No changes to save/,
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2200,7 +2200,9 @@ function MetadataResourceEditPageImpl({
saving
? t('engine.edit.saving', locale)
: inspectorBlocking > 0
? t('perm.cel.saveBlocked', locale)
// Neutral: the inspector channel carries CEL faults AND the
// objectui#6900 required-field refusal (ruling 5831744213).
? t('perm.inspector.saveBlocked', locale)
: refusalBlocking > 0
? tFormat('engine.validation.serverRefused', locale, {
issue: saveRefusal?.summary ?? '',
Expand Down
20 changes: 20 additions & 0 deletions packages/app-shell/src/views/metadata-admin/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,14 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
'engine.inspector.view.object': 'Object',
'engine.inspector.view.objectPlaceholder': 'e.g. crm_lead',
'engine.inspector.view.noSchema': 'Spec schema unavailable — basic properties only.',
// objectui#6900 — an object-required field inside a predicate-gated form
// section (ruling 5749269225): name both facts, then the three remedies.
'engine.inspector.view.gatedRequired.title': 'Save is blocked: a required field sits in a conditional section.',
'engine.inspector.view.gatedRequired.issue': 'Field "{fieldLabel}" ({field}) is required on object "{object}", but it sits in section "{section}", which is shown only when {predicate}. The server never evaluates that condition, so whenever the section is hidden the form cannot be completed — the record save is refused over a field the user cannot see.',
'engine.inspector.view.gatedRequired.remedies': 'Fix it one of three ways:',
'engine.inspector.view.gatedRequired.remedyMove': 'Move "{fieldLabel}" into a section that has no visibleWhen.',
'engine.inspector.view.gatedRequired.remedyDrop': 'Remove visibleWhen from section "{section}".',
'engine.inspector.view.gatedRequired.remedyUnrequire': 'Make "{fieldLabel}" not required on object "{object}".',
// Conditional formatting editor (list/grid views)
'engine.inspector.view.cf.title': 'Conditional formatting',
'engine.inspector.view.cf.add': 'Add rule',
Expand Down Expand Up @@ -1286,6 +1294,9 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
'perm.cel.type.unknownHint':
'the engine cannot prove a single type. Wrap operands in double() / int() / string() to pin it — only proven-Number formulas are offered as dataset measures.',
'perm.cel.saveBlocked': 'Fix the CEL syntax errors before saving.',
// The editor's inspector-blocked Save title (objectui#6900, ruling 5831744213):
// that channel carries more than CEL faults, so its copy names no one kind.
'perm.inspector.saveBlocked': 'Fix the issues shown in the inspector before saving.',
'perm.cel.test.title': 'Test policy',
'perm.cel.test.help':
'Dry-run this predicate against a sample record and acting user — the same CEL engine the server uses — to see whether the row is allowed or denied before you save.',
Expand Down Expand Up @@ -2479,6 +2490,13 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
'engine.inspector.view.object': '对象',
'engine.inspector.view.objectPlaceholder': '例如:crm_lead',
'engine.inspector.view.noSchema': 'spec 模式不可用 —— 仅显示基础属性。',
// objectui#6900 — 见英文表同名条目。
'engine.inspector.view.gatedRequired.title': '保存已被拦截:必填字段位于条件区块中。',
'engine.inspector.view.gatedRequired.issue': '字段“{fieldLabel}”({field})在对象“{object}”上是必填的,但它位于区块“{section}”中,而该区块仅在 {predicate} 时显示。服务端从不计算这个条件,因此区块被隐藏时表单无法完成——保存记录会被拒绝,报错指向一个用户看不到的字段。',
'engine.inspector.view.gatedRequired.remedies': '可用以下三种方式之一修复:',
'engine.inspector.view.gatedRequired.remedyMove': '把“{fieldLabel}”移到没有 visibleWhen 的区块中。',
'engine.inspector.view.gatedRequired.remedyDrop': '去掉区块“{section}”上的 visibleWhen。',
'engine.inspector.view.gatedRequired.remedyUnrequire': '在对象“{object}”上把“{fieldLabel}”改为非必填。',
// 条件格式化编辑器(列表/表格视图)
'engine.inspector.view.cf.title': '条件格式化',
'engine.inspector.view.cf.add': '添加规则',
Expand Down Expand Up @@ -3290,6 +3308,8 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
'perm.cel.type.unknownHint':
'引擎无法证明唯一类型。用 double() / int() / string() 包裹操作数以固定类型——只有可证明为数字的公式才能作为数据集度量。',
'perm.cel.saveBlocked': '请先修复 CEL 语法错误再保存。',
// objectui#6900 — 见英文表同名条目。
'perm.inspector.saveBlocked': '请先修复检查器中列出的问题再保存。',
'perm.cel.test.title': '测试策略',
'perm.cel.test.help': '用示例记录与操作用户对该条件进行试运行(与服务端同一 CEL 引擎),在保存前查看该行是允许还是拒绝。',
'perm.cel.test.run': '测试',
Expand Down
Loading
Loading