Skip to content

Commit aeb2110

Browse files
authored
fix(rest): split multi-value fields on import so multiple:true columns resolve per-token (#3063) (#3068)
1 parent 7bc9e79 commit aeb2110

5 files changed

Lines changed: 240 additions & 16 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest): split multi-value fields on import so `multiple: true` columns resolve per-token (#3063)
6+
7+
The bulk-import coercion (`import-coerce.ts`) resolved a reference cell as a
8+
single value regardless of the field's `multiple` flag: a `multiple: true`
9+
lookup/user cell like `张焊工;李质检` was passed whole to name resolution and
10+
always failed with `no <object> matches "张焊工;李质检"`, so every multi-value
11+
association had to be back-filled by hand in the record UI after import.
12+
13+
Coercion now mirrors objectql's `isMultiValueField` predicate. A field whose
14+
stored value is an array — an inherently-multi type (multiselect/checkboxes/tags)
15+
or a multi-capable type flagged `multiple: true` (per the spec: select, lookup,
16+
file, image; `radio` shares select's branch and `user` shares lookup's) — has
17+
its cell split on the export separator (`, ` / `;` / `` / newline) and each
18+
token coerced individually:
19+
20+
- **lookup / user (`multiple: true`)** — resolve each name token to an id, store
21+
the id array; an unmatched/ambiguous token reports the **specific token**
22+
(`no sys_user matches "查无此人"`) instead of the whole string.
23+
- **select / radio (`multiple: true`)** — match each token against the options,
24+
store the option-value array.
25+
- **file / image (`multiple: true`)** — split into an id/url array.
26+
27+
Single-value fields and the non-multi-capable reference types (master_detail /
28+
reference / tree) are unchanged — a stray `multiple: true` on them stays a
29+
single resolved value, matching the engine.

packages/rest/src/export-format.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ export interface ExportFieldMeta {
2222
reference?: string;
2323
/** Field on the referenced record to show as its label. */
2424
displayField?: string;
25+
/** Field holds multiple values (an array), e.g. a `multiple: true` lookup. */
26+
multiple?: boolean;
2527
// The following four drive the import path's required-field pre-check
2628
// (import-runner.ts). They mirror the engine's insert-time validation
2729
// (objectql record-validator.ts) so a dry run can predict a NOT NULL /
@@ -127,6 +129,7 @@ export function buildFieldMetaMap(schema: unknown): Map<string, ExportFieldMeta>
127129
options: Array.isArray(f.options) ? f.options : undefined,
128130
reference: typeof f.reference === 'string' ? f.reference : undefined,
129131
displayField: typeof f.displayField === 'string' ? f.displayField : undefined,
132+
multiple: f.multiple === true,
130133
required: f.required === true,
131134
system: f.system === true,
132135
readonly: f.readonly === true,

packages/rest/src/import-coerce.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,93 @@ describe('coerceRow', () => {
158158
expect(none.errors[0]).toMatchObject({ field: 'owner', code: 'reference_not_found' });
159159
});
160160

161+
it('splits a multi-value lookup cell and resolves each token to an id', async () => {
162+
const metaMap = meta({
163+
members: { type: 'lookup', reference: 'sys_user', displayField: 'name', multiple: true },
164+
});
165+
const ids: Record<string, string> = { 张焊工: 'u1', 李质检: 'u2' };
166+
const seen: string[] = [];
167+
const resolveRef = async (_obj: string, display: string) => {
168+
seen.push(display);
169+
return ids[display];
170+
};
171+
// Semicolon-separated (issue's CSV) and comma-separated (export round-trip).
172+
const semi = await coerceRow({ members: '张焊工;李质检' }, metaMap, { resolveRef });
173+
expect(semi.errors).toEqual([]);
174+
expect(semi.data).toEqual({ members: ['u1', 'u2'] });
175+
const comma = await coerceRow({ members: '张焊工, 李质检' }, metaMap, { resolveRef });
176+
expect(comma.data).toEqual({ members: ['u1', 'u2'] });
177+
expect(seen).toEqual(['张焊工', '李质检', '张焊工', '李质检']);
178+
});
179+
180+
it('names the specific unmatched token in a multi-value lookup', async () => {
181+
const metaMap = meta({
182+
members: { type: 'lookup', reference: 'sys_user', displayField: 'name', multiple: true },
183+
});
184+
const resolveRef = async (_obj: string, display: string) =>
185+
display === '张焊工' ? 'u1' : undefined;
186+
const { data, errors } = await coerceRow({ members: '张焊工;查无此人' }, metaMap, { resolveRef });
187+
expect(data.members).toBeUndefined();
188+
expect(errors[0]).toMatchObject({ field: 'members', code: 'reference_not_found' });
189+
expect(errors[0].message).toContain('查无此人');
190+
expect(errors[0].message).not.toContain('张焊工');
191+
});
192+
193+
it('keeps raw multi-value lookup tokens when no resolver is supplied', async () => {
194+
const metaMap = meta({
195+
members: { type: 'lookup', reference: 'sys_user', multiple: true },
196+
});
197+
const { data } = await coerceRow({ members: 'u1;u2' }, metaMap, {});
198+
expect(data).toEqual({ members: ['u1', 'u2'] });
199+
});
200+
201+
it('splits a select flagged multiple:true into an array of option values', async () => {
202+
const metaMap = meta({
203+
skills: {
204+
type: 'select', multiple: true,
205+
options: [{ label: '焊接', value: 'weld' }, { label: '质检', value: 'qc' }],
206+
},
207+
});
208+
const { data, errors } = await coerceRow({ skills: '焊接;质检' }, metaMap, {});
209+
expect(errors).toEqual([]);
210+
expect(data).toEqual({ skills: ['weld', 'qc'] });
211+
// A single-value select (no multiple flag) still stores one scalar.
212+
const single = meta({ s: { type: 'select', options: [{ label: '焊接', value: 'weld' }] } });
213+
const one = await coerceRow({ s: '焊接' }, single, {});
214+
expect(one.data).toEqual({ s: 'weld' });
215+
});
216+
217+
it('names the specific unmatched token in a multiple:true select', async () => {
218+
const metaMap = meta({
219+
skills: { type: 'select', multiple: true, options: [{ label: '焊接', value: 'weld' }] },
220+
});
221+
const { errors } = await coerceRow({ skills: '焊接,搬砖' }, metaMap, {});
222+
expect(errors[0]).toMatchObject({ field: 'skills', code: 'invalid_option' });
223+
expect(errors[0].message).toContain('搬砖');
224+
expect(errors[0].message).not.toContain('焊接');
225+
});
226+
227+
it('splits a file/image flagged multiple:true into an array of ids/urls', async () => {
228+
const metaMap = meta({ photos: { type: 'image', multiple: true } });
229+
const { data } = await coerceRow({ photos: 'a.png;b.png' }, metaMap, {});
230+
expect(data).toEqual({ photos: ['a.png', 'b.png'] });
231+
// A single-value file passes through untouched.
232+
const single = meta({ f: { type: 'file' } });
233+
const one = await coerceRow({ f: 'a.png' }, single, {});
234+
expect(one.data).toEqual({ f: 'a.png' });
235+
});
236+
237+
it('ignores multiple:true on types the spec does not make multi (master_detail)', async () => {
238+
// master_detail is not multi-capable per the spec — a stray multiple flag
239+
// must not split it; it stays a single resolved reference (engine parity).
240+
const metaMap = meta({
241+
parent: { type: 'master_detail', reference: 'order', displayField: 'name', multiple: true },
242+
});
243+
const resolveRef = async (_obj: string, display: string) => (display === 'A;B' ? 'o1' : undefined);
244+
const { data } = await coerceRow({ parent: 'A;B' }, metaMap, { resolveRef });
245+
expect(data).toEqual({ parent: 'o1' });
246+
});
247+
161248
it('reports coercion errors per field instead of throwing', async () => {
162249
const metaMap = meta({ n: { type: 'number' }, b: { type: 'boolean' } });
163250
const { data, errors } = await coerceRow({ n: 'abc', b: 'maybe' }, metaMap, {});

packages/rest/src/import-coerce.ts

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020
* - select / radio → an option *value*
2121
* - multiselect / checkboxes / tags → an array of option values
2222
* - lookup / master_detail / user / reference → a record id (resolved async)
23+
* - file / image → a file id / url (as-is)
24+
*
25+
* Any of the last four whose field is flagged `multiple: true` (per the spec,
26+
* `multiple` applies to select / lookup / file / image; `radio`/`user` share
27+
* their branch) instead store an **array** — the cell is split on the export
28+
* separator and each token coerced individually. See `isMultiValueField`.
2329
*
2430
* Contract: when a field carries no usable metadata the value passes through
2531
* untouched, so an import stays byte-identical to the pre-coercion behaviour.
@@ -37,6 +43,31 @@ const MULTI_OPTION_TYPES = new Set(['multiselect', 'checkboxes', 'tags']);
3743
const NUMBER_TYPES = new Set(['number', 'currency', 'percent', 'rating', 'slider']);
3844
/** Boolean field types (store a real boolean). */
3945
const BOOL_TYPES = new Set(['boolean', 'toggle']);
46+
/** Attachment field types (store a file id / url, or an array when `multiple`). */
47+
const FILE_TYPES = new Set(['file', 'image']);
48+
49+
/**
50+
* Single-value field types that become an ARRAY when flagged `multiple: true`.
51+
* Mirrors objectql `record-validator.ts` (`MULTI_CAPABLE_TYPES`): per the spec
52+
* (`field.zod.ts`), `multiple` applies to select / lookup / file / image;
53+
* `radio` shares the select branch and `user` is stored identically to `lookup`.
54+
* master_detail / reference / tree are NOT multi-capable, so a stray
55+
* `multiple: true` on them is ignored (they stay single — same as the engine).
56+
*/
57+
const MULTI_CAPABLE_TYPES = new Set(['select', 'radio', 'lookup', 'user', 'file', 'image']);
58+
59+
/**
60+
* Whether a field's stored value is an array — an inherently-multi type
61+
* (multiselect / checkboxes / tags) or a multi-capable type flagged
62+
* `multiple: true`. Kept in lock-step with the engine's `isMultiValueField`
63+
* so a coerced cell has the SAME shape the engine will accept on insert.
64+
*/
65+
function isMultiValueField(meta: ExportFieldMeta | undefined): boolean {
66+
const t = meta?.type;
67+
if (!t) return false;
68+
if (MULTI_OPTION_TYPES.has(t)) return true;
69+
return MULTI_CAPABLE_TYPES.has(t) && meta?.multiple === true;
70+
}
4071

4172
/**
4273
* Structured outcome of a reference lookup. `id` set → a single record matched.
@@ -279,7 +310,24 @@ export async function coerceFieldValue(
279310
return { value: d };
280311
}
281312

282-
if (OPTION_TYPES.has(t)) {
313+
// select / radio / multiselect / checkboxes / tags — match the cell against
314+
// the field's option list. Multi-valued when the type is inherently multi
315+
// (multiselect/…) OR a select/radio is flagged `multiple: true`; split then
316+
// and match each token, else match the whole cell as one option.
317+
if (OPTION_TYPES.has(t) || MULTI_OPTION_TYPES.has(t)) {
318+
if (isMultiValueField(meta)) {
319+
const parts = splitMulti(raw);
320+
const out: unknown[] = [];
321+
for (const part of parts) {
322+
const v = matchOption(part, meta?.options);
323+
if (v === undefined) {
324+
if (ctx.createMissingOptions) { out.push(part); continue; }
325+
return { error: { field, code: 'invalid_option', message: `${field}: "${part}" is not a known option` } };
326+
}
327+
out.push(v);
328+
}
329+
return { value: out };
330+
}
283331
const v = matchOption(raw, meta?.options);
284332
if (v === undefined) {
285333
if (ctx.createMissingOptions) return { value: String(raw).trim() };
@@ -288,21 +336,29 @@ export async function coerceFieldValue(
288336
return { value: v };
289337
}
290338

291-
if (MULTI_OPTION_TYPES.has(t)) {
292-
const parts = splitMulti(raw);
293-
const out: unknown[] = [];
294-
for (const part of parts) {
295-
const v = matchOption(part, meta?.options);
296-
if (v === undefined) {
297-
if (ctx.createMissingOptions) { out.push(part); continue; }
298-
return { error: { field, code: 'invalid_option', message: `${field}: "${part}" is not a known option` } };
339+
if (REFERENCE_TYPES.has(t)) {
340+
// Multi-value reference (a `multiple: true` lookup / user): the cell holds
341+
// several display names joined by the export separator (`, ` / `;`). Split
342+
// first, then resolve each token; store an array of ids. Mirrors the
343+
// multi-option branch above and the export path's `formatReference` join.
344+
if (isMultiValueField(meta)) {
345+
const tokens = splitMulti(raw);
346+
// If we have no resolver / no target object, store the raw tokens and let
347+
// referential integrity be enforced downstream.
348+
if (!ctx.resolveRef || !meta.reference) return { value: tokens };
349+
const out: unknown[] = [];
350+
for (const token of tokens) {
351+
const m = normalizeRefMatch(await ctx.resolveRef(meta.reference, token, meta));
352+
if (m.ambiguous) {
353+
return { error: { field, code: 'reference_ambiguous', message: `${field}: "${token}" matches more than one ${meta.reference} — use a unique value or the record id` } };
354+
}
355+
if (m.id === undefined) {
356+
return { error: { field, code: 'reference_not_found', message: `${field}: no ${meta.reference} matches "${token}"` } };
357+
}
358+
out.push(m.id);
299359
}
300-
out.push(v);
360+
return { value: out };
301361
}
302-
return { value: out };
303-
}
304-
305-
if (REFERENCE_TYPES.has(t)) {
306362
const display = String(raw).trim();
307363
// If it already looks resolved (an id was pasted) or we have no resolver /
308364
// no target object, store the raw value and let referential integrity be
@@ -318,8 +374,17 @@ export async function coerceFieldValue(
318374
return { value: match.id };
319375
}
320376

321-
// Everything else (text, email, phone, json, html, file, …): pass through,
322-
// trimming string cells so stray spreadsheet padding doesn't leak into storage.
377+
// Attachment fields (file / image): the value is a file id / url the importer
378+
// does not resolve. When `multiple: true` the cell holds several joined by the
379+
// export separator — split into an array so the stored shape matches what the
380+
// engine expects; a single-value attachment passes through untouched below.
381+
if (FILE_TYPES.has(t) && isMultiValueField(meta)) {
382+
return { value: splitMulti(raw) };
383+
}
384+
385+
// Everything else (text, email, phone, json, html, single file, …): pass
386+
// through, trimming string cells so stray spreadsheet padding doesn't leak
387+
// into storage.
323388
return { value: trim && typeof raw === 'string' ? raw.trim() : raw };
324389
}
325390

packages/rest/src/import-integration.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ const TASK = {
9494
score: { name: 'score', type: 'number' as const, label: '分数' },
9595
due: { name: 'due', type: 'date' as const, label: '截止' },
9696
owner: { name: 'owner', type: 'lookup' as const, label: '负责人', reference: 'user', displayField: 'name' },
97+
members: { name: 'members', type: 'lookup' as const, label: '成员', reference: 'user', displayField: 'name', multiple: true },
98+
skills: {
99+
name: 'skills', type: 'select' as const, label: '技能', multiple: true,
100+
options: [{ label: '焊接', value: 'weld' }, { label: '质检', value: 'qc' }],
101+
},
97102
},
98103
};
99104

@@ -188,6 +193,41 @@ describe('import route — real engine + protocol integration', () => {
188193
expect(String(stored.due)).toContain('2026-06-30');
189194
});
190195

196+
it('splits a multi-value lookup cell and resolves every token to an id (issue #3063)', async () => {
197+
// The cell holds several display names joined by `;` (issue's CSV). Before
198+
// the fix the whole string was resolved as one reference and always failed.
199+
const csv = ['ID,标题,成员', '1,结构一班,张三;李四'].join('\n');
200+
const res = await call(route, {
201+
format: 'csv', csv,
202+
mapping: { ID: 'id', 标题: 'title', 成员: 'members' },
203+
});
204+
expect(res._json).toMatchObject({ total: 1, ok: 1, errors: 0, created: 1 });
205+
const stored = await engine.findOne('task', { where: { id: '1' } });
206+
expect(stored.members).toEqual(['u1', 'u2']);
207+
});
208+
209+
it('splits a select flagged multiple:true into an option-value array on insert (issue #3063)', async () => {
210+
const csv = ['ID,标题,技能', '1,焊工,焊接;质检'].join('\n');
211+
const res = await call(route, {
212+
format: 'csv', csv,
213+
mapping: { ID: 'id', 标题: 'title', 技能: 'skills' },
214+
});
215+
expect(res._json).toMatchObject({ total: 1, ok: 1, errors: 0, created: 1 });
216+
const stored = await engine.findOne('task', { where: { id: '1' } });
217+
expect(stored.skills).toEqual(['weld', 'qc']);
218+
});
219+
220+
it('names the specific unmatched token in a multi-value lookup (issue #3063)', async () => {
221+
const res = await call(route, {
222+
format: 'json',
223+
rows: [{ id: 'a', title: 'x', members: '张三;查无此人' }],
224+
});
225+
const failed = res._json.results.find((r: any) => !r.ok);
226+
expect(failed).toMatchObject({ field: 'members', code: 'reference_not_found' });
227+
expect(failed.error).toContain('查无此人');
228+
expect(failed.error).not.toContain('张三');
229+
});
230+
191231
it('resolves a lookup by email when displayField is not the match, and reports not-found', async () => {
192232
// Default candidate fields include email — resolve 李四 via email.
193233
const res = await call(route, {

0 commit comments

Comments
 (0)