amzn / amazon-payments-magento-2-plugin

Extension to enable Amazon Pay on Magento 2
https://amzn.github.io/amazon-payments-magento-2-plugin/
Apache License 2.0
109 stars 77 forks source link

Wrong API response because of the incorrect amout format #1070

Closed vkalchenko closed 3 years ago

vkalchenko commented 3 years ago

Pre-conditions

  1. Have an integration with Tax calculation service
  2. Have not-round tax percentage, for example 14.45%

What I expected

Customer successfully can place an order.

What happened instead

Customer stuck on order review step. Reason - request /V1/amazon-checkout-session//update?_=1628604178081 returns empty value

Steps to reproduce the issue

  1. Add product with applicable taxes
  2. Go to checkout
  3. Enter customer data, select shipping method
  4. Select Amazon Pay as payment method
  5. Click Place Order button image Tax calculation formula: itemPrice + itemPrice*taxPercentage + shippingPrice 5 + 5*14.45% + 2 = 7.7225

Your setup

Amazon Pay Request payload

{"webCheckoutDetails":{"checkoutResultReturnUrl":"https:\/\/local.mystore.com\/amazon_pay\/checkout\/completeSession\/"},"paymentDetails":{"paymentIntent":"Authorize","canHandlePendingAuthorization":false,"chargeAmount":{"amount":7.7225,"currencyCode":"USD"}},"merchantMetadata":{"merchantStoreName":null,"customInformation":"Magento Version: 2.4.2-p1, Plugin Version: 5.6.0"},"platformId":"A2ZAYEJU54T1BM"}

Error in response

image

jaybeckr commented 3 years ago

Hi @vkalchenko - thanks for reporting the issue, and submitting the PR!

I am curious which tax service you are using, as it seems like a bad idea to have more than 2 decimal points stored in Magento - it could produce some odd "off by one cent" type rounding issues. We tested using Magento's built in tax calculations and it doesn't store more than 2 decimal points.

Regardless, it certainly shouldn't hurt to round this amount, I believe all currencies other than JPY use 2 decimal but will double check. One issue on the PR is that it would include thousands separator as is, please review the update I made there.

vkalchenko commented 3 years ago

Hi @jaybeckr Thank you so much for the quick reaction. We use TaxJar (and their Magento 2 module). I did some additional research, and this seems like a valid issue on their side, see https://github.com/taxjar/taxjar-magento2-extension/issues/173

I fully agree with the proposed update to PR (it's applied now). I didn't realize that the default 4th parameter in number_format() is comma.

jaybeckr commented 3 years ago

This will be included in the next release, which should be in the next week or two. Can leave the issue open until that is live if you'd like. Thanks!

jaybeckr commented 3 years ago

@vkalchenko - this fix has been released in 5.7.0, thanks again for your contribution! Closing this issue out.