-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Intercom - new-conversation-rating-added source #14456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe pull request introduces several updates to the Changes
Assessment against linked issues
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
components/intercom/sources/new-event/new-event.mjs (2)
Line range hint 39-46: Fix incorrect URL assignment in pagination logic.
The current implementation stores the old URL instead of the new one, which could break pagination:
Apply this fix:
if (results.nextUrl) {
- this._setNextUrl(userId, nextUrl);
+ this._setNextUrl(userId, results.nextUrl);
}Line range hint 37-47: Add error handling and rate limiting.
The run method could benefit from improved robustness:
- Add try-catch blocks for API calls
- Implement rate limiting between requests
Consider this implementation:
async run() {
for (const userId of this.userIds) {
+ try {
let nextUrl = this._getNextUrl(userId);
const results = await this.intercom.getEvents(userId, nextUrl);
for (const result of results.events) {
const meta = this.generateMeta(result);
this.$emit(result, meta);
}
if (results.nextUrl) {
- this._setNextUrl(userId, nextUrl);
+ this._setNextUrl(userId, results.nextUrl);
}
+ // Add delay to prevent rate limiting
+ await new Promise(resolve => setTimeout(resolve, 1000));
+ } catch (error) {
+ console.error(`Failed to process events for user ${userId}:`, error);
+ this.$emit({
+ message: `Failed to process events for user ${userId}`,
+ error: error.message,
+ }, {
+ id: `error-${userId}-${Date.now()}`,
+ summary: `Error: ${error.message}`,
+ ts: Date.now(),
+ });
+ }
}
},components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs (1)
6-8: Enhance the component description for better clarity.
The current description could be more specific about what constitutes a "rating" in Intercom conversations and its business value.
Consider updating to something like:
- description: "Emit new event each time a new rating is added to a conversation.",
+ description: "Emit new event when a customer adds a CSAT or other rating to an Intercom conversation. Useful for monitoring customer satisfaction and driving team improvements.",components/intercom/sources/new-user-reply/new-user-reply.mjs (1)
Line range hint 22-47: Consider enhancing robustness and performance.
While the core functionality is solid, consider these improvements:
- Add rate limiting to prevent API abuse when fetching many conversations
- Use strict timestamp comparison (e.g.,
>=instead of>) to avoid missing edge cases - Add error handling for API calls and invalid conversation data
Here's a suggested implementation with these improvements:
async run() {
let lastContactReplyAt = this._getLastUpdate();
const data = {
query: {
field: "statistics.last_contact_reply_at",
operator: ">",
value: lastContactReplyAt,
},
};
- const results = await this.intercom.searchConversations(data);
- for (const conversation of results) {
+ try {
+ const results = await this.intercom.searchConversations(data);
+ for (const conversation of results) {
+ // Add delay to prevent rate limiting
+ await this.$.flow.delay(100);
+
+ if (conversation.statistics.last_contact_reply_at >= lastContactReplyAt) {
+ lastContactReplyAt = conversation.statistics.last_contact_reply_at;
+ }
+
+ try {
+ const conversationData = await this.intercom.getConversation(conversation.id);
+ if (!conversationData?.conversation_parts) {
+ console.log("Skipping conversation with invalid data:", conversation.id);
+ continue;
+ }
+
+ const totalCount = conversationData.conversation_parts.total_count;
+ const conversationBody =
+ conversationData?.conversation_parts?.conversation_parts[totalCount - 1]?.body;
+
+ if (totalCount > 0 && conversationBody) {
+ const meta = this.generateMeta(
+ conversation,
+ conversationData,
+ conversationBody,
+ totalCount,
+ );
+ this.$emit(conversationData, meta);
+ }
+ } catch (err) {
+ console.error("Error processing conversation:", conversation.id, err);
+ continue;
+ }
+ }
+ this._setLastUpdate(lastContactReplyAt);
+ } catch (err) {
+ console.error("Error fetching conversations:", err);
+ throw err;
+ }
- if (conversation.statistics.last_contact_reply_at > lastContactReplyAt)
- lastContactReplyAt = conversation.statistics.last_contact_reply_at;
- const conversationData = (
- await this.intercom.getConversation(conversation.id)
- );
- const totalCount = conversationData.conversation_parts.total_count;
- const conversationBody =
- conversationData?.conversation_parts?.conversation_parts[totalCount - 1]?.body;
- if (totalCount > 0 && conversationBody) {
- // emit id & summary from last part/reply added
- const meta =
- this.generateMeta(conversation, conversationData, conversationBody, totalCount);
- this.$emit(conversationData, meta);
- }
- }
-
- this._setLastUpdate(lastContactReplyAt);
},📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
- components/intercom/package.json (2 hunks)
- components/intercom/sources/common/common.mjs (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-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 (4)
- components/intercom/sources/common/common.mjs
- components/intercom/sources/new-conversation/new-conversation.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
🔇 Additional comments (25)
components/intercom/package.json (2)
3-3: LGTM: Version bump follows semver.
The increment from 0.3.9 to 0.4.0 appropriately reflects the addition of the new conversation rating feature.
17-17: Verify compatibility with the updated platform dependency.
The major version jump from ^1.2.0 to ^3.0.3 in @pipedream/platform could introduce breaking changes.
Let's check for any breaking changes in the platform dependency:
components/intercom/sources/new-company/new-company.mjs (2)
8-8: Question: Is this version bump necessary?
This file (new-company.mjs) appears unrelated to the PR's objective of adding conversation rating functionality. While version bumps are good practice, they should be purposeful and related to actual changes in functionality.
Could you clarify why this file needs a version bump when it doesn't seem to be directly related to the conversation rating feature being added?
Let's check for actual changes in this file:
✅ Verification successful
Let me generate another script to check the changes more thoroughly:
Version bump is justified due to import path change
The version bump from 0.0.4 to 0.0.5 is necessary as there's a change in the import path from "../common.mjs" to "../common/common.mjs". This is a structural change that affects the module's dependencies and warrants a version increment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for substantial changes in this file
git log -p components/intercom/sources/new-company/new-company.mjs | grep -v "version:" | grep -v "import"
Length of output: 2282
Script:
#!/bin/bash
# Get the full file content to check for any functional changes
git show 093146cf1ccfb75ac7f947b7ff97285d337423a3 components/intercom/sources/new-company/new-company.mjs
Length of output: 920
1-1: Verify consistency of common module restructuring.
The import path has been updated to reflect the new directory structure.
Let's verify that this restructuring is consistent across all Intercom source files:
✅ Verification successful
The previous command had syntax issues and didn't provide clear results. This new script will help us understand:
- All files that import from common module
- Verify the existence and structure of the common directory
- See all import patterns used in the intercom component
Import path is correctly structured and consistent across the codebase
The verification shows that:
- The common module exists at the correct location:
components/intercom/sources/common/common.mjs - All source files in the Intercom component consistently use the same import path
"../common/common.mjs" - No instances of the old import path were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining old import paths and verify new structure
# Test 1: Search for old import path. Expect: No results
rg "from \"\.\./common\.mjs\"" components/intercom/
# Test 2: Verify new common module location exists
test -f components/intercom/sources/common/common.mjs || echo "New common module path does not exist"
# Test 3: Check all imports use the new path
rg "from \"\.\./common/common\.mjs\"" components/intercom/
Length of output: 2282
Script:
#!/bin/bash
# Let's try a different approach to verify the import paths
# Check all files that import from common module in intercom component
rg -l "import.*from.*common" components/intercom/
# Check the actual content of the common module location
ls -la components/intercom/sources/common/
# Check all import statements in intercom component for better context
rg "^import.*from" components/intercom/
Length of output: 3537
components/intercom/sources/tag-added-to-user/tag-added-to-user.mjs (2)
8-8: LGTM: Version bump is consistent.
The version increment from "0.0.4" to "0.0.5" follows semantic versioning and is consistent with other module updates in this PR.
1-1: Verify the common module path exists.
The import path has been updated to reflect the new directory structure. Let's verify the new path exists to prevent runtime errors.
✅ Verification successful
Import path change is valid and consistent
The verification confirms:
- The common module exists at the new location:
components/intercom/sources/common/common.mjs - All source files in the Intercom component consistently use the same import path
"../common/common.mjs"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the common module in the new location
# and check for consistency across other files
# Test 1: Check if the new common module exists
fd --type f "common.mjs$" components/intercom/sources/common
# Test 2: Verify all files use the same import path
rg "import.*common.*from.*common" components/intercom/sources/
Length of output: 1695
components/intercom/sources/new-unsubscription/new-unsubscription.mjs (2)
8-8: LGTM: Version bump is appropriate.
The version increment from 0.0.4 to 0.0.5 follows semantic versioning and is appropriate for non-breaking changes.
1-1: Verify consistent import path updates across components.
The import path change reflects a directory restructure moving the common module to a dedicated directory.
Let's verify this change is consistent across all Intercom components:
✅ Verification successful
Import path changes are consistent across all Intercom components
The verification shows that:
- No instances of the old import path (
../common.mjs) were found - The new import path (
../common/common.mjs) is consistently used across all Intercom source components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining old-style imports of common.mjs
# Expect: No matches for the old import path
# Test 1: Search for old import path
echo "Checking for old import paths..."
rg "from \"\.\./common\.mjs\"" components/intercom/
# Test 2: Verify new import path is used consistently
echo "Verifying new import path usage..."
rg "from \"\.\./common/common\.mjs\"" components/intercom/
Length of output: 1786
components/intercom/sources/new-user/new-user.mjs (3)
1-1: LGTM: Import path update is consistent with the component restructuring.
The updated import path reflects the new directory structure, maintaining consistency across the component.
8-8: Verify version consistency across the component.
The version bump from "0.0.4" to "0.0.5" appears appropriate for the non-breaking changes.
Let's verify version consistency across all Intercom source files:
Line range hint 1-50: Verify integration with new conversation rating feature.
While this file's changes are correct, since it's part of a PR adding conversation rating support, we should verify that this source's events don't overlap with the new rating functionality.
Let's check for any potential overlap or dependencies:
✅ Verification successful
No overlap between user events and conversation rating functionality
Based on the comprehensive analysis:
- The new user source operates independently from the conversation rating functionality
- The common code (
common.mjs) contains no rating-related logic - The rating functionality is isolated in its own source component (
new-conversation-rating-added.mjs) - There are no cross-references between user events and conversation ratings
The changes in this file are safe as both features operate on different data streams with no shared dependencies or potential event overlaps.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any cross-references between user events and conversation ratings
# Expected: No direct dependencies between these features
echo "Checking for references to user events in rating-related code:"
rg -l 'conversation.*rating' components/intercom/sources/new-user/
Length of output: 203
Script:
#!/bin/bash
# Let's perform a more comprehensive check:
# 1. Find all files related to conversation ratings
echo "=== Files containing conversation rating patterns ==="
rg -l "conversation.*rating|rating.*conversation" components/intercom/
# 2. Check the common code since this source imports from it
echo -e "\n=== Checking common code for rating references ==="
cat components/intercom/sources/common/common.mjs
# 3. Check for any user-related patterns in rating files
echo -e "\n=== Checking user references in rating-related code ==="
rg "user|contact" -C 2 components/intercom/sources/ | grep -i "rating"
# 4. Look for other sources that might handle ratings
echo -e "\n=== Looking for other rating-related sources ==="
fd "new-.*rating.*\.mjs$|rating.*\.mjs$" components/intercom/sources/
Length of output: 1620
components/intercom/sources/new-lead/new-lead.mjs (2)
1-1: LGTM: Import path update aligns with new directory structure.
The updated import path reflects the improved organization of common utilities in a dedicated directory.
8-8: LGTM: Version increment follows semantic versioning.
The version bump from "0.0.4" to "0.0.5" is appropriate for the directory structure changes and maintains consistency with other updated modules.
components/intercom/sources/conversation-closed/conversation-closed.mjs (3)
Line range hint 1-43: LGTM! The implementation looks solid.
The source component correctly:
- Implements deduplication using unique IDs combining conversation ID and timestamp
- Tracks last update timestamp for proper pagination
- Handles conversation metadata generation
- Emits events with appropriate metadata
1-1: Verify the new common module path exists.
The import path has been updated to reflect the new directory structure.
✅ Verification successful
Let me verify the exact import paths being used to ensure consistency.
Import path change is correct and consistent
The new import path "../common/common.mjs" is verified to be:
- Present at the expected location
components/intercom/sources/common/common.mjs - Used consistently across all Intercom source components
- Properly referenced with the correct relative path
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the common module and its consistent usage across other files
# Test 1: Check if the new common module exists
fd --type f "common.mjs$" components/intercom/
# Test 2: Verify consistent import path usage across all source files
rg -l "import.*from.*common" components/intercom/
Length of output: 1066
Script:
#!/bin/bash
# Check the actual import statements to verify consistency
rg "import.*from.*common" components/intercom/sources/ -A 1
Length of output: 2556
8-8: Verify version bump consistency across components.
The version increment from "0.0.4" to "0.0.5" appears to be part of a coordinated update.
components/intercom/sources/new-event/new-event.mjs (2)
1-1: LGTM: Import path update reflects improved organization.
The updated import path aligns with the broader codebase restructuring effort, improving module organization.
8-8: LGTM: Version bump follows semantic versioning.
The version increment is consistent with the changes across the Intercom component.
components/intercom/sources/lead-added-email/lead-added-email.mjs (3)
1-1: LGTM: Import path standardization
The updated import path aligns with the standardized module structure being implemented across the Intercom sources.
8-8: LGTM: Appropriate version bump
Version increment from 0.0.4 to 0.0.5 follows semantic versioning for non-breaking changes.
Line range hint 1-54: Verify the impact of common module restructuring
While the changes look good, let's verify that the common module restructuring hasn't affected other components.
✅ Verification successful
Common module restructuring has been consistently applied across all Intercom sources
The verification shows that all Intercom source components are using the new import path "../common/common.mjs", and there are no remaining references to the old path "../common.mjs". The changes have been properly propagated across all 14 source components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all Intercom source components use the new common module path
# and that there are no remaining references to the old path.
echo "Checking for any remaining old import paths..."
rg "from \"\.\./common\.mjs\"" components/intercom/sources/
echo "Verifying all source files use the new import path..."
rg "from \"\.\./common/common\.mjs\"" components/intercom/sources/
Length of output: 1868
components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs (1)
24-30: Verify the correct query field for rating updates.
The current query uses conversation_rating.requested_at, but you might want to use conversation_rating.created_at to ensure consistency with the timestamp you're tracking.
components/intercom/sources/new-admin-reply/new-admin-reply.mjs (1)
1-1: LGTM! Import path update improves module organization.
The common module has been moved to a dedicated directory, which improves the codebase structure.
components/intercom/sources/new-user-reply/new-user-reply.mjs (2)
8-8: LGTM: Version bump is appropriate.
The version increment from "0.0.4" to "0.0.5" follows semantic versioning and is appropriate for the non-breaking changes made.
1-1: Verify the new common module path exists.
The import path has been updated to reflect a new directory structure. Let's verify this path exists across the codebase.
✅ Verification successful
Import path change is correctly implemented and widely used
The new path ../common/common.mjs exists and is consistently used across all Intercom source components, confirming this is a valid structural change that's been properly implemented throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and usage of the new common module path
# Test 1: Check if the new common module exists
fd --type f "common.mjs$" components/intercom/
# Test 2: Check if other files are using the same import pattern
rg "import.*from.*common/common.mjs" components/intercom/
Length of output: 1676
components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs
Show resolved
Hide resolved
components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs
Show resolved
Hide resolved
components/intercom/sources/new-conversation-rating-added/new-conversation-rating-added.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @michelle0927 lgtm! Ready for QA!
Resolves #14363
Summary by CodeRabbit
Release Notes
New Features
Improvements
0.0.5, enhancing overall functionality.Bug Fixes
Chores