Add support for Adyen payment method Open banking / PaybyBank#42
Conversation
Summary of ChangesHello @remcotolsma, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the Adyen Open Banking / PaybyBank payment method into the system. This enhancement allows the application to process transactions using direct bank transfers via Adyen's Open Banking capabilities, providing users with an additional, modern payment option. The changes involve registering the new method in the gateway and defining its type for proper system recognition. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds support for the Adyen 'Pay by Bank' payment method. The changes are a good start, but there are a few issues to address.
First, there's a naming inconsistency for the new constant in PaymentMethodType.php. It should be PAY_BY_BANK instead of PAYBYBANK to follow the existing convention.
Critically, the implementation is incomplete. According to Adyen's documentation, 'Pay by Bank' is an API-only integration. This means it must be added to the can_api_only() method in Gateway.php. Without this change, payments with this method will fail because the integration will incorrectly attempt to use the Drop-in flow, which is not supported for 'Pay by Bank'.
Finally, the unit tests for PaymentMethodType in tests/src/PaymentMethodTypeTest.php should be updated to include a test case for the new payment method to ensure the mapping is correct.
| * Constant for the 'PaybyBank' payment method type. | ||
| * | ||
| * @link https://www.adyen.com/nl_NL/betaalmethoden/open-banking | ||
| * @var string | ||
| */ | ||
| const PAYBYBANK = 'paybybank'; |
There was a problem hiding this comment.
For consistency with other constants in this class (e.g., APPLE_PAY, KLARNA_PAY_NOW), the new constant should be named PAY_BY_BANK instead of PAYBYBANK. The PHPDoc comment should also be updated to 'Pay by Bank'.
Additionally, it's better to link to the official developer documentation for Pay by Bank instead of a marketing page, as it provides more relevant technical details.
* Constant for the 'Pay by Bank' payment method type.
*
* @link https://docs.adyen.com/payment-methods/pay-by-bank
* @var string
*/
const PAY_BY_BANK = 'paybybank';| PaymentMethods::MAESTRO => self::MAESTRO, | ||
| PaymentMethods::MB_WAY => self::MB_WAY, | ||
| PaymentMethods::MOBILEPAY => self::MOBILEPAY, | ||
| PaymentMethods::PAY_BY_BANK => self::PAYBYBANK, |
|
@gemini-code-assist I think this is incorrect:
I tested the integration and although I was unable to successfully complete the payment ("Acquirer Error (No response from ACM)"), the payment method is available with Drop-in too. Integration methods "Web", "API only" and "Pay by Link" are mentioned at https://docs.adyen.com/payment-methods/pay-by-bank-europe. Approving this PR. |
|
Thank you, @rvdsteege, for this valuable clarification and for pointing out the Adyen documentation. I apologize for the incorrect statement in my summary regarding the 'Pay by Bank' integration being API-only. Your testing and the provided link clearly demonstrate that it is also available with Drop-in and other integration methods. I appreciate you correcting my understanding on this point. |
See: