feat(quickbooks): Add missing invoice, estimate, and purchase order actions #16908
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@krikera is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces new QuickBooks action modules to create, send, update, and void invoices and estimates, as well as create purchase orders. It also adds supporting methods and property definitions to the QuickBooks app module, enabling these endpoints and their required input handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActionModule
participant QuickBooksApp
participant QuickBooksAPI
User->>ActionModule: Provide input (e.g., create/update/send/void)
ActionModule->>QuickBooksApp: Call appropriate method with input
QuickBooksApp->>QuickBooksAPI: Make HTTP request to API endpoint
QuickBooksAPI-->>QuickBooksApp: Return API response
QuickBooksApp-->>ActionModule: Return processed result
ActionModule-->>User: Output summary/result
Assessment against linked issues
Poem
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
components/quickbooks/actions/update-estimate/update-estimate.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/quickbooks/actions/create-estimate/create-estimate.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/quickbooks/actions/create-purchase-order/create-purchase-order.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
components/quickbooks/actions/update-invoice/update-invoice.mjs (1)
92-97: Verify if taxCodeId should be optional.The
taxCodeIdprop is not marked as optional and is not being used in the update data, which seems inconsistent for an update operation.
🧹 Nitpick comments (2)
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs (1)
118-131: Improve validation error handling.The validation logic is good but could be more specific and handle edge cases better.
Consider these improvements:
async run({ $ }) { - if ((!this.numLineItems && !this.lineItemsAsObjects) || !this.vendorRefValue) { - throw new ConfigurationError("Must provide lineItems and vendorRefValue parameters."); + if (!this.vendorRefValue) { + throw new ConfigurationError("Vendor is required to create a purchase order."); + } + + if (!this.numLineItems && !this.lineItemsAsObjects) { + throw new ConfigurationError("At least one line item is required. Either specify the number of line items or provide line items as objects."); } const lines = this.lineItemsAsObjects ? parseLineItems(this.lineItems) : this.buildLineItems(); + if (!lines || lines.length === 0) { + throw new ConfigurationError("No valid line items were provided."); + } lines.forEach((line) => { if (line.DetailType !== "ItemBasedExpenseLineDetail" && line.DetailType !== "AccountBasedExpenseLineDetail") { throw new ConfigurationError("Line Item DetailType must be `ItemBasedExpenseLineDetail` or `AccountBasedExpenseLineDetail`"); } });components/quickbooks/actions/update-invoice/update-invoice.mjs (1)
149-164: Consider extracting common line item building logic.The
buildLineItemsmethod is duplicated across all three action files. Consider extracting this to a shared utility.You could create a shared utility in
components/quickbooks/common/utils.mjs:export function buildSalesLineItems(numLineItems, context) { const lineItems = []; for (let i = 1; i <= numLineItems; i++) { lineItems.push({ DetailType: "SalesItemLineDetail", Amount: context[`amount_${i}`], SalesItemLineDetail: { ItemRef: { value: context[`item_${i}`], }, }, }); } return lineItems; }This would reduce code duplication and make maintenance easier across all QuickBooks actions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/quickbooks/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks/actions/send-estimate/send-estimate.mjs(1 hunks)components/quickbooks/actions/send-invoice/send-invoice.mjs(1 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs(1 hunks)components/quickbooks/actions/void-invoice/void-invoice.mjs(1 hunks)components/quickbooks/quickbooks.app.mjs(2 hunks)
🔇 Additional comments (19)
components/quickbooks/actions/send-estimate/send-estimate.mjs (1)
1-43: LGTM! Well-structured action with proper conditional logic.This action follows good patterns for Pipedream QuickBooks actions:
- Uses propDefinition for consistent ID selection
- Properly handles optional email parameter
- Clean conditional data construction
- Appropriate success messaging
The implementation correctly handles the optional email parameter by conditionally adding it to the data object only when provided.
components/quickbooks/actions/send-invoice/send-invoice.mjs (1)
1-43: LGTM! Consistent implementation with send-estimate action.This action correctly mirrors the send-estimate implementation with appropriate changes for invoices:
- Uses
invoiceIdpropDefinition consistently- Maintains the same conditional email logic pattern
- Calls the corresponding
sendInvoicemethod- Provides appropriate success messaging
The consistency between similar actions is excellent and makes the codebase more maintainable.
components/quickbooks/actions/create-estimate/create-estimate.mjs (4)
162-166: Good validation of DetailType values!The validation correctly checks for allowed DetailType values and provides a clear error message.
5-212: Well-structured action implementation!The action follows good patterns with:
- Clear prop definitions and descriptions
- Dynamic property generation based on user input
- Proper error handling with ConfigurationError
- Informative success message
79-84:✅ Verification successful
Verify if taxCodeId should be optional.
The
taxCodeIdprop is not marked as optional, but it's not clear from the code whether it's always added to the request data or if it should be conditional.Run the following script to check how taxCodeId is used in the codebase:
🏁 Script executed:
#!/bin/bash # Search for taxCodeId usage in QuickBooks actions rg -A 5 -B 5 "taxCodeId" --glob "components/quickbooks/**/*.mjs"Length of output: 8098
🌐 Web query:
Is the taxCodeId field required when creating an estimate in QuickBooks Online API?💡 Result:
In the QuickBooks Online API, the
taxCodeIdfield is not required when creating an estimate. However, if your QuickBooks Online company is set up to charge sales tax, it's advisable to include thetaxCodeIdto specify the taxability of the estimate. This field determines whether the estimate is taxable or non-taxable. By default, QuickBooks Online provides two general tax codes:
TAX(Taxable)NON(Non-Taxable)If you don't specify a
taxCodeId, QuickBooks Online will assign the default non-taxable code to all sales. (developer.intuit.com)To specify a tax code when creating an estimate, you can include the
taxCodeIdin your API request. For example:{ "Line": [ { "Amount": 100.00, "DetailType": "SalesItemLineDetail", "SalesItemLineDetail": { "ItemRef": { "value": "8", "name": "Lighting" }, "TaxCodeRef": { "value": "TAX" } } } ], "TxnTaxDetail": { "TxnTaxCodeRef": { "value": "3", "name": "Tucson" } }, "CustomerRef": { "value": "1" } }In this example, the
TaxCodeReffor the line item is set toTAX, indicating that the item is taxable. TheTxnTaxCodeRefin theTxnTaxDetailspecifies the overall tax code for the transaction. (developer.intuit.com)If your company is not set up to charge sales tax, you can omit the
taxCodeIdfield, and QuickBooks Online will treat the estimate as non-taxable by default.Citations:
- 1: https://developer.intuit.com/app/developer/qbdesktop/docs/api-reference/qbdesktop/estimateadd?utm_source=openai
- 2: https://developer.intuit.com/app/developer/qbo/docs/workflows/manage-sales-tax-for-us-locales?utm_source=openai
taxCodeId is already optional via propDefinition
The
taxCodeIdprop increate-estimate.mjsuses the sharedquickbooks.taxCodeIdpropDefinition, which is defined withoptional: truein the app component. The QuickBooks Online API also treatstaxCodeIdas optional when creating an estimate. No changes needed.
154-156:⚠️ Potential issueFix incorrect validation logic for line items.
The current validation logic incorrectly requires both
numLineItemsandlineItemsAsObjectsto be falsy to throw an error. This would fail valid scenarios where the user provideslineItemsAsObjectsbut notnumLineItems.Apply this diff to fix the validation:
- if ((!this.numLineItems && !this.lineItemsAsObjects) || !this.customerRefValue) { + if ((!this.lineItemsAsObjects && !this.numLineItems) || !this.customerRefValue) { throw new ConfigurationError("Must provide lineItems and customerRefValue parameters."); }Additionally, consider splitting this into two separate checks for clearer error messages:
- if ((!this.numLineItems && !this.lineItemsAsObjects) || !this.customerRefValue) { - throw new ConfigurationError("Must provide lineItems and customerRefValue parameters."); - } + if (!this.customerRefValue) { + throw new ConfigurationError("customerRefValue is required."); + } + if (!this.lineItemsAsObjects && !this.numLineItems) { + throw new ConfigurationError("Must provide either lineItems or numLineItems."); + }Likely an incorrect or invalid review comment.
components/quickbooks/actions/update-estimate/update-estimate.mjs (2)
86-91: Verify if taxCodeId should be optional.The
taxCodeIdprop is not marked as optional, and it's not being used in the update data. This seems inconsistent.Since this is an update operation and not all fields need to be provided,
taxCodeIdshould likely be optional. Verify if it's used elsewhere or should be added to the update data when provided.
160-230: Excellent update implementation!The update logic properly:
- Fetches the current estimate to obtain the required SyncToken
- Only includes fields that are explicitly provided
- Maintains data integrity by not overwriting fields with undefined values
components/quickbooks/actions/update-invoice/update-invoice.mjs (1)
200-201: Excellent boolean value handling!Using
typeofchecks for boolean props is the correct approach to distinguish betweenfalse(a valid value) andundefined(field not provided).components/quickbooks/quickbooks.app.mjs (10)
119-135: LGTM! Property definition follows existing patterns.The
estimateIdproperty definition is well-structured and consistent with the existinginvoiceIdproperty. The mapper correctly extracts the ID, document number, and customer reference for display.
136-152: LGTM! Vendor property definition is properly implemented.The
vendorproperty definition follows the same pattern as the existingcustomerproperty and correctly maps vendor data for selection UI.
556-564: LGTM! Invoice sending method is correctly implemented.The
sendInvoicemethod properly destructures theinvoiceIdparameter and constructs the correct API endpoint path for sending invoices via email.
565-574: LGTM! Invoice update method follows API conventions.The
updateInvoicemethod correctly uses the POST method with theoperation: "update"parameter, which is consistent with QuickBooks API conventions for update operations.
575-584: LGTM! Invoice void method properly implemented.The
voidInvoicemethod correctly implements the void operation using POST with theoperation: "void"parameter, following QuickBooks API patterns.
585-591: LGTM! Purchase order creation method is consistent.The
createPurchaseOrdermethod follows the same pattern as other entity creation methods in the codebase, using POST to the appropriate endpoint.
592-598: LGTM! Estimate creation method properly implemented.The
createEstimatemethod is consistent with other create methods and uses the correct endpoint for estimate creation.
599-607: LGTM! Estimate sending method mirrors invoice sending pattern.The
sendEstimatemethod correctly follows the same pattern assendInvoice, properly destructuring theestimateIdparameter and constructing the send endpoint path.
608-617: LGTM! Estimate update method follows established conventions.The
updateEstimatemethod correctly implements the update operation using the same pattern asupdateInvoicewith theoperation: "update"parameter.
618-625: LGTM! Estimate retrieval method is properly implemented.The
getEstimatemethod follows the same pattern as other get methods (likegetInvoice), correctly constructing the path with the estimate ID.
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
Outdated
Show resolved
Hide resolved
components/quickbooks/actions/create-estimate/create-estimate.mjs
Outdated
Show resolved
Hide resolved
…k - Add shared utilities, ensure optional props, enhance error handling
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
components/quickbooks/actions/update-invoice/update-invoice.mjs (3)
183-187: Improve line item validation with detailed error messages.The validation logic is good but could provide more helpful error messages for debugging.
Consider enhancing the error message to include which line item failed validation:
lines.forEach((line, index) => { if (line.DetailType !== "SalesItemLineDetail" && line.DetailType !== "GroupLineDetail" && line.DetailType !== "DescriptionOnly") { - throw new ConfigurationError("Line Item DetailType must be `SalesItemLineDetail`, `GroupLineDetail`, or `DescriptionOnly`"); + throw new ConfigurationError(`Line Item at index ${index + 1} has invalid DetailType '${line.DetailType}'. Must be 'SalesItemLineDetail', 'GroupLineDetail', or 'DescriptionOnly'`); } });
192-218: Consider consolidating conditional assignments.The multiple conditional assignments could be more maintainable with a helper function or object mapping approach.
Consider refactoring for better maintainability:
+// Helper function to conditionally add properties +const addIfDefined = (target, source, mappings) => { + Object.entries(mappings).forEach(([sourceKey, targetConfig]) => { + const value = source[sourceKey]; + if (value !== undefined && value !== null) { + if (typeof targetConfig === 'string') { + target[targetConfig] = value; + } else if (typeof targetConfig === 'object') { + target[targetConfig.key] = targetConfig.transform ? targetConfig.transform(value) : value; + } + } + }); +}; -if (this.dueDate) data.DueDate = this.dueDate; -if (typeof this.allowOnlineCreditCardPayment === "boolean") data.AllowOnlineCreditCardPayment = this.allowOnlineCreditCardPayment; -if (typeof this.allowOnlineACHPayment === "boolean") data.AllowOnlineACHPayment = this.allowOnlineACHPayment; -if (this.docNumber) data.DocNumber = this.docNumber; -if (this.billAddr) data.BillAddr = this.billAddr; -if (this.shipAddr) data.ShipAddr = this.shipAddr; -if (this.trackingNum) data.TrackingNum = this.trackingNum; -if (this.privateNote) data.PrivateNote = this.privateNote; + +// Add simple field mappings +addIfDefined(data, this, { + dueDate: 'DueDate', + docNumber: 'DocNumber', + billAddr: 'BillAddr', + shipAddr: 'ShipAddr', + trackingNum: 'TrackingNum', + privateNote: 'PrivateNote', +}); + +// Handle boolean fields explicitly +if (typeof this.allowOnlineCreditCardPayment === "boolean") { + data.AllowOnlineCreditCardPayment = this.allowOnlineCreditCardPayment; +} +if (typeof this.allowOnlineACHPayment === "boolean") { + data.AllowOnlineACHPayment = this.allowOnlineACHPayment; +}
224-226: Improve response validation.The response validation could be more robust to handle edge cases.
Add better response validation:
-if (response) { - $.export("summary", `Successfully updated invoice with ID ${response.Invoice.Id}`); -} +if (response?.Invoice?.Id) { + $.export("summary", `Successfully updated invoice with ID ${response.Invoice.Id}`); +} else { + throw new ConfigurationError("Failed to update invoice: Invalid response from QuickBooks API"); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/quickbooks/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs(1 hunks)components/quickbooks/actions/void-invoice/void-invoice.mjs(1 hunks)components/quickbooks/common/utils.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
- components/quickbooks/actions/void-invoice/void-invoice.mjs
- components/quickbooks/actions/update-estimate/update-estimate.mjs
- components/quickbooks/actions/create-estimate/create-estimate.mjs
🔇 Additional comments (1)
components/quickbooks/actions/update-invoice/update-invoice.mjs (1)
53-59: Previous feedback has been addressed.The
currencyRefValueprop is now correctly marked as optional, addressing the previous review comment.
components/quickbooks/actions/update-invoice/update-invoice.mjs
Outdated
Show resolved
Hide resolved
… - Add comprehensive validation to buildSalesLineItems and buildPurchaseLineItems - Refactor conditional logic for better readability - Add error handling in update-invoice after getInvoice call
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/quickbooks/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs(1 hunks)components/quickbooks/common/utils.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/quickbooks/actions/create-estimate/create-estimate.mjs
- components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
- components/quickbooks/actions/update-invoice/update-invoice.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/quickbooks/common/utils.mjs
[error] 62-62: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 65-65: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 117-117: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 120-120: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (4)
components/quickbooks/actions/update-estimate/update-estimate.mjs (4)
14-104: Well-structured props definition.The props are well-defined with appropriate optional flags and good reuse of propDefinitions from the main QuickBooks app. The past review comment about
currencyRefValuebeing optional has been properly addressed.
105-147: Excellent dynamic props implementation.The conditional property loading pattern is well-implemented with clean logic flow and proper async options for item selection. The early return pattern and consistent naming for dynamic props makes this very readable.
148-165: Clean helper methods with good abstraction.The
buildLineItemsdelegation andaddIfDefinedhelper provide clean abstractions that reduce code repetition. The mapping object pattern inaddIfDefinedis particularly elegant for conditional property assignment.
166-240: Solid implementation of estimate update logic.The implementation properly handles QuickBooks concurrency control with SyncToken retrieval, has good validation of line item DetailType values, and includes proper error handling. The conditional building of the update payload and use of helper methods keeps the code clean and maintainable.
components/quickbooks/actions/create-estimate/create-estimate.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs (2)
125-128: Consider making quantity conditional for ItemBasedExpenseLineDetail only.The quantity field is always shown for all line items, but it's primarily relevant for ItemBasedExpenseLineDetail. AccountBasedExpenseLineDetail typically doesn't use quantity since it's account-based rather than item-based.
Make quantity conditional:
+ if (this[`detailType_${i}`] === "ItemBasedExpenseLineDetail" || !this[`detailType_${i}`]) { props[`quantity_${i}`] = { type: "string", label: `Line ${i} - Quantity`, }; + }
17-25: Clarify the purpose of accountId prop.The accountId prop uses the
accountIdspropDefinition but overrides the type to string and has a description that might be unclear. This appears to be for the APAccountRef in the purchase order, but the relationship isn't immediately clear.Consider renaming or improving the description for clarity:
accountId: { propDefinition: [ quickbooks, "accountIds", ], type: "string", - label: "Account ID", - description: "The ID of the account to use for the purchase order", + label: "AP Account ID", + description: "The accounts payable account ID to use for the purchase order", },components/quickbooks/actions/update-estimate/update-estimate.mjs (4)
148-170: Add JSDoc documentation for the helper method.The
addIfDefinedhelper method is well-implemented but should include JSDoc documentation for clarity and maintainability as mentioned in the Pipedream guidelines.+ /** + * Conditionally adds properties from source to target object based on mappings + * @param {Object} target - The target object to modify + * @param {Object} source - The source object containing values + * @param {Object} mappings - Mapping configuration for property transformation + */ addIfDefined(target, source, mappings) {
181-183: Optimize address parsing to avoid unnecessary operations.The
parseObjectcalls forbillAddrandshipAddrare executed even when the values are undefined, which is unnecessary.const data = { Id: this.estimateId, SyncToken: estimate.SyncToken, - BillAddr: parseObject(this.billAddr), - ShipAddr: parseObject(this.shipAddr), }; + if (this.billAddr) { + data.BillAddr = parseObject(this.billAddr); + } + if (this.shipAddr) { + data.ShipAddr = parseObject(this.shipAddr); + }
197-201: Extract line validation to a helper method for reusability.The line item validation logic could be extracted to a reusable helper method to improve code organization and potential reuse across other actions.
+ validateLineItemDetailTypes(lines) { + const validDetailTypes = new Set(['SalesItemLineDetail', 'GroupLineDetail', 'DescriptionOnly']); + lines.forEach((line, index) => { + if (!validDetailTypes.has(line.DetailType)) { + throw new ConfigurationError(`Line Item at index ${index + 1} has invalid DetailType '${line.DetailType}'. Must be 'SalesItemLineDetail', 'GroupLineDetail', or 'DescriptionOnly'`); + } + }); + } - lines.forEach((line, index) => { - if (line.DetailType !== "SalesItemLineDetail" && line.DetailType !== "GroupLineDetail" && line.DetailType !== "DescriptionOnly") { - throw new ConfigurationError(`Line Item at index ${index + 1} has invalid DetailType '${line.DetailType}'. Must be 'SalesItemLineDetail', 'GroupLineDetail', or 'DescriptionOnly'`); - } - }); + this.validateLineItemDetailTypes(lines);
238-242: Improve error handling specificity.The error handling could be more specific about what constitutes a successful response from the QuickBooks API.
- if (response?.Estimate?.Id) { + if (response?.QueryResponse?.Estimate?.[0]?.Id || response?.Estimate?.Id) { $.export("$summary", `Successfully updated estimate with ID ${response.Estimate?.Id || response.QueryResponse.Estimate[0].Id}`); } else { throw new ConfigurationError("Failed to update estimate: Invalid response from QuickBooks API"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (49)
components/quickbooks/actions/create-ap-aging-report/create-ap-aging-report.mjs(1 hunks)components/quickbooks/actions/create-bill/create-bill.mjs(1 hunks)components/quickbooks/actions/create-customer/create-customer.mjs(1 hunks)components/quickbooks/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks/actions/create-invoice/create-invoice.mjs(1 hunks)components/quickbooks/actions/create-payment/create-payment.mjs(1 hunks)components/quickbooks/actions/create-pl-report/create-pl-report.mjs(1 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks/actions/create-purchase/create-purchase.mjs(1 hunks)components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs(1 hunks)components/quickbooks/actions/delete-purchase/delete-purchase.mjs(1 hunks)components/quickbooks/actions/get-bill/get-bill.mjs(1 hunks)components/quickbooks/actions/get-customer/get-customer.mjs(1 hunks)components/quickbooks/actions/get-invoice/get-invoice.mjs(1 hunks)components/quickbooks/actions/get-my-company/get-my-company.mjs(1 hunks)components/quickbooks/actions/get-payment/get-payment.mjs(1 hunks)components/quickbooks/actions/get-purchase-order/get-purchase-order.mjs(1 hunks)components/quickbooks/actions/get-purchase/get-purchase.mjs(1 hunks)components/quickbooks/actions/get-sales-receipt/get-sales-receipt.mjs(1 hunks)components/quickbooks/actions/get-time-activity/get-time-activity.mjs(1 hunks)components/quickbooks/actions/search-accounts/search-accounts.mjs(1 hunks)components/quickbooks/actions/search-customers/search-customers.mjs(1 hunks)components/quickbooks/actions/search-invoices/search-invoices.mjs(1 hunks)components/quickbooks/actions/search-items/search-items.mjs(1 hunks)components/quickbooks/actions/search-products/search-products.mjs(1 hunks)components/quickbooks/actions/search-purchases/search-purchases.mjs(1 hunks)components/quickbooks/actions/search-query/search-query.mjs(1 hunks)components/quickbooks/actions/search-services/search-services.mjs(1 hunks)components/quickbooks/actions/search-time-activities/search-time-activities.mjs(1 hunks)components/quickbooks/actions/search-vendors/search-vendors.mjs(1 hunks)components/quickbooks/actions/send-estimate/send-estimate.mjs(1 hunks)components/quickbooks/actions/send-invoice/send-invoice.mjs(1 hunks)components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs(1 hunks)components/quickbooks/actions/update-customer/update-customer.mjs(1 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs(1 hunks)components/quickbooks/actions/update-item/update-item.mjs(1 hunks)components/quickbooks/actions/void-invoice/void-invoice.mjs(1 hunks)components/quickbooks/common/utils.mjs(2 hunks)components/quickbooks/package.json(1 hunks)components/quickbooks/quickbooks.app.mjs(3 hunks)components/quickbooks/sources/new-customer-created/new-customer-created.mjs(1 hunks)components/quickbooks/sources/new-customer-updated/new-customer-updated.mjs(1 hunks)components/quickbooks/sources/new-employee-created/new-employee-created.mjs(1 hunks)components/quickbooks/sources/new-employee-updated/new-employee-updated.mjs(1 hunks)components/quickbooks/sources/new-invoice-created/new-invoice-created.mjs(1 hunks)components/quickbooks/sources/new-invoice-updated/new-invoice-updated.mjs(1 hunks)components/quickbooks/sources/new-item-created/new-item-created.mjs(1 hunks)components/quickbooks/sources/new-item-updated/new-item-updated.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (40)
- components/quickbooks/actions/create-purchase/create-purchase.mjs
- components/quickbooks/actions/search-products/search-products.mjs
- components/quickbooks/actions/create-ap-aging-report/create-ap-aging-report.mjs
- components/quickbooks/sources/new-employee-created/new-employee-created.mjs
- components/quickbooks/actions/get-sales-receipt/get-sales-receipt.mjs
- components/quickbooks/actions/search-purchases/search-purchases.mjs
- components/quickbooks/actions/get-bill/get-bill.mjs
- components/quickbooks/actions/update-item/update-item.mjs
- components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs
- components/quickbooks/actions/search-accounts/search-accounts.mjs
- components/quickbooks/actions/get-my-company/get-my-company.mjs
- components/quickbooks/actions/search-vendors/search-vendors.mjs
- components/quickbooks/package.json
- components/quickbooks/sources/new-item-created/new-item-created.mjs
- components/quickbooks/sources/new-item-updated/new-item-updated.mjs
- components/quickbooks/actions/delete-purchase/delete-purchase.mjs
- components/quickbooks/actions/create-invoice/create-invoice.mjs
- components/quickbooks/actions/get-customer/get-customer.mjs
- components/quickbooks/actions/create-bill/create-bill.mjs
- components/quickbooks/sources/new-customer-created/new-customer-created.mjs
- components/quickbooks/actions/search-services/search-services.mjs
- components/quickbooks/actions/search-customers/search-customers.mjs
- components/quickbooks/actions/create-payment/create-payment.mjs
- components/quickbooks/actions/get-time-activity/get-time-activity.mjs
- components/quickbooks/actions/search-items/search-items.mjs
- components/quickbooks/actions/search-invoices/search-invoices.mjs
- components/quickbooks/actions/get-payment/get-payment.mjs
- components/quickbooks/actions/get-purchase/get-purchase.mjs
- components/quickbooks/actions/create-pl-report/create-pl-report.mjs
- components/quickbooks/sources/new-customer-updated/new-customer-updated.mjs
- components/quickbooks/actions/search-query/search-query.mjs
- components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs
- components/quickbooks/sources/new-employee-updated/new-employee-updated.mjs
- components/quickbooks/actions/search-time-activities/search-time-activities.mjs
- components/quickbooks/actions/create-customer/create-customer.mjs
- components/quickbooks/sources/new-invoice-created/new-invoice-created.mjs
- components/quickbooks/sources/new-invoice-updated/new-invoice-updated.mjs
- components/quickbooks/actions/update-customer/update-customer.mjs
- components/quickbooks/actions/get-invoice/get-invoice.mjs
- components/quickbooks/actions/get-purchase-order/get-purchase-order.mjs
🚧 Files skipped from review as they are similar to previous changes (7)
- components/quickbooks/actions/send-estimate/send-estimate.mjs
- components/quickbooks/actions/send-invoice/send-invoice.mjs
- components/quickbooks/actions/create-estimate/create-estimate.mjs
- components/quickbooks/actions/void-invoice/void-invoice.mjs
- components/quickbooks/common/utils.mjs
- components/quickbooks/actions/update-invoice/update-invoice.mjs
- components/quickbooks/quickbooks.app.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (4)
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs (2)
1-7: LGTM: Clean imports and utility usage.The imports are well-organized and appropriately utilize the common utilities for parsing and building line items, promoting code reuse.
137-191: LGTM: Solid validation and API interaction logic.The run method has comprehensive validation for required fields, proper error handling, and follows the expected QuickBooks API patterns. The conditional currency handling and response processing are well-implemented.
components/quickbooks/actions/update-estimate/update-estimate.mjs (2)
1-14: LGTM! Well-structured module definition.The imports, module key, and documentation follow Pipedream best practices correctly.
105-147: Excellent dynamic prop generation logic.The conditional prop generation correctly handles both array-based and individual line item inputs, with proper async options for item selection.
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
Show resolved
Hide resolved
components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
Show resolved
Hide resolved
components/quickbooks/actions/update-estimate/update-estimate.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/quickbooks/actions/update-estimate/update-estimate.mjs (1)
90-96: Remove unusedtaxCodeIdprop.The
taxCodeIdproperty is defined but never used in therunmethod. This creates unnecessary user input that has no effect on the API call.Apply this diff to remove the unused prop:
- taxCodeId: { - propDefinition: [ - quickbooks, - "taxCodeId", - ], - optional: true, - },
🧹 Nitpick comments (1)
components/quickbooks/actions/update-estimate/update-estimate.mjs (1)
197-201: Improve DetailType validation error message.The validation logic is correct, but the error message could be more informative by listing the supported types more clearly.
Consider this improvement:
- if (line.DetailType !== "SalesItemLineDetail" && line.DetailType !== "GroupLineDetail" && line.DetailType !== "DescriptionOnly") { - throw new ConfigurationError(`Line Item at index ${index + 1} has invalid DetailType '${line.DetailType}'. Must be 'SalesItemLineDetail', 'GroupLineDetail', or 'DescriptionOnly'`); - } + const validDetailTypes = ["SalesItemLineDetail", "GroupLineDetail", "DescriptionOnly"]; + if (!validDetailTypes.includes(line.DetailType)) { + throw new ConfigurationError(`Line Item at index ${index + 1} has invalid DetailType '${line.DetailType}'. Must be one of: ${validDetailTypes.join(", ")}`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/quickbooks/actions/create-bill/create-bill.mjs(2 hunks)components/quickbooks/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks/actions/create-invoice/create-invoice.mjs(2 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks/actions/create-purchase/create-purchase.mjs(2 hunks)components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs(2 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- components/quickbooks/actions/create-sales-receipt/create-sales-receipt.mjs
- components/quickbooks/actions/create-purchase/create-purchase.mjs
- components/quickbooks/actions/create-invoice/create-invoice.mjs
- components/quickbooks/actions/create-bill/create-bill.mjs
- components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
- components/quickbooks/actions/create-estimate/create-estimate.mjs
- components/quickbooks/actions/update-invoice/update-invoice.mjs
🔇 Additional comments (3)
components/quickbooks/actions/update-estimate/update-estimate.mjs (3)
172-176: Excellent use of SyncToken for concurrency control.Retrieving the current estimate to obtain the SyncToken before updating is a best practice that prevents concurrent modification issues in QuickBooks API.
152-169: Well-designed helper method.The
addIfDefinedhelper method provides a clean, reusable pattern for conditionally adding properties to the request data while supporting both simple and complex transformations.
238-242: Robust error handling for API response.The response validation properly checks for the expected structure and provides a meaningful error message if the update fails.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/quickbooks/actions/update-estimate/update-estimate.mjs (2)
22-27: MarkcustomerRefValueas optional.The
customerRefValueprop should be marked as optional since it's only conditionally added to the request data (lines 276-280).Apply this diff:
customerRefValue: { propDefinition: [ quickbooks, "customer", ], + optional: true, },
149-155: Remove unusedtaxCodeIdprop.The
taxCodeIdproperty is defined in the props but isn't used in therunmethod. It's not included in theaddIfDefinedmapping (lines 297-303) or handled elsewhere.Apply this diff to remove the unused prop:
- taxCodeId: { - propDefinition: [ - quickbooks, - "taxCodeId", - ], - optional: true, - },
🧹 Nitpick comments (2)
components/quickbooks/actions/update-invoice/update-invoice.mjs (2)
22-27: MarkcustomerRefValueas optional.The
customerRefValueprop should be marked as optional since it's only conditionally added to the request data (lines 297-301).Apply this diff:
customerRefValue: { propDefinition: [ quickbooks, "customer", ], + optional: true, },
155-161: Remove unusedtaxCodeIdprop.The
taxCodeIdproperty is defined in the props but isn't used in therunmethod. It's not included in theaddIfDefinedmapping (lines 317-322) or handled elsewhere.Apply this diff to remove the unused prop:
- taxCodeId: { - propDefinition: [ - quickbooks, - "taxCodeId", - ], - optional: true, - },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/quickbooks/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks/actions/update-invoice/update-invoice.mjs(1 hunks)components/quickbooks/quickbooks.app.mjs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/quickbooks/actions/create-estimate/create-estimate.mjs
- components/quickbooks/actions/create-purchase-order/create-purchase-order.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (12)
components/quickbooks/actions/update-estimate/update-estimate.mjs (4)
164-205: Well-implemented dynamic props generation.The
additionalPropsmethod properly handles both object-based and individual line item input modes, with appropriate async options for item selection.
207-228: Helper methods are well-designed.The
buildLineItemsandaddIfDefinedmethods provide good abstraction and reusability for line item construction and conditional field mapping.
230-295: Solid implementation of estimate update logic.The method correctly retrieves the current estimate for SyncToken, conditionally constructs address objects, and validates line item DetailTypes against allowed values. The error handling for invalid DetailTypes is appropriate.
296-335: Clean field mapping and proper response handling.The conditional field mapping and API response validation are well-implemented. The success message provides clear feedback to users.
components/quickbooks/actions/update-invoice/update-invoice.mjs (4)
235-256: Excellent error handling improvements.The comprehensive validation for invoice existence, ID, and SyncToken properly addresses potential API response issues with clear error messages.
170-233: Consistent dynamic props and helper methods.The
additionalPropsmethod and helper functions follow the same well-designed pattern as the update-estimate action, ensuring consistency across the QuickBooks integration.
258-315: Well-structured invoice update logic.The conditional address construction, line item validation, and data object building follow the same robust pattern as the estimate action.
317-361: Proper invoice-specific field handling.The field mapping correctly handles invoice-specific properties like payment options, due date, and tracking number. The explicit boolean checks for payment options ensure proper API data formatting.
components/quickbooks/quickbooks.app.mjs (4)
119-154: Well-designed new prop definitions.The
estimateIdandvendorprops follow the established pattern with proper async options and clear labels. The estimate mapping logic correctly handles optional DocNumber values.
476-547: Comprehensive address field definitions.The billing and shipping address props provide complete coverage including geolocation data, with clear labels and descriptions. All fields are appropriately marked as optional.
630-658: Properly implemented invoice operation methods.The
sendInvoice,updateInvoice, andvoidInvoicemethods correctly follow the established_makeRequestpattern with appropriate HTTP methods and operation parameters.
659-699: Consistent estimate and purchase order methods.The new methods for estimate and purchase order operations properly follow the established patterns with correct API paths and parameter handling. The
getEstimatemethod appropriately uses GET while create/update operations use POST.
This PR addresses GitHub issue #16906 by implementing missing QuickBooks Online API endpoints that are available in Zapier but were missing from Pipedream. This brings the Pipedream QuickBooks integration to feature parity with Zapier for invoice, estimate, and purchase order operations.
🔧 New API Methods Added to
quickbooks.app.mjs:sendInvoice()- Send invoice via emailupdateInvoice()- Update existing invoicevoidInvoice()- Void an invoicecreatePurchaseOrder()- Create purchase orderscreateEstimate()- Create estimates/quotessendEstimate()- Send estimate via emailupdateEstimate()- Update existing estimategetEstimate()- Retrieve estimate by ID📋 New Prop Definitions:
estimateId- For selecting estimates with customer contextvendor- For selecting vendors for purchase ordersFixes #16906
Summary by CodeRabbit