adyen-magento2 icon indicating copy to clipboard operation
adyen-magento2 copied to clipboard

[PW-8375] Possible incorrect implementation of `isFullInvoiceAmountManuallyCaptured`

Open maaarghk opened this issue 2 years ago • 6 comments

Adyen\Payment\Helper\Invoice::isFullInvoiceAmountManuallyCaptured seems like it may have a bug on multi-currency setups.

I'm not sure about generic reproduction steps but this is an accurate description of behaviour I'm seeing in prod:

  • Magento: GBP base currency with alternative currencies enabled
  • Adyen: GBP only
  1. Enable manual capture
  2. As customer select alternative currency (JPY and USD both seem affected)
  3. Place order
  4. As admin, note order with base_grand_total = 1000 GBP, grand_total = 150000 JPY
  5. Create invoice, observe same figures on invoice
  6. Capture from interface and await webhook

Observe:

  • adyen_invoice has amount = 1000; status = successful
  • adyen_order_payment has amount = 1000; status = manually captured
  • isFullInvoiceAmountManuallyCaptured compares $invoiceAmountCents based on sales_invoice.grand_total in alternative currency:
    $invoiceAmountCents = $this->adyenDataHelper->formatAmount(
        $invoice->getGrandTotal(),
        $invoice->getOrderCurrencyCode()
    );
    
    with $invoiceCapturedAmount based on adyen_invoice.amount in store base currency:
    $invoiceCapturedAmount += $adyenInvoice[\Adyen\Payment\Api\Data\InvoiceInterface::AMOUNT];
    
    and therefore returns false
  • Magento\Sales\Model\Order\Invoice::pay() is never called and order remains in pending state

Expected behavior isFullInvoiceAmountManuallyCaptured should be aware of the adyen_invoice.amount currency and do a comparison on the appropriate sales_order.<base_>grand_total amount

Magento version

            "name": "adyen/module-payment",
            "version": "8.14.0",
            "name": "adyen/php-api-library",
            "version": "13.0.4",
            "name": "adyen/php-webhook-module",
            "version": "0.6.0",
            "name": "magento/magento2-base",
            "version": "2.4.3-p3",

maaarghk avatar Feb 10 '23 14:02 maaarghk

Hi @maaarghk,

Apologies for the delayed response. Just to ensure that we understand the problem, the issue is that ultimately in the isFullInvoiceAmountManuallyCaptured function we are comparing:

  • adyen_invoice.amount to sales_order.grand_total

whereas based on your suggestion we should be comparing:

  • adyen_invoice.amount to sales_order.base_grand_total

Is that correct? Also, since you mentioned that this is something you think is happening on prod, were you able to replicate it on a test environment as well?

Regards, Jean Adyen

Morerice avatar Feb 27 '23 12:02 Morerice

Hey, yes, that is an accurate summary. My summary is just based on reading the code and observing in the database that the adyen_invoice table always contains base currency. I don't really have the time to create a reproducible test case for this unfortunately but if you have a magento test environment somewhere with multi currency + manual capture configured I can log in to it and compare the configuration / place some test orders demonstrating the issue, if that's something that would be useful.

maaarghk avatar Mar 02 '23 13:03 maaarghk

@Morerice has there been any internal investigation of this yet? I'm still seeing this problem where capturing an order which is in a different currency than the store base currency does not get moved to processing.

If you have a test magento 2 instance somewhere you can provide credentials for, I can try to add a store view which exhibits this behaviour

maaarghk avatar Apr 04 '23 19:04 maaarghk

Hello @maaarghk,

We are still investigating this issue and as a part of this investigation we found a bug that pops-up while inserting values to adyen_invoice table. That bug will be resolved and released as soon as possible.

Besides that, we realised that the configuration field payment/adyen_abstract/charged_currency creates a confusion and has inconsistent implementations. We are also revisiting that configuration field and the usage in the plugin. As you mentioned, that comparison is wrong if the config field is set to base.

Meanwhile, could you please share us the value of the configuration field payment/adyen_abstract/charged_currency in your database?

Once again, thank you for raising this issue and for your patience.

Best Regards, Can

candemiralp avatar Apr 24 '23 11:04 candemiralp

Hi, the default scope value is display, but indeed on all store views it is overridden to base. This matches the observed behaviour where a GBP amount is seen in Adyen for the order, therefore adyen_order_payment has amount in GBP.

maaarghk avatar Apr 27 '23 14:04 maaarghk

Hello @maaarghk,

Thank you for the update. Indeed, that is an expected issue for the value base for this possible wrong comparison. Meanwhile, we are revisiting the charged_currency configuration field. But I assume the issue is now clear with the latest update from your side. I will create a ticket to solve this issue.

Once again, thank you for raising this issue.

Best Regards, Can

candemiralp avatar Apr 28 '23 14:04 candemiralp