Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omnipay itemBag price should be converted to payment currency #23

Closed
wants to merge 2 commits into from

Conversation

joostwaaijer
Copy link

@joostwaaijer joostwaaijer commented Jun 24, 2021

Description

Fixes bug when trying purchase items in a secondary currency, in stead of the primary currency

Related issues

#22

@boboldehampsink
Copy link
Contributor

@lukeholder any chance of merging this?

@boboldehampsink
Copy link
Contributor

@nfourtythree maybe?

@nfourtythree
Copy link
Contributor

Hi @boboldehampsink & @joostwaaijer

We will take a review of this to see if this is something we can merge. Commerce only supports paying in a different currency and for that method, we only convert the payment amount (usually the order total). Converting each line item's price could potentially introduce totalling errors where conversion rate would mean that the sum of their totals might not add up to the order total.

Will update this PR when we have further info.

Thanks!

@boboldehampsink
Copy link
Contributor

@nfourtythree thanks. I see what you mean but right now it is also buggy, see #22

@nfourtythree
Copy link
Contributor

Certainly, @boboldehampsink, as I mentioned we will review and see what the best course of action is here.

#22 does relate to the, now deprecated, PayPal gateway plugin. The superseded PayPal gateway plugin does deal with this, albeit in a rudimentary way by not sending the breakdown. Unfortunately, this is a limitation at the moment, as mentioned, due to the way Commerce deals with paying in a different currency to the base currency.

We will update you when we have more information, thanks!

@boboldehampsink
Copy link
Contributor

@nfourtythree any information yet?

@nfourtythree
Copy link
Contributor

Hi All

We have spent a good amount of time looking into and discussing this. The limitation still stands that because Commerce only ever converts the payment amount/order total to the payment currency we can't integrate this PR. This is due to the reasons already stated that there is a chance that converting all the line item prices might end up in a situation where the items do not total up correctly. This could cause problems with gateways and that is something we need to avoid.

In future versions of the gateways we are also looking at the fact that if the payment is in a different currency to the base currency, it is a partial payment or you are paying an outstanding balance that is part of the order total we may have to dynamically turn of the "send cart info". This will be a decision that is made on a per gateway scenario depending on what their individual platforms/APIs accept/expect.

For now, your best bet here is to use the Gateway::EVENT_AFTER_CREATE_ITEM_BAG event, this gives you the opportunity to overwrite the item bag with whatever data you see fit, so if you would like to convert the currency you can do. This would be then be left up to you to make sure to meet the gateway's requirements.

Hope this explanation and guide to a solution helps, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants