Calculate discount for free orders / products correctly

Any further info on this? I could not reproduce with a clean commerce 1.10 site + coupons.

please disregard - commented on wrong tab.

Looks good and proper. I'd like to see a test for this, though. To ensure 100% discount can work. I'll let maintainers decide to mark CNR for this or commit.

Can someone confirm this is inline with the approach in discount for this issue? #2468159: Percentage based discounts : validation needs to be elsewhere

This one looks to be taking the value as x * 100 and the other issue is going to x / 100. I think x / 100 seems to be a better approach.

I think this issue highlights what appears to be a fairly convoluted way to calculate the percentage discount as well as generate its label. IMO since the form element for Percentage has a % suffix in the discount UI I would expect that entering 90 would create a 90% discount, and entering 0.5 would create a 0.5% discount.

That's not the case currently. If I enter 100 I get a 100% discount. If I enter 0.5 I get a 50% discount. From the latest -dev of commerce_discount the calculation in commerce_discount_percentage() uses the following to set $rate

if ($rate > 1) {
   $rate = $rate / 100;
 }

and then later on $rate is multiplied by price values to calculate the discount. We can get rid of that and just do

$rate = $discount_wrapper->commerce_discount_offer->commerce_percentage->value()/100;

This gets us the correct discount value, but not label. Right now in commerce_coupon_commerce_coupon_discount_value_display_alter() the label is being created with

$rate = $offer_wrapper->commerce_percentage->value();
  if ($rate < 1) {
    $rate = $rate * 100;
  }
$text = $rate . '% ' . t('off order');

Again, we could just remove the $rate changing condition and get the correct label. I was thinking I'd try for the commerce_discount update first, and than re-visit this with a patch. Feedback is appreciated.

Ok, so commerce_discount has been updated to calculate the discount $rate as discussed in #9. This patch should fix its display.

Thank you:) Less silliness.

Nice catch, should have looked closer.

Quick question while we are at it:

+++ b/commerce_coupon.module
@@ -1053,11 +1053,7 @@ function commerce_coupon_commerce_coupon_discount_value_display_alter(&$text, $d
-          $text = $rate . '% ' . t('off order');
+          $text = $offer_wrapper->commerce_percentage->value() . '% ' . t('off order');

Maybe it's worth providing better context to the translators? t('@percentage% off order', array('@percentage' => ...))

+++ b/commerce_coupon.module
@@ -1053,11 +1053,7 @@ function commerce_coupon_commerce_coupon_discount_value_display_alter(&$text, $d
+          $text = t('@percentage% off order', array('@percentage' => $offer_wrapper->commerce_percentage->value()));

So clean. So fresh.

Oh duh. Because there's no tests, just applies tests. I wrote initial tests here #2614392: Add tests! Start with basic UI tests..

Committed!