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.
This commit is contained in:
Oliver Windall Juhl authored and GitHub committed 2022-08-16 09:21:01 +00:00
1 parent f1c2c6c68b
commit 0ba63c70b0
5 files changed
+93 -52

No files matched your search

@@ -1635,6 +1635,36 @@ describe("/store/carts", () => {
expect(getRes.data.type).toEqual("order") 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 () => { it("complete cart with items inventory covered", async () => {
const api = useApi() const api = useApi()
const getRes = await api.post(`/store/carts/test-cart-2/complete-cart`) const getRes = await api.post(`/store/carts/test-cart-2/complete-cart`)
+25 -21
View File
@@ -1,11 +1,11 @@
import { IdMap, MockManager, MockRepository } from "medusa-test-utils" import { IdMap, MockManager, MockRepository } from "medusa-test-utils"
import OrderService from "../order" import OrderService from "../order"
import { InventoryServiceMock } from "../__mocks__/inventory" import { InventoryServiceMock } from "../__mocks__/inventory"
import { LineItemServiceMock } from "../__mocks__/line-item"; import { LineItemServiceMock } from "../__mocks__/line-item"
describe("OrderService", () => { describe("OrderService", () => {
const totalsService = { const totalsService = {
withTransaction: function () { withTransaction: function() {
return this return this
}, },
getLineItemRefund: () => {}, getLineItemRefund: () => {},
@@ -40,7 +40,7 @@ describe("OrderService", () => {
const eventBusService = { const eventBusService = {
emit: jest.fn(), emit: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -56,20 +56,20 @@ describe("OrderService", () => {
}) })
const lineItemService = { const lineItemService = {
update: jest.fn(), update: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
const shippingOptionService = { const shippingOptionService = {
updateShippingMethod: jest.fn(), updateShippingMethod: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
const giftCardService = { const giftCardService = {
update: jest.fn(), update: jest.fn(),
createTransaction: jest.fn(), createTransaction: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -81,7 +81,7 @@ describe("OrderService", () => {
cancelPayment: jest.fn().mockImplementation((payment) => { cancelPayment: jest.fn().mockImplementation((payment) => {
return Promise.resolve({ ...payment, status: "cancelled" }) return Promise.resolve({ ...payment, status: "cancelled" })
}), }),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -120,7 +120,7 @@ describe("OrderService", () => {
total: 100, total: 100,
}) })
}), }),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -210,6 +210,8 @@ describe("OrderService", () => {
"discounts.rule", "discounts.rule",
"gift_cards", "gift_cards",
"shipping_methods", "shipping_methods",
"items",
"items.adjustments",
], ],
}) })
@@ -521,7 +523,7 @@ describe("OrderService", () => {
manager: MockManager, manager: MockManager,
orderRepository: orderRepo, orderRepository: orderRepo,
eventBusService, eventBusService,
lineItemService: LineItemServiceMock lineItemService: LineItemServiceMock,
}) })
beforeEach(async () => { beforeEach(async () => {
@@ -615,14 +617,14 @@ describe("OrderService", () => {
const fulfillmentService = { const fulfillmentService = {
cancelFulfillment: jest.fn(), cancelFulfillment: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
const paymentProviderService = { const paymentProviderService = {
cancelPayment: jest.fn(), cancelPayment: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -719,7 +721,7 @@ describe("OrderService", () => {
? Promise.reject() ? Promise.reject()
: Promise.resolve({ ...p, captured_at: "notnull" }) : Promise.resolve({ ...p, captured_at: "notnull" })
), ),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -824,7 +826,7 @@ describe("OrderService", () => {
const lineItemService = { const lineItemService = {
update: jest.fn(), update: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -837,7 +839,7 @@ describe("OrderService", () => {
}, },
]) ])
}), }),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -1004,7 +1006,7 @@ describe("OrderService", () => {
}) })
} }
}), }),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -1073,7 +1075,7 @@ describe("OrderService", () => {
.mockImplementation((p) => .mockImplementation((p) =>
p.id === "payment_fail" ? Promise.reject() : Promise.resolve() p.id === "payment_fail" ? Promise.reject() : Promise.resolve()
), ),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -1214,7 +1216,7 @@ describe("OrderService", () => {
.fn() .fn()
.mockImplementation(() => Promise.resolve({})), .mockImplementation(() => Promise.resolve({})),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -1348,7 +1350,7 @@ describe("OrderService", () => {
const lineItemService = { const lineItemService = {
update: jest.fn(), update: jest.fn(),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -1371,7 +1373,7 @@ describe("OrderService", () => {
], ],
}) })
}), }),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
@@ -1398,7 +1400,9 @@ describe("OrderService", () => {
) )
expect(fulfillmentService.createShipment).toHaveBeenCalledTimes(1) expect(fulfillmentService.createShipment).toHaveBeenCalledTimes(1)
expect(fulfillmentService.createShipment).toHaveBeenCalledWith( expect(
fulfillmentService.createShipment
).toHaveBeenCalledWith(
IdMap.getId("fulfillment"), IdMap.getId("fulfillment"),
[{ tracking_number: "1234" }, { tracking_number: "2345" }], [{ tracking_number: "1234" }, { tracking_number: "2345" }],
{ metadata: undefined, no_notification: true } { metadata: undefined, no_notification: true }
@@ -1490,7 +1494,7 @@ describe("OrderService", () => {
refundPayment: jest refundPayment: jest
.fn() .fn()
.mockImplementation((p) => Promise.resolve({ id: "ref" })), .mockImplementation((p) => Promise.resolve({ id: "ref" })),
withTransaction: function () { withTransaction: function() {
return this return this
}, },
} }
+9 -9
View File
@@ -3,17 +3,18 @@ import { MedusaError, Validator } from "medusa-core-utils"
import { DeepPartial, EntityManager, In } from "typeorm" import { DeepPartial, EntityManager, In } from "typeorm"
import { TransactionBaseService } from "../interfaces" import { TransactionBaseService } from "../interfaces"
import { IPriceSelectionStrategy } from "../interfaces/price-selection-strategy" import { IPriceSelectionStrategy } from "../interfaces/price-selection-strategy"
import SalesChannelFeatureFlag from "../loaders/feature-flags/sales-channels"
import { import {
Address, Address,
Cart, Cart,
CustomShippingOption,
Customer, Customer,
CustomShippingOption,
Discount, Discount,
LineItem,
ShippingMethod,
SalesChannel,
DiscountRuleType, DiscountRuleType,
LineItem,
PaymentSession, PaymentSession,
SalesChannel,
ShippingMethod,
} from "../models" } from "../models"
import { AddressRepository } from "../repositories/address" import { AddressRepository } from "../repositories/address"
import { CartRepository } from "../repositories/cart" import { CartRepository } from "../repositories/cart"
@@ -28,11 +29,13 @@ import {
} from "../types/cart" } from "../types/cart"
import { AddressPayload, FindConfig, TotalField } from "../types/common" import { AddressPayload, FindConfig, TotalField } from "../types/common"
import { buildQuery, isDefined, setMetadata, validateId } from "../utils" import { buildQuery, isDefined, setMetadata, validateId } from "../utils"
import { FlagRouter } from "../utils/flag-router"
import CustomShippingOptionService from "./custom-shipping-option" import CustomShippingOptionService from "./custom-shipping-option"
import CustomerService from "./customer" import CustomerService from "./customer"
import DiscountService from "./discount" import DiscountService from "./discount"
import EventBusService from "./event-bus" import EventBusService from "./event-bus"
import GiftCardService from "./gift-card" import GiftCardService from "./gift-card"
import { SalesChannelService } from "./index"
import InventoryService from "./inventory" import InventoryService from "./inventory"
import LineItemService from "./line-item" import LineItemService from "./line-item"
import LineItemAdjustmentService from "./line-item-adjustment" import LineItemAdjustmentService from "./line-item-adjustment"
@@ -41,12 +44,9 @@ import ProductService from "./product"
import ProductVariantService from "./product-variant" import ProductVariantService from "./product-variant"
import RegionService from "./region" import RegionService from "./region"
import ShippingOptionService from "./shipping-option" import ShippingOptionService from "./shipping-option"
import StoreService from "./store"
import TaxProviderService from "./tax-provider" import TaxProviderService from "./tax-provider"
import TotalsService from "./totals" 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 = { type InjectedDependencies = {
manager: EntityManager 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 is 0, we don't perform anything payment related
if (cart.total <= 0) { if (cart.total <= 0) {
cart.payment_authorized_at = new Date() cart.payment_authorized_at = new Date()
return cartRepository.save(cart) return await cartRepository.save(cart)
} }
if (!cart.payment_session) { if (!cart.payment_session) {
+23 -21
View File
@@ -1,25 +1,6 @@
import { MedusaError } from "medusa-core-utils" import { MedusaError } from "medusa-core-utils"
import { Brackets, EntityManager } from "typeorm" 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 { TransactionBaseService } from "../interfaces"
import { buildQuery, setMetadata } from "../utils"
import { FindConfig, QuerySelector, Selector } from "../types/common"
import { import {
Address, Address,
ClaimOrder, ClaimOrder,
@@ -36,12 +17,31 @@ import {
Swap, Swap,
TrackingLink, TrackingLink,
} from "../models" } from "../models"
import { UpdateOrderInput } from "../types/orders" import { AddressRepository } from "../repositories/address"
import { CreateShippingMethodDto } from "../types/shipping-options" import { OrderRepository } from "../repositories/order"
import { FindConfig, QuerySelector, Selector } from "../types/common"
import { import {
CreateFulfillmentOrder, CreateFulfillmentOrder,
FulFillmentItemType, FulFillmentItemType,
} from "../types/fulfillment" } 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 = { type InjectedDependencies = {
manager: EntityManager manager: EntityManager
@@ -492,6 +492,8 @@ class OrderService extends TransactionBaseService {
"discounts.rule", "discounts.rule",
"gift_cards", "gift_cards",
"shipping_methods", "shipping_methods",
"items",
"items.adjustments",
], ],
}) })
@@ -158,7 +158,12 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
.withTransaction(manager) .withTransaction(manager)
.retrieve(id, { .retrieve(id, {
select: ["total"], select: ["total"],
relations: ["payment", "payment_sessions"], relations: [
"items",
"items.adjustments",
"payment",
"payment_sessions",
],
}) })
// If cart is part of swap, we register swap as complete // If cart is part of swap, we register swap as complete