New Components - elastic_email #15692
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces several new modules for the Elastic Email integration. It adds actions for adding contacts, sending emails, and unsubscribing contacts, along with new event source modules for tracking new contacts, email clicks, and email opens. A new app module with comprehensive API methods replaces an older TypeScript version, and supporting utility and constants modules have been added. Additionally, outdated files (the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as Action Module (e.g., Add Contact)
participant E as Elastic Email App
participant API as Elastic Email API
U->>A: Trigger Action
A->>E: Call corresponding method (e.g., addContact)
E->>API: Send API Request
API-->>E: Return Response
E-->>A: Return result
A-->>U: Provide summary message
sequenceDiagram
participant API as Elastic Email API
participant B as Base Module
participant S as Event Source (e.g., New Contact Added)
participant DB as Database
API->>B: Return events (paginated)
B->>DB: Retrieve/Update last processed timestamp
B->>S: Filter and process event data
S-->>B: Emit formatted event
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/elastic_email/elastic_email.app.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 (5)
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 (
|
Sources - New Email Open - New Email Click - New Contact Added Actions - Send Email - Add Contact - Unsubscribe Contact
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
components/elastic_email/sources/new-email-open/test-event.mjs (1)
13-14: Generic placeholder values.The
IPAddressandPoolNamefields are using generic "string" placeholders. While this works for testing, consider using more realistic sample values to better represent actual data."Message": "Lorem ipsum", - "IPAddress": "string", - "PoolName": "string" + "IPAddress": "192.168.1.1", + "PoolName": "Marketing"components/elastic_email/sources/new-email-click/test-event.mjs (1)
13-14: Generic placeholder values.The
IPAddressandPoolNamefields are using generic "string" placeholders. While this works for testing, consider using more realistic sample values to better represent actual data."Message": "Lorem ipsum", - "IPAddress": "string", - "PoolName": "string" + "IPAddress": "192.168.1.1", + "PoolName": "Marketing"components/elastic_email/sources/new-contact-added/test-event.mjs (1)
6-9: Consider consistent JSON formattingThe indentation in the CustomFields object is inconsistent with the rest of the file. Consider aligning all properties at the same level.
"CustomFields": { - "city": "New York", - "age": "34" + "city": "New York", + "age": "34" },components/elastic_email/actions/send-email/send-email.mjs (1)
33-33: Fix example format in descriptionThe description contains empty key-value pairs in the example which should be removed or corrected.
- description: "List of e-mail body parts, with user-provided MIME types (text/html, text/plain etc). **Format: {\"ContentType\": \"HTML\", \"Content\": \"email content\", \"\": \"\"}**", + description: "List of e-mail body parts, with user-provided MIME types (text/html, text/plain etc). **Format: {\"ContentType\": \"HTML\", \"Content\": \"email content\"}**",components/elastic_email/sources/new-email-click/new-email-click.mjs (1)
4-30: Consider enhancing the summary with more contextThe implementation for email click tracking looks good overall. However, the
getSummary()method returns a static string "New event click" without any details about the specific email that was clicked.Consider enhancing the summary to include more context, similar to how it's done in the new-contact-added source:
- getSummary() { - return "New event click"; - }, + getSummary(item) { + return `New email click: ${item.Subject || item.MsgID}`; + },This would make the event notifications more informative for users.
components/elastic_email/sources/new-email-open/new-email-open.mjs (1)
4-30: Consider enhancing the summary with email detailsThe implementation for email open tracking is well-structured and follows a consistent pattern. However, similar to the email click source, the
getSummary()method returns a static string without any details about the specific email that was opened.Consider enhancing the summary to include more context:
- getSummary() { - return "New Email opened"; - }, + getSummary(item) { + return `Email opened: ${item.Subject || item.MsgID}`; + },This would provide more informative event notifications to users.
components/elastic_email/sources/common/base.mjs (1)
28-63: Add error handling to the emitEvent method.The
emitEventmethod lacks error handling for API calls and data processing, which could lead to unhandled exceptions if the API returns errors or if the data format is unexpected.Add try/catch blocks to handle potential errors:
async emitEvent(maxResults = false) { + try { const lastDate = this._getLastDate(); const dateField = this.getDateField(); const idField = this.getIdField(); const response = this.app.paginate({ fn: this.getFunction(), maxResults, params: { from: lastDate, }, }); let responseArray = []; for await (const item of response) { const eventType = this.getEventType(); if (eventType && !eventType.includes(item.EventType)) continue; if (Date.parse(item[dateField]) <= Date.parse(lastDate)) break; responseArray.push(item); } if (responseArray.length) { if (maxResults && (responseArray.length > maxResults)) { responseArray.length = maxResults; } this._setLastDate(responseArray[0][dateField]); } for (const item of responseArray.reverse()) { this.$emit(item, { id: item[idField], summary: this.getSummary(item), ts: Date.parse(item[dateField] || new Date()), }); } + } catch (error) { + console.error("Error emitting events:", error); + throw error; + } },components/elastic_email/elastic_email.app.mjs (5)
51-53: Consider defining the API URL as a constant.The API URL is hardcoded in the
_baseUrl()method. While it's unlikely to change, it might be better to define it as a constant at the file level for better visibility and easier maintenance.+const BASE_URL = "https://api.elasticemail.com/v4"; + export default { type: "app", app: "elastic_email", // ... methods: { _baseUrl() { - return "https://api.elasticemail.com/v4"; + return BASE_URL; }, // ... },
59-67: Add error handling to API requests.The
_makeRequestmethod doesn't include error handling for API responses. Adding explicit error handling would improve the robustness of the integration.Consider adding error handling to the
_makeRequestmethod:_makeRequest({ $ = this, path, ...opts }) { - return axios($, { - url: this._baseUrl() + path, - headers: this._headers(), - ...opts, - }); + return axios($, { + url: this._baseUrl() + path, + headers: this._headers(), + ...opts, + }).catch((error) => { + const status = error.response?.status; + const message = error.response?.data?.message || error.message; + throw new Error(`Elastic Email API error (${status}): ${message}`); + }); },
113-139: Improve the paginate method with additional error handling.The
paginatemethod is well-structured but lacks error handling for API errors during pagination.Enhance the method with try/catch:
async *paginate({ fn, params = {}, maxResults = null, ...opts }) { let hasMore = false; let count = 0; let page = 0; do { params.limit = LIMIT; params.offset = LIMIT * page; - const data = await fn({ - params, - ...opts, - }); + try { + const data = await fn({ + params, + ...opts, + }); + + for (const d of data) { + yield d; + + if (maxResults && ++count === maxResults) { + return count; + } + } + + page++; + hasMore = data.length; + } catch (error) { + console.error(`Error during pagination (page ${page}):`, error); + throw error; + } - for (const d of data) { - yield d; - - if (maxResults && ++count === maxResults) { - return count; - } - } - - page++; - hasMore = data.length; } while (hasMore); },
45-48: Add type validation for email addresses.The
unsubscribeEmailsproperty accepts a string array but doesn't validate that the strings are valid email addresses. Consider adding a pattern or validation function to ensure correct email format.unsubscribeEmails: { type: "string[]", label: "Email Addresses", description: "A list of email addresses to unsubscribe", + validate: (emails) => { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + for (const email of emails) { + if (!emailRegex.test(email)) { + return `Invalid email format: ${email}`; + } + } + return true; + }, },
92-98: Document expected payload format for sendBulkEmails.The
sendBulkEmailsmethod doesn't provide documentation on the expected payload format. Adding JSDoc comments would help users understand how to use this method correctly.+/** + * Send emails in bulk + * @param {Object} opts - Request options + * @param {Object} opts.data - Request payload + * @param {Array<Object>} opts.data.Recipients - List of recipients + * @param {String} opts.data.Content - Email content details + * @return {Promise<Object>} API response + */ sendBulkEmails(opts = {}) { return this._makeRequest({ method: "POST", path: "/emails", ...opts, }); },
📜 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 (16)
components/elastic_email/.gitignore(0 hunks)components/elastic_email/actions/add-contact/add-contact.mjs(1 hunks)components/elastic_email/actions/send-email/send-email.mjs(1 hunks)components/elastic_email/actions/unsubscribe-contact/unsubscribe-contact.mjs(1 hunks)components/elastic_email/app/elastic_email.app.ts(0 hunks)components/elastic_email/common/constants.mjs(1 hunks)components/elastic_email/common/utils.mjs(1 hunks)components/elastic_email/elastic_email.app.mjs(1 hunks)components/elastic_email/package.json(1 hunks)components/elastic_email/sources/common/base.mjs(1 hunks)components/elastic_email/sources/new-contact-added/new-contact-added.mjs(1 hunks)components/elastic_email/sources/new-contact-added/test-event.mjs(1 hunks)components/elastic_email/sources/new-email-click/new-email-click.mjs(1 hunks)components/elastic_email/sources/new-email-click/test-event.mjs(1 hunks)components/elastic_email/sources/new-email-open/new-email-open.mjs(1 hunks)components/elastic_email/sources/new-email-open/test-event.mjs(1 hunks)
💤 Files with no reviewable changes (2)
- components/elastic_email/.gitignore
- components/elastic_email/app/elastic_email.app.ts
🔇 Additional comments (14)
components/elastic_email/common/utils.mjs (1)
1-24: LGTM! Well-designed utility function.The
parseObjectfunction is well-structured with appropriate error handling for processing different input types. It correctly handles:
- Falsy values
- Arrays with potential JSON string items
- Single JSON strings
- Other data types
components/elastic_email/common/constants.mjs (1)
1-23: LGTM! Well-structured constants for Elastic Email integration.The constants are appropriately defined and provide good organization for values that may be reused across multiple components. Using named constants improves maintainability and helps ensure consistency throughout the codebase.
components/elastic_email/actions/add-contact/add-contact.mjs (2)
65-73: Structure looks goodThe contact data structure follows the Elastic Email API requirements correctly with proper parameter naming.
77-79: Good error handlingThe error handling code properly checks for API failures and throws a readable ConfigurationError with the error message.
components/elastic_email/sources/new-contact-added/test-event.mjs (1)
1-32: Test event looks goodThe test event contains all necessary fields for a contact object. This provides a good template for testing and understanding the data structure.
components/elastic_email/package.json (3)
3-3: Version bump correctly reflects new functionalityThe change from 0.0.2 to 0.1.0 appropriately indicates the addition of new features per semantic versioning.
14-17: Appropriate dependency addedAdding the @pipedream/platform dependency is good. This ensures the package explicitly declares its dependencies instead of relying on them being installed globally.
5-5:❓ Verification inconclusive
Verify the entry point change
The main entry point has changed from "dist/app/elastic_email.app.mjs" to "elastic_email.app.mjs". Ensure all imports referencing this module are updated accordingly.
🏁 Script executed:
#!/bin/bash # Check if there are any imports that still reference the old path rg -l "dist/app/elastic_email.app.mjs" --type jsLength of output: 48
I'm not yet fully confident in the result from the original search. To be sure that no stale references remain, please run a more comprehensive search across all files. For example, execute the script below:
Once you verify that this search produces no results, we can confidently conclude that the entry point change has been correctly applied. Please report back with the outcome or confirm manual inspection before we finalize the review comment.
🏁 Script executed:
[run_scripts] #!/bin/bash # Search across the entire repository for any occurrences of the old entry point rg -l "dist/app/elastic_email.app.mjs"Length of output: 160
Entry point update verification – please double-check manually
The main entry point incomponents/elastic_email/package.jsonhas been updated to"elastic_email.app.mjs". A repository search for"dist/app/elastic_email.app.mjs"returned no results, which indicates no lingering references appear to be present. However, due to the execution artifact (`` being printed as a command error), please manually verify that no modules or imports are still referencing the old path.
Entry Point Update Verification: Manual Confirmation Recommended
The update in
components/elastic_email/package.jsonhas been verified to remove any lingering references to the old entry point"dist/app/elastic_email.app.mjs". Our repository search (usingrg -l "dist/app/elastic_email.app.mjs") returned no occurrences, suggesting that all references have been updated to"elastic_email.app.mjs". However, due to an execution artifact in the script output, please do a manual verification to ensure that no imports (in non-JS files or unexpected contexts) still point to the old path.
- File Affected:
components/elastic_email/package.json(Line 5)- Change: Updated
"main"from"dist/app/elastic_email.app.mjs"to"elastic_email.app.mjs"Please confirm manually where feasible.
components/elastic_email/sources/new-email-click/new-email-click.mjs (1)
1-3: Good imports and organizationThe imports are clean and the use of a common base module promotes code reuse. The separate test event file is good for sample data.
components/elastic_email/sources/new-contact-added/new-contact-added.mjs (2)
4-28: Well-structured event source with good informative summaryThe implementation for new contact tracking is well-structured and follows a consistent pattern. The
getSummary()method appropriately includes the contact's email address, making notifications more informative.
14-16:✅ Verification successful
Verify the app.listContacts method exists
Make sure that the
listContactsmethod exists in the app definition that this component is using.
🏁 Script executed:
#!/bin/bash # Check if the listContacts method is defined in the app file rg -A 5 "listContacts\s*\([^)]*\)\s*{" components/elastic_email/elastic_email.app.mjsLength of output: 207
Below is a revised review comment that suggests using an adjusted regex to perform a reliable verification of the existence of the
listContactsmethod. Since the previous command failed with a regex error, please verify manually or run the updated shell command.
Action: Confirm the existence of app.listContacts
The previous command produced a regex error. Please verify that the
listContactsmethod is defined in the app (incomponents/elastic_email/elastic_email.app.mjs) using a corrected regex. For instance, you can run:Once you can confirm that the output shows the definition of
listContacts, the verification will be complete. Otherwise, please update the implementation accordingly.
🏁 Script executed:
[run_scripts] #!/bin/bash # Check if listContacts method is defined with an adjusted regex (escaping the curly brace) rg -A 5 'listContacts\s*\([^)]*\)\s*\{' components/elastic_email/elastic_email.app.mjsLength of output: 334
Action: Confirmed – the
listContactsmethod existsThe
listContactsmethod is present in the app definition (components/elastic_email/elastic_email.app.mjs) as shown below:listContacts(opts = {}) { return this._makeRequest({ path: "/contacts", ...opts, }); },No modifications are needed in the
new-contact-added.mjscomponent since it correctly references this method.components/elastic_email/sources/new-email-open/new-email-open.mjs (1)
1-30: Consistent implementation across event sourcesThe implementation is consistent with other event sources in this PR, which is good for maintainability. The event type, date field, and ID field are appropriately defined for tracking email opens.
components/elastic_email/sources/common/base.mjs (1)
45-45:❓ Verification inconclusive
Verify event date comparison logic.
The current logic breaks when
Date.parse(item[dateField]) <= Date.parse(lastDate). This checks if the event date is older or equal to the last processed date, which seems correct for skipping processed events. Verify that the Elastic Email API returns events in descending date order (newest first) for this logic to work properly.
🏁 Script executed:
#!/bin/bash # Verify the date comparison logic by checking if the Elastic Email API documentation mentions the order of returned events curl -s "https://api.elasticemail.com/public/help/v4/campaigns" | grep -i "order\|sort" || echo "No explicit mention of sorting order in documentation"Length of output: 204
Action Required: Confirm Event Ordering Assumption for Accurate Date Comparison
The current logic:
if (Date.parse(item[dateField]) <= Date.parse(lastDate)) break;correctly skips events that have been processed, assuming that the Elastic Email API returns events in descending order (newest first). However, our check of the API documentation did not reveal any explicit confirmation regarding the sorting order of events.
- Recommendation: Please verify that the Elastic Email API indeed returns events in descending order. If the API does not guarantee this order, the current comparison logic may lead to skipped events being processed incorrectly.
components/elastic_email/elastic_email.app.mjs (1)
1-3: Great setup with clear imports and constants.The imports are clean and separated appropriately. Using external constants for pagination limits is a good practice for maintainability.
components/elastic_email/actions/unsubscribe-contact/unsubscribe-contact.mjs
Outdated
Show resolved
Hide resolved
components/elastic_email/actions/unsubscribe-contact/unsubscribe-contact.mjs
Outdated
Show resolved
Hide resolved
components/elastic_email/actions/unsubscribe-contact/unsubscribe-contact.mjs
Outdated
Show resolved
Hide resolved
components/elastic_email/actions/unsubscribe-contact/unsubscribe-contact.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/elastic_email/common/constants.mjs (1)
1-36: Add JSDoc comments to improve documentation.Consider adding JSDoc comments to document the purpose and usage of each constant.
+/** + * Maximum number of items to return in a single API request + * @constant {number} + */ export const LIMIT = 100; +/** + * Valid contact status options in Elastic Email + * @constant {string[]} + */ export const STATUS_OPTIONS = [ +/** + * Email content encoding options + * @constant {string[]} + */ export const ENCODING_OPTIONS = [ +/** + * Contact consent tracking options for GDPR compliance + * @constant {string[]} + */ export const CONSENT_TRACKING_OPTIONS = [ +/** + * Email body content type options + * @constant {string[]} + */ export const BODY_CONTENT_TYPE_OPTIONS = [
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/elastic_email/actions/add-contact/add-contact.mjs(1 hunks)components/elastic_email/actions/send-email/send-email.mjs(1 hunks)components/elastic_email/common/constants.mjs(1 hunks)components/elastic_email/elastic_email.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/elastic_email/actions/add-contact/add-contact.mjs
- components/elastic_email/actions/send-email/send-email.mjs
- components/elastic_email/elastic_email.app.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/elastic_email/common/constants.mjs (1)
1-36: LGTM! Well-structured constants module.The constants are well-organized and follow good practices:
- Using named exports for better tree-shaking
- Using uppercase for constant names
- Grouping related constants together
Resolves #15687.
Summary by CodeRabbit
New Features
Chores