Reviewed & tested by the community
Project:
Commerce Discount
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
26 Sep 2014 at 07:53 UTC
Updated:
28 Jun 2019 at 15:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
tnfno commentedAgree. Also the % off discount sometimes has issues with multicurrency. The total amount are sometimes off due to rounding of decimals, this gives a payment error since the line item total does not add up.
Comment #2
bardzusny commentedHi,
I didn't really have problems with percentage discounts (so far...) - but indeed, fixed discounts on orders in different currencies than default site currency (the one in which we define discount amount) are simply not being handled.
I tinkered up a quick solution, patch included.
Basically: if commerce_multicurrency (the one we need anyway) is enabled, make sure discount is in order currency before applying it to it.
There is still no way to enter discount amount separately for each enabled currency, but it still seems to be enough for majority of use-cases. Comments and suggestions welcome.
Comment #3
skyredwangComment #4
millionleaves commentedI've tried the patch in #2 and it appears to give the correct result - thanks. I've handed it to my customer for more thorough testing and will report back if any issues arise.
One point I'd note is that the patch is currently hard coded to assume the Discount module is in a Commerce Kickstart installation:
When I supplied the correct path/filename, the patch installed without any further problems.
Comment #5
joelpittetThis feels like a rabbit hole... Are there other parts of commerce_discount that will need the same check? Can we do this upstream in commerce instead, maybe with an alter hook that commerce_multicurrency can implement to adjust the order currency?
I'd rather not liter drupal_commerce with multicurrency checks, at most if we must do it here we can add an alter hook that allows them to adjust fix it there but I think I'd prefer commerce to deal with this problem in a more generic fashion.
Comment #6
bardzusny commentedOops, that is correct - this is just a raw diff from installation I happened to work with. Feel invited to upload version with generic file paths.
I'm not very enthusiastic about this solution myself, to be honest. Multi-currency support in Commerce core seems very limited; this patch is just a mere workaround. It's not up to me to decide whether it's good enough of a solution to get included in the main branch - at the very least, you're not the only one having doubts. :)
Comment #7
millionleaves commentedFollow up to my comment in #4.
I have a site with 5 currencies. NZD is the default.
I've created a Discount with a Coupon that will discount an order by NZ$40 if the total order value is $118 or higher.
If I add a product worth NZD$199 to the cart and enter the coupon, it applies successfully and discounts the order correctly.
If I change the currency to GBP, the order value changes to £84 and the voucher no longer works. It appears to be applying the discount based on the absolute number value of the order rather than converting the qualifying rule (>NZD118) to the equivalent GBP before evaluating whether the order qualifies for the discount.
I've got the patch applied, but the issue persists.
It looks like I can go and edit the rule manually to create OR conditions and supply a minimum value for each currency, but it would be tidier if the module handled the currency conversion of the order value on the fly - or allowed the conditions to be set within the Discount settings rather than hacking the rule after the fact.
Comment #8
japerryHere is a patch with a slightly different approach.
We make the commerce module have a context aware default currency, which helps set the currency for a discount. We add a currency row for the offer type, which helps us get a default currency for the offer.
There is also an alter to the view to fetch the currency dynamically, instead of from the entity label cache, which can only really bet set globally.
No interdiff because this really is different from what the original patch was looking to do.
Comment #10
bohemier commented@japerry: I tried your patch but I am not getting a currency selector when adding a discount... Should I expect one?
Comment #11
joelpittetStill think this issue belongs on commerce. I'm not getting a clear picture of what scenario we need to manually change currencies. The discount should use the order or line item's currency, shouldn't it not?
Comment #12
czigor commented@joelpittet We might not need to change the currency we just need to set it once on discount creation (admin/commerce/discounts/add). Our use case is separate discounts for each currency. Do you think that's possible without patching commerce_discount?
Comment #13
czigor commentedI have given this a look and found that this issue is just about field widget of the commerce_fixed_amount field on the commerce_discount_usage entity fixed_amount bundle. The attached patch changes the widget from 'commerce_price_simple' to 'commerce_price_full'.
Comment #14
czigor commented(To make it clear: the patch changes the fixed default currency value and adds a currency dropdown on admin/commerce/discounts/add and discount edit pages.)
Comment #16
czigor commentedComment #17
czigor commentedTests pass locally, this might be just a bot issue.
Comment #21
joelpittetThat sounds like extra work, pardon my ignorance but couldn't you set discount conditions on currency for when to apply the amount change. Why does the discount amount need a currency?
Comment #22
bohemier commentedWhat I have experimented is that the discount is considered to be in the store default currency (USD in my case). So when a customer buys in Euros, the USD amount is converted to the EUR equivalent. This makes it impossible to have for instance 10 off for 100 to 199 value in cart, independent of currency. It works for USD but for EUR it would give something like 8.96.
Also, I couldn't set the amount with rules, as it will pick the amount from the discount configuration.
Comment #23
bohemier commentedThanks for the patch czigor... Multicurrency discounts are now possible!
I think the patch should also include a currency for the condition ('Apply to') amount in the discount config... (although this can be changed by editing the rule, it's not obvious)
Comment #24
czigor commented@joelpittet The discount condition on currency is still needed I guess, i.e. the discount currency has no functionality out-of-the-box, we need to set up one with rules. I just think it's not a very good UX to display always the default currency for the discount when in fact it's something else.
Comment #25
czigor commentedFor the record, I could not find an inline condition for the order currency so I ended up implementing hook_commerce_coupon_condition_outcome_alter() to check if the discount has the same currency as the order. Note, that this would have been *very* tedious without a currency information on the discount: I would have to maintain a list of discounts for each currency. So I really think this patch has a valid use case.
Comment #27
jofitzAttempt to resolve "D7 CI error" - using
array()instead of[].Comment #29
jofitzComment #30
jofitzAlso allow selection of currency when setting Order Discount Conditions (e.g. "Total amount" condition).
Comment #31
mirom commentedI tested patch in #30 and it works nice. The only problem we realised during testing is that coupon is applied also for orders with different currency. Yes, this can be validated through custom rule, but if we expect non-drupal people to create coupons, this needs to be automatic. In our patch we validate this during compatibility check.
Comment #33
joelpittetMay be unrelated but I had this added to commerce to help with default currency changing
#2415237: Change site's default currency through a 'commerce_default_currency' alter hook
Some of you may find this useful
Comment #34
emileacroweb commentedA useful comment at https://www.drupal.org/node/1869722#comment-11861978 fixes a bug where a % discount seems to force the order total into the default site currency.
Comment #35
czigor commentedReroll of #31.
Comment #36
czigor commentedFixing tests.
Comment #37
czigor commentedRerolling patch to apply to latest dev.
Comment #39
czigor commentedUploaded the same patch, this is the correct one.
Comment #40
bramdriesenRe-rolled the patch because there was a conflict. Any updates on this matter? :)
Comment #42
bramdriesenOkay... applied the codesniffer remark. Patch and interdiff attached.
Comment #44
bramdriesenI don't really understand why the tests are failing as I just did a re-roll of the previous patch.
Anyway I did some testing and discovered that you can apply a $ discount coupon to a € price. Meaning there is no validation if the currency of the coupon matches the price of the product/order. This is something I think should get added!
Comment #45
berenddeboer commentedPatch worked fine here. I think this can be released. There currently is no validation anyway of applying a discount in default currency against any other currency, so that issue is not relevan.t
Comment #46
dinakaran.ilango commentedI have Applied the patch provided in #42. It has provided me the currency drop down while adding the discount, But when I try to set the fixed amount discount for the Currency Other Then the default currency the coupon applies but the price doesn't change(it actually increases the discount amount with the unit price and removes the same amount from the updated unit price) am i missing something? is there some thing i need to check on the pricing rules?
Comment #47
dinakaran.ilango commentedI found that the issue is with the custom code that i wrote, not with the patch. So changing the status back
Comment #48
dinakaran.ilango commentedComment #49
bramdriesenAny idea when this could be put in the next release? :)
Comment #50
mnakov commentedI've made a little addition to change the "@currency off" label to "Currency off". It was misleading, because the user will always see the default currency.
Comment #51
mnakov commentedAdding interdiff file between #42 and #50.
Comment #52
mnakov commentedAdding interdiff file between #42 and latest.
Comment #53
grahlFound another issue in this, currencies don't get their decimals assigned correctly:
Sorry, don't have the time to update the patch at the moment.