Skip to content

Commit 2b5e95b

Browse files
baozhoutaoclaude
andauthored
fix(app-shell,plugin-grid,i18n): autonumber/readonly fields become match-only import targets so "update if the record number exists" works (#3061)
ObjectView dropped computed AND readonly/autonumber fields from the ImportWizard's target list wholesale, so a record number (autonumber) could never be mapped — and since match fields must be mapped columns, it could never be the update/upsert match key either ("编号存在则更新" was impossible from the UI even though the server import accepts any stored column as a matchField). The derivation (extracted to importTargetFields for unit tests) now keeps storage-backed non-writable fields — autonumber and readonly, FLS-read gated — as `matchOnly` targets: mappable, rendered with a "(match only)" hint, never counted as a required write column. Their values still travel in the rows (the server needs them to locate the record; the engine strips readonly values from updates and respects explicitly-seeded values on insert). Computed formula/summary fields stay excluded — no storage column to match on. FLS-write-denied but readable fields degrade to match-only instead of disappearing. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 6b83a55 commit 2b5e95b

14 files changed

Lines changed: 159 additions & 25 deletions

File tree

packages/app-shell/src/views/ObjectView.tsx

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import { RecordDetailView } from './RecordDetailView';
5858
import { resolveCrudAffordances } from '../utils/crudAffordances';
5959
import { createIdentityImportDataSource, IDENTITY_IMPORT_OBJECT, type IdentityPasswordPolicy } from './identityImport';
6060
import { IdentityImportOptions, IdentityImportResultExtra, identityImportFields } from './IdentityImportPanels';
61+
import { importTargetFields } from './importTargetFields';
6162
import { useExpressionContext } from '../providers/ExpressionProvider';
6263
import { resolveManagedByEmptyState } from '../utils/managedByEmptyState';
6364
import { resolveViewId } from '../utils/resolveViewId';
@@ -1806,30 +1807,11 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
18061807
// (readonly for CRUD; identity writes go through
18071808
// better-auth, framework#2782).
18081809
? identityImportFields(objectDef.fields)
1809-
: Object.entries(objectDef.fields || {})
1810-
// Only writable fields are importable targets. Computed
1811-
// types (formula/summary/autonumber) and readonly fields
1812-
// are server-rejected, and FLS-restricted fields get a
1813-
// 403 from the write path — so we omit them from the
1814-
// mapping step (and the downloadable template) rather
1815-
// than let a user map to a column the import will
1816-
// reject. The FLS bit comes from the server-resolved
1817-
// /me/permissions channel (checkField), not from the
1818-
// schema, which carries no per-caller permission bits
1819-
// (objectstack#3661).
1820-
.filter(([name, def]: [string, any]) =>
1821-
!['formula', 'summary', 'autonumber'].includes(def?.type) &&
1822-
!def?.readonly &&
1823-
(!perms?.isLoaded || perms.checkField(objectDef.name, name, 'write')),
1824-
)
1825-
.map(([name, def]: [string, any]) => ({
1826-
name,
1827-
label: def?.label || name,
1828-
type: def?.type || 'text',
1829-
required: !!def?.required,
1830-
// Enum options seed the downloadable template's example row.
1831-
...(def?.options ? { options: def.options } : {}),
1832-
}))}
1810+
// Writable fields as WRITE targets + storage-backed
1811+
// readonly/autonumber fields as MATCH-ONLY targets (so
1812+
// update/upsert can match on the record number, #020) —
1813+
// see importTargetFields for the full derivation.
1814+
: importTargetFields(objectDef.name, objectDef.fields, perms)}
18331815
dataSource={identityDataSource ?? dataSource}
18341816
{...(identityImportEnabled ? {
18351817
extraOptionsContent: (
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* importTargetFields — writable vs match-only derivation for the ImportWizard
3+
* (#020: "编号存在则更新" — the record number must be usable as an upsert match
4+
* key even though it is autonumber/readonly and therefore not writable).
5+
*/
6+
import { describe, it, expect } from 'vitest';
7+
import { importTargetFields, type ImportFieldPerms } from './importTargetFields';
8+
9+
const fields = {
10+
code: { type: 'autonumber', label: '编号', required: true },
11+
name: { type: 'text', label: '名称', required: true },
12+
slot: { type: 'text', label: '机位号' },
13+
total: { type: 'formula', label: '合计' },
14+
child_sum: { type: 'summary', label: '子表合计' },
15+
locked_note: { type: 'text', label: '锁定备注', readonly: true },
16+
};
17+
18+
const permsAllowing = (allow: (field: string, op: 'read' | 'write') => boolean): ImportFieldPerms => ({
19+
isLoaded: true,
20+
checkField: (_obj, field, op) => allow(field, op),
21+
});
22+
23+
describe('importTargetFields', () => {
24+
it('keeps writable fields as write targets and marks autonumber/readonly as match-only', () => {
25+
const out = importTargetFields('device', fields);
26+
const byName = Object.fromEntries(out.map((f) => [f.name, f]));
27+
28+
expect(byName.name).toMatchObject({ required: true });
29+
expect(byName.name.matchOnly).toBeUndefined();
30+
expect(byName.slot.matchOnly).toBeUndefined();
31+
32+
// The record number is present again — but only for matching.
33+
expect(byName.code).toMatchObject({ matchOnly: true, type: 'autonumber' });
34+
expect(byName.locked_note).toMatchObject({ matchOnly: true });
35+
// A match-only column is never a required WRITE column, even when the
36+
// schema declares required (the runtime owns the value).
37+
expect(byName.code.required).toBe(false);
38+
});
39+
40+
it('still drops computed fields — no storage column to match or write', () => {
41+
const out = importTargetFields('device', fields);
42+
const names = out.map((f) => f.name);
43+
expect(names).not.toContain('total');
44+
expect(names).not.toContain('child_sum');
45+
});
46+
47+
it('match-only targets are gated on FLS read; write targets on FLS write', () => {
48+
// Reader who cannot write `name` and cannot read `locked_note`.
49+
const out = importTargetFields('device', fields, permsAllowing((field, op) =>
50+
op === 'write' ? field === 'slot' : field !== 'locked_note',
51+
));
52+
const names = out.map((f) => f.name);
53+
expect(names).toContain('slot'); // writable
54+
expect(names).toContain('code'); // readable → match-only
55+
expect(names).not.toContain('locked_note'); // unreadable → gone entirely
56+
// `name` lost write access → degrades to a match-only (readable) target
57+
// rather than disappearing: it can still locate rows.
58+
expect(out.find((f) => f.name === 'name')).toMatchObject({ matchOnly: true, required: false });
59+
});
60+
61+
it('without a loaded perms channel, falls back to schema-only derivation', () => {
62+
const out = importTargetFields('device', fields, { isLoaded: false, checkField: () => false });
63+
expect(out.map((f) => f.name).sort()).toEqual(
64+
['code', 'locked_note', 'name', 'slot'],
65+
);
66+
});
67+
});
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Import-wizard target fields for an object (extracted from ObjectView so the
3+
* writable / match-only derivation is unit-testable).
4+
*
5+
* Writable fields are importable WRITE targets. Computed types
6+
* (formula/summary) have no storage column and FLS-restricted fields get a 403
7+
* from the write path — so those are omitted from the mapping step (and the
8+
* downloadable template) rather than let a user map to a column the import
9+
* will reject. The FLS bit comes from the server-resolved /me/permissions
10+
* channel (checkField), not from the schema, which carries no per-caller
11+
* permission bits (objectstack#3661).
12+
*
13+
* Storage-backed but non-writable fields — autonumber and `readonly` — stay in
14+
* the list as MATCH-ONLY targets (#020: "update the row whose record number
15+
* already exists"). They are mappable so update/upsert can match on them (the
16+
* record number is usually the only natural key the file carries); the engine
17+
* strips readonly values from updates and respects explicitly-seeded values on
18+
* insert, so passing their values through is safe. Gated on FLS read —
19+
* matching leaks the column's values by existence.
20+
*/
21+
22+
export interface ImportTargetField {
23+
name: string;
24+
label: string;
25+
type: string;
26+
required?: boolean;
27+
matchOnly?: boolean;
28+
options?: unknown;
29+
}
30+
31+
/** Minimal slice of the permissions channel this helper consults. */
32+
export interface ImportFieldPerms {
33+
isLoaded: boolean;
34+
checkField: (objectName: string, fieldName: string, op: 'read' | 'write') => boolean;
35+
}
36+
37+
const COMPUTED_TYPES = new Set(['formula', 'summary']);
38+
39+
export function importTargetFields(
40+
objectName: string,
41+
fields: Record<string, any> | undefined,
42+
perms?: ImportFieldPerms | null,
43+
): ImportTargetField[] {
44+
const out: ImportTargetField[] = [];
45+
for (const [name, def] of Object.entries(fields || {})) {
46+
const computed = COMPUTED_TYPES.has(def?.type);
47+
const writable = !computed
48+
&& def?.type !== 'autonumber'
49+
&& !def?.readonly
50+
&& (!perms?.isLoaded || perms.checkField(objectName, name, 'write'));
51+
const matchOnly = !writable && !computed
52+
&& (!perms?.isLoaded || perms.checkField(objectName, name, 'read'));
53+
if (!writable && !matchOnly) continue;
54+
out.push({
55+
name,
56+
label: def?.label || name,
57+
type: def?.type || 'text',
58+
// A non-writable target can never be a required WRITE column —
59+
// `required` only steers the mapping step's completeness hint.
60+
required: writable && !!def?.required,
61+
...(matchOnly ? { matchOnly: true } : {}),
62+
// Enum options seed the downloadable template's example row.
63+
...(def?.options ? { options: def.options } : {}),
64+
});
65+
}
66+
return out;
67+
}

packages/i18n/src/locales/ar.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ const ar = {
321321
matchFieldsPlaceholder: "اختر حقول المطابقة…",
322322
matchFieldsHint: "تتم مطابقة الصفوف بالسجلات الموجودة عبر هذه الحقول.",
323323
needMatchFields: "اختر حقلًا واحدًا على الأقل للمطابقة.",
324+
matchOnlyField: "(للمطابقة فقط)",
324325
optCreateOptions: "الاحتفاظ بقيم الخيارات غير المعروفة",
325326
optRunAutomations: "تشغيل الأتمتة والمشغّلات",
326327
optTreatHistorical: "الاستيراد كبيانات تاريخية",

packages/i18n/src/locales/de.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ const de = {
321321
matchFieldsPlaceholder: "Abgleichsfeld(er) wählen…",
322322
matchFieldsHint: "Zeilen werden über diese Felder vorhandenen Datensätzen zugeordnet.",
323323
needMatchFields: "Wählen Sie mindestens ein Feld für den Abgleich aus.",
324+
matchOnlyField: "(nur Abgleich)",
324325
optCreateOptions: "Unbekannte Optionswerte beibehalten",
325326
optRunAutomations: "Automatisierungen und Trigger ausführen",
326327
optTreatHistorical: "Als historische Daten importieren",

packages/i18n/src/locales/en.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ const en = {
356356
matchFieldsPlaceholder: 'Choose match field(s)…',
357357
matchFieldsHint: 'Rows are matched to existing records by these field(s).',
358358
needMatchFields: 'Select at least one field to match on.',
359+
matchOnlyField: '(match only)',
359360
optCreateOptions: 'Keep unknown option values',
360361
optRunAutomations: 'Run automations & triggers',
361362
optTreatHistorical: 'Import as historical data',

packages/i18n/src/locales/es.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ const es = {
321321
matchFieldsPlaceholder: "Elija el campo o campos de coincidencia…",
322322
matchFieldsHint: "Las filas se emparejan con registros existentes por estos campos.",
323323
needMatchFields: "Seleccione al menos un campo por el que coincidir.",
324+
matchOnlyField: "(solo coincidencia)",
324325
optCreateOptions: "Conservar los valores de opción desconocidos",
325326
optRunAutomations: "Ejecutar automatizaciones y disparadores",
326327
optTreatHistorical: "Importar como datos históricos",

packages/i18n/src/locales/fr.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ const fr = {
321321
matchFieldsPlaceholder: "Choisir le ou les champs de correspondance…",
322322
matchFieldsHint: "Les lignes sont rapprochées des enregistrements existants via ces champs.",
323323
needMatchFields: "Sélectionnez au moins un champ de correspondance.",
324+
matchOnlyField: "(correspondance seule)",
324325
optCreateOptions: "Conserver les valeurs d'option inconnues",
325326
optRunAutomations: "Exécuter les automatisations et déclencheurs",
326327
optTreatHistorical: "Importer comme données historiques",

packages/i18n/src/locales/ja.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ const ja = {
321321
matchFieldsPlaceholder: "照合する項目を選択…",
322322
matchFieldsHint: "これらの項目で既存レコードと照合します。",
323323
needMatchFields: "照合する項目を 1 つ以上選択してください。",
324+
matchOnlyField: "(照合のみ)",
324325
optCreateOptions: "未知の選択肢の値を保持する",
325326
optRunAutomations: "自動化とトリガーを実行する",
326327
optTreatHistorical: "過去データとしてインポートする",

packages/i18n/src/locales/ko.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ const ko = {
321321
matchFieldsPlaceholder: "일치 기준 필드 선택…",
322322
matchFieldsHint: "이 필드를 기준으로 기존 레코드와 대조합니다.",
323323
needMatchFields: "일치 기준 필드를 하나 이상 선택하세요.",
324+
matchOnlyField: "(일치 전용)",
324325
optCreateOptions: "알 수 없는 선택 값 유지",
325326
optRunAutomations: "자동화 및 트리거 실행",
326327
optTreatHistorical: "과거 데이터로 가져오기",

0 commit comments

Comments
 (0)