diff --git a/src/features/invoice-sync/lib/SyncedContacts.service.ts b/src/features/invoice-sync/lib/SyncedContacts.service.ts index 108cfa4..d2e37ad 100644 --- a/src/features/invoice-sync/lib/SyncedContacts.service.ts +++ b/src/features/invoice-sync/lib/SyncedContacts.service.ts @@ -140,26 +140,59 @@ class SyncedContactsService extends AuthenticatedXeroService { } const syncLogsService = new SyncLogsService(this.user, this.connection) + const clientOrCompanyId = isNotPlaceholderCompany ? companyId : z.string().parse(client?.id) + const contactUserType = isNotPlaceholderCompany + ? SyncedContactUserType.COMPANY + : SyncedContactUserType.CLIENT try { const contact = await this.xero.createContact(this.connection.tenantId, contactPayload) + const contactId = z.string().parse(contact.contactID) - await this.db.insert(syncedContacts).values({ - portalId: this.user.portalId, - clientOrCompanyId: isNotPlaceholderCompany ? companyId : z.string().parse(client?.id), - userType: isNotPlaceholderCompany - ? SyncedContactUserType.COMPANY - : SyncedContactUserType.CLIENT, - contactId: z.string().parse(contact.contactID), - tenantId: this.connection.tenantId, - }) + // Ignore a duplicate from a concurrent sync instead of failing. + const [inserted] = await this.db + .insert(syncedContacts) + .values({ + portalId: this.user.portalId, + clientOrCompanyId, + userType: contactUserType, + contactId, + tenantId: this.connection.tenantId, + }) + .onConflictDoNothing({ + target: [ + syncedContacts.portalId, + syncedContacts.tenantId, + syncedContacts.clientOrCompanyId, + ], + }) + .returning({ contactId: syncedContacts.contactId }) + + // A concurrent sync won the insert; reuse its stored contact. + if (!inserted) { + const [existing] = await this.db + .select({ contactId: syncedContacts.contactId }) + .from(syncedContacts) + .where( + and( + eq(syncedContacts.portalId, this.user.portalId), + eq(syncedContacts.tenantId, this.connection.tenantId), + eq(syncedContacts.clientOrCompanyId, clientOrCompanyId), + ), + ) + logger.info( + 'SyncedContactsService#createContact :: Mapping already exists, reusing contact', + existing?.contactId, + ) + return { ...contact, contactID: z.string().parse(existing?.contactId) } + } await syncLogsService.createSyncLog({ entityType: SyncEntityType.CUSTOMER, eventType: SyncEventType.CREATED, status: SyncStatus.SUCCESS, syncDate: new Date(), - copilotId: isNotPlaceholderCompany ? companyId : z.string().parse(client?.id), + copilotId: clientOrCompanyId, xeroId: contact.contactID, customerName: contact.name, customerEmail: contact.emailAddress, @@ -167,7 +200,7 @@ class SyncedContactsService extends AuthenticatedXeroService { return { ...contact, - contactID: z.string().parse(contact.contactID), + contactID: contactId, } } catch (error: unknown) { throw new APIError('Failed to create synced contact', status.INTERNAL_SERVER_ERROR, { @@ -175,7 +208,7 @@ class SyncedContactsService extends AuthenticatedXeroService { failedSyncLogPayload: { entityType: SyncEntityType.CUSTOMER, eventType: SyncEventType.CREATED, - copilotId: isNotPlaceholderCompany ? companyId : z.string().parse(client?.id), + copilotId: clientOrCompanyId, customerName: contactPayload.name, customerEmail: contactPayload.emailAddress, }, diff --git a/src/features/invoice-sync/lib/SyncedInvoices.service.ts b/src/features/invoice-sync/lib/SyncedInvoices.service.ts index 30536e1..425fb1e 100644 --- a/src/features/invoice-sync/lib/SyncedInvoices.service.ts +++ b/src/features/invoice-sync/lib/SyncedInvoices.service.ts @@ -38,6 +38,9 @@ class SyncedInvoicesService extends AuthenticatedXeroService { xeroInvoiceId: string | null salesAccountId: string | null status: NonNullable + // Exposed so callers reuse it instead of resolving the contact again. + customerName?: string + customerEmail?: string }> { logger.info('SyncedInvoicesService#syncInvoiceToXero :: Syncing invoice to xero:', data.id) @@ -160,7 +163,7 @@ class SyncedInvoicesService extends AuthenticatedXeroService { logger.info( `SyncedInvoicesService#syncInvoiceToXero :: Synced Copilot invoice ${syncedInvoiceRecord.copilotInvoiceId} (${syncedInvoice?.invoiceNumber}) to Xero invoice ${syncedInvoiceRecord.xeroInvoiceId} for portalId ${this.connection.portalId}`, ) - return syncedInvoiceRecord + return { ...syncedInvoiceRecord, customerName, customerEmail } } private async createMissingXeroInvoice(copilotInvoiceId: string) { @@ -169,17 +172,23 @@ class SyncedInvoicesService extends AuthenticatedXeroService { copilotInvoiceId, ) const invoice = await this.copilot.getInvoice(copilotInvoiceId) - const [record, contact] = await Promise.all([ - this.syncInvoiceToXero(invoice), - this.getContact(invoice), - ]) - if (!('xeroInvoiceId' in record) || !record.xeroInvoiceId) { + // Reuse the contact syncInvoiceToXero resolved; a second fetch here races it. + const record = await this.syncInvoiceToXero(invoice) + if (!record.xeroInvoiceId) { throw new APIError( `Failed to create Xero invoice for Copilot invoice ${copilotInvoiceId}`, status.INTERNAL_SERVER_ERROR, ) } + // Already-synced short-circuit returns no contact; fetch it for the log. + let { customerName, customerEmail } = record + if (!customerName && !customerEmail) { + const contact = await this.getContact(invoice) + customerName = contact.name + customerEmail = contact.emailAddress + } + const syncLogsService = new SyncLogsService(this.user, this.connection) await syncLogsService.createSyncLog({ entityType: SyncEntityType.INVOICE, @@ -191,8 +200,8 @@ class SyncedInvoicesService extends AuthenticatedXeroService { invoiceNumber: invoice.number, copilotId: invoice.id, xeroId: record.xeroInvoiceId, - customerEmail: contact.emailAddress, - customerName: contact.name, + customerEmail, + customerName, }) return record diff --git a/test/integration/syncedContacts/createContactIdempotency.test.ts b/test/integration/syncedContacts/createContactIdempotency.test.ts new file mode 100644 index 0000000..7d07807 --- /dev/null +++ b/test/integration/syncedContacts/createContactIdempotency.test.ts @@ -0,0 +1,67 @@ +import AuthService from '@auth/lib/Auth.service' +import SyncedContactsService from '@invoice-sync/lib/SyncedContacts.service' +import { TEST_CLIENT, TEST_COMPANY, TEST_TOKENS, TEST_XERO_CONTACT } from '@test/helpers/constants' +import { createMockXeroAPI } from '@test/helpers/mocks' +import { seedConnectedPortal, seedSyncedContact } from '@test/helpers/seed' +import { setupWebhookTest } from '@test/helpers/webhookTestSetup' +import { eq } from 'drizzle-orm' +import { describe, expect, it, vi } from 'vitest' +import db from '@/db' +import { syncedContacts } from '@/db/schema/syncedContacts.schema' +import { SyncEntityType, syncLogs } from '@/db/schema/syncLogs.schema' +import User from '@/lib/copilot/models/User.model' + +// A concurrent sync already stored this mapping; our insert collides on it. +const WINNER_CONTACT_ID = TEST_XERO_CONTACT.id +// The losing sync still creates its own (orphan) Xero contact before inserting. +const ORPHAN_CONTACT_ID = '99999999-9999-4999-8999-999999999999' + +const CLIENT = { + id: TEST_CLIENT.id, + givenName: TEST_CLIENT.givenName, + familyName: TEST_CLIENT.familyName, + email: TEST_CLIENT.email, + companyIds: [TEST_COMPANY.id], + status: 'active', + avatarImageUrl: null, + fallbackColor: null, + createdAt: '2026-01-01T00:00:00.000Z', +} + +describe('SyncedContactsService.createContact — concurrent-safe get-or-create', () => { + setupWebhookTest(() => ({ + xero: createMockXeroAPI({ + createContact: vi.fn().mockResolvedValue({ + contactID: ORPHAN_CONTACT_ID, + name: `${TEST_CLIENT.givenName} ${TEST_CLIENT.familyName}`, + emailAddress: TEST_CLIENT.email, + }), + }), + })) + + it('returns the existing mapping instead of failing on a duplicate insert', async () => { + await seedConnectedPortal() + await seedSyncedContact({ contactId: WINNER_CONTACT_ID }) + + const user = await User.authenticate(TEST_TOKENS.webhook) + const connection = await new AuthService(user).authorizeXeroForCopilotWorkspace() + const service = new SyncedContactsService(user, connection) + + const contact = await service.createContact(CLIENT) + + // Returns the stored winner, not the orphan just created in Xero. + expect(contact.contactID).toBe(WINNER_CONTACT_ID) + + // The unique mapping is untouched — no duplicate row. + const rows = await db.select().from(syncedContacts) + expect(rows).toHaveLength(1) + expect(rows[0].contactId).toBe(WINNER_CONTACT_ID) + + // A conflict is a no-op, not a create — no customer sync log is written. + const customerLogs = await db + .select() + .from(syncLogs) + .where(eq(syncLogs.entityType, SyncEntityType.CUSTOMER)) + expect(customerLogs).toHaveLength(0) + }) +}) diff --git a/test/integration/webhook/invoicePaid/missingXeroInvoice.test.ts b/test/integration/webhook/invoicePaid/missingXeroInvoice.test.ts index 947e7f2..fe7e423 100644 --- a/test/integration/webhook/invoicePaid/missingXeroInvoice.test.ts +++ b/test/integration/webhook/invoicePaid/missingXeroInvoice.test.ts @@ -34,6 +34,9 @@ describe('POST /api/webhook — invoice.paid missing Xero invoice', () => { expect(apis.xero.createInvoice).toHaveBeenCalledTimes(1) expect(apis.xero.markInvoicePaid).toHaveBeenCalledTimes(1) + // Contact resolved once; the old path resolved it twice and self-raced. + expect(apis.xero.createContact).toHaveBeenCalledTimes(1) + // Invoice row now mapped to Xero and marked success. const invoices = await db.select().from(syncedInvoices) expect(invoices).toHaveLength(1)