feat(quickbooks): add generic query action#21
Conversation
WalkthroughThis PR introduces a new Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 954912b1da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const withStart = /\\bSTARTPOSITION\\b/i.test(query) ? query : `${query} STARTPOSITION ${startPosition}`; | ||
| return /\\bMAXRESULTS\\b/i.test(withStart) ? withStart : `${withStart} MAXRESULTS ${maxResults}`; |
There was a problem hiding this comment.
The pagination guard regexes are double-escaped (/\\b...\\b/), so they never match existing STARTPOSITION or MAXRESULTS tokens in user-provided queries. As a result, queries that already include either clause will have duplicates appended, which can produce invalid QuickBooks query strings or unexpected paging behavior for callers who pass explicit pagination.
Useful? React with 👍 / 👎.
| const parsed = responseSchema.parse(response.data); | ||
| const queryResponse = parsed.QueryResponse ?? { maxResults: 0, startPosition: 1, totalCount: 0 }; | ||
| const records = extractRecords(queryResponse); |
There was a problem hiding this comment.
When QuickBooks returns a Fault payload without QueryResponse (e.g., malformed query or permission error), this code parses successfully and falls back to an empty synthetic response, returning records: [] instead of raising an error. That silently converts real API failures into apparent successful empty results, which can hide production issues and mislead downstream automation.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nango.dev/quickbooks/actions/query-entities-shared.ts`:
- Around line 51-53: The responseSchema currently marks QueryResponse as
optional which masks malformed responses; change responseSchema to require
QueryResponse (remove .optional()) and remove any code that supplies a fallback
empty QueryResponse object when parsing/validating (ensure you use the strict
zod parsing on responseSchema and let validation fail), referencing the
responseSchema and queryResponseSchema symbols so the validator enforces a
present QueryResponse instead of defaulting to { maxResults: 0, startPosition:
1, totalCount: 0 }.
- Around line 105-106: The regexes used to detect existing pagination clauses
are incorrectly escaped (they contain literal backslashes), so change the
patterns used when testing query to use real word-boundary tokens rather than
backslash characters; update the two tests that reference STARTPOSITION and
MAXRESULTS (the expressions around withStart, query, startPosition, and
maxResults) to use /\bSTARTPOSITION\b/i and /\bMAXRESULTS\b/i (preserving the i
flag) so existing clauses are detected correctly and duplicates are not
appended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ef97deb-dae8-4b38-b890-6b0bf76e337e
📒 Files selected for processing (4)
nango.dev/index.tsnango.dev/quickbooks-sandbox/actions/query-entities.tsnango.dev/quickbooks/actions/query-entities-shared.tsnango.dev/quickbooks/actions/query-entities.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-07T15:17:35.164Z
Learnt from: mchlggr
Repo: ariso-ai/link PR: 6
File: nango.dev/index.ts:17-23
Timestamp: 2026-05-07T15:17:35.164Z
Learning: In this repository, TypeScript ESM import specifiers intentionally use `.js` extensions (e.g., `import './path/to/file.js'`) even when the referenced source files are written in `.ts`. Treat `.js` extensions in TS/TSX import paths as correct for this project’s established convention—do not flag them as incorrect, since the project’s ESM TypeScript setup resolves the `.js`-suffixed specifiers to `.ts` sources.
Applied to files:
nango.dev/quickbooks/actions/query-entities.tsnango.dev/quickbooks-sandbox/actions/query-entities.tsnango.dev/index.tsnango.dev/quickbooks/actions/query-entities-shared.ts
🔇 Additional comments (3)
nango.dev/quickbooks/actions/query-entities.ts (1)
1-14: LGTM!nango.dev/quickbooks-sandbox/actions/query-entities.ts (1)
1-18: LGTM!nango.dev/index.ts (1)
31-32: LGTM!
| const responseSchema = z.object({ | ||
| QueryResponse: queryResponseSchema.optional(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the QuickBooks Online Accounting API v3 query endpoint, does every successful response include a top-level QueryResponse object?
💡 Result:
Yes, every successful response from the QuickBooks Online Accounting API v3 query endpoint includes a top-level QueryResponse object [1]. When you execute a query using the /query endpoint, the API returns a JSON (or XML) payload where the root element is QueryResponse [1]. This object acts as a container for the requested data, which is nested within it under the name of the entity being queried (e.g., Customer, Invoice) [1]. If no records match the query criteria, the QueryResponse object will still be present, but the entity-specific collection within it may be empty or omitted [2][1].
Citations:
- 1: https://medium.com/intuitdev/deep-dive-into-quickbooks-online-data-queries-b77034bdc144
- 2: https://help.developer.intuit.com/s/question/0D5G000004Lsf6VKAR/vendor-queryresponse-returns-empty-object-for-customer-via-qbo-api
🏁 Script executed:
fd query-entities-shared.ts --type fRepository: ariso-ai/link
Length of output: 111
🏁 Script executed:
cat -n nango.dev/quickbooks/actions/query-entities-shared.ts | head -150Repository: ariso-ai/link
Length of output: 6186
Remove the optional flag and fallback to prevent masking malformed API responses.
QuickBooks Online Accounting API v3 always returns a QueryResponse object in successful responses. Making it optional and defaulting to an empty object { maxResults: 0, startPosition: 1, totalCount: 0 } silently masks unexpected or malformed payloads as a successful empty result instead of surfacing the error.
Proposed fix
const responseSchema = z.object({
- QueryResponse: queryResponseSchema.optional(),
+ QueryResponse: queryResponseSchema,
});
@@
- const queryResponse = parsed.QueryResponse ?? { maxResults: 0, startPosition: 1, totalCount: 0 };
+ const queryResponse = parsed.QueryResponse;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nango.dev/quickbooks/actions/query-entities-shared.ts` around lines 51 - 53,
The responseSchema currently marks QueryResponse as optional which masks
malformed responses; change responseSchema to require QueryResponse (remove
.optional()) and remove any code that supplies a fallback empty QueryResponse
object when parsing/validating (ensure you use the strict zod parsing on
responseSchema and let validation fail), referencing the responseSchema and
queryResponseSchema symbols so the validator enforces a present QueryResponse
instead of defaulting to { maxResults: 0, startPosition: 1, totalCount: 0 }.
| const withStart = /\\bSTARTPOSITION\\b/i.test(query) ? query : `${query} STARTPOSITION ${startPosition}`; | ||
| return /\\bMAXRESULTS\\b/i.test(withStart) ? withStart : `${withStart} MAXRESULTS ${maxResults}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the current file contains double-escaped word-boundary markers.
rg -nP '\/\\\\bSTARTPOSITION\\\\b\/i|\/\\\\bMAXRESULTS\\\\b\/i' nango.dev/quickbooks/actions/query-entities-shared.tsRepository: ariso-ai/link
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Read lines 105-106 directly from the file to see exact source content
sed -n '105,106p' nango.dev/quickbooks/actions/query-entities-shared.tsRepository: ariso-ai/link
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Read the exact content using Python to see the real characters
python3 << 'EOF'
with open('nango.dev/quickbooks/actions/query-entities-shared.ts', 'r') as f:
lines = f.readlines()
for i in range(104, 106): # 0-indexed, so 104-105 for lines 105-106
line = lines[i]
print(f"Line {i+1}: {repr(line)}")
EOFRepository: ariso-ai/link
Length of output: 305
🏁 Script executed:
#!/bin/bash
# Read the full appendPagination function context
sed -n '95,115p' nango.dev/quickbooks/actions/query-entities-shared.tsRepository: ariso-ai/link
Length of output: 879
Fix regex word boundary escaping to properly detect existing pagination clauses.
The patterns /\\bSTARTPOSITION\\b/i and /\\bMAXRESULTS\\b/i match literal backslash characters instead of word boundaries, so clause detection fails and duplicate pagination parameters can be appended.
Proposed fix
- const withStart = /\\bSTARTPOSITION\\b/i.test(query) ? query : `${query} STARTPOSITION ${startPosition}`;
- return /\\bMAXRESULTS\\b/i.test(withStart) ? withStart : `${withStart} MAXRESULTS ${maxResults}`;
+ const withStart = /\bSTARTPOSITION\b/i.test(query) ? query : `${query} STARTPOSITION ${startPosition}`;
+ return /\bMAXRESULTS\b/i.test(withStart) ? withStart : `${withStart} MAXRESULTS ${maxResults}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const withStart = /\\bSTARTPOSITION\\b/i.test(query) ? query : `${query} STARTPOSITION ${startPosition}`; | |
| return /\\bMAXRESULTS\\b/i.test(withStart) ? withStart : `${withStart} MAXRESULTS ${maxResults}`; | |
| const withStart = /\bSTARTPOSITION\b/i.test(query) ? query : `${query} STARTPOSITION ${startPosition}`; | |
| return /\bMAXRESULTS\b/i.test(withStart) ? withStart : `${withStart} MAXRESULTS ${maxResults}`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nango.dev/quickbooks/actions/query-entities-shared.ts` around lines 105 -
106, The regexes used to detect existing pagination clauses are incorrectly
escaped (they contain literal backslashes), so change the patterns used when
testing query to use real word-boundary tokens rather than backslash characters;
update the two tests that reference STARTPOSITION and MAXRESULTS (the
expressions around withStart, query, startPosition, and maxResults) to use
/\bSTARTPOSITION\b/i and /\bMAXRESULTS\b/i (preserving the i flag) so existing
clauses are detected correctly and duplicates are not appended.
1 participant
Adds a generic QuickBooks
query-entitiesNango action based on Nango's template. It runs SQL-like QBO queries, applies start/max pagination when omitted, and returns normalized records/count/paging metadata. Registered for bothquickbooksandquickbooks-sandboxwithout duplicating implementation. Deployed to Nango dev for both providers; compile/typecheck passed.