fix(medusa): Complete cart with 100% discount (#2032)

**What** Naive fix to allow carts with 100% discount to be completed. **Why** Discount total is wrongly calculated if `items` and `items.adjustments` is not included in relations upon retrieving the cart. **Thought** This is yet another example of why we need to rethink and refactor totals computation to not depend on what is provided by the user.
medusajs · Aug 16, 2022 · 0ba63c7 · 0ba63c7
1 parent f1c2c6c
commit 0ba63c7
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 52 deletions.
diff --git a/integration-tests/api/__tests__/store/cart.js b/integration-tests/api/__tests__/store/cart.js
@@ -1635,6 +1635,36 @@ describe("/store/carts", () => {
       expect(getRes.data.type).toEqual("order")
     })
 
+    it("complete cart with 100% discount", async () => {
+      await simpleDiscountFactory(dbConnection, {
+        code: "100PERCENT",
+        rule: {
+          type: "percentage",
+          value: 100,
+        },
+        regions: ["test-region"],
+      })
+
+      const api = useApi()
+
+      await api
+        .post(`/store/carts/test-cart-3`, {
+          discounts: [{ code: "100PERCENT" }],
+        })
+        .catch((err) => {
+          console.log(err.response.data)
+        })
+
+      const getRes = await api
+        .post(`/store/carts/test-cart-3/complete`)
+        .catch((err) => {
+          console.log(err.response.data)
+        })
+
+      expect(getRes.status).toEqual(200)
+      expect(getRes.data.type).toEqual("order")
+    })
+
     it("complete cart with items inventory covered", async () => {
       const api = useApi()
       const getRes = await api.post(`/store/carts/test-cart-2/complete-cart`)

diff --git a/packages/medusa/src/services/__tests__/order.js b/packages/medusa/src/services/__tests__/order.js
@@ -1,11 +1,11 @@
 import { IdMap, MockManager, MockRepository } from "medusa-test-utils"
 import OrderService from "../order"
 import { InventoryServiceMock } from "../__mocks__/inventory"
-import { LineItemServiceMock } from "../__mocks__/line-item";
+import { LineItemServiceMock } from "../__mocks__/line-item"
 
 describe("OrderService", () => {
   const totalsService = {
-    withTransaction: function () {
+    withTransaction: function() {
       return this
     },
     getLineItemRefund: () => {},
@@ -40,7 +40,7 @@ describe("OrderService", () => {
 
   const eventBusService = {
     emit: jest.fn(),
-    withTransaction: function () {
+    withTransaction: function() {
       return this
     },
   }
@@ -56,20 +56,20 @@ describe("OrderService", () => {
     })
     const lineItemService = {
       update: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
     const shippingOptionService = {
       updateShippingMethod: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
     const giftCardService = {
       update: jest.fn(),
       createTransaction: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -81,7 +81,7 @@ describe("OrderService", () => {
       cancelPayment: jest.fn().mockImplementation((payment) => {
         return Promise.resolve({ ...payment, status: "cancelled" })
       }),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -120,7 +120,7 @@ describe("OrderService", () => {
           total: 100,
         })
       }),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -210,6 +210,8 @@ describe("OrderService", () => {
           "discounts.rule",
           "gift_cards",
           "shipping_methods",
+          "items",
+          "items.adjustments",
         ],
       })
 
@@ -521,7 +523,7 @@ describe("OrderService", () => {
       manager: MockManager,
       orderRepository: orderRepo,
       eventBusService,
-      lineItemService: LineItemServiceMock
+      lineItemService: LineItemServiceMock,
     })
 
     beforeEach(async () => {
@@ -615,14 +617,14 @@ describe("OrderService", () => {
 
     const fulfillmentService = {
       cancelFulfillment: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
 
     const paymentProviderService = {
       cancelPayment: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -719,7 +721,7 @@ describe("OrderService", () => {
             ? Promise.reject()
             : Promise.resolve({ ...p, captured_at: "notnull" })
         ),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -824,7 +826,7 @@ describe("OrderService", () => {
 
     const lineItemService = {
       update: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -837,7 +839,7 @@ describe("OrderService", () => {
           },
         ])
       }),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -1004,7 +1006,7 @@ describe("OrderService", () => {
             })
         }
       }),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -1073,7 +1075,7 @@ describe("OrderService", () => {
         .mockImplementation((p) =>
           p.id === "payment_fail" ? Promise.reject() : Promise.resolve()
         ),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -1214,7 +1216,7 @@ describe("OrderService", () => {
         .fn()
         .mockImplementation(() => Promise.resolve({})),
 
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -1348,7 +1350,7 @@ describe("OrderService", () => {
 
     const lineItemService = {
       update: jest.fn(),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -1371,7 +1373,7 @@ describe("OrderService", () => {
             ],
           })
         }),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }
@@ -1398,7 +1400,9 @@ describe("OrderService", () => {
       )
 
       expect(fulfillmentService.createShipment).toHaveBeenCalledTimes(1)
-      expect(fulfillmentService.createShipment).toHaveBeenCalledWith(
+      expect(
+        fulfillmentService.createShipment
+      ).toHaveBeenCalledWith(
         IdMap.getId("fulfillment"),
         [{ tracking_number: "1234" }, { tracking_number: "2345" }],
         { metadata: undefined, no_notification: true }
@@ -1490,7 +1494,7 @@ describe("OrderService", () => {
       refundPayment: jest
         .fn()
         .mockImplementation((p) => Promise.resolve({ id: "ref" })),
-      withTransaction: function () {
+      withTransaction: function() {
         return this
       },
     }

diff --git a/packages/medusa/src/services/cart.ts b/packages/medusa/src/services/cart.ts
@@ -3,17 +3,18 @@ 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 SalesChannelFeatureFlag from "../loaders/feature-flags/sales-channels"
 import {
   Address,
   Cart,
-  CustomShippingOption,
   Customer,
+  CustomShippingOption,
   Discount,
-  LineItem,
-  ShippingMethod,
-  SalesChannel,
   DiscountRuleType,
+  LineItem,
   PaymentSession,
+  SalesChannel,
+  ShippingMethod,
 } from "../models"
 import { AddressRepository } from "../repositories/address"
 import { CartRepository } from "../repositories/cart"
@@ -28,11 +29,13 @@ import {
 } from "../types/cart"
 import { AddressPayload, FindConfig, TotalField } from "../types/common"
 import { buildQuery, isDefined, setMetadata, validateId } from "../utils"
+import { FlagRouter } from "../utils/flag-router"
 import CustomShippingOptionService from "./custom-shipping-option"
 import CustomerService from "./customer"
 import DiscountService from "./discount"
 import EventBusService from "./event-bus"
 import GiftCardService from "./gift-card"
+import { SalesChannelService } from "./index"
 import InventoryService from "./inventory"
 import LineItemService from "./line-item"
 import LineItemAdjustmentService from "./line-item-adjustment"
@@ -41,12 +44,9 @@ import ProductService from "./product"
 import ProductVariantService from "./product-variant"
 import RegionService from "./region"
 import ShippingOptionService from "./shipping-option"
+import StoreService from "./store"
 import TaxProviderService from "./tax-provider"
 import TotalsService from "./totals"
-import SalesChannelFeatureFlag from "../loaders/feature-flags/sales-channels"
-import { FlagRouter } from "../utils/flag-router"
-import StoreService from "./store"
-import { SalesChannelService } from "./index"
 
 type InjectedDependencies = {
   manager: EntityManager
@@ -1373,7 +1373,7 @@ class CartService extends TransactionBaseService {
         // If cart total is 0, we don't perform anything payment related
         if (cart.total <= 0) {
           cart.payment_authorized_at = new Date()
-          return cartRepository.save(cart)
+          return await cartRepository.save(cart)
         }
 
         if (!cart.payment_session) {

diff --git a/packages/medusa/src/services/order.ts b/packages/medusa/src/services/order.ts
@@ -1,25 +1,6 @@
 import { MedusaError } from "medusa-core-utils"
 import { Brackets, EntityManager } from "typeorm"
-import CustomerService from "./customer"
-import { OrderRepository } from "../repositories/order"
-import PaymentProviderService from "./payment-provider"
-import ShippingOptionService from "./shipping-option"
-import ShippingProfileService from "./shipping-profile"
-import DiscountService from "./discount"
-import FulfillmentProviderService from "./fulfillment-provider"
-import FulfillmentService from "./fulfillment"
-import LineItemService from "./line-item"
-import TotalsService from "./totals"
-import RegionService from "./region"
-import CartService from "./cart"
-import { AddressRepository } from "../repositories/address"
-import GiftCardService from "./gift-card"
-import DraftOrderService from "./draft-order"
-import InventoryService from "./inventory"
-import EventBusService from "./event-bus"
 import { TransactionBaseService } from "../interfaces"
-import { buildQuery, setMetadata } from "../utils"
-import { FindConfig, QuerySelector, Selector } from "../types/common"
 import {
   Address,
   ClaimOrder,
@@ -36,12 +17,31 @@ import {
   Swap,
   TrackingLink,
 } from "../models"
-import { UpdateOrderInput } from "../types/orders"
-import { CreateShippingMethodDto } from "../types/shipping-options"
+import { AddressRepository } from "../repositories/address"
+import { OrderRepository } from "../repositories/order"
+import { FindConfig, QuerySelector, Selector } from "../types/common"
 import {
   CreateFulfillmentOrder,
   FulFillmentItemType,
 } from "../types/fulfillment"
+import { UpdateOrderInput } from "../types/orders"
+import { CreateShippingMethodDto } from "../types/shipping-options"
+import { buildQuery, setMetadata } from "../utils"
+import CartService from "./cart"
+import CustomerService from "./customer"
+import DiscountService from "./discount"
+import DraftOrderService from "./draft-order"
+import EventBusService from "./event-bus"
+import FulfillmentService from "./fulfillment"
+import FulfillmentProviderService from "./fulfillment-provider"
+import GiftCardService from "./gift-card"
+import InventoryService from "./inventory"
+import LineItemService from "./line-item"
+import PaymentProviderService from "./payment-provider"
+import RegionService from "./region"
+import ShippingOptionService from "./shipping-option"
+import ShippingProfileService from "./shipping-profile"
+import TotalsService from "./totals"
 
 type InjectedDependencies = {
   manager: EntityManager
@@ -492,6 +492,8 @@ class OrderService extends TransactionBaseService {
           "discounts.rule",
           "gift_cards",
           "shipping_methods",
+          "items",
+          "items.adjustments",
         ],
       })
 

diff --git a/packages/medusa/src/strategies/cart-completion.ts b/packages/medusa/src/strategies/cart-completion.ts
@@ -158,7 +158,12 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
                     .withTransaction(manager)
                     .retrieve(id, {
                       select: ["total"],
-                      relations: ["payment", "payment_sessions"],
+                      relations: [
+                        "items",
+                        "items.adjustments",
+                        "payment",
+                        "payment_sessions",
+                      ],
                     })
 
                   // If cart is part of swap, we register swap as complete