Skip to content

Commit de43b7f

Browse files
Bill Leoutsakoswaleedlatif1
authored andcommitted
refactor(quickbooks): align webhook trigger routing
1 parent 0df9ca6 commit de43b7f

8 files changed

Lines changed: 208 additions & 321 deletions

File tree

‎apps/sim/background/quickbooks-webhook-ingress.test.ts‎

Lines changed: 37 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22

33
import { beforeEach, describe, expect, it, vi } from 'vitest'
44

5-
const { mockDispatch, mockEnqueue, mockFindPage } = vi.hoisted(() => ({
5+
const { mockDispatch, mockEnqueue, mockFindWebhooks } = vi.hoisted(() => ({
66
mockDispatch: vi.fn(),
77
mockEnqueue: vi.fn(),
8-
mockFindPage: vi.fn(),
8+
mockFindWebhooks: vi.fn(),
99
}))
10-
vi.mock('@trigger.dev/sdk', () => ({ task: vi.fn((config: unknown) => config) }))
11-
vi.mock('@/lib/webhooks/processor', () => ({ dispatchResolvedWebhookTarget: mockDispatch }))
12-
vi.mock('@/background/quickbooks-webhook-targets', () => ({
13-
findQuickBooksWebhookTargetPage: mockFindPage,
10+
vi.mock('@trigger.dev/sdk', () => ({
11+
task: vi.fn((config: unknown) => config),
12+
}))
13+
vi.mock('@/lib/webhooks/processor', () => ({
14+
dispatchResolvedWebhookTarget: mockDispatch,
15+
findWebhooksByRoutingKey: mockFindWebhooks,
1416
}))
1517
vi.mock('@/lib/core/async-jobs', () => ({
1618
getJobQueue: vi.fn(async () => ({ enqueue: mockEnqueue })),
@@ -46,45 +48,33 @@ describe('QuickBooks webhook ingress job', () => {
4648

4749
it('routes the batch by company and dispatches targets sequentially', async () => {
4850
const order: string[] = []
49-
mockFindPage
50-
.mockResolvedValueOnce({
51-
hasMore: true,
52-
nextCursor: 'w2',
53-
targets: [
54-
{ webhook: { id: 'w1' }, workflow: { id: 'wf1' } },
55-
{ webhook: { id: 'w2' }, workflow: { id: 'wf2' } },
56-
],
57-
})
58-
.mockResolvedValueOnce({
59-
hasMore: false,
60-
nextCursor: null,
61-
targets: [{ webhook: { id: 'w3' }, workflow: { id: 'wf3' } }],
62-
})
63-
.mockResolvedValueOnce({
64-
hasMore: false,
65-
nextCursor: null,
66-
targets: [{ webhook: { id: 'w4' }, workflow: { id: 'wf4' } }],
67-
})
51+
mockFindWebhooks
52+
.mockResolvedValueOnce([
53+
{ webhook: { id: 'w1' }, workflow: { id: 'wf1' } },
54+
{ webhook: { id: 'w2' }, workflow: { id: 'wf2' } },
55+
])
56+
.mockResolvedValueOnce([{ webhook: { id: 'w3' }, workflow: { id: 'wf3' } }])
6857
mockDispatch.mockImplementation(async (webhook: { id: string }) => {
6958
order.push(webhook.id)
7059
return { outcome: 'queued' }
7160
})
7261
await expect(executeQuickBooksWebhookIngress(payload)).resolves.toEqual({
7362
failed: 0,
7463
ignored: 0,
75-
processed: 4,
76-
targetCount: 4,
64+
processed: 3,
65+
targetCount: 3,
7766
})
78-
expect(mockFindPage).toHaveBeenCalledWith('456', 'request-1', undefined)
79-
expect(mockFindPage).toHaveBeenCalledWith('456', 'request-1', 'w2')
80-
expect(mockFindPage).toHaveBeenCalledWith('789', 'request-1', undefined)
81-
expect(order).toEqual(['w1', 'w2', 'w3', 'w4'])
67+
expect(mockFindWebhooks).toHaveBeenNthCalledWith(1, '456', 'request-1', 'quickbooks')
68+
expect(mockFindWebhooks).toHaveBeenNthCalledWith(2, '789', 'request-1', 'quickbooks')
69+
expect(order).toEqual(['w1', 'w2', 'w3'])
8270
})
8371

8472
it('enqueues the bounded delivery once without copying it into continuation jobs', async () => {
85-
mockFindPage.mockResolvedValue({ hasMore: false, nextCursor: null, targets: [] })
73+
mockFindWebhooks.mockResolvedValue([])
8674
await enqueueQuickBooksWebhookIngress(payload)
87-
const options = mockEnqueue.mock.calls[0][2] as { runner: () => Promise<void> }
75+
const options = mockEnqueue.mock.calls[0][2] as {
76+
runner: () => Promise<void>
77+
}
8878
await options.runner()
8979
expect(mockEnqueue).toHaveBeenCalledOnce()
9080
expect(mockEnqueue).toHaveBeenCalledWith(
@@ -97,46 +87,40 @@ describe('QuickBooks webhook ingress job', () => {
9787
})
9888

9989
it('continues later events before retrying a delivery with failed targets', async () => {
100-
mockFindPage
101-
.mockResolvedValueOnce({
102-
hasMore: false,
103-
nextCursor: null,
104-
targets: [
105-
{ webhook: { id: 'w1' }, workflow: { id: 'wf1' } },
106-
{ webhook: { id: 'w2' }, workflow: { id: 'wf2' } },
107-
],
108-
})
109-
.mockResolvedValueOnce({
110-
hasMore: false,
111-
nextCursor: null,
112-
targets: [{ webhook: { id: 'w3' }, workflow: { id: 'wf3' } }],
113-
})
90+
mockFindWebhooks
91+
.mockResolvedValueOnce([
92+
{ webhook: { id: 'w1' }, workflow: { id: 'wf1' } },
93+
{ webhook: { id: 'w2' }, workflow: { id: 'wf2' } },
94+
])
95+
.mockResolvedValueOnce([{ webhook: { id: 'w3' }, workflow: { id: 'wf3' } }])
11496
mockDispatch
11597
.mockRejectedValueOnce(new Error('dispatch unavailable'))
11698
.mockResolvedValueOnce({ outcome: 'failed' })
11799
.mockResolvedValueOnce({ outcome: 'queued' })
118100

119101
await enqueueQuickBooksWebhookIngress(payload)
120-
const options = mockEnqueue.mock.calls[0][2] as { runner: () => Promise<void> }
102+
const options = mockEnqueue.mock.calls[0][2] as {
103+
runner: () => Promise<void>
104+
}
121105
await expect(options.runner()).rejects.toThrow(
122106
'QuickBooks webhook delivery completed with 2 failures'
123107
)
124-
expect(mockFindPage).toHaveBeenCalledWith('789', 'request-1', undefined)
108+
expect(mockFindWebhooks).toHaveBeenCalledWith('789', 'request-1', 'quickbooks')
125109
expect(mockDispatch).toHaveBeenCalledTimes(3)
126110
expect(mockEnqueue).toHaveBeenCalledOnce()
127111
})
128112

129-
it('continues later events when a target page cannot be resolved', async () => {
130-
mockFindPage
113+
it('continues later events when targets cannot be resolved', async () => {
114+
mockFindWebhooks
131115
.mockRejectedValueOnce(new Error('database unavailable'))
132-
.mockResolvedValueOnce({ hasMore: false, nextCursor: null, targets: [] })
116+
.mockResolvedValueOnce([])
133117

134118
await expect(executeQuickBooksWebhookIngress(payload)).resolves.toEqual({
135119
failed: 1,
136120
ignored: 0,
137121
processed: 0,
138122
targetCount: 0,
139123
})
140-
expect(mockFindPage).toHaveBeenCalledWith('789', 'request-1', undefined)
124+
expect(mockFindWebhooks).toHaveBeenCalledWith('789', 'request-1', 'quickbooks')
141125
})
142126
})

‎apps/sim/background/quickbooks-webhook-ingress.ts‎

Lines changed: 43 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import { task } from '@trigger.dev/sdk'
33
import { NextRequest } from 'next/server'
44
import type { QuickBooksWebhookEvent } from '@/lib/api/contracts/webhooks'
55
import { getJobQueue } from '@/lib/core/async-jobs'
6-
import { dispatchResolvedWebhookTarget } from '@/lib/webhooks/processor'
7-
import { findQuickBooksWebhookTargetPage } from '@/background/quickbooks-webhook-targets'
6+
import { dispatchResolvedWebhookTarget, findWebhooksByRoutingKey } from '@/lib/webhooks/processor'
87

98
const logger = createLogger('QuickBooksWebhookIngressTask')
109

@@ -25,7 +24,7 @@ export interface QuickBooksWebhookIngressResult {
2524
targetCount: number
2625
}
2726

28-
/** Process the bounded delivery sequentially, retaining at most one target page at a time. */
27+
/** Process the bounded delivery sequentially through the shared routing-key dispatcher. */
2928
export async function executeQuickBooksWebhookIngress(
3029
payload: QuickBooksWebhookIngressPayload
3130
): Promise<QuickBooksWebhookIngressResult> {
@@ -40,61 +39,51 @@ export async function executeQuickBooksWebhookIngress(
4039
headers: payload.headers,
4140
body: JSON.stringify(event),
4241
})
43-
let afterWebhookId: string | undefined
4442

45-
while (true) {
46-
try {
47-
const page = await findQuickBooksWebhookTargetPage(
48-
event.intuitaccountid,
49-
payload.requestId,
50-
afterWebhookId
51-
)
52-
const nextCursor = page.hasMore ? page.nextCursor : null
53-
if (page.hasMore && (!nextCursor || nextCursor === afterWebhookId)) {
54-
throw new Error('QuickBooks webhook target pagination did not advance')
55-
}
43+
try {
44+
const targets = await findWebhooksByRoutingKey(
45+
event.intuitaccountid,
46+
payload.requestId,
47+
'quickbooks'
48+
)
49+
targetCount += targets.length
5650

57-
targetCount += page.targets.length
58-
for (const { webhook, workflow } of page.targets) {
59-
try {
60-
const result = await dispatchResolvedWebhookTarget(webhook, workflow, event, request, {
61-
requestId: payload.requestId,
62-
path: webhook.path ?? undefined,
63-
receivedAt: payload.receivedAt,
64-
triggerTimestampMs: Date.parse(event.time),
65-
})
66-
if (result.outcome === 'queued') processed += 1
67-
else if (result.outcome === 'ignored') ignored += 1
68-
else failed += 1
69-
} catch (error) {
70-
failed += 1
71-
logger.error(`[${payload.requestId}] QuickBooks webhook target dispatch failed`, {
72-
error,
73-
eventId: event.id,
74-
eventIndex,
75-
webhookId: webhook.id,
76-
})
77-
}
51+
for (const { webhook, workflow } of targets) {
52+
try {
53+
const result = await dispatchResolvedWebhookTarget(webhook, workflow, event, request, {
54+
requestId: payload.requestId,
55+
path: webhook.path ?? undefined,
56+
receivedAt: payload.receivedAt,
57+
triggerTimestampMs: Date.parse(event.time),
58+
})
59+
if (result.outcome === 'queued') processed += 1
60+
else if (result.outcome === 'ignored') ignored += 1
61+
else failed += 1
62+
} catch (error) {
63+
failed += 1
64+
logger.error(`[${payload.requestId}] QuickBooks webhook target dispatch failed`, {
65+
error,
66+
eventId: event.id,
67+
eventIndex,
68+
webhookId: webhook.id,
69+
})
7870
}
79-
80-
logger.info(`[${payload.requestId}] QuickBooks webhook page completed`, {
81-
eventId: event.id,
82-
eventIndex,
83-
ignored,
84-
processed,
85-
targetCount: page.targets.length,
86-
})
87-
if (!nextCursor) break
88-
afterWebhookId = nextCursor
89-
} catch (error) {
90-
failed += 1
91-
logger.error(`[${payload.requestId}] QuickBooks webhook event page failed`, {
92-
error,
93-
eventId: event.id,
94-
eventIndex,
95-
})
96-
break
9771
}
72+
73+
logger.info(`[${payload.requestId}] QuickBooks webhook event completed`, {
74+
eventId: event.id,
75+
eventIndex,
76+
ignored,
77+
processed,
78+
targetCount: targets.length,
79+
})
80+
} catch (error) {
81+
failed += 1
82+
logger.error(`[${payload.requestId}] QuickBooks webhook target lookup failed`, {
83+
error,
84+
eventId: event.id,
85+
eventIndex,
86+
})
9887
}
9988
}
10089

‎apps/sim/background/quickbooks-webhook-targets.test.ts‎

Lines changed: 0 additions & 98 deletions
This file was deleted.

0 commit comments

Comments
 (0)