Skip to content

Commit 4d1f52c

Browse files
fix(plugin-grid): "Match on" offers only fields the server takes as a match key (#27567) (#27645)
domain:ui#2 · os-bill · session_01Utp6CY2n1JmZZKrHgtp9jC <!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgP6XkfpjCfyzzG2KwQPUZfy)_ Fixes #27567 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - 改了什么:导入向导「Match on」只列出服务端接受的匹配字段:多值查找、多选、标签、复选组、JSON/地址等结构化字段不再出现;向导字段形状新增可选 `multiple`,由控制台的两处字段构建(列表导入、AI Excel 导入)从对象定义带过来。 - 为什么改:服务端已拒绝把这类字段当匹配键(#27465 / PR #27562,`400 INVALID_REQUEST`),但向导照样让用户勾选,要到「Validate」才被整单拒绝。 - 风险与回滚:只动前端:一个过滤条件、一个可选 prop 键、两处字段构建各一行;不改 spec、不改服务端、不加元数据键。回滚即 revert 本 PR。 - 先看:`ui/packages/plugin-grid/src/ImportWizard.tsx`(`isMatchKeyField` 与 `matchable` 过滤)→ `ui/packages/app-shell/src/views/importTargetFields.ts`(带 `multiple`)→ 新测试 `ImportWizard.matchKeyFields-27567.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/plugin-grid/src/ImportWizard.matchKeyFields-27567.test.tsx`,预期 1/1 通过(只剩 code、email、owner、stage 四个可选)。 - 需要你做的:无,席位落地。 ## What changes In "Update existing" and "Update if matched, else create", the import wizard's "Match on" listed every mapped field. Since #27465 the server refuses a match key whose value is a list or a structured payload, so ticking such a field got the whole import refused at Validate. "Match on" now offers only the mapped fields the server takes as a key. - **One rule, read from the spec.** `isMatchKeyField` in `ImportWizard.tsx` is `!STRUCTURED_JSON_TYPES.has(type) && !isMultiValueField({ type, multiple })`, the same two `@objectstack/spec/data` definitions the server's `multiValueMatchKeyRefusal` (`@objectstack/core`) composes. `plugin-grid` already imported from `@objectstack/spec/data`, so no dependency edge and no `package.json` change. `matchable` is the existing list with that filter added: no new widget, dialog or hook. - **`multiple` on the field shape.** `ImportWizardProps['fields'][number]` takes an optional `multiple?: boolean` (the declared widening: `ImportWizardProps` is re-exported by name from the package entry). The wizard does not build its field shape; its hosts do, so the flag is carried at both: - `importTargetFields` (`app-shell/src/views/importTargetFields.ts`, the console list's Import), only when the schema says `multiple: true`; - `normalizeFields` in `ExcelImportBar.tsx` (the AI build panel's spreadsheet import, which offers all three write modes), the same way. Cleared for this PR by the seat that holds that path. `identityImportFields` (text/email only) is unchanged. - **Fields left out.** A `multiple: true` `lookup`, `user`, `select`, `radio`, `file` or `image`; any `multiselect`, `checkboxes` or `tags`; any `json`, `composite`, `repeater`, `record`, `location`, `address` or `vector`. A single-value lookup or select and every scalar field stay offered. A host that leaves `multiple` out still has the inherently multi and structured types filtered by type alone. - **Docblock.** `refusedOptionValue`'s note said the field shape does not carry `multiple`; it now says the cell check does not read it (a host may leave it out), so its behaviour is unchanged. ## Tests - New `ui/packages/plugin-grid/src/ImportWizard.matchKeyFields-27567.test.tsx`: on an upsert import with ten mapped fields, "Match on" lists exactly `code` (text), `email`, `owner` (single lookup), `stage` (single select); `owners` (lookup, `multiple: true`), `stages` (select, `multiple: true`), `labels` (tags), `regions` (multiselect), `meta` (json), `site` (address) are absent. - `importTargetFields.test.ts`: a new case pins that `multiple: true` is carried and `multiple: false` / absent is not. ## Acceptance notes - The card's three acceptance lines: the field shape carries `multiple` from the object schema (both hosts that build it); "Match on" lists only mapped fields the rule admits; the change is a filter on `matchable`. - `ExcelImportBar`'s one carried line has no pin of its own: its existing tests (`ExcelImportBar.sharedSelect-11865`) compare the wizard's fields as JSON text, and they stay byte-identical because the key is written only when true. - A saved server mapping's `upsertKey` that names such a field is applied as the mapping says (`setMatchFields(m.upsertKey…)`), and the server's refusal names the mapping; the wizard does not edit an authored mapping. Not changed here. - `importTargetFields.test.ts` is outside both of `app-shell`'s tsc programs (`tsc --noEmit` and `tsconfig.test.json` list neither it nor the other `views/*.test.ts` beside it, measured with `--listFiles`); vitest runs it. Pre-existing, not widened by this PR. ## Verification <details><summary>Ablation (filter removed, then restored)</summary> `node scripts/ablation-replace.mjs --file ui/packages/plugin-grid/src/ImportWizard.tsx --anchor 'mappedFieldNames.has(f.name) && isMatchKeyField(f)' --replacement 'mappedFieldNames.has(f.name) /* ABLATED_27567 */' -- pnpm --dir ui exec vitest run packages/plugin-grid/src/ImportWizard.matchKeyFields-27567.test.tsx` - mutation landed: anchor 1 → 0, blob `bff5282cf050` → `cd3b19e941f3` - red: `Tests 1 failed (1)`, received the six refused fields in addition (`owners`, `stages`, `labels`, `regions`, `meta`, `site`) - restored: blob == HEAD (`bff5282cf050`), `git diff HEAD` empty; the pin passes again (`Tests 1 passed (1)`) `@object-ui/*` resolves to source in ui vitest, so no rebuild between legs. </details> <details><summary>Tests and type-check</summary> - Pre-merge head `06d76839`: `pnpm --dir ui exec vitest run --maxWorkers=2 packages/plugin-grid/ packages/app-shell/src/views/importTargetFields.test.ts packages/app-shell/src/console/ai/__tests__/` → `Test Files 305 passed (305)`, `Tests 2530 passed (2530)` (lock VERDICT command-exit 0). - Merged head `0c73a9da`: the new pin, `importWriteModes-12697`, `importTargetFields.test.ts`, both `ExcelImportBar` test files → `Test Files 5 passed (5)`, `Tests 31 passed (31)`. The `origin/main` merge moved no `plugin-grid` file. - `pnpm --filter @object-ui/app-shell --filter @object-ui/plugin-grid run type-check` on the merged head → VERDICT command-exit 0. - eslint on the five touched source files: 0 errors (warnings are pre-existing lines; the new test adds none). </details> <details><summary>Derived gates (dispatch-gates --commands, merged head)</summary> Head `0c73a9da`, `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran FILE` → `34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED`. ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:pm-changeset-deadline-census :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CXKADE9Bt6J32Lacv5)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b761968 commit 4d1f52c

6 files changed

Lines changed: 135 additions & 6 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/plugin-grid': patch
3+
---
4+
5+
fix(plugin-grid): the import wizard's "Match on" no longer offers a field the server refuses as a match key (#27567)
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
In "Update existing" and "Update if matched, else create", "Match on" listed every mapped field. The server refuses a match key whose value is a list or a structured payload (`400 INVALID_REQUEST`, #27465), so ticking such a field got the whole import refused at Validate. The list now leaves those fields out, by the same spec definitions the server's `multiValueMatchKeyRefusal` reads (`isMultiValueField`, `STRUCTURED_JSON_TYPES` from `@objectstack/spec/data`): a `multiple: true` lookup, user, select, radio, file or image field; a multiselect, checkboxes or tags field; and a JSON-stored type such as `json` or `address`. Single-value fields are offered as before.
11+
12+
The wizard's field shape (`ImportWizardProps['fields']`) takes an optional `multiple` flag, carried from the object schema by the console's import targets; a host that leaves it out still has the inherently multi and structured types filtered by type.
13+
14+
What to do: nothing.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ interface WizardField {
4242
type: string;
4343
required?: boolean;
4444
options?: Array<{ label?: string; value?: string | number } | string>;
45+
multiple?: boolean;
4546
}
4647

4748
interface ExcelImportBarProps {
@@ -71,6 +72,8 @@ function normalizeFields(schema: any): WizardField[] {
7172
type: (def?.type ?? 'text') as string,
7273
required: !!def?.required,
7374
options: def?.options,
75+
// A field holding a list is never offered as a match key (#27567).
76+
...(def?.multiple === true ? { multiple: true } : {}),
7477
}))
7578
.filter((f) => f.name && !NON_WRITABLE_TYPES.includes(f.type));
7679
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,18 @@ describe('importTargetFields', () => {
7777
expect(editOnly.find((f) => f.name === 'name')).toMatchObject({ matchOnly: true, required: false });
7878
});
7979

80+
it('#27567: carries the schema\'s `multiple: true`, so "Match on" can leave a list field out', () => {
81+
const out = importTargetFields('device', {
82+
owners: { type: 'lookup', label: 'Owners', reference: 'user', multiple: true },
83+
owner: { type: 'lookup', label: 'Owner', reference: 'user', multiple: false },
84+
name: { type: 'text', label: 'Name' },
85+
});
86+
const byName = Object.fromEntries(out.map((f) => [f.name, f]));
87+
expect(byName.owners.multiple).toBe(true);
88+
expect(byName.owner.multiple).toBeUndefined();
89+
expect(byName.name.multiple).toBeUndefined();
90+
});
91+
8092
it('without a loaded perms channel, falls back to schema-only derivation', () => {
8193
const out = importTargetFields('device', fields, { isLoaded: false, checkField: () => false });
8294
expect(out.map((f) => f.name).sort()).toEqual(

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export interface ImportTargetField {
3232
required?: boolean;
3333
matchOnly?: boolean;
3434
options?: unknown;
35+
multiple?: boolean;
3536
}
3637

3738
/** Minimal slice of the permissions channel this helper consults. */
@@ -67,6 +68,8 @@ export function importTargetFields(
6768
...(matchOnly ? { matchOnly: true } : {}),
6869
// Enum options seed the downloadable template's example row.
6970
...(def?.options ? { options: def.options } : {}),
71+
// A field holding a list is never offered as a match key (#27567).
72+
...(def?.multiple === true ? { multiple: true } : {}),
7073
});
7174
}
7275
return out;
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* "Match on" offers only the mapped fields the server takes as a match key
3+
* (objectstack#27567).
4+
*
5+
* The server refuses a match key whose value is a list or a structured payload
6+
* (`multiValueMatchKeyRefusal`, objectstack#27465): a `multiple: true`
7+
* lookup or select, a multiselect / tags / checkboxes field, and a JSON-stored
8+
* type. The wizard offered every mapped field, so the user could tick one the
9+
* import then refused at Validate. It now reads the same spec definitions.
10+
*/
11+
import { describe, it, expect, beforeAll, vi } from 'vitest';
12+
import { render, screen, fireEvent, act } from '@testing-library/react';
13+
import '@testing-library/jest-dom';
14+
import React from 'react';
15+
16+
import { ImportWizard } from './ImportWizard';
17+
18+
beforeAll(() => {
19+
// Radix Select reads these on open; happy-dom lacks them.
20+
if (!Element.prototype.hasPointerCapture) {
21+
Element.prototype.hasPointerCapture = vi.fn(() => false);
22+
}
23+
if (!Element.prototype.scrollIntoView) {
24+
Element.prototype.scrollIntoView = vi.fn();
25+
}
26+
});
27+
28+
const FIELDS = [
29+
{ name: 'code', label: 'code', type: 'text' },
30+
{ name: 'email', label: 'email', type: 'email' },
31+
{ name: 'owner', label: 'owner', type: 'lookup' },
32+
{ name: 'owners', label: 'owners', type: 'lookup', multiple: true },
33+
{ name: 'stage', label: 'stage', type: 'select', options: ['a', 'b'] },
34+
{ name: 'stages', label: 'stages', type: 'select', options: ['a', 'b'], multiple: true },
35+
{ name: 'labels', label: 'labels', type: 'tags' },
36+
{ name: 'regions', label: 'regions', type: 'multiselect', options: ['a', 'b'] },
37+
{ name: 'meta', label: 'meta', type: 'json' },
38+
{ name: 'site', label: 'site', type: 'address' },
39+
];
40+
41+
/** Paste TSV into the upload step via the wizard's window-level handler. */
42+
function pasteRows(text: string) {
43+
const evt = new Event('paste', { bubbles: true, cancelable: true }) as Event & {
44+
clipboardData: { getData: (type: string) => string };
45+
};
46+
evt.clipboardData = { getData: (type: string) => (type === 'text/plain' ? text : '') };
47+
act(() => { window.dispatchEvent(evt); });
48+
}
49+
50+
async function toMatchFields() {
51+
const headers = FIELDS.map((f) => f.name);
52+
pasteRows(`${headers.join('\t')}\n${headers.map(() => 'x').join('\t')}`);
53+
fireEvent.click(await screen.findByTestId('import-next-btn'));
54+
return screen.findByTestId('import-matchfields');
55+
}
56+
57+
describe('"Match on" offers only the fields the server takes as a match key (objectstack#27567)', () => {
58+
it('leaves out a list or structured field and offers the single-value ones', async () => {
59+
render(
60+
<ImportWizard
61+
objectName="deal"
62+
fields={FIELDS}
63+
dataSource={{}}
64+
open
65+
onOpenChange={() => {}}
66+
writeModes={['upsert']}
67+
/>,
68+
);
69+
await toMatchFields();
70+
71+
const offered = FIELDS
72+
.map((f) => f.name)
73+
.filter((name) => screen.queryByTestId(`import-matchfield-${name}`) !== null);
74+
expect(offered).toEqual(['code', 'email', 'owner', 'stage']);
75+
});
76+
});

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

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import { resolveAffordance, sanitizeFileNameBase } from '@object-ui/core';
1616
import { useDisplayLocale } from '@object-ui/i18n';
1717
import { usePermissions } from '@object-ui/permissions';
1818
import {
19-
BOOLEAN_VALUE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES, SINGLE_OPTION_TYPES,
19+
BOOLEAN_VALUE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES, SINGLE_OPTION_TYPES, STRUCTURED_JSON_TYPES,
20+
isMultiValueField,
2021
} from '@objectstack/spec/data';
2122
import {
2223
BOOLEAN_IMPORT_TOKENS, REFERENCE_IMPORT_TYPES, isIdentityEmail, isImportableDateCell, isImportableTimeCell,
@@ -362,6 +363,11 @@ export interface ImportWizardProps {
362363
/** Allowed values for select/enum fields. Accepts option objects or bare
363364
* strings. */
364365
options?: Array<{ label?: string; value?: string | number } | string>;
366+
/** The object schema's `multiple` flag: a `select` / `radio` / `lookup` /
367+
* `user` / `file` / `image` field flagged `multiple: true` stores a list.
368+
* "Match on" reads it, as the server does, to leave such a field out
369+
* (`isMatchKeyField`). */
370+
multiple?: boolean;
365371
/** Storage-backed but non-writable target (autonumber / readonly): mappable
366372
* so update/upsert can MATCH rows on it (e.g. "update the row whose record
367373
* number already exists"), rendered with a "(match only)" hint. The value
@@ -605,6 +611,17 @@ function formatDryRunError(
605611
type ImportTargetField = ImportWizardProps['fields'][number];
606612
type ImportTargetOption = NonNullable<ImportTargetField['options']>[number];
607613

614+
/**
615+
* Whether the server takes this field as a match key. It refuses a field whose
616+
* value is a list or a structured payload, as one value per field is what a
617+
* match key compares: the server's `multiValueMatchKeyRefusal` reads the same
618+
* two spec definitions, `STRUCTURED_JSON_TYPES` and `isMultiValueField`.
619+
*/
620+
function isMatchKeyField(field: Pick<ImportTargetField, 'type' | 'multiple'>): boolean {
621+
if (STRUCTURED_JSON_TYPES.has(field.type)) return false;
622+
return !isMultiValueField({ type: field.type, multiple: field.multiple === true });
623+
}
624+
608625
/** The only comma a numeric cell may carry: a thousands grouping of its integer part. */
609626
const THOUSANDS_GROUPED_INTEGER = /^[+-]?\d{1,3}(?:,\d{3})+(?![\d,])[^,]*$/;
610627

@@ -671,9 +688,9 @@ function splitImportMultiValue(cell: string): string[] {
671688
*
672689
* One reading is not the server's exactly, and it errs towards taking a cell:
673690
* a `select` / `radio` flagged `multiple: true` is split by the server as well,
674-
* and the wizard's field shape does not carry `multiple`. So a single-option
675-
* cell is refused only when both readings refuse it: as one value, and as a
676-
* split list.
691+
* and a host may leave the field shape's `multiple` out, so this check does not
692+
* read it. So a single-option cell is refused only when both readings refuse
693+
* it: as one value, and as a split list.
677694
*
678695
* With *Keep unknown option values* on (the request's `createMissingOptions`),
679696
* the server takes every option cell, on a writable field and a `matchOnly`
@@ -1962,9 +1979,13 @@ const ImportOptions: React.FC<{
19621979
showBackground, backgroundImport, onBackgroundImport,
19631980
}) => {
19641981
const { t } = useImportTranslation();
1965-
// Only fields that are actually mapped can serve as match keys.
1982+
// Only fields that are actually mapped, and that the server takes as a
1983+
// match key, can serve as match keys.
19661984
const mappedFieldNames = useMemo(() => new Set(Object.values(mapping)), [mapping]);
1967-
const matchable = useMemo(() => fields.filter((f) => mappedFieldNames.has(f.name)), [fields, mappedFieldNames]);
1985+
const matchable = useMemo(
1986+
() => fields.filter((f) => mappedFieldNames.has(f.name) && isMatchKeyField(f)),
1987+
[fields, mappedFieldNames],
1988+
);
19681989
const needsMatch = writeMode !== 'insert';
19691990

19701991
return (

0 commit comments

Comments
 (0)