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/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 components/quickbooks/actions/update-invoice/update-invoice.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.
… - 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.
| export function buildPurchaseLineItems(numLineItems, context) { | ||
| // Validate numLineItems parameter | ||
| if (typeof numLineItems !== "number" || !Number.isInteger(numLineItems) || numLineItems <= 0) { | ||
| throw new ConfigurationError("numLineItems must be a positive integer"); | ||
| } | ||
|
|
||
| // Validate context parameter | ||
| if (!context || typeof context !== "object" || Array.isArray(context)) { | ||
| throw new ConfigurationError("context must be an object"); | ||
| } | ||
|
|
||
| // Validate required keys exist for each line item | ||
| const missingKeys = []; | ||
| for (let i = 1; i <= numLineItems; i++) { | ||
| if (!context.hasOwnProperty(`amount_${i}`)) { | ||
| missingKeys.push(`amount_${i}`); | ||
| } | ||
| if (!context.hasOwnProperty(`item_${i}`)) { | ||
| missingKeys.push(`item_${i}`); | ||
| } | ||
| } | ||
|
|
||
| if (missingKeys.length > 0) { | ||
| throw new ConfigurationError(`Missing required keys in context: ${missingKeys.join(", ")}`); | ||
| } | ||
|
|
||
| // Validate amount values are valid numbers | ||
| const invalidAmounts = []; | ||
| for (let i = 1; i <= numLineItems; i++) { | ||
| const amount = context[`amount_${i}`]; | ||
| if (amount !== undefined && amount !== null && amount !== "" && | ||
| (typeof amount !== "number" && (typeof amount !== "string" || isNaN(parseFloat(amount))))) { | ||
| invalidAmounts.push(`amount_${i}`); | ||
| } | ||
| } | ||
|
|
||
| if (invalidAmounts.length > 0) { | ||
| throw new ConfigurationError(`Invalid amount values for: ${invalidAmounts.join(", ")}. Amounts must be valid numbers.`); | ||
| } | ||
|
|
||
| // Validate detailType values if provided | ||
| const validDetailTypes = ["ItemBasedExpenseLineDetail", "AccountBasedExpenseLineDetail"]; | ||
| const invalidDetailTypes = []; | ||
| for (let i = 1; i <= numLineItems; i++) { | ||
| const detailType = context[`detailType_${i}`]; | ||
| if (detailType && !validDetailTypes.includes(detailType)) { | ||
| invalidDetailTypes.push(`detailType_${i}: ${detailType}`); | ||
| } | ||
| } | ||
|
|
||
| if (invalidDetailTypes.length > 0) { | ||
| throw new ConfigurationError(`Invalid detailType values for: ${invalidDetailTypes.join(", ")}. Valid types are: ${validDetailTypes.join(", ")}`); | ||
| } | ||
|
|
||
| const lineItems = []; | ||
| for (let i = 1; i <= numLineItems; i++) { | ||
| const detailType = context[`detailType_${i}`] || "ItemBasedExpenseLineDetail"; | ||
|
|
||
| // Extract conditional logic into clear variables | ||
| const isItemBased = detailType === "ItemBasedExpenseLineDetail"; | ||
| const detailPropertyName = isItemBased ? "ItemBasedExpenseLineDetail" : "AccountBasedExpenseLineDetail"; | ||
| const refPropertyName = isItemBased ? "ItemRef" : "AccountRef"; | ||
|
|
||
| // Build line item with clearer structure | ||
| const lineItem = { | ||
| DetailType: detailType, | ||
| Amount: context[`amount_${i}`], | ||
| [detailPropertyName]: { | ||
| [refPropertyName]: { | ||
| value: context[`item_${i}`], | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| lineItems.push(lineItem); | ||
| } | ||
| return lineItems; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hasOwnProperty with Object.hasOwn() and excellent refactoring work!
The conditional logic refactoring with clear variable names is excellent - much more readable than nested conditionals. However, the same hasOwnProperty security issue needs addressing.
Apply this diff:
for (let i = 1; i <= numLineItems; i++) {
- if (!context.hasOwnProperty(`amount_${i}`)) {
+ if (!Object.hasOwn(context, `amount_${i}`)) {
missingKeys.push(`amount_${i}`);
}
- if (!context.hasOwnProperty(`item_${i}`)) {
+ if (!Object.hasOwn(context, `item_${i}`)) {
missingKeys.push(`item_${i}`);
}
}The refactoring from lines 162-164 with isItemBased, detailPropertyName, and refPropertyName variables significantly improves code readability compared to inline conditionals.
📝 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.
| export function buildPurchaseLineItems(numLineItems, context) { | |
| // Validate numLineItems parameter | |
| if (typeof numLineItems !== "number" || !Number.isInteger(numLineItems) || numLineItems <= 0) { | |
| throw new ConfigurationError("numLineItems must be a positive integer"); | |
| } | |
| // Validate context parameter | |
| if (!context || typeof context !== "object" || Array.isArray(context)) { | |
| throw new ConfigurationError("context must be an object"); | |
| } | |
| // Validate required keys exist for each line item | |
| const missingKeys = []; | |
| for (let i = 1; i <= numLineItems; i++) { | |
| if (!context.hasOwnProperty(`amount_${i}`)) { | |
| missingKeys.push(`amount_${i}`); | |
| } | |
| if (!context.hasOwnProperty(`item_${i}`)) { | |
| missingKeys.push(`item_${i}`); | |
| } | |
| } | |
| if (missingKeys.length > 0) { | |
| throw new ConfigurationError(`Missing required keys in context: ${missingKeys.join(", ")}`); | |
| } | |
| // Validate amount values are valid numbers | |
| const invalidAmounts = []; | |
| for (let i = 1; i <= numLineItems; i++) { | |
| const amount = context[`amount_${i}`]; | |
| if (amount !== undefined && amount !== null && amount !== "" && | |
| (typeof amount !== "number" && (typeof amount !== "string" || isNaN(parseFloat(amount))))) { | |
| invalidAmounts.push(`amount_${i}`); | |
| } | |
| } | |
| if (invalidAmounts.length > 0) { | |
| throw new ConfigurationError(`Invalid amount values for: ${invalidAmounts.join(", ")}. Amounts must be valid numbers.`); | |
| } | |
| // Validate detailType values if provided | |
| const validDetailTypes = ["ItemBasedExpenseLineDetail", "AccountBasedExpenseLineDetail"]; | |
| const invalidDetailTypes = []; | |
| for (let i = 1; i <= numLineItems; i++) { | |
| const detailType = context[`detailType_${i}`]; | |
| if (detailType && !validDetailTypes.includes(detailType)) { | |
| invalidDetailTypes.push(`detailType_${i}: ${detailType}`); | |
| } | |
| } | |
| if (invalidDetailTypes.length > 0) { | |
| throw new ConfigurationError(`Invalid detailType values for: ${invalidDetailTypes.join(", ")}. Valid types are: ${validDetailTypes.join(", ")}`); | |
| } | |
| const lineItems = []; | |
| for (let i = 1; i <= numLineItems; i++) { | |
| const detailType = context[`detailType_${i}`] || "ItemBasedExpenseLineDetail"; | |
| // Extract conditional logic into clear variables | |
| const isItemBased = detailType === "ItemBasedExpenseLineDetail"; | |
| const detailPropertyName = isItemBased ? "ItemBasedExpenseLineDetail" : "AccountBasedExpenseLineDetail"; | |
| const refPropertyName = isItemBased ? "ItemRef" : "AccountRef"; | |
| // Build line item with clearer structure | |
| const lineItem = { | |
| DetailType: detailType, | |
| Amount: context[`amount_${i}`], | |
| [detailPropertyName]: { | |
| [refPropertyName]: { | |
| value: context[`item_${i}`], | |
| }, | |
| }, | |
| }; | |
| lineItems.push(lineItem); | |
| } | |
| return lineItems; | |
| } | |
| // Validate required keys exist for each line item | |
| const missingKeys = []; | |
| for (let i = 1; i <= numLineItems; i++) { | |
| if (!Object.hasOwn(context, `amount_${i}`)) { | |
| missingKeys.push(`amount_${i}`); | |
| } | |
| if (!Object.hasOwn(context, `item_${i}`)) { | |
| missingKeys.push(`item_${i}`); | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[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)
🤖 Prompt for AI Agents
In components/quickbooks/common/utils.mjs between lines 103 and 180, replace all
uses of hasOwnProperty with Object.has() for safer property existence checks.
Specifically, update the validation loops checking for required keys in the
context object to use Object.has(context, key) instead of
context.hasOwnProperty(key). Keep the existing refactoring with clear variable
names for conditional logic as it improves readability.
| props.lineItems = { | ||
| type: "string[]", | ||
| label: "Line Items", | ||
| description: "Line items of an estimate. Set DetailType to `SalesItemLineDetail`, `GroupLineDetail`, or `DescriptionOnly`. Example: `{ \"DetailType\": \"SalesItemLineDetail\", \"Amount\": 100.0, \"SalesItemLineDetail\": { \"ItemRef\": { \"name\": \"Services\", \"value\": \"1\" } } }`", |
There was a problem hiding this comment.
We should link to the relevant documentation here if possible (as well as on the other instances of this prop in other actions - ideally we would import this object from a common file)
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