Skip to content

Commit cae71fe

Browse files
fix(plugin-grid): the import wizard spares a required field that carries a default (#28975)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbTqSRkWmphyUSnApdf9ipxp)_ Fixes #28885 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - 改了什么:导入向导的映射步骤不再要求文件提供「必填但带默认值」的字段。字段声明了 `defaultValue`(静态值或表达式),或某个选项标了 `default: true`,未映射时视为已满足,值交给服务端写入时填默认;预览里该列的空单元格也不再标红。必填且无默认值的字段照旧拦住。向导只判断「有没有声明默认值」,从不求值表达式。 - 为什么改:duly 的 `review_status` 必填、带按记录计算的 CEL 默认值,不带该列的样例文件在 18.2 卡在「Can't continue — required field(s) not mapped」,17.2 能导入 19 行。实测服务端导入对缺列与空单元格都会填默认值,向导问的问题与引擎答的不一致。 - 风险与回滚:向导字段类型多了两个可选键(`defaultValue`、选项上的 `default`),控制台两个宿主(对象列表的导入、AI 聊天的表格导入)把字段的 `defaultValue` 原样传入。已知边界:表达式默认值读到的键本行也没给时(如 `record.source` 缺失),服务端逐行以 `required` 拒收,向导不预判。回滚即还原本 PR,无数据或协议变更。 - 先看:`ui/packages/plugin-grid/src/ImportWizard.tsx`(`importFillsDefault` / `importRequiresValue` 及其三处调用)、`ui/packages/app-shell/src/views/importTargetFields.ts`、`ui/packages/app-shell/src/console/ai/ExcelImportBar.tsx`。 - 两分钟验证:`cd ui && pnpm exec vitest run packages/plugin-grid/src/ImportWizard.requiredDefault.test.tsx packages/app-shell/src/views/importTargetFields.test.ts`,预期 2 个文件全绿。 - 需要你做的:无,席位落地。 ## What changes - **`@object-ui/plugin-grid`, `ImportWizard.tsx`:** one presence-only predicate, `importFillsDefault(field)`: a declared `defaultValue` (`null` and absent declare none; an expression counts, whatever it reads), else an option marked `default: true` that carries a value. That is what the engine's insert defaulting reads (`applyFieldDefaults`, then `resolveOptionDefault` in `packages/objectql/src/engine.ts`). `importRequiresValue(field)` is `required` and no default. It gates three places that asked `field.required` before: - the mapping step's missing-required list, so the hint and the held Next skip a defaulted field; - the mapping dropdown's required star; - the preview's blank-cell `required` refusal (`checkImportCell`), so a blank cell in a mapped defaulted column is not marked and its row is counted. - The legacy per-row create path (used only when the server import route is unreachable) leaves a blank cell of a defaulted field out of the record, so the create fills the default instead of storing an empty string. - `ImportWizardProps['fields']` gains an optional `defaultValue` (passed through as the object schema declares it) and its option objects an optional `default`. - **`@object-ui/app-shell`:** the wizard's two hosts pass the field's `defaultValue` through: `importTargetFields` (the object list's Import) and `ExcelImportBar`'s `normalizeFields` (the AI chat's spreadsheet import). Options already passed through raw. - The wizard never evaluates an expression default. ## Why The card's expected behaviour: the mapping step treats a required field that has a `defaultValue` (static or expression) as satisfied when unmapped, and leaves the value to the write path; a required field with no default still blocks. The wizard never saw defaults, because both hosts dropped `defaultValue` when they built its field list, so the fix needed the two producers too (claim amendment 2026-11-02T04:19Z widened the file surface to them). ## Tests - Write path measured first, as the order asked, with a scratch probe (deleted, not committed) on the real `POST /api/v1/data/:object/import` route over a sqlite `:memory:` engine (`packages/rest`, the `import-integration.test.ts` harness), at `origin/main` `9a9823d9`. Object `duty`: `review_status` required select with the reported CEL default `record.source == 'self' ? 'to_review' : 'to_confirm'`, `static_status` required with `defaultValue: 'open'`, `title` required with no default. - Column absent, JSON rows: dry run and commit both `ok`; stored `review_status` `to_review` for `source: self`, `to_confirm` for `source: assigned`; `static_status` `open`. - Column absent, CSV: 2 of 2 created, same stored values. - Blank and whitespace cells in a mapped `review_status` / `static_status` column: dry run and commit both `ok`, defaults stored. - Control: `title` missing is refused, `code: required`, `Title is required`. - Edge: a row that also omits `source` is refused as `required` on `review_status` (the expression reads a key the row does not carry), on the dry run and the commit alike. - New `ImportWizard.requiredDefault.test.tsx` (6 cases): an unmapped required field with an expression default does not hold Next; nor a static default, nor an option marked default; a required field with no default is still named, and only it; a `null` defaultValue declares none; in the preview a blank cell in a defaulted required column is unmarked and its row counted (`Import 2 Rows`), while a blank cell with no default is still marked (`Import 1 Row`). - `importTargetFields.test.ts`: a new case that a declared `defaultValue` (an expression envelope and a static value) passes through as declared, and none is set when it is absent or `null`. - Ablations through `scripts/ablation-replace.mjs`, each restored to the HEAD blob (`git diff HEAD` empty): - missing-required list back to `f.required`: 3 of 6 red (the three mapping-step cases). - blank-cell arm back to `field.required`: 1 of 6 red (the defaulted blank cell). - On the merged head `745445a4`: `turbo run type-check --filter=@object-ui/plugin-grid` 50/50, and every plugin-grid import suite plus the two `ExcelImportBar` suites and `importTargetFields.test.ts`: 35 files, 307 tests passed. - On `5e39a211` (before the merge of main, which touched none of these files): `turbo run type-check --filter=@object-ui/app-shell` 65/65; `plugin-form/src/affordanceGrantMap-12082.test.tsx` green with the app-shell suites (4 files, 80 tests). ## Acceptance notes - Known limit, named on the seat's instruction: an expression default that reads a key the row also omits (here `record.source` absent) is refused per row by the server as `required`. The wizard does not predict it; the dry run reports it per row. - Not measured in a browser console; the jsdom suites drive the real wizard component. ## Verification <details><summary>Derived gates, local subset (34 of 34, exit 0, reconciled)</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 families at `5e39a211`; each ran with exit 0, among them `check-sdui-manifest.mjs` and its self-test, `check-changeset-no-major`, `check-empty-changeset`, `check-adr-0087-registration`, `check:type-check-debt` (under the verify lock), `check:query-options-erasure`, and `pnpm --dir ui exec vitest run scripts/` (169 files passed, 2 skipped). `dispatch-gates --ran`: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. After the merge of main: `pnpm --filter @objectstack/spec build && check:generated` exit 0. </details> Families CI runs (Lint and Repo Gates, Lint UI, Test UI, Type Check UI and the rest) are read from this PR's check-runs. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (Not touched.) - [x] 2. Every name I added uses the casing for its kind. - [ ] 3. No formatter ran; `pnpm lint` passes. (No formatter ran; `pnpm lint` is CI's Lint UI run.) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [ ] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jTdCHU9bPL21tsQXgm --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTdCHU9bPL21tsQXgm)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2ac698b commit cae71fe

6 files changed

Lines changed: 197 additions & 5 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@object-ui/plugin-grid': patch
3+
'@object-ui/app-shell': patch
4+
---
5+
6+
fix(plugin-grid): the import wizard no longer demands a required field that carries a default
7+
8+
Clause-②: yes (widening)
9+
Metadata-protocol: none
10+
11+
The mapping step held Next with "required field(s) not mapped" for a required field left unmapped even when the field declares a `defaultValue`, static or an expression, which the create fills for a row that leaves the field out. A file without that column could not be imported. The step now treats such a field as satisfied, and leaves the value to the server. The same check spares the preview's blank cell in a mapped column of such a field, which the server also fills with the default. An option marked `default: true` counts as a default when no `defaultValue` is declared, as the create reads it. A required field with no default still blocks, and its blank cell is still marked. The wizard reads only whether a default is declared; it never evaluates one.
12+
13+
`ImportWizardProps['fields']` gains an optional `defaultValue` (passed through as the object schema declares it), and its option objects an optional `default`. The Console's two hosts of the wizard, the object list's Import and the AI chat's spreadsheet import, now pass the field's `defaultValue` through.
14+
15+
What to do: a host that builds the wizard's `fields` itself passes each field's `defaultValue` through to get the same behaviour; without it a defaulted required field is still demanded.

‎ui/packages/app-shell/src/console/ai/ExcelImportBar.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ interface WizardField {
4141
label: string;
4242
type: string;
4343
required?: boolean;
44+
defaultValue?: unknown;
4445
options?: Array<{ label?: string; value?: string | number } | string>;
4546
multiple?: boolean;
4647
}
@@ -71,6 +72,8 @@ function normalizeFields(schema: any): WizardField[] {
7172
label: (def?.label ?? def?.name ?? name) as string,
7273
type: (def?.type ?? 'text') as string,
7374
required: !!def?.required,
75+
// The wizard reads only that a default is declared (the create fills it).
76+
...(def?.defaultValue != null ? { defaultValue: def.defaultValue } : {}),
7477
options: def?.options,
7578
// A field holding a list is never offered as a match key (#27567).
7679
...(def?.multiple === true ? { multiple: true } : {}),

‎ui/packages/app-shell/src/views/importTargetFields.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ describe('importTargetFields', () => {
3737
expect(byName.code.required).toBe(false);
3838
});
3939

40+
it('passes a declared defaultValue through as declared, and none when absent or null', () => {
41+
const expression = { dialect: 'cel', source: "record.source == 'self' ? 'to_review' : 'to_confirm'" };
42+
const out = importTargetFields('duty', {
43+
review_status: { type: 'select', label: 'Review status', required: true, defaultValue: expression },
44+
priority: { type: 'select', label: 'Priority', required: true, defaultValue: 'normal' },
45+
owner_name: { type: 'text', label: 'Owner', required: true, defaultValue: null },
46+
title: { type: 'text', label: 'Title', required: true },
47+
});
48+
const byName = Object.fromEntries(out.map((f) => [f.name, f]));
49+
// The expression envelope is carried as is, never evaluated or flattened.
50+
expect(byName.review_status.defaultValue).toBe(expression);
51+
expect(byName.priority.defaultValue).toBe('normal');
52+
expect('defaultValue' in byName.owner_name).toBe(false);
53+
expect('defaultValue' in byName.title).toBe(false);
54+
// `required` still states the schema; the wizard decides what a default spares.
55+
expect(byName.review_status.required).toBe(true);
56+
});
57+
4058
it('still drops computed fields — no storage column to match or write', () => {
4159
const out = importTargetFields('device', fields);
4260
const names = out.map((f) => f.name);

‎ui/packages/app-shell/src/views/importTargetFields.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export interface ImportTargetField {
3030
label: string;
3131
type: string;
3232
required?: boolean;
33+
defaultValue?: unknown;
3334
matchOnly?: boolean;
3435
options?: unknown;
3536
multiple?: boolean;
@@ -65,6 +66,10 @@ export function importTargetFields(
6566
// A non-writable target can never be a required WRITE column —
6667
// `required` only steers the mapping step's completeness hint.
6768
required: writable && !!def?.required,
69+
// Passed through as declared: the wizard reads only that a default is
70+
// there (the create fills it), so a defaulted required field is not
71+
// demanded from the file. Never evaluated client-side.
72+
...(def?.defaultValue != null ? { defaultValue: def.defaultValue } : {}),
6873
...(matchOnly ? { matchOnly: true } : {}),
6974
// Enum options seed the downloadable template's example row.
7075
...(def?.options ? { options: def.options } : {}),
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* ObjectUI – Copyright (c) 2024-present ObjectStack Inc.
3+
* Licensed under MIT.
4+
*/
5+
6+
/**
7+
* A required field that carries a default is not demanded from the file.
8+
*
9+
* The create fills a declared `defaultValue` (static or an expression) and,
10+
* when none is declared, an option marked `default: true`, for a row that
11+
* leaves the field out or blank. So the mapping step does not hold Next for
12+
* such a field left unmapped, and the preview does not mark its blank cell.
13+
* A required field with no default still blocks. The wizard only asks whether
14+
* a default is declared: the expression below is never evaluated here.
15+
*/
16+
import { describe, it, expect } from 'vitest';
17+
import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
18+
import '@testing-library/jest-dom';
19+
import React from 'react';
20+
21+
import { ImportWizard } from './ImportWizard';
22+
23+
type WizardField = React.ComponentProps<typeof ImportWizard>['fields'][number];
24+
25+
const REVIEW_OPTIONS = [
26+
{ label: 'To review', value: 'to_review' },
27+
{ label: 'To confirm', value: 'to_confirm' },
28+
];
29+
30+
/** A record-derived expression default, the shape the reported app declares. */
31+
const EXPRESSION_DEFAULT = { dialect: 'cel', source: "record.source == 'self' ? 'to_review' : 'to_confirm'" };
32+
33+
const title: WizardField = { name: 'title', label: 'Title', type: 'text' };
34+
const owner: WizardField = { name: 'owner_name', label: 'Owner', type: 'text', required: true };
35+
const reviewStatus: WizardField = {
36+
name: 'review_status', label: 'Review status', type: 'select', required: true,
37+
defaultValue: EXPRESSION_DEFAULT, options: REVIEW_OPTIONS,
38+
};
39+
const priority: WizardField = {
40+
name: 'priority', label: 'Priority', type: 'select', required: true, defaultValue: 'normal',
41+
options: [{ label: 'Normal', value: 'normal' }, { label: 'High', value: 'high' }],
42+
};
43+
const stage: WizardField = {
44+
name: 'stage', label: 'Stage', type: 'select', required: true,
45+
options: [{ label: 'New', value: 'new', default: true }, { label: 'Done', value: 'done' }],
46+
};
47+
48+
/** Paste TSV into the upload step via the wizard's window-level handler. */
49+
function pasteRows(text: string) {
50+
const evt = new Event('paste', { bubbles: true, cancelable: true }) as Event & {
51+
clipboardData: { getData: (type: string) => string };
52+
};
53+
evt.clipboardData = { getData: (type: string) => (type === 'text/plain' ? text : '') };
54+
act(() => { window.dispatchEvent(evt); });
55+
}
56+
57+
function renderWizard(fields: WizardField[]) {
58+
render(<ImportWizard objectName="duty" fields={fields} dataSource={{}} open onOpenChange={() => {}} />);
59+
}
60+
61+
describe('ImportWizard mapping step: a required field with a default', () => {
62+
it('does not hold Next for an unmapped required field with an expression default', async () => {
63+
renderWizard([title, reviewStatus]);
64+
pasteRows('Title\nAlpha');
65+
const next = await screen.findByTestId('import-next-btn');
66+
await waitFor(() => expect(next).toBeEnabled());
67+
expect(screen.queryByTestId('import-missing-required')).not.toBeInTheDocument();
68+
});
69+
70+
it('does not hold Next for a static default or an option marked default', async () => {
71+
renderWizard([title, priority, stage]);
72+
pasteRows('Title\nAlpha');
73+
const next = await screen.findByTestId('import-next-btn');
74+
await waitFor(() => expect(next).toBeEnabled());
75+
expect(screen.queryByTestId('import-missing-required')).not.toBeInTheDocument();
76+
});
77+
78+
it('still names a required field with no default, and only that one', async () => {
79+
renderWizard([title, owner, reviewStatus, priority, stage]);
80+
pasteRows('Title\nAlpha');
81+
const hint = await screen.findByTestId('import-missing-required');
82+
expect(hint).toHaveTextContent('Owner (owner_name)');
83+
expect(hint).not.toHaveTextContent('review_status');
84+
expect(hint).not.toHaveTextContent('priority');
85+
expect(hint).not.toHaveTextContent('stage');
86+
expect(screen.getByTestId('import-next-btn')).toBeDisabled();
87+
});
88+
89+
it('a null defaultValue declares no default', async () => {
90+
renderWizard([title, { ...owner, defaultValue: null }]);
91+
pasteRows('Title\nAlpha');
92+
expect(await screen.findByTestId('import-missing-required')).toHaveTextContent('Owner (owner_name)');
93+
});
94+
});
95+
96+
describe('ImportWizard preview: a blank cell in a required field with a default', () => {
97+
const cell = (row: number, col: number) => screen.getByTestId(`import-preview-cell-${row}-${col}`);
98+
99+
async function openPreview(fields: WizardField[], tsv: string) {
100+
renderWizard(fields);
101+
pasteRows(tsv);
102+
const next = await screen.findByTestId('import-next-btn');
103+
await waitFor(() => expect(next).toBeEnabled());
104+
fireEvent.click(next);
105+
return screen.findByTestId('import-run-btn');
106+
}
107+
108+
it('leaves the blank cell of a defaulted required column unmarked and counts its row', async () => {
109+
const button = await openPreview([title, reviewStatus], 'Title\tReview status\nAlpha\t\nBeta\tTo confirm');
110+
expect(cell(0, 1)).toHaveAttribute('aria-invalid', 'false');
111+
expect(button).toHaveTextContent('Import 2 Rows');
112+
});
113+
114+
it('still marks the blank cell of a required column with no default', async () => {
115+
const button = await openPreview([title, owner], 'Title\tOwner\nAlpha\t\nBeta\tAda');
116+
expect(cell(0, 1)).toHaveAttribute('aria-invalid', 'true');
117+
expect(button).toHaveTextContent('Import 1 Row');
118+
});
119+
});

‎ui/packages/plugin-grid/src/ImportWizard.tsx‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,15 @@ export interface ImportWizardProps {
360360
label: string;
361361
type: string;
362362
required?: boolean;
363+
/** The object schema's `defaultValue`, static or an expression, passed
364+
* through as declared. The wizard reads only whether one is declared
365+
* ({@link importFillsDefault}) and never evaluates it: the create fills
366+
* it, so a required field carrying one need not come from the file. */
367+
defaultValue?: unknown;
363368
/** Allowed values for select/enum fields. Accepts option objects or bare
364-
* strings. */
365-
options?: Array<{ label?: string; value?: string | number } | string>;
369+
* strings. An option marked `default: true` is the field's default when it
370+
* declares no `defaultValue`, as the create reads it. */
371+
options?: Array<{ label?: string; value?: string | number; default?: boolean } | string>;
366372
/** The object schema's `multiple` flag: a `select` / `radio` / `lookup` /
367373
* `user` / `file` / `image` field flagged `multiple: true` stores a list.
368374
* "Match on" reads it, as the server does, to leave such a field out
@@ -622,6 +628,29 @@ function isMatchKeyField(field: Pick<ImportTargetField, 'type' | 'multiple'>): b
622628
return !isMultiValueField({ type: field.type, multiple: field.multiple === true });
623629
}
624630

631+
/**
632+
* Whether the create fills this field when a row leaves it out or blank. It
633+
* asks what the engine's insert defaulting asks: a declared `defaultValue`
634+
* (`null` and absent declare none; an expression counts, whatever it reads),
635+
* else an option marked `default: true` that carries a value. Presence only:
636+
* the value, and an expression's result, are the server's.
637+
*/
638+
function importFillsDefault(field: Pick<ImportTargetField, 'defaultValue' | 'options'>): boolean {
639+
if (field.defaultValue != null) return true;
640+
return (field.options ?? []).some(
641+
(o) => typeof o === 'object' && o !== null && o.default === true && o.value != null,
642+
);
643+
}
644+
645+
/**
646+
* Whether a create needs this field's value from the file: it is required and
647+
* the create has no default to fill it with. The mapping step's missing-field
648+
* check and the preview's blank-cell check both ask this.
649+
*/
650+
function importRequiresValue(field: ImportTargetField): boolean {
651+
return field.required === true && !importFillsDefault(field);
652+
}
653+
625654
/** The only comma a numeric cell may carry: a thousands grouping of its integer part. */
626655
const THOUSANDS_GROUPED_INTEGER = /^[+-]?\d{1,3}(?:,\d{3})+(?![\d,])[^,]*$/;
627656

@@ -759,7 +788,7 @@ function checkImportCell(
759788
opts: CellCheckOptions,
760789
): CellRefusal | undefined {
761790
const value = cell.trim();
762-
if (value === '') return field.required && opts.requireValues ? { code: 'required' } : undefined;
791+
if (value === '') return importRequiresValue(field) && opts.requireValues ? { code: 'required' } : undefined;
763792
const { type } = field;
764793
if (BOOLEAN_VALUE_TYPES.has(type)) {
765794
return BOOLEAN_IMPORT_TOKENS.has(value.toLowerCase()) ? undefined : { code: 'invalid_boolean', value };
@@ -883,6 +912,9 @@ function validateRow(row: string[], mappedCols: MappedCol[], rowIndex: number, k
883912
errors.push({ row: rowIndex, field: col.field.name, message: `Invalid ${col.field.type} value: "${refusal.value}"` });
884913
continue;
885914
}
915+
// A blank cell of a defaulted field is left out, so the create fills the
916+
// default, as the server import does; an empty string would be stored.
917+
if (raw.trim() === '' && importFillsDefault(col.field)) continue;
886918
record[col.field.name] = raw;
887919
}
888920
return { record, errors };
@@ -1741,7 +1773,7 @@ const StepMapping: React.FC<{
17411773
<SelectItem value="__skip__">{t('grid.import.skip')}</SelectItem>
17421774
{fields.map((f) => (
17431775
<SelectItem key={f.name} value={f.name} disabled={usedFields.has(f.name) && mapping[idx] !== f.name}>
1744-
{f.label}{f.required ? ' *' : ''}{f.matchOnly ? ` ${t('grid.import.matchOnlyField')}` : ''}
1776+
{f.label}{importRequiresValue(f) ? ' *' : ''}{f.matchOnly ? ` ${t('grid.import.matchOnlyField')}` : ''}
17451777
</SelectItem>
17461778
))}
17471779
</SelectContent>
@@ -2486,7 +2518,7 @@ export const ImportWizard: React.FC<ImportWizardProps> = ({
24862518

24872519
const missingRequired = useMemo(() => {
24882520
const mapped = new Set(Object.values(mapping));
2489-
return fields.filter((f) => f.required && !mapped.has(f.name));
2521+
return fields.filter((f) => importRequiresValue(f) && !mapped.has(f.name));
24902522
}, [fields, mapping]);
24912523
// objectui#12303 — the "Can't continue" hint's id, so the Next it disables
24922524
// is described by it. A saved mapping lets the server own the columns, so

0 commit comments

Comments
 (0)