fix(medusa): Refresh adjustments when region on cart is changed (#1827)

* fix(medusa): Refresh adjustments when region on cart is changed

* fix test

* Fix unit test

* fix: integration tests

* fix: comment

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

@@ -1376,7 +1376,7 @@ describe("/store/carts", () => {
       )
     })
 
-    it("updates prices when cart region id is updated", async () => {
+    it("on region update: updates line item prices", async () => {
       const api = useApi()
 
       const beforeUpdate = await api
@@ -1394,7 +1394,108 @@ describe("/store/carts", () => {
 
       expect(response.status).toEqual(200)
       expect(response.data.cart.region_id).toEqual("test-region-multiple")
-      expect(response.data.cart.items[0].unit_price).toEqual(700)
+      expect(response.data.cart.items).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            unit_price: 2000,
+            variant_id: "test-variant",
+          }),
+          expect.objectContaining({
+            unit_price: 700,
+            variant_id: "test-variant-sale-cg",
+          }),
+        ])
+      )
+    })
+
+    it("on region update: refreshes line item adjustments", async () => {
+      const api = useApi()
+
+      const usdRegion = await api
+        .post(`/store/carts/test-cart-3/`, {
+          discounts: [{ code: "10PERCENT" }],
+        })
+        .catch((error) => console.log(error))
+
+      // current state of the cart:
+      // - currency -> USD
+      // - items -> [{ unit_price: 8000, ... }, { unit_price: 8000, ... }]
+      // - discount -> 10%
+      // - discount total -> 1600
+
+      expect(usdRegion.data.cart.region_id).toEqual("test-region")
+      expect(usdRegion.data.cart.discount_total).toEqual(1600)
+
+      // now, we change the region to one operating in EUR
+      // this should trigger a refresh of the line item adjustments
+      const eurRegion = await api
+        .post("/store/carts/test-cart-3", {
+          region_id: "eur-region",
+        })
+        .catch((error) => console.log(error))
+
+      // state of the cart after update:
+      // - currency -> EUR
+      // - items -> [{ unit_price: 2000, ... }, { unit_price: 700, ... }]
+      // - discount -> 10%
+      // - discount total -> 270
+
+      expect(eurRegion.data.cart.region_id).toEqual("eur-region")
+      expect(eurRegion.data.cart.discount_total).toEqual(270)
+    })
+
+    it("on region update: removes line item adjustment if discount is not applicable in region", async () => {
+      const api = useApi()
+
+      const withDiscount = await api
+        .post(`/store/carts/test-cart-3/`, {
+          discounts: [{ code: "15PERCENT" }],
+        })
+        .catch((error) => console.log(error))
+
+      expect(withDiscount.data.cart.region_id).toEqual("test-region")
+      expect(withDiscount.data.cart.discount_total).toEqual(2400)
+      expect(withDiscount.data.cart.items).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            variant_id: "test-variant-sale-cg",
+            adjustments: expect.arrayContaining([
+              expect.objectContaining({
+                amount: 1200,
+              }),
+            ]),
+          }),
+          expect.objectContaining({
+            variant_id: "test-variant",
+            adjustments: expect.arrayContaining([
+              expect.objectContaining({
+                amount: 1200,
+              }),
+            ]),
+          }),
+        ])
+      )
+
+      const response = await api
+        .post("/store/carts/test-cart-3", {
+          region_id: "eur-region",
+        })
+        .catch((error) => console.log(error))
+
+      expect(response.data.cart.region_id).toEqual("eur-region")
+      expect(response.data.cart.discount_total).toEqual(0)
+      expect(response.data.cart.items).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            variant_id: "test-variant-sale-cg",
+            adjustments: [],
+          }),
+          expect.objectContaining({
+            variant_id: "test-variant",
+            adjustments: [],
+          }),
+        ])
+      )
     })
 
     it("updates address using string id", async () => {

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

@@ -57,6 +57,16 @@ module.exports = async (connection, data = {}) => {
 
   await manager.save(r)
 
+  const europeRegion = manager.create(Region, {
+    id: "eur-region",
+    name: "Europe Region",
+    payment_providers: [{ id: "test-pay" }],
+    currency_code: "eur",
+    tax_rate: 0,
+  })
+
+  await manager.save(europeRegion)
+
   // Region with multiple countries
   const regionWithMultipleCoutries = manager.create(Region, {
     id: "test-region-multiple",
@@ -141,7 +151,7 @@ module.exports = async (connection, data = {}) => {
     ends_at: tenDaysFromToday,
   })
 
-  tenPercent.regions = [r]
+  tenPercent.regions = [r, europeRegion]
   tenPercent.rule = tenPercentRule
   await manager.save(tenPercent)
 
@@ -533,6 +543,14 @@ module.exports = async (connection, data = {}) => {
   })
   await manager.save(ma)
 
+  const maEur = manager.create(MoneyAmount, {
+    variant_id: "test-variant",
+    currency_code: "eur",
+    type: "default",
+    amount: 2000,
+  })
+  await manager.save(maEur)
+
   const ma_sale = manager.create(MoneyAmount, {
     variant_id: "test-variant-sale",
     currency_code: "usd",

integration-tests/api/package.json

@@ -8,16 +8,16 @@
     "build": "babel src -d dist --extensions \".ts,.js\""
   },
   "dependencies": {
-    "@medusajs/medusa": "1.3.3-dev-1657050014476",
+    "@medusajs/medusa": "1.3.3-dev-1657484310311",
     "faker": "^5.5.3",
-    "medusa-interfaces": "1.3.1-dev-1657050014476",
+    "medusa-interfaces": "1.3.1-dev-1657484310311",
     "typeorm": "^0.2.31"
   },
   "devDependencies": {
     "@babel/cli": "^7.12.10",
     "@babel/core": "^7.12.10",
     "@babel/node": "^7.12.10",
-    "babel-preset-medusa-package": "1.1.19-dev-1657050014476",
+    "babel-preset-medusa-package": "1.1.19-dev-1657484310311",
     "jest": "^26.6.3"
   }
 }

packages/medusa/src/services/__tests__/cart.js

@@ -7,7 +7,7 @@ import { LineItemAdjustmentServiceMock } from "../__mocks__/line-item-adjustment
 
 const eventBusService = {
   emit: jest.fn(),
-  withTransaction: function() {
+  withTransaction: function () {
     return this
   },
 }
@@ -193,7 +193,7 @@ describe("CartService", () => {
 
   describe("create", () => {
     const regionService = {
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
       retrieve: () => {
@@ -217,7 +217,7 @@ describe("CartService", () => {
           postal_code: "12345",
           country_code: "us",
         }
-      }
+      },
     })
     const cartRepository = MockRepository()
     const customerService = {
@@ -227,7 +227,7 @@ describe("CartService", () => {
           email: "email@test.com",
         })
       ),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -335,14 +335,14 @@ describe("CartService", () => {
     const lineItemService = {
       update: jest.fn(),
       create: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
 
     const shippingOptionService = {
       deleteShippingMethods: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -558,7 +558,7 @@ describe("CartService", () => {
     const lineItemService = {
       delete: jest.fn(),
       update: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -600,7 +600,7 @@ describe("CartService", () => {
 
     const shippingOptionService = {
       deleteShippingMethods: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -663,12 +663,14 @@ describe("CartService", () => {
       expect(shippingOptionService.deleteShippingMethods).toHaveBeenCalledTimes(
         1
       )
-      expect(shippingOptionService.deleteShippingMethods).toHaveBeenCalledWith([{
+      expect(shippingOptionService.deleteShippingMethods).toHaveBeenCalledWith([
+        {
           id: IdMap.getId("ship-method"),
           shipping_option: {
             profile_id: IdMap.getId("prevPro"),
           },
-      }])
+        },
+      ])
 
       expect(LineItemAdjustmentServiceMock.delete).toHaveBeenCalledTimes(1)
       expect(LineItemAdjustmentServiceMock.delete).toHaveBeenCalledWith({
@@ -760,7 +762,7 @@ describe("CartService", () => {
           quantity: 1,
         })
       ),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -872,7 +874,7 @@ describe("CartService", () => {
           email: data.email,
         })
       ),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -1094,7 +1096,7 @@ describe("CartService", () => {
     const lineItemService = {
       update: jest.fn((r) => r),
       delete: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -1114,7 +1116,7 @@ describe("CartService", () => {
           countries: [{ iso_2: "us" }],
         })
       ),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -1147,13 +1149,13 @@ describe("CartService", () => {
       deleteSession: jest.fn(),
       updateSession: jest.fn(),
       createSession: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
 
     const priceSelectionStrat = {
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
       calculateVariantPrice: async (variantId, context) => {
@@ -1175,6 +1177,7 @@ describe("CartService", () => {
       cartRepository,
       regionService,
       lineItemService,
+      lineItemAdjustmentService: LineItemAdjustmentServiceMock,
       productVariantService,
       eventBusService,
       paymentSessionRepository: MockRepository(),
@@ -1369,7 +1372,7 @@ describe("CartService", () => {
       deleteSession: jest.fn(),
       updateSession: jest.fn(),
       createSession: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -1527,7 +1530,7 @@ describe("CartService", () => {
 
     const lineItemService = {
       update: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
@@ -1540,13 +1543,13 @@ describe("CartService", () => {
         })
       }),
       deleteShippingMethods: jest.fn(),
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
     }
 
     const customShippingOptionService = {
-      withTransaction: function() {
+      withTransaction: function () {
         return this
       },
       list: jest.fn().mockImplementation(({ cart_id }) => {
@@ -1587,9 +1590,11 @@ describe("CartService", () => {
         IdMap.getId("option"),
         data
       )
-      expect(
+      expect(shippingOptionService.createShippingMethod).toHaveBeenCalledWith(
-        shippingOptionService.createShippingMethod
+        IdMap.getId("option"),
-      ).toHaveBeenCalledWith(IdMap.getId("option"), data, { cart: cart1 })
+        data,
+        { cart: cart1 }
+      )
     })
 
     it("successfully overrides existing profile shipping method", async () => {
@@ -1601,9 +1606,11 @@ describe("CartService", () => {
         IdMap.getId("profile1"),
         data
       )
-      expect(
+      expect(shippingOptionService.createShippingMethod).toHaveBeenCalledWith(
-        shippingOptionService.createShippingMethod
+        IdMap.getId("profile1"),
-      ).toHaveBeenCalledWith(IdMap.getId("profile1"), data, { cart: cart2 })
+        data,
+        { cart: cart2 }
+      )
       expect(shippingOptionService.deleteShippingMethods).toHaveBeenCalledWith({
         id: IdMap.getId("ship1"),
         shipping_option: {
@@ -1629,9 +1636,11 @@ describe("CartService", () => {
       expect(shippingOptionService.createShippingMethod).toHaveBeenCalledTimes(
         1
       )
-      expect(
+      expect(shippingOptionService.createShippingMethod).toHaveBeenCalledWith(
-        shippingOptionService.createShippingMethod
+        IdMap.getId("additional"),
-      ).toHaveBeenCalledWith(IdMap.getId("additional"), data, { cart: cart2 })
+        data,
+        { cart: cart2 }
+      )
     })
 
     it("updates item shipping", async () => {
@@ -1651,9 +1660,11 @@ describe("CartService", () => {
       expect(shippingOptionService.createShippingMethod).toHaveBeenCalledTimes(
         1
       )
-      expect(
+      expect(shippingOptionService.createShippingMethod).toHaveBeenCalledWith(
-        shippingOptionService.createShippingMethod
+        IdMap.getId("profile1"),
-      ).toHaveBeenCalledWith(IdMap.getId("profile1"), data, { cart: cart3 })
+        data,
+        { cart: cart3 }
+      )
 
       expect(lineItemService.update).toHaveBeenCalledTimes(1)
       expect(lineItemService.update).toHaveBeenCalledWith(IdMap.getId("line"), {

packages/medusa/src/services/cart.ts

@@ -3,6 +3,7 @@ import { MedusaError, Validator } from "medusa-core-utils"
 import { DeepPartial, EntityManager, In } from "typeorm"
 import { TransactionBaseService } from "../interfaces"
 import { IPriceSelectionStrategy } from "../interfaces/price-selection-strategy"
+import { DiscountRuleType } from "../models"
 import { Address } from "../models/address"
 import { Cart } from "../models/cart"
 import { CustomShippingOption } from "../models/custom-shipping-option"
@@ -38,7 +39,6 @@ import RegionService from "./region"
 import ShippingOptionService from "./shipping-option"
 import TaxProviderService from "./tax-provider"
 import TotalsService from "./totals"
-import { DiscountRuleType } from "../models"
 
 type InjectedDependencies = {
   manager: EntityManager
@@ -288,11 +288,8 @@ class CartService extends TransactionBaseService<CartService> {
         )
         const validatedId = validateId(cartId)
 
-        const {
+        const { select, relations, totalsToSelect } =
-          select,
+          this.transformQueryForTotals_(options)
-          relations,
-          totalsToSelect,
-        } = this.transformQueryForTotals_(options)
 
         const query = buildQuery(
           { id: validatedId },
@@ -766,12 +763,6 @@ class CartService extends TransactionBaseService<CartService> {
           }
         }
 
-        if (typeof data.region_id !== "undefined") {
-          const countryCode =
-            (data.country_code || data.shipping_address?.country_code) ?? null
-          await this.setRegion_(cart, data.region_id, countryCode)
-        }
-
         if (
           typeof data.customer_id !== "undefined" ||
           typeof data.region_id !== "undefined"
@@ -779,6 +770,12 @@ class CartService extends TransactionBaseService<CartService> {
           await this.updateUnitPrices_(cart, data.region_id, data.customer_id)
         }
 
+        if (typeof data.region_id !== "undefined") {
+          const countryCode =
+            (data.country_code || data.shipping_address?.country_code) ?? null
+          await this.setRegion_(cart, data.region_id, countryCode)
+        }
+
         const addrRepo = transactionManager.getCustomRepository(
           this.addressRepository_
         )
@@ -898,9 +895,7 @@ class CartService extends TransactionBaseService<CartService> {
   protected async createOrFetchUserFromEmail_(
     email: string
   ): Promise<Customer> {
-    const schema = Validator.string()
+    const schema = Validator.string().email().required()
-      .email()
-      .required()
     const { value, error } = schema.validate(email.toLowerCase())
     if (error) {
       throw new MedusaError(
@@ -1559,7 +1554,8 @@ class CartService extends TransactionBaseService<CartService> {
           ],
         })
 
-        const cartCustomShippingOptions = await this.customShippingOptionService_
+        const cartCustomShippingOptions =
+          await this.customShippingOptionService_
             .withTransaction(transactionManager)
             .list({ cart_id: cart.id })
 
@@ -1734,18 +1730,17 @@ class CartService extends TransactionBaseService<CartService> {
       )
     }
 
-    // Set the new region for the cart
     const region = await this.regionService_
-      .withTransaction(this.transactionManager_)
+      .withTransaction(transactionManager)
       .retrieve(regionId, {
         relations: ["countries"],
       })
-    const addrRepo = transactionManager.getCustomRepository(
-      this.addressRepository_
-    )
     cart.region = region
     cart.region_id = region.id
 
+    const addrRepo = transactionManager.getCustomRepository(
+      this.addressRepository_
+    )
     /*
      * When changing the region you are changing the set of countries that your
      * cart can be shipped to so we need to make sure that the current shipping
@@ -1828,6 +1823,11 @@ class CartService extends TransactionBaseService<CartService> {
       })
     }
 
+    if (cart?.items?.length) {
+      // line item adjustments should be refreshed on region change after having filtered out inapplicable discounts
+      await this.refreshAdjustments_(cart)
+    }
+
     cart.gift_cards = []
 
     if (cart.payment_sessions && cart.payment_sessions.length) {