reaction icon indicating copy to clipboard operation
reaction copied to clipboard

Discount calculation for orders with multiple fulfillment groups

Open bayareacoder opened this issue 4 years ago • 3 comments

It seems to me discount calculations for orders with multiple fulfillment groups cannot be correct as only the discountTotal from the cart is passed to each buildOrderFulfillmentGroupFromInput inside placeOrder:

  let discountTotal = 0;
  if (cart) {
    const discountsResult = await context.queries.getDiscountsTotalForCart(context, cart);
    ({ discounts } = discountsResult);
    discountTotal = discountsResult.total;
  }

// ...

const { group, groupSurcharges } = await buildOrderFulfillmentGroupFromInput(context, {
      accountId,
      billingAddress,
      cartId,
      currencyCode,
      discountTotal,
      inputGroup,
      orderId
    });

Inside buildOrderFulfillmentGroupFromInput there is a function addInvoiceToGroup which is called as:

  // Build and set the group invoice
  addInvoiceToGroup({
    currencyCode,
    group,
    groupDiscountTotal: discountTotal,
    groupSurchargeTotal,
    taxableAmount,
    taxTotal
  });

So suddenly the passed discountTotal is interpreted as a groupDiscountTotal:

/**
 * @summary Calculate final shipping, discounts, surcharges, and taxes; builds an invoice object
 *   with the totals on it; and sets group.invoice.
 * @param {String} currencyCode Currency code of totals
 * @param {Object} group The fulfillment group to be mutated
 * @param {Number} groupDiscountTotal Total discount amount for group
 * @param {Number} groupSurchargeTotal Total surcharge amount for group
 * @param {Number} taxableAmount Total taxable amount for group
 * @param {Number} taxTotal Total tax for group
 * @returns {undefined}
 */
export default function addInvoiceToGroup({
  currencyCode,
  group,
  groupDiscountTotal,
  groupSurchargeTotal,
  taxableAmount,
  taxTotal
}) {
// ...
}

So the full discountTotal of the cart will be applied to each group.

Solution:

  • don't pass discountTotal but pass all discountCodeIds applied to the cart to buildOrderFulfillmentGroupFromInput
  • calculate the discount for the group inside this function for each group separately so it correctly takes into account only discounts on items in that group (discountCalculationMethod ===sale), or relative % discount off of the value of each group (DiscountCalculationMethod === discount). Absolute discount amounts (DiscountCalculationMethod === credit) could be applied to 1st fulfillment group until that one reaches a net invoice amount of $0, then to 2nd one for the balance if any etc.

bayareacoder avatar Nov 02 '20 21:11 bayareacoder

To fix this, I removed the subtraction of discountTotal from the expectedGroupTotal and it worked in our usecase because client sends the discounted value as the total value in the request. Not sure if that is the right approach with respect to other feature you have in mind. Please let me know if that is correct and I can send a PR.

dineshdb avatar Nov 04 '20 06:11 dineshdb

First, I am not a maintainer just a user like you so I cannot do a PR. But I think there are a few issues with your proposal:

1/ As is mentioned in the original comment in the code, it is expected that the client does NOT include discounts.Your solution requires the client to send the discounted value. So incorporating this in the plugin would make the Reaction API incompatible with existing clients.

2/ You cannot rely on browser client to send correct total amount, it should be verified by the server. Therefore the server should know each discount applied and verify it. This is probably the reason why the client does not send the discounted amount but rather the gross item amounts and the applied discounts so server can check everything.

3/ It is not a fix to the reported issue since it doesn't address the fundamental problem of discounts not working with multiple fulfillment methods.

So if your "fix" works for you, you may just want to do your own private fork of this plugin that works with your client.

bayareacoder avatar Nov 04 '20 17:11 bayareacoder

Alright, I'll keep an eye on this issue so that I can upgrade later eliminating the need for my private fork. Thanks for the help. Appreciate it!

dineshdb avatar Nov 05 '20 09:11 dineshdb

Closing as this is an issue with the existing discount code implementation which is being deprecated

brent-hoover avatar Oct 27 '22 06:10 brent-hoover