From 39b975c8e2d85318af97a101b54400ce5e3ec255 Mon Sep 17 00:00:00 2001 From: Rojan Rajbhandari Date: Tue, 9 Dec 2025 13:16:48 +0545 Subject: [PATCH 1/4] refactor(OUT-2776): make max attempts a constant --- src/features/failed-syncs/lib/RetryFailedSyncs.service.ts | 3 ++- src/features/failed-syncs/lib/constants.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 src/features/failed-syncs/lib/constants.ts diff --git a/src/features/failed-syncs/lib/RetryFailedSyncs.service.ts b/src/features/failed-syncs/lib/RetryFailedSyncs.service.ts index e1626cf..08d4b40 100644 --- a/src/features/failed-syncs/lib/RetryFailedSyncs.service.ts +++ b/src/features/failed-syncs/lib/RetryFailedSyncs.service.ts @@ -1,4 +1,5 @@ import AuthService from '@auth/lib/Auth.service' +import { MAX_RETRY_ATTEMPTS } from '@failed-syncs/lib/constants' import WebhookService from '@webhook/lib/webhook.service' import { eq, lte } from 'drizzle-orm' import env from '@/config/server.env' @@ -13,7 +14,7 @@ class RetryFailedSyncsService { const failedSyncRecords = await db .select() .from(failedSyncs) - .where(lte(failedSyncs.attempts, 3)) + .where(lte(failedSyncs.attempts, MAX_RETRY_ATTEMPTS)) const tokenMap: Record = {} diff --git a/src/features/failed-syncs/lib/constants.ts b/src/features/failed-syncs/lib/constants.ts new file mode 100644 index 0000000..2d32103 --- /dev/null +++ b/src/features/failed-syncs/lib/constants.ts @@ -0,0 +1 @@ +export const MAX_RETRY_ATTEMPTS = 3 From bfe5c050c89e4a83a181e23d57aba6270e3d078c Mon Sep 17 00:00:00 2001 From: Rojan Rajbhandari Date: Tue, 9 Dec 2025 14:47:14 +0545 Subject: [PATCH 2/4] fix(OUT-2776): safe fetching for getOrCreate settings handling race condition --- src/app/(home)/page.tsx | 2 +- .../lib/SyncedContacts.service.ts | 2 +- src/features/settings/lib/Settings.service.ts | 61 ++++++++++++------- .../webhook/api/webhook.controller.ts | 2 +- src/features/webhook/lib/webhook.service.ts | 4 +- 5 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/app/(home)/page.tsx b/src/app/(home)/page.tsx index 2d9e553..a476a21 100644 --- a/src/app/(home)/page.tsx +++ b/src/app/(home)/page.tsx @@ -27,7 +27,7 @@ const getSettings = async (user: User, connection: XeroConnection) => { if (connection.tenantId) { // Using tenantID even though tokenSet might be expired because the sync-settings feature don't need to perform Xero API calls const settingsService = new SettingsService(user, connection as XeroConnectionWithTokenSet) - settings = await settingsService.getSettings() + settings = await settingsService.getOrCreateSettings() } else { settings = defaultSettings } diff --git a/src/features/invoice-sync/lib/SyncedContacts.service.ts b/src/features/invoice-sync/lib/SyncedContacts.service.ts index bc59d47..481c3e6 100644 --- a/src/features/invoice-sync/lib/SyncedContacts.service.ts +++ b/src/features/invoice-sync/lib/SyncedContacts.service.ts @@ -25,7 +25,7 @@ class SyncedContactsService extends AuthenticatedXeroService { logger.info('SyncedContactsService#getSyncedContact :: Getting synced contact for', clientId) const settingsService = new SettingsService(this.user, this.connection) - const { useCompanyName } = await settingsService.getSettings() + const { useCompanyName } = await settingsService.getOrCreateSettings() const client = clientId ? await this.copilot.getClient(clientId) : undefined diff --git a/src/features/settings/lib/Settings.service.ts b/src/features/settings/lib/Settings.service.ts index 38f5288..09ffc4b 100644 --- a/src/features/settings/lib/Settings.service.ts +++ b/src/features/settings/lib/Settings.service.ts @@ -1,24 +1,51 @@ import { defaultSettings } from '@settings/constants/defaults' import { and, eq } from 'drizzle-orm' +import status from 'http-status' import { getTableFields } from '@/db/db.helpers' import { type SettingsFields, settings } from '@/db/schema/settings.schema' +import APIError from '@/errors/APIError' import logger from '@/lib/logger' import AuthenticatedXeroService from '@/lib/xero/AuthenticatedXero.service' class SettingsService extends AuthenticatedXeroService { - async getSettings(): Promise { + private readonly settingsFields = getTableFields(settings, [ + 'syncProductsAutomatically', + 'addAbsorbedFees', + 'useCompanyName', + 'isSyncEnabled', + 'initialInvoiceSettingsMapping', + 'initialProductSettingsMapping', + ]) + + private readonly MAX_RETRY_ATTEMPTS = 3 + + async getOrCreateSettings(attempt = 0): Promise { + const syncSettings = await this.getSettings() + if (syncSettings) return syncSettings + + const [newSyncSettings] = await this.db + .insert(settings) + .values({ + portalId: this.user.portalId, + tenantId: this.connection.tenantId, + // Default sync settings + ...defaultSettings, + }) + .onConflictDoNothing() + .returning(this.settingsFields) + + if (newSyncSettings) return newSyncSettings + + if (attempt > this.MAX_RETRY_ATTEMPTS) + throw new APIError('Failed to query settings for user', status.INTERNAL_SERVER_ERROR) + + return await this.getOrCreateSettings(attempt + 1) + } + + async getSettings(): Promise { logger.info('SettingsService#getSettings :: Getting settings for portalId', this.user.portalId) const [syncSettings] = await this.db - .select( - getTableFields(settings, [ - 'syncProductsAutomatically', - 'addAbsorbedFees', - 'useCompanyName', - 'isSyncEnabled', - 'initialInvoiceSettingsMapping', - 'initialProductSettingsMapping', - ]), - ) + .select(this.settingsFields) .from(settings) .where( and( @@ -26,17 +53,7 @@ class SettingsService extends AuthenticatedXeroService { eq(settings.tenantId, this.connection.tenantId), ), ) - if (syncSettings) { - return syncSettings - } - - const [newSyncSettings] = await this.db.insert(settings).values({ - portalId: this.user.portalId, - tenantId: this.connection.tenantId, - // Default sync settings - ...defaultSettings, - }) - return newSyncSettings + return syncSettings } } diff --git a/src/features/webhook/api/webhook.controller.ts b/src/features/webhook/api/webhook.controller.ts index ebdda9b..10227a2 100644 --- a/src/features/webhook/api/webhook.controller.ts +++ b/src/features/webhook/api/webhook.controller.ts @@ -14,7 +14,7 @@ export const handleCopilotWebhook = async (req: NextRequest) => { const connection = await authService.authorizeXeroForCopilotWorkspace() const settingsService = new SettingsService(user, connection) - const settings = await settingsService.getSettings() + const settings = await settingsService.getOrCreateSettings() if (!settings.isSyncEnabled) { logger.info( 'webhook/api/webhook.controller#handleCopilotWebhook :: Sync is disabled for this workspace. Skipping...', diff --git a/src/features/webhook/lib/webhook.service.ts b/src/features/webhook/lib/webhook.service.ts index 100b168..518344b 100644 --- a/src/features/webhook/lib/webhook.service.ts +++ b/src/features/webhook/lib/webhook.service.ts @@ -184,7 +184,7 @@ class WebhookService extends AuthenticatedXeroService { const data = PaymentSucceededEventSchema.parse(eventData) const settingsService = new SettingsService(this.user, this.connection) - const { addAbsorbedFees } = await settingsService.getSettings() + const { addAbsorbedFees } = await settingsService.getOrCreateSettings() if (!addAbsorbedFees) { logger.info( 'WebhookService#handlePaymentSucceeded :: addAbsorbedFees is disabled, skipping fee addition', @@ -200,7 +200,7 @@ class WebhookService extends AuthenticatedXeroService { private checkAutomaticProductSyncEnabled = async (): Promise => { const settingsService = new SettingsService(this.user, this.connection) - const settings = await settingsService.getSettings() + const settings = await settingsService.getOrCreateSettings() logger.info( 'WebhookService#checkAutomaticProductSyncEnabled :: Sync Products Automatically is set to', settings.syncProductsAutomatically, From d497df1c768ba371ecdded8b5dcb6147710f3955 Mon Sep 17 00:00:00 2001 From: Rojan Rajbhandari Date: Tue, 9 Dec 2025 14:54:56 +0545 Subject: [PATCH 3/4] fix(OUT-2775): force react to store a fresh copy of app bridge callbacks --- src/features/settings/hooks/useAppMenu.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/features/settings/hooks/useAppMenu.ts b/src/features/settings/hooks/useAppMenu.ts index ba282ed..e701498 100644 --- a/src/features/settings/hooks/useAppMenu.ts +++ b/src/features/settings/hooks/useAppMenu.ts @@ -11,7 +11,7 @@ export const useAppBridge = ({ token }: { token: string }) => { const { connectionStatus } = useAuthContext() const { isSyncEnabled, updateSettings, initialSettings } = useSettingsContext() - const disconnectAppAction = async () => { + const _disconnectAppAction = async () => { await disconnectApp(token) updateSettings({ isSyncEnabled: false, @@ -32,10 +32,12 @@ export const useAppBridge = ({ token }: { token: string }) => { // Quickfix for now (it will probably stay like this for the end of time) const [downloadCsvAction, setDownloadCsvAction] = useState(() => _downloadCsvAction) + const [disconnectAppAction, setDisconnectAppAction] = useState(() => _disconnectAppAction) setTimeout(() => { setDownloadCsvAction(() => _downloadCsvAction) - }, 1000) + setDisconnectAppAction(() => _disconnectAppAction) + }, 0) let actions: { label: string; icon?: Icons; onClick: () => Promise }[] = [] if (connectionStatus) { From 6e79ba041e5b346f4cd1fa3c5984b6e3ed0b0cc7 Mon Sep 17 00:00:00 2001 From: Rojan Rajbhandari Date: Tue, 9 Dec 2025 14:56:54 +0545 Subject: [PATCH 4/4] fix(OUT-2775): see if callback solves this issue --- src/features/settings/hooks/useAppMenu.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/features/settings/hooks/useAppMenu.ts b/src/features/settings/hooks/useAppMenu.ts index e701498..0003d96 100644 --- a/src/features/settings/hooks/useAppMenu.ts +++ b/src/features/settings/hooks/useAppMenu.ts @@ -3,7 +3,7 @@ import { useAuthContext } from '@auth/hooks/useAuth' import { disconnectApp } from '@settings/actions/disconnectApp' import { useSettingsContext } from '@settings/hooks/useSettings' -import { useState } from 'react' +import { useCallback } from 'react' import { useActionsMenu } from '@/lib/copilot/hooks/app-bridge' import { Icons } from '@/lib/copilot/hooks/app-bridge/types' @@ -11,16 +11,16 @@ export const useAppBridge = ({ token }: { token: string }) => { const { connectionStatus } = useAuthContext() const { isSyncEnabled, updateSettings, initialSettings } = useSettingsContext() - const _disconnectAppAction = async () => { + const disconnectAppAction = useCallback(async () => { await disconnectApp(token) updateSettings({ isSyncEnabled: false, initialSettings: { ...initialSettings, isSyncEnabled: false }, }) - } + }, [token, updateSettings, initialSettings]) // biome-ignore lint/suspicious/useAwait: there is no async action being done here but the type signature requires it - const _downloadCsvAction = async () => { + const downloadCsvAction = useCallback(async () => { const url = `/api/sync-logs?token=${token}` const link = document.createElement('a') link.href = url @@ -28,16 +28,7 @@ export const useAppBridge = ({ token }: { token: string }) => { document.body.appendChild(link) link.click() link.remove() - } - - // Quickfix for now (it will probably stay like this for the end of time) - const [downloadCsvAction, setDownloadCsvAction] = useState(() => _downloadCsvAction) - const [disconnectAppAction, setDisconnectAppAction] = useState(() => _disconnectAppAction) - - setTimeout(() => { - setDownloadCsvAction(() => _downloadCsvAction) - setDisconnectAppAction(() => _disconnectAppAction) - }, 0) + }, [token]) let actions: { label: string; icon?: Icons; onClick: () => Promise }[] = [] if (connectionStatus) {