From ac869509f91e32ae2baf3d023c2f4a0339e2e51a Mon Sep 17 00:00:00 2001 From: Chris Barton Date: Mon, 4 Nov 2024 13:24:03 -0800 Subject: [PATCH] fix: discount rounding half up with half cents Follow tax rounding rules with discounts and round the half cent up to the nearest cent: `39.495 => 39.50`, not `39.49`. --- lib/recurly/pricing/checkout/calculations.js | 24 ++++---------- .../pricing/subscription/calculations.js | 19 +++++------ lib/util/decimalize-member.js | 13 -------- lib/util/decimalize.js | 33 +++++++++++++++++-- lib/util/tax-round.js | 14 -------- .../fixtures/coupons/coop-pct-50.json | 13 ++++++++ test/unit/pricing/checkout/checkout.test.js | 16 ++++++++- 7 files changed, 74 insertions(+), 58 deletions(-) delete mode 100644 lib/util/decimalize-member.js delete mode 100644 lib/util/tax-round.js create mode 100644 packages/public-api-fixture-server/fixtures/coupons/coop-pct-50.json diff --git a/lib/recurly/pricing/checkout/calculations.js b/lib/recurly/pricing/checkout/calculations.js index 61725a955..6d2161223 100644 --- a/lib/recurly/pricing/checkout/calculations.js +++ b/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)); -} diff --git a/lib/recurly/pricing/subscription/calculations.js b/lib/recurly/pricing/subscription/calculations.js index bc2791db9..3999ab448 100644 --- a/lib/recurly/pricing/subscription/calculations.js +++ b/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 { diff --git a/lib/util/decimalize-member.js b/lib/util/decimalize-member.js deleted file mode 100644 index 629c2450e..000000000 --- a/lib/util/decimalize-member.js +++ /dev/null @@ -1,13 +0,0 @@ -import decimalize from './decimalize'; - -/** - * 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 default function decimalizeMember (prop) { - if (typeof this[prop] !== 'number') return; - this[prop] = decimalize(this[prop]); -} diff --git a/lib/util/decimalize.js b/lib/util/decimalize.js index 75d46898c..8a361c6ed 100644 --- a/lib/util/decimalize.js +++ b/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); } diff --git a/lib/util/tax-round.js b/lib/util/tax-round.js deleted file mode 100644 index c53ab9666..000000000 --- a/lib/util/tax-round.js +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Round the second decimal of a number without risk of - * floating point math errors - * - * - returns zero if the result is negative - * - * @param {Number} number - * @return {Number} - */ - -export default function taxRound (number) { - number = Math.max(number, 0); - return +((number < 0 ? -1 : 1) * Math.round(Math.abs(number) + 'e+2') + 'e-2'); -} diff --git a/packages/public-api-fixture-server/fixtures/coupons/coop-pct-50.json b/packages/public-api-fixture-server/fixtures/coupons/coop-pct-50.json new file mode 100644 index 000000000..1be8ccb51 --- /dev/null +++ b/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" +} diff --git a/test/unit/pricing/checkout/checkout.test.js b/test/unit/pricing/checkout/checkout.test.js index e975eac1d..2efe48f00 100644 --- a/test/unit/pricing/checkout/checkout.test.js +++ b/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);