QuickBooks Webhook Sources #1564
Conversation
|
@js07 Thank you again for reviewing. I made a few changes in addition to the ones you suggested:
|
| const relevantEntities = this.getEntities(); | ||
| const relevantOperations = this.getOperations(); | ||
|
|
||
| if (!relevantEntities.includes(name)) { |
There was a problem hiding this comment.
I think all events will be skipped when the webhookNames prop is left blank, rather than included.
The "isEntityRelevant" check was changed from:
To emit events from all entities when webhookNames is left blank, I'd suggest to either:
- Change the
ifstatement to:
if (relevantEntities?.length > 0 && !relevantEntities.includes(name)) {- Or change the
getEntities()method incustom-webhook-events.jsto:
getEntities() {
return this.entitiesToEmit ?? WEBHOOK_ENTITIES;
},| console.log("Skipping event from unrecognized source."); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
I completely removed the sendHttpResponse() method from common.js since I realized I only needed it for testing purposes.
I don't think entities is required in the response body, but Quickbooks expects a response from webhook endpoints.
According to the Quickbooks webhooks docs, webhooks are required to respond to requests. Otherwise, Quickbooks will retry the request several times and blacklist the endpoint if it's still "inactive".
Example 200 response from Quickbooks' sample app: return res.status(200).send('SUCCESS');
| this.http.respond({ | |
| status: 200, | |
| }); | |
| id: { | ||
| type: "string", | ||
| label: "Record ID", | ||
| description: null, |
There was a problem hiding this comment.
To help users find an entity's Record ID, I'd recommend adding a description that explains where to find the ID or links to relevant QuickBooks API docs.
Alternatively/additionally, use async options (with label/value definitions) so users can make selections from a drop down menu.
For example:
idprop indownload-pdf.js
id: {
type: "string",
label: "Record ID",
description: null,
async options({ page }) {
const limit = 100;
const { entity } = this;
if (!entity) {
return [];
}
let response;
try {
response = await this.quickbooks.queryEntity(this, entity, {
startPosition: 1 + (page) * limit,
maxResults: limit,
});
} catch (err) {
return [];
}
if (!response[entity]) {
return [];
}
return response[entity].map((e) => ({
label: this.quickbooks.getOptionLabel(e, entity),
value: e.Id,
}));
},
},- New
methodsinquickbooks.app.js
getOptionLabel(entity, entityName) {
switch (entityName) {
case "CreditMemo":
return entity.DocNumber
? `# ${entity.DocNumber}`
: entity.Id;
case "Estimate":
return `${entity.totalAmount} (# ${entity.DocNumber})`;
case "Invoice":
return `Invoice # ${entity.DocNumber}`;
case "Payment":
return `${entity.CustomerRef?.name ?? ""} (${entity.Id})`;
case "PurchaseOrder":
return `${entity.VendorRef?.name ?? ""} (# ${entity.DocNumber})`;
case "RefundReceipt":
return entity.DocNumber
? `# ${entity.DocNumber}`
: entity.Id;
case "SalesReceipt":
return entity.DocNumber
? `# ${entity.DocNumber}`
: entity.Id;
default:
return entity.Id;
}
},
async queryEntity($, entityName, config = {}) {
const {
startPosition = 1,
maxResults = 100,
} = config;
if (!WEBHOOK_ENTITIES.includes(entityName)) {
throw new Error(`"${entityName}" is not a valid QuickBooks entity`);
}
// Query operations docs:
// https://developer.intuit.com/app/developer/qbo/docs/learn/explore-the-quickbooks-online-api/data-queries
const query = `
SELECT * FROM ${entityName}
ORDER BY Metadata.LastUpdatedTime DESC
STARTPOSITION ${startPosition} MAXRESULTS ${maxResults}`;
const companyId = this.companyId();
return (await this._makeRequest($, {
path: `company/${companyId}/query`,
params: {
query,
},
})).QueryResponse;
},
|
@js07 can you tell how much work is left in order to wrap this up? |
Looks like there are just a few changes left. |
|
I noticed that some apps like Slack and Zoom use an The current sources in this PR require users to manually create a new App in the Intuit Developer Dashboard for each new event source. Could an app hook be used instead to simplify creating QuickBooks webhook sources? |
dylburger
added
the
triaged
|
Hello @celestebancos @js07, since this PR has been hanging for quite a while, I suggest we move this back to TODO and have our component dev to take over in a new PR. What do you think? |
@vunguyenhung Moving this back to TODO sounds good. Supporting webhook sources for QuickBooks via a Pipedream app (#1564 (comment)) requires apphook (internal) changes, so we may want to create a separate issue for QuickBooks Webhook Sources and mark it as blocked. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughA new set of modules has been introduced to enhance the integration with QuickBooks, focusing on downloading PDF documents for various entities and handling webhook events. Key functionalities include a method for downloading PDFs, constants for webhook entities and operations, and modules for processing webhook events related to customer changes. This update provides structured interactions with the QuickBooks API, facilitating the emission of events and retrieval of documents. Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
@celestebancos is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (9)
components/quickbooks/sources/custom-webhook-events/custom-webhook-events.js (1)
4-11: Consider addressing the eslint-disable comment.The module export and its properties are well-structured and follow Pipedream conventions. However, there's an eslint-disable comment on the
nameproperty line. This might indicate that the line is too long according to your linting rules.Consider breaking the
nameproperty into multiple lines to avoid the need for the eslint-disable comment. For example:name: "Custom Webhook Events: Created, Updated, Merged, Deleted, " + "Voided or Emailed (Instant)",This approach maintains readability while adhering to linting rules.
components/quickbooks/sources/new-or-modified-customer/new-or-modified-customer.js (2)
28-38: LGTM: Methods are well-defined, with a minor suggestion.The methods are correctly implemented and provide the necessary functionality. However, there's a small optimization opportunity:
For
getEntities, consider returning thesourceEntitydirectly instead of wrapping it in an array, unless multiple entities are expected in the future:getEntities() { - return [ - sourceEntity, - ]; + return sourceEntity; },This change simplifies the code slightly while maintaining the same functionality.
1-39: Overall, excellent implementation with a suggestion for improved documentation.The QuickBooks customer event source is well-implemented, utilizing common patterns and reusable components. It follows best practices and provides a concise, flexible solution for emitting customer events.
To enhance maintainability and clarity, consider adding a brief comment at the beginning of the file explaining the purpose of this source and how it fits into the larger QuickBooks integration. For example:
/** * QuickBooks Customer Event Source * * This source emits events for customer-related operations in QuickBooks, * including creation, updates, merges, and deletions. It allows users to * configure which specific operations they want to receive events for. */This addition would provide valuable context for developers who may work on this file in the future.
components/quickbooks/actions/download-pdf/download-pdf.js (1)
12-39: LGTM: Props are well-defined with helpful descriptions.The props object correctly defines the required properties for the action. The descriptions for each prop provide useful guidance for users.
Consider updating the fileName description to include the full path where the file will be saved:
- description: "The name to give the PDF when it is stored in the /tmp directory, e.g. `myFile.pdf`. If no file name is provided, the PDF will be named using the record ID, e.g. '22743.pdf'", + description: "The name to give the PDF when it is stored in the /tmp directory, e.g. `/tmp/myFile.pdf`. If no file name is provided, the PDF will be named using the record ID, e.g. '/tmp/22743.pdf'",components/quickbooks/constants.js (2)
42-204: LGTM: Comprehensive mapping of supported operations per entity.The
SUPPORTED_WEBHOOK_OPERATIONSconstant provides a detailed and well-structured mapping of supported operations for each QuickBooks entity. The comment referencing the Intuit documentation is helpful for future maintenance.Consider implementing a mechanism to keep this mapping up-to-date with the QuickBooks API. This could involve:
- Automating the generation of this object from the QuickBooks API documentation.
- Adding a periodic review process to ensure the information remains current.
This approach would help maintain accuracy and reduce the risk of outdated information as the QuickBooks API evolves.
1-210: Overall: Well-structured and comprehensive constants file for QuickBooks webhooks.This file effectively defines the necessary constants for working with QuickBooks webhooks. It aligns well with the PR objectives and provides a solid foundation for implementing custom webhook sources for QuickBooks.
Key strengths:
- Comprehensive lists of entities and operations.
- Detailed mapping of supported operations per entity.
- Clear documentation reference.
- Consistent naming and structure.
As the QuickBooks integration grows, consider creating a dedicated
quickbooksfolder within thecomponentsdirectory to house related files. This would improve organization and scalability, especially if more QuickBooks-specific files are added in the future.components/quickbooks/quickbooks.app.js (1)
28-34: Improve readability of string concatenation in 'webhookVerifierToken' descriptionThe
descriptionfield ofwebhookVerifierTokenuses string concatenation with+operators across multiple lines, which can be less readable. Consider using template literals for better readability.Apply this diff:
webhookVerifierToken: { type: "string", label: "Verifier Token", - description: "[Create an app](https://developer.intuit.com/app/developer/qbo/docs/build-your-first-app) " + - "on the Intuit Developer Dashboard and [set up a webhook](https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks). " + - "Once you have a [verifier token](https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks/managing-webhooks-notifications), " + - "fill it in below. Note that if you want to send webhooks to more than one Pipedream source, you will have to create a new Dashboard app for each different source.", + description: `[Create an app](https://developer.intuit.com/app/developer/qbo/docs/build-your-first-app) on the Intuit Developer Dashboard and [set up a webhook](https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks). Once you have a [verifier token](https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks/managing-webhooks-notifications), fill it in below. Note that if you want to send webhooks to more than one Pipedream source, you will have to create a new Dashboard app for each different source.`, secret: true, },components/quickbooks/sources/common.js (2)
16-16: Provide a meaningful description for the 'http' propThe
descriptionfield for thehttpprop is currently empty. Adding a description enhances code readability and helps other developers understand its purpose.Apply this diff to add a description:
http: { type: "$.interface.http", customResponse: true, label: "HTTP", - description: "", + description: "HTTP interface, enables receipt of webhook events from QuickBooks", },
151-154: Avoid variable shadowing by renaming 'event' in 'forEach' callbackUsing
eventas the variable name in theforEachcallback may cause confusion due to variable shadowing sinceeventis also a parameter in the enclosing scope. Consider renaming the variable to improve readability.Apply this diff to rename the variable:
events.forEach((event) => { + events.forEach((eventData) => { - const meta = this.generateMeta(event); - this.$emit(event, meta); + const meta = this.generateMeta(eventData); + this.$emit(eventData, meta); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- components/quickbooks/actions/download-pdf/download-pdf.js (1 hunks)
- components/quickbooks/constants.js (1 hunks)
- components/quickbooks/quickbooks.app.js (1 hunks)
- components/quickbooks/sources/common.js (1 hunks)
- components/quickbooks/sources/custom-webhook-events/custom-webhook-events.js (1 hunks)
- components/quickbooks/sources/new-or-modified-customer/new-or-modified-customer.js (1 hunks)
🔇 Additional comments (13)
components/quickbooks/sources/custom-webhook-events/custom-webhook-events.js (4)
1-2: LGTM: Import statements are appropriate.The import statements for
quickbooksandcommonare correctly implemented and align with the module's requirements.
11-25: LGTM: Props are well-defined and utilize QuickBooks propDefinitions.The props definition extends
common.propsand correctly addsentitiesToEmitandoperationsToEmitusing propDefinitions from the QuickBooks app. This approach ensures consistency and reusability.
26-34: LGTM: Methods are well-defined and provide clean access to props.The methods definition extends
common.methodsand correctly addsgetEntities()andgetOperations(). These methods provide a clean interface to access the selected entities and operations, which is good for maintainability and potential future extensions.
1-35: Overall, the implementation is well-structured and follows Pipedream conventions.This new custom webhook source for QuickBooks is implemented correctly, extending common functionality and adding specific features for entity and operation selection. The code is clean, maintainable, and follows Pipedream guidelines.
A minor suggestion was made to address the eslint-disable comment, but otherwise, the implementation looks solid and ready for use.
components/quickbooks/sources/new-or-modified-customer/new-or-modified-customer.js (2)
1-6: LGTM: Imports and constant definitions are well-structured.The imports, constant definition, and retrieval of supported operations are implemented correctly and follow best practices. This approach enhances maintainability and flexibility.
15-27: LGTM: Props definition is well-structured and flexible.The props definition is well-implemented:
- It reuses
common.propsfor consistency.- The
operationsToEmitprop is correctly defined using the QuickBooks app's propDefinition.- Overriding options and setting defaults based on supported operations provides flexibility and ensures compatibility.
components/quickbooks/actions/download-pdf/download-pdf.js (4)
1-4: LGTM: Imports are appropriate for the task.The required modules (quickbooks app, fs, util, and stream) are correctly imported for downloading and saving a PDF file.
6-11: LGTM: Module exports follow Pipedream conventions.The module.exports object correctly defines the component's name, description, key, version, and type. The key follows the recommended pattern, and the version is set to "0.0.1" as per Pipedream's guidelines for new components.
40-52: LGTM: downloadPDF method correctly handles PDF retrieval and saving.The downloadPDF method efficiently uses the quickbooks.getPDF method to retrieve the PDF and saves it to the /tmp directory using a stream pipeline. This approach is memory-efficient and follows best practices for file handling.
53-63: LGTM: run method effectively orchestrates the PDF download process.The run method correctly handles file naming, calls the downloadPDF method, and exports the necessary information. The inclusion of the $summary export provides a clear indication of the action's result, which is helpful for users.
components/quickbooks/constants.js (3)
1-31: LGTM: Comprehensive list of QuickBooks entities.The
WEBHOOK_ENTITIESconstant provides a thorough list of QuickBooks entities. The naming convention is consistent, and the alphabetical order enhances readability and maintainability.
33-40: LGTM: Well-defined list of webhook operations.The
WEBHOOK_OPERATIONSconstant effectively captures the main CRUD operations along with QuickBooks-specific operations. The naming convention is consistent, and the order is logical.
206-210: LGTM: Clear and consistent module exports.The module exports are well-defined, exporting all the necessary constants with consistent naming.
| module.exports = { | ||
| ...common, | ||
| key: "quickbooks-new-or-modified-customer", | ||
| name: "New or Modified Customer: Created, Updated, Merged or Deleted (Instant)", | ||
| description: "Emit new or modified customers. Visit the documentation page to learn how to configure webhooks for your QuickBooks company: https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks", | ||
| version: "0.0.1", | ||
| type: "source", |
There was a problem hiding this comment.
Update the source name to follow the recommended pattern.
The current name doesn't follow the pattern suggested in a previous review comment. To align with the Pipedream guidelines for source names, please update the name as follows:
- name: "New or Modified Customer: Created, Updated, Merged or Deleted (Instant)",
+ name: "New or Modified Customer (Instant)",This change makes the name more concise while still indicating that it's a real-time event source.
📝 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.
| module.exports = { | |
| ...common, | |
| key: "quickbooks-new-or-modified-customer", | |
| name: "New or Modified Customer: Created, Updated, Merged or Deleted (Instant)", | |
| description: "Emit new or modified customers. Visit the documentation page to learn how to configure webhooks for your QuickBooks company: https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks", | |
| version: "0.0.1", | |
| type: "source", | |
| module.exports = { | |
| ...common, | |
| key: "quickbooks-new-or-modified-customer", | |
| name: "New or Modified Customer (Instant)", | |
| description: "Emit new or modified customers. Visit the documentation page to learn how to configure webhooks for your QuickBooks company: https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks", | |
| version: "0.0.1", | |
| type: "source", |
| companyId() { | ||
| return this.$auth.company_id; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistent method naming conventions
Private methods are typically prefixed with an underscore (e.g., _apiUrl(), _authToken()). For consistency, consider renaming companyId() to _companyId().
Apply this diff to rename the method and update its references:
},
- companyId() {
+ _companyId() {
return this.$auth.company_id;
},Update references to the method:
const companyId = this.companyId();
+ const companyId = this._companyId();This change applies to all instances where companyId() is called:
- In the
getPDFmethod (lines 74-75) - In the
getRecordDetailsmethod (lines 91-92)
Also applies to: 74-93
| } catch (ex) { | ||
| if (ex.response.data.statusCode === 400) { | ||
| throw new Error(`Request failed with status code 400. Double-check that '${id}' is a valid ${entity} record ID.`); | ||
| } else { | ||
| throw ex; | ||
| } | ||
| } |
There was a problem hiding this comment.
Enhance error handling in the 'getPDF' method
In the catch block of the getPDF method, accessing ex.response.data.statusCode without checking if ex.response and ex.response.data exist could lead to a runtime error if they are undefined. To prevent potential TypeError, please add checks to ensure these properties are defined before accessing statusCode.
Apply this diff to improve error handling:
} catch (ex) {
- if (ex.response.data.statusCode === 400) {
+ if (ex.response && ex.response.data && ex.response.data.statusCode === 400) {
throw new Error(`Request failed with status code 400. Double-check that '${id}' is a valid ${entity} record ID.`);
} else {
throw ex;
}
}📝 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.
| } catch (ex) { | |
| if (ex.response.data.statusCode === 400) { | |
| throw new Error(`Request failed with status code 400. Double-check that '${id}' is a valid ${entity} record ID.`); | |
| } else { | |
| throw ex; | |
| } | |
| } | |
| } catch (ex) { | |
| if (ex.response && ex.response.data && ex.response.data.statusCode === 400) { | |
| throw new Error(`Request failed with status code 400. Double-check that '${id}' is a valid ${entity} record ID.`); | |
| } else { | |
| throw ex; | |
| } | |
| } |
| async getRecordDetails(entityName, id) { | ||
| const companyId = this.companyId(); | ||
| return await this._makeRequest(this, { | ||
| path: `company/${companyId}/${entityName.toLowerCase()}/${id}`, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent usage of the context parameter in 'getRecordDetails'
In the getRecordDetails method, you pass this as the first argument to _makeRequest. However, in other methods like getPDF, you pass $ as the context parameter. For consistency and clarity, consider adding $ as a parameter to getRecordDetails and passing it to _makeRequest.
Apply this diff to make the method consistent:
},
- async getRecordDetails(entityName, id) {
+ async getRecordDetails($, entityName, id) {
const companyId = this.companyId();
- return await this._makeRequest(this, {
+ return await this._makeRequest($, {
path: `company/${companyId}/${entityName.toLowerCase()}/${id}`,
});
},📝 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.
| async getRecordDetails(entityName, id) { | |
| const companyId = this.companyId(); | |
| return await this._makeRequest(this, { | |
| path: `company/${companyId}/${entityName.toLowerCase()}/${id}`, | |
| }); | |
| }, | |
| async getRecordDetails($, entityName, id) { | |
| const companyId = this.companyId(); | |
| return await this._makeRequest($, { | |
| path: `company/${companyId}/${entityName.toLowerCase()}/${id}`, | |
| }); | |
| }, |
| if (!relevantOperations?.length > 0 && !relevantOperations.includes(operation)) { | ||
| console.log(`Skipping '${operation} ${name}' event. (Accepted operations: ${relevantOperations.join(", ")})`); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Fix logical error in operations check within 'isEntityRelevant' function
The condition in line 95 incorrectly negates the length check, causing the function to improperly filter operations. The '!' before 'relevantOperations?.length' should be removed to ensure correct behavior.
Apply this diff to correct the condition:
- if (!relevantOperations?.length > 0 && !relevantOperations.includes(operation)) {
+ if (relevantOperations?.length > 0 && !relevantOperations.includes(operation)) {📝 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.
| if (!relevantOperations?.length > 0 && !relevantOperations.includes(operation)) { | |
| console.log(`Skipping '${operation} ${name}' event. (Accepted operations: ${relevantOperations.join(", ")})`); | |
| return false; | |
| } | |
| if (relevantOperations?.length > 0 && !relevantOperations.includes(operation)) { | |
| console.log(`Skipping '${operation} ${name}' event. (Accepted operations: ${relevantOperations.join(", ")})`); | |
| return false; | |
| } |
| companyId(event) { | ||
| return event.body.eventNotifications[0].realmId; | ||
| }, |
There was a problem hiding this comment.
Add error handling for missing 'eventNotifications' in 'companyId' method
Accessing event.body.eventNotifications[0] without verifying if eventNotifications exists could result in an error if the property is missing or empty. Consider adding a check to ensure eventNotifications is present and has elements.
Apply this diff to add error handling:
companyId(event) {
+ if (event.body.eventNotifications && event.body.eventNotifications.length > 0) {
return event.body.eventNotifications[0].realmId;
+ } else {
+ throw new Error("Invalid event: 'eventNotifications' is missing.");
+ }
},📝 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.
| companyId(event) { | |
| return event.body.eventNotifications[0].realmId; | |
| }, | |
| companyId(event) { | |
| if (event.body.eventNotifications && event.body.eventNotifications.length > 0) { | |
| return event.body.eventNotifications[0].realmId; | |
| } else { | |
| throw new Error("Invalid event: 'eventNotifications' is missing."); | |
| } | |
| }, |
| toPastTense(operations) { | ||
| const pastTenseVersion = { | ||
| Create: "Created", | ||
| Update: "Updated", | ||
| Merge: "Merged", | ||
| Delete: "Deleted", | ||
| Void: "Voided", | ||
| Emailed: "Emailed", | ||
| }; | ||
| if (Array.isArray(operations)) { | ||
| return operations.map((operation) => pastTenseVersion[operation]); | ||
| } else { | ||
| return pastTenseVersion[operations]; | ||
| } | ||
| }, |
There was a problem hiding this comment.
Handle unexpected operation names in 'toPastTense' function
If an operation name is not present in the pastTenseVersion mapping, the function will return undefined, which may lead to unclear summaries or errors downstream. Consider providing a default value or handling unexpected operations gracefully.
Apply this diff to handle unexpected operation names:
if (Array.isArray(operations)) {
- return operations.map((operation) => pastTenseVersion[operation]);
+ return operations.map((operation) => pastTenseVersion[operation] || operation);
} else {
- return pastTenseVersion[operations];
+ return pastTenseVersion[operations] || operations;
}📝 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.
| toPastTense(operations) { | |
| const pastTenseVersion = { | |
| Create: "Created", | |
| Update: "Updated", | |
| Merge: "Merged", | |
| Delete: "Deleted", | |
| Void: "Voided", | |
| Emailed: "Emailed", | |
| }; | |
| if (Array.isArray(operations)) { | |
| return operations.map((operation) => pastTenseVersion[operation]); | |
| } else { | |
| return pastTenseVersion[operations]; | |
| } | |
| }, | |
| toPastTense(operations) { | |
| const pastTenseVersion = { | |
| Create: "Created", | |
| Update: "Updated", | |
| Merge: "Merged", | |
| Delete: "Deleted", | |
| Void: "Voided", | |
| Emailed: "Emailed", | |
| }; | |
| if (Array.isArray(operations)) { | |
| return operations.map((operation) => pastTenseVersion[operation] || operation); | |
| } else { | |
| return pastTenseVersion[operations] || operations; | |
| } | |
| }, |
| const { entities } = event.body.eventNotifications[0].dataChangeEvent; | ||
|
|
There was a problem hiding this comment.
Validate 'eventNotifications' before processing entities in 'processEvent'
Accessing event.body.eventNotifications[0].dataChangeEvent without checking may cause an error if eventNotifications is missing or empty. Add a check to ensure eventNotifications exists and has at least one element before accessing dataChangeEvent.
Apply this diff to add validation:
async processEvent(event) {
+ if (!(event.body.eventNotifications && event.body.eventNotifications.length > 0)) {
+ console.log("No 'eventNotifications' found in the event.");
+ return;
+ }
const { entities } = event.body.eventNotifications[0].dataChangeEvent;📝 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 { entities } = event.body.eventNotifications[0].dataChangeEvent; | |
| async processEvent(event) { | |
| if (!(event.body.eventNotifications && event.body.eventNotifications.length > 0)) { | |
| console.log("No 'eventNotifications' found in the event."); | |
| return; | |
| } | |
| const { entities } = event.body.eventNotifications[0].dataChangeEvent; |
6 participants
This is a custom webhook source for QuickBooks that allows the user to choose which entities to emit (e.g. Invoices, Bills, Purchase Orders) as well as which operations to emit and which to ignore (e.g. Created, Updated, Emailed)
Since the contribution guidelines encourage creating components that address specific use-cases, I also created a source that is hard-coded to emit only Customer events. This file can be a template for creating other sources that are specific to different entities but I figured I’d better first get feedback on what I currently have.
Instead of creating separate sources to handle New Customers, Updated Customers, Deleted Customers, etc. I put all the operations into one source with a prop to allow the user to customize which operations are emitted.
Webhook Setup
I searched for a way to subscribe to webhooks programmatically but unfortunately it looks like setting up QuickBooks webhooks requires a complicated manual process. The worst thing is that this has to be done for each endpoint the user wants to connect to — you have to create a new App in the Intuit Developer Dashboard and authenticate it with your QB company before any webhooks are sent to the new endpoint. It’s a big hassle.
I wrote a step-by-step guide and included a link in the source descriptions to give users all the info in one place so they don’t have to go spelunking through the official QB documentation.
Source Names
I included the list of operations (Created, Updated, Merged, Deleted, Voided or Emailed) in the source names so that they’d be searchable when the user is typing what source they want. Let me know if it would be better to use shorter names. I wish the description was searchable so the names could be simpler while still having the applicable source appear if someone types in e.g. “Email Purchase Order”
Props
Once the QB app is set up on Intuit Dashboard, the user receives a verifier token that they can paste into the source config to make sure incoming webhooks are actually from QuickBooks.
The user can configure which entities and operations they want to emit and which to ignore. I set it up so that if they leave these props empty, everything will be emitted. Otherwise it’s kind of a pain to switch between emitting just a few items to emitting everything on the long list of entities because you’d have to click each option individually to select or deselect.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation