Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Rounding Issue in Cart/Orders #5091

Merged
merged 2 commits into from
Apr 3, 2019
Merged

fix: Rounding Issue in Cart/Orders #5091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d83aba
fix: rounding pricing maths in the cart now making sure any math done…
nnnnat Apr 2, 2019
80bfa92
fix: rounding price maths in orders, making sure that any caculations…
nnnnat Apr 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions imports/plugins/core/cart/server/no-meteor/util/addCartItems.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Random from "@reactioncommerce/random";
import SimpleSchema from "simpl-schema";
import { toFixed } from "accounting-js";
import ReactionError from "@reactioncommerce/reaction-error";
import findProductAndVariant from "/imports/plugins/core/catalog/server/no-meteor/utils/findProductAndVariant";

Expand Down Expand Up @@ -133,7 +134,7 @@ export default async function addCartItems(context, currentItems, inputItems, op
shopId: catalogProduct.shopId,
// Subtotal will be kept updated by event handler watching for catalog changes.
subtotal: {
amount: variantPriceInfo.price * quantity,
amount: +toFixed(variantPriceInfo.price * quantity, 3),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I reading this right that we're fixing to 3 decimal places? If so, why 3?

Copy link
Member

@kieckhafer kieckhafer Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based of @aldeed's previous work on related issues, some currency codes go up to 3 decimals, so we need to accommodate those.

Check the E column of the Active Codes table: https://en.wikipedia.org/wiki/ISO_4217. Those listed with 4 are special inter-country currencies, and don't apply.

currencyCode: price.currencyCode
},
taxCode: chosenVariant.taxCode,
Expand Down Expand Up @@ -166,7 +167,7 @@ export default async function addCartItems(context, currentItems, inputItems, op
// testable code.
const updatedQuantity = currentCartItem.quantity + cartItem.quantity;
// Recalculate subtotal with new quantity number
const updatedSubtotalAmount = updatedQuantity * cartItem.price.amount;
const updatedSubtotalAmount = +toFixed(updatedQuantity * cartItem.price.amount, 3);
updatedItemList[currentMatchingItemIndex] = {
...currentCartItem,
...cartItem,
Expand Down
12 changes: 7 additions & 5 deletions imports/plugins/core/cart/server/no-meteor/util/xformCartGroupToCommonOrder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { toFixed } from "accounting-js";

/**
* @param {Object} cart A cart
* @param {Object} group The cart fulfillment group
Expand Down Expand Up @@ -25,7 +27,7 @@ export default async function xformCartGroupToCommonOrder(cart, group, context)
quantity: item.quantity,
shopId: item.shopId,
subtotal: {
amount: item.price.amount * item.quantity,
amount: +toFixed(item.price.amount * item.quantity, 3),
currencyCode
},
taxCode: item.taxCode,
Expand Down Expand Up @@ -55,7 +57,7 @@ export default async function xformCartGroupToCommonOrder(cart, group, context)
currencyCode
},
total: {
amount: (shipmentMethod.handling || 0) + shipmentMethod.rate,
amount: +toFixed((shipmentMethod.handling || 0) + shipmentMethod.rate, 3),
currencyCode
}
};
Expand All @@ -66,7 +68,7 @@ export default async function xformCartGroupToCommonOrder(cart, group, context)
// TODO: In the future, we should update this with a discounts update
// Discounts are stored as the sum of all discounts, per cart. This will need to be updated when we refactor discounts to go by group.
const discountTotal = cart.discount || 0;
const groupItemTotal = items.reduce((sum, item) => (sum + item.subtotal.amount), 0);
const groupItemTotal = +toFixed(items.reduce((sum, item) => (sum + item.subtotal.amount), 0), 3);
// orderItemTotal will need to be updated to be the actual total when we eventually have more than one group available
const orderItemTotal = groupItemTotal;

Expand All @@ -80,7 +82,7 @@ export default async function xformCartGroupToCommonOrder(cart, group, context)
currencyCode: cart.currencyCode
},
groupTotal: {
amount: groupItemTotal - discountTotal,
amount: +toFixed(groupItemTotal - discountTotal, 3),
currencyCode: cart.currencyCode
},
orderDiscountTotal: {
Expand All @@ -92,7 +94,7 @@ export default async function xformCartGroupToCommonOrder(cart, group, context)
currencyCode: cart.currencyCode
},
orderTotal: {
amount: orderItemTotal - discountTotal,
amount: +toFixed(orderItemTotal - discountTotal, 3),
currencyCode: cart.currencyCode
}
};
Expand Down
5 changes: 3 additions & 2 deletions imports/plugins/core/orders/server/no-meteor/util/addInvoiceToGroup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { toFixed } from "accounting-js";

/**
* @summary Calculate final shipping, discounts, surcharges, and taxes; builds an invoice object
Expand All @@ -19,7 +20,7 @@ export default function addInvoiceToGroup({
taxTotal
}) {
// Items
const itemTotal = group.items.reduce((sum, item) => (sum + item.subtotal), 0);
const itemTotal = +toFixed(group.items.reduce((sum, item) => (sum + item.subtotal), 0), 3);

// Taxes
const effectiveTaxRate = taxableAmount > 0 ? taxTotal / taxableAmount : 0;
Expand All @@ -32,7 +33,7 @@ export default function addInvoiceToGroup({
// Totals
// To avoid rounding errors, be sure to keep this calculation the same between here and
// `buildOrderInputFromCart.js` in the client code.
const total = Math.max(0, itemTotal + fulfillmentTotal + taxTotal + groupSurchargeTotal - groupDiscountTotal);
const total = +toFixed(Math.max(0, itemTotal + fulfillmentTotal + taxTotal + groupSurchargeTotal - groupDiscountTotal), 3);

group.invoice = {
currencyCode,
Expand Down
3 changes: 2 additions & 1 deletion imports/plugins/core/orders/server/no-meteor/util/buildOrderItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { toFixed } from "accounting-js";
import Random from "@reactioncommerce/random";
import ReactionError from "@reactioncommerce/reaction-error";

Expand Down Expand Up @@ -48,7 +49,7 @@ export default async function buildOrderItem(context, { currencyCode, inputItem
productVendor: chosenProduct.vendor,
quantity,
shopId: chosenProduct.shopId,
subtotal: quantity * finalPrice,
subtotal: +toFixed(quantity * finalPrice, 3),
title: chosenProduct.title,
updatedAt: now,
variantId: chosenVariant.variantId,
Expand Down
12 changes: 7 additions & 5 deletions imports/plugins/core/orders/server/no-meteor/util/xformOrderGroupToCommonOrder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { toFixed } from "accounting-js";

/**
* @param {Object} [billingAddress] Billing address, if one was collected
* @param {String} [cartId] The source cart ID, if applicable
Expand All @@ -20,7 +22,7 @@ export default async function xformOrderGroupToCommonOrder({ billingAddress = nu
quantity: item.quantity,
shopId: item.shopId,
subtotal: {
amount: item.price.amount * item.quantity,
amount: +toFixed(item.price.amount * item.quantity, 3),
currencyCode
},
taxCode: item.taxCode,
Expand Down Expand Up @@ -50,7 +52,7 @@ export default async function xformOrderGroupToCommonOrder({ billingAddress = nu
currencyCode
},
total: {
amount: (shipmentMethod.handling || 0) + (shipmentMethod.rate || 0),
amount: +toFixed((shipmentMethod.handling || 0) + (shipmentMethod.rate || 0), 3),
currencyCode
}
};
Expand All @@ -60,7 +62,7 @@ export default async function xformOrderGroupToCommonOrder({ billingAddress = nu

// TODO: In the future, we should update this with a discounts update
// Discounts are stored as the sum of all discounts, per cart. This will need to be updated when we refactor discounts to go by group.
const groupItemTotal = group.items.reduce((sum, item) => (sum + item.subtotal), 0);
const groupItemTotal = +toFixed(group.items.reduce((sum, item) => (sum + item.subtotal), 0), 3);
// orderItemTotal will need to be updated to be the actual total when we eventually have more than one group available
const orderItemTotal = groupItemTotal;

Expand All @@ -74,7 +76,7 @@ export default async function xformOrderGroupToCommonOrder({ billingAddress = nu
currencyCode
},
groupTotal: {
amount: groupItemTotal - discountTotal,
amount: +toFixed(groupItemTotal - discountTotal, 3),
currencyCode
},
orderDiscountTotal: {
Expand All @@ -86,7 +88,7 @@ export default async function xformOrderGroupToCommonOrder({ billingAddress = nu
currencyCode
},
orderTotal: {
amount: orderItemTotal - discountTotal,
amount: +toFixed(orderItemTotal - discountTotal, 3),
currencyCode
}
};
Expand Down