magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.5k stars 9.3k forks source link

Shipping options sent to Paypal only if catalog prices are excl. taxes #35877

Closed OvalMedia closed 1 year ago

OvalMedia commented 2 years ago

Summary (*)

Even though the paypal configuration option "Transfer Shipping Options" is set to yes the options will not be sent to paypal unless the catalog prices exclude taxes.

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Paypal/Model/Express/Checkout.php#L561

if (!$this->_taxData->getConfig()->priceIncludesTax()) {
    $this->setShippingOptions($cart, $address);
}

Why is this?

This will render the set option in the configuration useless.

m2-assistant[bot] commented 2 years ago

Hi @OvalMedia. Thank you for your report. To speed up processing of this issue, make sure that you provided the following information:

Make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, review the Magento Contributor Assistant documentation.

Add a comment to assign the issue: @magento I am working on this

To learn more about issue processing workflow, refer to the Code Contributions.


:clock10: You can find the schedule on the Magento Community Calendar page.

:telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

:pencil2: Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

mklooss commented 2 years ago

PSD2 (payment law does not allow "overcharge") and most european online shops filling the prices incl. tax.

More detailed information: https://www.paypal.com/us/smarthelp/article/FAQ4645

Example:

  • Merchant sends the buyer to PayPal to authorize a transaction of 100.00 USD.

  • Consumer reviews and authorizes the transaction at PayPal for 100.00 USD.

  • Consumer returns to the merchant site where the transaction amount increases to 110.00 USD due to the addition of shipping, taxes, FX conversion, etc.

m2-assistant[bot] commented 2 years ago

Hi @engcom-Echo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:


engcom-Echo commented 2 years ago

Hello @OvalMedia,

Thanks for contribution and collaboration. Please provide the expected and actual result and provide more information with exact steps and screenshot so that we can reproduce this issue.

Thanks

engcom-Hotel commented 2 years ago

Dear @OvalMedia,

We have noticed that this issue has not been updated for a period of 14 Days. Hence we assume that this issue is fixed now, so we are closing it. Please raise a fresh ticket or reopen this ticket if you need more assistance on this.

Regards

mklooss commented 2 years ago

@engcom-Echo and @engcom-Hotel

Most of our M2 Stores a stores in EU (Germany)... and Prices will be filled in with Taxes (19 %). So the Tax Configuration is "prices include tax". But PayPal Express does not send the Shipping Options to Paypal bcz. of this line

https://github.com/magento/magento2/blob/caf390a9cde8ef2eaa7287492a5ab01e7ef15670/app/code/Magento/Paypal/Model/Express/Checkout.php#L560-L562

What we Expect the Shipping Dropdown on the Paypal Page ¯_(ツ)_/¯.

bitumin commented 1 year ago

hi @mklooss do you know any good solution for this? I am tempted to allow set shipping options even if prices includes tax (at least when there are shipping amounts in the quote), but I would like to know if someone has tested the situation before (and found any issues with this approach).

mklooss commented 1 year ago

Hey an Solutions, yes, an Good Solution, no:

You can use this Code https://fishpig.co.uk/magento/tutorials/automatically-set-shipping-method-magento-2/

an Create two "Plugins"

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <type name="Magento\Paypal\Controller\Express\Start">
        <plugin name="paypal_fix" type="Custom\Paypal\Plugin\Controller\Express\StartPlugin" />
<!-- NVP API -->
    </type>
    <type name="Magento\Paypal\Controller\Express\GetTokenData">
        <plugin name="paypal_fix" type="Custom\Paypal\Plugin\Controller\Express\GetTokenDataPlugin" />
<!-- Context API -->
    </type>
</config>

The Main Store CountryId will be set as Shipping Country so the Customer has the store country shipping costs at paypal

bitumin commented 1 year ago

@mklooss @engcom-Echo @engcom-Hotel I have been studying the issue for a while and it seems to me the only good approach, in case paypal returns an error 10486 due to a substantial change in amounts (for example, due to the addition of shipping amounts during magento payment review), is to implement (ideally, in the core magento paypal module) the Second redirect flow documented here: https://developer.paypal.com/docs/archive/express-checkout/integration-guide/ECReference/#second-redirect-flow

Basically, during handling of the error 10486 (if the error cause is suspected to be the difference in amounts sent during SetExpressCheckout as compared to the ones sent during DoExpressCheckoutPayment), we should do a second SetExpressCheckout api call reusing the original (current) paypal token. The customer will be again redirected (from magento) to paypal, and again redirected (from paypal) to magento review, and finally (after review confirmation) the payment will succeed (the second DoExpressCheckoutPayment will succeed).

I don't see any reliable way to approach this issue by trying to provide the "right" shipping amount during the first SetExpressCheckout, but if someone realises how... or in case its safe to send the shipping options to paypal when amounts include taxes... let me know, because that way we would not need to implement the additional redirection flow (which is not very user friendly, IMO) :)

m2-assistant[bot] commented 1 year ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Hotel commented 1 year ago

Hello @mklooss @bitumin @OvalMedia,

We have tried to reproduce this issue in the Magento latest development branch i.e. 2.4-develop, but the issue is not seem to reproducible for us.

In both the cases, either prices are including tax or excluding, we can see the shipping information at the Paypal end. Please have a look in to the below screenshot for reference:

Excluding: Excluding

Including Including

Please let us know in case we have missed anything. We are reopening this ticket for further analysis.

Thanks

OvalMedia commented 1 year ago

Your screenshots are referencing shipping COSTS. The issue at hand are the missing shipping OPTIONS on the paypal side so buyers can pick from available delivery methods. My initial post clearly points to the issue. The code explicitly skips the transmission of shipping options if prices include taxes.

bitumin commented 1 year ago

@engcom-Hotel The missing shipping options in paypal issue is obviously due to the piece of code @mklooss pointed out and @OvalMedia has accurately described.

The missing shipping costs issue that causes error 10486 only happens when initiated paypal express payment from a paypal button in PDP or minicart (not normal checkout), when there is no shipping costs pre-calculated yet in magento side (for example, no address has been provided yet, guest user, no paypal session initiated before, etc.). In this case there is no shipping amounts in the quote and therefore none are passed during first SetExpressCheckout API call (as you will see in payment.log). IMO The issue that better describes this second case is https://github.com/magento/magento2/issues/36068 (the title of the issue could be improved, though).

The thing is, passing shipping options (when prices incl. tax) could probably fix both issues. But according to https://github.com/magento/magento2/commit/45d86d5d1ff7eda20e7a874bf0701bf49eb8c31d it might introduce paypal amount mismatch errors.

engcom-Hotel commented 1 year ago

Hello @bitumin,

Thanks for the clarification!

It looks like both the issues https://github.com/magento/magento2/issues/36068 and this one are identical.

And this issue https://github.com/magento/magento2/issues/36068 is marked as a feature request. So as per the process, we can close this issue as a duplicate of #36068.

engcom-Hotel commented 1 year ago

Dear @bitumin,

We have noticed that this issue has not been updated for a period of 14 Days. Hence we assume that this issue is fixed now, so we are closing it. Please raise a fresh ticket or reopen this ticket if you need more assistance on this.

Regards

OvalMedia commented 1 year ago

This isn't fixed, you just chose to ignore it.

engcom-Hotel commented 1 year ago

Hello @OvalMedia,

Actually, there is a similar issue marked as a feature request that's why I closed this. Please refer my https://github.com/magento/magento2/issues/35877#issuecomment-1436569176 here.

Thanks