-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - zendesk_sell #14583
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 ↗︎ |
WalkthroughThis pull request introduces several new modules and enhancements to the Zendesk Sell application. It includes modules for creating contacts, leads, and tasks, as well as sources for emitting events related to new contacts, deals, and leads. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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: 8
🧹 Outside diff range and nitpick comments (13)
components/zendesk_sell/sources/new-lead-created/new-lead-created.mjs (2)
7-7
: Consider enhancing the description.The current description could be more specific about what constitutes a "new lead" and any important field information that users should be aware of.
- description: "Emit new event when a new lead is created in Zendesk Sell.", + description: "Emit new event when a lead is created in Zendesk Sell. The event includes the lead's ID and is triggered once per new lead.",
16-18
: Enhance lead summary information and add parameter validation.The summary could be more informative, and the lead parameter should be validated.
- getSummary(lead) { - return `New Lead ID: ${lead.id}`; + getSummary(lead) { + if (!lead?.id) { + throw new Error("Invalid lead object received"); + } + return `New Lead ID: ${lead.id}${lead.name ? ` - ${lead.name}` : ""}`;components/zendesk_sell/sources/new-contact-created/new-contact-created.mjs (1)
16-18
: Consider enhancing the contact summary message.The current summary only includes the contact ID. Consider adding more meaningful information like the contact's name or email to make the event message more user-friendly.
- return `New Contact ID: ${contact.id}`; + return `New Contact Created: ${contact.name || contact.email || contact.id}`;components/zendesk_sell/actions/create-contact/create-contact.mjs (2)
9-48
: Consider adding validation for email and phone formats.While the props are well-structured and properly linked to zendeskSell definitions, consider enhancing validation for the following fields:
- Email: Ensure proper email format validation
- Phone: Add phone number format validation
Example enhancement:
email: { propDefinition: [ zendeskSell, "email", ], + validate: (value) => { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(value)) { + throw new Error("Invalid email format"); + } + }, },
49-70
: Add required field validation for name properties.The dynamic props logic is well-implemented, but consider marking these fields as required since they are essential for contact creation.
if (this.isOrganization) { props.name = { type: "string", label: "Name", description: "Name of the contact", + required: true, }; } else { props.firstName = { type: "string", label: "First Name", description: "First name of the contact", + required: true, }; props.lastName = { type: "string", label: "Last Name", description: "Last name of the contact", + required: true, }; }components/zendesk_sell/actions/create-task/create-task.mjs (3)
1-8
: LGTM! Consider starting with version 1.0.0The configuration is well-structured with proper documentation links. However, since this is a new component being released as part of a larger integration, consider starting with version 1.0.0 instead of 0.0.1 to indicate production readiness.
48-53
: Consider adding a default value for the completed flagThe
completed
boolean field might benefit from a default value to improve user experience.completed: { type: "boolean", label: "Completed", description: "Indicator of whether the task is completed or not", optional: true, + default: false, },
61-66
: Optimize additionalProps implementationThe current implementation can be simplified by removing the unnecessary empty object return.
async additionalProps(props) { props.contactId.hidden = this.resourceType !== "contact"; props.leadId.hidden = this.resourceType !== "lead"; props.dealId.hidden = this.resourceType !== "deal"; - return {}; },
components/zendesk_sell/actions/create-lead/create-lead.mjs (1)
9-54
: Consider adding input validation for email and phone fields.While the props are well-defined, adding validation patterns would improve data quality:
- Email format validation
- Phone number format validation (considering international formats)
Example improvement:
email: { propDefinition: [ zendeskSell, "email", ], description: "The lead's email address", + pattern: "^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", + errorMessage: "Please enter a valid email address", }, phone: { propDefinition: [ zendeskSell, "phone", ], description: "The lead's phone number", + pattern: "^\\+?[1-9]\\d{1,14}$", + errorMessage: "Please enter a valid phone number (E.164 format preferred)", },components/zendesk_sell/sources/common/base.mjs (2)
71-71
: Simplify the condition by removing unnecessary optional chainingSince
items
is always initialized as an array, the optional chaining operator?.
is unnecessary in this context.Apply this diff to simplify the condition:
- if (!items?.length) { + if (!items.length) {
63-63
: Ensure consistent date parsing by specifying the date formatUsing
Date.parse
can lead to inconsistencies across different environments. To ensure reliable parsing, consider specifying the date format explicitly.Modify the date parsing like this:
- const ts = Date.parse(item[tsField]); + const ts = new Date(item[tsField]).getTime();And
- this._setLastTs(Date.parse(items[0][tsField])); + this._setLastTs(new Date(items[0][tsField]).getTime());Also applies to: 75-75
components/zendesk_sell/zendesk_sell.app.mjs (2)
103-103
: Use standard header capitalization for 'Accept' headerHTTP header field names are case-insensitive, but it's conventional to use 'Accept' with an uppercase 'A' for consistency.
Apply this diff to adjust the header key:
headers: { Authorization: `Bearer ${this.$auth.oauth_access_token}`, - accept: "application/json", + Accept: "application/json", },
168-170
: Ensure pagination continues correctly when there are no more itemsThe
total
variable may be undefined ifitems
is undefined. Assign a default value to prevent possible issues with the loop condition.Apply this diff:
} - total = items?.length; + total = items?.length || 0; params.page++; } while (total);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
components/zendesk_sell/actions/create-contact/create-contact.mjs
(1 hunks)components/zendesk_sell/actions/create-lead/create-lead.mjs
(1 hunks)components/zendesk_sell/actions/create-task/create-task.mjs
(1 hunks)components/zendesk_sell/package.json
(2 hunks)components/zendesk_sell/sources/common/base.mjs
(1 hunks)components/zendesk_sell/sources/new-contact-created/new-contact-created.mjs
(1 hunks)components/zendesk_sell/sources/new-deal-created/new-deal-created.mjs
(1 hunks)components/zendesk_sell/sources/new-lead-created/new-lead-created.mjs
(1 hunks)components/zendesk_sell/zendesk_sell.app.mjs
(1 hunks)
🔇 Additional comments (17)
components/zendesk_sell/package.json (2)
3-3
: LGTM! Version bump follows semantic versioning.
The version increment from 0.0.1 to 0.1.0 appropriately reflects the addition of new features (contacts, leads, tasks, and event sources) while maintaining backward compatibility.
15-17
: LGTM! Dependencies are correctly specified.
The @pipedream/platform dependency is properly defined with a caret range (^3.0.3) allowing for compatible minor version updates.
Let's verify this is the latest compatible version:
✅ Verification successful
Based on the search results, I can see that version 3.0.3 of @pipedream/platform is being used consistently across many recently updated components, including the zendesk_sell component. This appears to be the latest stable version in active use.
The dependency version ^3.0.3 is appropriate and matches the current ecosystem standard.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are newer compatible versions of @pipedream/platform
# that might include important bug fixes or security patches.
# Search for any package.json files using this dependency
rg -l "@pipedream/platform.*3\.0\.3" --type json
# Look for other versions of @pipedream/platform in use
rg "@pipedream/platform.*\d+\.\d+\.\d+" --type json
Length of output: 95803
components/zendesk_sell/sources/new-deal-created/new-deal-created.mjs (4)
1-1
: LGTM: Import statement is correct
The import statement correctly references the common base module using relative path navigation.
11-19
: Consider enhancing error handling and deal summary information
The methods implementation could benefit from the following improvements:
- Add error handling in
getResourceFn
- Enhance
getSummary
with more deal information (e.g., name, value) - Add parameter validation
Consider this enhancement:
methods: {
...common.methods,
getResourceFn() {
+ if (!this.zendeskSell?.listDeals) {
+ throw new Error("listDeals method not available");
+ }
return this.zendeskSell.listDeals;
},
getSummary(deal) {
+ if (!deal?.id) {
+ throw new Error("Invalid deal object");
+ }
- return `New Deal ID: ${deal.id}`;
+ return `New Deal ID: ${deal.id}${deal.name ? ` - ${deal.name}` : ''}`;
},
},
#!/bin/bash
# Description: Verify the existence and usage of listDeals method
# Expected: Find references to listDeals method definition
echo "Checking for listDeals method definition:"
rg -A 5 "listDeals.*=.*function" components/zendesk_sell/
1-20
: Verify alignment with source component patterns
As this is part of the Zendesk Sell integration, let's ensure it follows the same patterns as other source components.
#!/bin/bash
# Description: Check for consistency in source component implementations
# Expected: Similar structure across source components
echo "Checking source component patterns:"
# Check for similar source component structures
fd -e mjs$ . components/zendesk_sell/sources -x ast-grep --pattern $'export default {
...common,
key: $_,
name: $_,
description: $_,
type: "source",
version: $_,
dedupe: $_,
methods: {
$$$
}
}'
3-10
: Verify version number consistency
The component metadata looks good, but let's verify the version number aligns with other Zendesk Sell components.
✅ Verification successful
Let me try another search pattern to find version information in Zendesk Sell components.
Let me try one more time with a modified search pattern that should work better:
Version number is consistent with other Zendesk Sell components
The version "0.0.1" is consistent across all Zendesk Sell components, including both sources and actions. All components are at the same initial version, which is appropriate for this component ecosystem.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version numbers of other Zendesk Sell components
# Expected: Version numbers should be consistent across related components
echo "Checking version numbers in other Zendesk Sell components:"
rg '"version":\s*"[0-9]+\.[0-9]+\.[0-9]+"' components/zendesk_sell/ || \
rg 'version:\s*"[0-9]+\.[0-9]+\.[0-9]+"' components/zendesk_sell/
Length of output: 299
Script:
#!/bin/bash
# Search for version field in Zendesk Sell components with more context
rg -A 1 -B 1 'version:' components/zendesk_sell/ --type js --type mjs
Length of output: 97
Script:
#!/bin/bash
# Search for version field in Zendesk Sell components
rg -A 1 -B 1 'version:' components/zendesk_sell/
Length of output: 1590
components/zendesk_sell/sources/new-lead-created/new-lead-created.mjs (3)
1-1
: LGTM! Clean import statement.
The import follows proper ES module conventions and uses clear naming.
3-10
: LGTM! Well-structured component definition.
The component metadata follows best practices with clear naming and proper structure.
13-15
: Verify the listLeads method and add error handling.
The getResourceFn
implementation needs verification and could benefit from error handling.
Let's verify the listLeads
method exists in the Zendesk Sell app:
components/zendesk_sell/sources/new-contact-created/new-contact-created.mjs (3)
3-10
: LGTM! Well-structured component metadata.
The component metadata is well-defined with:
- Clear naming and description
- Proper key format following
service-action
pattern - Appropriate initial version
- Correct deduplication strategy for contact events
13-15
: Verify zendeskSell instance initialization.
The getResourceFn
method assumes this.zendeskSell
is properly initialized and has a listContacts
method.
#!/bin/bash
# Description: Verify the zendeskSell instance setup and listContacts method
# Test 1: Check for zendeskSell initialization in base or related files
rg -l "zendeskSell" "components/zendesk_sell/"
# Test 2: Look for listContacts method definition or usage
ast-grep --pattern 'listContacts = $_' "components/zendesk_sell/"
1-1
: Verify the common base module implementation.
Let's ensure the base module exists and contains the expected shared functionality for Zendesk Sell sources.
✅ Verification successful
Base module import is correctly implemented
The common base module exists and provides the expected shared functionality for Zendesk Sell sources, including:
- Standard props configuration (zendeskSell app, db, timer)
- Common methods for timestamp handling and pagination
- Base event processing logic
- Abstract methods (getResourceFn, getSummary) for source-specific implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and content of the common base module
# Test 1: Check if the base module exists
if ! fd -p "components/zendesk_sell/sources/common/base.mjs$"; then
echo "Error: Common base module not found"
exit 1
fi
# Test 2: Examine the base module content
rg -A 10 "export default" "components/zendesk_sell/sources/common/base.mjs"
Length of output: 654
Script:
#!/bin/bash
# Let's try a different approach to verify the base module
# Check the content of the base module
cat components/zendesk_sell/sources/common/base.mjs
# If the above doesn't show content, let's try to find any exports or class definitions
rg -A 5 "export|class" components/zendesk_sell/sources/common/base.mjs
Length of output: 2151
components/zendesk_sell/actions/create-contact/create-contact.mjs (1)
1-8
: LGTM! Documentation link should be verified.
The module definition is well-structured with appropriate metadata. The documentation link in the description is properly formatted.
Let's verify the documentation link:
✅ Verification successful
Let me verify if this module's version and key naming pattern is consistent with other Zendesk Sell components in the codebase.
Let me try a different approach to verify the module structure:
Let me check the package.json and other action files to verify version consistency:
Module structure and versioning are consistent
The initial review was correct. The findings show:
- All action modules follow the same structure with consistent
key
naming pattern (prefixed with "zendesk_sell-") - All actions use version "0.0.1" which is appropriate as these are new components
- The documentation link format is consistent across all actions
- The module properly imports the app file which contains all the necessary API methods
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Extract and verify the documentation URL
DOC_URL=$(rg -o 'https://developer\.zendesk\.com/api-reference/sales-crm/resources/contacts/#create-a-contact' components/zendesk_sell/actions/create-contact/create-contact.mjs)
if [ ! -z "$DOC_URL" ]; then
echo "Found documentation URL: $DOC_URL"
# Curl would be used here to verify the URL if we had internet access
fi
Length of output: 619
Script:
#!/bin/bash
# Check other Zendesk Sell component versions and keys
rg '"version":.*"0\.0\.1"' components/zendesk_sell/ -A 1 -B 2
rg '"key": "zendesk_sell-' components/zendesk_sell/ -A 1 -B 2
Length of output: 127
Script:
#!/bin/bash
# List all files in zendesk_sell directory to understand the component structure
fd . components/zendesk_sell/
# Check the app file for version patterns
cat components/zendesk_sell/zendesk_sell.app.mjs
Length of output: 4957
Script:
#!/bin/bash
# Check package.json for version
cat components/zendesk_sell/package.json
# Check versions and keys in other action files
cat components/zendesk_sell/actions/create-lead/create-lead.mjs
cat components/zendesk_sell/actions/create-task/create-task.mjs
Length of output: 5067
components/zendesk_sell/actions/create-task/create-task.mjs (1)
67-85
: Simplify resource_id selection and verify error handling
The resource ID selection can be simplified using an object lookup. Also, we should verify the error handling implementation in the createTask method.
async run({ $ }) {
+ const resourceIdMap = {
+ contact: this.contactId,
+ lead: this.leadId,
+ deal: this.dealId,
+ };
const response = await this.zendeskSell.createTask({
$,
data: {
data: {
resource_type: this.resourceType,
- resource_id: this.resourceType === "contact"
- ? this.contactId
- : this.resourceType === "lead"
- ? this.leadId
- : this.dealId,
+ resource_id: resourceIdMap[this.resourceType],
content: this.content,
completed: this.completed,
due_date: this.dueDate,
},
},
});
return response;
},
Let's verify the error handling implementation:
✅ Verification successful
Let me gather more information about the error handling implementation by checking the Zendesk Sell app file and HTTP client usage.
Let me check the platform's axios implementation to understand the error handling.
Based on the analysis of the codebase, I can now provide the final response:
Error handling is properly implemented, resource ID selection can be simplified
The error handling is properly implemented through multiple layers:
- The
createTask
method uses the platform's axios client which handles HTTP errors - The
_makeRequest
method in the Zendesk Sell app provides base error handling - Errors are propagated up to the action level for proper user feedback
The suggested resource ID selection optimization is valid but optional:
async run({ $ }) {
+ const resourceIdMap = {
+ contact: this.contactId,
+ lead: this.leadId,
+ deal: this.dealId,
+ };
const response = await this.zendeskSell.createTask({
$,
data: {
data: {
resource_type: this.resourceType,
- resource_id: this.resourceType === "contact"
- ? this.contactId
- : this.resourceType === "lead"
- ? this.leadId
- : this.dealId,
+ resource_id: resourceIdMap[this.resourceType],
content: this.content,
completed: this.completed,
due_date: this.dueDate,
},
},
});
return response;
},
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check error handling in createTask method
# Expected: Find error handling implementation in the app file
# Look for createTask method implementation
ast-grep --pattern 'createTask($$$) {
$$$
}'
# Check for try-catch blocks or error handling
rg -A 5 'catch|throw|error' components/zendesk_sell/
Length of output: 831
Script:
#!/bin/bash
# Check the Zendesk Sell app implementation
fd "zendesk.*\.(mjs|js)" components/zendesk_sell/ --exec cat {}
# Look for HTTP client implementation or error handling in the app
rg -A 5 "class ZendeskSell|_makeRequest|createTask" components/zendesk_sell/
Length of output: 7422
Script:
#!/bin/bash
# Check the platform's axios implementation
rg -A 10 "export.*axios" -g "platform.{js,mjs,ts}"
# Check for any global error handling
fd "index|app" --extension js --extension mjs --extension ts --exec rg -A 5 "catch|error.*handler" {}
Length of output: 29690
components/zendesk_sell/actions/create-lead/create-lead.mjs (2)
1-8
: LGTM! Well-structured action definition with proper documentation.
The action definition follows best practices with clear naming, proper versioning for a new component, and includes relevant API documentation links.
78-92
: Verify API payload structure with Zendesk Sell documentation.
Please verify that the API payload structure matches the Zendesk Sell API documentation requirements.
✅ Verification successful
Based on the code analysis, I can now provide a final response:
API payload structure is correctly implemented
The implementation matches the Zendesk Sell API requirements:
- The payload is properly nested under
data.data
structure - All field names (
first_name
,last_name
,organization_name
,status
,title
,description
,email
,phone
) align with the API field specifications - The API endpoint is correctly set to
/leads
in the app implementation - The authorization and content-type headers are properly configured in the base request method
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other Zendesk Sell API calls to compare payload structure
rg -l "zendeskSell.*createLead" --type js
Length of output: 43
Script:
#!/bin/bash
# Let's try a broader search for Zendesk Sell API related files and patterns
fd -e mjs -e js . | xargs cat | grep -A 10 "zendeskSell"
# Also search for any SDK or API client initialization
rg -A 5 "class ZendeskSell" --type js
Length of output: 6040
Script:
#!/bin/bash
# Let's look for the Zendesk Sell app implementation to verify the API client
fd zendesk_sell.app.mjs | xargs cat
# Also search for any API documentation references
rg -A 2 "https://developer.zendesk.com/api-reference/sales-crm/resources/leads" --type js
Length of output: 4073
components/zendesk_sell/sources/common/base.mjs (1)
42-47
: Implement getResourceFn
and getSummary
in derived classes
The methods getResourceFn
and getSummary
are currently throwing errors. Ensure these methods are implemented in all subclasses to prevent runtime exceptions.
Please confirm that all subclasses properly implement these methods.
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!
* new sources * new actions * pnpm-lock.yaml
Resolves #14559
Summary by CodeRabbit
Release Notes
New Features
Improvements
Version Update