[Components] textline - new components #15477
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a comprehensive set of modules for the Textline integration in Pipedream. The changes include creating new actions for sending messages, sending announcements, and managing contacts. A new constants module provides shared configuration, and the main app module is enhanced with additional methods for API interactions. The package version is updated to reflect these significant additions, and dependencies are specified to ensure proper functionality. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ 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 (5)
components/textline/common/constants.mjs (1)
1-1: Consider making the base URL configurable.The base URL is hardcoded to the production environment. Consider making it configurable to support different environments (e.g., staging, development).
-const BASE_URL = "https://application.textline.com"; +const BASE_URL = process.env.TEXTLINE_BASE_URL || "https://application.textline.com";components/textline/actions/send-message/send-message.mjs (1)
31-50: Add input validation and make resolve parameter configurable.The implementation could be improved by:
- Adding validation for phone number format
- Making the resolve parameter configurable instead of hardcoding it to "1"
props: { // ... existing props + resolve: { + type: "boolean", + label: "Resolve", + description: "Whether to resolve the conversation after sending the message.", + optional: true, + default: true, + }, }, methods: { + validatePhoneNumber(number) { + const regex = /^\(\d{3}\) \d{3}-\d{4}$/; + if (!regex.test(number)) { + throw new Error("Phone number must be in format (XXX) XXX-XXXX"); + } + }, // ... existing methods }, async run({ $ }) { const { sendMessage, phoneNumber, comment, + resolve, } = this; + + this.validatePhoneNumber(phoneNumber); + const response = await sendMessage({ $, data: { phone_number: phoneNumber, comment: { body: comment, }, - resolve: "1", + resolve: resolve ? "1" : "0", }, });components/textline/actions/create-update-contact/create-update-contact.mjs (1)
87-99: Improve error handling for better user experience.The error handling could be improved by:
- Adding more specific error messages
- Handling other potential error cases
} catch (error) { - if (error.response.status === 400 && error.response.data?.errors?.phone_number[0] === "Already in use") { + if (error.response?.status === 400) { + const phoneError = error.response.data?.errors?.phone_number?.[0]; + if (phoneError === "Already in use") { + ({ customer: { uuid } } = await app.getCustomerByPhoneNumber({ + $, + params: { + phone_number: phoneNumber, + }, + })); + } else { + throw new Error(`Invalid phone number: ${phoneError}`); + } + } else if (error.response?.status === 429) { + throw new Error("Rate limit exceeded. Please try again later."); + } else { throw error; } }components/textline/textline.app.mjs (1)
94-99: Consider adding response type checking for getCustomerByPhoneNumber.The method should validate the response structure to ensure it contains the expected customer data.
getCustomerByPhoneNumber(args = {}) { - return this._makeRequest({ + const response = this._makeRequest({ path: "/customers", ...args, }); + if (!response?.customers?.length) { + throw new Error("Customer not found"); + } + return response; },components/textline/actions/send-announcement/send-announcement.mjs (1)
38-80: Enhance error handling in phone number options.The phone number query implementation could benefit from error handling and rate limiting considerations.
Consider this enhancement:
options: async ({ page, query, }) => { + try { const { customers } = await this.app.listCustomers({ params: { page, page_size: 30, query: query || "", }, }); return customers.map(({ name: label, phone_number: value, }) => ({ label, value, })); + } catch (error) { + console.error('Error fetching customers:', error); + throw new Error('Failed to fetch customer phone numbers. Please try again.'); + } },
📜 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 (6)
components/textline/actions/create-update-contact/create-update-contact.mjs(1 hunks)components/textline/actions/send-announcement/send-announcement.mjs(1 hunks)components/textline/actions/send-message/send-message.mjs(1 hunks)components/textline/common/constants.mjs(1 hunks)components/textline/package.json(2 hunks)components/textline/textline.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (4)
components/textline/common/constants.mjs (1)
5-14: LGTM! Well-structured selection type constants.The SELECTION_TYPE object is well-structured with clear label/value pairs.
components/textline/actions/send-announcement/send-announcement.mjs (2)
1-9: LGTM! Component metadata and imports look good.The component is well-documented with clear name, description, and version. The documentation link is properly included.
10-37: Consider adding input validation for message content.While the props are well-defined, consider adding validation for
massTextCommentBodyto ensure it's not empty and meets any length requirements specified by the Textline API.Let's verify the API requirements:
components/textline/package.json (1)
3-3: Verify platform dependency compatibility.The version bump to 0.1.0 is appropriate for the new feature additions. However, let's verify the compatibility of @pipedream/platform@^3.0.3 with the new components.
Run this script to check for any compatibility issues:
Also applies to: 15-17
✅ Verification successful
Dependency Compatibility Check Confirmed
The output confirms that the latest version of @pipedream/platform is 3.0.3 and no deprecation notices were found, indicating compatibility with the new components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check @pipedream/platform version compatibility # Get the latest version and check for any breaking changes # Get the latest version npm view @pipedream/platform versions --json | jq -r '.[-1]' # Check for any deprecation notices npm view @pipedream/platform | grep -i "deprecat"Length of output: 118
components/textline/actions/create-update-contact/create-update-contact.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
textline-new-components
branch
from
2b42f8a to
cdab5bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/textline/textline.app.mjs (1)
66-81:⚠️ Potential issueCritical: Incomplete error handling in
_makeRequest.
Currently, only errors with a 500 status are caught and transformed, while other errors are swallowed (i.e. not re-thrown), which may lead to silent failures downstream. It is recommended to rethrow the error (or handle additional statuses) when the status is not 500. For example:- } catch (error) { - if (error.response?.status === 500) { - console.log("Error response", error.response); - throw new Error("Textline is currently experiencing issues. Please try again later."); - } - } + } catch (error) { + if (error.response?.status === 500) { + console.log("Error response", error.response); + throw new Error("Textline is currently experiencing issues. Please try again later."); + } + throw error; + }This will ensure that errors of any other type are properly surfaced.
components/textline/actions/send-announcement/send-announcement.mjs (1)
89-120:⚠️ Potential issueReview the
runmethod for response validation and property usage.
- The method destructures several properties including
savedSearchUuid, which is not defined in the props. Please ensure this value is either defined or removed from the payload.- Additionally, the response from
sendAnnouncementis not validated before exporting the summary. It is advisable to add a validation step—if the response does not indicate a successful send (for example, by missing an expectedidor status flag), throw an appropriate error before exporting a success summary. For instance:- $.export("$summary", "Successfully sent the announcement."); - return response; + if (!response?.id) { + throw new Error("Failed to send announcement: Invalid response from Textline API"); + } + + $.export("$summary", "Successfully sent the announcement."); + return response;These adjustments will make the action more robust.
🧹 Nitpick comments (2)
components/textline/textline.app.mjs (2)
7-32: Good implementation for thephoneNumberproperty.
The structure is clear and properly uses an async options function to query and map customers. For improved resiliency, consider adding error handling (e.g., a try/catch) to gracefully handle failures fromthis.listCustomers.
33-52: Solid definition for thegroupUuidproperty.
It follows a similar pattern to thephoneNumberproperty and cleanly maps organization details. Again, consider incorporating error handling in the async options function so that a failed API call won’t break the property’s option resolution.
📜 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 (6)
components/textline/actions/create-update-contact/create-update-contact.mjs(1 hunks)components/textline/actions/send-announcement/send-announcement.mjs(1 hunks)components/textline/actions/send-message/send-message.mjs(1 hunks)components/textline/common/constants.mjs(1 hunks)components/textline/package.json(2 hunks)components/textline/textline.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/textline/package.json
- components/textline/common/constants.mjs
- components/textline/actions/create-update-contact/create-update-contact.mjs
- components/textline/actions/send-message/send-message.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (5)
components/textline/textline.app.mjs (2)
56-65: Clear URL and header generation functions.
BothgetUrlandgetHeadersare implemented in a straightforward manner and correctly compose the request details.
82-110: Consistent API helper methods.
The methodspost,put,getCustomerByPhoneNumber,listOrganizationDetails, andlistCustomersconsistently delegate to_makeRequestusing a spread of arguments. This pattern is clear and promotes reusability.components/textline/actions/send-announcement/send-announcement.mjs (3)
1-3: Clean import statements.
The imports ofappandconstantsare straightforward and appropriately reference the required modules.
38-77: Dynamic additional properties implemented appropriately.
TheadditionalPropsmethod dynamically returns either atagfield or aphoneNumbersfield depending on the value ofselectionType. This pattern enhances the flexibility of the action.
81-88: Simple and effectivesendAnnouncementmethod.
This method correctly wraps the POST call to the/announcementsendpoint using the app’s API helper. Consider, as a follow-up, validating the response to ensure that the announcement was sent successfully (see the past review comment for response validation).
components/textline/actions/send-announcement/send-announcement.mjs
Outdated
Show resolved
Hide resolved
components/textline/actions/send-announcement/send-announcement.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
textline-new-components
branch
from
cdab5bb to
ad65d4a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/textline/textline.app.mjs (1)
94-99:getCustomerByPhoneNumberMethod: Verify Filtering LogicThe method delegates to
_makeRequestwith the/customersendpoint. While this is a useful abstraction, ensure that any arguments passed intogetCustomerByPhoneNumberinclude the proper filters to retrieve a customer by phone number. The naming suggests a more specialized query, so clarifying documentation or additional query parameter handling might be beneficial.
📜 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 (6)
components/textline/actions/create-update-contact/create-update-contact.mjs(1 hunks)components/textline/actions/send-announcement/send-announcement.mjs(1 hunks)components/textline/actions/send-message/send-message.mjs(1 hunks)components/textline/common/constants.mjs(1 hunks)components/textline/package.json(2 hunks)components/textline/textline.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/textline/package.json
- components/textline/common/constants.mjs
- components/textline/actions/send-announcement/send-announcement.mjs
- components/textline/actions/send-message/send-message.mjs
- components/textline/actions/create-update-contact/create-update-contact.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (10)
components/textline/textline.app.mjs (10)
1-2: Import Statements: Clean and ClearThe import of
axiosfrom@pipedream/platformand the constants from"./common/constants.mjs"is straightforward and adheres to the project’s module structure.
7-32:phoneNumberProperty Options Method: Functional and ExtensibleThe async
optionsfunction under thephoneNumberproperty is well structured. It uses destructuring for parameters and a defaultmapperfunction to transform customer data. One suggestion would be to consider adding error handling in casethis.listCustomersfails, although the current implementation may be sufficient if errors are handled upstream.
33-53:groupUuidProperty Options Method: Straightforward ImplementationThe async
optionsfunction forgroupUuidis implemented similarly tophoneNumber. It correctly provides default parameters and maps the results to the expected format. As withphoneNumber, consider whether additional error handling is needed when callingthis.listOrganizationDetailsin production scenarios.
56-58:getUrlMethod: URL Construction is ClearThe
getUrlmethod concatenatesconstants.BASE_URL,constants.VERSION_PATH, and the providedpath(with a.jsonextension) accurately. This method is concise and does the intended job.
59-65:getHeadersMethod: Proper Header AssemblyThis method correctly merges any provided headers with standard headers such as
"Content-Type"and the access token fromthis.$auth.access_token. The implementation is clear and aligns with expected patterns.
66-81: **Enhance Error Handling in_makeRequestMethod **The
_makeRequestmethod currently only handles the case whereerror.response?.status === 500. As suggested in previous reviews, consider broadening the error handling to cover:
- Network errors: cases where
error.responsemay be undefined.- Rates or authentication issues: e.g., 429 (rate limit) and 401 (authentication failures).
- Request timeouts: including an optional
timeoutparameter.Below is a diff suggestion to enhance the error handling:
async _makeRequest({ - $ = this, path, headers, ...args + $ = this, path, headers, timeout = 10000, ...args } = {}) { try { return await axios($, { ...args, url: this.getUrl(path), headers: this.getHeaders(headers), + timeout, }); } catch (error) { - if (error.response?.status === 500) { - console.log("Error response", error.response); - throw new Error("Textline is currently experiencing issues. Please try again later."); - } + if (!error.response) { + throw new Error("Network error. Please check your connection."); + } + + const { status, data } = error.response; + + if (status === 500) { + console.log("Error response", error.response); + throw new Error("Textline is currently experiencing issues. Please try again later."); + } + + if (status === 429) { + const retryAfter = error.response.headers["retry-after"] || "a short while"; + throw new Error(`Rate limit exceeded. Please try again after ${retryAfter} seconds.`); + } + + if (status === 401) { + throw new Error("Authentication failed. Please verify your access token."); + } + + throw new Error(data?.message || `Request failed with status ${status}`); + } },This will result in more robust and user-friendly error messages.
82-87:postMethod: Concise Wrapper over_makeRequestThe implementation of the
postmethod correctly wraps_makeRequestwith the HTTP method set toPOST. The use of argument spreading ensures flexibility.
88-93:putMethod: Consistent withpostMethodThe
putmethod mirrors the approach used in thepostmethod and correctly sets the method toPUT. This consistency helps maintain the codebase’s clarity.
100-105:listOrganizationDetailsMethod: Simple and EffectiveThis method is a clean abstraction over
_makeRequestfor retrieving organization details. The fixed"/organization"endpoint is clearly defined, making the implementation easy to follow.
106-110:listCustomersMethod: Clear Abstraction for Customer ListingSimilar to the previous methods,
listCustomerswraps_makeRequesteffectively to call the"/customers"endpoint. Its simplicity and consistency with the rest of the codebase make it easy to maintain.
jcortes
force-pushed
the
textline-new-components
branch
from
ad65d4a to
cdc2446
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/textline/textline.app.mjs (2)
8-32: Add phone number format validation.Consider adding format validation to ensure phone numbers match the expected format
(222) 222-2222.phoneNumber: { type: "string", label: "Phone Number", description: "The phone number of the contact. Eg. `(222) 222-2222`.", + pattern: "^\\(\\d{3}\\)\\s\\d{3}-\\d{4}$", + errorMessage: "Please enter a valid phone number in the format (222) 222-2222", useQuery: true,
55-112: Add missing HTTP methods and JSDoc documentation.Consider the following improvements:
- Add GET and DELETE methods for completeness
- Add JSDoc documentation for better maintainability
methods: { + /** + * Constructs the API URL + * @param {string} path - The API endpoint path + * @returns {string} The complete API URL + */ getUrl(path) { return `${constants.BASE_URL}${constants.VERSION_PATH}${path}.json`; }, + /** + * Get method for making GET requests + * @param {Object} args - Request arguments + * @returns {Promise} API response + */ + get(args = {}) { + return this._makeRequest({ + method: "GET", + ...args, + }); + }, + /** + * Delete method for making DELETE requests + * @param {Object} args - Request arguments + * @returns {Promise} API response + */ + delete(args = {}) { + return this._makeRequest({ + method: "DELETE", + ...args, + }); + },
📜 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 (6)
components/textline/actions/create-update-contact/create-update-contact.mjs(1 hunks)components/textline/actions/send-announcement/send-announcement.mjs(1 hunks)components/textline/actions/send-message/send-message.mjs(1 hunks)components/textline/common/constants.mjs(1 hunks)components/textline/package.json(2 hunks)components/textline/textline.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/textline/common/constants.mjs
- components/textline/package.json
- components/textline/actions/send-message/send-message.mjs
- components/textline/actions/create-update-contact/create-update-contact.mjs
- components/textline/actions/send-announcement/send-announcement.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (1)
components/textline/textline.app.mjs (1)
66-82: Enhance error handling in _makeRequest method.The current error handling only covers 500 status codes. Consider implementing the comprehensive error handling suggested in the past review.
jcortes
force-pushed
the
textline-new-components
branch
from
cdc2446 to
04ae144
Compare
|
/approve |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/textline/textline.app.mjs (2)
7-54: Consider extracting reusable components and constants.The prop definitions are well-structured, but could benefit from some refinements:
- The mapper function in
phoneNumber.optionscould be extracted for reusability- The default params in
groupUuid.optionscould be moved to constants+const customerMapper = ({ + name: label, phone_number: value, +}) => ({ + label, + value, +}); propDefinitions: { phoneNumber: { // ... async options({ page, query, - mapper = ({ - name: label, phone_number: value, - }) => ({ - label, - value, - }), + mapper = customerMapper, }) { // ... }, }, groupUuid: { // ... async options({ params = { - include_groups: true, - include_users: false, + include_groups: constants.DEFAULT_INCLUDE_GROUPS, + include_users: constants.DEFAULT_INCLUDE_USERS, }, }) { // ... }, }, },
83-112: Add JSDoc documentation for methods.The methods are well-structured but would benefit from JSDoc documentation describing parameters, return types, and possible errors.
+/** + * Makes a POST request to the Textline API + * @param {Object} args - Request arguments + * @returns {Promise<Object>} API response + */ post(args = {}) { return this._makeRequest({ method: "POST", ...args, }); }, +/** + * Makes a PUT request to the Textline API + * @param {Object} args - Request arguments + * @returns {Promise<Object>} API response + */ put(args = {}) { return this._makeRequest({ method: "PUT", ...args, }); }, +/** + * Retrieves a customer by their phone number + * @param {Object} args - Request arguments + * @returns {Promise<Object>} Customer data + */ getCustomerByPhoneNumber(args = {}) { return this._makeRequest({ path: "/customers", ...args, }); },
📜 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 (6)
components/textline/actions/create-update-contact/create-update-contact.mjs(1 hunks)components/textline/actions/send-announcement/send-announcement.mjs(1 hunks)components/textline/actions/send-message/send-message.mjs(1 hunks)components/textline/common/constants.mjs(1 hunks)components/textline/package.json(2 hunks)components/textline/textline.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/textline/common/constants.mjs
- components/textline/package.json
- components/textline/actions/send-message/send-message.mjs
- components/textline/actions/send-announcement/send-announcement.mjs
- components/textline/actions/create-update-contact/create-update-contact.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (2)
components/textline/textline.app.mjs (2)
1-6: LGTM!The imports and basic app configuration are properly set up.
66-82: Enhance error handling and add request timeout.The _makeRequest method could be improved with more comprehensive error handling and request timeout.
WHY
Resolves #15204
Summary by CodeRabbit
New Features
Improvements
Dependencies
@pipedream/platformdependency.