Skip to content

Commit fc6fa6f

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(quickbooks): address parity review findings
1 parent 67cbcab commit fc6fa6f

9 files changed

Lines changed: 139 additions & 47 deletions

File tree

apps/docs/content/docs/en/integrations/quickbooks.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ List or read one account, class, customer, department, employee, item, or vendor
117117
|`BillAddr` | json | Customer or vendor billing address |
118118
|`ShipAddr` | json | Customer shipping address |
119119
|`Balance` | number | Customer or vendor balance |
120-
|`PrintOnCheckName` | string | Employee name printed on checks |
120+
|`PrintOnCheckName` | string | Vendor or employee name printed on checks |
121121
|`Vendor1099` | boolean | Whether the vendor is tracked for 1099 reporting |
122122
|`AcctNum` | string | Vendor account number |
123123
|`Description` | string | Item sales description |
@@ -173,7 +173,7 @@ List or read one account, class, customer, department, employee, item, or vendor
173173
|`BillAddr` | json | Customer or vendor billing address |
174174
|`ShipAddr` | json | Customer shipping address |
175175
|`Balance` | number | Customer or vendor balance |
176-
|`PrintOnCheckName` | string | Employee name printed on checks |
176+
|`PrintOnCheckName` | string | Vendor or employee name printed on checks |
177177
|`Vendor1099` | boolean | Whether the vendor is tracked for 1099 reporting |
178178
|`AcctNum` | string | Vendor account number |
179179
|`Description` | string | Item sales description |

apps/sim/app/api/tools/quickbooks/documents.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,30 @@ describe('QuickBooks document API routes', () => {
153153
expect(String(mockFetch.mock.calls[0][0])).toContain('/invoice/A%2FB/pdf')
154154
})
155155

156+
it('accepts Payment through the canonical PDF route contract', async () => {
157+
mockFetch.mockResolvedValueOnce(
158+
new Response('%PDF-1.4 fixture', {
159+
headers: { 'content-type': 'application/pdf' },
160+
})
161+
)
162+
163+
const response = await downloadTransactionPdf(
164+
createMockRequest('POST', {
165+
...auth,
166+
transactionType: 'payment',
167+
transactionId: '42',
168+
})
169+
)
170+
171+
expect(response.status).toBe(200)
172+
await expect(response.json()).resolves.toMatchObject({
173+
success: true,
174+
output: { transactionType: 'payment', transactionId: '42' },
175+
})
176+
expect(mockFetch).toHaveBeenCalledTimes(1)
177+
expect(String(mockFetch.mock.calls[0][0])).toContain('/payment/42/pdf')
178+
})
179+
156180
it('rejects non-PDF and oversized PDF responses', async () => {
157181
mockFetch.mockResolvedValueOnce(
158182
new Response('not a PDF', { headers: { 'content-type': 'text/plain' } })

apps/sim/blocks/blocks/quickbooks.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,11 +774,14 @@ export const QuickBooksBlock: BlockConfig<QuickBooksResponse> = {
774774
description:
775775
'Supported for purchase orders, bills, bill payments, and vendor credits. Purchase/Expense filtering is not exposed because its reference contract differs.',
776776
mode: 'advanced',
777-
condition: {
777+
condition: (values) => ({
778778
field: 'operation',
779779
value: PURCHASING_READ_OPERATION,
780-
and: { field: 'readMode', value: 'list' },
781-
},
780+
and:
781+
values?.purchasingTransactionType === 'purchase'
782+
? { field: 'purchasingTransactionType', value: 'purchase', not: true }
783+
: { field: 'readMode', value: 'list' },
784+
}),
782785
},
783786
{
784787
id: 'transactionType',
@@ -2090,7 +2093,10 @@ export const QuickBooksBlock: BlockConfig<QuickBooksResponse> = {
20902093
readMode: params.readMode,
20912094
startDate: optionalValue(params.readStartDate),
20922095
endDate: optionalValue(params.readEndDate),
2093-
vendorId: optionalValue(params.readVendorId),
2096+
vendorId:
2097+
params.purchasingTransactionType === 'purchase'
2098+
? undefined
2099+
: optionalValue(params.readVendorId),
20942100
startPosition: parsePaginationInteger(params.startPosition, 'startPosition', 1),
20952101
maxResults: parsePaginationInteger(params.maxResults, 'maxResults', 25),
20962102
}

apps/sim/lib/api/contracts/tools/quickbooks.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const documentTransactionTypeSchema = z.enum([
2626
'credit_memo',
2727
'estimate',
2828
'invoice',
29+
'payment',
2930
'purchase_order',
3031
'refund_receipt',
3132
'sales_receipt',

apps/sim/tools/generated/tool-outputs.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/quickbooks/purchasing.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { resetEnvMock, setEnv } from '@sim/testing'
22
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
3+
import { evaluateSubBlockCondition } from '@/lib/workflows/subblocks/visibility'
34
import { QuickBooksBlock } from '@/blocks/blocks/quickbooks'
45
import {
56
quickbooksCreateBillPaymentTool,
@@ -830,6 +831,35 @@ describe('QuickBooks BillPayment account compatibility', () => {
830831
})
831832

832833
describe('QuickBooks purchasing block', () => {
834+
it('hides and omits the unsupported Purchase/Expense vendor filter', () => {
835+
const readVendorId = QuickBooksBlock.subBlocks.find(
836+
(candidate) => candidate.id === 'readVendorId'
837+
)
838+
expect(
839+
evaluateSubBlockCondition(readVendorId?.condition, {
840+
operation: 'quickbooks_read_purchasing_transactions',
841+
readMode: 'list',
842+
purchasingTransactionType: 'bill',
843+
})
844+
).toBe(true)
845+
expect(
846+
evaluateSubBlockCondition(readVendorId?.condition, {
847+
operation: 'quickbooks_read_purchasing_transactions',
848+
readMode: 'list',
849+
purchasingTransactionType: 'purchase',
850+
})
851+
).toBe(false)
852+
expect(
853+
QuickBooksBlock.tools.config!.params!({
854+
operation: 'quickbooks_read_purchasing_transactions',
855+
oauthCredential: 'credential-id',
856+
purchasingTransactionType: 'purchase',
857+
readMode: 'list',
858+
readVendorId: '62',
859+
})
860+
).toMatchObject({ transactionType: 'purchase', vendorId: undefined })
861+
})
862+
833863
it('keeps the shared purchasing-lines example valid for every supported operation', () => {
834864
const subBlock = QuickBooksBlock.subBlocks.find(
835865
(candidate) => candidate.id === 'purchasingLines'

apps/sim/tools/quickbooks/reports.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,23 @@ describe('QuickBooks financial report request construction', () => {
155155
})
156156
})
157157

158+
it.each([
159+
['transactionType', 'bogus'],
160+
['groupBy', 'bogus'],
161+
['accountsPayablePaid', 'bogus'],
162+
['accountsReceivablePaid', 'bogus'],
163+
['clearedStatus', 'bogus'],
164+
['sourceAccountType', 'bogus'],
165+
] as const)('rejects unsupported Transaction List %s values before fetch', (field, value) => {
166+
expect(() =>
167+
buildQuickBooksReportUrl({
168+
...authParams,
169+
reportType: 'transaction_list',
170+
[field]: value,
171+
} as QuickBooksRunFinancialReportParams)
172+
).toThrow(`Unsupported QuickBooks ${field}`)
173+
})
174+
158175
it('omits blank and default controls', () => {
159176
const url = buildQuickBooksReportUrl({
160177
...authParams,
@@ -191,7 +208,7 @@ describe('QuickBooks financial report request construction', () => {
191208
).toThrow(message)
192209
})
193210

194-
it('contains exactly the sandbox-verified 15-report support matrix', () => {
211+
it('contains exactly the fixed 15-report support matrix', () => {
195212
expect(Object.keys(QUICKBOOKS_REPORTS)).toEqual(reportEndpoints.map(([type]) => type))
196213
expect(Object.keys(QUICKBOOKS_REPORTS)).not.toContain('general_ledger')
197214
expect(quickbooksRunFinancialReportTool.params).toHaveProperty('transactionType')

apps/sim/tools/quickbooks/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,6 +1543,11 @@ export const QUICKBOOKS_MASTER_DATA_PROPERTIES: Record<string, OutputProperty> =
15431543
...QUICKBOOKS_VENDOR_PROPERTIES,
15441544
...QUICKBOOKS_ITEM_PROPERTIES,
15451545
...QUICKBOOKS_EMPLOYEE_PROPERTIES,
1546+
PrintOnCheckName: {
1547+
type: 'string',
1548+
description: 'Vendor or employee name printed on checks',
1549+
optional: true,
1550+
},
15461551
Name: {
15471552
type: 'string',
15481553
description: 'Account, item, class, or department name',

apps/sim/tools/quickbooks/utils.ts

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -408,19 +408,52 @@ const QUICKBOOKS_TRANSACTION_LIST_VALUES = {
408408
},
409409
} as const
410410

411+
function getQuickBooksTransactionListControl(
412+
value: unknown,
413+
values: Record<string, string>,
414+
field: string
415+
): string | undefined {
416+
if (value === undefined || value === 'default') return undefined
417+
if (typeof value !== 'string' || !Object.hasOwn(values, value) || !values[value]) {
418+
throw new Error(`Unsupported QuickBooks ${field}: ${String(value)}`)
419+
}
420+
return values[value]
421+
}
422+
411423
function addQuickBooksTransactionListFilters(
412424
url: URL,
413425
params: QuickBooksRunFinancialReportParams
414426
): void {
415-
const transactionType = params.transactionType === 'default' ? undefined : params.transactionType
416-
const groupBy = params.groupBy === 'default' ? undefined : params.groupBy
417-
const accountsPayablePaid =
418-
params.accountsPayablePaid === 'default' ? undefined : params.accountsPayablePaid
419-
const accountsReceivablePaid =
420-
params.accountsReceivablePaid === 'default' ? undefined : params.accountsReceivablePaid
421-
const clearedStatus = params.clearedStatus === 'default' ? undefined : params.clearedStatus
422-
const sourceAccountType =
423-
params.sourceAccountType === 'default' ? undefined : params.sourceAccountType
427+
const transactionType = getQuickBooksTransactionListControl(
428+
params.transactionType,
429+
QUICKBOOKS_TRANSACTION_LIST_VALUES.transactionType,
430+
'transactionType'
431+
)
432+
const groupBy = getQuickBooksTransactionListControl(
433+
params.groupBy,
434+
QUICKBOOKS_TRANSACTION_LIST_VALUES.groupBy,
435+
'groupBy'
436+
)
437+
const accountsPayablePaid = getQuickBooksTransactionListControl(
438+
params.accountsPayablePaid,
439+
QUICKBOOKS_TRANSACTION_LIST_VALUES.paidStatus,
440+
'accountsPayablePaid'
441+
)
442+
const accountsReceivablePaid = getQuickBooksTransactionListControl(
443+
params.accountsReceivablePaid,
444+
QUICKBOOKS_TRANSACTION_LIST_VALUES.paidStatus,
445+
'accountsReceivablePaid'
446+
)
447+
const clearedStatus = getQuickBooksTransactionListControl(
448+
params.clearedStatus,
449+
QUICKBOOKS_TRANSACTION_LIST_VALUES.clearedStatus,
450+
'clearedStatus'
451+
)
452+
const sourceAccountType = getQuickBooksTransactionListControl(
453+
params.sourceAccountType,
454+
QUICKBOOKS_TRANSACTION_LIST_VALUES.sourceAccountType,
455+
'sourceAccountType'
456+
)
424457
const controls = {
425458
transaction_type: transactionType,
426459
group_by: groupBy,
@@ -436,37 +469,13 @@ function addQuickBooksTransactionListFilters(
436469
return
437470
}
438471

439-
if (transactionType) {
440-
url.searchParams.set(
441-
'transaction_type',
442-
QUICKBOOKS_TRANSACTION_LIST_VALUES.transactionType[transactionType]
443-
)
444-
}
445-
if (groupBy) {
446-
url.searchParams.set('group_by', QUICKBOOKS_TRANSACTION_LIST_VALUES.groupBy[groupBy])
447-
}
448-
if (accountsPayablePaid) {
449-
url.searchParams.set(
450-
'appaid',
451-
QUICKBOOKS_TRANSACTION_LIST_VALUES.paidStatus[accountsPayablePaid]
452-
)
453-
}
454-
if (accountsReceivablePaid) {
455-
url.searchParams.set(
456-
'arpaid',
457-
QUICKBOOKS_TRANSACTION_LIST_VALUES.paidStatus[accountsReceivablePaid]
458-
)
459-
}
460-
if (clearedStatus) {
461-
url.searchParams.set('cleared', QUICKBOOKS_TRANSACTION_LIST_VALUES.clearedStatus[clearedStatus])
462-
}
472+
if (transactionType) url.searchParams.set('transaction_type', transactionType)
473+
if (groupBy) url.searchParams.set('group_by', groupBy)
474+
if (accountsPayablePaid) url.searchParams.set('appaid', accountsPayablePaid)
475+
if (accountsReceivablePaid) url.searchParams.set('arpaid', accountsReceivablePaid)
476+
if (clearedStatus) url.searchParams.set('cleared', clearedStatus)
463477
if (controls.docnum) url.searchParams.set('docnum', controls.docnum)
464-
if (sourceAccountType) {
465-
url.searchParams.set(
466-
'source_account_type',
467-
QUICKBOOKS_TRANSACTION_LIST_VALUES.sourceAccountType[sourceAccountType]
468-
)
469-
}
478+
if (sourceAccountType) url.searchParams.set('source_account_type', sourceAccountType)
470479
}
471480

472481
export function buildQuickBooksReportUrl(params: QuickBooksRunFinancialReportParams): URL {

0 commit comments

Comments
 (0)