Merge pull request #906 from recurly/discount-rounding

fix: discount rounding half up with half cents

lib/recurly/pricing/checkout/calculations.js

@@ -2,9 +2,8 @@ import each from 'component-each';
 import isEmpty from 'lodash.isempty';
 import Promise from 'promise';
 import uniq from 'array-unique';
-import decimalizeMember from '../../../util/decimalize-member';
+import { clampToZero, decimalizeMember, round } from '../../../util/decimalize';
 import groupBy from '../../../util/group-by';
-import taxRound from '../../../util/tax-round';
 
 /**
  * Checkout pricing calculation handler
@@ -182,8 +181,8 @@ export default class Calculations {
 
     // If tax amount has been specified, simply apply it
     if (this.items.tax && this.items.tax.amount) {
-      this.price.now.taxes = taxRound(this.items.tax.amount.now);
-      this.price.next.taxes = taxRound(this.items.tax.amount.next);
+      this.price.now.taxes = clampToZero(round(this.items.tax.amount.now));
+      this.price.next.taxes = clampToZero(round(this.items.tax.amount.next));
       return Promise.resolve();
     }
 
@@ -238,8 +237,8 @@ export default class Calculations {
           return taxRates.indexOf(taxRate) === i;
         }).map(JSON.parse);
         this.price.taxes = taxRates;
-        this.price.now.taxes = taxRound(taxNow);
-        this.price.next.taxes = taxRound(taxNext);
+        this.price.now.taxes = clampToZero(round(taxNow));
+        this.price.next.taxes = clampToZero(round(taxNext));
       });
   }
 
@@ -337,10 +336,10 @@ export default class Calculations {
         // Amounts are left zero
       } else if (coupon.discount.rate) {
         const { discountableNow, discountableNext } = this.discountableSubtotals(coupon, { setupFees: false });
-        discountNow = roundForDiscount(discountableNow * coupon.discount.rate);
+        discountNow = round(discountableNow * coupon.discount.rate, 6);
         // If coupon is single use, we want discountNext to be zero
         if (!coupon.single_use) {
-          discountNext = roundForDiscount(discountableNext * coupon.discount.rate);
+          discountNext = round(discountableNext * coupon.discount.rate, 6);
         }
       } else if (coupon.discount.amount) {
         const { discountableNow, discountableNext } = this.discountableSubtotals(coupon);
@@ -513,12 +512,3 @@ function filterSubscriptionsForCoupon (subscriptions, coupon) {
   if (coupon.applies_to_all_plans) return subscriptions;
   return subscriptions.filter(sub => sub.couponIsValidForSubscription(coupon));
 }
-
-/**
- * Rounds a Number and sets it to a fixed length
- * @param  {Number} amount
- * @return {Number}
- */
-function roundForDiscount (amount) {
-  return parseFloat((Math.round(amount * 100) / 100).toFixed(6));
-}

lib/recurly/pricing/subscription/calculations.js

@@ -1,8 +1,7 @@
 import each from 'component-each';
 import find from 'component-find';
 import isEmpty from 'lodash.isempty';
-import decimalizeMember from '../../../util/decimalize-member';
-import taxRound from '../../../util/tax-round';
+import { clampToZero, decimalizeMember, round } from '../../../util/decimalize';
 import { getTieredPricingTotal, getTieredPricingUnitAmount, isTieredAddOn } from './tiered-pricing-calculator';
 
 /**
@@ -115,8 +114,8 @@ export default class Calculations {
 
     // If tax amount has been specified, simply apply it
     if (this.items.tax && this.items.tax.amount) {
-      this.price.now.tax = taxRound(this.items.tax.amount.now);
-      this.price.next.tax = taxRound(this.items.tax.amount.next);
+      this.price.now.tax = clampToZero(round(this.items.tax.amount.now));
+      this.price.next.tax = clampToZero(round(this.items.tax.amount.next));
       return done.call(this);
     }
 
@@ -140,8 +139,8 @@ export default class Calculations {
           });
 
           // tax estimation prefers partial cents to round to the nearest cent
-          this.price.now.tax = taxRound(this.price.now.tax);
-          this.price.next.tax = taxRound(this.price.next.tax);
+          this.price.now.tax = clampToZero(round(this.price.now.tax));
+          this.price.next.tax = clampToZero(round(this.price.next.tax));
         }
         done.call(this);
       });
@@ -227,10 +226,10 @@ export default class Calculations {
     if (!coupon) return;
 
     if (coupon.discount.rate) {
-      var discountNow = parseFloat((this.price.now.subtotal * coupon.discount.rate).toFixed(6));
-      var discountNext = parseFloat((this.price.next.subtotal * coupon.discount.rate).toFixed(6));
-      this.price.now.discount = Math.round(discountNow * 100) / 100;
-      this.price.next.discount = Math.round(discountNext * 100) / 100;
+      var discountNow = round(this.price.now.subtotal * coupon.discount.rate);
+      var discountNext = round(this.price.next.subtotal * coupon.discount.rate);
+      this.price.now.discount = discountNow;
+      this.price.next.discount = discountNext;
     } else if (coupon.discount.type === 'free_trial') {
       // Handled in separate trial logic
     } else {

lib/util/decimalize.js

@@ -1,9 +1,36 @@
+export function clampToZero (number) {
+  return number < 0 ? 0 : number;
+}
+
+/**
+ * Round the second decimal of a number without risk of
+ * floating point math errors
+ *
+ * @param {Number} number
+ * @return {Number}
+ */
+export function round (number, digits = 2) {
+  const rounded = +((number < 0 ? -1 : 1) * Math.round(Math.abs(number) + 'e+2') + 'e-2');
+  return parseFloat(rounded.toFixed(digits));
+}
+
+/**
+ * Applies a decimal transform on an object's member
+ *
+ * @param {String} prop Property on {this} to transform
+ * @this {Object} on which to apply decimal transformation
+ */
+
+export function decimalizeMember (prop) {
+  if (typeof this[prop] !== 'number') return;
+  this[prop] = decimalize(this[prop]);
+}
+
 /**
  * Applies a decimal transform
  *
  * @param {Number} number to transform
  */
-
-export default function decimalize (number) {
-  return (Math.round(number * 100) / 100).toFixed(2);
+export default function decimalize (number, digits = 2) {
+  return round(number, digits).toFixed(digits);
 }

packages/public-api-fixture-server/fixtures/coupons/coop-pct-50.json

@@ -0,0 +1,13 @@
+{
+  "code": "coop-pct-50",
+  "name": "Test coupon: 50% off",
+  "discount": {
+    "rate": 0.50
+  },
+  "plans": [],
+  "single_use": false,
+  "applies_to_non_plan_charges": true,
+  "applies_to_plans": true,
+  "applies_to_all_plans": true,
+  "redemption_resource": "account"
+}

test/unit/pricing/checkout/checkout.test.js

@@ -854,6 +854,20 @@ describe('CheckoutPricing', function () {
     });
 
     describe('Calculations', () => {
+      it('rounds rate coupons correctly', function (done) {
+        this.pricing
+          .adjustment({ amount: 70.99 })
+          .coupon('coop-pct-50')
+          .reprice()
+          .done(price => {
+            assert.equal(price.now.discount, 35.50);
+            assert.equal(price.now.adjustments, 70.99);
+            assert.equal(price.next.discount, 0);
+            assert.equal(price.next.adjustments, 0);
+            done();
+          });
+      });
+
       describe('given a CheckoutPricing containing multiple subscriptions and adjustments', () => {
         beforeEach(function (done) {
           subscriptionPricingFactory('basic', this.recurly, sub => {
@@ -1771,7 +1785,7 @@ describe('CheckoutPricing', function () {
 
             it('discounts only the subscriptions now, and applies no discounts next cycle', function () {
               assert.equal(this.price.now.subtotal, 41.99); // 19.99 + 2 (setup fee) + 20 (adj) + 20 (adj) - $20 discount
-              assert.equal(this.price.now.discount, 20); 
+              assert.equal(this.price.now.discount, 20);
               assert.equal(this.price.next.subscriptions, 19.99);
               assert.equal(this.price.next.discount, 0);
               assert.equal(this.price.now.taxes, 3.67);