From 262027bb941b3fe98d9d76ad4db49f95b6441812 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 17 Mar 2026 01:13:14 +0000 Subject: [PATCH 1/3] fix(google-sheets-sync): skip backfill writes when target range unchanged --- .../googleSheetsSync/service/backfill.spec.ts | 114 ++++++++++++++++++ .../googleSheetsSync/service/backfill.ts | 55 ++++++++- 2 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts diff --git a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts new file mode 100644 index 00000000000..ee43c911882 --- /dev/null +++ b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts @@ -0,0 +1,114 @@ +import { Job } from 'bullmq' +import { Logger } from 'pino' + +import { prismaMock } from '../../../../test/prismaMock' +import { getIntegrationGoogleAccessToken } from '../../../lib/google/googleAuth' +import { + clearSheet, + ensureSheet, + readValues, + writeValues +} from '../../../lib/google/sheets' +import { GoogleSheetsSyncBackfillJobData } from '../queue' + +import { backfillService } from './backfill' + +jest.mock('../../../lib/google/googleAuth', () => ({ + getIntegrationGoogleAccessToken: jest.fn() +})) + +jest.mock('../../../lib/google/sheets', () => { + const actual = jest.requireActual('../../../lib/google/sheets') + return { + ...actual, + clearSheet: jest.fn(), + ensureSheet: jest.fn(), + readValues: jest.fn(), + writeValues: jest.fn() + } +}) + +const mockGetIntegrationGoogleAccessToken = + getIntegrationGoogleAccessToken as jest.MockedFunction< + typeof getIntegrationGoogleAccessToken + > +const mockEnsureSheet = ensureSheet as jest.MockedFunction +const mockReadValues = readValues as jest.MockedFunction +const mockClearSheet = clearSheet as jest.MockedFunction +const mockWriteValues = writeValues as jest.MockedFunction + +const backfillJob: Job = { + name: 'google-sheets-sync-backfill', + data: { + type: 'backfill', + journeyId: 'journey-id', + teamId: 'team-id', + syncId: 'sync-id', + spreadsheetId: 'spreadsheet-id', + sheetName: 'Sheet1', + timezone: 'UTC', + integrationId: 'integration-id' + } +} as unknown as Job + +describe('backfillService', () => { + let logger: Logger + + beforeEach(() => { + jest.clearAllMocks() + logger = { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn() + } as unknown as Logger + + prismaMock.googleSheetsSync.findFirst.mockResolvedValue({ + id: 'sync-id' + } as any) + prismaMock.journey.findUnique.mockResolvedValue({ + id: 'journey-id', + blocks: [] + } as any) + prismaMock.event.findMany.mockResolvedValue([] as any) + prismaMock.journeyVisitor.findMany.mockResolvedValue([] as any) + + mockGetIntegrationGoogleAccessToken.mockResolvedValue({ + accessToken: 'access-token', + accountEmail: 'example@example.com' + }) + mockEnsureSheet.mockResolvedValue(undefined) + }) + + it('skips clear/write when target range is unchanged', async () => { + mockReadValues.mockResolvedValue([['Visitor ID', 'Date']]) + + await backfillService(backfillJob, logger) + + expect(mockReadValues).toHaveBeenCalledWith({ + accessToken: 'access-token', + spreadsheetId: 'spreadsheet-id', + range: 'Sheet1!A1:B1' + }) + expect(mockClearSheet).not.toHaveBeenCalled() + expect(mockWriteValues).not.toHaveBeenCalled() + }) + + it('clears and rewrites when target range changed', async () => { + mockReadValues.mockResolvedValue([['Visitor ID', 'Date (Old)']]) + + await backfillService(backfillJob, logger) + + expect(mockClearSheet).toHaveBeenCalledWith({ + accessToken: 'access-token', + spreadsheetId: 'spreadsheet-id', + sheetTitle: 'Sheet1' + }) + expect(mockWriteValues).toHaveBeenCalledWith({ + accessToken: 'access-token', + spreadsheetId: 'spreadsheet-id', + sheetTitle: 'Sheet1', + values: [['Visitor ID', 'Date']], + append: false + }) + }) +}) diff --git a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts index 184239155a1..16a0b8fa535 100644 --- a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts +++ b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts @@ -6,7 +6,9 @@ import { Prisma, prisma } from '@core/prisma/journeys/client' import { getIntegrationGoogleAccessToken } from '../../../lib/google/googleAuth' import { clearSheet, + columnIndexToA1, ensureSheet, + readValues, writeValues } from '../../../lib/google/sheets' import { computeConnectedBlockIds } from '../../../schema/journeyVisitor/export/connectivity' @@ -44,6 +46,28 @@ interface JourneyVisitorExportRow { [key: string]: string } +function isTargetRangeUnchanged({ + nextValues, + existingValues, + writeWidth +}: { + nextValues: (string | null)[][] + existingValues: (string | null)[][] + writeWidth: number +}): boolean { + return nextValues.every((nextRow, rowIndex) => { + const existingRow = existingValues[rowIndex] ?? [] + + for (let colIndex = 0; colIndex < writeWidth; colIndex++) { + const nextCell = String(nextRow[colIndex] ?? '') + const existingCell = String(existingRow[colIndex] ?? '') + if (nextCell !== existingCell) return false + } + + return true + }) +} + async function* getJourneyVisitors( journeyId: string, eventWhere: Prisma.EventWhereInput, @@ -303,15 +327,36 @@ export async function backfillService( values.push(aligned) } - // Write all data at once - await writeValues({ + const writeWidth = finalHeader.length + const writeHeight = values.length + const lastColumnA1 = columnIndexToA1(writeWidth - 1) + const targetRange = `${sheetName}!A1:${lastColumnA1}${writeHeight}` + const existingTargetValues = await readValues({ accessToken, spreadsheetId, - sheetTitle: sheetName, - values, - append: false + range: targetRange + }) + + const targetRangeUnchanged = isTargetRangeUnchanged({ + nextValues: values, + existingValues: existingTargetValues, + writeWidth }) + if (!targetRangeUnchanged) { + // Clear existing data + await clearSheet({ accessToken, spreadsheetId, sheetTitle: sheetName }) + + // Write all data at once + await writeValues({ + accessToken, + spreadsheetId, + sheetTitle: sheetName, + values, + append: false + }) + } + // Update exportOrder on blocks that don't have it set yet. // This ensures columns maintain their positions for future syncs. const blocksToUpdate = columns From 63a63bad664e7dc174b74dd08152565ba25f2548 Mon Sep 17 00:00:00 2001 From: Mike Allison Date: Wed, 18 Mar 2026 19:47:08 +0000 Subject: [PATCH 2/3] fix(google-sheets-sync): enhance backfill logic to handle sheet content changes and errors --- .../googleSheetsSync/service/backfill.spec.ts | 65 ++++++++++++++++++- .../googleSheetsSync/service/backfill.ts | 47 ++++++++------ 2 files changed, 90 insertions(+), 22 deletions(-) diff --git a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts index ee43c911882..5fee2008b9a 100644 --- a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts +++ b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts @@ -79,7 +79,7 @@ describe('backfillService', () => { mockEnsureSheet.mockResolvedValue(undefined) }) - it('skips clear/write when target range is unchanged', async () => { + it('skips clear/write when sheet content is unchanged', async () => { mockReadValues.mockResolvedValue([['Visitor ID', 'Date']]) await backfillService(backfillJob, logger) @@ -87,13 +87,13 @@ describe('backfillService', () => { expect(mockReadValues).toHaveBeenCalledWith({ accessToken: 'access-token', spreadsheetId: 'spreadsheet-id', - range: 'Sheet1!A1:B1' + range: 'Sheet1!A:B' }) expect(mockClearSheet).not.toHaveBeenCalled() expect(mockWriteValues).not.toHaveBeenCalled() }) - it('clears and rewrites when target range changed', async () => { + it('clears and rewrites when sheet content changed', async () => { mockReadValues.mockResolvedValue([['Visitor ID', 'Date (Old)']]) await backfillService(backfillJob, logger) @@ -111,4 +111,63 @@ describe('backfillService', () => { append: false }) }) + + it('clears and rewrites when existing sheet has more rows than new data', async () => { + mockReadValues.mockResolvedValue([ + ['Visitor ID', 'Date'], + ['visitor-1', '2026-01-01'] + ]) + + await backfillService(backfillJob, logger) + + expect(mockClearSheet).toHaveBeenCalled() + expect(mockWriteValues).toHaveBeenCalled() + }) + + it('clears and rewrites when existing sheet is empty', async () => { + mockReadValues.mockResolvedValue([]) + + await backfillService(backfillJob, logger) + + expect(mockClearSheet).toHaveBeenCalled() + expect(mockWriteValues).toHaveBeenCalled() + }) + + it('falls through to write when readValues fails', async () => { + mockReadValues.mockRejectedValue(new Error('API rate limit exceeded')) + + await backfillService(backfillJob, logger) + + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ err: expect.any(Error) }), + 'Failed to read existing sheet values, proceeding with full write' + ) + expect(mockClearSheet).toHaveBeenCalled() + expect(mockWriteValues).toHaveBeenCalled() + }) + + it('skips backfill when sync is not found', async () => { + prismaMock.googleSheetsSync.findFirst.mockResolvedValue(null) + + await backfillService(backfillJob, logger) + + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ syncId: 'sync-id' }), + 'Sync not found or deleted, skipping backfill' + ) + expect(mockEnsureSheet).not.toHaveBeenCalled() + expect(mockReadValues).not.toHaveBeenCalled() + }) + + it('skips backfill when journey is not found', async () => { + prismaMock.journey.findUnique.mockResolvedValue(null) + + await backfillService(backfillJob, logger) + + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ journeyId: 'journey-id' }), + 'Journey not found, skipping backfill' + ) + expect(mockEnsureSheet).not.toHaveBeenCalled() + }) }) diff --git a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts index 16a0b8fa535..35ebbd83e6c 100644 --- a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts +++ b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts @@ -46,7 +46,7 @@ interface JourneyVisitorExportRow { [key: string]: string } -function isTargetRangeUnchanged({ +function isSheetContentUnchanged({ nextValues, existingValues, writeWidth @@ -55,6 +55,8 @@ function isTargetRangeUnchanged({ existingValues: (string | null)[][] writeWidth: number }): boolean { + if (existingValues.length !== nextValues.length) return false + return nextValues.every((nextRow, rowIndex) => { const existingRow = existingValues[rowIndex] ?? [] @@ -305,9 +307,6 @@ export async function backfillService( // Ensure sheet exists await ensureSheet({ accessToken, spreadsheetId, sheetTitle: sheetName }) - // Clear existing data - await clearSheet({ accessToken, spreadsheetId, sheetTitle: sheetName }) - // Build data rows const sanitizedHeaderRow = headerRow.map((cell) => sanitizeGoogleSheetsCell(cell) @@ -328,26 +327,36 @@ export async function backfillService( } const writeWidth = finalHeader.length - const writeHeight = values.length const lastColumnA1 = columnIndexToA1(writeWidth - 1) - const targetRange = `${sheetName}!A1:${lastColumnA1}${writeHeight}` - const existingTargetValues = await readValues({ - accessToken, - spreadsheetId, - range: targetRange - }) + const fullRange = `${sheetName}!A:${lastColumnA1}` - const targetRangeUnchanged = isTargetRangeUnchanged({ - nextValues: values, - existingValues: existingTargetValues, - writeWidth - }) + let existingValues: (string | null)[][] = [] + let readFailed = false + try { + existingValues = await readValues({ + accessToken, + spreadsheetId, + range: fullRange + }) + } catch (err) { + readFailed = true + logger?.warn( + { err, range: fullRange }, + 'Failed to read existing sheet values, proceeding with full write' + ) + } + + const contentUnchanged = + !readFailed && + isSheetContentUnchanged({ + nextValues: values, + existingValues, + writeWidth + }) - if (!targetRangeUnchanged) { - // Clear existing data + if (!contentUnchanged) { await clearSheet({ accessToken, spreadsheetId, sheetTitle: sheetName }) - // Write all data at once await writeValues({ accessToken, spreadsheetId, From 3350c0c31f9d5468ab8ec7e1d18aeaddcb5df65d Mon Sep 17 00:00:00 2001 From: Mike Allison Date: Wed, 18 Mar 2026 20:01:03 +0000 Subject: [PATCH 3/3] feat(google-sheets-sync): implement escapeSheetName function for sheet title formatting --- apis/api-journeys-modern/src/lib/google/sheets.ts | 6 ++++++ .../src/workers/googleSheetsSync/service/backfill.spec.ts | 2 +- .../src/workers/googleSheetsSync/service/backfill.ts | 4 +++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apis/api-journeys-modern/src/lib/google/sheets.ts b/apis/api-journeys-modern/src/lib/google/sheets.ts index a39e8f8f4cf..d9eda06ea5f 100644 --- a/apis/api-journeys-modern/src/lib/google/sheets.ts +++ b/apis/api-journeys-modern/src/lib/google/sheets.ts @@ -267,6 +267,12 @@ export async function readValues({ return values } +// Escape a sheet title for use in A1 notation ranges. +// Per Google Sheets: wrap in single quotes, doubling any internal single quotes. +export function escapeSheetName(name: string): string { + return `'${name.replace(/'/g, "''")}'` +} + // Convert a 0-based column index to A1 column letters export function columnIndexToA1(colIndexZeroBased: number): string { let n = colIndexZeroBased + 1 diff --git a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts index 5fee2008b9a..b57f122d64a 100644 --- a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts +++ b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.spec.ts @@ -87,7 +87,7 @@ describe('backfillService', () => { expect(mockReadValues).toHaveBeenCalledWith({ accessToken: 'access-token', spreadsheetId: 'spreadsheet-id', - range: 'Sheet1!A:B' + range: "'Sheet1'!A:B" }) expect(mockClearSheet).not.toHaveBeenCalled() expect(mockWriteValues).not.toHaveBeenCalled() diff --git a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts index 35ebbd83e6c..13e260614de 100644 --- a/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts +++ b/apis/api-journeys-modern/src/workers/googleSheetsSync/service/backfill.ts @@ -8,6 +8,7 @@ import { clearSheet, columnIndexToA1, ensureSheet, + escapeSheetName, readValues, writeValues } from '../../../lib/google/sheets' @@ -328,7 +329,8 @@ export async function backfillService( const writeWidth = finalHeader.length const lastColumnA1 = columnIndexToA1(writeWidth - 1) - const fullRange = `${sheetName}!A:${lastColumnA1}` + const escapedName = escapeSheetName(sheetName) + const fullRange = `${escapedName}!A:${lastColumnA1}` let existingValues: (string | null)[][] = [] let readFailed = false