diff --git a/.changeset/25577-import-applyon-matchkeys.md b/.changeset/25577-import-applyon-matchkeys.md new file mode 100644 index 00000000000..8bbd64ee21a --- /dev/null +++ b/.changeset/25577-import-applyon-matchkeys.md @@ -0,0 +1,31 @@ +--- +'@objectstack/spec': minor +'@objectstack/core': minor +'@objectstack/rest': minor +--- + +feat(spec,core,rest): an import column can be written only when its row is created (`applyOn`), and `matchFields` takes ordered key sets + +Clause-②: yes (widening) +Metadata-protocol: added + +Two additions to the import contract (`POST /data/:object/import` and the async import job), both honoured by the import runner: + +- **`FieldMappingEntry.applyOn: 'create' | 'always'`** (default `always`, today's behaviour). An inline mapping entry marked `create` is written when the import creates the row and left out when an `upsert` updates an existing one; an `update` import never writes it. One upsert can then be re-run safely: master-data columns refresh, while a column the app has changed since the first run (a status, a flag) keeps its current value. A create-only column is still read for matching, and the key a row was found by stays in that row's update. An `applyOn` value outside `create` / `always` refuses the request with `400 INVALID_REQUEST`. +- **`matchFields` accepts ordered key sets** (`string[][]`) beside one key set (`string[]`, unchanged). Each row is matched by the first set whose fields are all non-blank on it, and only by that set, so one sheet can identify domestic suppliers by `credit_code` and overseas ones by `country` + `registration_no`. A row blank on every set fails with `BLANK_MATCH_KEY` (`ok: false`, `action: 'failed'`) instead of being created or skipped; `skipBlankMatchKey: true` with key sets is refused with `400 INVALID_REQUEST`, as are malformed key sets (an empty set or name, or names mixed with sets). A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as before. + +```json +{ + "writeMode": "upsert", + "matchFields": [["credit_code"], ["country", "registration_no"]], + "mapping": [ + { "sourceField": "Code", "targetField": "credit_code" }, + { "sourceField": "Country", "targetField": "country" }, + { "sourceField": "Reg. no.", "targetField": "registration_no" }, + { "sourceField": "Name", "targetField": "name" }, + { "sourceField": "Status", "targetField": "status", "applyOn": "create" } + ] +} +``` + +`@objectstack/spec` also exports `isMatchKeySets(matchFields)`, which tells the two `matchFields` spellings apart. `RunImportOptions` in `@objectstack/core` gains `createOnlyFields` and accepts both `matchFields` spellings. diff --git a/content/docs/data-modeling/import-mappings.mdx b/content/docs/data-modeling/import-mappings.mdx index 85625b12851..dcd14ee881f 100644 --- a/content/docs/data-modeling/import-mappings.mdx +++ b/content/docs/data-modeling/import-mappings.mdx @@ -198,7 +198,7 @@ sending both is `400 CONFLICTING_MAPPING`. | | inline `mapping` | `mappingName` | |:---|:---|:---| | Shape | A flat `{ "": "" }` rename (or a `sourceField`/`targetField` array) | A registered `mapping` artifact | -| Transforms | Per entry of the array form: `transform` (`trim`, `uppercase`, `lowercase`; `none` and `lookup` copy the cell), `defaultValue` for a blank cell, `required`. The flat record is a rename only | The `fieldMapping` pipeline | +| Transforms | Per entry of the array form: `transform` (`trim`, `uppercase`, `lowercase`; `none` and `lookup` copy the cell), `defaultValue` for a blank cell, `required`, `applyOn`. The flat record is a rename only | The `fieldMapping` pipeline | | Unmapped columns | **Pass through** to the write path under their own header | **Dropped.** The artifact is a strict projection: only mapped targets survive | | Lives | In the one request | In a package or in metadata, reusable | @@ -206,6 +206,45 @@ That projection difference is the one to keep in mind: a file from an external s routinely carries columns that must not reach the write path, and the artifact path guarantees they do not. +### Columns written only when a row is created: `applyOn` + +An entry of the array form may declare `applyOn: 'create'` (the default is `'always'`). +Its column is written when the import creates the row and left out when an `upsert` +updates an existing one; an `update` import never writes it. That makes one upsert +safe to re-run: master-data columns refresh, while a column the app has moved on since +the first run (a status, a flag) keeps its current value. + +```json +{ + "writeMode": "upsert", "matchFields": ["credit_code"], + "mapping": [ + { "sourceField": "Code", "targetField": "credit_code" }, + { "sourceField": "Name", "targetField": "name" }, + { "sourceField": "Status", "targetField": "status", "applyOn": "create" } + ] +} +``` + +A create-only column is still read for matching: the match key a row was found by stays +in that row's update, with the value it was found by. + +### Rows identified in different ways: ordered key sets + +`matchFields` takes one key set (`["email"]`) or an ordered list of key sets, for a sheet +whose rows are identified in different ways: + +```json +{ "writeMode": "upsert", "matchFields": [["credit_code"], ["country", "registration_no"]] } +``` + +Each row is matched by the **first** set whose fields are all non-blank on that row, and +only by that set: a domestic row with a credit code is matched on it (and created when +nothing matches), an overseas row with a blank credit code is matched on country and +registration number. A row blank on every set fails with `BLANK_MATCH_KEY` instead of +being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set +that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set +does. + --- ## Rejections before any row is read @@ -221,7 +260,7 @@ These are whole-request failures — nothing is written, and there is no per-row | 400 | `UNSUPPORTED_TRANSFORM` | Some artifact entry declares `javascript`, or a transform name the pipeline does not implement; or an inline entry declares `date_format` (it carries no format to apply) or a name outside `none`, `lookup`, `trim`, `uppercase`, `lowercase`. | | 400 | `MISSING_REQUIRED_FIELD` | An inline entry is marked `required: true` and some row leaves its column blank after its `defaultValue`. The error names the column and the rows; it applies to the dry run and the commit alike. | | 400 | `CONFLICTING_MAPPING` | Both `mappingName` and an inline `mapping` were supplied. | -| 400 | `INVALID_REQUEST` | No recognizable payload, or `writeMode` resolved to update/upsert with no `matchFields` from either the request or the artifact. | +| 400 | `INVALID_REQUEST` | No recognizable payload; `writeMode` resolved to update/upsert with no `matchFields` from either the request or the artifact; malformed key sets in `matchFields` (an empty set or name, or names mixed with sets); `skipBlankMatchKey: true` with key sets; or an inline entry's `applyOn` outside `create` / `always`. | | 413 | `PAYLOAD_TOO_LARGE` | More than 5,000 rows on the synchronous route. | --- @@ -236,7 +275,8 @@ fails does **not** stop the import. | `created` | `true` | — | Inserted. | | `updated` | `true` | — | Matched an existing record and updated it. | | `skipped` | `true` | `NO_MATCH` | `writeMode: 'update'` and the match fields matched nothing. Nothing is created. | -| `skipped` | `true` | `BLANK_MATCH_KEY` | The row's match fields are blank. Upsert creates such a row by default; `update` skips it, and `skipBlankMatchKey: true` skips it in either mode. | +| `skipped` | `true` | `BLANK_MATCH_KEY` | One key set in `matchFields`, and a field of it is blank on the row. Upsert creates such a row by default; `update` skips it, and `skipBlankMatchKey: true` skips it in either mode. | +| `failed` | `false` | `BLANK_MATCH_KEY` | Ordered key sets in `matchFields`, and every set has a blank field on the row. Nothing is written for that row. | | `failed` | `false` | `AMBIGUOUS_MATCH` | The match fields matched **more than one** record. Nothing is written for that row. | | `failed` | `false` | a field error code | A cell could not be coerced, or the engine rejected the record. `field` names the column and `error` carries the message. | diff --git a/content/docs/references/api/export.mdx b/content/docs/references/api/export.mdx index ca8a06d964c..f3dcc205120 100644 --- a/content/docs/references/api/export.mdx +++ b/content/docs/references/api/export.mdx @@ -53,13 +53,13 @@ const result = CreateImportJobRequestSchema.parse(data); | **mappingName** | `string` | optional | Name of a registered `mapping` metadata artifact to apply; the server resolves it (org-scoped rows first, then env-wide) and projects columns through it. Mutually exclusive with an inline `mapping` — supplying both is refused (400 CONFLICTING_MAPPING). | | **dryRun** | `boolean` | optional (default: `false`) | Validate + coerce every row without persisting. The verdict is the engine's own write-path validation, with one boundary an author should know: a preview runs NO automations. Hooks never fire in a dry run — a preview that executed user-authored side effects (mail, outbound calls, writes to other objects) would be the retired `validateOnly` defect in a new spelling. So a dry run with `runAutomations: true` can report `required` for a field a `beforeInsert` hook would populate during the real import; for hook-derived fields the real write is authoritative. | | **writeMode** | `Enum<'insert' \| 'update' \| 'upsert'>` | optional (default: `"insert"`) | insert / update / upsert semantics | -| **matchFields** | `string[]` | optional | Fields that identify an existing record (required for update/upsert) | +| **matchFields** | `string[] \| string[][]` | optional | Fields that identify an existing record (required for update/upsert). Either one key set (`['email']`), or an ordered list of key sets (`[['credit_code'], ['country', 'registration_no']]`) for a sheet whose rows are identified in different ways: each row is matched by the first set whose fields are all non-blank on that row, and only by that set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does. | | **runAutomations** | `boolean` | optional (default: `true`) | Fire triggers/hooks for each imported row. ON by default, and opting out must be explicit: automations always ran on import historically (the engine ignored this flag until this flag was honoured), so omitting the key runs them. This matches platform convention (Salesforce fires triggers on import by default). Only the system principal (an in-process caller such as a seed or migration job) may send `runAutomations: false`, because it takes the object's triggers and metadata-bound hooks, its approvals and record freezes among them, off every row: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. One boundary: a `dryRun` preview runs NO automations whatever this flag says. | | **treatAsHistorical** | `boolean` | optional (default: `false`) | Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates; and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them. Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped. Only the system principal (an in-process caller such as a seed or migration job) may send `treatAsHistorical: true`: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. | | **trimWhitespace** | `boolean` | optional (default: `true`) | Trim leading/trailing whitespace from string cells | | **nullValues** | `string[]` | optional | Strings treated as null/blank (besides empty string) | | **createMissingOptions** | `boolean` | optional (default: `false`) | Keep a select / radio / multiselect cell that matches none of the field's options instead of failing the row. The cell is stored as written (trimmed; for a multi-value cell, each unmatched item), and the engine's option check admits exactly the values this import kept, on this import's writes (dry run included). The field's option list is not changed: a later write that sends the field is judged against the options again, while one that leaves the field out is not affected. A field bound to a shared picklist that did not resolve is still refused. | -| **skipBlankMatchKey** | `boolean` | optional (default: `false`) | Skip rows whose matchFields are blank (default: upsert creates them, update skips them) | +| **skipBlankMatchKey** | `boolean` | optional (default: `false`) | Skip rows whose matchFields are blank (default: upsert creates them, update skips them). One key set only: with ordered key sets a row blank on every set is a row error. | ### Nested Shape: `CreateImportJobRequest.mapping[number]` @@ -71,6 +71,7 @@ const result = CreateImportJobRequestSchema.parse(data); | **transform** | `Enum<'none' \| 'uppercase' \| 'lowercase' \| 'trim' \| 'date_format' \| 'lookup'>` | optional (default: `"none"`) | Transformation to apply during mapping | | **defaultValue** | `any` | optional | Default value if source field is null/empty | | **required** | `boolean` | optional (default: `false`) | Whether this field is required (import validation) | +| **applyOn** | `Enum<'create' \| 'always'>` | optional (default: `"always"`) | When an import writes this column. `always` (default) writes it on every row the import creates or updates. `create` writes it only when the row is created: an `upsert` that matches an existing record leaves that record's value as it is, and an `update` import never writes it, so re-running one upsert refreshes the `always` columns without reverting state written since the first run. A create-only column is still read for matching: the match key a row was found by stays in that row's update. | --- @@ -143,6 +144,7 @@ const result = CreateImportJobRequestSchema.parse(data); | **transform** | `Enum<'none' \| 'uppercase' \| 'lowercase' \| 'trim' \| 'date_format' \| 'lookup'>` | optional (default: `"none"`) | Transformation to apply during mapping | | **defaultValue** | `any` | optional | Default value if source field is null/empty | | **required** | `boolean` | optional (default: `false`) | Whether this field is required (import validation) | +| **applyOn** | `Enum<'create' \| 'always'>` | optional (default: `"always"`) | When an import writes this column. `always` (default) writes it on every row the import creates or updates. `create` writes it only when the row is created: an `upsert` that matches an existing record leaves that record's value as it is, and an `update` import never writes it, so re-running one upsert refreshes the `always` columns without reverting state written since the first run. A create-only column is still read for matching: the match key a row was found by stays in that row's update. | --- @@ -159,6 +161,7 @@ const result = CreateImportJobRequestSchema.parse(data); | **transform** | `Enum<'none' \| 'uppercase' \| 'lowercase' \| 'trim' \| 'date_format' \| 'lookup'>` | optional (default: `"none"`) | Transformation to apply during mapping | | **defaultValue** | `any` | optional | Default value if source field is null/empty | | **required** | `boolean` | optional (default: `false`) | Whether this field is required (import validation) | +| **applyOn** | `Enum<'create' \| 'always'>` | optional (default: `"always"`) | When an import writes this column. `always` (default) writes it on every row the import creates or updates. `create` writes it only when the row is created: an `upsert` that matches an existing record leaves that record's value as it is, and an `update` import never writes it, so re-running one upsert refreshes the `always` columns without reverting state written since the first run. A create-only column is still read for matching: the match key a row was found by stays in that row's update. | --- @@ -307,13 +310,13 @@ Type: `{ sourceField: string; targetField: string; targetLabel?: string; transfo | **mappingName** | `string` | optional | Name of a registered `mapping` metadata artifact to apply; the server resolves it (org-scoped rows first, then env-wide) and projects columns through it. Mutually exclusive with an inline `mapping` — supplying both is refused (400 CONFLICTING_MAPPING). | | **dryRun** | `boolean` | optional (default: `false`) | Validate + coerce every row without persisting. The verdict is the engine's own write-path validation, with one boundary an author should know: a preview runs NO automations. Hooks never fire in a dry run — a preview that executed user-authored side effects (mail, outbound calls, writes to other objects) would be the retired `validateOnly` defect in a new spelling. So a dry run with `runAutomations: true` can report `required` for a field a `beforeInsert` hook would populate during the real import; for hook-derived fields the real write is authoritative. | | **writeMode** | `Enum<'insert' \| 'update' \| 'upsert'>` | optional (default: `"insert"`) | insert / update / upsert semantics | -| **matchFields** | `string[]` | optional | Fields that identify an existing record (required for update/upsert) | +| **matchFields** | `string[] \| string[][]` | optional | Fields that identify an existing record (required for update/upsert). Either one key set (`['email']`), or an ordered list of key sets (`[['credit_code'], ['country', 'registration_no']]`) for a sheet whose rows are identified in different ways: each row is matched by the first set whose fields are all non-blank on that row, and only by that set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does. | | **runAutomations** | `boolean` | optional (default: `true`) | Fire triggers/hooks for each imported row. ON by default, and opting out must be explicit: automations always ran on import historically (the engine ignored this flag until this flag was honoured), so omitting the key runs them. This matches platform convention (Salesforce fires triggers on import by default). Only the system principal (an in-process caller such as a seed or migration job) may send `runAutomations: false`, because it takes the object's triggers and metadata-bound hooks, its approvals and record freezes among them, off every row: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. One boundary: a `dryRun` preview runs NO automations whatever this flag says. | | **treatAsHistorical** | `boolean` | optional (default: `false`) | Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates; and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them. Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped. Only the system principal (an in-process caller such as a seed or migration job) may send `treatAsHistorical: true`: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. | | **trimWhitespace** | `boolean` | optional (default: `true`) | Trim leading/trailing whitespace from string cells | | **nullValues** | `string[]` | optional | Strings treated as null/blank (besides empty string) | | **createMissingOptions** | `boolean` | optional (default: `false`) | Keep a select / radio / multiselect cell that matches none of the field's options instead of failing the row. The cell is stored as written (trimmed; for a multi-value cell, each unmatched item), and the engine's option check admits exactly the values this import kept, on this import's writes (dry run included). The field's option list is not changed: a later write that sends the field is judged against the options again, while one that leaves the field out is not affected. A field bound to a shared picklist that did not resolve is still refused. | -| **skipBlankMatchKey** | `boolean` | optional (default: `false`) | Skip rows whose matchFields are blank (default: upsert creates them, update skips them) | +| **skipBlankMatchKey** | `boolean` | optional (default: `false`) | Skip rows whose matchFields are blank (default: upsert creates them, update skips them). One key set only: with ordered key sets a row blank on every set is a row error. | ### Nested Shape: `ImportRequest.mapping[number]` @@ -325,6 +328,7 @@ Type: `{ sourceField: string; targetField: string; targetLabel?: string; transfo | **transform** | `Enum<'none' \| 'uppercase' \| 'lowercase' \| 'trim' \| 'date_format' \| 'lookup'>` | optional (default: `"none"`) | Transformation to apply during mapping | | **defaultValue** | `any` | optional | Default value if source field is null/empty | | **required** | `boolean` | optional (default: `false`) | Whether this field is required (import validation) | +| **applyOn** | `Enum<'create' \| 'always'>` | optional (default: `"always"`) | When an import writes this column. `always` (default) writes it on every row the import creates or updates. `create` writes it only when the row is created: an `upsert` that matches an existing record leaves that record's value as it is, and an `update` import never writes it, so re-running one upsert refreshes the `always` columns without reverting state written since the first run. A create-only column is still read for matching: the match key a row was found by stays in that row's update. | --- diff --git a/packages/core/src/utils/import-runner.ts b/packages/core/src/utils/import-runner.ts index d9b249d961d..8b20cb7e077 100644 --- a/packages/core/src/utils/import-runner.ts +++ b/packages/core/src/utils/import-runner.ts @@ -10,6 +10,7 @@ import { } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { CreateDataRequest, FindDataRequest, GetMetaItemRequest, UpdateDataRequest, ValidateDataIssue, ValidateDataRequest, ValidateDataResponse } from '@objectstack/spec/api'; +import { isMatchKeySets } from '@objectstack/spec/api'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from './bulk-write.js'; import { isUniqueViolationError, uniqueViolationColumn, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage, isSandboxOrigin, isSandboxOwnFault } from '@objectstack/types'; @@ -234,7 +235,19 @@ export interface RunImportOptions { /** Field metadata for value coercion (name→id lookups, select codes, …). */ metaMap: Map; writeMode: 'insert' | 'update' | 'upsert'; - matchFields: string[]; + /** + * One key set (`string[]`), or ordered key sets (`string[][]`): each row is + * matched by the first set whose fields are all non-blank on it, and a row + * blank on every set fails with `BLANK_MATCH_KEY` (`skipBlankMatchKey` is + * not consulted for that form). One key set keeps its own blank handling. + */ + matchFields: string[] | string[][]; + /** + * Target fields written only when a row is created (`FieldMappingEntry.applyOn: + * 'create'`): an update leaves them out of its patch, except the fields of + * the key set the row was matched by. + */ + createOnlyFields?: string[]; dryRun: boolean; runAutomations: boolean; /** #3479 / #3493 — treat rows as established historical facts. The write @@ -665,8 +678,23 @@ export function runImport(opts: RunImportOptions): Promise { p, objectName, environmentId, context, rows, metaMap, translate: messageTranslator, writeMode, matchFields, dryRun, runAutomations, treatAsHistorical, trimWhitespace, nullValues, createMissingOptions, skipBlankMatchKey, - onProgress, shouldCancel, captureUndo, + onProgress, shouldCancel, captureUndo, createOnlyFields, } = opts; + // One key set is the one-element case of ordered key sets, except for a row + // blank on it: only the ordered form reports that row as an error. + const blankKeyIsRowError = isMatchKeySets(matchFields); + const keySets: string[][] = blankKeyIsRowError ? matchFields : [matchFields as string[]]; + const createOnly = new Set(createOnlyFields ?? []); + // The patch an update writes: the row minus its create-only columns. The + // key set the row was matched by stays, as the value it was found by. + const updatePatch = (data: Record, keySet: readonly string[]): Record => { + if (createOnly.size === 0) return data; + const patch: Record = {}; + for (const [k, v] of Object.entries(data)) { + if (!createOnly.has(k) || keySet.includes(k)) patch[k] = v; + } + return patch; + }; const collectUndo = !!captureUndo && !dryRun; const undoLog: ImportUndoLog = { created: [], updated: [] }; // Snapshot only the fields the import touched, so undo restores exactly what @@ -778,26 +806,30 @@ export function runImport(opts: RunImportOptions): Promise { return match; }; - // Locate an existing record for update/upsert by matchFields. Returns the - // record, or a sentinel: 'blank' (a match field was empty), 'none' (no - // match), 'ambiguous' (>1 match — too risky to update). + // Locate an existing record for update/upsert by matchFields, using the + // first key set whose fields are all non-blank on the row (and only that + // set: a miss on it is 'none', not a reason to try the next). Returns the + // record, or a sentinel: 'blank' (every key set has an empty field), 'none' + // (no match), 'ambiguous' (>1 match — too risky to update); and the key set + // the row was matched by. const findExisting = async ( data: Record, - ): Promise | 'blank' | 'none' | 'ambiguous'> => { - const filter: Record = {}; - for (const f of matchFields) { + ): Promise<{ match: Record | 'blank' | 'none' | 'ambiguous'; keySet: string[] }> => { + const keySet = keySets.find((set) => set.every((f) => { const v = data[f]; - if (v === undefined || v === null || v === '') return 'blank'; - filter[f] = v; - } + return !(v === undefined || v === null || v === ''); + })); + if (!keySet) return { match: 'blank', keySet: [] }; + const filter: Record = {}; + for (const f of keySet) filter[f] = data[f]; const r = await p.findData(findArgsBase({ object: objectName, query: { object: objectName, where: filter, limit: 2 }, })); const recs = findRows(r); - if (recs.length === 0) return 'none'; - if (recs.length > 1) return 'ambiguous'; - return recs[0]; + if (recs.length === 0) return { match: 'none', keySet }; + if (recs.length > 1) return { match: 'ambiguous', keySet }; + return { match: recs[0], keySet }; }; const writeCtx = { @@ -1137,13 +1169,23 @@ export function runImport(opts: RunImportOptions): Promise { const rowCtx = writeCtxFor([keptOptionValues]); // 2. Decide create vs update vs skip. let existing: Record | 'blank' | 'none' | 'ambiguous' = 'none'; + let matchedKeySet: string[] = []; let handled = false; if (writeMode !== 'insert') { - existing = await findExisting(data); + ({ match: existing, keySet: matchedKeySet } = await findExisting(data)); if (existing === 'ambiguous') { errCount++; results[i] = { row: rowNo, ok: false, action: 'failed', code: 'AMBIGUOUS_MATCH', error: `matchFields matched more than one ${objectName} record` }; handled = true; + } else if (existing === 'blank' && blankKeyIsRowError) { + // Ordered key sets: a row no set can identify is an error, never + // a silent create or skip. + errCount++; + results[i] = { + row: rowNo, ok: false, action: 'failed', code: 'BLANK_MATCH_KEY', + error: `The row has no match key: every key set in matchFields (${keySets.map((set) => set.join(' + ')).join('; ')}) has a blank field on it`, + }; + handled = true; } else if (existing === 'blank' && (skipBlankMatchKey || writeMode === 'update')) { // Blank match key: skip when asked, else fall through to create. skipped++; @@ -1175,7 +1217,7 @@ export function runImport(opts: RunImportOptions): Promise { // protocol: the id is the address every update door already // carries in the payload. const verdict = willUpdate - ? await previewVerdict({ ...data, id: (existing as Record).id }, 'update', rowCtx) + ? await previewVerdict({ ...updatePatch(data, matchedKeySet), id: (existing as Record).id }, 'update', rowCtx) : await previewVerdict(data, 'insert', rowCtx); if (verdict && !verdict.valid) { errCount++; @@ -1199,11 +1241,12 @@ export function runImport(opts: RunImportOptions): Promise { } } else if (willUpdate) { const target = existing as Record; + const patch = updatePatch(data, matchedKeySet); let res2: unknown; let updateSummaryStale = false; let updateDrops: readonly DroppedFieldsEvent[] | undefined; try { - res2 = await withTransientRetry(() => p.updateData({ object: objectName, id: target.id, data, context: rowCtx, ...(environmentId ? { environmentId } : {}) })); + res2 = await withTransientRetry(() => p.updateData({ object: objectName, id: target.id, data: patch, context: rowCtx, ...(environmentId ? { environmentId } : {}) })); updateDrops = responseDrops(res2); // [#20701] read off the response, never off a recovered record } catch (e) { // Record updated but summary recompute failed (framework#3147): @@ -1215,7 +1258,7 @@ export function runImport(opts: RunImportOptions): Promise { const id = extractRecordId(res2) ?? String(target.id); okCount++; updated++; if (collectUndo && target.id != null) { - undoLog.updated.push({ id: String(target.id), before: captureBefore(target, data) }); + undoLog.updated.push({ id: String(target.id), before: captureBefore(target, patch) }); } results[i] = { row: rowNo, ok: true, action: 'updated', id, ...(updateSummaryStale ? { code: 'SUMMARY_RECOMPUTE_FAILED' } : {}), diff --git a/packages/rest/src/import-apply-on-match-key-sets.test.ts b/packages/rest/src/import-apply-on-match-key-sets.test.ts new file mode 100644 index 00000000000..33c4bb4685c --- /dev/null +++ b/packages/rest/src/import-apply-on-match-key-sets.test.ts @@ -0,0 +1,313 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Two import contract keys, driven through the REAL `POST /data/:object/import` + * route on a real ObjectQL engine + protocol (sqlite `:memory:`), as + * `import-inline-mapping-entry.test.ts` is: + * + * - [#25577] `FieldMappingEntry.applyOn: 'create'` — a create-only column is + * written when the row is created and left alone when an upsert updates an + * existing row, so one upsert re-run refreshes master data without + * reverting state written since the first run. + * - [#25578] `matchFields` as ordered key sets (`string[][]`) — each row is + * matched by the first set whose fields are all non-blank, and a row blank + * on every set is a row error, not a silent create or skip. One key set + * (`string[]`) keeps its own blank handling. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core'; +import { RestServer } from './rest-server'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const SUPPLIER = { + name: 'supplier', label: 'Supplier', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + status: { name: 'status', type: 'text' as const, label: 'Status' }, + credit_code: { name: 'credit_code', type: 'text' as const, label: 'Credit code' }, + country: { name: 'country', type: 'text' as const, label: 'Country' }, + registration_no: { name: 'registration_no', type: 'text' as const, label: 'Registration no.' }, + }, +}; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + }), true); + await engine.init(); + engine.registry.registerObject(SUPPLIER as any); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + for (const o of [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) { + if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as any); + } + const protocol = new ObjectStackProtocolImplementation(engine as any); + const noop = () => {}; + const server = { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; + const rest = new RestServer(server as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const route = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/import'); + expect(route).toBeDefined(); + return { engine, route }; +} + +describe('import route — applyOn and ordered match key sets (#25577, #25578)', () => { + let engine: any; + let route: any; + const call = async (body: any) => { + const res = makeRes(); + await route.handler({ params: { object: 'supplier' }, body } as any, res); + return res; + }; + const byCode = async (credit_code: string) => (await engine.find('supplier', { where: { credit_code } }))[0]; + + beforeEach(async () => { + ({ engine, route } = await boot()); + }); + + describe('applyOn (#25577)', () => { + const mapping = [ + { sourceField: 'Code', targetField: 'credit_code' }, + { sourceField: 'Name', targetField: 'name' }, + { sourceField: 'Status', targetField: 'status', applyOn: 'create' }, + ]; + + it('one upsert run twice: the create-only column keeps the state written since, the always column refreshes', async () => { + const first = await call({ + format: 'csv', csv: 'Code,Name,Status\nC1,Acme,qualified\n', + mapping, writeMode: 'upsert', matchFields: ['credit_code'], + }); + expect(first._json).toMatchObject({ ok: 1, created: 1, errors: 0 }); + expect(await byCode('C1')).toMatchObject({ name: 'Acme', status: 'qualified' }); + + // The app moves the supplier on between the two runs. + const stored = await byCode('C1'); + await engine.update('supplier', { id: stored.id, status: 'frozen' }); + + const second = await call({ + format: 'csv', csv: 'Code,Name,Status\nC1,Acme Ltd,qualified\nC2,Beta,qualified\n', + mapping, writeMode: 'upsert', matchFields: ['credit_code'], + }); + expect(second._json).toMatchObject({ ok: 2, created: 1, updated: 1, errors: 0 }); + expect(await byCode('C1')).toMatchObject({ name: 'Acme Ltd', status: 'frozen' }); + // A row the second run creates gets its create-only column. + expect(await byCode('C2')).toMatchObject({ name: 'Beta', status: 'qualified' }); + }); + + it('update mode never writes a create-only column; the dry run previews the same rows', async () => { + await engine.insert('supplier', { id: 's1', credit_code: 'C1', name: 'Acme', status: 'frozen' }); + const body = { + format: 'csv', csv: 'Code,Name,Status\nC1,Acme Ltd,qualified\n', + mapping, writeMode: 'update', matchFields: ['credit_code'], + }; + const dry = await call({ ...body, dryRun: true }); + expect(dry._json).toMatchObject({ ok: 1, updated: 1, errors: 0 }); + const res = await call(body); + expect(res._json).toMatchObject({ ok: 1, updated: 1, errors: 0 }); + expect(await byCode('C1')).toMatchObject({ name: 'Acme Ltd', status: 'frozen' }); + }); + + it('a create-only match-key column still matches, and the update keeps the key it was found by', async () => { + await engine.insert('supplier', { id: 's1', credit_code: 'C1', name: 'Acme', status: 'frozen' }); + const res = await call({ + format: 'csv', csv: 'Code,Name\nC1,Acme Ltd\n', + mapping: [ + { sourceField: 'Code', targetField: 'credit_code', applyOn: 'create' }, + { sourceField: 'Name', targetField: 'name' }, + ], + writeMode: 'upsert', matchFields: ['credit_code'], + }); + expect(res._json).toMatchObject({ ok: 1, updated: 1, created: 0 }); + expect(await engine.find('supplier', {})).toHaveLength(1); + expect(await byCode('C1')).toMatchObject({ id: 's1', name: 'Acme Ltd' }); + }); + + it('control: applyOn always (and absent) writes the column on update, as before', async () => { + await engine.insert('supplier', { id: 's1', credit_code: 'C1', name: 'Acme', status: 'frozen' }); + const res = await call({ + format: 'csv', csv: 'Code,Name,Status\nC1,Acme Ltd,qualified\n', + mapping: [ + { sourceField: 'Code', targetField: 'credit_code' }, + { sourceField: 'Name', targetField: 'name', applyOn: 'always' }, + { sourceField: 'Status', targetField: 'status' }, + ], + writeMode: 'upsert', matchFields: ['credit_code'], + }); + expect(res._json).toMatchObject({ ok: 1, updated: 1 }); + expect(await byCode('C1')).toMatchObject({ name: 'Acme Ltd', status: 'qualified' }); + }); + + it('an applyOn value outside create / always refuses the request with 400 INVALID_REQUEST, and nothing is written', async () => { + const res = await call({ + format: 'csv', csv: 'Code,Status\nC1,qualified\n', + mapping: [ + { sourceField: 'Code', targetField: 'credit_code' }, + { sourceField: 'Status', targetField: 'status', applyOn: 'insert' }, + ], + writeMode: 'upsert', matchFields: ['credit_code'], + }); + expect(res._status).toBe(400); + expect(res._json.code).toBe('INVALID_REQUEST'); + expect(await engine.find('supplier', {})).toHaveLength(0); + }); + }); + + describe('ordered match key sets (#25578)', () => { + const keySets = [['credit_code'], ['country', 'registration_no']]; + + it('a mixed sheet matches domestic rows on set 1 and overseas rows on set 2 in one import', async () => { + await engine.insert('supplier', { id: 'dom', credit_code: 'C1', name: 'Domestic' }); + await engine.insert('supplier', { id: 'ovs', country: 'DE', registration_no: 'HRB1', name: 'Overseas' }); + const res = await call({ + format: 'json', writeMode: 'upsert', matchFields: keySets, + rows: [ + { credit_code: 'C1', name: 'Domestic Ltd' }, + { country: 'DE', registration_no: 'HRB1', name: 'Overseas GmbH' }, + { country: 'FR', registration_no: 'RCS9', name: 'New Overseas' }, + ], + }); + expect(res._json).toMatchObject({ ok: 3, updated: 2, created: 1, errors: 0 }); + expect(res._json.results.map((r: any) => r.action)).toEqual(['updated', 'updated', 'created']); + const all = await engine.find('supplier', {}); + expect(all).toHaveLength(3); + expect(all.find((r: any) => r.id === 'dom')).toMatchObject({ name: 'Domestic Ltd' }); + expect(all.find((r: any) => r.id === 'ovs')).toMatchObject({ name: 'Overseas GmbH' }); + + // Re-running the same sheet creates nothing: it is idempotent across both kinds of row. + const again = await call({ + format: 'json', writeMode: 'upsert', matchFields: keySets, + rows: [ + { credit_code: 'C1', name: 'Domestic Ltd' }, + { country: 'DE', registration_no: 'HRB1', name: 'Overseas GmbH' }, + { country: 'FR', registration_no: 'RCS9', name: 'New Overseas' }, + ], + }); + expect(again._json).toMatchObject({ ok: 3, updated: 3, created: 0 }); + expect(await engine.find('supplier', {})).toHaveLength(3); + }); + + it('a row is matched by the first non-blank set only: a miss on it creates, it never falls through to the next set', async () => { + await engine.insert('supplier', { id: 'ovs', country: 'DE', registration_no: 'HRB1', name: 'Overseas' }); + const res = await call({ + format: 'json', writeMode: 'upsert', matchFields: keySets, + rows: [{ credit_code: 'C9', country: 'DE', registration_no: 'HRB1', name: 'Has a credit code' }], + }); + expect(res._json).toMatchObject({ ok: 1, created: 1, updated: 0 }); + expect(await engine.find('supplier', {})).toHaveLength(2); + }); + + it('a row blank on every set is a row error BLANK_MATCH_KEY, on the dry run and the commit alike; the other rows import', async () => { + const body = { + format: 'json', writeMode: 'upsert', matchFields: keySets, + rows: [ + { credit_code: 'C1', name: 'Domestic' }, + { country: 'DE', name: 'No registration no.' }, + ], + }; + const dry = await call({ ...body, dryRun: true }); + expect(dry._json).toMatchObject({ ok: 1, errors: 1, created: 1, skipped: 0 }); + expect(dry._json.results[1]).toMatchObject({ row: 2, ok: false, action: 'failed', code: 'BLANK_MATCH_KEY' }); + + const res = await call(body); + expect(res._json).toMatchObject({ ok: 1, errors: 1, created: 1, skipped: 0 }); + expect(res._json.results[1]).toMatchObject({ row: 2, ok: false, action: 'failed', code: 'BLANK_MATCH_KEY' }); + expect(await engine.find('supplier', {})).toHaveLength(1); + }); + + it('a set that matches more than one record fails the row with AMBIGUOUS_MATCH, as one key set does', async () => { + await engine.insert('supplier', { id: 'a', country: 'DE', registration_no: 'HRB1', name: 'One' }); + await engine.insert('supplier', { id: 'b', country: 'DE', registration_no: 'HRB1', name: 'Two' }); + const res = await call({ + format: 'json', writeMode: 'upsert', matchFields: keySets, + rows: [{ country: 'DE', registration_no: 'HRB1', name: 'Which?' }], + }); + expect(res._json).toMatchObject({ ok: 0, errors: 1 }); + expect(res._json.results[0]).toMatchObject({ ok: false, action: 'failed', code: 'AMBIGUOUS_MATCH' }); + }); + + it('skipBlankMatchKey with key sets refuses the request with 400 INVALID_REQUEST before any row', async () => { + const res = await call({ + format: 'json', writeMode: 'upsert', matchFields: keySets, skipBlankMatchKey: true, + rows: [{ credit_code: 'C1', name: 'Domestic' }], + }); + expect(res._status).toBe(400); + expect(res._json.code).toBe('INVALID_REQUEST'); + expect(await engine.find('supplier', {})).toHaveLength(0); + }); + + it.each([ + ['an empty set', [['credit_code'], []]], + ['an empty name', [['credit_code'], ['country', '']]], + ['a non-string name', [['credit_code'], ['country', 7]]], + ['names mixed with sets', ['credit_code', ['country', 'registration_no']]], + ])('malformed key sets (%s) refuse the request with 400 INVALID_REQUEST', async (_label, matchFields) => { + const res = await call({ + format: 'json', writeMode: 'upsert', matchFields, + rows: [{ credit_code: 'C1', name: 'Domestic' }], + }); + expect(res._status).toBe(400); + expect(res._json.code).toBe('INVALID_REQUEST'); + expect(await engine.find('supplier', {})).toHaveLength(0); + }); + + it('control: one key set keeps its blank handling — upsert creates a blank-key row, update and skipBlankMatchKey skip it', async () => { + const rows = [{ name: 'No credit code' }]; + const upsert = await call({ format: 'json', writeMode: 'upsert', matchFields: ['credit_code'], rows }); + expect(upsert._json).toMatchObject({ ok: 1, created: 1, errors: 0 }); + + const update = await call({ format: 'json', writeMode: 'update', matchFields: ['credit_code'], rows }); + expect(update._json.results[0]).toMatchObject({ ok: true, action: 'skipped', code: 'BLANK_MATCH_KEY' }); + + const skip = await call({ format: 'json', writeMode: 'upsert', matchFields: ['credit_code'], skipBlankMatchKey: true, rows }); + expect(skip._json.results[0]).toMatchObject({ ok: true, action: 'skipped', code: 'BLANK_MATCH_KEY' }); + expect(await engine.find('supplier', {})).toHaveLength(1); + }); + }); + + it('both keys in one sheet: key sets pick the record, applyOn keeps its create-only column', async () => { + await engine.insert('supplier', { id: 'ovs', country: 'DE', registration_no: 'HRB1', name: 'Overseas', status: 'frozen' }); + const res = await call({ + format: 'json', writeMode: 'upsert', matchFields: [['credit_code'], ['country', 'registration_no']], + mapping: [ + { sourceField: 'credit_code', targetField: 'credit_code' }, + { sourceField: 'country', targetField: 'country' }, + { sourceField: 'registration_no', targetField: 'registration_no' }, + { sourceField: 'name', targetField: 'name' }, + { sourceField: 'status', targetField: 'status', applyOn: 'create' }, + ], + rows: [ + { country: 'DE', registration_no: 'HRB1', name: 'Overseas GmbH', status: 'qualified' }, + { credit_code: 'C1', name: 'Domestic', status: 'qualified' }, + ], + }); + expect(res._json).toMatchObject({ ok: 2, updated: 1, created: 1, errors: 0 }); + const all = await engine.find('supplier', {}); + expect(all.find((r: any) => r.id === 'ovs')).toMatchObject({ name: 'Overseas GmbH', status: 'frozen' }); + expect(all.find((r: any) => r.credit_code === 'C1')).toMatchObject({ name: 'Domestic', status: 'qualified' }); + }); +}); diff --git a/packages/rest/src/import-prepare.ts b/packages/rest/src/import-prepare.ts index c170dc38857..200ca8bf0b7 100644 --- a/packages/rest/src/import-prepare.ts +++ b/packages/rest/src/import-prepare.ts @@ -18,6 +18,7 @@ import { } from './import-mapping.js'; import { asXlsxLoadInput, loadExcelJs } from './xlsx-module.js'; import { isBlank } from '@objectstack/core'; +import { isMatchKeySets } from '@objectstack/spec/api'; /** * Minimal RFC-4180-style CSV parser used by the bulk-import endpoint @@ -301,12 +302,39 @@ function applyInlineEntryRules( }; } +/** + * [#25578] Read a request's `matchFields`. One key set (`string[]`) is read + * as it always was, non-string and empty names dropped. Ordered key sets + * (`string[][]`) are read strictly: every set non-empty and every name a + * non-empty string, and an array mixing names with sets is refused, because + * a dropped set or name would silently change which record a row matches. + */ +function readMatchFieldsBody(raw: unknown): { ok: true; matchFields: string[] | string[][] } | ImportRefusal { + if (!Array.isArray(raw)) return { ok: true, matchFields: [] }; + if (!raw.some((x) => Array.isArray(x))) { + return { ok: true, matchFields: raw.filter((f: any) => typeof f === 'string' && f.length > 0) }; + } + const malformed = !isMatchKeySets(raw) + || raw.some((set) => set.length === 0 || set.some((f: unknown) => typeof f !== 'string' || f.length === 0)); + if (malformed) { + return { + ok: false, status: 400, code: 'INVALID_REQUEST', + error: 'matchFields is one key set of field names (["email"]) or ordered key sets, each a non-empty list of field names ' + + '([["credit_code"], ["country", "registration_no"]]); it cannot mix names with sets or carry an empty set or name.', + }; + } + return { ok: true, matchFields: raw as string[][] }; +} + /** Everything the import runner needs, parsed & validated from a request body. */ export interface PreparedImport { rows: Array>; metaMap: Map; writeMode: 'insert' | 'update' | 'upsert'; - matchFields: string[]; + /** One key set, or ordered key sets (first all-non-blank set per row). */ + matchFields: string[] | string[][]; + /** Targets of the inline entries declaring `applyOn: 'create'`. */ + createOnlyFields: string[]; dryRun: boolean; runAutomations: boolean; /** #3479 — import established historical facts: skip the state_machine rule @@ -503,9 +531,12 @@ export async function prepareImportRequest( let writeMode: 'insert' | 'update' | 'upsert' = body?.writeMode === 'update' || body?.writeMode === 'upsert' ? body.writeMode : 'insert'; - let matchFields: string[] = Array.isArray(body?.matchFields) - ? body.matchFields.filter((f: any) => typeof f === 'string' && f.length > 0) - : []; + // `matchFields` is one key set (`string[]`, read as it always was) or + // ordered key sets (`string[][]`), which are read strictly: a malformed + // set would change which rows match, so it refuses the request. + const readMatchFields = readMatchFieldsBody(body?.matchFields); + if (!readMatchFields.ok) return readMatchFields; + let matchFields: string[] | string[][] = readMatchFields.matchFields; // The two control flags, read where the door's gate reads them (#24348): // the import doors refuse either opt-out from any caller but the system // principal BEFORE this parse (`importControlOptOutRefusal`), so a value @@ -558,6 +589,13 @@ export async function prepareImportRequest( if (writeMode !== 'insert' && matchFields.length === 0) { return { ok: false, status: 400, code: 'INVALID_REQUEST', error: `writeMode "${writeMode}" requires a non-empty matchFields[]` }; } + if (skipBlankMatchKey && isMatchKeySets(matchFields)) { + return { + ok: false, status: 400, code: 'INVALID_REQUEST', + error: 'skipBlankMatchKey applies to one key set only. With ordered key sets in matchFields, a row blank on every set ' + + 'is reported as a row error (BLANK_MATCH_KEY) rather than skipped: drop skipBlankMatchKey, or give matchFields one key set.', + }; + } // Normalize `mapping` to a `{ sourceColumn: targetField }` record. Accepts // either that compact form or a FieldMappingEntry[]. An entry's own @@ -565,6 +603,9 @@ export async function prepareImportRequest( // beside the rename and applied once the rows are built (#25491). const mapping: Record = {}; const entryRules: InlineEntryRule[] = []; + // [#25577] Targets of the entries declaring `applyOn: 'create'`: written + // when the runner creates the row, left out of the patch when it updates one. + const createOnlyFields: string[] = []; if (Array.isArray(body?.mapping)) { for (const e of body.mapping) { if (e && typeof e.sourceField === 'string' && typeof e.targetField === 'string') { @@ -572,6 +613,14 @@ export async function prepareImportRequest( const read = readInlineEntryRule(e); if (!read.ok) return read; if (read.rule) entryRules.push(read.rule); + if (e.applyOn !== undefined && e.applyOn !== 'always' && e.applyOn !== 'create') { + return { + ok: false, status: 400, code: 'INVALID_REQUEST', + error: `Mapping entry "${e.sourceField}" to "${e.targetField}" declares applyOn ${JSON.stringify(e.applyOn)}; ` + + 'applyOn is "always" (the default: written on create and update) or "create" (written only when the row is created).', + }; + } + if (e.applyOn === 'create' && !createOnlyFields.includes(e.targetField)) createOnlyFields.push(e.targetField); } } } else if (body?.mapping && typeof body.mapping === 'object') { @@ -676,7 +725,7 @@ export async function prepareImportRequest( return { ok: true, prepared: { - rows, metaMap, writeMode, matchFields, dryRun, runAutomations, + rows, metaMap, writeMode, matchFields, createOnlyFields, dryRun, runAutomations, treatAsHistorical, trimWhitespace, nullValues, createMissingOptions, skipBlankMatchKey, }, diff --git a/packages/spec/api-surface/api.json b/packages/spec/api-surface/api.json index 0b319049b8b..ef27f11c6bd 100644 --- a/packages/spec/api-surface/api.json +++ b/packages/spec/api-surface/api.json @@ -1105,6 +1105,7 @@ "getDefaultRouteRegistrations (function)", "identityFreeEndpointGateFailure (function)", "isAppPackageIdKey (function)", + "isMatchKeySets (function)", "isSubscribableChannel (function)", "makeApiErrorSchema (function)", "normalizeEndpointPath (function)", diff --git a/packages/spec/authorable-defaults/api.json b/packages/spec/authorable-defaults/api.json index 73937aefe52..6e490e29e20 100644 --- a/packages/spec/authorable-defaults/api.json +++ b/packages/spec/authorable-defaults/api.json @@ -60,6 +60,7 @@ "api/ErrorHandlingConfig:includeStackTrace = false", "api/ErrorHandlingConfig:includeTimestamp = true", "api/ErrorHandlingConfig:logErrors = true", + "api/FieldMappingEntry:applyOn = \"always\"", "api/FieldMappingEntry:required = false", "api/FieldMappingEntry:transform = \"none\"", "api/GetMetaItemCachedResponse:notModified = false", diff --git a/packages/spec/authorable-surface/api.json b/packages/spec/authorable-surface/api.json index 50721d89bc2..e08316adb4b 100644 --- a/packages/spec/authorable-surface/api.json +++ b/packages/spec/authorable-surface/api.json @@ -648,6 +648,7 @@ "api/FieldError:label", "api/FieldError:message", "api/FieldError:value", + "api/FieldMappingEntry:applyOn", "api/FieldMappingEntry:defaultValue", "api/FieldMappingEntry:required", "api/FieldMappingEntry:sourceField", diff --git a/packages/spec/export-origins/api.json b/packages/spec/export-origins/api.json index b02214c6afe..3852cf7e674 100644 --- a/packages/spec/export-origins/api.json +++ b/packages/spec/export-origins/api.json @@ -1059,6 +1059,7 @@ "getDefaultRouteRegistrations": "src/api/plugin-rest-api.zod.ts#getDefaultRouteRegistrations (function)", "identityFreeEndpointGateFailure": "src/api/endpoint-publish-gate.ts#identityFreeEndpointGateFailure (function)", "isAppPackageIdKey": "src/api/protocol.zod.ts#isAppPackageIdKey (function)", + "isMatchKeySets": "src/api/export.zod.ts#isMatchKeySets (function)", "isSubscribableChannel": "src/api/discovery.zod.ts#isSubscribableChannel (function)", "makeApiErrorSchema": "src/api/contract.zod.ts#makeApiErrorSchema (function)", "normalizeEndpointPath": "src/api/endpoint.zod.ts#normalizeEndpointPath (function)", diff --git a/packages/spec/src/api/export.test.ts b/packages/spec/src/api/export.test.ts index 26646e88e35..2afddab094b 100644 --- a/packages/spec/src/api/export.test.ts +++ b/packages/spec/src/api/export.test.ts @@ -12,6 +12,7 @@ import { CreateImportJobRequestSchema, ImportRowResultSchema, ImportJobResultsSchema, + isMatchKeySets, } from './export.zod'; import type { ValidateDataIssue } from './protocol.zod'; import { DroppedFieldsEventSchema } from '../data/data-engine.zod'; @@ -476,3 +477,77 @@ describe('ImportRowResultSchema — droppedFields and warnings', () => { expectTypeOf[number]>().toEqualTypeOf(); }); }); + +// ========================================== +// applyOn (create-only import columns) and ordered match key sets +// ========================================== + +describe('FieldMappingEntrySchema — applyOn', () => { + it('defaults to always, so an entry without it is written on create and update as before', () => { + expect(FieldMappingEntrySchema.parse({ sourceField: 'a', targetField: 'b' }).applyOn).toBe('always'); + }); + + it.each(['create', 'always'] as const)('accepts applyOn %s, with no unrecognized key', (applyOn) => { + const r = FieldMappingEntrySchema.safeParse({ sourceField: 'Status', targetField: 'status', applyOn }); + expect(r.success).toBe(true); + expect(r.data?.applyOn).toBe(applyOn); + }); + + it('refuses a value outside create / always at the applyOn path', () => { + const r = FieldMappingEntrySchema.safeParse({ sourceField: 'a', targetField: 'b', applyOn: 'insert' }); + expect(r.success).toBe(false); + expect(r.error?.issues[0]).toMatchObject({ code: 'invalid_value', path: ['applyOn'] }); + }); + + it('rides the inline import mapping', () => { + const r = ImportRequestSchema.safeParse({ + format: 'csv', csv: 'Code,Status\nC1,qualified\n', writeMode: 'upsert', matchFields: ['credit_code'], + mapping: [{ sourceField: 'Code', targetField: 'credit_code' }, { sourceField: 'Status', targetField: 'status', applyOn: 'create' }], + }); + expect(r.success).toBe(true); + }); +}); + +describe('ImportRequestSchema — matchFields as ordered key sets', () => { + const base = { format: 'json' as const, rows: [], writeMode: 'upsert' as const }; + + it('one key set is unchanged: parsed as written', () => { + const r = ImportRequestSchema.safeParse({ ...base, matchFields: ['credit_code'] }); + expect(r.success).toBe(true); + expect(r.data?.matchFields).toEqual(['credit_code']); + expect(isMatchKeySets(r.data?.matchFields)).toBe(false); + }); + + it('ordered key sets parse, in order', () => { + const matchFields = [['credit_code'], ['country', 'registration_no']]; + const r = ImportRequestSchema.safeParse({ ...base, matchFields }); + expect(r.success).toBe(true); + expect(r.data?.matchFields).toEqual(matchFields); + expect(isMatchKeySets(r.data?.matchFields)).toBe(true); + }); + + it.each([ + ['an empty set', [['credit_code'], []]], + ['an empty list of sets', [[]]], + ['names mixed with sets', ['credit_code', ['country']]], + ])('refuses %s at the matchFields path', (_label, matchFields) => { + const r = ImportRequestSchema.safeParse({ ...base, matchFields }); + expect(r.success).toBe(false); + expect(r.error?.issues[0]?.path[0]).toBe('matchFields'); + }); + + it('refuses skipBlankMatchKey with key sets at the skipBlankMatchKey path; one key set keeps it', () => { + const sets = ImportRequestSchema.safeParse({ ...base, matchFields: [['credit_code']], skipBlankMatchKey: true }); + expect(sets.success).toBe(false); + expect(sets.error?.issues[0]).toMatchObject({ code: 'custom', path: ['skipBlankMatchKey'] }); + + const one = ImportRequestSchema.safeParse({ ...base, matchFields: ['credit_code'], skipBlankMatchKey: true }); + expect(one.success).toBe(true); + }); + + it('the typed SDK request can express both spellings', () => { + const one: ImportRequest = { matchFields: ['email'] }; + const sets: ImportRequest = { matchFields: [['credit_code'], ['country', 'registration_no']] }; + expect([one, sets]).toHaveLength(2); + }); +}); diff --git a/packages/spec/src/api/export.zod.ts b/packages/spec/src/api/export.zod.ts index c6730e00d13..e86924ddb38 100644 --- a/packages/spec/src/api/export.zod.ts +++ b/packages/spec/src/api/export.zod.ts @@ -170,6 +170,15 @@ export const FieldMappingEntrySchema = lazySchema(() => z.object({ .describe('Default value if source field is null/empty'), required: z.boolean().default(false) .describe('Whether this field is required (import validation)'), + applyOn: z.enum(['create', 'always']).default('always') + .describe( + 'When an import writes this column. `always` (default) writes it on every row the import ' + + 'creates or updates. `create` writes it only when the row is created: an `upsert` that ' + + 'matches an existing record leaves that record\'s value as it is, and an `update` import ' + + 'never writes it, so re-running one upsert refreshes the `always` columns without ' + + 'reverting state written since the first run. A create-only column is still read for ' + + 'matching: the match key a row was found by stays in that row\'s update.', + ), })); export type FieldMappingEntry = z.input; /** Post-parse shape of {@link FieldMappingEntry} — defaults applied, transforms run (ADR-0122). */ @@ -282,8 +291,19 @@ export const ImportRequestSchema = lazySchema(() => z.object({ ), writeMode: ImportWriteMode.default('insert') .describe('insert / update / upsert semantics'), - matchFields: z.array(z.string()).optional() - .describe('Fields that identify an existing record (required for update/upsert)'), + matchFields: z.union([ + z.array(z.string()), + z.array(z.array(z.string()).min(1)).min(1), + ]).optional() + .describe( + 'Fields that identify an existing record (required for update/upsert). Either one key set ' + + '(`[\'email\']`), or an ordered list of key sets (`[[\'credit_code\'], [\'country\', ' + + '\'registration_no\']]`) for a sheet whose rows are identified in different ways: each row ' + + 'is matched by the first set whose fields are all non-blank on that row, and only by that ' + + 'set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or ' + + 'skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more ' + + 'than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does.', + ), runAutomations: z.boolean().default(true) .describe( 'Fire triggers/hooks for each imported row. ON by default, and opting out must be ' + @@ -312,7 +332,7 @@ export const ImportRequestSchema = lazySchema(() => z.object({ 'to a shared picklist that did not resolve is still refused.', ), skipBlankMatchKey: z.boolean().default(false) - .describe('Skip rows whose matchFields are blank (default: upsert creates them, update skips them)'), + .describe('Skip rows whose matchFields are blank (default: upsert creates them, update skips them). One key set only: with ordered key sets a row blank on every set is a row error.'), }).refine((body) => !(body.mappingName && body.mapping), { // Same exclusion the route enforces (400 CONFLICTING_MAPPING in // `packages/rest/src/import-prepare.ts`) — surfaced at authoring time for @@ -321,7 +341,22 @@ export const ImportRequestSchema = lazySchema(() => z.object({ // callers having used this schema. message: 'Provide either mappingName or an inline mapping, not both', path: ['mappingName'], +}).refine((body) => !(body.skipBlankMatchKey && isMatchKeySets(body.matchFields)), { + // Same refusal the route answers (400 INVALID_REQUEST in + // `packages/rest/src/import-prepare.ts`): ordered key sets report a row + // blank on every set as a row error, so a request asking to skip such rows + // silently contradicts the form it chose. + message: 'skipBlankMatchKey applies to one key set only; with ordered key sets a row blank on every set is reported as a row error', + path: ['skipBlankMatchKey'], })); + +/** + * Whether an import's `matchFields` uses the ordered key-set form + * (`string[][]`) rather than one key set (`string[]`). + */ +export function isMatchKeySets(matchFields: unknown): matchFields is string[][] { + return Array.isArray(matchFields) && matchFields.length > 0 && matchFields.every((set) => Array.isArray(set)); +} export type ImportRequest = z.input; /** Post-parse shape of {@link ImportRequest} — defaults applied, transforms run (ADR-0122). */ export type ImportRequestParsed = z.infer;