Skip to content

Commit

Permalink
fix: discount rounding half up with half cents
Browse files Browse the repository at this point in the history
Follow tax rounding rules with discounts and round the half cent up to
the nearest cent: `39.495 => 39.50`, not `39.49`.
  • Loading branch information
cbarton committed Nov 4, 2024
1 parent 6b9d477 commit ac86950
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 58 deletions.
24 changes: 7 additions & 17 deletions lib/recurly/pricing/checkout/calculations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));
}
19 changes: 9 additions & 10 deletions lib/recurly/pricing/subscription/calculations.js
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
});
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 0 additions & 13 deletions lib/util/decimalize-member.js

This file was deleted.

33 changes: 30 additions & 3 deletions lib/util/decimalize.js
Original file line number Diff line number Diff line change
@@ -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);
}
14 changes: 0 additions & 14 deletions lib/util/tax-round.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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"
}
16 changes: 15 additions & 1 deletion test/unit/pricing/checkout/checkout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ac86950

Please sign in to comment.