[ACTION] Intercom Upsert Contact #14268 #14479
Conversation
Actions - Upsert Contact
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe changes in this pull request involve updates to various files within the Intercom component. Key modifications include the introduction of a new action for upserting contacts, the addition of a constant for role options, and multiple version increments across existing modules. The functionality of the existing methods remains unchanged, with enhancements primarily focused on version updates and new method implementations for improved contact management. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![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
🧹 Outside diff range and nitpick comments (9)
components/intercom/common/constants.mjs (1)
1-10: Add documentation and prevent mutations.The implementation correctly defines the role options for Intercom contacts. Consider these improvements:
Add JSDoc documentation and freeze the array to prevent mutations:
+/** + * Valid role options for Intercom contacts. + * @see https://developers.intercom.com/docs/references/rest-api/api.intercom.io/Contacts/ + */ -export const ROLE_OPTIONS = [ +export const ROLE_OPTIONS = Object.freeze([ { label: "User", value: "user", }, { label: "Lead", value: "lead", }, -]; +]);components/intercom/sources/new-topic/new-topic.mjs (2)
Line range hint
36-42: Fix extra brace in summary string.The summary string contains an extra closing brace that should be removed.
Apply this fix:
this.$emit(body, { id: id ?? uuid(), - summary: `${eventType} - id: ${id ?? "(no id found)"}}`, + summary: `${eventType} - id: ${id ?? "(no id found)"}`, ts: new Date().getTime(), });
Line range hint
23-26: Consider revising error handling approach.The error messages suggest events are being skipped, but throwing an error actually terminates execution. Consider either:
- Actually skipping events by returning early instead of throwing
- Revising the error messages to remove "Skipping event..."
Also, consider making the error messages consistent across both validation checks.
Here's a suggested implementation:
isSignatureValid(body, signature) { if (!signature) { - throw new Error("Signature is missing in the request header. Skipping event..."); + throw new Error("Signature is missing in the request header"); } const hmac = crypto.createHmac("sha1", this.clientSecret); hmac.update(body); const digest = hmac.digest("hex"); return `sha1=${digest}` === signature; }, async run(event) { if (this.clientSecret && !this.isSignatureValid(event.bodyRaw, event.headers["x-hub-signature"])) { - throw new Error("Signature is not valid. Skipping event..."); + throw new Error("Signature is not valid"); } this.emit(event.body); },Alternatively, if you want to skip invalid events instead of throwing:
isSignatureValid(body, signature) { if (!signature) { - throw new Error("Signature is missing in the request header. Skipping event..."); + console.log("Signature is missing in the request header. Skipping event..."); + return false; } const hmac = crypto.createHmac("sha1", this.clientSecret); hmac.update(body); const digest = hmac.digest("hex"); return `sha1=${digest}` === signature; }, async run(event) { if (this.clientSecret && !this.isSignatureValid(event.bodyRaw, event.headers["x-hub-signature"])) { - throw new Error("Signature is not valid. Skipping event..."); + console.log("Signature is not valid. Skipping event..."); + return; } this.emit(event.body); },Also applies to: 44-47
components/intercom/actions/upsert-contact/upsert-contact.mjs (2)
8-8: Consider starting with version 1.0.0Since this is a new production-ready action being added to handle critical contact data, consider starting with version 1.0.0 following semantic versioning best practices.
7-7: Enhance documentation referenceThe current documentation link points to the general contact creation endpoint. Consider adding links to both create and update endpoints since this action handles both operations.
- description: "Create a new contact. If there is already a contact with the email provided, the existing contact will be updated. [See the docs here](https://developers.intercom.com/docs/references/rest-api/api.intercom.io/contacts/createcontact)", + description: "Create a new contact or update an existing one based on the email provided. [See the create docs](https://developers.intercom.com/docs/references/rest-api/api.intercom.io/contacts/createcontact) and [update docs](https://developers.intercom.com/docs/references/rest-api/api.intercom.io/contacts/updatecontact)",components/intercom/intercom.app.mjs (4)
48-62: LGTM! Consider adding TypeScript-style param types in JSDoc.The refactoring to use destructuring improves the method's flexibility while maintaining backward compatibility. The documentation is comprehensive.
Consider enhancing the JSDoc by adding TypeScript-style param types:
/** * @param {Object} opts * @param {string} opts.method - HTTP method * @param {string} [opts.url] - Full URL (optional) * @param {string} [opts.endpoint] - API endpoint * @param {import("@pipedream/platform").$.Interface} [opts.$] - Pipedream axios instance */
212-218: Add JSDoc documentation for searchContact method.The implementation is correct, but documentation would improve maintainability.
Add JSDoc documentation:
+/** + * Search for a contact using specified criteria + * @param {Object} opts - Search options + * @param {Object} [opts.query] - Search query parameters + * @returns {Promise<Object>} Search results + */ searchContact(opts = {}) {
226-234: Add JSDoc documentation and error handling for updateContact method.The implementation is correct but needs documentation and error handling guidance.
Add documentation and validation:
+/** + * Update an existing contact + * @param {Object} params - Update parameters + * @param {string} params.contactId - Contact ID to update + * @param {Object} params.data - Contact data to update + * @throws {Error} When contactId is not provided + * @returns {Promise<Object>} Updated contact + */ updateContact({ contactId, ...opts }) { + if (!contactId) { + throw new Error("Contact ID is required for updates"); + } return this.makeRequest({
212-234: Consider implementing a contact service layer.The current implementation directly exposes API methods. Consider introducing a service layer that:
- Implements the upsert logic by combining search, create, and update operations
- Handles common error cases
- Provides retry logic for rate limits
- Implements proper logging
This would improve maintainability and reusability.
Would you like me to provide an example implementation of the contact service layer?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (21)
components/intercom/actions/create-note/create-note.mjs(1 hunks)components/intercom/actions/send-incoming-message/send-incoming-message.mjs(1 hunks)components/intercom/actions/upsert-contact/upsert-contact.mjs(1 hunks)components/intercom/common/constants.mjs(1 hunks)components/intercom/intercom.app.mjs(2 hunks)components/intercom/package.json(1 hunks)components/intercom/sources/conversation-closed/conversation-closed.mjs(1 hunks)components/intercom/sources/lead-added-email/lead-added-email.mjs(1 hunks)components/intercom/sources/new-admin-reply/new-admin-reply.mjs(1 hunks)components/intercom/sources/new-company/new-company.mjs(1 hunks)components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs(1 hunks)components/intercom/sources/new-conversation/new-conversation.mjs(1 hunks)components/intercom/sources/new-event/new-event.mjs(1 hunks)components/intercom/sources/new-lead/new-lead.mjs(1 hunks)components/intercom/sources/new-topic/new-topic.mjs(1 hunks)components/intercom/sources/new-unsubscription/new-unsubscription.mjs(1 hunks)components/intercom/sources/new-user-reply/new-user-reply.mjs(1 hunks)components/intercom/sources/new-user/new-user.mjs(1 hunks)components/intercom/sources/tag-added-to-conversation/tag-added-to-conversation.mjs(1 hunks)components/intercom/sources/tag-added-to-lead/tag-added-to-lead.mjs(1 hunks)components/intercom/sources/tag-added-to-user/tag-added-to-user.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (17)
- components/intercom/actions/create-note/create-note.mjs
- components/intercom/actions/send-incoming-message/send-incoming-message.mjs
- components/intercom/package.json
- components/intercom/sources/conversation-closed/conversation-closed.mjs
- components/intercom/sources/lead-added-email/lead-added-email.mjs
- components/intercom/sources/new-admin-reply/new-admin-reply.mjs
- components/intercom/sources/new-company/new-company.mjs
- components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs
- components/intercom/sources/new-conversation/new-conversation.mjs
- components/intercom/sources/new-event/new-event.mjs
- components/intercom/sources/new-lead/new-lead.mjs
- components/intercom/sources/new-unsubscription/new-unsubscription.mjs
- components/intercom/sources/new-user-reply/new-user-reply.mjs
- components/intercom/sources/new-user/new-user.mjs
- components/intercom/sources/tag-added-to-conversation/tag-added-to-conversation.mjs
- components/intercom/sources/tag-added-to-lead/tag-added-to-lead.mjs
- components/intercom/sources/tag-added-to-user/tag-added-to-user.mjs
🔇 Additional comments (4)
components/intercom/common/constants.mjs (1)
1-10: Verify role options against Intercom's API documentation.
Let's verify that these are all the valid role types supported by Intercom's API.
components/intercom/sources/new-topic/new-topic.mjs (2)
9-9: LGTM! Version bump is appropriate.
The version increment from 0.0.2 to 0.0.3 correctly reflects the patch-level changes made to improve error handling.
Line range hint 23-47: Verify webhook implementation against Intercom's documentation.
Please ensure the webhook implementation aligns with Intercom's latest API specifications:
- Signature validation should match Intercom's webhook security documentation.
- The event payload structure (
event.body.data?.item) should handle all possible webhook event types.
✅ Verification successful
Let me search for more information about the webhook implementation.
Let me search for more implementation details to verify the webhook security and payload structure.
Based on the gathered information, I can now provide a final response about the webhook implementation verification.
Webhook implementation correctly follows Intercom's specifications
The implementation correctly follows Intercom's webhook security and payload handling requirements:
- The signature validation uses SHA1 HMAC with the client secret, matching Intercom's security requirements
- The payload structure handles both standard and notification events through proper null coalescing:
body.data?.item.type ?? body.type ?? "notification_event"body.data?.item.id ?? body.data?.item.external_id
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other Intercom webhook implementations to compare
rg -l "x-hub-signature" components/intercom/
Length of output: 96
Script:
#!/bin/bash
# Search for webhook payload handling patterns in Intercom components
rg -A 5 "data\?.item" components/intercom/
# Look for any documentation or comments about webhook handling
rg -i "webhook" components/intercom/ --type md
Length of output: 766
Script:
#!/bin/bash
# Look for app.mjs or similar files that might contain webhook configuration
fd -e mjs -e js . components/intercom/ --exec head -n 20 {}
# Search for any security-related imports or configurations
rg -A 3 "createHmac|crypto" components/intercom/
Length of output: 11694
components/intercom/intercom.app.mjs (1)
219-225: Add JSDoc documentation and input validation for createContact method.
The implementation is correct but could benefit from documentation and validation.
Add documentation and validation:
+/**
+ * Create a new contact
+ * @param {Object} opts - Contact creation options
+ * @param {Object} opts.role - Contact role (user or lead)
+ * @param {string} [opts.email] - Contact's email
+ * @param {string} [opts.phone] - Contact's phone
+ * @returns {Promise<Object>} Created contact
+ */
createContact(opts = {}) {
+ if (!opts.role) {
+ throw new Error("Contact role is required");
+ }
return this.makeRequest({Let's verify the required fields in the Intercom API:
|
|
||
| data = Object.entries(data).filter(([ | ||
| , | ||
| value, | ||
| ]) => (value != "" && value != undefined)) | ||
| .reduce((obj, [ | ||
| key, | ||
| value, | ||
| ]) => Object.assign(obj, { | ||
| [key]: value, | ||
| }), {}); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify object filtering
The current object filtering logic could be simplified using Object.fromEntries for better readability.
- data = Object.entries(data).filter(([
- ,
- value,
- ]) => (value != "" && value != undefined))
- .reduce((obj, [
- key,
- value,
- ]) => Object.assign(obj, {
- [key]: value,
- }), {});
+ data = Object.fromEntries(
+ Object.entries(data).filter(([, value]) =>
+ value !== "" && value !== undefined
+ )
+ );📝 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.
| data = Object.entries(data).filter(([ | |
| , | |
| value, | |
| ]) => (value != "" && value != undefined)) | |
| .reduce((obj, [ | |
| key, | |
| value, | |
| ]) => Object.assign(obj, { | |
| [key]: value, | |
| }), {}); | |
| data = Object.fromEntries( | |
| Object.entries(data).filter(([, value]) => | |
| value !== "" && value !== undefined | |
| ) | |
| ); |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
jcortes
left a comment
jcortes
left a comment
There was a problem hiding this comment.
Hi @luancazarine lgtm! Ready for QA!
* [ACTION] Intercom Upsert Contact #14268 Actions - Upsert Contact * Update components/intercom/actions/upsert-contact/upsert-contact.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update components/intercom/actions/upsert-contact/upsert-contact.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update components/intercom/actions/upsert-contact/upsert-contact.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Resolves #14268
Summary by CodeRabbit
Release Notes
New Features
Version Updates
Enhancements
new-topicmodule.