fix(medusa): Discount allocation precision issues (#3244)

.changeset/fast-onions-tie.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix(medusa): Discount allocation precision issues

integration-tests/api/__tests__/admin/order-edit/order-edit.js

@@ -2569,15 +2569,13 @@ describe("/admin/order-edits", () => {
               ]),
             }),
           ]),
-          // rounding issue since we are allocating 1/3 of the discount to one item and 2/3 to the other item where both cost 10
+          discount_total: 2000,
-          // resulting in adjustment amounts like: 1333...
-          discount_total: 2001,
-          total: 1099,
           gift_card_total: 0,
           gift_card_tax_total: 0,
           shipping_total: 0,
           subtotal: 3000,
           tax_total: 100,
+          total: 1100,
         })
       )
 

integration-tests/api/__tests__/admin/product.js

@@ -2210,16 +2210,18 @@ describe("/admin/products", () => {
 
       expect(deleteRegionPriceResponse.status).toEqual(200)
       expect(finalPriceArray).toHaveLength(2)
-      expect(finalPriceArray).toEqual([
+      expect(finalPriceArray).toEqual(
-        expect.objectContaining({
+        expect.arrayContaining([
-          amount: 1000,
+          expect.objectContaining({
-          currency_code: "usd",
+            amount: 1000,
-        }),
+            currency_code: "usd",
-        expect.objectContaining({
+          }),
-          amount: 900,
+          expect.objectContaining({
-          currency_code: "eur",
+            amount: 900,
-        }),
+            currency_code: "eur",
-      ])
+          }),
+        ])
+      )
     })
 
     it("successfully updates a variants prices by deleting both a currency and region price", async () => {

integration-tests/api/__tests__/returns/index.js

@@ -10,7 +10,13 @@ const {
   simpleOrderFactory,
   simpleProductFactory,
   simpleShippingOptionFactory,
+  simpleRegionFactory,
 } = require("../../factories")
+const {
+  simpleDiscountFactory,
+} = require("../../factories/simple-discount-factory")
+
+jest.setTimeout(30000)
 
 describe("/admin/orders", () => {
   let medusaProcess
@@ -236,6 +242,157 @@ describe("/admin/orders", () => {
     )
   })
 
+  test("creates a return w. fixed discount on the total and return the total with the right precision", async () => {
+    await adminSeeder(dbConnection)
+
+    const variant1Price = 4452600
+    const product1 = await simpleProductFactory(dbConnection, {
+      variants: [
+        {
+          id: "test-variant",
+          prices: [
+            {
+              amount: variant1Price,
+              currency: "usd",
+              variant_id: "test-variant",
+            },
+          ],
+        },
+      ],
+    })
+
+    const variant2Price = 482200
+    const product2 = await simpleProductFactory(dbConnection, {
+      variants: [
+        {
+          id: "test-variant-2",
+          prices: [
+            {
+              amount: variant2Price,
+              currency: "usd",
+              variant_id: "test-variant-2",
+            },
+          ],
+        },
+      ],
+    })
+
+    const region = await simpleRegionFactory(dbConnection, {
+      id: "test-region",
+      tax_rate: 12.5,
+    })
+
+    const discountAmount = 10000
+    const discount = await simpleDiscountFactory(dbConnection, {
+      id: "test-discount",
+      code: "TEST-2",
+      regions: [region.id],
+      rule: {
+        type: "fixed",
+        value: discountAmount,
+        allocation: "total",
+      },
+    })
+
+    const item1Id = "test-item"
+    const item2Id = "test-item-2"
+
+    const order = await simpleOrderFactory(dbConnection, {
+      email: "test@testson.com",
+      region: region.id,
+      currency_code: "usd",
+      line_items: [
+        {
+          id: item1Id,
+          variant_id: product1.variants[0].id,
+          quantity: 2,
+          fulfilled_quantity: 2,
+          shipped_quantity: 2,
+          unit_price: variant1Price,
+          adjustments: [
+            {
+              amount: 9023,
+              discount_code: discount.code,
+              description: "discount",
+              item_id: "test-item",
+            },
+          ],
+          tax_lines: [
+            {
+              name: "default",
+              code: "default",
+              rate: 20,
+            },
+          ],
+        },
+        {
+          id: item2Id,
+          variant_id: product2.variants[0].id,
+          quantity: 2,
+          fulfilled_quantity: 2,
+          shipped_quantity: 2,
+          unit_price: variant2Price,
+          adjustments: [
+            {
+              amount: 977,
+              discount_code: discount.code,
+              description: "discount",
+              item_id: "test-item",
+            },
+          ],
+          tax_lines: [
+            {
+              name: "default",
+              code: "default",
+              rate: 20,
+            },
+          ],
+        },
+      ],
+    })
+
+    const api = useApi()
+
+    const response = await api.post(
+      `/admin/orders/${order.id}/return`,
+      {
+        items: [
+          {
+            item_id: item1Id,
+            quantity: 1,
+            note: "TOO SMALL",
+          },
+        ],
+      },
+      {
+        headers: {
+          authorization: "Bearer test_token",
+        },
+      }
+    )
+
+    expect(response.status).toEqual(200)
+
+    /*
+     * Region has default tax rate 12.5 but line item has tax rate 20
+     * total item 1 amount (4452600 * 2 - 9023) * 1.2 = 10675412
+     * therefore refund amount should be (4452600 - 9023 / 2) * 1.2 = 5337706
+     * therefore if the second item gets refunded 5337706.2 * 2 = 10675412 which is the expected total amount
+     */
+    expect(response.data.order.returns[0].refund_amount).toEqual(5337706)
+
+    expect(response.data.order.returns[0].items).toHaveLength(1)
+    expect(response.data.order.returns[0].items).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          item_id: "test-item",
+          quantity: 1,
+          note: "TOO SMALL",
+        }),
+      ])
+    )
+  })
+
   test("receives a return with a claimed line item", async () => {
     await adminSeeder(dbConnection)
 

integration-tests/api/__tests__/store/cart/cart.js

@@ -1466,18 +1466,97 @@ describe("/store/carts", () => {
     it("Applies dynamic discount to cart correctly", async () => {
       const api = useApi()
 
-      const cart = await api.post(
+      const cart = await api.post("/store/carts/test-cart", {
-        "/store/carts/test-cart",
+        discounts: [{ code: "DYN_DISC" }],
-        {
+      })
-          discounts: [{ code: "DYN_DISC" }],
-        },
-        { withCredentials: true }
-      )
 
       expect(cart.data.cart.shipping_total).toBe(1000)
       expect(cart.status).toEqual(200)
     })
 
+    it("successfully apply a fixed discount with total allocation and return the total with the right precision", async () => {
+      const api = useApi()
+
+      const cartId = "test-cart"
+      const quantity = 2
+
+      const variant1Price = 4452600
+      const product1 = await simpleProductFactory(dbConnection, {
+        variants: [
+          {
+            id: "test-variant-1-fixed-discount-total",
+            prices: [
+              {
+                amount: variant1Price,
+                currency: "usd",
+                variant_id: "test-variant-1-fixed-discount-total",
+              },
+            ],
+          },
+        ],
+      })
+
+      await api.post(`/store/carts/${cartId}/line-items`, {
+        variant_id: product1.variants[0].id,
+        quantity,
+      })
+
+      const variant2Price = 482200
+      const product2 = await simpleProductFactory(dbConnection, {
+        variants: [
+          {
+            id: "test-variant-2-fixed-discount-total",
+            prices: [
+              {
+                amount: variant2Price,
+                currency: "usd",
+                variant_id: "test-variant-2-fixed-discount-total",
+              },
+            ],
+          },
+        ],
+      })
+
+      await api.post(`/store/carts/${cartId}/line-items`, {
+        variant_id: product2.variants[0].id,
+        quantity,
+      })
+
+      const discountAmount = 10000
+      const discount = await simpleDiscountFactory(dbConnection, {
+        id: "test-discount",
+        code: "TEST",
+        regions: ["test-region"],
+        rule: {
+          type: "fixed",
+          value: discountAmount,
+          allocation: "total",
+        },
+      })
+
+      const response = await api.post(`/store/carts/${cartId}`, {
+        discounts: [{ code: discount.code }],
+      })
+
+      expect(response.status).toBe(200)
+
+      const cart = response.data.cart
+
+      const shippingAmount = 1000
+      const expectedTotal =
+        quantity * variant1Price +
+        quantity * variant2Price +
+        shippingAmount -
+        discountAmount
+
+      const expectedSubtotal = expectedTotal + discountAmount - shippingAmount
+
+      expect(cart.total).toBe(expectedTotal)
+      expect(cart.subtotal).toBe(expectedSubtotal)
+      expect(cart.discount_total).toBe(discountAmount)
+      expect(cart.shipping_total).toBe(1000)
+    })
+
     it("updates cart customer id", async () => {
       const api = useApi()
 

integration-tests/api/factories/simple-order-factory.ts

@@ -1,13 +1,37 @@
 import { Connection } from "typeorm"
 import faker from "faker"
-import { Discount, FulfillmentStatus, Order, PaymentStatus, Refund, } from "@medusajs/medusa"
+import {
-import { DiscountFactoryData, simpleDiscountFactory, } from "./simple-discount-factory"
+  Discount,
+  FulfillmentStatus,
+  Order,
+  PaymentStatus,
+  Refund,
+} from "@medusajs/medusa"
+import {
+  DiscountFactoryData,
+  simpleDiscountFactory,
+} from "./simple-discount-factory"
 import { RegionFactoryData, simpleRegionFactory } from "./simple-region-factory"
-import { LineItemFactoryData, simpleLineItemFactory, } from "./simple-line-item-factory"
+import {
-import { AddressFactoryData, simpleAddressFactory, } from "./simple-address-factory"
+  LineItemFactoryData,
-import { ShippingMethodFactoryData, simpleShippingMethodFactory, } from "./simple-shipping-method-factory"
+  simpleLineItemFactory,
-import { SalesChannelFactoryData, simpleSalesChannelFactory, } from "./simple-sales-channel-factory"
+} from "./simple-line-item-factory"
-import { CustomerFactoryData, simpleCustomerFactory, } from "./simple-customer-factory"
+import {
+  AddressFactoryData,
+  simpleAddressFactory,
+} from "./simple-address-factory"
+import {
+  ShippingMethodFactoryData,
+  simpleShippingMethodFactory,
+} from "./simple-shipping-method-factory"
+import {
+  SalesChannelFactoryData,
+  simpleSalesChannelFactory,
+} from "./simple-sales-channel-factory"
+import {
+  CustomerFactoryData,
+  simpleCustomerFactory,
+} from "./simple-customer-factory"
 
 export type OrderFactoryData = {
   id?: string
@@ -40,6 +64,7 @@ export const simpleOrderFactory = async (
   let currencyCode: string
   let regionId: string
   let taxRate: number
+
   if (typeof data.region === "string") {
     currencyCode = data.currency_code as string
     regionId = data.region
@@ -51,6 +76,7 @@ export const simpleOrderFactory = async (
     currencyCode = region.currency_code
     regionId = region.id
   }
+
   const address = await simpleAddressFactory(connection, data.shipping_address)
 
   const customer = await simpleCustomerFactory(connection, {

integration-tests/api/helpers/admin-seeder.js

@@ -1,6 +1,5 @@
 const Scrypt = require("scrypt-kdf")
 const { User } = require("@medusajs/medusa")
-const { simpleSalesChannelFactory } = require("../factories")
 
 module.exports = async (connection, data = {}) => {
   const manager = connection.manager

packages/medusa/src/services/new-totals.ts

@@ -172,8 +172,7 @@ export default class NewTotalsService extends TransactionBaseService {
       subtotal = 0 // in that case we need to know the tax rate to compute it later
     }
 
-    const discount_total =
+    const discount_total = lineItemAllocation.discount?.amount ?? 0
-      (lineItemAllocation.discount?.unit_amount || 0) * item.quantity
 
     const totals: LineItemTotals = {
       unit_price: item.unit_price,
@@ -275,8 +274,7 @@ export default class NewTotalsService extends TransactionBaseService {
       subtotal = 0 // in that case we need to know the tax rate to compute it later
     }
 
-    const discount_total =
+    const discount_total = lineItemAllocation.discount?.amount ?? 0
-      (lineItemAllocation.discount?.unit_amount || 0) * item.quantity
 
     const totals: LineItemTotals = {
       unit_price: item.unit_price,
@@ -428,8 +426,7 @@ export default class NewTotalsService extends TransactionBaseService {
         )
 
     const discountAmount =
-      (calculationContext.allocation_map[lineItem.id]?.discount?.unit_amount ||
+      calculationContext.allocation_map[lineItem.id]?.discount?.amount ?? 0
-        0) * lineItem.quantity
 
     const lineSubtotal =
       (lineItem.unit_price - taxAmountIncludedInPrice) * lineItem.quantity -

packages/medusa/src/services/totals.ts

@@ -462,12 +462,18 @@ class TotalsService extends TransactionBaseService {
         if (allocationMap[ld.item.id]) {
           allocationMap[ld.item.id].discount = {
             amount: adjustmentAmount,
+            /**
+             * Used for the refund computation
+             */
             unit_amount: Math.round(adjustmentAmount / ld.item.quantity),
           }
         } else {
           allocationMap[ld.item.id] = {
             discount: {
               amount: adjustmentAmount,
+              /**
+               * Used for the refund computation
+               */
               unit_amount: Math.round(adjustmentAmount / ld.item.quantity),
             },
           }
@@ -794,8 +800,7 @@ class TotalsService extends TransactionBaseService {
       subtotal = 0 // in that case we need to know the tax rate to compute it later
     }
 
-    const discount_total =
+    const discount_total = lineItemAllocation.discount?.amount ?? 0
-      (lineItemAllocation.discount?.unit_amount || 0) * lineItem.quantity
 
     const lineItemTotals: LineItemTotals = {
       unit_price: lineItem.unit_price,

packages/medusa/src/strategies/tax-calculation.ts

@@ -73,9 +73,7 @@ class TaxCalculationStrategy implements ITaxCalculationStrategy {
         taxableAmount = item.unit_price * item.quantity
       }
 
-      taxableAmount -=
+      taxableAmount -= allocations.discount?.amount ?? 0
-        ((allocations.discount && allocations.discount.unit_amount) || 0) *
-        item.quantity
 
       for (const filteredTaxLine of filteredTaxLines) {
         taxTotal += Math.round(