When discounts are very small and are applied to large volumes of inexpensive products, they are rounded and end up differing a lot from the intended discount. Below is a screenshot of 1% discount applied to a volume of 5000 boxes costing $0.74 each. The discount is $50 instead of $36 because of rounding. Please review the attached patch to remove the rounding.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexrayu’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
856 bytes

Attaching a proposed patch.

torgosPizza’s picture

Possibly a duplicate of this (older) issue: #2238723: Price component not rounded causing line item and order total mismatch - should consider closing one in favor of the other.

(I am almost inclined to close that other one, as this patch seems cleaner, and not rounding discounts at all may be a safer bet.)

deggertsen’s picture

Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community
alexrayu’s picture

joelpittet’s picture

Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

Just want to make sure this is correct because isn't there cases where the discount amount would be a decimal?

Amounts are stored as integers and it would likely get truncated or rounded on storage?

alexrayu’s picture

Hello joelpittet, discount amounts are stored as decimals (10,2). This issue relates to #2468159: Percentage based discounts : validation needs to be elsewhere, which allows to add discounts like 0.05%

joelpittet’s picture

@alexrayu I could be wrong but have a look here, it looks like an INT to me:

+++ b/commerce_discount.rules.inc
@@ -534,10 +534,10 @@ function commerce_discount_add_price_component(EntityDrupalWrapper $line_item_wr
-  $updated_amount = commerce_round(COMMERCE_ROUND_HALF_UP, $current_amount + $discount_amount['amount']);
+  $updated_amount = $current_amount + $discount_amount['amount'];
...
-    'amount' => commerce_round(COMMERCE_ROUND_HALF_UP, $discount_amount['amount']),
+    'amount' => $discount_amount['amount'],

Stored on

$line_item_wrapper->commerce_unit_price->amount = $updated_amount;

So without the rounding it may truncate decimal values on the amount.

alexrayu’s picture

@joelpittet: That is correct. The item price is converted to integer and saved as int. So, $2.25 is converted to 225 and thus saved. It is formatted into $2.25 by the discount module again when retrieved. And the discount amount is stored as decimal with precision of 2.

alexrayu’s picture

Issue summary: View changes
FileSize
54.7 KB
33.89 KB
alexrayu’s picture

Issue summary: View changes

A dupe comment.

alexrayu’s picture

Discounts:

Prices:

joelpittet’s picture

Status: Postponed (maintainer needs more info) » Needs work
Issue tags: +Needs tests

@alexrayu I don't really think those screenshots helped as we aren't talking the same thing... I'm willing to commit this if we can provide some tests to show that the calculated totals are incorrect caused by the rounding.

I've turned on the automated testing on this project and trying to fix the tests that are there, there is currently around 10 failing tests and half of them are to do with import/export.

Also plan on sending out another release once the tests are working.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
856 bytes

Reposting to trigger testbot. I fixed the old tests and so -dev should have all it's original tests passing.

joelpittet’s picture

Hiding other files.

joelpittet’s picture

@alexrayu I don't mind trying to write up the test if you can hook me up with scenario in which it will fail currently?

Status: Needs review » Needs work

The last submitted patch, 13: remove_rounding_of-2468943-13.patch, failed testing.

Status: Needs work » Needs review
alexrayu’s picture

Thanks joelpittet. The scenario is this. There are products, that cost $0.74 each. A discount for 0.5% is set for cases, when more than 10000 products have been bought.

In such scenario, you will notice, that the discount amount calculated by the module, is strikingly different from expected. See the screenshot in the first post. The solution is to no round.

In fact, even without rounding there still will be a small discrepancy because of rounding to cents, which happens inevitably. but that small discrepancy is insignificant, though may make testing problematic. The result without rounding, though, can have huge consequences for small prices and large quantities.

Please tell me if I can help more.

Status: Needs review » Needs work

The last submitted patch, 13: remove_rounding_of-2468943-13.patch, failed testing.

joelpittet’s picture

Those tests work locally I just need to get the test_dependencies setup correct I think. Thanks @alexrayu that should do the trick, we may need to make some decisions in the test to reasonable rounding as you mentioned in #18

torgosPizza’s picture

Not sure how easily testable it is but in our experience the rounding issues can affect checkout with PayPal, which totals up the cart as well as the individual cart line items. If those prices don't match when added on the PayPal side, the order will be rejected with a vague error (if any shows up at all). So users could be attempting to checkout with PayPal only to have the Checkout Review page get reloaded with a silent failure, and when you check Watchdog you'll see the error that the totals in the cart and items don't match.

I think I have some screengrabs with dpm() output if that would help. The method to reproduce it was to just have a % product discount, one or two items in the cart. I could reproduce the issue nearly every time.

I've patched the module to remove rounding in Prod and we haven't had the issue since.

joelpittet’s picture

@torgosPizza are you using this patch?

torgosPizza’s picture

Actually no, I was using patch #1 from #2238723: Price component not rounded causing line item and order total mismatch but I can swap it out to see if this one also satisfies out needs.

alexrayu’s picture

Assigned: alexrayu » Unassigned

Status: Needs work » Needs review
joelpittet’s picture

Let's see how the tests that are enabled are going to treat this as is now that they are working(most of them)

poulou’s picture

Hello!

My case: I have a product with 104.05€ and I apply 30% discount
(I only changed the price field in the view to "price with components")
Before patch:
before patch

After patch:
after patch

The problem is that, for the end customer, the Order total should end with a x.x3€ (because of x.x5€ - x.x2€)

I can tolerate rounding or truncation of any kind, as long as the numbers work for him

Thank you

joelpittet’s picture

Priority: Critical » Normal

Thank you @poulou, so it may be worth leaving it in there.

Also this is not critical.

torgosPizza’s picture

Status: Needs review » Needs work

The screenshots in #27 make it seem like the patch is not working correctly? Since the rounding is incorrect "after" the patch is applied. Is this an accurate statement? If so, setting back to Needs work.

AdamGerthel’s picture

Just throwing my two cents in there: I don't think removing rounding sounds like a good idea. Maybe having the option not to round would be better. In my case, I'd actually have a bigger rounding that is currently implemented. Our products often end up costing 672 or 1022 where 670 or 1020 would be better. Or maybe even rounding to nearest 50.

joelpittet’s picture

Hmm interesting idea. How about a variable_get('discount_rounding_strategy', COMMERCE_ROUND_HALF_UP)?

$rounding_strategy = variable_get('discount_rounding_strategy', COMMERCE_ROUND_HALF_UP);
$updated_amount = commerce_round($rounding_strategy, $current_amount + $discount_amount['amount']);

Then if you don't want it, change your settings.php to:

$conf['discount_rounding_strategy'] = COMMERCE_ROUND_NONE;

FYI I was using this patch but ran into a similar issue as #27 today so dropped the patch from my production site.

torgosPizza’s picture

@joelpittet By "this patch" do you mean the one in #13? If so, what is your solution now? I'd like to get this fixed as it has the potential to break PayPal orders (as was mentioned in #2238723: Price component not rounded causing line item and order total mismatch)

joelpittet’s picture

Plan is write a test that proves either is the way we want if possible.

If not have it as a configurable variable.

Feel free to have a crack at a test.

torgosPizza’s picture

I've been meaning to get into learning some Simpletest. This might be a good way to start. Thanks!

Ser_Mir’s picture

Hello

Check my updated patch please. The problem was totally resolved for me with moving rounding functional to commerce_discount_percentage() function calculation step

joelpittet’s picture

@Ser_Mir see #33, we need tests to prove and continuously assert that changes don't break the rounding. We've have conflicting reports that this rounding helps and for others it does not.

That's why tests are important. You're word is only as good as your tests:P

alexrayu’s picture

Maybe the tests issue should be brought out into a separate ticket? They are not resulting from the edits in this issue, I think, are they?

joelpittet’s picture

@alexrayu not sure I understand your question?

We need tests to prove that the rounding results can be asserted as they are expected. To prove that this is a bug in HEAD and/or the patch will solve the issue if there is indeed failing expected results.

alexrayu’s picture

@joelpittet - It seemed to be a fast and small fix initially - so I am wondering whether those test fails are coming from the changes that we have made in this ticket, or is it a code debt that accumulated over time as the module was developed, that is not directly related to rounding, in which case it could be made another ticket.

joelpittet’s picture

@alexrayu ok still confused, can you please point to what you are referring to? Which test fails?

alexrayu’s picture

@joelpittet - Please ignore my response for now. I am catching up and testing the patches proposed above. I will respond if I have something meaningful to propose.

alexrayu’s picture

Assigned: Unassigned » alexrayu
alexrayu’s picture

@joelpittet For some reason, patch from #35 broke the discount amount for unit prices < $1. I have also used the latest dev version of the module and re-rolled the patch, implementing your suggestion about the rounding strategy variable.

Now, this is not set to 'Needs review', because there is a tests error I referred about. I am not sure if everyone has it, but it goes like this:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /batch?id=13&op=do StatusText: OK ResponseText: ( ! ) Fatal error: Class name must be a valid object or a string in /home/alex/www/sites/commerce_kickstart/includes/common.inc on line 8108 Call Stack #TimeMemoryFunctionLocation 10.0001242552{main}( ).../index.php:0 20.02374690000menu_execute_active_handler( ).../index.php:33 30.02384691496call_user_func_array:{/home/alex/www/sites/commerce_kickstart/includes/menu.inc:519} ( ).../menu.inc:519 40.02384691720system_batch_page( ).../menu.inc:519 50.02384691824_batch_page( ).../system.admin.inc:2379 60.02384694496_batch_do( ).../batch.inc:80 70.02384694696_batch_process( ).../batch.inc:161 80.02494727128call_user_func_array:{/home/alex/www/sites/commerce_kickstart/includes/batch.inc:284} ( ).../batch.inc:284 90.02494727320_simpletest_batch_operation( ).../batch.inc:284 100.02735117120DrupalTestCase->run( ).../simpletest.module:178 1156.89434060912CommerceDiscountTest->testCommerceDiscountUIAddDiscount( ).../drupal_web_test_case.php:517 1260.02074337320entity_load_single( ).../commerce_discount.test:230 1360.02074337624entity_load( ).../entity.module:231 1460.02074337808entity_get_controller( ).../common.inc:8072

I will debug it as well, but it seems like something not belonging to this ticket. Can someone please verify? I will open another issue then.

torgosPizza’s picture

You're using Commerce Kickstart - are the versions of Commerce Discount etc. the latest -dev of those?

alexrayu’s picture

Only commerce_discount is the latest.
Upd: Have set up latest D7 + modules set up, running tests. Have been running for some time now, so I guess, that makes it for the tests error.

@TorgosPizza, you said you were working on the tests for this issue. Do you happen to already have something written for it? Or scenario cases probably that I can put into test?

joelpittet’s picture

Please open a new issue for that

torgosPizza’s picture

@alexrayu: I don't have any tests yet, in fact I had to switch to other stuff for the time being.

Scenarios are basically just percentage discounts and how they are totaled at the cart, and how they are totaled by a payment gateway. The issue we found that was most important was the fact that the way Commerce PayPal loops through and totals up items for submission to the API is how I discovered this issue - the totals did not match what they were in the cart due to the rounding issue. I'm not sure how possible it is to mock up a PayPal order but perhaps there is something to be gleaned from that.

Perhaps @joelpittet has some more robust ideas for testing scenarios?

joelpittet’s picture

@torgosPizza don't need robust, just need a fire starter:) A couple tests that assert expected rounded values.

joelpittet’s picture

We can iterate and improve on the tests.

alexrayu’s picture

Assigned: alexrayu » Unassigned

Something is still wrong with my setup. Test of only 1 issue (Drupal Discounts) runs for half and hour and then results in 'SQL gone away for lunch'. I guess I will have to step out till my issue is resolved, and only hope that my latest patch is helpful.

alexrayu’s picture

May I ask someone to run a commerce discount test with the latest dev, to confirm that the tests are broken in the latest dev? I will open the issue then.

mglaman’s picture

alexrayu, as an FYI don't try to run simpletest for modules if you've installed via a profile - the modules won't be found :) Commerce Discount installs the "minimal" profile for each test.

mglaman’s picture

Status: Needs work » Needs review

Marking Needs Review to trigger #43

alexrayu’s picture

@mglaman - ah I see. I will wait for the reviewing then.

For all who test, please test the patch especially in relation to #27. Figuring out how to prevent a 1 cent discrepancy has been the biggest challenge.

torgosPizza’s picture

I think one possible way to test the discrepancy would be to do the following case:

1. Create an order that has one product in it.
2. Create both a 15% off order discount and a 15% off product discount.
3. Apply the order discount to the order total,
4. Apply the product discount to each product line item and total that up.
5. Compare the results from 3 and 4.

This is basically what was happening with PayPal - the order total did not match the sum of the product amounts when a product-based total when each item was summed up on its own. See the function commerce_paypal_ec_itemize_order() (and, in fact, I just noticed that module is also rounding up taxes:

// Determine the order level item amount and tax amount line items. To prevent
  // rounding problems getting in the way, we calculate them based on the order
  // total instead of tallying it from each line item.
  if (module_exists('commerce_tax')) {
    $taxamt = commerce_round(COMMERCE_ROUND_HALF_UP, commerce_tax_total_amount($order_total['data']['components'], FALSE, $currency_code));
  }
  else {
    $taxamt = 0;
  }

so that may be a related issue there, and it's possible that removing all rounding will also solve our test case. (I would like to know the rationale behind rounding up half way to avoid tax issues, especially given the problem introduced when using a % off discount).

mglaman’s picture

Ok, so I'm trying something out here. I feel like we're tackling a LOT of issues here around rounding in general and losing scope (well, it's all in scope, but let's fix one thing and then solve all the things.)

Original issue: 5000 * 0.75 * 0.01 for discount was not equating properly. It was rounding and giving customers a significant discount. Solution? Control rouinding.

Here is a patch which adds the ability to configure the rounding of a percentage discount, defaulting to the current COMMERCE_ROUND_HALF_UP. I've added a test which replicates the exact issue reported in the issue summary. It sets the discount to use COMMERCE_ROUND_NONE and passes. Other tests should still pass, because using default rounding.

This, in my opinion, is a proper approach to allow granularity since percentages can get bonkers in bulk or minuscule amounts.

BR0kEN’s picture

Status: Needs review » Reviewed & tested by the community

Works fine! Lets have it merged!

BR0kEN’s picture

BR0kEN’s picture

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
5.94 KB

Rerolled.

DamienMcKenna’s picture

torgosPizza’s picture

This is still not working correctly, or there is more to it than this.

We currently have a Discount for 15% off for Columbus Day with it configured to "Do not round anything". I added a bunch of products that end in $.99 for the price, and added the discount.
Base price: 3994
Discount: -509.1
Bringing the total to -2884.9, but this displays at Checkout as $28.85.

So it seems that the Commerce Order Total does not follow the same rules for rounding, leaving us with a $0.01 discrepancy, which in turn breaks PayPal.

The issue is only resolved when I select "Round the half up" which doesn't really make a lot of sense to me. Where is the rounding happening in the first place?

EDIT: As I'm digging, it looks like the function commerce_paypal_price_amount($amount, $currency_code) does not yet follow the behavior for rounding discounts, and does a commerce_currency_round() on it. Either it should follow a rounding type (make it configurable) or it should simply not round at all by default:

/**
 * Formats a price amount into a decimal value as expected by PayPal.
 *
 * @param $amount
 *   An integer price amount.
 * @param $currency_code
 *   The currency code of the price.
 *
 * @return
 *   The decimal price amount as expected by PayPal API servers.
 */
function commerce_paypal_price_amount($amount, $currency_code) {
  $rounded_amount = commerce_currency_round($amount, commerce_currency_load($currency_code));
  return number_format(commerce_currency_amount_to_decimal($rounded_amount, $currency_code), 2, '.', '');
}
torgosPizza’s picture

Issue tags: +commerce-sprint

Tagging. To quote Ryan in IRC today:

rszrama hates rounding.
rszrama:	we should just stop doing that

;)

rszrama’s picture

Sorry, I'm coming into this late, but I'll be pushing for a solution this evening.

The real issue here is that commerce_discount_add_price_component() should be receiving rounded values only. It's mixing concerns for that function to do more than just add a price component (as the name indicates).

I like the idea of adding rounding settings, though, as Matt's patch does (but we'd need to move rounding out of the price component function's scope). I just think we might want to move it out of the scope of this issue and then ensure it addresses any Discount offer that's based on a percentage. It's going to be quite cluttery in the UI, though, so it might need to be just a blanket setting - but I also don't mind just deciding this for merchants so they don't have to think about it.

A discount should always be "rounded up" in the sense that the extra half cent goes to the customer. Therefore, the quickest fix to this function will be to round the $discount_amount once at the top of the function and then use the rounded variable to set the line item's unit price amount and discount price component amount. I don't understand why the original author called commerce_round() twice, but for that to have been successful, it would have to have been reversed:

  // Calculate the updated amount and create a price array representing the
  // difference between it and the current amount.
  $updated_amount = commerce_round(COMMERCE_ROUND_HALF_DOWN, $current_amount + $discount_amount['amount']);

  $difference = array(
    'amount' => commerce_round(COMMERCE_ROUND_HALF_UP, $discount_amount['amount']),
    'currency_code' => $discount_amount['currency_code'],

For a $10.25 product with a 30% discount, this would produce as expected a $3.08 discount with a line item unit price of $7.17.

Patch attached is my light fix of just rounding once.

I do wonder why commerce_price_component_add() wasn't good enough for Commerce Discount.

torgosPizza’s picture

Status: Needs review » Reviewed & tested by the community

Ryan's approach gets my vote. It also fixes my issue altogether.

I agree that rounding is problematic and I can't really ever see when someone would want to change how prices are rounded with regard to negative adjustments, and especially not on a per-discount level. I can only imagine the chaos that would ensue if someone had even just a few discounts that were all doing different rounding schemes. A rounding scheme is nice in theory but in practice leads to headaches and for shop owners with PayPal, lost sales - and that's the worst part.

Ryan's fix is working for me, and I would highly suggest we roll it into the module. I imagine it will fix the vast majority of issues on d.o. that involve rounding with discounts (and those interactions with PayPal!) and probably others I haven't thought of. RTBC from my point of view!

rszrama’s picture

And it's worth pointing out this only affected discounts that came through Commerce Discount, which is why I could never reproduce these issues. Until recently, I've avoided this module like the plague. ; )

torgosPizza’s picture

Which is interesting because lately I've found myself reusing tons of stuff. I hardly ever do any custom code anymore... I patch modules where I think the feature I need will be useful to others, and use that custom version in production ... to test it. :)

joelpittet’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: -Needs tests
FileSize
3.41 KB

Re adding the tests #60 and assertions to the

For a $10.25 product with a 30% discount, this would produce as expected
a $3.08 discount with a line item unit price of $7.17.

I wrote these fast and they may be wrong but I'd like to see a bit of test coverage here before we commit this. If this fails I'll dig into it further tomorrow at BadCamp sprints.

torgosPizza’s picture

Looks good to me!

Status: Needs review » Needs work

The last submitted patch, 68: remove_rounding_of-2468943-68.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
3.46 KB
1.9 KB

Let's try that again. I think the discount on the bulk should be 3700, but I could be wrong.

Status: Needs review » Needs work

The last submitted patch, 71: remove_rounding_of-2468943-71.patch, failed testing.

rszrama’s picture

@joelpittet So, reviewing the OP and your first assertion, I believe this issue will need to be left open and the following test deferred to a later date:

    // Create a .01% discount applied to a $0.74 product.
    $discount = $this->createDiscount('product_discount', 'percentage', 0.01, 'chintzy_bulk_discount');
    $bulk_product = $this->createDummyProduct('BULK-PRODUCT', 'Bulk discount testing', 74);

    // Create the order and apply discount.
    $order = $this->createDummyOrder($this->store_customer->uid, array($bulk_product->product_id => 5000), 'completed');
    $order_wrapper = commerce_cart_order_refresh($order);

    // Verify rounding came out properly.
    $price = $order_wrapper->commerce_line_items->get(0)->commerce_total->amount_decimal->value();
    $this->assertEqual($price, 3700, 'Rounding does not inflate discount amount');

That is testing something other than my patch satisfies. In fact, I think this thread got hijacked from the OP. The OP actually does need a more robust rounding solution as implemented by Matt in comment #56 above, because what he wants is to defer rounding of the discount from the unit price level to the line item total price level. This is done by not rounding when the discount is calculated and allowing it to be rounded when the order total is finally determined, much the same as with sales taxes.

I'm inclined in this case to suggest the OP should really just be using a pricing rule. If the discount had been implemented via a simple product pricing rule, a core feature of Drupal Commerce, instead of via Commerce Discount, the rounding strategy would have already been selectable and the problem would be avoided.

However, the comments still brought to light the bug I patched in comment #64, so I'm going to revise the rounding test to test that with some explanation in the comments and then reset this issue to a feature request for rounding strategies in Commerce Discount created discounts.

rszrama’s picture

Status: Needs work » Needs review
FileSize
3.33 KB

Good grief I lost a lot of time to this. We had an undocumented "feature" that prevented a discount name beginning with a number from working via $this->createDiscount()... so I documented that to hopefully save someone else hours.

I also found that I had put the patch up with rounding down instead of rounding up like I'd intended. That was stupid.

Patch attached.

rszrama’s picture

Ok, I fixed a typo in the comments and moved them to the docblock. I'm going to commit that, close out this issue with all of its history, and spawn a new one to discuss providing rounding options where percentage discounts are concerned.

rszrama’s picture

Status: Needs review » Fixed

Aaaaand committed.

  • rszrama committed 51ed85a on 7.x-1.x
    Issue #2468943 by joelpittet, rszrama, alexrayu, mglaman, DamienMcKenna...

Status: Fixed » Needs work

The last submitted patch, 75: 2468943-74.fix_discount_price_component_rounding.patch, failed testing.

rszrama’s picture

Title: Remove rounding of discounts » Ensure proper rounding of price amounts and components when components are added via discounts
Status: Needs work » Fixed

Yes, yes, I know. Thanks for the warning, bot, and try not to test patches that have already been committed. : P

Retitling the issue so it's clear what was fixed here.

torgosPizza’s picture

This makes me so.. happy... *single tear*

Thank you Ryan! I understand what it's like to lose hours to this issue, trust me. Your hard work is appreciated.

alexrayu’s picture

Thank you all contributors! This issue has taken much more effort than I expected. Thank you for taking this issue through.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

BR0kEN’s picture

Status: Closed (fixed) » Needs work

Unfortunately I can't say that this issue fixed, as for me. Cases from my project:

Enzira (10 doses), quantity = 5, amount = 262.50 £
if discount = 5%:
Expected result for discount amount -----> 5% of 262.50 £ = 13.125 £
Actual result: 13.15 £
if discount = 6%:
Expected result for discount amount -----> 6% of 262.50 £ = 15.75 £
Actual result: 15.80 £
CSL GENERIC (10 doses), quantity = 10, amount = 659.00 £
if discount = 14%:
Expected result for discount amount -----> 14% of 659.00 = 92.26 £
Actual result: 92.30 £

#56 - coolest patch in this thread.

torgosPizza’s picture

Expected result for discount amount -----> 5% of 262.50 £ = 13.125 £
Actual result: 13.15 £

From 13.125 to 13.15? That sounds like it's not even in the same ballpark as this original issue.

Can you describe more what types of discounts you're using? If this issue were still happening for you I would expect 13.125 to be rounded up to 13.13 but it seems like you have some other issues as well. I'm afraid we're going to need more info.

BR0kEN’s picture

I use percentage discounts and this line of code really surprise me: $discount_amount['amount'] = commerce_round(COMMERCE_ROUND_HALF_UP, $discount_amount['amount']);.

My problem solved by changing the constant to "COMMERCE_ROUND_NONE" or remove that line.

joelpittet’s picture

Status: Needs work » Closed (fixed)

This is a tricky issue. It may have not solved all the issues but it does solve a bit. Please open up a follow-up issue with your specific details and maybe a patch with some tests to show the problem or even just steps to reproduce so someone else can write a test. Every slightly different use-case that breaks with rounding is not going to be solve in issues but with tests and picking them off we may be able to prevent regressions and move forward on this problem.

maxplus’s picture

Hi,

thanks @BROkEN

I also changed line 1412 of commerce_discount.rules.inc:
$discount_amount['amount'] = commerce_round(COMMERCE_ROUND_HALF_UP, $discount_amount['amount']);
to
$discount_amount['amount'] = commerce_round(COMMERCE_ROUND_NONE, $discount_amount['amount']);

and now my discount amount is displayed correct without rounding

maxplus’s picture

Created a patch to disable rounding

torgosPizza’s picture

I think the patch from #88 should roll into a new issue, since this one is Closed.

I've applied it on our site and am testing it now, because even after updating to the latest Commerce Discounts module, we are seeing occasional PayPal errors about item totals being invalid.

Earlier in this issue thread there are some patches which added rounding methods, which I have always set to "none" - so it's interesting that the function commerce_discount_add_price_component() is forcing a COMMERCE_ROUND_HALF_UP in all cases.

Hopefully this will work for our problem (I'll know once this customer tries again) and if so we should consider committing that patch as well.