[ECP-9847] Improve authorized amount comparison

[candemiralp]

Description

Tested scenarios

Fixes

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 significantly refactors the payment authorization process within the Adyen module. It introduces a new AuthorizationHandler class to centralize the logic for handling payment authorizations, including creating order payments, managing invoices, and handling fraud reviews. This change aims to decouple the authorization logic from webhook and payment response handlers, making the system more modular and testable. Additionally, it introduces an observer to ensure authorization is processed correctly after order placement, and streamlines several helper methods by removing direct dependencies on the Notification object.

Highlights

• Centralized Authorization Logic: Introduced a new AuthorizationHandler class to centralize the logic for handling payment authorizations, including creating order payments, managing invoices, and handling fraud reviews.
• Decoupled Notification Object: Removed direct dependencies on the Notification object from method signatures in AdyenOrderPayment, Invoice, and Order helpers, promoting cleaner code and better separation of concerns.
• Post-Order Placement Authorization: Added a new observer, AuthorizeAfterOrderPlacement, to process AUTHORISED payment responses after an order has been placed, ensuring robust authorization handling.
• Simplified Authorization Check: The isFullAmountAuthorized method in AdyenOrderPayment was simplified to directly compare order totals with the authorized amount based on the charged currency.
• Removed Boleto Status Logic: The getBoletoStatus method, which handled specific status logic for Boleto payments, was removed from the PaymentMethods helper.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• Helper/AdyenOrderPayment.php
  • Updated isFullAmountAuthorized method to use ChargedCurrency and PaymentAuthorizationAmount directly.
  • Changed createAdyenOrderPayment method signature to accept individual payment details instead of a Notification object.
  • Updated logging for createAdyenOrderPayment to use order->getIncrementId() instead of merchantReference.
• Helper/ChargedCurrency.php
  • Added DISPLAY constant.
• Helper/Invoice.php
  • Changed createInvoice method signature to accept individual payment details instead of a Notification object.
  • Updated logging for createInvoice to use pspReference and order->getIncrementId().
  • Updated invoice->setTransactionId to use pspReference.
  • Updated value for offline invoice capture to use amountValue.
  • Updated error logging for createInvoice.
• Helper/Order.php
  • Changed updatePaymentDetails method signature to accept pspReference instead of Notification.
  • Changed finalizeOrder method signature to accept pspReference and amount instead of Notification.
  • Removed Boleto specific status handling from finalizeOrder.
  • Updated logging in finalizeOrder to use pspReference and order->getIncrementId().
• Helper/PaymentMethods.php
  • Removed getBoletoStatus method.
• Helper/PaymentResponseHandler.php
  • Added AuthorizationHandler dependency.
  • Updated handlePaymentsDetailsResponse to call AuthorizationHandler::execute for AUTHORISED payments.
• Helper/Webhook/AuthorisationWebhookHandler.php
  • Removed dependencies on AdyenOrderPayment helper, CaseManagement, Invoice, PaymentMethods.
  • Added dependency on AuthorizationHandler and AdyenOrderPayment resource model.
  • Refactored handleSuccessfulAuthorisation to delegate authorization logic to AuthorizationHandler.
  • Removed markPaymentCapturedIfNeeded, sendOrderMailIfNeeded, createCashShipmentIfNeeded, handleAutoCapture, handleManualCapture methods, as their logic is now handled by AuthorizationHandler or is no longer needed here.
• Model/AuthorizationHandler.php
  • Added new file to centralize payment authorization logic.
  • Implemented execute method to handle creating adyen_order_payment entries, updating order payment details, managing auto/manual capture, fraud review, and sending order emails.
  • Included private methods handleAutoCapture, handleManualCapture, and sendOrderMailIfNeeded.
• Observer/AuthorizeAfterOrderPlacement.php
  • Added new file for an observer for the checkout_submit_all_after event.
  • Implemented logic to process AUTHORISED payment responses stored in the adyen_payment_response table by calling AuthorizationHandler::execute.
• Test/Unit/Helper/AdyenOrderPaymentTest.php
  • Updated testCreateAdyenOrderPayment to reflect changes in createAdyenOrderPayment method signature.
• Test/Unit/Helper/InvoiceTest.php
  • Updated testCreateInvoice and testCreateInvoiceManualCapture to reflect changes in createInvoice method signature.
• Test/Unit/Helper/OrderTest.php
  • Updated testFinalizeOrderFinalized, testFinalizeOrderPartialPayment, testUpdatePaymentDetailsWithOrderInitiallyInStatePaymentReview, testUpdatePaymentDetailsWithOrderNotInStatePaymentReview to reflect changes in method signatures.
  • Removed HistoryFactory mock from createOrderHelper.
• Test/Unit/Helper/PaymentMethodsTest.php
  • Removed tests related to getBoletoStatus method.
• Test/Unit/Helper/PaymentResponseHandlerTest.php
  • Added AuthorizationHandler mock.
  • Updated testHandlePaymentsDetailsResponseAuthorised and testHandlePaymentsDetailsResponseSetsCcType to include amount and currency in the response and to expect AuthorizationHandler::execute call.
• Test/Unit/Helper/Webhook/AuthorisationWebhookHandlerTest.php
  • Updated dependencies and constructor for AuthorisationWebhookHandler.
  • Refactored testHandleSuccessfulAuthorisationCallsAuthorizationHandler and testHandleSuccessfulAuthorisationSkipsAuthorizationHandlerWhenPaymentExists to test the new delegation to AuthorizationHandler.
  • Removed tests for handleAutoCapture, handleManualCapture, handleSuccessfulAuthorisationNotFullAmountAuthorizedAddsHistoryCommentOnly, handleSuccessfulAuthorisationBoletoDoesNotSendOrderMail, handleSuccessfulAuthorisationCashCreatesShipmentWhenConfigured.
  • Updated testDisableQuote, testHandleWebhookFailedRoutesToFailedHandler, testHandleSuccessfulAuthorisationDeactivatesActiveQuote, testHandleSuccessfulAuthorisationQuoteDeactivationExceptionLogs, testHandleFailedAuthorisationOrderAlreadyCancelledDoesNothing, testHandleFailedAuthorisationPayByLinkBelowMaxDoesNotCancel, testHandleFailedAuthorisationPayByLinkAtMaxCancels to reflect the new AuthorizationHandler usage and simplified AuthorisationWebhookHandler logic.
  • Removed createAuthorisationWebhookHandler helper method.
• Test/Unit/Helper/Webhook/CaptureWebhookHandlerTest.php
  • Updated testHandleWebhookWithoutAutoCapture to reflect changes in finalizeOrder method signature.
• Test/Unit/Helper/Webhook/ManualReviewAcceptWebhookHandlerTest.php
  • Added new file with tests for ManualReviewAcceptWebhookHandler, verifying markCaseAsAccepted and conditional finalizeOrder calls.
• Test/Unit/Model/AuthorizationHandlerTest.php
  • Added new file with comprehensive unit tests for the new AuthorizationHandler class, covering partial/full authorization, auto/manual capture, fraud review, and email sending logic.
• Test/Unit/Observer/AuthorizeAfterOrderPlacementTest.php
  • Added new file with unit tests for the new AuthorizeAfterOrderPlacement observer, verifying its behavior for different payment responses and error handling.
• etc/events.xml
  • Added adyen_authorize_payment_after_order_placement observer for the checkout_submit_all_after event.

Ignored Files

• Ignored by pattern: .github/workflows/** (3)
  • .github/workflows/e2e-test.yml
  • .github/workflows/main.yml
  • .github/workflows/mftf-test.yml

Activity

• The pull request introduces a significant refactoring by centralizing authorization logic into a new AuthorizationHandler class.
• Several helper methods have been updated to remove direct dependencies on the Notification object, promoting cleaner code and better separation of concerns.
• A new observer has been added to handle authorization after order placement, ensuring that payments are correctly processed even if webhooks are delayed or missed.
• Extensive unit tests have been added or updated to cover the new AuthorizationHandler and AuthorizeAfterOrderPlacement observer, as well as changes in existing helper methods.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a significant and beneficial refactoring by centralizing authorization logic into a new AuthorizationHandler class and adding an observer to handle authorization immediately after order placement. This improves the architecture by enhancing separation of concerns and simplifying webhook handlers. However, I've identified a critical issue in a new test file that will cause a fatal error, a potential regression concerning Boleto payment status handling, and a minor bug in the new observer that could lead to warnings.

[gemini-code-assist]

The constructor for AuthorizationHandler is being called with 6 arguments, but it expects 7. The $dataHelper dependency is missing. This will cause a fatal error when running the tests.

Please also add a mock for \Adyen\Payment\Helper\Data and pass it to the constructor.

        $dataHelperMock = $this->createMock(\Adyen\Payment\Helper\Data::class);

        $this->authorizationHandler = new AuthorizationHandler(
            $this->adyenOrderPaymentHelperMock,
            $this->caseManagementHelperMock,
            $this->invoiceHelperMock,
            $this->paymentMethodsHelperMock,
            $this->orderHelperMock,
            $this->adyenLoggerMock,
            $dataHelperMock
        );

[gemini-code-assist]

The logic for handling Boleto payment statuses, specifically for overpayment and underpayment, has been removed. This was handled by the getBoletoStatus method, which has also been removed from PaymentMethods helper. This appears to be a regression as there is no replacement logic. Was the removal of this functionality intentional?

[gemini-code-assist]

The $response['additionalData'] key may not exist in the payment response, which would cause a warning. It's safer to use the null coalescing operator to provide an empty array as a default.

                $order = $this->authorizationHandler->execute(
                    $order,
                    $response['paymentMethod']['brand'],
                    $response['pspReference'],
                    $response['amount']['value'],
                    $response['amount']['currency'],
                    $response['additionalData'] ?? []
                );