From a286df164ceefa11b03a3d536d9fb3b6d489376d Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 11:47:20 +0200 Subject: [PATCH 1/8] =?UTF-8?q?feat(lookup):=20lookupByField=20=E2=80=94?= =?UTF-8?q?=20tri-state=20find-by-field=20(refuse-on-ambiguous)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/lookup.js | 144 +++++++++++++++++++++ test/lib/lookup.test.js | 279 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 423 insertions(+) create mode 100644 src/lib/lookup.js create mode 100644 test/lib/lookup.test.js diff --git a/src/lib/lookup.js b/src/lib/lookup.js new file mode 100644 index 0000000..cc12f0e --- /dev/null +++ b/src/lib/lookup.js @@ -0,0 +1,144 @@ +import { CliError } from './errors.js' + +/** v2 custom-field types that the search endpoints can match on. */ +const SEARCHABLE_TYPES = new Set([ + 'address', + 'varchar', + 'varchar_auto', + 'text', + 'double', + 'monetary', + 'phone', +]) +const NUMERIC_TYPES = new Set(['double', 'monetary']) + +/** + * Per-entity scoped-search config + built-in match fields. Each built-in gives + * the search `fields` scope, how to pull the comparable value(s) off a result + * record, and whether the compare is case-insensitive. + */ +const SEARCH_CONFIG = { + person: { + searchPath: '/api/v2/persons/search', + builtins: { + email: { + scope: 'email', + extract: (it) => (it.emails ?? []).map((e) => e.value), + ci: true, + }, + name: { scope: 'name', extract: (it) => [it.name], ci: false }, + phone: { + scope: 'phone', + extract: (it) => (it.phones ?? []).map((p) => p.value), + ci: false, + }, + }, + }, + org: { + searchPath: '/api/v2/organizations/search', + builtins: { + name: { scope: 'name', extract: (it) => [it.name], ci: false }, + }, + }, + deal: { + searchPath: '/api/v2/deals/search', + builtins: { + title: { scope: 'title', extract: (it) => [it.title], ci: false }, + }, + }, +} + +function valuesEqual(actual, expected, ci) { + if (actual == null) return false + if (typeof expected === 'number') return Number(actual) === expected + return ci + ? String(actual).toLowerCase() === String(expected).toLowerCase() + : String(actual) === String(expected) +} + +/** + * Find the record of `entity` whose `field` exactly equals `value`. Routes a + * built-in field (person email/name/phone, org name, deal title) or a + * searchable custom field (by name) to the scoped /search endpoint, collects + * ALL matches across cursor pages, then RE-VERIFIES each client-side — because + * `exact_match` is not a unique-key lookup (it's case-insensitive and, for + * custom fields, searches every custom field at once). The surviving count + * decides the tri-state: 0 → create, 1 → update, >1 → caller must refuse. + * + * @param {{ get: Function }} client + * @param {'person'|'org'|'deal'} entity + * @param {object[]} [defs] field definitions (needed for a custom --by field) + * @param {string} field built-in name or custom-field name + * @param {string} value the value to match + * @returns {Promise<{ status: 'none' } | { status: 'unique', id: number, record: object } | { status: 'ambiguous', matches: number[] }>} + */ +export async function lookupByField({ + client, + entity, + defs = [], + field, + value, +}) { + const cfg = SEARCH_CONFIG[entity] + const builtin = cfg.builtins[field.toLowerCase()] + + let scope + let extract + let ci + let compareValue + + if (builtin) { + ;({ scope, extract, ci } = builtin) + compareValue = value + } else { + const def = defs.find( + (d) => + d.field_name?.toLowerCase() === field.toLowerCase() || + d.field_code === field, + ) + if (!def) { + throw new CliError( + `Unknown field "${field}" for ${entity}. Run: pdcli field list ${entity}`, + { exitCode: 64 }, + ) + } + if (!SEARCHABLE_TYPES.has(def.field_type)) { + throw new CliError( + `Field "${field}" (${def.field_type}) is not searchable — --by needs a ` + + `built-in key or a searchable custom field (text/number/phone/address)`, + { exitCode: 64 }, + ) + } + scope = 'custom_fields' + const key = def.field_code + extract = (it) => [it.custom_fields?.[key]] + ci = false + compareValue = NUMERIC_TYPES.has(def.field_type) ? Number(value) : value + } + + const matches = [] + let cursor + do { + const body = await client.get(cfg.searchPath, { + query: { + term: value, + exact_match: true, + fields: scope, + limit: 500, + cursor, + }, + }) + for (const entry of body.data?.items ?? []) matches.push(entry.item) + cursor = body.additional_data?.next_cursor ?? null + } while (cursor) + + const survivors = matches.filter((item) => + extract(item).some((v) => valuesEqual(v, compareValue, ci)), + ) + + if (survivors.length === 0) return { status: 'none' } + if (survivors.length === 1) { + return { status: 'unique', id: survivors[0].id, record: survivors[0] } + } + return { status: 'ambiguous', matches: survivors.map((s) => s.id) } +} diff --git a/test/lib/lookup.test.js b/test/lib/lookup.test.js new file mode 100644 index 0000000..3dcf3e4 --- /dev/null +++ b/test/lib/lookup.test.js @@ -0,0 +1,279 @@ +import { describe, it, expect } from 'vitest' +import { lookupByField } from '../../src/lib/lookup.js' + +/** Fake client returning queued search pages and capturing queries. */ +function fakeClient(pages) { + let i = 0 + const calls = [] + return { + calls, + async get(path, opts) { + calls.push({ path, query: opts?.query }) + return pages[i++] ?? { data: { items: [] } } + }, + } +} +const page = (items, next = null) => ({ + data: { items: items.map((item) => ({ item })) }, + additional_data: { next_cursor: next }, +}) + +describe('lookupByField', () => { + it('returns none when nothing matches', async () => { + const client = fakeClient([page([])]) + const r = await lookupByField({ + client, + entity: 'person', + field: 'email', + value: 'a@x.com', + }) + expect(r.status).toBe('none') + }) + + it('returns unique with the id for exactly one match', async () => { + const client = fakeClient([ + page([{ id: 7, emails: [{ value: 'a@x.com' }] }]), + ]) + const r = await lookupByField({ + client, + entity: 'person', + field: 'email', + value: 'a@x.com', + }) + expect(r).toMatchObject({ status: 'unique', id: 7 }) + }) + + it('refuses (ambiguous) on more than one verified match', async () => { + const client = fakeClient([ + page([ + { id: 7, emails: [{ value: 'a@x.com' }] }, + { id: 8, emails: [{ value: 'a@x.com' }] }, + ]), + ]) + const r = await lookupByField({ + client, + entity: 'person', + field: 'email', + value: 'a@x.com', + }) + expect(r.status).toBe('ambiguous') + expect(r.matches).toEqual([7, 8]) + }) + + it('matches email case-insensitively', async () => { + const client = fakeClient([ + page([{ id: 7, emails: [{ value: 'A@X.com' }] }]), + ]) + const r = await lookupByField({ + client, + entity: 'person', + field: 'email', + value: 'a@x.com', + }) + expect(r).toMatchObject({ status: 'unique', id: 7 }) + }) + + it('re-verifies client-side, discarding search over-matches', async () => { + // search leaked a non-matching person (exact_match is not a unique key) + const client = fakeClient([ + page([{ id: 9, emails: [{ value: 'other@x.com' }] }]), + ]) + const r = await lookupByField({ + client, + entity: 'person', + field: 'email', + value: 'a@x.com', + }) + expect(r.status).toBe('none') + }) + + it('sends exact_match and the scoped fields param', async () => { + const client = fakeClient([page([])]) + await lookupByField({ + client, + entity: 'org', + field: 'name', + value: 'Acme', + }) + expect(client.calls[0]).toMatchObject({ + path: '/api/v2/organizations/search', + query: { term: 'Acme', exact_match: true, fields: 'name' }, + }) + }) + + it('pages through next_cursor before deciding ambiguity', async () => { + const client = fakeClient([ + page([{ id: 1, name: 'Acme' }], 'CUR'), + page([{ id: 2, name: 'Acme' }]), + ]) + const r = await lookupByField({ + client, + entity: 'org', + field: 'name', + value: 'Acme', + }) + expect(r.status).toBe('ambiguous') + expect(client.calls).toHaveLength(2) + expect(client.calls[1].query.cursor).toBe('CUR') + }) + + it('matches a searchable custom field by name, verifying the hash key', async () => { + const defs = [ + { + field_name: 'External ID', + field_code: 'abc123', + field_type: 'varchar', + }, + ] + const client = fakeClient([ + page([ + { id: 5, custom_fields: { abc123: 'D-42' } }, + { id: 6, custom_fields: { abc123: 'D-99' } }, // search leaked; verify drops it + ]), + ]) + const r = await lookupByField({ + client, + entity: 'deal', + defs, + field: 'External ID', + value: 'D-42', + }) + expect(r).toMatchObject({ status: 'unique', id: 5 }) + expect(client.calls[0].query.fields).toBe('custom_fields') + }) + + it('coerces numeric custom-field values for comparison', async () => { + const defs = [ + { field_name: 'Score', field_code: 'sc', field_type: 'double' }, + ] + const client = fakeClient([page([{ id: 5, custom_fields: { sc: 42 } }])]) + const r = await lookupByField({ + client, + entity: 'deal', + defs, + field: 'Score', + value: '42', + }) + expect(r).toMatchObject({ status: 'unique', id: 5 }) + }) + + it('matches a deal by title', async () => { + const client = fakeClient([page([{ id: 3, title: 'Acme expansion' }])]) + const r = await lookupByField({ + client, + entity: 'deal', + field: 'title', + value: 'Acme expansion', + }) + expect(r).toMatchObject({ status: 'unique', id: 3 }) + expect(client.calls[0].query.fields).toBe('title') + }) + + it('matches a person by name or phone (with empty-array tolerance)', async () => { + const byName = await lookupByField({ + client: fakeClient([page([{ id: 1, name: 'Jane Doe', phones: [] }])]), + entity: 'person', + field: 'name', + value: 'Jane Doe', + }) + expect(byName).toMatchObject({ status: 'unique', id: 1 }) + + const byPhone = await lookupByField({ + client: fakeClient([ + page([{ id: 2, phones: [{ value: '+15551234' }], emails: [] }]), + ]), + entity: 'person', + field: 'phone', + value: '+15551234', + }) + expect(byPhone).toMatchObject({ status: 'unique', id: 2 }) + }) + + it('accepts a custom field referenced by its hash code', async () => { + const defs = [ + { + field_name: 'External ID', + field_code: 'abc123', + field_type: 'varchar', + }, + ] + const client = fakeClient([ + page([{ id: 5, custom_fields: { abc123: 'D-42' } }]), + ]) + const r = await lookupByField({ + client, + entity: 'deal', + defs, + field: 'abc123', + value: 'D-42', + }) + expect(r).toMatchObject({ status: 'unique', id: 5 }) + }) + + it('tolerates a malformed search response (no data/cursor)', async () => { + const client = fakeClient([{}]) // no .data, no .additional_data + const r = await lookupByField({ + client, + entity: 'org', + field: 'name', + value: 'Acme', + }) + expect(r.status).toBe('none') + }) + + it('tolerates records missing the matched field', async () => { + const noEmail = await lookupByField({ + client: fakeClient([page([{ id: 1 }])]), // no emails property + entity: 'person', + field: 'email', + value: 'a@x.com', + }) + expect(noEmail.status).toBe('none') + + const noCustom = await lookupByField({ + client: fakeClient([page([{ id: 2 }])]), // no custom_fields property + entity: 'deal', + defs: [ + { field_name: 'External ID', field_code: 'k', field_type: 'varchar' }, + ], + field: 'External ID', + value: 'D-1', + }) + expect(noCustom.status).toBe('none') + + const noPhone = await lookupByField({ + client: fakeClient([page([{ id: 3 }])]), // no phones property + entity: 'person', + field: 'phone', + value: '+1555', + }) + expect(noPhone.status).toBe('none') + }) + + it('rejects a non-searchable field type with exit 64', async () => { + const defs = [ + { field_name: 'Stage Color', field_code: 'cc', field_type: 'enum' }, + ] + const err = await lookupByField({ + client: fakeClient([]), + entity: 'deal', + defs, + field: 'Stage Color', + value: 'Red', + }).catch((e) => e) + expect(err.exitCode).toBe(64) + expect(err.message).toMatch(/not searchable/i) + }) + + it('rejects an unknown field with exit 64', async () => { + const err = await lookupByField({ + client: fakeClient([]), + entity: 'deal', + defs: [], + field: 'Nonexistent', + value: 'x', + }).catch((e) => e) + expect(err.exitCode).toBe(64) + expect(err.message).toMatch(/unknown field/i) + }) +}) From 2e3ee5f362fe42a63733e8c184a168ba3ff63e05 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 11:50:28 +0200 Subject: [PATCH 2/8] =?UTF-8?q?feat(upsert):=20diffBody=20+=20runUpsert=20?= =?UTF-8?q?=E2=80=94=20idempotent=20match-or-create=20(export=20canon/eq)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/backup-diff.js | 4 +- src/lib/upsert.js | 123 +++++++++++++++++++ test/lib/upsert.test.js | 265 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 390 insertions(+), 2 deletions(-) create mode 100644 src/lib/upsert.js create mode 100644 test/lib/upsert.test.js diff --git a/src/lib/backup-diff.js b/src/lib/backup-diff.js index cd8ed36..eb31926 100644 --- a/src/lib/backup-diff.js +++ b/src/lib/backup-diff.js @@ -17,7 +17,7 @@ const FIELDS_FOR = { const SCHEMA_RESOURCES = new Set(Object.values(FIELDS_FOR)) /** Recursively sort object keys so equality is key-order insensitive. */ -function canon(value) { +export function canon(value) { if (Array.isArray(value)) return value.map(canon) if (value && typeof value === 'object') { return Object.fromEntries( @@ -30,7 +30,7 @@ function canon(value) { } /** Stable equality for scalar/array/object field values (key-order safe). */ -function eq(a, b) { +export function eq(a, b) { return JSON.stringify(canon(a ?? null)) === JSON.stringify(canon(b ?? null)) } diff --git a/src/lib/upsert.js b/src/lib/upsert.js new file mode 100644 index 0000000..b322d97 --- /dev/null +++ b/src/lib/upsert.js @@ -0,0 +1,123 @@ +import { CliError } from './errors.js' +import { eq } from './backup-diff.js' +import { lookupByField } from './lookup.js' + +/** Entity → v2 write path. */ +const WRITE_PATH = { + person: '/api/v2/persons', + org: '/api/v2/organizations', + deal: '/api/v2/deals', +} + +const NUMERIC_TYPES = new Set(['double', 'monetary']) + +/** + * Inject the match field+value into a CREATE body (so a created record actually + * carries the value it was matched on). `??=` so an explicit body value wins. + */ +function injectMatch(body, entity, field, value, defs) { + const key = field.toLowerCase() + if (entity === 'person' && key === 'email') { + body.emails ??= [{ value, primary: true }] + return + } + if (entity === 'person' && key === 'phone') { + body.phones ??= [{ value, primary: true }] + return + } + if ((entity === 'person' || entity === 'org') && key === 'name') { + body.name ??= value + return + } + if (entity === 'deal' && key === 'title') { + body.title ??= value + return + } + // custom field + const def = defs.find( + (d) => d.field_name?.toLowerCase() === key || d.field_code === field, + ) + body.custom_fields ??= {} + body.custom_fields[def.field_code] ??= NUMERIC_TYPES.has(def.field_type) + ? Number(value) + : value +} + +/** + * Field-level diff for an idempotent PATCH: the subset of `incoming` whose + * value differs from `existing`. Top-level fields compare directly; the nested + * v2 `custom_fields` object is diffed key by key. Equality is key-order + * insensitive. An empty result means "no change → skip the PATCH". + * @param {object} incoming the desired body + * @param {object} existing the current record + * @returns {object} the changed-only body + */ +export function diffBody(incoming, existing) { + const changed = {} + for (const [key, value] of Object.entries(incoming)) { + if (key === 'custom_fields' && value && typeof value === 'object') { + const cf = {} + for (const [ck, cv] of Object.entries(value)) { + if (!eq(cv, existing?.custom_fields?.[ck])) cf[ck] = cv + } + if (Object.keys(cf).length > 0) changed.custom_fields = cf + } else if (!eq(value, existing?.[key])) { + changed[key] = value + } + } + return changed +} + +/** + * Idempotent match-or-create. Looks up `entity` by `by`=`value`: + * - 0 matches → create (the match field is injected into the body) + * - 1 match → PATCH only the fields that differ (diffBody); none → unchanged + * - >1 match → refuse (exit 65) — never guess which to write + * + * @param {{ get, post, patch }} client + * @param {'person'|'org'|'deal'} entity + * @param {string} by match field (built-in or searchable custom field) + * @param {string} value match value + * @param {object} body desired write body (from buildWriteBody) + * @param {object[]} [defs] field definitions (for custom --by + injection) + * @param {boolean} [dryRun] + * @returns {Promise<{ action: 'created'|'updated'|'unchanged', id?: number, + * changed?: object, dryRun?: boolean, record?: object }>} + */ +export async function runUpsert({ + client, + entity, + by, + value, + body, + defs = [], + dryRun = false, +}) { + const writePath = WRITE_PATH[entity] + const match = await lookupByField({ client, entity, defs, field: by, value }) + + if (match.status === 'ambiguous') { + throw new CliError( + `--by ${by}="${value}" matches ${match.matches.length} ${entity} records ` + + `(ids: ${match.matches.join(', ')}) — refusing to guess. Narrow the match value.`, + { exitCode: 65 }, + ) + } + + if (match.status === 'none') { + const createBody = { ...body } + injectMatch(createBody, entity, by, value, defs) + if (dryRun) return { action: 'created', dryRun: true, body: createBody } + const res = await client.post(writePath, { body: createBody }) + return { action: 'created', id: res.data?.id, record: res.data } + } + + // unique → diff-before-PATCH + const changed = diffBody(body, match.record) + if (Object.keys(changed).length === 0) { + return { action: 'unchanged', id: match.id } + } + if (dryRun) return { action: 'updated', id: match.id, dryRun: true, changed } + const res = await client.patch(`${writePath}/${match.id}`, { body: changed }) + return { action: 'updated', id: match.id, changed, record: res.data } +} diff --git a/test/lib/upsert.test.js b/test/lib/upsert.test.js new file mode 100644 index 0000000..30e24cc --- /dev/null +++ b/test/lib/upsert.test.js @@ -0,0 +1,265 @@ +import { describe, it, expect } from 'vitest' +import { diffBody, runUpsert } from '../../src/lib/upsert.js' + +describe('diffBody', () => { + it('returns only the changed top-level fields', () => { + expect( + diffBody({ title: 'B', value: 100 }, { title: 'A', value: 100 }), + ).toEqual({ + title: 'B', + }) + }) + + it('diffs nested custom_fields key by key', () => { + expect( + diffBody( + { custom_fields: { a: 1, b: 2 } }, + { custom_fields: { a: 1, b: 9 } }, + ), + ).toEqual({ custom_fields: { b: 2 } }) + }) + + it('includes a field the existing record lacks', () => { + expect(diffBody({ value: 5 }, {})).toEqual({ value: 5 }) + }) + + it('returns empty when nothing changed (key-order insensitive)', () => { + expect( + diffBody({ meta: { x: 1, y: 2 } }, { meta: { y: 2, x: 1 } }), + ).toEqual({}) + }) + + it('omits custom_fields entirely when no custom value changed', () => { + expect( + diffBody({ custom_fields: { a: 1 } }, { custom_fields: { a: 1 } }), + ).toEqual({}) + }) +}) + +/** Fake client: queued search items for lookup + captured post/patch. */ +function fakeClient({ items = [] } = {}) { + const calls = { post: [], patch: [] } + return { + calls, + async get() { + return { + data: { items: items.map((item) => ({ item })) }, + additional_data: { next_cursor: null }, + } + }, + async post(path, opts) { + calls.post.push({ path, body: opts.body }) + return { data: { id: 99, ...opts.body } } + }, + async patch(path, opts) { + calls.patch.push({ path, body: opts.body }) + return { data: { id: 7, ...opts.body } } + }, + } +} + +describe('runUpsert', () => { + it('creates when no match, injecting the match field into the body', async () => { + const client = fakeClient({ items: [] }) + const r = await runUpsert({ + client, + entity: 'person', + by: 'email', + value: 'a@x.com', + body: {}, + }) + expect(r).toMatchObject({ action: 'created', id: 99 }) + expect(client.calls.post[0]).toMatchObject({ + path: '/api/v2/persons', + body: { emails: [{ value: 'a@x.com', primary: true }] }, + }) + }) + + it('updates only the changed fields when exactly one matches', async () => { + const client = fakeClient({ + items: [ + { id: 7, emails: [{ value: 'a@x.com' }], custom_fields: { k: 6 } }, + ], + }) + const r = await runUpsert({ + client, + entity: 'person', + by: 'email', + value: 'a@x.com', + body: { custom_fields: { k: 5 } }, + }) + expect(r).toMatchObject({ action: 'updated', id: 7 }) + expect(client.calls.patch[0]).toMatchObject({ + path: '/api/v2/persons/7', + body: { custom_fields: { k: 5 } }, + }) + }) + + it('reports unchanged and issues no PATCH when nothing differs', async () => { + const client = fakeClient({ + items: [ + { id: 7, emails: [{ value: 'a@x.com' }], custom_fields: { k: 5 } }, + ], + }) + const r = await runUpsert({ + client, + entity: 'person', + by: 'email', + value: 'a@x.com', + body: { custom_fields: { k: 5 } }, + }) + expect(r).toMatchObject({ action: 'unchanged', id: 7 }) + expect(client.calls.patch).toHaveLength(0) + }) + + it('refuses an ambiguous match with exit 65', async () => { + const client = fakeClient({ + items: [ + { id: 7, emails: [{ value: 'a@x.com' }] }, + { id: 8, emails: [{ value: 'a@x.com' }] }, + ], + }) + const err = await runUpsert({ + client, + entity: 'person', + by: 'email', + value: 'a@x.com', + body: {}, + }).catch((e) => e) + expect(err.exitCode).toBe(65) + expect(err.message).toMatch(/7.*8|8.*7/) + }) + + it('dry-run create writes nothing', async () => { + const client = fakeClient({ items: [] }) + const r = await runUpsert({ + client, + entity: 'org', + by: 'name', + value: 'Acme', + body: {}, + dryRun: true, + }) + expect(r).toMatchObject({ action: 'created', dryRun: true }) + expect(client.calls.post).toHaveLength(0) + }) + + it('dry-run update writes nothing but reports the change set', async () => { + const client = fakeClient({ items: [{ id: 3, title: 'Acme', value: 1 }] }) + const r = await runUpsert({ + client, + entity: 'deal', + by: 'title', + value: 'Acme', + body: { value: 2 }, + dryRun: true, + }) + expect(r).toMatchObject({ action: 'updated', id: 3, dryRun: true }) + expect(r.changed).toEqual({ value: 2 }) + expect(client.calls.patch).toHaveLength(0) + }) + + it('injects a custom match field into the create body', async () => { + const defs = [ + { field_name: 'External ID', field_code: 'ext', field_type: 'varchar' }, + ] + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'deal', + by: 'External ID', + value: 'D-42', + body: {}, + defs, + }) + expect(client.calls.post[0].body.custom_fields).toEqual({ ext: 'D-42' }) + }) + + it('injects a phone match field when creating a person by phone', async () => { + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'person', + by: 'phone', + value: '+15551234', + body: {}, + }) + expect(client.calls.post[0].body.phones).toEqual([ + { value: '+15551234', primary: true }, + ]) + }) + + it('injects a title match field when creating a deal by title', async () => { + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'deal', + by: 'title', + value: 'Acme expansion', + body: {}, + }) + expect(client.calls.post[0].body.title).toBe('Acme expansion') + }) + + it('injects a numeric custom match field, preserving other custom fields', async () => { + const defs = [ + { field_name: 'Score', field_code: 'sc', field_type: 'double' }, + { field_name: 'Region', field_code: 'rg', field_type: 'varchar' }, + ] + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'deal', + by: 'Score', + value: '42', + body: { custom_fields: { rg: 'EMEA' } }, + defs, + }) + expect(client.calls.post[0].body.custom_fields).toEqual({ + rg: 'EMEA', + sc: 42, + }) + }) + + it('does not clobber an explicit body value with the injected match field', async () => { + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'org', + by: 'name', + value: 'Acme', + body: { name: 'Acme Corporation' }, // explicit wins + }) + expect(client.calls.post[0].body.name).toBe('Acme Corporation') + }) + + it('matches a custom field referenced by hash code (no field_name)', async () => { + const defs = [{ field_code: 'ext', field_type: 'varchar' }] // no field_name + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'deal', + by: 'ext', + value: 'D-1', + body: {}, + defs, + }) + expect(client.calls.post[0].body.custom_fields).toEqual({ ext: 'D-1' }) + }) + + it('does not clobber an explicit custom match value', async () => { + const defs = [ + { field_name: 'External ID', field_code: 'ext', field_type: 'varchar' }, + ] + const client = fakeClient({ items: [] }) + await runUpsert({ + client, + entity: 'deal', + by: 'External ID', + value: 'D-42', + body: { custom_fields: { ext: 'EXPLICIT' } }, + defs, + }) + expect(client.calls.post[0].body.custom_fields.ext).toBe('EXPLICIT') + }) +}) From 5df7e229eed857f0286a4cdeb6970464b990db86 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 12:05:40 +0200 Subject: [PATCH 3/8] feat(upsert): person/org/deal idempotent upsert commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thin commands over a shared upsertWithDefs/summarizeUpsert lib pair: fetch field defs, build the write body (--field/--body), match by --by (built-in key or searchable custom field), then create or PATCH only the changed fields. Ambiguous matches refuse with exit 65 — never guess. --dry-run previews without writing; table prints a one-line summary, json emits the full action result. --- src/commands/deal/upsert.js | 54 ++++++++++++ src/commands/org/upsert.js | 54 ++++++++++++ src/commands/person/upsert.js | 55 ++++++++++++ src/lib/upsert.js | 33 +++++++ test/commands/deal/upsert.test.js | 103 ++++++++++++++++++++++ test/commands/org/upsert.test.js | 100 ++++++++++++++++++++++ test/commands/person/upsert.test.js | 128 ++++++++++++++++++++++++++++ test/lib/upsert.test.js | 37 +++++++- 8 files changed, 563 insertions(+), 1 deletion(-) create mode 100644 src/commands/deal/upsert.js create mode 100644 src/commands/org/upsert.js create mode 100644 src/commands/person/upsert.js create mode 100644 test/commands/deal/upsert.test.js create mode 100644 test/commands/org/upsert.test.js create mode 100644 test/commands/person/upsert.test.js diff --git a/src/commands/deal/upsert.js b/src/commands/deal/upsert.js new file mode 100644 index 0000000..c56cf9f --- /dev/null +++ b/src/commands/deal/upsert.js @@ -0,0 +1,54 @@ +import { Args, Flags } from '@oclif/core' +import BaseCommand from '../../base-command.js' +import { upsertWithDefs, summarizeUpsert } from '../../lib/upsert.js' + +export default class DealUpsertCommand extends BaseCommand { + static description = + 'Idempotent deal upsert: match by --by, then create or PATCH only the ' + + 'changed fields. Refuses (exit 65) if more than one record matches.' + + static examples = [ + '<%= config.bin %> deal upsert "Acme expansion" --by title --field "Stage=Won"', + '<%= config.bin %> deal upsert "D-42" --by "External ID" --body \'{"value":5000}\'', + '<%= config.bin %> deal upsert "Acme expansion" --by title --field "Stage=Won" --dry-run', + ] + + static args = { + value: Args.string({ required: true, description: 'value to match on' }), + } + + static flags = { + ...BaseCommand.baseFlags, + by: Flags.string({ + required: true, + description: 'Match field: title, or a searchable custom field', + }), + field: Flags.string({ + multiple: true, + description: 'Field to set as "Name=Value" (repeatable)', + }), + body: Flags.string({ description: 'Raw JSON body to merge' }), + 'dry-run': Flags.boolean({ + description: 'Preview the action without writing', + }), + } + + async run() { + const { args, flags } = await this.parse(DealUpsertCommand) + const result = await upsertWithDefs({ + client: this.apiClient, + entity: 'deal', + by: flags.by, + value: args.value, + fields: flags.field, + rawBody: flags.body, + dryRun: flags['dry-run'], + }) + + if (this.resolveFormat() !== 'table') { + await this.outputResults(result, {}) + return + } + this.log(summarizeUpsert(result, 'deal')) + } +} diff --git a/src/commands/org/upsert.js b/src/commands/org/upsert.js new file mode 100644 index 0000000..97bf2f0 --- /dev/null +++ b/src/commands/org/upsert.js @@ -0,0 +1,54 @@ +import { Args, Flags } from '@oclif/core' +import BaseCommand from '../../base-command.js' +import { upsertWithDefs, summarizeUpsert } from '../../lib/upsert.js' + +export default class OrgUpsertCommand extends BaseCommand { + static description = + 'Idempotent organization upsert: match by --by, then create or PATCH only ' + + 'the changed fields. Refuses (exit 65) if more than one record matches.' + + static examples = [ + '<%= config.bin %> org upsert Acme --by name --field "Tier=Gold"', + '<%= config.bin %> org upsert "D-42" --by "External ID" --body \'{"owner_id":42}\'', + '<%= config.bin %> org upsert Acme --by name --field "Tier=Gold" --dry-run', + ] + + static args = { + value: Args.string({ required: true, description: 'value to match on' }), + } + + static flags = { + ...BaseCommand.baseFlags, + by: Flags.string({ + required: true, + description: 'Match field: name, or a searchable custom field', + }), + field: Flags.string({ + multiple: true, + description: 'Field to set as "Name=Value" (repeatable)', + }), + body: Flags.string({ description: 'Raw JSON body to merge' }), + 'dry-run': Flags.boolean({ + description: 'Preview the action without writing', + }), + } + + async run() { + const { args, flags } = await this.parse(OrgUpsertCommand) + const result = await upsertWithDefs({ + client: this.apiClient, + entity: 'org', + by: flags.by, + value: args.value, + fields: flags.field, + rawBody: flags.body, + dryRun: flags['dry-run'], + }) + + if (this.resolveFormat() !== 'table') { + await this.outputResults(result, {}) + return + } + this.log(summarizeUpsert(result, 'org')) + } +} diff --git a/src/commands/person/upsert.js b/src/commands/person/upsert.js new file mode 100644 index 0000000..4ecae7f --- /dev/null +++ b/src/commands/person/upsert.js @@ -0,0 +1,55 @@ +import { Args, Flags } from '@oclif/core' +import BaseCommand from '../../base-command.js' +import { upsertWithDefs, summarizeUpsert } from '../../lib/upsert.js' + +export default class PersonUpsertCommand extends BaseCommand { + static description = + 'Idempotent person upsert: match by --by, then create or PATCH only the ' + + 'changed fields. Refuses (exit 65) if more than one record matches.' + + static examples = [ + '<%= config.bin %> person upsert a@x.com --by email --field "Tier=Gold"', + '<%= config.bin %> person upsert "Jane Doe" --by name --body \'{"owner_id":42}\'', + '<%= config.bin %> person upsert a@x.com --by email --field "Tier=Gold" --dry-run', + ] + + static args = { + value: Args.string({ required: true, description: 'value to match on' }), + } + + static flags = { + ...BaseCommand.baseFlags, + by: Flags.string({ + required: true, + description: + 'Match field: email, name, phone, or a searchable custom field', + }), + field: Flags.string({ + multiple: true, + description: 'Field to set as "Name=Value" (repeatable)', + }), + body: Flags.string({ description: 'Raw JSON body to merge' }), + 'dry-run': Flags.boolean({ + description: 'Preview the action without writing', + }), + } + + async run() { + const { args, flags } = await this.parse(PersonUpsertCommand) + const result = await upsertWithDefs({ + client: this.apiClient, + entity: 'person', + by: flags.by, + value: args.value, + fields: flags.field, + rawBody: flags.body, + dryRun: flags['dry-run'], + }) + + if (this.resolveFormat() !== 'table') { + await this.outputResults(result, {}) + return + } + this.log(summarizeUpsert(result, 'person')) + } +} diff --git a/src/lib/upsert.js b/src/lib/upsert.js index b322d97..c4396db 100644 --- a/src/lib/upsert.js +++ b/src/lib/upsert.js @@ -1,6 +1,8 @@ import { CliError } from './errors.js' import { eq } from './backup-diff.js' import { lookupByField } from './lookup.js' +import { getFields } from './fields.js' +import { buildWriteBody } from './input.js' /** Entity → v2 write path. */ const WRITE_PATH = { @@ -121,3 +123,34 @@ export async function runUpsert({ const res = await client.patch(`${writePath}/${match.id}`, { body: changed }) return { action: 'updated', id: match.id, changed, record: res.data } } + +/** + * Command-facing wrapper: fetch field defs, build the write body from --field / + * --body, then run the upsert. Keeps the three entity commands trivial. + * @param {object} options + * @returns {Promise} the runUpsert result + */ +export async function upsertWithDefs({ + client, + entity, + by, + value, + fields = [], + rawBody, + dryRun = false, +}) { + const defs = await getFields(client, entity) + const body = buildWriteBody({ fields, rawBody, defs }) + return runUpsert({ client, entity, by, value, body, defs, dryRun }) +} + +/** One-line human summary of an upsert result for table output. */ +export function summarizeUpsert(result, entity) { + const prefix = result.dryRun ? '[dry-run] would ' : '' + if (result.action === 'unchanged') return `${entity} #${result.id} unchanged` + if (result.action === 'created') { + return `${prefix}create ${entity}${result.id != null ? ` #${result.id}` : ''}` + } + const n = result.changed ? Object.keys(result.changed).length : 0 + return `${prefix}update ${entity} #${result.id} (${n} field${n === 1 ? '' : 's'})` +} diff --git a/test/commands/deal/upsert.test.js b/test/commands/deal/upsert.test.js new file mode 100644 index 0000000..a6612e8 --- /dev/null +++ b/test/commands/deal/upsert.test.js @@ -0,0 +1,103 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import nock from 'nock' + +const mockResolveCredentials = vi.fn() +vi.mock('../../../src/lib/auth.js', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, resolveCredentials: mockResolveCredentials } +}) + +vi.mock('../../../src/lib/config.js', () => ({ + loadConfig: vi.fn().mockReturnValue({ activeProfile: 'default' }), +})) + +const { default: DealUpsertCommand } = + await import('../../../src/commands/deal/upsert.js') +const { clearFieldsCache } = await import('../../../src/lib/fields.js') +import { runCmd, mockApi } from '../../helpers.js' + +function mockFields(defs = []) { + mockApi().get('/api/v2/dealFields').reply(200, { success: true, data: defs }) +} + +function mockSearch(records = []) { + mockApi() + .get('/api/v2/deals/search') + .query(true) + .reply(200, { + success: true, + data: { items: records.map((item) => ({ item })) }, + additional_data: { next_cursor: null }, + }) +} + +describe('deal upsert', () => { + beforeEach(() => { + nock.cleanAll() + clearFieldsCache() + mockResolveCredentials.mockResolvedValue({ + companyDomain: 'acme', + token: 'tok', + source: 'profile', + }) + }) + + afterEach(() => { + nock.cleanAll() + }) + + it('creates when no deal matches, injecting the title (json)', async () => { + mockFields() + mockSearch([]) + mockApi() + .post('/api/v2/deals', { title: 'Acme expansion' }) + .reply(201, { success: true, data: { id: 3, title: 'Acme expansion' } }) + + const stdout = await runCmd(DealUpsertCommand, [ + 'Acme expansion', + '--by', + 'title', + '--output', + 'json', + ]) + const out = JSON.parse(stdout) + expect(out.action).toBe('created') + expect(out.id).toBe(3) + }) + + it('PATCHes one field and prints a singular summary (table)', async () => { + mockFields() + mockSearch([{ id: 7, title: 'Acme expansion', value: 100 }]) + mockApi() + .patch('/api/v2/deals/7', { value: 200 }) + .reply(200, { success: true, data: { id: 7, value: 200 } }) + + const stdout = await runCmd(DealUpsertCommand, [ + 'Acme expansion', + '--by', + 'title', + '--body', + '{"value":200}', + '--output', + 'table', + ]) + expect(stdout).toBe('update deal #7 (1 field)') + }) + + it('previews an update without writing, pluralizing the count (dry-run table)', async () => { + mockFields() + mockSearch([{ id: 3, title: 'Acme', value: 1, owner_id: 1 }]) + + const stdout = await runCmd(DealUpsertCommand, [ + 'Acme', + '--by', + 'title', + '--body', + '{"value":2,"owner_id":9}', + '--dry-run', + '--output', + 'table', + ]) + expect(stdout).toBe('[dry-run] would update deal #3 (2 fields)') + }) +}) diff --git a/test/commands/org/upsert.test.js b/test/commands/org/upsert.test.js new file mode 100644 index 0000000..729e27c --- /dev/null +++ b/test/commands/org/upsert.test.js @@ -0,0 +1,100 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import nock from 'nock' + +const mockResolveCredentials = vi.fn() +vi.mock('../../../src/lib/auth.js', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, resolveCredentials: mockResolveCredentials } +}) + +vi.mock('../../../src/lib/config.js', () => ({ + loadConfig: vi.fn().mockReturnValue({ activeProfile: 'default' }), +})) + +const { default: OrgUpsertCommand } = + await import('../../../src/commands/org/upsert.js') +const { clearFieldsCache } = await import('../../../src/lib/fields.js') +import { runCmd, mockApi } from '../../helpers.js' + +function mockFields(defs = []) { + mockApi() + .get('/api/v2/organizationFields') + .reply(200, { success: true, data: defs }) +} + +function mockSearch(records = []) { + mockApi() + .get('/api/v2/organizations/search') + .query(true) + .reply(200, { + success: true, + data: { items: records.map((item) => ({ item })) }, + additional_data: { next_cursor: null }, + }) +} + +describe('org upsert', () => { + beforeEach(() => { + nock.cleanAll() + clearFieldsCache() + mockResolveCredentials.mockResolvedValue({ + companyDomain: 'acme', + token: 'tok', + source: 'profile', + }) + }) + + afterEach(() => { + nock.cleanAll() + }) + + it('creates when no org matches, injecting the name (json)', async () => { + mockFields() + mockSearch([]) + mockApi() + .post('/api/v2/organizations', { name: 'Acme' }) + .reply(201, { success: true, data: { id: 5, name: 'Acme' } }) + + const stdout = await runCmd(OrgUpsertCommand, [ + 'Acme', + '--by', + 'name', + '--output', + 'json', + ]) + const out = JSON.parse(stdout) + expect(out.action).toBe('created') + expect(out.id).toBe(5) + }) + + it('reports unchanged when the matched org already matches (table)', async () => { + mockFields() + mockSearch([{ id: 7, name: 'Acme', owner_id: 1 }]) + + const stdout = await runCmd(OrgUpsertCommand, [ + 'Acme', + '--by', + 'name', + '--body', + '{"owner_id":1}', + '--output', + 'table', + ]) + expect(stdout).toBe('org #7 unchanged') + }) + + it('previews a create without writing (dry-run table)', async () => { + mockFields() + mockSearch([]) + + const stdout = await runCmd(OrgUpsertCommand, [ + 'Acme', + '--by', + 'name', + '--dry-run', + '--output', + 'table', + ]) + expect(stdout).toBe('[dry-run] would create org') + }) +}) diff --git a/test/commands/person/upsert.test.js b/test/commands/person/upsert.test.js new file mode 100644 index 0000000..a162975 --- /dev/null +++ b/test/commands/person/upsert.test.js @@ -0,0 +1,128 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import nock from 'nock' + +const mockResolveCredentials = vi.fn() +vi.mock('../../../src/lib/auth.js', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, resolveCredentials: mockResolveCredentials } +}) + +vi.mock('../../../src/lib/config.js', () => ({ + loadConfig: vi.fn().mockReturnValue({ activeProfile: 'default' }), +})) + +const { default: PersonUpsertCommand } = + await import('../../../src/commands/person/upsert.js') +const { clearFieldsCache } = await import('../../../src/lib/fields.js') +import { runCmd, mockApi } from '../../helpers.js' + +/** Mock the field-defs fetch (getFields always runs first). */ +function mockFields(defs = []) { + mockApi() + .get('/api/v2/personFields') + .reply(200, { success: true, data: defs }) +} + +/** Mock the scoped search, returning the given records as search items. */ +function mockSearch(records = []) { + mockApi() + .get('/api/v2/persons/search') + .query(true) + .reply(200, { + success: true, + data: { items: records.map((item) => ({ item })) }, + additional_data: { next_cursor: null }, + }) +} + +describe('person upsert', () => { + beforeEach(() => { + nock.cleanAll() + clearFieldsCache() + mockResolveCredentials.mockResolvedValue({ + companyDomain: 'acme', + token: 'tok', + source: 'profile', + }) + }) + + afterEach(() => { + nock.cleanAll() + }) + + it('creates when no match, injecting the match field (json)', async () => { + mockFields() + mockSearch([]) + mockApi() + .post('/api/v2/persons', { + emails: [{ value: 'a@x.com', primary: true }], + }) + .reply(201, { success: true, data: { id: 99, name: 'A' } }) + + const stdout = await runCmd(PersonUpsertCommand, [ + 'a@x.com', + '--by', + 'email', + '--output', + 'json', + ]) + const out = JSON.parse(stdout) + expect(out.action).toBe('created') + expect(out.id).toBe(99) + }) + + it('PATCHes only the changed fields when exactly one matches (json)', async () => { + mockFields() + mockSearch([{ id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 }]) + mockApi() + .patch('/api/v2/persons/7', { owner_id: 42 }) + .reply(200, { success: true, data: { id: 7, owner_id: 42 } }) + + const stdout = await runCmd(PersonUpsertCommand, [ + 'a@x.com', + '--by', + 'email', + '--body', + '{"owner_id":42}', + '--output', + 'json', + ]) + const out = JSON.parse(stdout) + expect(out.action).toBe('updated') + expect(out.id).toBe(7) + expect(out.changed).toEqual({ owner_id: 42 }) + }) + + it('refuses an ambiguous match with exit 65', async () => { + mockFields() + mockSearch([ + { id: 7, emails: [{ value: 'a@x.com' }] }, + { id: 8, emails: [{ value: 'a@x.com' }] }, + ]) + const err = await PersonUpsertCommand.run([ + 'a@x.com', + '--by', + 'email', + '--output', + 'json', + ]).catch((e) => e) + expect(err.exitCode ?? err.oclif?.exit).toBe(65) + }) + + it('prints a one-line summary for the created record (table)', async () => { + mockFields() + mockSearch([]) + mockApi() + .post('/api/v2/persons') + .reply(201, { success: true, data: { id: 99 } }) + + const stdout = await runCmd(PersonUpsertCommand, [ + 'a@x.com', + '--by', + 'email', + '--output', + 'table', + ]) + expect(stdout).toBe('create person #99') + }) +}) diff --git a/test/lib/upsert.test.js b/test/lib/upsert.test.js index 30e24cc..bc0104c 100644 --- a/test/lib/upsert.test.js +++ b/test/lib/upsert.test.js @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { diffBody, runUpsert } from '../../src/lib/upsert.js' +import { diffBody, runUpsert, summarizeUpsert } from '../../src/lib/upsert.js' describe('diffBody', () => { it('returns only the changed top-level fields', () => { @@ -263,3 +263,38 @@ describe('runUpsert', () => { expect(client.calls.post[0].body.custom_fields.ext).toBe('EXPLICIT') }) }) + +describe('summarizeUpsert', () => { + it('counts changed fields, pluralizing past one', () => { + expect( + summarizeUpsert({ action: 'updated', id: 7, changed: { a: 1 } }, 'deal'), + ).toBe('update deal #7 (1 field)') + expect( + summarizeUpsert( + { action: 'updated', id: 7, changed: { a: 1, b: 2 } }, + 'deal', + ), + ).toBe('update deal #7 (2 fields)') + }) + + it('treats an updated result with no change set as zero fields', () => { + expect(summarizeUpsert({ action: 'updated', id: 7 }, 'deal')).toBe( + 'update deal #7 (0 fields)', + ) + }) + + it('prefixes dry-run and omits the id when a created record has none', () => { + expect(summarizeUpsert({ action: 'created', dryRun: true }, 'org')).toBe( + '[dry-run] would create org', + ) + expect(summarizeUpsert({ action: 'created', id: 5 }, 'org')).toBe( + 'create org #5', + ) + }) + + it('reports an unchanged record', () => { + expect(summarizeUpsert({ action: 'unchanged', id: 7 }, 'person')).toBe( + 'person #7 unchanged', + ) + }) +}) From 5c6b97243fe08583bbd768dd1852b9c6299f88d9 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 12:13:32 +0200 Subject: [PATCH 4/8] feat(import): CSV --upsert/--match-on for person and org import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an idempotent CSV mode: with --upsert --match-on , each row is matched on that field's per-row value, then created if absent or PATCHed (changed fields only) if exactly one matches. Ambiguous rows (and empty match values) are collected as per-row failures and exit 1 without aborting the batch — never guessing which record to write. --dry-run looks up and reports created/updated/unchanged counts without writing. Shared bulkUpsertRows lib runs the batch through bulkRun's pacing. --- src/commands/org/import.js | 87 ++++++++++++++- src/commands/person/import.js | 87 ++++++++++++++- src/lib/upsert.js | 57 ++++++++++ test/commands/org/import.test.js | 152 ++++++++++++++++++++++++++ test/commands/person/import.test.js | 164 ++++++++++++++++++++++++++++ test/lib/upsert.test.js | 123 ++++++++++++++++++++- 6 files changed, 667 insertions(+), 3 deletions(-) diff --git a/src/commands/org/import.js b/src/commands/org/import.js index 8ff24bb..9c314e6 100644 --- a/src/commands/org/import.js +++ b/src/commands/org/import.js @@ -6,6 +6,7 @@ import BaseCommand from '../../base-command.js' import { parseCsv } from '../../lib/csv-parse.js' import { prepareImportBodies } from '../../lib/import.js' import { bulkRun } from '../../lib/bulk.js' +import { bulkUpsertRows } from '../../lib/upsert.js' import { getFields } from '../../lib/fields.js' import { confirmAction } from '../../lib/confirm.js' import { CliError } from '../../lib/errors.js' @@ -34,6 +35,13 @@ export default class OrgImportCommand extends BaseCommand { static flags = { ...BaseCommand.baseFlags, + upsert: Flags.boolean({ + description: 'Match each row on --match-on, then create or update', + default: false, + }), + 'match-on': Flags.string({ + description: 'Field to match rows on in --upsert mode (e.g. name)', + }), 'dry-run': Flags.boolean({ description: 'Validate every row without creating anything', default: false, @@ -53,14 +61,39 @@ export default class OrgImportCommand extends BaseCommand { throw new CliError('CSV must include a "name" column', { exitCode: 64 }) } + let matchIdx + if (flags.upsert) { + if (!flags['match-on']) { + throw new CliError('--upsert requires --match-on ', { + exitCode: 64, + }) + } + matchIdx = headers.findIndex( + (h) => h.toLowerCase() === flags['match-on'].toLowerCase(), + ) + if (matchIdx < 0) { + throw new CliError( + `--match-on "${flags['match-on']}" is not a column in ${args.file}`, + { exitCode: 64 }, + ) + } + } + const needsDefs = headers.some((h) => !(h.toLowerCase() in SPECIAL_COLUMNS)) + const defs = + needsDefs || flags.upsert ? await getFields(this.apiClient, 'org') : [] const bodies = prepareImportBodies({ headers, rows, specialColumns: SPECIAL_COLUMNS, - defs: needsDefs ? await getFields(this.apiClient, 'org') : [], + defs, }) + if (flags.upsert) { + await this.upsertRows({ args, flags, rows, bodies, matchIdx, defs }) + return + } + if (flags['dry-run']) { this.log(chalk.green(`${bodies.length} rows valid — nothing created`)) return @@ -106,4 +139,56 @@ export default class OrgImportCommand extends BaseCommand { ) } } + + /** Idempotent CSV path: match each row on --match-on, then create or PATCH. */ + async upsertRows({ args, flags, rows, bodies, matchIdx, defs }) { + const matchOn = flags['match-on'] + const items = bodies.map((body, i) => ({ body, value: rows[i][matchIdx] })) + + if (!flags['dry-run']) { + const ok = await confirmAction( + `Upsert ${items.length} organizations from ${args.file} (match on ${matchOn})?`, + flags.yes, + ) + if (!ok) { + throw new CliError('Aborted', { exitCode: 1 }) + } + } + + const spinner = ora(`Upserting ${items.length} organizations...`).start() + let summary + try { + summary = await bulkUpsertRows({ + client: this.apiClient, + entity: 'org', + matchOn, + rows: items, + defs, + dryRun: flags['dry-run'], + onProgress: (done, total) => { + spinner.text = `Upserting organizations ${done}/${total}` + }, + }) + } finally { + spinner.stop() + } + + const { created, updated, unchanged } = summary.counts + const prefix = flags['dry-run'] ? '[dry-run] ' : '' + this.log( + chalk.green( + `${prefix}${created} created, ${updated} updated, ${unchanged} unchanged`, + ), + ) + + if (summary.failed.length > 0) { + for (const { item, error } of summary.failed) { + this.log(chalk.red(` ✘ ${matchOn}="${item.value}": ${error}`)) + } + throw new CliError( + `${summary.failed.length} of ${items.length} rows failed`, + { exitCode: 1 }, + ) + } + } } diff --git a/src/commands/person/import.js b/src/commands/person/import.js index a640b33..35427fb 100644 --- a/src/commands/person/import.js +++ b/src/commands/person/import.js @@ -6,6 +6,7 @@ import BaseCommand from '../../base-command.js' import { parseCsv } from '../../lib/csv-parse.js' import { prepareImportBodies } from '../../lib/import.js' import { bulkRun } from '../../lib/bulk.js' +import { bulkUpsertRows } from '../../lib/upsert.js' import { getFields } from '../../lib/fields.js' import { confirmAction } from '../../lib/confirm.js' import { CliError } from '../../lib/errors.js' @@ -43,6 +44,13 @@ export default class PersonImportCommand extends BaseCommand { static flags = { ...BaseCommand.baseFlags, + upsert: Flags.boolean({ + description: 'Match each row on --match-on, then create or update', + default: false, + }), + 'match-on': Flags.string({ + description: 'Field to match rows on in --upsert mode (e.g. email)', + }), 'dry-run': Flags.boolean({ description: 'Validate every row without creating anything', default: false, @@ -62,14 +70,39 @@ export default class PersonImportCommand extends BaseCommand { throw new CliError('CSV must include a "name" column', { exitCode: 64 }) } + let matchIdx + if (flags.upsert) { + if (!flags['match-on']) { + throw new CliError('--upsert requires --match-on ', { + exitCode: 64, + }) + } + matchIdx = headers.findIndex( + (h) => h.toLowerCase() === flags['match-on'].toLowerCase(), + ) + if (matchIdx < 0) { + throw new CliError( + `--match-on "${flags['match-on']}" is not a column in ${args.file}`, + { exitCode: 64 }, + ) + } + } + const needsDefs = headers.some((h) => !(h.toLowerCase() in SPECIAL_COLUMNS)) + const defs = + needsDefs || flags.upsert ? await getFields(this.apiClient, 'person') : [] const bodies = prepareImportBodies({ headers, rows, specialColumns: SPECIAL_COLUMNS, - defs: needsDefs ? await getFields(this.apiClient, 'person') : [], + defs, }) + if (flags.upsert) { + await this.upsertRows({ args, flags, rows, bodies, matchIdx, defs }) + return + } + if (flags['dry-run']) { this.log(chalk.green(`${bodies.length} rows valid — nothing created`)) return @@ -115,4 +148,56 @@ export default class PersonImportCommand extends BaseCommand { ) } } + + /** Idempotent CSV path: match each row on --match-on, then create or PATCH. */ + async upsertRows({ args, flags, rows, bodies, matchIdx, defs }) { + const matchOn = flags['match-on'] + const items = bodies.map((body, i) => ({ body, value: rows[i][matchIdx] })) + + if (!flags['dry-run']) { + const ok = await confirmAction( + `Upsert ${items.length} persons from ${args.file} (match on ${matchOn})?`, + flags.yes, + ) + if (!ok) { + throw new CliError('Aborted', { exitCode: 1 }) + } + } + + const spinner = ora(`Upserting ${items.length} persons...`).start() + let summary + try { + summary = await bulkUpsertRows({ + client: this.apiClient, + entity: 'person', + matchOn, + rows: items, + defs, + dryRun: flags['dry-run'], + onProgress: (done, total) => { + spinner.text = `Upserting persons ${done}/${total}` + }, + }) + } finally { + spinner.stop() + } + + const { created, updated, unchanged } = summary.counts + const prefix = flags['dry-run'] ? '[dry-run] ' : '' + this.log( + chalk.green( + `${prefix}${created} created, ${updated} updated, ${unchanged} unchanged`, + ), + ) + + if (summary.failed.length > 0) { + for (const { item, error } of summary.failed) { + this.log(chalk.red(` ✘ ${matchOn}="${item.value}": ${error}`)) + } + throw new CliError( + `${summary.failed.length} of ${items.length} rows failed`, + { exitCode: 1 }, + ) + } + } } diff --git a/src/lib/upsert.js b/src/lib/upsert.js index c4396db..12c819e 100644 --- a/src/lib/upsert.js +++ b/src/lib/upsert.js @@ -3,6 +3,7 @@ import { eq } from './backup-diff.js' import { lookupByField } from './lookup.js' import { getFields } from './fields.js' import { buildWriteBody } from './input.js' +import { bulkRun } from './bulk.js' /** Entity → v2 write path. */ const WRITE_PATH = { @@ -144,6 +145,62 @@ export async function upsertWithDefs({ return runUpsert({ client, entity, by, value, body, defs, dryRun }) } +/** + * Upsert a batch of prepared bodies — one per CSV row — each matched on the + * shared `matchOn` field using that row's own `value`. Runs sequentially + * through bulkRun's pacing; a per-row failure (empty match value, ambiguous + * match, or API error) is collected, never aborting the batch. The surviving + * results are tallied by action so the caller can report created/updated/ + * unchanged counts. + * + * @param {object} options + * @param {{ get, post, patch }} options.client + * @param {'person'|'org'|'deal'} options.entity + * @param {string} options.matchOn the match field shared by every row + * @param {{ body: object, value: string }[]} options.rows + * @param {object[]} options.defs field definitions (for a custom match field) + * @param {boolean} [options.dryRun] + * @param {number} [options.gapMs] pacing gap forwarded to bulkRun + * @param {(done: number, total: number) => void} [options.onProgress] + * @returns {Promise<{ succeeded, failed, counts: { created: number, + * updated: number, unchanged: number } }>} + */ +export async function bulkUpsertRows({ + client, + entity, + matchOn, + rows, + defs = [], + dryRun = false, + gapMs, + onProgress, +}) { + const summary = await bulkRun( + rows, + ({ body, value }) => { + if (value == null || value === '') { + throw new CliError(`empty "${matchOn}" value — cannot match a row`, { + exitCode: 65, + }) + } + return runUpsert({ + client, + entity, + by: matchOn, + value, + body, + defs, + dryRun, + }) + }, + { gapMs, onProgress }, + ) + + const counts = { created: 0, updated: 0, unchanged: 0 } + for (const { result } of summary.succeeded) counts[result.action] += 1 + return { ...summary, counts } +} + /** One-line human summary of an upsert result for table output. */ export function summarizeUpsert(result, entity) { const prefix = result.dryRun ? '[dry-run] would ' : '' diff --git a/test/commands/org/import.test.js b/test/commands/org/import.test.js index 7fa7708..132e8ad 100644 --- a/test/commands/org/import.test.js +++ b/test/commands/org/import.test.js @@ -184,3 +184,155 @@ describe('org import unnamed failures', () => { ) }) }) + +describe('org import --upsert', () => { + beforeEach(() => { + nock.cleanAll() + clearFieldsCache() + mockConfirmAction.mockReset() + mockConfirmAction.mockResolvedValue(true) + mockReadFileSync.mockReset() + mockResolveCredentials.mockResolvedValue({ + mode: 'token', + companyDomain: 'acme', + token: 'tok', + source: 'profile', + }) + }) + + afterEach(() => nock.cleanAll()) + + function mockFields() { + mockApi() + .get('/api/v2/organizationFields') + .reply(200, { success: true, data: [] }) + } + + it('creates new rows and patches matched ones, reporting counts', async () => { + mockFields() + mockReadFileSync.mockReturnValue('name,owner_id\nAcme,42\nGlobex,5\n') + mockApi() + .get('/api/v2/organizations/search') + .query((q) => q.term === 'Acme') + .reply(200, { + success: true, + data: { items: [{ item: { id: 7, name: 'Acme', owner_id: 1 } }] }, + additional_data: { next_cursor: null }, + }) + mockApi() + .get('/api/v2/organizations/search') + .query((q) => q.term === 'Globex') + .reply(200, { + success: true, + data: { items: [] }, + additional_data: { next_cursor: null }, + }) + mockApi() + .patch('/api/v2/organizations/7') + .reply(200, { success: true, data: { id: 7 } }) + mockApi() + .post('/api/v2/organizations') + .reply(201, { success: true, data: { id: 8 } }) + + const stdout = await runCmd(OrgImportCommand, [ + 'o.csv', + '--upsert', + '--match-on', + 'name', + '--yes', + ]) + expect(stdout).toContain('1 created') + expect(stdout).toContain('1 updated') + }) + + it('requires --match-on', async () => { + mockReadFileSync.mockReturnValue('name\nAcme\n') + nock.disableNetConnect() + try { + await expect( + OrgImportCommand.run(['o.csv', '--upsert', '--yes']), + ).rejects.toThrow(/match-on/i) + } finally { + nock.enableNetConnect() + } + }) + + it('rejects a --match-on column that is not in the CSV', async () => { + mockReadFileSync.mockReturnValue('name\nAcme\n') + nock.disableNetConnect() + try { + await expect( + OrgImportCommand.run([ + 'o.csv', + '--upsert', + '--match-on', + 'industry', + '--yes', + ]), + ).rejects.toThrow(/column/i) + } finally { + nock.enableNetConnect() + } + }) + + it('--dry-run looks up but writes nothing', async () => { + mockFields() + mockReadFileSync.mockReturnValue('name\nAcme\n') + mockApi() + .get('/api/v2/organizations/search') + .query((q) => q.term === 'Acme') + .reply(200, { + success: true, + data: { items: [] }, + additional_data: { next_cursor: null }, + }) + + const stdout = await runCmd(OrgImportCommand, [ + 'o.csv', + '--upsert', + '--match-on', + 'name', + '--dry-run', + ]) + expect(stdout).toContain('[dry-run]') + expect(stdout).toContain('1 created') + expect(mockConfirmAction).not.toHaveBeenCalled() + }) + + it('collects an ambiguous row as a failure and exits 1', async () => { + mockFields() + mockReadFileSync.mockReturnValue('name\ndup\n') + mockApi() + .get('/api/v2/organizations/search') + .query((q) => q.term === 'dup') + .reply(200, { + success: true, + data: { + items: [ + { item: { id: 1, name: 'dup' } }, + { item: { id: 2, name: 'dup' } }, + ], + }, + additional_data: { next_cursor: null }, + }) + + await expect( + OrgImportCommand.run([ + 'o.csv', + '--upsert', + '--match-on', + 'name', + '--yes', + ]), + ).rejects.toThrow(/1 of 1/i) + }) + + it('aborts when the upsert confirmation is declined', async () => { + mockFields() + mockReadFileSync.mockReturnValue('name\nAcme\n') + mockConfirmAction.mockResolvedValue(false) + await expect( + OrgImportCommand.run(['o.csv', '--upsert', '--match-on', 'name']), + ).rejects.toThrow(/abort/i) + }) +}) diff --git a/test/commands/person/import.test.js b/test/commands/person/import.test.js index 1d78864..498c8dc 100644 --- a/test/commands/person/import.test.js +++ b/test/commands/person/import.test.js @@ -207,3 +207,167 @@ describe('person import custom-field headers and unnamed failures', () => { ) }) }) + +describe('person import --upsert', () => { + beforeEach(() => { + nock.cleanAll() + mockConfirmAction.mockReset() + mockConfirmAction.mockResolvedValue(true) + mockReadFileSync.mockReset() + mockResolveCredentials.mockResolvedValue({ + mode: 'token', + companyDomain: 'acme', + token: 'tok', + source: 'profile', + }) + }) + + afterEach(() => nock.cleanAll()) + + async function withEmptyFields(fn) { + const { clearFieldsCache } = await import('../../../src/lib/fields.js') + clearFieldsCache() + mockApi() + .get('/api/v2/personFields') + .reply(200, { success: true, data: [] }) + return fn() + } + + it('creates new rows and patches matched ones, reporting counts', async () => { + await withEmptyFields(async () => { + mockReadFileSync.mockReturnValue( + 'name,email,owner_id\nJane,a@x.com,42\nBob,b@x.com,5\n', + ) + mockApi() + .get('/api/v2/persons/search') + .query((q) => q.term === 'a@x.com') + .reply(200, { + success: true, + data: { + items: [ + { item: { id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 } }, + ], + }, + additional_data: { next_cursor: null }, + }) + mockApi() + .get('/api/v2/persons/search') + .query((q) => q.term === 'b@x.com') + .reply(200, { + success: true, + data: { items: [] }, + additional_data: { next_cursor: null }, + }) + mockApi() + .patch('/api/v2/persons/7') + .reply(200, { success: true, data: { id: 7 } }) + mockApi() + .post('/api/v2/persons') + .reply(201, { success: true, data: { id: 8 } }) + + const stdout = await runCmd(PersonImportCommand, [ + 'p.csv', + '--upsert', + '--match-on', + 'email', + '--yes', + ]) + expect(stdout).toContain('1 created') + expect(stdout).toContain('1 updated') + }) + }) + + it('requires --match-on', async () => { + mockReadFileSync.mockReturnValue('name,email\nJane,a@x.com\n') + nock.disableNetConnect() + try { + await expect( + PersonImportCommand.run(['p.csv', '--upsert', '--yes']), + ).rejects.toThrow(/match-on/i) + } finally { + nock.enableNetConnect() + } + }) + + it('rejects a --match-on column that is not in the CSV', async () => { + mockReadFileSync.mockReturnValue('name,email\nJane,a@x.com\n') + nock.disableNetConnect() + try { + await expect( + PersonImportCommand.run([ + 'p.csv', + '--upsert', + '--match-on', + 'phone', + '--yes', + ]), + ).rejects.toThrow(/column/i) + } finally { + nock.enableNetConnect() + } + }) + + it('--dry-run looks up but writes nothing', async () => { + await withEmptyFields(async () => { + mockReadFileSync.mockReturnValue('name,email\nJane,a@x.com\n') + mockApi() + .get('/api/v2/persons/search') + .query((q) => q.term === 'a@x.com') + .reply(200, { + success: true, + data: { items: [] }, + additional_data: { next_cursor: null }, + }) + + const stdout = await runCmd(PersonImportCommand, [ + 'p.csv', + '--upsert', + '--match-on', + 'email', + '--dry-run', + ]) + expect(stdout).toContain('[dry-run]') + expect(stdout).toContain('1 created') + expect(mockConfirmAction).not.toHaveBeenCalled() + }) + }) + + it('collects an ambiguous row as a failure and exits 1', async () => { + await withEmptyFields(async () => { + mockReadFileSync.mockReturnValue('name,email\nJane,dup@x.com\n') + mockApi() + .get('/api/v2/persons/search') + .query((q) => q.term === 'dup@x.com') + .reply(200, { + success: true, + data: { + items: [ + { item: { id: 1, emails: [{ value: 'dup@x.com' }] } }, + { item: { id: 2, emails: [{ value: 'dup@x.com' }] } }, + ], + }, + additional_data: { next_cursor: null }, + }) + + await expect( + PersonImportCommand.run([ + 'p.csv', + '--upsert', + '--match-on', + 'email', + '--yes', + ]), + ).rejects.toThrow(/1 of 1/i) + }) + }) + + it('aborts when the upsert confirmation is declined', async () => { + await withEmptyFields(async () => { + mockReadFileSync.mockReturnValue('name,email\nJane,a@x.com\n') + mockConfirmAction.mockResolvedValue(false) + await expect( + PersonImportCommand.run(['p.csv', '--upsert', '--match-on', 'email']), + ).rejects.toThrow(/abort/i) + }) + }) +}) diff --git a/test/lib/upsert.test.js b/test/lib/upsert.test.js index bc0104c..0993a68 100644 --- a/test/lib/upsert.test.js +++ b/test/lib/upsert.test.js @@ -1,5 +1,10 @@ import { describe, it, expect } from 'vitest' -import { diffBody, runUpsert, summarizeUpsert } from '../../src/lib/upsert.js' +import { + diffBody, + runUpsert, + summarizeUpsert, + bulkUpsertRows, +} from '../../src/lib/upsert.js' describe('diffBody', () => { it('returns only the changed top-level fields', () => { @@ -298,3 +303,119 @@ describe('summarizeUpsert', () => { ) }) }) + +/** Fake client whose search results vary by the search `term`. */ +function termFakeClient(byTerm = {}) { + const calls = { post: [], patch: [], terms: [] } + return { + calls, + async get(path, opts) { + calls.terms.push(opts.query.term) + const items = byTerm[opts.query.term] ?? [] + return { + data: { items: items.map((item) => ({ item })) }, + additional_data: { next_cursor: null }, + } + }, + async post(path, opts) { + calls.post.push({ path, body: opts.body }) + return { data: { id: 99, ...opts.body } } + }, + async patch(path, opts) { + calls.patch.push({ path, body: opts.body }) + return { data: { ...opts.body } } + }, + } +} + +describe('bulkUpsertRows', () => { + it('creates new rows, patches existing ones, and tallies actions', async () => { + const client = termFakeClient({ + 'a@x.com': [{ id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 }], + 'b@x.com': [], + }) + const r = await bulkUpsertRows({ + client, + entity: 'person', + matchOn: 'email', + rows: [ + { body: { owner_id: 42 }, value: 'a@x.com' }, + { body: { name: 'Bob' }, value: 'b@x.com' }, + ], + defs: [], + gapMs: 0, + }) + expect(r.counts).toEqual({ created: 1, updated: 1, unchanged: 0 }) + expect(client.calls.patch).toHaveLength(1) + expect(client.calls.post).toHaveLength(1) + }) + + it('counts an already-matching row as unchanged', async () => { + const client = termFakeClient({ + 'a@x.com': [{ id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 }], + }) + const r = await bulkUpsertRows({ + client, + entity: 'person', + matchOn: 'email', + rows: [{ body: { owner_id: 1 }, value: 'a@x.com' }], + defs: [], + gapMs: 0, + }) + expect(r.counts).toEqual({ created: 0, updated: 0, unchanged: 1 }) + expect(client.calls.patch).toHaveLength(0) + }) + + it('collects an empty match value as a failed row', async () => { + const client = termFakeClient({}) + const r = await bulkUpsertRows({ + client, + entity: 'person', + matchOn: 'email', + rows: [{ body: {}, value: '' }], + defs: [], + gapMs: 0, + }) + expect(r.failed).toHaveLength(1) + expect(r.failed[0].error).toMatch(/empty|match/i) + expect(client.calls.post).toHaveLength(0) + }) + + it('collects an ambiguous match as a failed row without aborting', async () => { + const client = termFakeClient({ + dup: [ + { id: 1, name: 'dup' }, + { id: 2, name: 'dup' }, + ], + 'ok@x.com': [], + }) + const r = await bulkUpsertRows({ + client, + entity: 'org', + matchOn: 'name', + rows: [ + { body: {}, value: 'dup' }, + { body: { name: 'ok@x.com' }, value: 'ok@x.com' }, + ], + defs: [], + gapMs: 0, + }) + expect(r.failed).toHaveLength(1) + expect(r.counts.created).toBe(1) + }) + + it('looks up but never writes under dry-run', async () => { + const client = termFakeClient({ 'a@x.com': [] }) + const r = await bulkUpsertRows({ + client, + entity: 'person', + matchOn: 'email', + rows: [{ body: {}, value: 'a@x.com' }], + defs: [], + dryRun: true, + gapMs: 0, + }) + expect(r.counts.created).toBe(1) + expect(client.calls.post).toHaveLength(0) + }) +}) From eb0f620d2fce74ecd5c573ded7a3edaada518b25 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 12:31:41 +0200 Subject: [PATCH 5/8] fix: contrarian-round findings for v0.16 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - lookup: reject a non-numeric value for a numeric custom field (exit 65) instead of coercing to NaN — NaN loses every comparison, so a match would silently miss and create / inject a NaN value. - diffBody: set-aware field equality so re-running an unchanged upsert emits no PATCH. emails/phones compare by their value set (case-insensitive, ignoring the primary/label flags the API echoes); label_ids and multi-option custom fields compare order-insensitively. - bulk: preserve each failed item's exit code; CSV --upsert now exits 65 when every row failure is a data-validation error (ambiguous / empty match) and 1 for mixed or transport errors. - import: reject duplicate CSV column headers (exit 65) rather than silently keeping only the last cell. Dismissed: a finding that per-entity v2 /search rejects the 'fields' param — the OpenAPI spec confirms persons/deals/organizations search all accept 'fields' (enum incl. custom_fields) and 'cursor'. --- src/commands/org/import.js | 5 ++- src/commands/person/import.js | 5 ++- src/lib/bulk.js | 4 +- src/lib/import.js | 11 ++++++ src/lib/lookup.js | 15 +++++++- src/lib/upsert.js | 42 ++++++++++++++++++-- test/commands/org/import.test.js | 47 +++++++++++++++++----- test/commands/person/import.test.js | 52 ++++++++++++++++++++----- test/lib/bulk.test.js | 12 ++++++ test/lib/import.test.js | 28 ++++++++++++++ test/lib/lookup.test.js | 15 ++++++++ test/lib/upsert.test.js | 60 +++++++++++++++++++++++++++++ 12 files changed, 269 insertions(+), 27 deletions(-) diff --git a/src/commands/org/import.js b/src/commands/org/import.js index 9c314e6..e2c7402 100644 --- a/src/commands/org/import.js +++ b/src/commands/org/import.js @@ -185,9 +185,12 @@ export default class OrgImportCommand extends BaseCommand { for (const { item, error } of summary.failed) { this.log(chalk.red(` ✘ ${matchOn}="${item.value}": ${error}`)) } + // Surface 65 when every failure is a data-validation error (ambiguous + // match, empty match value); fall back to 1 for mixed/transport errors. + const allData = summary.failed.every((f) => f.exitCode === 65) throw new CliError( `${summary.failed.length} of ${items.length} rows failed`, - { exitCode: 1 }, + { exitCode: allData ? 65 : 1 }, ) } } diff --git a/src/commands/person/import.js b/src/commands/person/import.js index 35427fb..c9202c3 100644 --- a/src/commands/person/import.js +++ b/src/commands/person/import.js @@ -194,9 +194,12 @@ export default class PersonImportCommand extends BaseCommand { for (const { item, error } of summary.failed) { this.log(chalk.red(` ✘ ${matchOn}="${item.value}": ${error}`)) } + // Surface 65 when every failure is a data-validation error (ambiguous + // match, empty match value); fall back to 1 for mixed/transport errors. + const allData = summary.failed.every((f) => f.exitCode === 65) throw new CliError( `${summary.failed.length} of ${items.length} rows failed`, - { exitCode: 1 }, + { exitCode: allData ? 65 : 1 }, ) } } diff --git a/src/lib/bulk.js b/src/lib/bulk.js index 85c4d26..6987e52 100644 --- a/src/lib/bulk.js +++ b/src/lib/bulk.js @@ -97,7 +97,9 @@ export async function bulkRun( succeeded.push({ item, result }) } catch (err) { debug('bulk item %o failed: %s', item, err.message) - failed.push({ item, error: err.message }) + // Keep the exit code so the caller can map a batch of data-validation + // failures (e.g. ambiguous upsert matches → 65) to the right exit code. + failed.push({ item, error: err.message, exitCode: err.exitCode }) } onProgress?.(index + 1, items.length) } diff --git a/src/lib/import.js b/src/lib/import.js index 36b7648..6b59225 100644 --- a/src/lib/import.js +++ b/src/lib/import.js @@ -23,6 +23,17 @@ export function prepareImportBodies({ Object.entries(specialColumns).map(([k, v]) => [k.toLowerCase(), v]), ) + // Duplicate headers silently overwrite each other (last cell wins) — reject + // them up front rather than losing a column's data without warning. + const seen = new Set() + for (const header of headers) { + const lower = header.toLowerCase() + if (seen.has(lower)) { + throw new CliError(`Duplicate CSV column "${header}"`, { exitCode: 65 }) + } + seen.add(lower) + } + return rows.map((row, index) => { const typed = {} const fields = [] diff --git a/src/lib/lookup.js b/src/lib/lookup.js index cc12f0e..c17add0 100644 --- a/src/lib/lookup.js +++ b/src/lib/lookup.js @@ -113,7 +113,20 @@ export async function lookupByField({ const key = def.field_code extract = (it) => [it.custom_fields?.[key]] ci = false - compareValue = NUMERIC_TYPES.has(def.field_type) ? Number(value) : value + if (NUMERIC_TYPES.has(def.field_type)) { + compareValue = Number(value) + // A non-numeric value coerces to NaN, which loses every comparison + // (NaN !== NaN) — so a match would silently miss and we'd create or + // inject a NaN value. Refuse loudly instead. + if (!Number.isFinite(compareValue)) { + throw new CliError( + `"${value}" is not a valid number for field "${field}"`, + { exitCode: 65 }, + ) + } + } else { + compareValue = value + } } const matches = [] diff --git a/src/lib/upsert.js b/src/lib/upsert.js index 12c819e..1d17a40 100644 --- a/src/lib/upsert.js +++ b/src/lib/upsert.js @@ -46,11 +46,47 @@ function injectMatch(body, entity, field, value, defs) { : value } +/** True if every element of an array is a primitive (or null). */ +function isPrimitiveArray(value) { + return ( + Array.isArray(value) && + value.every((v) => v == null || typeof v !== 'object') + ) +} + +/** + * Idempotency-aware field equality for diffBody. Beyond key-order insensitive + * `eq`, it treats two classes of field as equal when they carry the same + * *content* regardless of incidental shape the API adds back: + * - emails/phones: compared by their set of `value`s (case-insensitive), + * ignoring the `primary`/`label` flags the API echoes — otherwise an + * injected `primary:true` would PATCH on every run. + * - set-like primitive arrays (label_ids, multi-option custom fields): + * compared order-insensitively, since the API may return them sorted. + */ +function fieldEq(incoming, existing, key) { + if ( + (key === 'emails' || key === 'phones') && + Array.isArray(incoming) && + Array.isArray(existing) + ) { + const values = (arr) => + arr.map((e) => String(e?.value ?? '').toLowerCase()).sort() + return eq(values(incoming), values(existing)) + } + if (isPrimitiveArray(incoming) && isPrimitiveArray(existing)) { + return eq([...incoming].sort(), [...existing].sort()) + } + return eq(incoming, existing) +} + /** * Field-level diff for an idempotent PATCH: the subset of `incoming` whose * value differs from `existing`. Top-level fields compare directly; the nested * v2 `custom_fields` object is diffed key by key. Equality is key-order - * insensitive. An empty result means "no change → skip the PATCH". + * insensitive and set-aware for emails/phones/primitive arrays (see fieldEq), + * so re-running an unchanged upsert produces no PATCH. An empty result means + * "no change → skip the PATCH". * @param {object} incoming the desired body * @param {object} existing the current record * @returns {object} the changed-only body @@ -61,10 +97,10 @@ export function diffBody(incoming, existing) { if (key === 'custom_fields' && value && typeof value === 'object') { const cf = {} for (const [ck, cv] of Object.entries(value)) { - if (!eq(cv, existing?.custom_fields?.[ck])) cf[ck] = cv + if (!fieldEq(cv, existing?.custom_fields?.[ck], ck)) cf[ck] = cv } if (Object.keys(cf).length > 0) changed.custom_fields = cf - } else if (!eq(value, existing?.[key])) { + } else if (!fieldEq(value, existing?.[key], key)) { changed[key] = value } } diff --git a/test/commands/org/import.test.js b/test/commands/org/import.test.js index 132e8ad..6b3db0f 100644 --- a/test/commands/org/import.test.js +++ b/test/commands/org/import.test.js @@ -299,7 +299,7 @@ describe('org import --upsert', () => { expect(mockConfirmAction).not.toHaveBeenCalled() }) - it('collects an ambiguous row as a failure and exits 1', async () => { + it('exits 65 when an ambiguous row is the only failure', async () => { mockFields() mockReadFileSync.mockReturnValue('name\ndup\n') mockApi() @@ -316,15 +316,42 @@ describe('org import --upsert', () => { additional_data: { next_cursor: null }, }) - await expect( - OrgImportCommand.run([ - 'o.csv', - '--upsert', - '--match-on', - 'name', - '--yes', - ]), - ).rejects.toThrow(/1 of 1/i) + const err = await OrgImportCommand.run([ + 'o.csv', + '--upsert', + '--match-on', + 'name', + '--yes', + ]).catch((e) => e) + expect(err.message).toMatch(/1 of 1/i) + expect(err.exitCode ?? err.oclif?.exit).toBe(65) + }) + + it('exits 1 when a row fails on a non-validation API error', async () => { + mockFields() + mockReadFileSync.mockReturnValue('name,owner_id\nAcme,42\n') + mockApi() + .get('/api/v2/organizations/search') + .query((q) => q.term === 'Acme') + .reply(200, { + success: true, + data: { items: [{ item: { id: 7, name: 'Acme', owner_id: 1 } }] }, + additional_data: { next_cursor: null }, + }) + mockApi() + .patch('/api/v2/organizations/7') + .reply(401, { success: false, error: 'unauthorized' }) + + const err = await OrgImportCommand.run([ + 'o.csv', + '--upsert', + '--match-on', + 'name', + '--no-retry', + '--yes', + ]).catch((e) => e) + expect(err.message).toMatch(/1 of 1/i) + expect(err.exitCode ?? err.oclif?.exit).toBe(1) }) it('aborts when the upsert confirmation is declined', async () => { diff --git a/test/commands/person/import.test.js b/test/commands/person/import.test.js index 498c8dc..ec473b7 100644 --- a/test/commands/person/import.test.js +++ b/test/commands/person/import.test.js @@ -332,7 +332,7 @@ describe('person import --upsert', () => { }) }) - it('collects an ambiguous row as a failure and exits 1', async () => { + it('exits 65 when an ambiguous row is the only failure', async () => { await withEmptyFields(async () => { mockReadFileSync.mockReturnValue('name,email\nJane,dup@x.com\n') mockApi() @@ -349,15 +349,47 @@ describe('person import --upsert', () => { additional_data: { next_cursor: null }, }) - await expect( - PersonImportCommand.run([ - 'p.csv', - '--upsert', - '--match-on', - 'email', - '--yes', - ]), - ).rejects.toThrow(/1 of 1/i) + const err = await PersonImportCommand.run([ + 'p.csv', + '--upsert', + '--match-on', + 'email', + '--yes', + ]).catch((e) => e) + expect(err.message).toMatch(/1 of 1/i) + expect(err.exitCode ?? err.oclif?.exit).toBe(65) + }) + }) + + it('exits 1 when a row fails on a non-validation API error', async () => { + await withEmptyFields(async () => { + mockReadFileSync.mockReturnValue('name,email,owner_id\nJane,a@x.com,42\n') + mockApi() + .get('/api/v2/persons/search') + .query((q) => q.term === 'a@x.com') + .reply(200, { + success: true, + data: { + items: [ + { item: { id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 } }, + ], + }, + additional_data: { next_cursor: null }, + }) + mockApi() + .patch('/api/v2/persons/7') + .reply(401, { success: false, error: 'unauthorized' }) + + const err = await PersonImportCommand.run([ + 'p.csv', + '--upsert', + '--match-on', + 'email', + '--no-retry', + '--yes', + ]).catch((e) => e) + expect(err.message).toMatch(/1 of 1/i) + expect(err.exitCode ?? err.oclif?.exit).toBe(1) }) }) diff --git a/test/lib/bulk.test.js b/test/lib/bulk.test.js index 4d57da9..cbe98b7 100644 --- a/test/lib/bulk.test.js +++ b/test/lib/bulk.test.js @@ -108,6 +108,18 @@ describe('bulkRun', () => { expect(result.failed).toEqual([{ item: 2, error: 'boom on 2' }]) }) + it('preserves a failed item exit code for the caller', async () => { + const { CliError } = await import('../../src/lib/errors.js') + const result = await bulkRun( + [1], + async () => { + throw new CliError('ambiguous', { exitCode: 65 }) + }, + { gapMs: 0 }, + ) + expect(result.failed[0].exitCode).toBe(65) + }) + it('reports progress per item', async () => { const progress = [] await bulkRun([1, 2], async () => ({}), { diff --git a/test/lib/import.test.js b/test/lib/import.test.js index 876b1f9..104ef2d 100644 --- a/test/lib/import.test.js +++ b/test/lib/import.test.js @@ -29,6 +29,34 @@ const PERSON_SPECIALS = { }, } +describe('prepareImportBodies duplicate headers', () => { + it('rejects a duplicate column header with exit 65 instead of losing data', () => { + expect(() => + prepareImportBodies({ + headers: ['name', 'email', 'email'], + rows: [['Jane', 'first@a.com', 'second@a.com']], + specialColumns: PERSON_SPECIALS, + defs: DEFS, + }), + ).toThrowError(/duplicate/i) + }) + + it('rejects duplicate headers case-insensitively', () => { + let caught + try { + prepareImportBodies({ + headers: ['Name', 'name'], + rows: [['Jane', 'Jane']], + specialColumns: PERSON_SPECIALS, + defs: DEFS, + }) + } catch (e) { + caught = e + } + expect(caught.exitCode).toBe(65) + }) +}) + describe('prepareImportBodies', () => { it('maps special columns and resolves the rest via field defs', () => { const bodies = prepareImportBodies({ diff --git a/test/lib/lookup.test.js b/test/lib/lookup.test.js index 3dcf3e4..4e834ae 100644 --- a/test/lib/lookup.test.js +++ b/test/lib/lookup.test.js @@ -265,6 +265,21 @@ describe('lookupByField', () => { expect(err.message).toMatch(/not searchable/i) }) + it('rejects a non-numeric value for a numeric custom field with exit 65', async () => { + const defs = [ + { field_name: 'Score', field_code: 'sc', field_type: 'double' }, + ] + const err = await lookupByField({ + client: fakeClient([]), + entity: 'deal', + defs, + field: 'Score', + value: 'notanumber', + }).catch((e) => e) + expect(err.exitCode).toBe(65) + expect(err.message).toMatch(/number/i) + }) + it('rejects an unknown field with exit 64', async () => { const err = await lookupByField({ client: fakeClient([]), diff --git a/test/lib/upsert.test.js b/test/lib/upsert.test.js index 0993a68..f5c3bd2 100644 --- a/test/lib/upsert.test.js +++ b/test/lib/upsert.test.js @@ -39,6 +39,66 @@ describe('diffBody', () => { diffBody({ custom_fields: { a: 1 } }, { custom_fields: { a: 1 } }), ).toEqual({}) }) + + it('treats emails as unchanged when the value set matches (ignores primary, case)', () => { + expect( + diffBody( + { emails: [{ value: 'A@x.com', primary: true }] }, + { emails: [{ value: 'a@x.com', label: 'work' }] }, + ), + ).toEqual({}) + }) + + it('detects a genuinely changed email set', () => { + expect( + diffBody( + { emails: [{ value: 'b@x.com', primary: true }] }, + { emails: [{ value: 'a@x.com' }] }, + ), + ).toEqual({ emails: [{ value: 'b@x.com', primary: true }] }) + }) + + it('treats phones as unchanged when the value set matches', () => { + expect( + diffBody( + { phones: [{ value: '+15551234', primary: true }] }, + { phones: [{ value: '+15551234' }] }, + ), + ).toEqual({}) + }) + + it('emits emails when the existing record has none', () => { + expect(diffBody({ emails: [{ value: 'a@x.com' }] }, {})).toEqual({ + emails: [{ value: 'a@x.com' }], + }) + }) + + it('treats a reordered primitive array (label_ids) as unchanged', () => { + expect( + diffBody({ label_ids: [2, 1, 3] }, { label_ids: [1, 2, 3] }), + ).toEqual({}) + }) + + it('detects an added id in a primitive array', () => { + expect(diffBody({ label_ids: [1, 2, 3] }, { label_ids: [1, 2] })).toEqual({ + label_ids: [1, 2, 3], + }) + }) + + it('treats a reordered multi-option custom field as unchanged', () => { + expect( + diffBody( + { custom_fields: { k: [2, 1] } }, + { custom_fields: { k: [1, 2] } }, + ), + ).toEqual({}) + }) + + it('tolerates null / value-less email entries when comparing sets', () => { + expect( + diffBody({ emails: [{ value: 'a@x.com' }] }, { emails: [null, {}] }), + ).toEqual({ emails: [{ value: 'a@x.com' }] }) + }) }) /** Fake client: queued search items for lookup + captured post/patch. */ From d568c5c29490ee601fdc640d0e2222e236e36f49 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 12:34:32 +0200 Subject: [PATCH 6/8] docs: cover v0.16 idempotent upsert + CSV --upsert; release 0.16.0 CHANGELOG 0.16.0; README idempotent-writes section; bulk guide gains an upsert (match-or-create) section + CSV --upsert/--match-on; agents page gains the upsert capability paragraph. Regenerated command reference (145 commands) and cli-stats. Version bump to 0.16.0. --- CHANGELOG.md | 30 ++++++++ README.md | 12 ++++ docs/commands.md | 69 ++++++++++++++++++- package-lock.json | 4 +- package.json | 2 +- website/src/content/docs/guides/bulk.mdx | 66 ++++++++++++++++++ .../src/content/docs/reference/commands.mdx | 69 ++++++++++++++++++- website/src/content/docs/start/agents.mdx | 7 ++ website/src/data/cli-stats.json | 4 +- 9 files changed, 256 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca38dd6..5490b89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,36 @@ All notable changes to `pdcli` are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/); versions follow [SemVer](https://semver.org/). +## [0.16.0] - 2026-06-11 + +### Added + +- Idempotent writes — match-or-create that never guesses: + - `person upsert`, `org upsert`, `deal upsert` — match a record by `--by` + (a built-in key — person email/name/phone, org name, deal title — or a + searchable custom field), then **create** it if absent or **PATCH only the + changed fields** if exactly one matches. More than one match **refuses with + exit 65** rather than writing the wrong record: search `exact_match` is not + a unique key, so every candidate is re-verified client-side before the + count decides. `--dry-run` previews the action; table prints a one-line + summary, `--output json` emits the full action result. + - `person import --upsert --match-on ` and the same on `org import` — + the CSV equivalent: each row is matched on its `--match-on` value, then + created or PATCHed. Per-row failures (ambiguous matches, empty match + values) are collected without aborting the batch and reported as + `created / updated / unchanged` counts; a batch whose failures are all + data-validation errors exits 65, otherwise 1. `--dry-run` looks up and + reports the counts without writing. + +### Changed + +- `diffBody` (upsert) compares emails/phones by their value set + (case-insensitive, ignoring the `primary`/`label` flags the API echoes) and + treats `label_ids` and multi-option custom fields order-insensitively, so + re-running an unchanged upsert issues no PATCH. +- CSV import now rejects duplicate column headers (exit 65) instead of silently + keeping only the last cell. + ## [0.15.0] - 2026-06-11 ### Added diff --git a/README.md b/README.md index 83c4b6c..c16e001 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,18 @@ pdcli person import people.csv --dry-run # CSV headers map to fiel pdcli person import people.csv # custom fields by name ``` +## Idempotent writes + +Match-or-create, safe to re-run. More than one match **refuses** (exit 65) — +never guesses which record to write. + +```bash +pdcli person upsert a@x.com --by email --field "Tier=Gold" # create or PATCH only what changed +pdcli deal upsert "Acme expansion" --by title --body '{"value":5000}' +pdcli org upsert "D-42" --by "External ID" --field "Status=Active" --dry-run # preview +pdcli person import contacts.csv --upsert --match-on email # CSV: per-row create-or-update +``` + ## Analytics & housekeeping ```bash diff --git a/docs/commands.md b/docs/commands.md index ce239c2..7d533f2 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -5,7 +5,7 @@ description: Full command reference for the pdcli command-line interface. -Reference for `pdcli` v0.15.0 (142 commands). Every command also accepts the global flags `--output table|json|yaml|csv`, `--profile`, `--no-color`, `--verbose`, `--no-retry`, `--timeout`, and `--limit`. +Reference for `pdcli` v0.16.0 (145 commands). Every command also accepts the global flags `--output table|json|yaml|csv`, `--profile`, `--no-color`, `--verbose`, `--no-retry`, `--timeout`, and `--limit`. ## Top-level @@ -958,6 +958,27 @@ pdcli deal update 42 --status won pdcli deal update 42 --field "Deal Size=Large" ``` +### `pdcli deal upsert` + +Idempotent deal upsert: match by --by, then create or PATCH only the changed fields. Refuses (exit 65) if more than one record matches. + +``` +pdcli deal upsert [flags] +``` + +- `--by ` _(required)_ — Match field: title, or a searchable custom field +- `--field ` — Field to set as "Name=Value" (repeatable) +- `--body ` — Raw JSON body to merge +- `--dry-run` — Preview the action without writing + +Examples: + +```bash +pdcli deal upsert "Acme expansion" --by title --field "Stage=Won" +pdcli deal upsert "D-42" --by "External ID" --body '{"value":5000}' +pdcli deal upsert "Acme expansion" --by title --field "Stage=Won" --dry-run +``` + ## pdcli field ### `pdcli field create` @@ -1798,6 +1819,8 @@ Bulk-create organizations from a CSV (headers map to fields, custom fields by na pdcli org import [flags] ``` +- `--upsert` — Match each row on --match-on, then create or update +- `--match-on ` — Field to match rows on in --upsert mode (e.g. name) - `--dry-run` — Validate every row without creating anything - `-y, --yes` — Skip the confirmation prompt @@ -1923,6 +1946,27 @@ pdcli org update 7 --owner 9 pdcli org update 7 --field "Tier=Gold" ``` +### `pdcli org upsert` + +Idempotent organization upsert: match by --by, then create or PATCH only the changed fields. Refuses (exit 65) if more than one record matches. + +``` +pdcli org upsert [flags] +``` + +- `--by ` _(required)_ — Match field: name, or a searchable custom field +- `--field ` — Field to set as "Name=Value" (repeatable) +- `--body ` — Raw JSON body to merge +- `--dry-run` — Preview the action without writing + +Examples: + +```bash +pdcli org upsert Acme --by name --field "Tier=Gold" +pdcli org upsert "D-42" --by "External ID" --body '{"owner_id":42}' +pdcli org upsert Acme --by name --field "Tier=Gold" --dry-run +``` + ## pdcli person ### `pdcli person create` @@ -2039,6 +2083,8 @@ Bulk-create persons from a CSV (headers map to fields, custom fields by name) pdcli person import [flags] ``` +- `--upsert` — Match each row on --match-on, then create or update +- `--match-on ` — Field to match rows on in --upsert mode (e.g. email) - `--dry-run` — Validate every row without creating anything - `-y, --yes` — Skip the confirmation prompt @@ -2115,6 +2161,27 @@ pdcli person update 42 --email new@acme.com pdcli person update 42 --field "Segment=Enterprise" ``` +### `pdcli person upsert` + +Idempotent person upsert: match by --by, then create or PATCH only the changed fields. Refuses (exit 65) if more than one record matches. + +``` +pdcli person upsert [flags] +``` + +- `--by ` _(required)_ — Match field: email, name, phone, or a searchable custom field +- `--field ` — Field to set as "Name=Value" (repeatable) +- `--body ` — Raw JSON body to merge +- `--dry-run` — Preview the action without writing + +Examples: + +```bash +pdcli person upsert a@x.com --by email --field "Tier=Gold" +pdcli person upsert "Jane Doe" --by name --body '{"owner_id":42}' +pdcli person upsert a@x.com --by email --field "Tier=Gold" --dry-run +``` + ## pdcli pipeline ### `pdcli pipeline get` diff --git a/package-lock.json b/package-lock.json index 5096067..bcf3953 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@wavyx/pdcli", - "version": "0.15.0", + "version": "0.16.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@wavyx/pdcli", - "version": "0.15.0", + "version": "0.16.0", "license": "MIT", "dependencies": { "@inquirer/prompts": "8.5.0", diff --git a/package.json b/package.json index 564ce42..90054cb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@wavyx/pdcli", - "version": "0.15.0", + "version": "0.16.0", "publishConfig": { "access": "public" }, diff --git a/website/src/content/docs/guides/bulk.mdx b/website/src/content/docs/guides/bulk.mdx index 074814c..6d88469 100644 --- a/website/src/content/docs/guides/bulk.mdx +++ b/website/src/content/docs/guides/bulk.mdx @@ -101,6 +101,8 @@ each deal succeeds or fails on its own. `person import ` and `org import ` bulk-create records from a CSV. The first row is the header; **a `name` column is required** (case-insensitive), or it's exit 64. +Duplicate column headers are rejected (exit 65) rather than silently keeping only the last +cell. Each header maps to a field. Recognized special columns build standard values directly; every other header is resolved through the entity's field definitions — by human name, @@ -145,3 +147,67 @@ Imported 23/24 persons ✘ John Roe: Pipedrive API 422: email already in use Error: 1 of 24 rows failed ``` + +## Idempotent upsert (match-or-create) + +`person upsert`, `org upsert`, and `deal upsert` make a write **safe to re-run**: match an +existing record, then **create** it if absent or **PATCH only the fields that changed** if +exactly one matches. Re-running the same command a second time is a no-op. + +```bash +pdcli person upsert a@x.com --by email --field "Tier=Gold" +pdcli deal upsert "Acme expansion" --by title --body '{"value":5000}' +pdcli org upsert "D-42" --by "External ID" --field "Status=Active" +``` + +The match value is the positional argument; `--by` names the field to match on — a built-in +key or a **searchable** custom field (`address`, `varchar`, `text`, `double`, `monetary`, +`phone`): + +| Entity | Built-in `--by` keys | +| ------ | --------------------- | +| person | `email`, `name`, `phone` | +| org | `name` | +| deal | `title` | + +### Why it refuses instead of guessing + +Pipedrive's `exact_match` search is **not** a unique-key lookup — it's case-insensitive, and +a custom-field search scans every custom field at once. So pdcli re-verifies each candidate +client-side and counts the survivors: + +- **0 matches** → create (the match value is injected into the new record). +- **1 match** → PATCH only the differing fields; if nothing differs, it reports `unchanged` + and issues no write. +- **more than 1** → **refuse with exit 65**, listing the colliding IDs. It never picks one. + +```text +Error: --by email="a@x.com" matches 2 person records (ids: 7, 12) — refusing to guess. Narrow the match value. +``` + +`--dry-run` previews the action without writing. Table output prints a one-line summary +(`update person #7 (2 fields)`); `--output json` emits the full action result +(`action`, `id`, `changed`). + +### Upserting a whole CSV + +`person import` and `org import` gain `--upsert --match-on `: each row is matched on +its value in the `--match-on` column, then created or PATCHed like the single-record upsert. + +```bash +pdcli person import contacts.csv --upsert --match-on email --dry-run # preview the counts +pdcli person import contacts.csv --upsert --match-on email # apply +``` + +`--match-on` is required with `--upsert` and must name a column present in the CSV (else exit +64). Rows are paced like the other bulk flows; per-row failures — an ambiguous match or an +empty match cell — are collected without aborting the batch and summarized by action: + +```text +18 created, 5 updated, 1 unchanged + ✘ email="dup@x.com": --by email="dup@x.com" matches 2 person records (ids: 7, 12) — refusing to guess. Narrow the match value. +Error: 1 of 24 rows failed +``` + +When every failed row is a data-validation error (ambiguous match, empty match value) the +command exits **65**; a mix that includes a transport/API error exits **1**. diff --git a/website/src/content/docs/reference/commands.mdx b/website/src/content/docs/reference/commands.mdx index fcfd476..57ec8b8 100644 --- a/website/src/content/docs/reference/commands.mdx +++ b/website/src/content/docs/reference/commands.mdx @@ -5,7 +5,7 @@ description: Every pdcli command, flag, and example — generated from the CLI m {/* AUTO-GENERATED from the oclif manifest by scripts/gen-commands.mjs — do not edit by hand. */} -All 142 commands in `pdcli` v0.15.0. Every command also +All 145 commands in `pdcli` v0.16.0. Every command also accepts the [global flags](/pdcli/reference/config/) `--output`, `--jq`, `--fields`, `--profile`, `--limit`, `--no-color`, `--verbose`, `--no-retry`, and `--timeout`. Run `pdcli --help` for the live version. @@ -929,6 +929,27 @@ pdcli deal update 42 --status won pdcli deal update 42 --field "Deal Size=Large" ``` +### `pdcli deal upsert` + +Idempotent deal upsert: match by --by, then create or PATCH only the changed fields. Refuses (exit 65) if more than one record matches. + +```text +pdcli deal upsert [flags] +``` + +| Flag | Description | +| --- | --- | +| `--by` <value> | Match field: title, or a searchable custom field | +| `--field` <value> | Field to set as "Name=Value" (repeatable) | +| `--body` <value> | Raw JSON body to merge | +| `--dry-run` | Preview the action without writing | + +```bash +pdcli deal upsert "Acme expansion" --by title --field "Stage=Won" +pdcli deal upsert "D-42" --by "External ID" --body '{"value":5000}' +pdcli deal upsert "Acme expansion" --by title --field "Stage=Won" --dry-run +``` + ## pdcli field ### `pdcli field create` @@ -1749,6 +1770,8 @@ pdcli org import [flags] | Flag | Description | | --- | --- | +| `--upsert` | Match each row on --match-on, then create or update | +| `--match-on` <value> | Field to match rows on in --upsert mode (e.g. name) | | `--dry-run` | Validate every row without creating anything | | `-y, --yes` | Skip the confirmation prompt | @@ -1872,6 +1895,27 @@ pdcli org update 7 --owner 9 pdcli org update 7 --field "Tier=Gold" ``` +### `pdcli org upsert` + +Idempotent organization upsert: match by --by, then create or PATCH only the changed fields. Refuses (exit 65) if more than one record matches. + +```text +pdcli org upsert [flags] +``` + +| Flag | Description | +| --- | --- | +| `--by` <value> | Match field: name, or a searchable custom field | +| `--field` <value> | Field to set as "Name=Value" (repeatable) | +| `--body` <value> | Raw JSON body to merge | +| `--dry-run` | Preview the action without writing | + +```bash +pdcli org upsert Acme --by name --field "Tier=Gold" +pdcli org upsert "D-42" --by "External ID" --body '{"owner_id":42}' +pdcli org upsert Acme --by name --field "Tier=Gold" --dry-run +``` + ## pdcli person ### `pdcli person create` @@ -1986,6 +2030,8 @@ pdcli person import [flags] | Flag | Description | | --- | --- | +| `--upsert` | Match each row on --match-on, then create or update | +| `--match-on` <value> | Field to match rows on in --upsert mode (e.g. email) | | `--dry-run` | Validate every row without creating anything | | `-y, --yes` | Skip the confirmation prompt | @@ -2060,6 +2106,27 @@ pdcli person update 42 --email new@acme.com pdcli person update 42 --field "Segment=Enterprise" ``` +### `pdcli person upsert` + +Idempotent person upsert: match by --by, then create or PATCH only the changed fields. Refuses (exit 65) if more than one record matches. + +```text +pdcli person upsert [flags] +``` + +| Flag | Description | +| --- | --- | +| `--by` <value> | Match field: email, name, phone, or a searchable custom field | +| `--field` <value> | Field to set as "Name=Value" (repeatable) | +| `--body` <value> | Raw JSON body to merge | +| `--dry-run` | Preview the action without writing | + +```bash +pdcli person upsert a@x.com --by email --field "Tier=Gold" +pdcli person upsert "Jane Doe" --by name --body '{"owner_id":42}' +pdcli person upsert a@x.com --by email --field "Tier=Gold" --dry-run +``` + ## pdcli pipeline ### `pdcli pipeline get` diff --git a/website/src/content/docs/start/agents.mdx b/website/src/content/docs/start/agents.mdx index 94b7a34..97187ee 100644 --- a/website/src/content/docs/start/agents.mdx +++ b/website/src/content/docs/start/agents.mdx @@ -92,6 +92,13 @@ an exit-code-gated anomaly poller that fires only on findings new since the last (`pdcli watch || notify`), `sync warehouse` for an incremental NDJSON export with per-entity high-water marks, and `--updated-since` on the list commands for incremental polling. +For writes that must be safe to retry, `person`/`org`/`deal upsert` match a record by `--by` +(a built-in key or a searchable custom field) and then create or PATCH only what changed — +and **refuse with exit 65** when more than one record matches, so an agent never silently +writes the wrong one. `person import`/`org import --upsert --match-on ` apply the same +match-or-create per CSV row, reporting created/updated/unchanged counts. Pair upsert with an +external key (a custom field carrying your system's ID) for clean, repeatable sync. + ## The host-locked `api` escape hatch When no dedicated command exists, call any endpoint directly. The request is diff --git a/website/src/data/cli-stats.json b/website/src/data/cli-stats.json index 247efc2..c868b3c 100644 --- a/website/src/data/cli-stats.json +++ b/website/src/data/cli-stats.json @@ -1,6 +1,6 @@ { - "version": "0.15.0", - "commands": 142, + "version": "0.16.0", + "commands": 145, "topics": 26, "formats": 4 } From 15ef84ceb046fdbaacb15ae203b3365d1f986c6c Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 12:54:59 +0200 Subject: [PATCH 7/8] fix(lookup): correct upsert matching against the live search API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs only live testing on the sandbox surfaced: - The per-entity /search endpoints cap `limit` at 100 (the API 400s on more, despite the 500 list cap) — was sending 500, so every lookup errored. - The search result `item` is a lossy projection: emails/phones come back as bare strings (not {value} objects) and custom_fields as an unattributed value array. Verifying against it (and using it as the record to diff) silently discarded every match, so each upsert created a duplicate. lookupByField now treats the scoped exact_match search purely as a candidate-id finder, then fetches each candidate's full record and re-verifies the matched field against the authoritative shape — which also gives diffBody a correct record. Verified live: create → unchanged on re-run → updated on change → exit 65 on a duplicate. --- src/lib/lookup.js | 58 +++++-- test/commands/deal/upsert.test.js | 7 +- test/commands/org/import.test.js | 26 +++- test/commands/org/upsert.test.js | 7 +- test/commands/person/import.test.js | 43 ++++-- test/commands/person/upsert.test.js | 12 +- test/lib/lookup.test.js | 230 +++++++++++++++++++--------- test/lib/upsert.test.js | 41 +++-- 8 files changed, 297 insertions(+), 127 deletions(-) diff --git a/src/lib/lookup.js b/src/lib/lookup.js index c17add0..b93235c 100644 --- a/src/lib/lookup.js +++ b/src/lib/lookup.js @@ -13,13 +13,15 @@ const SEARCHABLE_TYPES = new Set([ const NUMERIC_TYPES = new Set(['double', 'monetary']) /** - * Per-entity scoped-search config + built-in match fields. Each built-in gives - * the search `fields` scope, how to pull the comparable value(s) off a result - * record, and whether the compare is case-insensitive. + * Per-entity config: the scoped-search endpoint, the record-fetch base, and + * the built-in match fields. Each built-in gives the search `fields` scope, + * how to pull the comparable value(s) off the *full* record (NOT the search + * item — see lookupByField), and whether the compare is case-insensitive. */ const SEARCH_CONFIG = { person: { searchPath: '/api/v2/persons/search', + recordPath: '/api/v2/persons', builtins: { email: { scope: 'email', @@ -36,12 +38,14 @@ const SEARCH_CONFIG = { }, org: { searchPath: '/api/v2/organizations/search', + recordPath: '/api/v2/organizations', builtins: { name: { scope: 'name', extract: (it) => [it.name], ci: false }, }, }, deal: { searchPath: '/api/v2/deals/search', + recordPath: '/api/v2/deals', builtins: { title: { scope: 'title', extract: (it) => [it.title], ci: false }, }, @@ -59,11 +63,16 @@ function valuesEqual(actual, expected, ci) { /** * Find the record of `entity` whose `field` exactly equals `value`. Routes a * built-in field (person email/name/phone, org name, deal title) or a - * searchable custom field (by name) to the scoped /search endpoint, collects - * ALL matches across cursor pages, then RE-VERIFIES each client-side — because - * `exact_match` is not a unique-key lookup (it's case-insensitive and, for - * custom fields, searches every custom field at once). The surviving count - * decides the tri-state: 0 → create, 1 → update, >1 → caller must refuse. + * searchable custom field (by name) to the scoped /search endpoint to collect + * candidate ids across cursor pages, then FETCHES each candidate's full record + * and RE-VERIFIES it client-side. The fetch is not optional: the search result + * `item` is a lossy projection (emails/phones come back as bare strings, + * custom_fields as an unattributed value array) so it can neither be trusted + * for verification nor used as the record to diff against. `exact_match` is + * also not a unique-key lookup (case-insensitive; a custom-field search scans + * every custom field at once), which is the other reason to re-verify against + * the authoritative record. The surviving count decides the tri-state: + * 0 → create, 1 → update, >1 → caller must refuse. * * @param {{ get: Function }} client * @param {'person'|'org'|'deal'} entity @@ -129,7 +138,10 @@ export async function lookupByField({ } } - const matches = [] + // 1. Collect candidate ids from the scoped exact_match search (the cheap + // finder). The item body is lossy, so we keep only the id. + const candidateIds = [] + const seen = new Set() let cursor do { const body = await client.get(cfg.searchPath, { @@ -137,17 +149,35 @@ export async function lookupByField({ term: value, exact_match: true, fields: scope, - limit: 500, + // The per-entity /search endpoints cap limit at 100 (the live API 400s + // on more, despite the 500 list cap); cursor paging collects the rest. + limit: 100, cursor, }, }) - for (const entry of body.data?.items ?? []) matches.push(entry.item) + for (const entry of body.data?.items ?? []) { + const id = entry.item?.id + if (id != null && !seen.has(id)) { + seen.add(id) + candidateIds.push(id) + } + } cursor = body.additional_data?.next_cursor ?? null } while (cursor) - const survivors = matches.filter((item) => - extract(item).some((v) => valuesEqual(v, compareValue, ci)), - ) + // 2. Fetch each candidate's full record and re-verify the matched field + // against its authoritative value(s). The surviving records are both the + // ambiguity count and (when unique) the record to diff against. + const survivors = [] + for (const id of candidateIds) { + const record = (await client.get(`${cfg.recordPath}/${id}`)).data + if ( + record && + extract(record).some((v) => valuesEqual(v, compareValue, ci)) + ) { + survivors.push(record) + } + } if (survivors.length === 0) return { status: 'none' } if (survivors.length === 1) { diff --git a/test/commands/deal/upsert.test.js b/test/commands/deal/upsert.test.js index a6612e8..dfbe3b6 100644 --- a/test/commands/deal/upsert.test.js +++ b/test/commands/deal/upsert.test.js @@ -26,9 +26,14 @@ function mockSearch(records = []) { .query(true) .reply(200, { success: true, - data: { items: records.map((item) => ({ item })) }, + data: { items: records.map((r) => ({ item: { id: r.id } })) }, additional_data: { next_cursor: null }, }) + for (const r of records) { + mockApi() + .get(`/api/v2/deals/${r.id}`) + .reply(200, { success: true, data: r }) + } } describe('deal upsert', () => { diff --git a/test/commands/org/import.test.js b/test/commands/org/import.test.js index 6b3db0f..5f525f5 100644 --- a/test/commands/org/import.test.js +++ b/test/commands/org/import.test.js @@ -216,9 +216,15 @@ describe('org import --upsert', () => { .query((q) => q.term === 'Acme') .reply(200, { success: true, - data: { items: [{ item: { id: 7, name: 'Acme', owner_id: 1 } }] }, + data: { items: [{ item: { id: 7 } }] }, additional_data: { next_cursor: null }, }) + mockApi() + .get('/api/v2/organizations/7') + .reply(200, { + success: true, + data: { id: 7, name: 'Acme', owner_id: 1 }, + }) mockApi() .get('/api/v2/organizations/search') .query((q) => q.term === 'Globex') @@ -307,14 +313,15 @@ describe('org import --upsert', () => { .query((q) => q.term === 'dup') .reply(200, { success: true, - data: { - items: [ - { item: { id: 1, name: 'dup' } }, - { item: { id: 2, name: 'dup' } }, - ], - }, + data: { items: [{ item: { id: 1 } }, { item: { id: 2 } }] }, additional_data: { next_cursor: null }, }) + mockApi() + .get('/api/v2/organizations/1') + .reply(200, { success: true, data: { id: 1, name: 'dup' } }) + mockApi() + .get('/api/v2/organizations/2') + .reply(200, { success: true, data: { id: 2, name: 'dup' } }) const err = await OrgImportCommand.run([ 'o.csv', @@ -335,9 +342,12 @@ describe('org import --upsert', () => { .query((q) => q.term === 'Acme') .reply(200, { success: true, - data: { items: [{ item: { id: 7, name: 'Acme', owner_id: 1 } }] }, + data: { items: [{ item: { id: 7 } }] }, additional_data: { next_cursor: null }, }) + mockApi() + .get('/api/v2/organizations/7') + .reply(200, { success: true, data: { id: 7, name: 'Acme', owner_id: 1 } }) mockApi() .patch('/api/v2/organizations/7') .reply(401, { success: false, error: 'unauthorized' }) diff --git a/test/commands/org/upsert.test.js b/test/commands/org/upsert.test.js index 729e27c..bb297b3 100644 --- a/test/commands/org/upsert.test.js +++ b/test/commands/org/upsert.test.js @@ -28,9 +28,14 @@ function mockSearch(records = []) { .query(true) .reply(200, { success: true, - data: { items: records.map((item) => ({ item })) }, + data: { items: records.map((r) => ({ item: { id: r.id } })) }, additional_data: { next_cursor: null }, }) + for (const r of records) { + mockApi() + .get(`/api/v2/organizations/${r.id}`) + .reply(200, { success: true, data: r }) + } } describe('org upsert', () => { diff --git a/test/commands/person/import.test.js b/test/commands/person/import.test.js index ec473b7..8857fcf 100644 --- a/test/commands/person/import.test.js +++ b/test/commands/person/import.test.js @@ -243,13 +243,15 @@ describe('person import --upsert', () => { .query((q) => q.term === 'a@x.com') .reply(200, { success: true, - data: { - items: [ - { item: { id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 } }, - ], - }, + data: { items: [{ item: { id: 7 } }] }, additional_data: { next_cursor: null }, }) + mockApi() + .get('/api/v2/persons/7') + .reply(200, { + success: true, + data: { id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 }, + }) mockApi() .get('/api/v2/persons/search') .query((q) => q.term === 'b@x.com') @@ -340,14 +342,21 @@ describe('person import --upsert', () => { .query((q) => q.term === 'dup@x.com') .reply(200, { success: true, - data: { - items: [ - { item: { id: 1, emails: [{ value: 'dup@x.com' }] } }, - { item: { id: 2, emails: [{ value: 'dup@x.com' }] } }, - ], - }, + data: { items: [{ item: { id: 1 } }, { item: { id: 2 } }] }, additional_data: { next_cursor: null }, }) + mockApi() + .get('/api/v2/persons/1') + .reply(200, { + success: true, + data: { id: 1, emails: [{ value: 'dup@x.com' }] }, + }) + mockApi() + .get('/api/v2/persons/2') + .reply(200, { + success: true, + data: { id: 2, emails: [{ value: 'dup@x.com' }] }, + }) const err = await PersonImportCommand.run([ 'p.csv', @@ -369,13 +378,15 @@ describe('person import --upsert', () => { .query((q) => q.term === 'a@x.com') .reply(200, { success: true, - data: { - items: [ - { item: { id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 } }, - ], - }, + data: { items: [{ item: { id: 7 } }] }, additional_data: { next_cursor: null }, }) + mockApi() + .get('/api/v2/persons/7') + .reply(200, { + success: true, + data: { id: 7, emails: [{ value: 'a@x.com' }], owner_id: 1 }, + }) mockApi() .patch('/api/v2/persons/7') .reply(401, { success: false, error: 'unauthorized' }) diff --git a/test/commands/person/upsert.test.js b/test/commands/person/upsert.test.js index a162975..9c57a7f 100644 --- a/test/commands/person/upsert.test.js +++ b/test/commands/person/upsert.test.js @@ -23,16 +23,24 @@ function mockFields(defs = []) { .reply(200, { success: true, data: defs }) } -/** Mock the scoped search, returning the given records as search items. */ +/** + * Mock the scoped search as a candidate-id finder, then a record fetch per + * candidate (lookup verifies against the full record, not the search item). + */ function mockSearch(records = []) { mockApi() .get('/api/v2/persons/search') .query(true) .reply(200, { success: true, - data: { items: records.map((item) => ({ item })) }, + data: { items: records.map((r) => ({ item: { id: r.id } })) }, additional_data: { next_cursor: null }, }) + for (const r of records) { + mockApi() + .get(`/api/v2/persons/${r.id}`) + .reply(200, { success: true, data: r }) + } } describe('person upsert', () => { diff --git a/test/lib/lookup.test.js b/test/lib/lookup.test.js index 4e834ae..0d9e175 100644 --- a/test/lib/lookup.test.js +++ b/test/lib/lookup.test.js @@ -1,26 +1,37 @@ import { describe, it, expect } from 'vitest' import { lookupByField } from '../../src/lib/lookup.js' -/** Fake client returning queued search pages and capturing queries. */ -function fakeClient(pages) { +/** + * Fake client modelling the two-step lookup: a `/search` GET returns lossy + * candidate items (id only is used), and a record-fetch GET + * (`/api/v2//`) returns the authoritative full record. `records` + * maps id → full record; `searchPages` is the queued list of search pages. + */ +function fakeClient({ searchPages = [], records = {} } = {}) { let i = 0 const calls = [] return { calls, async get(path, opts) { calls.push({ path, query: opts?.query }) - return pages[i++] ?? { data: { items: [] } } + if (path.endsWith('/search')) { + return searchPages[i++] ?? { data: { items: [] } } + } + const id = Number(path.split('/').pop()) + return { data: records[id] ?? null } }, } } -const page = (items, next = null) => ({ - data: { items: items.map((item) => ({ item })) }, + +/** A search page advertising candidate ids; full bodies live in `records`. */ +const page = (ids, next = null) => ({ + data: { items: ids.map((id) => ({ item: { id } })) }, additional_data: { next_cursor: next }, }) describe('lookupByField', () => { it('returns none when nothing matches', async () => { - const client = fakeClient([page([])]) + const client = fakeClient({ searchPages: [page([])] }) const r = await lookupByField({ client, entity: 'person', @@ -30,10 +41,11 @@ describe('lookupByField', () => { expect(r.status).toBe('none') }) - it('returns unique with the id for exactly one match', async () => { - const client = fakeClient([ - page([{ id: 7, emails: [{ value: 'a@x.com' }] }]), - ]) + it('returns unique with the id for exactly one verified match', async () => { + const client = fakeClient({ + searchPages: [page([7])], + records: { 7: { id: 7, emails: [{ value: 'a@x.com' }] } }, + }) const r = await lookupByField({ client, entity: 'person', @@ -41,15 +53,17 @@ describe('lookupByField', () => { value: 'a@x.com', }) expect(r).toMatchObject({ status: 'unique', id: 7 }) + expect(r.record).toEqual({ id: 7, emails: [{ value: 'a@x.com' }] }) }) it('refuses (ambiguous) on more than one verified match', async () => { - const client = fakeClient([ - page([ - { id: 7, emails: [{ value: 'a@x.com' }] }, - { id: 8, emails: [{ value: 'a@x.com' }] }, - ]), - ]) + const client = fakeClient({ + searchPages: [page([7, 8])], + records: { + 7: { id: 7, emails: [{ value: 'a@x.com' }] }, + 8: { id: 8, emails: [{ value: 'a@x.com' }] }, + }, + }) const r = await lookupByField({ client, entity: 'person', @@ -61,9 +75,10 @@ describe('lookupByField', () => { }) it('matches email case-insensitively', async () => { - const client = fakeClient([ - page([{ id: 7, emails: [{ value: 'A@X.com' }] }]), - ]) + const client = fakeClient({ + searchPages: [page([7])], + records: { 7: { id: 7, emails: [{ value: 'A@X.com' }] } }, + }) const r = await lookupByField({ client, entity: 'person', @@ -73,11 +88,13 @@ describe('lookupByField', () => { expect(r).toMatchObject({ status: 'unique', id: 7 }) }) - it('re-verifies client-side, discarding search over-matches', async () => { - // search leaked a non-matching person (exact_match is not a unique key) - const client = fakeClient([ - page([{ id: 9, emails: [{ value: 'other@x.com' }] }]), - ]) + it('re-verifies the full record, discarding a search over-match', async () => { + // search leaked a non-matching person (exact_match is not a unique key); + // the fetched record proves its email differs. + const client = fakeClient({ + searchPages: [page([9])], + records: { 9: { id: 9, emails: [{ value: 'other@x.com' }] } }, + }) const r = await lookupByField({ client, entity: 'person', @@ -87,25 +104,26 @@ describe('lookupByField', () => { expect(r.status).toBe('none') }) - it('sends exact_match and the scoped fields param', async () => { - const client = fakeClient([page([])]) - await lookupByField({ - client, - entity: 'org', - field: 'name', - value: 'Acme', + it('sends exact_match and the scoped fields/limit, then fetches the record', async () => { + const client = fakeClient({ + searchPages: [page([5])], + records: { 5: { id: 5, name: 'Acme' } }, }) + await lookupByField({ client, entity: 'org', field: 'name', value: 'Acme' }) expect(client.calls[0]).toMatchObject({ path: '/api/v2/organizations/search', - query: { term: 'Acme', exact_match: true, fields: 'name' }, + // Per-entity search caps limit at 100 (NOT the 500 list cap) — the live + // API 400s on limit > 100. + query: { term: 'Acme', exact_match: true, fields: 'name', limit: 100 }, }) + expect(client.calls[1].path).toBe('/api/v2/organizations/5') }) - it('pages through next_cursor before deciding ambiguity', async () => { - const client = fakeClient([ - page([{ id: 1, name: 'Acme' }], 'CUR'), - page([{ id: 2, name: 'Acme' }]), - ]) + it('pages through next_cursor before fetching candidates', async () => { + const client = fakeClient({ + searchPages: [page([1], 'CUR'), page([2])], + records: { 1: { id: 1, name: 'Acme' }, 2: { id: 2, name: 'Acme' } }, + }) const r = await lookupByField({ client, entity: 'org', @@ -113,8 +131,27 @@ describe('lookupByField', () => { value: 'Acme', }) expect(r.status).toBe('ambiguous') - expect(client.calls).toHaveLength(2) - expect(client.calls[1].query.cursor).toBe('CUR') + const searches = client.calls.filter((c) => c.path.endsWith('/search')) + expect(searches).toHaveLength(2) + expect(searches[1].query.cursor).toBe('CUR') + }) + + it('de-duplicates a candidate id that the search repeats across pages', async () => { + const client = fakeClient({ + searchPages: [page([7], 'CUR'), page([7])], + records: { 7: { id: 7, name: 'Acme' } }, + }) + const r = await lookupByField({ + client, + entity: 'org', + field: 'name', + value: 'Acme', + }) + expect(r).toMatchObject({ status: 'unique', id: 7 }) + const fetches = client.calls.filter( + (c) => c.path === '/api/v2/organizations/7', + ) + expect(fetches).toHaveLength(1) }) it('matches a searchable custom field by name, verifying the hash key', async () => { @@ -125,12 +162,13 @@ describe('lookupByField', () => { field_type: 'varchar', }, ] - const client = fakeClient([ - page([ - { id: 5, custom_fields: { abc123: 'D-42' } }, - { id: 6, custom_fields: { abc123: 'D-99' } }, // search leaked; verify drops it - ]), - ]) + const client = fakeClient({ + searchPages: [page([5, 6])], + records: { + 5: { id: 5, custom_fields: { abc123: 'D-42' } }, + 6: { id: 6, custom_fields: { abc123: 'D-99' } }, // verify drops it + }, + }) const r = await lookupByField({ client, entity: 'deal', @@ -146,7 +184,10 @@ describe('lookupByField', () => { const defs = [ { field_name: 'Score', field_code: 'sc', field_type: 'double' }, ] - const client = fakeClient([page([{ id: 5, custom_fields: { sc: 42 } }])]) + const client = fakeClient({ + searchPages: [page([5])], + records: { 5: { id: 5, custom_fields: { sc: 42 } } }, + }) const r = await lookupByField({ client, entity: 'deal', @@ -158,7 +199,10 @@ describe('lookupByField', () => { }) it('matches a deal by title', async () => { - const client = fakeClient([page([{ id: 3, title: 'Acme expansion' }])]) + const client = fakeClient({ + searchPages: [page([3])], + records: { 3: { id: 3, title: 'Acme expansion' } }, + }) const r = await lookupByField({ client, entity: 'deal', @@ -171,7 +215,10 @@ describe('lookupByField', () => { it('matches a person by name or phone (with empty-array tolerance)', async () => { const byName = await lookupByField({ - client: fakeClient([page([{ id: 1, name: 'Jane Doe', phones: [] }])]), + client: fakeClient({ + searchPages: [page([1])], + records: { 1: { id: 1, name: 'Jane Doe', phones: [] } }, + }), entity: 'person', field: 'name', value: 'Jane Doe', @@ -179,9 +226,10 @@ describe('lookupByField', () => { expect(byName).toMatchObject({ status: 'unique', id: 1 }) const byPhone = await lookupByField({ - client: fakeClient([ - page([{ id: 2, phones: [{ value: '+15551234' }], emails: [] }]), - ]), + client: fakeClient({ + searchPages: [page([2])], + records: { 2: { id: 2, phones: [{ value: '+15551234' }], emails: [] } }, + }), entity: 'person', field: 'phone', value: '+15551234', @@ -197,9 +245,10 @@ describe('lookupByField', () => { field_type: 'varchar', }, ] - const client = fakeClient([ - page([{ id: 5, custom_fields: { abc123: 'D-42' } }]), - ]) + const client = fakeClient({ + searchPages: [page([5])], + records: { 5: { id: 5, custom_fields: { abc123: 'D-42' } } }, + }) const r = await lookupByField({ client, entity: 'deal', @@ -211,7 +260,18 @@ describe('lookupByField', () => { }) it('tolerates a malformed search response (no data/cursor)', async () => { - const client = fakeClient([{}]) // no .data, no .additional_data + const client = fakeClient({ searchPages: [{}] }) + const r = await lookupByField({ + client, + entity: 'org', + field: 'name', + value: 'Acme', + }) + expect(r.status).toBe('none') + }) + + it('tolerates a candidate whose fetched record is null', async () => { + const client = fakeClient({ searchPages: [page([1])], records: {} }) const r = await lookupByField({ client, entity: 'org', @@ -221,9 +281,25 @@ describe('lookupByField', () => { expect(r.status).toBe('none') }) - it('tolerates records missing the matched field', async () => { + it('skips a search item without an id', async () => { + const client = fakeClient({ + searchPages: [{ data: { items: [{ item: {} }] } }], + }) + const r = await lookupByField({ + client, + entity: 'org', + field: 'name', + value: 'Acme', + }) + expect(r.status).toBe('none') + }) + + it('tolerates fetched records missing the matched field', async () => { const noEmail = await lookupByField({ - client: fakeClient([page([{ id: 1 }])]), // no emails property + client: fakeClient({ + searchPages: [page([1])], + records: { 1: { id: 1 } }, + }), entity: 'person', field: 'email', value: 'a@x.com', @@ -231,7 +307,10 @@ describe('lookupByField', () => { expect(noEmail.status).toBe('none') const noCustom = await lookupByField({ - client: fakeClient([page([{ id: 2 }])]), // no custom_fields property + client: fakeClient({ + searchPages: [page([2])], + records: { 2: { id: 2 } }, + }), entity: 'deal', defs: [ { field_name: 'External ID', field_code: 'k', field_type: 'varchar' }, @@ -242,7 +321,10 @@ describe('lookupByField', () => { expect(noCustom.status).toBe('none') const noPhone = await lookupByField({ - client: fakeClient([page([{ id: 3 }])]), // no phones property + client: fakeClient({ + searchPages: [page([3])], + records: { 3: { id: 3 } }, + }), entity: 'person', field: 'phone', value: '+1555', @@ -250,39 +332,39 @@ describe('lookupByField', () => { expect(noPhone.status).toBe('none') }) - it('rejects a non-searchable field type with exit 64', async () => { + it('rejects a non-numeric value for a numeric custom field with exit 65', async () => { const defs = [ - { field_name: 'Stage Color', field_code: 'cc', field_type: 'enum' }, + { field_name: 'Score', field_code: 'sc', field_type: 'double' }, ] const err = await lookupByField({ - client: fakeClient([]), + client: fakeClient({}), entity: 'deal', defs, - field: 'Stage Color', - value: 'Red', + field: 'Score', + value: 'notanumber', }).catch((e) => e) - expect(err.exitCode).toBe(64) - expect(err.message).toMatch(/not searchable/i) + expect(err.exitCode).toBe(65) + expect(err.message).toMatch(/number/i) }) - it('rejects a non-numeric value for a numeric custom field with exit 65', async () => { + it('rejects a non-searchable field type with exit 64', async () => { const defs = [ - { field_name: 'Score', field_code: 'sc', field_type: 'double' }, + { field_name: 'Stage Color', field_code: 'cc', field_type: 'enum' }, ] const err = await lookupByField({ - client: fakeClient([]), + client: fakeClient({}), entity: 'deal', defs, - field: 'Score', - value: 'notanumber', + field: 'Stage Color', + value: 'Red', }).catch((e) => e) - expect(err.exitCode).toBe(65) - expect(err.message).toMatch(/number/i) + expect(err.exitCode).toBe(64) + expect(err.message).toMatch(/not searchable/i) }) it('rejects an unknown field with exit 64', async () => { const err = await lookupByField({ - client: fakeClient([]), + client: fakeClient({}), entity: 'deal', defs: [], field: 'Nonexistent', diff --git a/test/lib/upsert.test.js b/test/lib/upsert.test.js index f5c3bd2..f2402c8 100644 --- a/test/lib/upsert.test.js +++ b/test/lib/upsert.test.js @@ -101,16 +101,24 @@ describe('diffBody', () => { }) }) -/** Fake client: queued search items for lookup + captured post/patch. */ +/** + * Fake client modelling lookup's two-step flow: `/search` advertises candidate + * ids, then a record-fetch returns the full record. `items` are full records. + */ function fakeClient({ items = [] } = {}) { const calls = { post: [], patch: [] } + const byId = new Map(items.map((it) => [it.id, it])) return { calls, - async get() { - return { - data: { items: items.map((item) => ({ item })) }, - additional_data: { next_cursor: null }, + async get(path) { + if (path.endsWith('/search')) { + return { + data: { items: items.map((it) => ({ item: { id: it.id } })) }, + additional_data: { next_cursor: null }, + } } + const id = Number(path.split('/').pop()) + return { data: byId.get(id) ?? null } }, async post(path, opts) { calls.post.push({ path, body: opts.body }) @@ -364,18 +372,29 @@ describe('summarizeUpsert', () => { }) }) -/** Fake client whose search results vary by the search `term`. */ +/** + * Fake client whose search candidates vary by the search `term`; a record + * fetch then returns the full record by id (lookup's two-step flow). + */ function termFakeClient(byTerm = {}) { const calls = { post: [], patch: [], terms: [] } + const byId = new Map() + for (const recs of Object.values(byTerm)) { + for (const r of recs) byId.set(r.id, r) + } return { calls, async get(path, opts) { - calls.terms.push(opts.query.term) - const items = byTerm[opts.query.term] ?? [] - return { - data: { items: items.map((item) => ({ item })) }, - additional_data: { next_cursor: null }, + if (path.endsWith('/search')) { + calls.terms.push(opts.query.term) + const items = byTerm[opts.query.term] ?? [] + return { + data: { items: items.map((r) => ({ item: { id: r.id } })) }, + additional_data: { next_cursor: null }, + } } + const id = Number(path.split('/').pop()) + return { data: byId.get(id) ?? null } }, async post(path, opts) { calls.post.push({ path, body: opts.body }) From 559bbfa390dee8ea00385158210b04b89509b622 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 11 Jun 2026 12:55:49 +0200 Subject: [PATCH 8/8] docs(bulk): note search eventual-consistency caveat for upsert Matching depends on Pipedrive's search index, which is eventually consistent, so upserting the same key twice in quick succession can still create a duplicate before the first write is indexed. Document the gotcha and the mitigation (key on a stable external id; let the index settle). --- website/src/content/docs/guides/bulk.mdx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/website/src/content/docs/guides/bulk.mdx b/website/src/content/docs/guides/bulk.mdx index 6d88469..ea5c18a 100644 --- a/website/src/content/docs/guides/bulk.mdx +++ b/website/src/content/docs/guides/bulk.mdx @@ -211,3 +211,13 @@ Error: 1 of 24 rows failed When every failed row is a data-validation error (ambiguous match, empty match value) the command exits **65**; a mix that includes a transport/API error exits **1**. + +:::caution +Matching relies on Pipedrive's **search index, which is eventually consistent** — a +just-created record is not searchable for a short window. So upserting the _same_ match +value twice in quick succession (the same key on two CSV rows, or an upsert moments after a +create) can still create a duplicate, because the second lookup runs before the first write +is indexed. For repeatable sync, key on a stable external ID and let the index settle +between runs; a periodic `backup diff` or a search will surface any duplicates that slip +through. +:::