[Components] engage #13410 #15656
[Components] engage #13410 #15656
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Context
participant A as Action Module
participant E as Engage App Module
U->>A: Trigger action (e.g., create user, add customer, add event)
A->>E: Call corresponding API method
E-->>A: Return response data
A-->>U: Return summary message and response
Possibly related PRs
Suggested labels
Suggested reviewers
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/engage/actions/add-customer/add-customer.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/engage/actions/add-event/add-event.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/engage/actions/create-user/create-user.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
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: 4
🧹 Nitpick comments (2)
components/engage/engage.app.mjs (1)
41-50: Avoid duplicate code in options functions.The
optionsfunctions inuidandcustomerIdproperties are identical. Consider extracting this into a shared method.+ methods: { + async _getUserOptions() { + const response = await this.getUsers(); + const usersIds = response.data; + return usersIds.map(({ + uid, first_name, last_name, + }) => ({ + label: `${first_name} ${last_name}`, + value: uid, + })); + }, }, propDefinitions: { uid: { // ... async options() { - const response = await this.getUsers(); - const usersIds = response.data; - return usersIds.map(({ - uid, first_name, last_name, - }) => ({ - label: `${first_name} ${last_name}`, - value: uid, - })); + return this._getUserOptions(); }, }, customerId: { // ... async options() { - const response = await this.getUsers(); - const usersIds = response.data; - return usersIds.map(({ - uid, first_name, last_name, - }) => ({ - label: `${first_name} ${last_name}`, - value: uid, - })); + return this._getUserOptions(); }, }, },Also applies to: 56-65
components/engage/actions/add-event/add-event.mjs (1)
36-47: Add timestamp validation and default handling.The timestamp field should be validated if provided, or default to current time if not provided.
async run({ $ }) { + const eventData = { + event: this.event, + properties: this.properties, + }; + + if (this.timestamp) { + // Validate timestamp format + if (!Date.parse(this.timestamp)) { + throw new Error("Invalid timestamp format"); + } + eventData.timestamp = this.timestamp; + } + const response = await this.app.addEvent({ $, uid: this.uid, - data: { - event: this.event, - timestamp: this.timestamp, - properties: this.properties, - }, + data: eventData, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
components/engage/.gitignore(0 hunks)components/engage/actions/add-customer/add-customer.mjs(1 hunks)components/engage/actions/add-event/add-event.mjs(1 hunks)components/engage/actions/create-user/create-user.mjs(1 hunks)components/engage/app/engage.app.ts(0 hunks)components/engage/engage.app.mjs(1 hunks)components/engage/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- components/engage/app/engage.app.ts
- components/engage/.gitignore
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (2)
components/engage/package.json (2)
3-5: Version and Main Entry Update Validation
- The package version has been updated to
"0.1.0", which signifies a new release. Please ensure that any breaking changes or new features introduced are documented in the changelog or release notes.- The
"main"entry point has been updated to"engage.app.mjs". Confirm that this file exists in the project directory and that all module consumers are updated accordingly.
14-16: PublishConfig and Dependencies Block Review
- The
"publishConfig"block now correctly terminates, ensuring proper package publishing settings.- The new dependency
"@pipedream/platform": "^3.0.3"is added. Make sure that this dependency is intended for use in the project and is compatible with the rest of the codebase.
| async _makeRequest(opts = {}) { | ||
| const { | ||
| $ = this, | ||
| path, | ||
| auth, | ||
| ...otherOpts | ||
| } = opts; | ||
| return axios($, { | ||
| ...otherOpts, | ||
| url: this._baseUrl() + path, | ||
| auth: { | ||
| ...auth, | ||
| username: `${this.$auth.public_key}`, | ||
| password: `${this.$auth.secret_key}`, | ||
| }, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling and request validation.
The _makeRequest method should include:
- Error handling for network failures
- Request validation
- Rate limiting considerations
async _makeRequest(opts = {}) {
const {
$ = this,
path,
auth,
...otherOpts
} = opts;
+ if (!path) {
+ throw new Error("Path is required");
+ }
+ try {
return axios($, {
...otherOpts,
url: this._baseUrl() + path,
auth: {
...auth,
username: `${this.$auth.public_key}`,
password: `${this.$auth.secret_key}`,
},
});
+ } catch (error) {
+ const errorMessage = error.response?.data?.message || error.message;
+ throw new Error(`Request failed: ${errorMessage}`);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| properties: { | ||
| type: "object", | ||
| label: "Properties", | ||
| description: "The properties of the event", | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding schema validation for properties object.
The properties field accepts any object structure. Consider adding schema validation to ensure the properties match expected format.
🌐 Web query:
What is the expected format for event properties in Engage.so?
💡 Result:
In Engage.so, event properties are structured as key-value pairs associated with user actions, allowing granular tracking and segmentation. Here's the breakdown based on available documentation:
Event Properties Format
-
Event Structure
Each event consists of:- Event Name: A descriptive label for the user action (e.g.,
button_clicked,order_placed). - Properties: Customizable key-value pairs providing context (e.g.,
plan_type: "premium",button_color: "blue").
- Event Name: A descriptive label for the user action (e.g.,
-
Data Types
Events support dynamic property values, including:- Strings (
"success") - Numbers (
42) - Booleans (
true/false) - Timestamps (automatically recorded for each event)[10].
- Strings (
-
Usage Guidelines
- Timestamps: Automatically attached to events for historical analysis[10].
- Overwriting Behavior: Unlike attributes (which store only the latest value), events retain all historical instances[10].
- Segmentation: Properties enable filtering users based on past actions (e.g., "users who clicked ‘Upgrade’ in the last 30 days")[10].
Example Implementation
// Tracking a plan upgrade event with properties
Engage.track('plan_upgraded', {
plan_name: "Pro",
previous_plan: "Basic",
payment_method: "credit_card"
});Key Differences from Attributes
| Feature | Events | Attributes |
|---|---|---|
| Storage | Historical records with timestamps[10] | Latest value only[10] |
| Use Case | Tracking actions over time | Storing static user traits |
| Example | plan_upgraded with details |
current_plan: "Pro" |
For optimal results, use events when you need to analyze temporal patterns or count occurrences (e.g., logins per week), and attributes for fixed user states (e.g., subscription tier)[10].
Citations:
- 1: https://support.higherlogic.com/hc/en-us/articles/360036208371-Define-Event-Properties
- 2: https://www.brightedge.com/blog/live-event-structure-data-markup
- 3: https://knowledgebase.webengage.com/docs/events-and-event-attributes
- 4: https://segment.com/docs/engage/using-engage-data/
- 5: https://developers.google.com/search/docs/appearance/structured-data/event
- 6: https://docs.precisely.com/docs/sftw/e1-compose-designer/6.6.11/en-us/pdf/engageone-designer-v6-6-11-users-guide.pdf
- 7: https://support.pendo.io/hc/en-us/articles/7710433678619-Event-properties
- 8: https://segment.com/docs/connections/sources/catalog/cloud-apps/engage-events/
- 9: https://developers.moengage.com/hc/en-us/articles/12513847534868-Tracking-Events
- 10: https://docs.engage.so/en-us/a/62d2bf84a168cece13b23cb3-understanding-the-data-typesattributes-and-events
Action Required: Enhance Schema Validation for Event Properties
The current definition for the properties field only specifies it as an object without enforcing the key-value structure demanded by Engage.so. According to the documentation, event properties should be dynamic key-value pairs with values limited to strings, numbers, booleans, or timestamps. Please add schema validation (for example, using JSON Schema with appropriate patternProperties or additionalProperties constraints) to ensure that properties strictly adhere to this format.
- Location:
components/engage/engage.app.mjs(lines 77-81) - Action: Enhance the validation logic for the
propertiesobject by enforcing allowed types and structure.
| async run({ $ }) { | ||
| const response = await this.app.createUser({ | ||
| $, | ||
| data: { | ||
| id: this.userId, | ||
| first_name: this.firstName, | ||
| last_name: this.lastName, | ||
| is_account: this.isAccount, | ||
| number: this.number, | ||
| email: this.email, | ||
| }, | ||
| }); | ||
| $.export("$summary", `Successfully created user with ID: ${response.id}`); | ||
| return response; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and error handling.
The run method should validate inputs and handle potential errors from the API call.
async run({ $ }) {
+ // Validate required fields
+ if (!this.userId || !this.email) {
+ throw new Error("User ID and email are required fields");
+ }
+
+ try {
const response = await this.app.createUser({
$,
data: {
id: this.userId,
first_name: this.firstName,
last_name: this.lastName,
is_account: this.isAccount,
number: this.number,
email: this.email,
},
});
$.export("$summary", `Successfully created user with ID: ${response.id}`);
return response;
+ } catch (error) {
+ throw new Error(`Failed to create user: ${error.message}`);
+ }
}📝 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 run({ $ }) { | |
| const response = await this.app.createUser({ | |
| $, | |
| data: { | |
| id: this.userId, | |
| first_name: this.firstName, | |
| last_name: this.lastName, | |
| is_account: this.isAccount, | |
| number: this.number, | |
| email: this.email, | |
| }, | |
| }); | |
| $.export("$summary", `Successfully created user with ID: ${response.id}`); | |
| return response; | |
| }, | |
| async run({ $ }) { | |
| // Validate required fields | |
| if (!this.userId || !this.email) { | |
| throw new Error("User ID and email are required fields"); | |
| } | |
| try { | |
| const response = await this.app.createUser({ | |
| $, | |
| data: { | |
| id: this.userId, | |
| first_name: this.firstName, | |
| last_name: this.lastName, | |
| is_account: this.isAccount, | |
| number: this.number, | |
| email: this.email, | |
| }, | |
| }); | |
| $.export("$summary", `Successfully created user with ID: ${response.id}`); | |
| return response; | |
| } catch (error) { | |
| throw new Error(`Failed to create user: ${error.message}`); | |
| } | |
| }, |
| async run({ $ }) { | ||
| const response = await this.app.addCustomer({ | ||
| $, | ||
| uid: this.uid, | ||
| data: { | ||
| accounts: [ | ||
| { | ||
| id: this.customerId, | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| $.export("$summary", "add-customer executed successfully"); | ||
| return response; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for customer ID and improve error handling.
The run method should validate inputs and handle potential errors from the API call.
async run({ $ }) {
+ if (!this.uid || !this.customerId) {
+ throw new Error("User ID and Customer ID are required");
+ }
+
+ try {
const response = await this.app.addCustomer({
$,
uid: this.uid,
data: {
accounts: [
{
id: this.customerId,
},
],
},
});
$.export("$summary", "add-customer executed successfully");
return response;
+ } catch (error) {
+ throw new Error(`Failed to add customer: ${error.message}`);
+ }
}📝 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 run({ $ }) { | |
| const response = await this.app.addCustomer({ | |
| $, | |
| uid: this.uid, | |
| data: { | |
| accounts: [ | |
| { | |
| id: this.customerId, | |
| }, | |
| ], | |
| }, | |
| }); | |
| $.export("$summary", "add-customer executed successfully"); | |
| return response; | |
| }, | |
| async run({ $ }) { | |
| if (!this.uid || !this.customerId) { | |
| throw new Error("User ID and Customer ID are required"); | |
| } | |
| try { | |
| const response = await this.app.addCustomer({ | |
| $, | |
| uid: this.uid, | |
| data: { | |
| accounts: [ | |
| { | |
| id: this.customerId, | |
| }, | |
| ], | |
| }, | |
| }); | |
| $.export("$summary", "add-customer executed successfully"); | |
| return response; | |
| } catch (error) { | |
| throw new Error(`Failed to add customer: ${error.message}`); | |
| } | |
| }, |
WHY
Summary by CodeRabbit
New Features
Chores