-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[Components] adyen #14308
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[Components] adyen #14308
Conversation
@jcortes is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThe pull request introduces several new modules for payment processing using the Adyen API. These include actions for creating, capturing, refunding, and canceling payments, as well as submitting additional payment details. Each module defines specific properties and methods to facilitate interactions with the Adyen API, enhancing the overall functionality of the payment processing system. Changes
Assessment against linked issues
Suggested reviewers
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: 13
🧹 Outside diff range and nitpick comments (6)
components/adyen/common/constants.mjs (1)
1-2
: Add explanatory comments for constantsWhile the constants are well-named, their specific purposes and usages are not immediately clear from the context. Consider adding brief comments to explain:
- What LAST_CREATED_AT represents and how it's used in the Adyen integration.
- What DEFAULT_MAX represents (e.g., is it a limit for API calls, pagination, etc.?) and why the value 600 was chosen.
This will improve code maintainability and help other developers understand the significance of these values.
components/adyen/package.json (2)
3-3
: Version update looks good. Consider documenting changes.The version bump from 0.0.1 to 0.1.0 is appropriate given the new features added for Adyen payment processing. This minor version increase correctly indicates non-breaking changes or additions.
Consider adding a CHANGELOG.md file or updating the existing one to document the changes introduced in this version. This helps users understand what's new or changed when updating the package.
15-18
: Dependency addition is appropriate. Consider flexible versioning.Adding the @pipedream/platform dependency is a good choice for leveraging Pipedream's platform capabilities. The explicit version pinning ensures consistency across installations.
Consider using a caret (^) for more flexible versioning:
"dependencies": { - "@pipedream/platform": "3.0.3" + "@pipedream/platform": "^3.0.3" }This allows for compatible updates within the same major version, potentially benefiting from bug fixes and minor improvements without manual intervention.
components/adyen/adyen.app.mjs (1)
18-22
: Provide guidance on theamountValue
format to prevent user errorsThe
amountValue
property expects the transaction amount in minor units, which may be confusing for users not familiar with this format. To enhance usability, consider adding helper text or validation to guide users in entering the correct amount.components/adyen/actions/create-payment/create-payment.mjs (2)
38-38
: Fix the typo in the description ofreturnUrl
There's a typo in the description: "For more information refer the the [documentation]". It should be "For more information, refer to the [documentation]".
Suggested fix:
- description: "The URL to return to in case of a redirection. The format depends on the channel. For more information refer the the [documentation](https://docs.adyen.com/api-explorer/Checkout/71/post/payments#request-returnUrl).", + description: "The URL to return to in case of a redirection. The format depends on the channel. For more information, refer to the [documentation](https://docs.adyen.com/api-explorer/Checkout/71/post/payments#request-returnUrl).",
52-53
: Correct the typo and clarify the description forPayment Method Details
The description contains a typo and may not accurately reflect the purpose of
paymentMethodDetails
in the "Create Payment" action.Suggested updated description:
- description: "The payment method details object required for submitting additional payment details. Should contain relevant payment details fields. For more information refer the the [documentation](https://docs.adyen.com/api-explorer/Checkout/71/post/payments#request-paymentMethod).", + description: "The payment method details required for processing the payment. Should contain relevant fields specific to the chosen payment method. For more information, refer to the [documentation](https://docs.adyen.com/api-explorer/Checkout/71/post/payments#request-paymentMethod).",
📜 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 (8)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/common/constants.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🧰 Additional context used
🔇 Additional comments (9)
components/adyen/common/constants.mjs (1)
1-15
: Overall assessment of constants.mjsThe file successfully establishes necessary constants and API configuration for the Adyen integration. However, there are several areas for improvement:
- Add explanatory comments for the constants to clarify their purpose and usage.
- Implement a more flexible API configuration system using environment variables.
- Consider using named exports instead of a default export for better import flexibility.
These changes will enhance the maintainability, flexibility, and clarity of the code, making it easier for other developers to work with and update in the future.
Despite these suggestions, the current implementation is functional and provides a solid foundation for the Adyen integration. Good job on setting up this constants file!
components/adyen/package.json (1)
Line range hint
1-18
: Overall package.json changes align well with PR objectivesThe updates to package.json accurately reflect the implementation of new Adyen payment functionalities as outlined in the PR objectives. The minor version bump and the addition of the @pipedream/platform dependency are appropriate for the scope of changes described.
These modifications set the groundwork for the new Adyen payment actions (create, submit details, cancel, capture, and refund) by ensuring the correct package version and necessary dependencies are in place.
components/adyen/actions/submit-details/submit-details.mjs (1)
3-40
: LGTM! The action is correctly implemented and aligns with the Adyen API documentationThe implementation follows best practices and correctly interfaces with the Adyen API to submit additional payment details.
components/adyen/actions/capture-payment/capture-payment.mjs (1)
30-35
:⚠️ Potential issueEnsure
amountValue
is validated as an integer.Adyen expects the
value
in theamount
object to be specified in minor units (e.g., cents) as an integer. To prevent API errors due to incorrect data types, ensure thatamountValue
is validated to be an integer.Consider updating the
amountValue
prop to enforce integer values:amountValue: { propDefinition: [ app, "amountValue", ], type: "integer", description: "The amount of the transaction in minor units (e.g., cents).", },components/adyen/actions/refund-payment/refund-payment.mjs (3)
4-36
: Action properties are correctly definedThe action's key, name, description, version, type, and props are appropriately specified, ensuring proper integration with the platform.
37-47
:refundPayment
method is correctly implementedThe
refundPayment
method constructs and sends the API request to refund a payment as per Adyen's API requirements.
48-70
:run
method correctly executes the refund operationThe
run
method properly retrieves the necessary parameters and callsrefundPayment
to perform the refund, handling the response appropriately.components/adyen/adyen.app.mjs (1)
51-56
:⚠️ Potential issueVerify that
this.$auth.url
is defined in the authentication configurationThe
getUrl
method usesthis.$auth.url
when theapi
parameter is not provided. Ensure that theurl
property is correctly defined in the authentication configuration (this.$auth
) for the Adyen app. Ifurl
is not set, requests may fail due to an undefined base URL.Run the following script to check if
url
is defined in the authentication configuration:#!/bin/bash # Description: Verify that 'url' is defined in authentication configuration # Test: Search for 'url' in authentication configuration files. Expect: 'url' property is defined. rg --type yaml --json "adyen" | jq '.[] | select(.data | contains("url")) | .data'components/adyen/actions/create-payment/create-payment.mjs (1)
1-96
: Overall code looks goodThe "Create Payment" action is well-implemented and follows best practices. The code is clear and appropriately structures the request to the Adyen API.
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 @jcortes, I just added a comment.
5728a4f
to
faf4bbb
Compare
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: 3
🧹 Outside diff range and nitpick comments (2)
components/adyen/adyen.app.mjs (1)
75-79
: Add documentation to listPaymentMethods methodThe
listPaymentMethods
method correctly implements the API call to fetch payment methods. However, it could benefit from additional documentation.Consider adding a JSDoc comment to provide more context:
+/** + * Fetches available payment methods from the Adyen API. + * @param {Object} args - Additional arguments to be passed to the API call. + * @param {Object} args.data - The request data, including merchantAccount. + * @returns {Promise<Object>} The API response containing available payment methods. + */ listPaymentMethods(args = {}) { return this.post({ path: "/paymentMethods", ...args, }); },This documentation will make it easier for other developers to understand and use the method correctly.
components/adyen/actions/create-payment/create-payment.mjs (1)
9-53
: Consider adding validation for paymentMethodDetails prop.The props are well-defined and use propDefinitions appropriately. However, the
paymentMethodDetails
prop, being an optional object, might benefit from additional validation to ensure it contains the correct fields based on the selectedpaymentMethodType
.Consider adding a custom validation function to the
paymentMethodDetails
prop:paymentMethodDetails: { type: "object", label: "Payment Method Details", description: "The payment method details object required for submitting additional payment details. Should contain relevant payment details fields. For more information refer the the [documentation](https://docs.adyen.com/api-explorer/Checkout/71/post/payments#request-paymentMethod).", optional: true, validateFunction: ({ paymentMethodType, paymentMethodDetails }) => { // Add logic to validate paymentMethodDetails based on paymentMethodType // Return an error message if validation fails, or true if it passes }, },
📜 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 (7)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/adyen/actions/cancel-payment/cancel-payment.mjs
- components/adyen/actions/capture-payment/capture-payment.mjs
- components/adyen/actions/refund-payment/refund-payment.mjs
- components/adyen/actions/submit-details/submit-details.mjs
- components/adyen/package.json
🧰 Additional context used
🔇 Additional comments (7)
components/adyen/adyen.app.mjs (4)
50-52
: LGTM: getUrl method implementationThe
getUrl
method correctly constructs the full URL by combining the base URL from auth and the provided path. This implementation is clean and follows best practices.
69-74
: LGTM: post method implementationThe
post
method is a clean and concise wrapper around_makeRequest
for making POST requests. This implementation promotes code reuse and maintainability.
Line range hint
1-83
: Overall assessment of adyen.app.mjs changesThe changes to
adyen.app.mjs
successfully implement the core functionality for interacting with the Adyen API, aligning well with the PR objectives. The new prop definitions and methods provide a solid foundation for creating, managing, and processing payments through Adyen.Key points:
- Prop definitions are well-structured and descriptive.
- New methods for URL construction, header management, and API requests are implemented.
- Specific methods for POST requests and listing payment methods are added.
Suggestions for improvement:
- Enhance error handling in the
paymentMethodType
options method and_makeRequest
method.- Verify the
api_key
configuration in authentication settings.- Add more detailed documentation to the
listPaymentMethods
method.These changes provide a good starting point for the Adyen integration. Implementing the suggested improvements will further enhance the robustness and maintainability of the code.
53-59
: Verify api_key configuration in authentication settingsThe
getHeaders
method correctly sets the required headers. However, ensure that theapi_key
is properly configured in the authentication settings.To confirm the
api_key
configuration, run the following script:This script will search for the
api_key
configuration related to Adyen in the typical configuration directories. Review the output to ensure theapi_key
is correctly set.components/adyen/actions/create-payment/create-payment.mjs (3)
1-8
: LGTM: Import and action metadata are well-defined.The import statement and action metadata are correctly implemented. The metadata provides clear information about the action's purpose, including a link to the relevant documentation.
55-62
: LGTM: createPayment method is well-implemented.The
createPayment
method is concise and correctly utilizes theapp.post
method to send a request to the "/payments" endpoint. The use of the spread operator forargs
provides flexibility for passing additional parameters.
1-94
: Overall, excellent implementation of the Adyen Create Payment actionThis implementation of the Adyen Create Payment action is well-structured, follows good coding practices, and aligns perfectly with the objectives outlined in the PR description. The code is organized logically, uses appropriate error handling, and leverages the Adyen API effectively.
A few minor improvements have been suggested in previous comments, including:
- Adding validation for the
paymentMethodDetails
prop- Handling potential
undefined
paymentMethodDetails
when spreadingOnce these small adjustments are made, this implementation will be robust and ready for production use.
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.
LGTM!
faf4bbb
to
a3e11bf
Compare
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: 0
🧹 Outside diff range and nitpick comments (5)
components/adyen/actions/cancel-payment/cancel-payment.mjs (3)
9-23
: LGTM: Props are well-definedThe props are correctly defined using propDefinitions from the app object, which is a good practice for consistency and reusability.
Consider adding descriptions to the
paymentPspReference
andmerchantAccount
props to provide more context for users of this action. For example:paymentPspReference: { propDefinition: [ app, "paymentPspReference", ], description: "The PSP reference of the payment to be cancelled", },
24-32
: LGTM: cancelPayment method is well-implementedThe
cancelPayment
method correctly uses the Adyen API to cancel a payment. The use of destructuring in the parameters enhances readability.Consider adding basic error handling within this method to provide more specific error messages. For example:
cancelPayment({ paymentPspReference, data, } = {}) { try { return this.app.getCheckoutAPI() .ModificationsApi .cancelAuthorisedPaymentByPspReference(paymentPspReference, data); } catch (error) { throw new Error(`Failed to cancel payment: ${error.message}`); } },
33-48
: Good improvements, but error handling is still missingThe
run
method has been improved by correctly using theawait
keyword when callingcancelPayment
. However, there are still some areas for improvement:
- Error handling is missing, as suggested in a previous review comment.
- The success message is exported before confirming the operation's success.
Consider implementing error handling and moving the success message export after confirming the operation's success. Here's a suggested implementation:
async run({ $ }) { const { cancelPayment, paymentPspReference, merchantAccount, } = this; try { const response = await cancelPayment({ paymentPspReference, data: { merchantAccount, }, }); $.export("$summary", "Successfully cancelled payment."); return response; } catch (error) { $.export("$summary", `Failed to cancel payment: ${error.message}`); throw error; } },This implementation addresses both the error handling and ensures the success message is only exported when the operation succeeds.
components/adyen/adyen.app.mjs (2)
6-47
: LGTM: Comprehensive prop definitions added for payment processing.The new prop definitions are well-structured and align with the PR objectives. Each prop has appropriate types, labels, and descriptions, which enhances code readability and usability.
Consider adding error handling to the
paymentMethodType.options
method:async options({ merchantAccount }) { if (!merchantAccount) { return []; } + try { const { paymentMethods } = await this.listPaymentMethods({ data: { merchantAccount, }, }); return paymentMethods.map(({ name: label, type: value, }) => ({ label, value, })); + } catch (error) { + console.error("Error fetching payment methods:", error); + return []; + } },This change will prevent unhandled exceptions and provide a fallback empty array in case of an error.
68-71
: LGTM: listPaymentMethods method added for fetching payment methods.The
listPaymentMethods
method is correctly implemented:
- It uses the CheckoutAPI to fetch payment methods.
- The method accepts data and options parameters, providing flexibility in API calls.
- It aligns with the PR objective of implementing payment-related actions.
Consider adding error handling to improve robustness:
listPaymentMethods({ data, options, } = {}) { - return this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options); + try { + return this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options); + } catch (error) { + console.error("Error listing payment methods:", error); + throw error; + } },This change will log errors while still allowing the caller to handle them as needed.
📜 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 (7)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/adyen/actions/capture-payment/capture-payment.mjs
- components/adyen/actions/create-payment/create-payment.mjs
- components/adyen/actions/refund-payment/refund-payment.mjs
- components/adyen/actions/submit-details/submit-details.mjs
- components/adyen/package.json
🧰 Additional context used
🔇 Additional comments (6)
components/adyen/actions/cancel-payment/cancel-payment.mjs (2)
1-8
: LGTM: Well-structured action definitionThe import statement and the exported object's properties are well-defined. The inclusion of the API documentation link in the description is particularly helpful for developers.
1-49
: Overall, a well-implemented cancel payment action with room for minor improvementsThis implementation of the cancel payment action for Adyen is generally well-structured and follows good practices. The use of propDefinitions, clear method structures, and correct API usage are commendable.
Main areas for potential improvement:
- Add error handling in both the
cancelPayment
andrun
methods.- Add descriptions to the props for better user understanding.
- Ensure the success message is only exported after confirming the operation's success.
Implementing these suggestions would enhance the robustness and user-friendliness of this action.
components/adyen/adyen.app.mjs (4)
1-1
: LGTM: Adyen API library import added.The import of the Adyen API library is correctly placed and necessary for the implementation of Adyen payment functionalities.
50-64
: LGTM: Well-implemented getClient method.The
getClient
method is correctly implemented:
- It properly initializes the Adyen client using authentication details.
- The method handles both live and test environments appropriately.
- The use of object destructuring for auth properties is a good practice.
65-67
: LGTM: Concise getCheckoutAPI method added.The
getCheckoutAPI
method is correctly implemented:
- It creates and returns a new instance of Adyen's CheckoutAPI.
- The method uses
getClient
, ensuring consistent client configuration.
1-71
: Overall, excellent implementation of Adyen payment integration.The changes in this file successfully implement the core functionality required for Adyen payment processing. The code is well-structured, with clear prop definitions and methods that align with the PR objectives. The minor suggestions for error handling will further improve the robustness of the implementation.
Great job on this comprehensive integration of Adyen payment functionalities!
a3e11bf
to
60b76f8
Compare
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: 1
📜 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 (7)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/adyen/actions/cancel-payment/cancel-payment.mjs
- components/adyen/actions/capture-payment/capture-payment.mjs
- components/adyen/actions/create-payment/create-payment.mjs
- components/adyen/actions/refund-payment/refund-payment.mjs
- components/adyen/actions/submit-details/submit-details.mjs
- components/adyen/package.json
🧰 Additional context used
🔇 Additional comments (5)
components/adyen/adyen.app.mjs (5)
1-1
: LGTM: Correct import of Adyen API libraryThe import statement for '@adyen/api-library' is correct and necessary for using the Adyen API in this integration.
6-47
: LGTM: Well-structured propDefinitionsThe propDefinitions are well-structured, providing clear type, label, and description for each property. The descriptions are informative and include links to relevant documentation, which is excellent for maintainability and user understanding.
50-64
: LGTM: Well-implemented getClient methodThe getClient method is well-implemented:
- It correctly uses the authentication details from this.$auth.
- Properly handles both live and test environments.
- Uses object spread for conditional properties, which is a good practice.
The method provides a clean way to create and configure the Adyen client instance.
65-67
: LGTM: Concise getCheckoutAPI methodThe getCheckoutAPI method is well-implemented:
- It's concise and correctly uses the getClient method to obtain the client instance.
- Provides a clean way to create and return an instance of adyen.CheckoutAPI.
26-41
:⚠️ Potential issueAdd error handling to async options method
The async options method in the paymentMethodType property is a good approach for dynamically fetching payment methods. However, it lacks error handling, which could lead to unhandled exceptions if the API call fails.
Consider adding a try-catch block to handle potential errors:
async options({ merchantAccount }) { if (!merchantAccount) { return []; } + try { const { paymentMethods } = await this.listPaymentMethods({ data: { merchantAccount, }, }); return paymentMethods.map(({ name: label, type: value, }) => ({ label, value, })); + } catch (error) { + console.error("Error fetching payment methods:", error); + return []; + } },This change will prevent unhandled exceptions and provide a fallback empty array in case of an error.
Likely invalid or redundant comment.
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 @jcortes, I just added a suggestion
60b76f8
to
62817f9
Compare
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: 4
🧹 Outside diff range and nitpick comments (4)
components/adyen/common/utils.mjs (1)
3-8
: Consider using strict equality for empty string check.The function looks good overall. For consistency and to prevent potential issues with type coercion, consider using strict equality (
===
) for the empty string check.Here's a suggested improvement:
function emptyStrToUndefined(value) { const trimmed = typeof(value) === "string" && value.trim(); - return trimmed === "" + return trimmed === "" ? undefined : value; }components/adyen/actions/submit-details/submit-details.mjs (2)
10-29
: Explicitly mark thedetails
prop as required for clarityWhile props are required by default in Pipedream, explicitly setting
required: true
enhances code readability and ensures that future maintainers understand that this prop is essential.Apply this diff to set
required
explicitly:details: { type: "object", + required: true, label: "Details", description: "Use this collection to submit the details that were returned as a result of the **Create Payment** action call.", },
37-54
: LGTM with suggestion: Consider adding error handlingThe
run
method is well-implemented, correctly using async/await and properly handling the data. However, consider adding try/catch error handling to provide more informative error messages to the user.Here's a suggested improvement:
async run({ $ }) { const { submitAdditionalDetails, details, authenticationData, paymentData, } = this; + try { const response = await submitAdditionalDetails({ data: { details: utils.parse(details), authenticationData: utils.parse(authenticationData), paymentData, }, }); $.export("$summary", "Successfully submitted additional payment details."); return response; + } catch (error) { + $.export("$summary", `Failed to submit additional payment details: ${error.message}`); + throw error; + } },components/adyen/adyen.app.mjs (1)
1-71
: Overall good implementation with room for improved error handlingThe changes to the Adyen app configuration are well-structured and implement the necessary functionality for interacting with the Adyen API. The prop definitions provide clear information, and the methods for client and API initialization are concise and effective.
The main area for improvement is consistent error handling across all methods. Implementing try-catch blocks in the
getClient
,getCheckoutAPI
,listPaymentMethods
, and theoptions
method ofpaymentMethodType
will greatly enhance the robustness of this module.Consider implementing these error handling suggestions to improve the overall reliability and maintainability of the code.
📜 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 (8)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/common/utils.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- components/adyen/actions/cancel-payment/cancel-payment.mjs
- components/adyen/actions/capture-payment/capture-payment.mjs
- components/adyen/actions/create-payment/create-payment.mjs
- components/adyen/actions/refund-payment/refund-payment.mjs
- components/adyen/package.json
🧰 Additional context used
🔇 Additional comments (11)
components/adyen/common/utils.mjs (2)
1-1
: LGTM: Import statement is correct.The import of
ConfigurationError
from "@pipedream/platform" is appropriate for error handling in this utility module.
1-24
: Summary: Utility functions align well with PR objectives.This new utility module introduces functions that are crucial for handling JSON parsing and string manipulation, which are essential for processing payment-related data in the Adyen integration. The functions are well-structured and handle error cases appropriately, contributing to the robustness of the payment processing implementation.
The suggested improvements, if implemented, will further enhance the module's functionality and reusability. Overall, this file aligns well with the PR objectives of implementing various payment actions for the Adyen service.
components/adyen/actions/submit-details/submit-details.mjs (3)
1-9
: LGTM: Imports and metadata are well-definedThe imports and object metadata are correctly implemented. The version is set to "0.0.1" as suggested in the past review comment.
30-36
: LGTM: submitAdditionalDetails method is well-implementedThe
submitAdditionalDetails
method is concise, follows the single responsibility principle, and correctly utilizes the app's API to submit the payment details.
1-55
: Overall implementation is solid and meets PR objectivesThe "Submit Additional Payment Details" action for Adyen is well-implemented, covering all necessary aspects including imports, metadata, props, methods, and the run function. The code structure is clean, and it correctly implements the functionality to submit additional payment details to Adyen, aligning with the PR objectives and resolving the linked issue #14286.
A few minor suggestions have been made to further improve the code:
- Explicitly marking the
details
prop as required.- Adding error handling in the
run
method.These changes will enhance the robustness and clarity of the implementation.
components/adyen/adyen.app.mjs (6)
1-1
: LGTM: Adyen API library import addedThe import statement for the Adyen API library is correctly placed at the top of the file. This is necessary for utilizing Adyen's API functionality throughout the module.
6-47
: LGTM: Well-structured prop definitionsThe prop definitions for merchantAccount, amountCurrency, amountValue, paymentMethodType, and paymentPspReference are well-structured and provide clear information about each property. The types, labels, and descriptions are informative and will be helpful for users of this component.
50-64
: LGTM: Well-implemented getClient methodThe getClient method is well-implemented. It correctly initializes the Adyen client with the necessary configuration, including handling both live and test environments. The conditional addition of liveEndpointUrlPrefix for the live environment is a good practice.
65-67
: LGTM: Concise getCheckoutAPI methodThe getCheckoutAPI method is concise and correctly uses the getClient method to initialize the CheckoutAPI. This approach ensures that the CheckoutAPI is always created with the most up-to-date client configuration.
26-41
:⚠️ Potential issueAdd error handling to the paymentMethodType options method
While the async options method for paymentMethodType is well-implemented, it lacks error handling. If the
listPaymentMethods
call fails, it could lead to an unhandled exception.Consider adding a try-catch block to handle potential errors:
async options({ merchantAccount }) { if (!merchantAccount) { return []; } + try { const { paymentMethods } = await this.listPaymentMethods({ data: { merchantAccount, }, }); return paymentMethods.map(({ name: label, type: value, }) => ({ label, value, })); + } catch (error) { + console.error("Error fetching payment methods:", error); + return []; + } },This change will prevent the method from throwing an unhandled exception and provide a fallback empty array in case of an error.
Likely invalid or redundant comment.
68-71
:⚠️ Potential issueAdd error handling to listPaymentMethods method
While the listPaymentMethods method provides a straightforward way to list payment methods, it lacks error handling. This could lead to unhandled exceptions if the API call fails.
Consider adding a try-catch block to handle potential errors:
listPaymentMethods({ data, options, } = {}) { + try { return this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options); + } catch (error) { + console.error("Error listing payment methods:", error); + throw error; // Re-throw to allow calling code to handle the error + } },This change will provide better error logging and allow the calling code to handle any errors that occur during the API call.
Likely invalid or redundant comment.
62817f9
to
379a9b2
Compare
/approve |
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
📜 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 (8)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/common/utils.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/adyen/package.json
🧰 Additional context used
🔇 Additional comments (10)
components/adyen/common/utils.mjs (2)
1-1
: LGTM: Import statement is correct.The import of
ConfigurationError
from "@pipedream/platform" is appropriate for error handling in this utility module.
3-8
: LGTM: Well-implemented utility function.The
emptyStrToUndefined
function is concise, follows the single responsibility principle, and correctly handles the conversion of empty strings to undefined. The use oftypeof(value) === "string"
for type checking is a good practice.components/adyen/actions/cancel-payment/cancel-payment.mjs (4)
1-8
: LGTM: Well-structured module definitionThe import statement and module definition are well-structured and include all necessary information. The description with a link to the API documentation is particularly helpful for developers.
9-23
: LGTM: Props correctly definedThe
paymentPspReference
andmerchantAccount
props are correctly defined using the app's propDefinitions. This approach ensures consistency with the app's configuration and is a good practice.
24-32
: LGTM: Well-implemented cancelPayment methodThe
cancelPayment
method is well-implemented. It correctly uses the app's API to perform the cancellation and employs destructuring for improved readability.
1-49
: LGTM: Well-structured and consistent implementationThe overall structure of this module is well-organized and consistent with other Adyen action modules. It correctly implements all necessary components for the "Cancel Payment" action, aligning with the PR objectives.
components/adyen/actions/capture-payment/capture-payment.mjs (2)
1-68
: Code implementation is correct and aligns with the PR objectives.The
Capture Payment
action is well-defined and correctly integrates with the Adyen API to capture authorized payments.
29-34
: 🛠️ Refactor suggestionEnsure
amountValue
is provided in minor units as required by Adyen API.The
value
field in theamount
object should represent the amount in minor units (e.g., cents for USD). Consider adding validation to ensureamountValue
is an integer representing the correct minor unit amount.Also applies to: 59-61
components/adyen/actions/refund-payment/refund-payment.mjs (1)
60-60
: EnsureamountValue
is provided in minor unitsIn the Adyen API, the
value
for the amount should be in minor units (e.g., cents). Verify thatamountValue
is correctly provided in minor units to prevent incorrect transaction amounts.components/adyen/actions/create-payment/create-payment.mjs (1)
84-86
: Previous Comment Still Applicable: Handleundefined
paymentMethodDetails
when SpreadingThe issue previously raised regarding handling potential
undefined
paymentMethodDetails
when spreading is still applicable.
379a9b2
to
e51aae1
Compare
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: 1
📜 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 (8)
- components/adyen/actions/cancel-payment/cancel-payment.mjs (1 hunks)
- components/adyen/actions/capture-payment/capture-payment.mjs (1 hunks)
- components/adyen/actions/create-payment/create-payment.mjs (1 hunks)
- components/adyen/actions/refund-payment/refund-payment.mjs (1 hunks)
- components/adyen/actions/submit-details/submit-details.mjs (1 hunks)
- components/adyen/adyen.app.mjs (1 hunks)
- components/adyen/common/utils.mjs (1 hunks)
- components/adyen/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- components/adyen/actions/cancel-payment/cancel-payment.mjs
- components/adyen/actions/capture-payment/capture-payment.mjs
- components/adyen/actions/create-payment/create-payment.mjs
- components/adyen/actions/refund-payment/refund-payment.mjs
- components/adyen/actions/submit-details/submit-details.mjs
- components/adyen/common/utils.mjs
- components/adyen/package.json
🧰 Additional context used
🔇 Additional comments (3)
components/adyen/adyen.app.mjs (3)
1-1
: LGTM: Adyen library importThe import statement for the Adyen API library is correct and necessary for the functionality of this module.
6-48
: Well-defined property definitionsThe
propDefinitions
section is comprehensive and well-structured. Each property has a clear type, label, and description, which will be helpful for users of this component. ThepaymentMethodType
property's async options method is a good approach for providing dynamic options based on the merchant account.
1-74
: LGTM: Well-structured and complete Adyen app implementationThe overall structure and implementation of the Adyen app are well-done. The file includes all necessary properties and methods for interacting with the Adyen API, aligning well with the PR objectives. The propDefinitions cover all required fields for various payment actions, and the methods provide a solid foundation for API interactions.
listPaymentMethods({ | ||
data, options, | ||
} = {}) { | ||
return this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options); |
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.
Add error handling to listPaymentMethods method
While the method correctly calls the PaymentsApi.paymentMethods function, it lacks error handling. Consider adding a try-catch block to handle potential API errors gracefully.
Apply this diff to add error handling:
- listPaymentMethods({
+ async listPaymentMethods({
data, options,
} = {}) {
- return this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options);
+ try {
+ return await this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options);
+ } catch (error) {
+ console.error("Error listing payment methods:", error);
+ throw error;
+ }
},
📝 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.
listPaymentMethods({ | |
data, options, | |
} = {}) { | |
return this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options); | |
async listPaymentMethods({ | |
data, options, | |
} = {}) { | |
try { | |
return await this.getCheckoutAPI().PaymentsApi.paymentMethods(data, options); | |
} catch (error) { | |
console.error("Error listing payment methods:", error); | |
throw error; | |
} | |
}, |
WHY
Resolves #14286
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores