-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - kustomer #15989
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
New Components - kustomer #15989
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces new modules for interacting with the Kustomer API. It adds actions to create and update both conversations and customers with detailed property configurations, error handling, and data parsing via utility functions. Additionally, a set of common constants and utility methods have been implemented to standardize operations. The application module now includes extensive property definitions and API methods. Several source modules have also been introduced for emitting events (new conversation, new customer, new message, updated conversation, updated customer), along with corresponding test event files. Lastly, the package configuration was updated for versioning and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Action as CreateConversation Action
participant Utils as parseObject Utility
participant API as Kustomer API
Action->>Utils: Parse input fields (tags, assignedUsers, assignedTeams)
Action->>API: Call createConversation with parsed data
API-->>Action: Return conversation ID or error
sequenceDiagram
participant Source as NewConversation Source
participant Common as Common Base Module
participant Emitter as Event Emitter
Source->>Common: Invoke getEventType() & getSummary()
Common->>Emitter: Emit event with details
Emitter-->>Source: Confirm event emission
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
components/kustomer/common/utils.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 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 (
|
Sources - New Conversation (Instant) - New Customer (Instant) - New Message (Instant) - Updated Conversation (Instant) - Updated Customer (Instant) Actions - Create Conversation - Update Conversation - Create Customer - Update Customer
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 @luancazarine lgtm! Ready for QA!
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: 11
🧹 Nitpick comments (12)
components/kustomer/common/constants.mjs (1)
1-22
: Consider using numeric values for SENTIMENT_OPTIONS.The SENTIMENT_OPTIONS array contains string values ("-1", "0", "1") rather than numeric values. This might cause type conversion issues if used in numeric comparisons or calculations.
export const SENTIMENT_OPTIONS = [ - "-1", - "0", - "1", + -1, + 0, + 1, ];components/kustomer/actions/create-conversation/create-conversation.mjs (1)
92-117
: Consider enhancing error handling with more context.While the current error handling catches and throws the error message, it would be helpful to add more context about the operation that failed.
async run({ $ }) { try { const response = await this.kustomer.createConversation({ $, data: { customer: this.customer, externalId: this.externalId, name: this.name, status: this.status, priority: this.priority, direction: this.direction, tags: parseObject(this.tags), assignedUsers: parseObject(this.assignedUsers), assignedTeams: parseObject(this.assignedTeams), defaultLang: this.defaultLang, queue: { id: this.queueId, }, }, }); $.export("$summary", `Created conversation with ID ${response.data.id}`); return response; } catch ({ message }) { - throwError(message); + throwError(`Failed to create conversation: ${message}`); } },components/kustomer/actions/create-customer/create-customer.mjs (2)
148-153
: Simplify sentiment object creation.The current approach for creating the sentiment object is somewhat complex. Consider using a more concise approach.
async run({ $ }) { try { - const sentiment = {}; - if (this.sentimentConfidence) sentiment.confidence = parseInt(this.sentimentConfidence); - if (this.sentimentPolarity) sentiment.polarity = parseInt(this.sentimentPolarity); + const sentiment = { + ...(this.sentimentConfidence && { confidence: parseInt(this.sentimentConfidence) }), + ...(this.sentimentPolarity && { polarity: parseInt(this.sentimentPolarity) }), + };
174-182
: Clarify the purpose of string conversion for phone numbers.The code uses template literals (
${phone}
) to convert phone numbers to strings. The purpose of this conversion should be made clearer with a comment.phones: parseObject(this.phones)?.map((phone) => ({ - phone: `${phone}`, + phone: `${phone}`, // Ensure phone is treated as a string })), sharedPhones: parseObject(this.sharedPhones)?.map((phone) => ({ - phone: `${phone}`, + phone: `${phone}`, // Ensure phone is treated as a string })), whatsapps: parseObject(this.whatsApps)?.map((phone) => ({ - phone: `${phone}`, + phone: `${phone}`, // Ensure phone is treated as a string })),components/kustomer/sources/new-conversation-instant/test-event.mjs (2)
1-192
: Test event structure looks comprehensive but has placeholder IDs that should be clarified.The test event structure provides a detailed representation of a Kustomer conversation creation event, which is excellent for testing. However, I noticed that all IDs (e.g., on lines 3, 32-34, 48-49, 52-54) use the same placeholder value "67dd47dfbc632c1804db1bf1". While this works for testing, it might be helpful to:
- Add a comment indicating these are placeholder IDs
- Consider using different placeholder IDs for different entities to help distinguish between them during testing
This would make the test event more representative of real-world data and help developers better understand the relationships between different entities.
15-101
: Future dates used in test event might cause confusion.The test event uses dates in 2025 (e.g., lines 16, 25-28, 91-99) for timestamps like
createdAt
,updatedAt
, etc. Using future dates might cause confusion or unexpected behavior in tests that rely on date comparisons.Consider using either:
- More recent dates (e.g., current year)
- A comment explaining why future dates are used
- Variables that generate dates relative to the current time when tests are run
components/kustomer/sources/new-customer-instant/new-customer-instant.mjs (1)
1-24
: Source component looks well-structured but version should follow semantic versioning.The new customer instant source component is well-structured, extending the common base functionality appropriately. The
getEventType()
andgetSummary()
methods are implemented correctly.However, I would recommend following semantic versioning best practices:
- For an initial release that's ready for production use, consider starting with version "1.0.0" instead of "0.0.1"
- If this is truly a pre-release/experimental version, add a comment indicating its status
Semantic versioning would make it clearer to users what to expect in terms of stability and compatibility.
components/kustomer/sources/common/base.mjs (1)
11-15
: The 'name' prop should have a default value.The
name
prop is required for webhook creation but doesn't have a default value. This could lead to errors if users don't provide a name.Consider adding a default value that includes a unique identifier (like the component key) to ensure webhooks are always named properly:
name: { type: "string", label: "Webhook Name", description: "The name of the webhook to be identified in the UI", + default: "Pipedream Kustomer Webhook", },
components/kustomer/sources/new-customer-instant/test-event.mjs (3)
1-111
: Test event structure is comprehensive but has the same placeholder ID issue.Similar to the conversation test event, this customer test event uses the same placeholder ID "67dd47dfbc632c1804db1bf1" throughout the object (e.g., lines 2-5, 9, etc.). Using different IDs for different entities would make the test data more realistic and easier to understand.
Additionally, this test event also uses future dates in 2025 (lines 31-36), which could cause the same potential issues mentioned previously.
6-6
: The 'orgName' property is hardcoded to a specific organization.Line 6 contains a hardcoded organization name "par-pipedream-berta" which appears to be a specific testing organization. This might cause confusion for other users of this component.
Consider:
- Using a more generic name like "Example Organization"
- Adding a comment explaining this is just an example value
10-64
: Consider adding example custom fields in customer attributes.The customer attributes section is comprehensive but doesn't include any example of custom fields that users might have in their Kustomer instance. Since custom fields are common in CRM systems like Kustomer, it would be helpful to show an example of how they appear in the payload.
Consider adding an example custom field like
custom
orcustomAttributes
to help users understand how to access these values.components/kustomer/sources/updated-customer-instant/updated-customer-instant.mjs (1)
7-7
: Rename source to follow Pipedream’s guidelines.
According to the lint warning, source names should start with "New". For example:- name: "Updated Customer (Instant)", + name: "New Updated Customer (Instant)",🧰 Tools
🪛 GitHub Check: Lint Code Base
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
components/kustomer/actions/create-conversation/create-conversation.mjs
(1 hunks)components/kustomer/actions/create-customer/create-customer.mjs
(1 hunks)components/kustomer/actions/update-conversation/update-conversation.mjs
(1 hunks)components/kustomer/actions/update-customer/update-customer.mjs
(1 hunks)components/kustomer/common/constants.mjs
(1 hunks)components/kustomer/common/utils.mjs
(1 hunks)components/kustomer/kustomer.app.mjs
(1 hunks)components/kustomer/package.json
(2 hunks)components/kustomer/sources/common/base.mjs
(1 hunks)components/kustomer/sources/new-conversation-instant/new-conversation-instant.mjs
(1 hunks)components/kustomer/sources/new-conversation-instant/test-event.mjs
(1 hunks)components/kustomer/sources/new-customer-instant/new-customer-instant.mjs
(1 hunks)components/kustomer/sources/new-customer-instant/test-event.mjs
(1 hunks)components/kustomer/sources/new-message-instant/new-message-instant.mjs
(1 hunks)components/kustomer/sources/new-message-instant/test-event.mjs
(1 hunks)components/kustomer/sources/updated-conversation-instant/test-event.mjs
(1 hunks)components/kustomer/sources/updated-conversation-instant/updated-conversation-instant.mjs
(1 hunks)components/kustomer/sources/updated-customer-instant/test-event.mjs
(1 hunks)components/kustomer/sources/updated-customer-instant/updated-customer-instant.mjs
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
components/kustomer/sources/updated-conversation-instant/updated-conversation-instant.mjs
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
components/kustomer/sources/updated-customer-instant/updated-customer-instant.mjs
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
🔇 Additional comments (14)
components/kustomer/package.json (2)
3-3
: Version bump looks appropriate for new component features.The version bump from 0.0.1 to 0.1.0 follows semantic versioning principles for adding new functionality.
14-16
: Dependency configuration properly structured.The dependency on @pipedream/platform is correctly specified with a caret range to allow for compatible updates.
components/kustomer/common/constants.mjs (1)
24-337
: Well-structured language options.The DEFAULT_LANG_OPTIONS array is comprehensive and consistently structured with label/value pairs.
components/kustomer/actions/create-conversation/create-conversation.mjs (1)
85-90
:❓ Verification inconclusive
Verify if queueId should be required.
Unlike other props which are marked as optional, queueId doesn't have the
optional: true
flag, making it a required field. Ensure this is intentional based on Kustomer API requirements.
🏁 Script executed:
#!/bin/bash # Check Kustomer API documentation to see if queueId is required rg -A 3 -B 3 "queue.*required" --type js components/kustomer/Length of output: 63
Action Required: Confirm if queueId Must Be Required According to Kustomer API
The current implementation marks
queueId
as required (i.e. without theoptional: true
flag), unlike other properties. Our automated search in the codebase for any explicit note regarding it being required returned no conclusive results. Please manually verify against the Kustomer API documentation to confirm if this field indeed must be required or if it should be made optional for consistency.
- File: components/kustomer/actions/create-conversation/create-conversation.mjs
- Snippet:
queueId: { propDefinition: [ kustomer, "queueId", ], },- Action: Double-check the Kustomer API docs and update the definition if needed (e.g., add
optional: true
if the field should not be required).components/kustomer/sources/updated-conversation-instant/updated-conversation-instant.mjs (1)
1-24
: LGTM! The source implementation follows best practicesThe component is well-structured and implements the correct methods for an instant source. The warning about source names starting with "New" is a false positive in this case since this source is for "Updated" conversations, not "New" ones.
🧰 Tools
🪛 GitHub Check: Lint Code Base
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-namecomponents/kustomer/sources/new-conversation-instant/new-conversation-instant.mjs (1)
1-24
: LGTM! The source implementation follows best practicesThe component is well-structured and implements the correct methods for an instant source. The name correctly reflects its purpose of detecting new conversation creation.
components/kustomer/sources/new-message-instant/new-message-instant.mjs (1)
4-24
: Module structure looks good and follows the component pattern.The module correctly extends the common base module and implements the necessary methods for handling new message events. The event type is properly defined as "kustomer.message.create" and the getSummary method formats a clear message with the dataId.
components/kustomer/sources/updated-conversation-instant/test-event.mjs (1)
1-250
: Sample test event appears well-structured.This test event provides comprehensive data for testing the updated conversation source. It includes a detailed conversation object with properties, relationships, and change tracking that accurately represents a Kustomer API response.
components/kustomer/sources/new-message-instant/test-event.mjs (1)
1-115
: Sample event provides good test coverage, including error states.The test event includes a realistic error scenario (lines 24-41) with error codes and messages, which is excellent for testing error handling. The message structure includes all necessary fields for testing the integration.
components/kustomer/sources/updated-customer-instant/test-event.mjs (1)
1-222
: Sample event structure provides comprehensive test data.Apart from the syntax issue noted above, the test event provides a thorough representation of an updated customer with detailed attributes, contact information, and change tracking. This is valuable for testing the integration.
components/kustomer/actions/update-customer/update-customer.mjs (3)
1-3
: Imports look good.
6-153
: Comprehensive props configuration.
These prop definitions are consistent with the Kustomer API fields. Great job making them optional unless required.
157-158
: Consider usingparseFloat
if confidence and polarity can be decimal values.
Currently, you are usingparseInt(this.sentimentConfidence)
andparseInt(this.sentimentPolarity)
, which will truncate decimal values. If the Kustomer API accepts fractional sentiment confidences (e.g., 0.95), usingparseInt
may lead to data loss.Do you want me to generate a quick check or query for the Kustomer API docs regarding whether fractional values are accepted for sentiment confidence?
components/kustomer/kustomer.app.mjs (1)
19-25
: Verify pagination logic for listing resources.
Eachoptions
method setspage: page + 1
andpageSize: LIMIT * page
. Whenpage
is 0,pageSize
is 0, possibly returning no results for the first page. You might instead do something like(page + 1) * LIMIT
forpageSize
, or maintain a constant limit regardless of the page to retrieve consistent results.Do you want me to generate a script to call these endpoints with different page values to confirm that you get the expected data?
Also applies to: 39-52, 58-71, 78-91, 97-109, 115-128
/approve |
Resolves #15920.
Summary by CodeRabbit
New Features
Chores