feat(medusa): Performance improvement of DraftOrder creation (#3350)

* chore(medusa): Improve draft order creation

* cleanup

* fix unit tests

* fix unit tests 2

* Create beige-ties-hunt.md

* cleanup feedback

---------

Co-authored-by: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com>
This commit is contained in:
Adrien de PerettiandOliver Windall Juhl authored and GitHub committed 2023-03-02 18:03:04 +01:00
1 parent f033711ad6
commit d6b1ad1ccd
8 files changed
+228 -118

No files matched your search

+5
View File
@@ -0,0 +1,5 @@
---
"@medusajs/medusa": patch
---
Chores(medusa): draft order create improvement perf (first step)
@@ -1,5 +1,5 @@
import CustomShippingOptionService from "../custom-shipping-option" import CustomShippingOptionService from "../custom-shipping-option"
import { MockManager, MockRepository, IdMap } from "medusa-test-utils" import { MockManager, MockRepository } from "medusa-test-utils"
describe("CustomShippingOptionService", () => { describe("CustomShippingOptionService", () => {
describe("list", () => { describe("list", () => {
@@ -38,7 +38,7 @@ describe("CustomShippingOptionService", () => {
cart_id: "test-cso-cart", cart_id: "test-cso-cart",
}, },
relations: { relations: {
"shipping_option": true shipping_option: true,
}, },
}) })
}) })
@@ -76,8 +76,8 @@ describe("CustomShippingOptionService", () => {
expect(customShippingOptionRepository.findOne).toHaveBeenCalledWith({ expect(customShippingOptionRepository.findOne).toHaveBeenCalledWith({
where: { id: "cso-test" }, where: { id: "cso-test" },
relations: { relations: {
"shipping_option": true, shipping_option: true,
"cart": true, cart: true,
}, },
}) })
}) })
@@ -113,20 +113,24 @@ describe("CustomShippingOptionService", () => {
await customShippingOptionService.create(customShippingOption) await customShippingOptionService.create(customShippingOption)
expect(customShippingOptionRepository.create).toHaveBeenCalledTimes(1) expect(customShippingOptionRepository.create).toHaveBeenCalledTimes(1)
expect(customShippingOptionRepository.create).toHaveBeenCalledWith({ expect(customShippingOptionRepository.create).toHaveBeenCalledWith([
cart_id: "test-cso-cart", {
shipping_option_id: "test-so", cart_id: "test-cso-cart",
price: 30, shipping_option_id: "test-so",
metadata: undefined, price: 30,
}) metadata: undefined,
},
])
expect(customShippingOptionRepository.save).toHaveBeenCalledTimes(1) expect(customShippingOptionRepository.save).toHaveBeenCalledTimes(1)
expect(customShippingOptionRepository.save).toHaveBeenCalledWith({ expect(customShippingOptionRepository.save).toHaveBeenCalledWith({
id: "test-cso", id: "test-cso",
cart_id: "test-cso-cart", 0: {
shipping_option_id: "test-so", cart_id: "test-cso-cart",
price: 30, shipping_option_id: "test-so",
metadata: undefined, price: 30,
metadata: undefined,
},
}) })
}) })
}) })
@@ -35,10 +35,12 @@ describe("DraftOrderService", () => {
const lineItemService = { const lineItemService = {
generate: jest.fn().mockImplementation(() => generate: jest.fn().mockImplementation(() =>
Promise.resolve({ Promise.resolve([
title: "test-item", {
variant_id: "test-variant", title: "test-item",
}) variant_id: "test-variant",
},
])
), ),
create: jest.fn().mockImplementation((data) => data), create: jest.fn().mockImplementation((data) => data),
withTransaction: function () { withTransaction: function () {
@@ -88,6 +90,12 @@ describe("DraftOrderService", () => {
...testOrder, ...testOrder,
}) })
), ),
retrieveWithTotals: jest.fn().mockReturnValue(
Promise.resolve({
id: "test-cart",
...testOrder,
})
),
update: jest.fn(), update: jest.fn(),
applyDiscount: jest.fn(), applyDiscount: jest.fn(),
addShippingMethod: jest.fn(), addShippingMethod: jest.fn(),
@@ -151,28 +159,37 @@ describe("DraftOrderService", () => {
expect(cartService.addShippingMethod).toHaveBeenCalledTimes(1) expect(cartService.addShippingMethod).toHaveBeenCalledTimes(1)
expect(cartService.addShippingMethod).toHaveBeenCalledWith( expect(cartService.addShippingMethod).toHaveBeenCalledWith(
"test-cart", {
id: "test-cart",
...testOrder,
},
"test-option", "test-option",
{} {}
) )
expect(lineItemService.generate).toHaveBeenCalledTimes(1) expect(lineItemService.generate).toHaveBeenCalledTimes(1)
expect(lineItemService.generate).toHaveBeenCalledWith( expect(lineItemService.generate).toHaveBeenCalledWith(
"test-variant", [
"test-region", {
2, variantId: "test-variant",
quantity: 2,
metadata: {},
unit_price: undefined,
},
],
{ {
metadata: {}, region_id: "test-region",
unit_price: undefined,
} }
) )
expect(lineItemService.create).toHaveBeenCalledTimes(1) expect(lineItemService.create).toHaveBeenCalledTimes(1)
expect(lineItemService.create).toHaveBeenCalledWith({ expect(lineItemService.create).toHaveBeenCalledWith([
cart_id: cartId, {
title, cart_id: cartId,
variant_id: "test-variant", title,
}) variant_id: "test-variant",
},
])
expect(cartService.applyDiscount).toHaveBeenCalledTimes(0) expect(cartService.applyDiscount).toHaveBeenCalledTimes(0)
}) })
@@ -181,6 +198,8 @@ describe("DraftOrderService", () => {
const cartId = "test-cart" const cartId = "test-cart"
const title = "test-item" const title = "test-item"
const originalTestOrder = { ...testOrder }
testOrder["discounts"] = [{ code: "TEST" }] testOrder["discounts"] = [{ code: "TEST" }]
await draftOrderService.create(testOrder) await draftOrderService.create(testOrder)
@@ -200,28 +219,37 @@ describe("DraftOrderService", () => {
expect(cartService.addShippingMethod).toHaveBeenCalledTimes(1) expect(cartService.addShippingMethod).toHaveBeenCalledTimes(1)
expect(cartService.addShippingMethod).toHaveBeenCalledWith( expect(cartService.addShippingMethod).toHaveBeenCalledWith(
"test-cart", {
id: "test-cart",
...originalTestOrder,
},
"test-option", "test-option",
{} {}
) )
expect(lineItemService.generate).toHaveBeenCalledTimes(1) expect(lineItemService.generate).toHaveBeenCalledTimes(1)
expect(lineItemService.generate).toHaveBeenCalledWith( expect(lineItemService.generate).toHaveBeenCalledWith(
"test-variant", [
"test-region", {
2, variantId: "test-variant",
quantity: 2,
metadata: {},
unit_price: undefined,
},
],
{ {
metadata: {}, region_id: "test-region",
unit_price: undefined,
} }
) )
expect(lineItemService.create).toHaveBeenCalledTimes(1) expect(lineItemService.create).toHaveBeenCalledTimes(1)
expect(lineItemService.create).toHaveBeenCalledWith({ expect(lineItemService.create).toHaveBeenCalledWith([
cart_id: cartId, {
title, cart_id: cartId,
variant_id: "test-variant", title,
}) variant_id: "test-variant",
},
])
expect(cartService.update).toHaveBeenCalledTimes(1) expect(cartService.update).toHaveBeenCalledTimes(1)
expect(cartService.update).toHaveBeenCalledWith(cartId, { expect(cartService.update).toHaveBeenCalledWith(cartId, {
+16 -14
View File
@@ -36,7 +36,7 @@ import {
TotalField, TotalField,
WithRequiredProperty, WithRequiredProperty,
} from "../types/common" } from "../types/common"
import { buildQuery, setMetadata } from "../utils" import { buildQuery, isString, setMetadata } from "../utils"
import { FlagRouter } from "../utils/flag-router" import { FlagRouter } from "../utils/flag-router"
import { validateEmail } from "../utils/is-email" import { validateEmail } from "../utils/is-email"
import { PaymentSessionInput } from "../types/payment" import { PaymentSessionInput } from "../types/payment"
@@ -2083,28 +2083,30 @@ class CartService extends TransactionBaseService {
* Shipping Option is a possible way to ship an order. Shipping Methods may * Shipping Option is a possible way to ship an order. Shipping Methods may
* also have additional details in the data field such as an id for a package * also have additional details in the data field such as an id for a package
* shop. * shop.
* @param cartId - the id of the cart to add shipping method to * @param cartOrId - the id of the cart to add shipping method to
* @param optionId - id of shipping option to add as valid method * @param optionId - id of shipping option to add as valid method
* @param data - the fulmillment data for the method * @param data - the fulmillment data for the method
* @return the result of the update operation * @return the result of the update operation
*/ */
async addShippingMethod( async addShippingMethod(
cartId: string, cartOrId: string | Cart,
optionId: string, optionId: string,
data: Record<string, unknown> = {} data: Record<string, unknown> = {}
): Promise<Cart> { ): Promise<Cart> {
return await this.atomicPhase_( return await this.atomicPhase_(
async (transactionManager: EntityManager) => { async (transactionManager: EntityManager) => {
const cart = await this.retrieveWithTotals(cartId, { const cart = !isString(cartOrId)
relations: [ ? cartOrId
"shipping_methods", : await this.retrieveWithTotals(cartOrId, {
"shipping_methods.shipping_option", relations: [
"items", "shipping_methods",
"items.variant", "shipping_methods.shipping_option",
"items.variant.product", "items",
"payment_sessions", "items.variant",
], "items.variant.product",
}) "payment_sessions",
],
})
const cartCustomShippingOptions = const cartCustomShippingOptions =
await this.customShippingOptionService_ await this.customShippingOptionService_
@@ -2164,7 +2166,7 @@ class CartService extends TransactionBaseService {
) )
} }
const updatedCart = await this.retrieve(cartId, { const updatedCart = await this.retrieve(cart.id, {
relations: ["discounts", "discounts.rule", "shipping_methods"], relations: ["discounts", "discounts.rule", "shipping_methods"],
}) })
@@ -6,6 +6,7 @@ import { CustomShippingOptionRepository } from "../repositories/custom-shipping-
import { FindConfig, Selector } from "../types/common" import { FindConfig, Selector } from "../types/common"
import { CreateCustomShippingOptionInput } from "../types/shipping-options" import { CreateCustomShippingOptionInput } from "../types/shipping-options"
import { buildQuery } from "../utils" import { buildQuery } from "../utils"
import { DeepPartial } from "typeorm/common/DeepPartial"
type InjectedDependencies = { type InjectedDependencies = {
manager: EntityManager manager: EntityManager
@@ -75,25 +76,30 @@ class CustomShippingOptionService extends TransactionBaseService {
/** /**
* Creates a custom shipping option * Creates a custom shipping option
* @param data - the custom shipping option to create * @param data - the custom shipping option to create
* @param config - any configurations if needed, including meta data
* @return resolves to the creation result * @return resolves to the creation result
*/ */
async create( async create<
data: CreateCustomShippingOptionInput T = CreateCustomShippingOptionInput | CreateCustomShippingOptionInput[],
): Promise<CustomShippingOption> { TResult = T extends CreateCustomShippingOptionInput[]
const { cart_id, shipping_option_id, price, metadata } = data ? CustomShippingOption[]
: CustomShippingOption
>(data: T): Promise<TResult> {
const customShippingOptionRepo = this.activeManager_.withRepository( const customShippingOptionRepo = this.activeManager_.withRepository(
this.customShippingOptionRepository_ this.customShippingOptionRepository_
) )
const customShippingOption = customShippingOptionRepo.create({ const data_ = (
cart_id, Array.isArray(data) ? data : [data]
shipping_option_id, ) as DeepPartial<CustomShippingOption>[]
price,
metadata, const customShippingOptions = customShippingOptionRepo.create(data_)
}) const shippingOptions = await customShippingOptionRepo.save(
return await customShippingOptionRepo.save(customShippingOption) customShippingOptions
)
return (Array.isArray(data)
? shippingOptions
: shippingOptions[0]) as unknown as TResult
} }
} }
+101 -50
View File
@@ -9,7 +9,13 @@ import {
UpdateResult, UpdateResult,
} from "typeorm" } from "typeorm"
import { TransactionBaseService } from "../interfaces" import { TransactionBaseService } from "../interfaces"
import { CartType, DraftOrder, DraftOrderStatus } from "../models" import {
CartType,
DraftOrder,
DraftOrderStatus,
LineItem,
ShippingMethod,
} from "../models"
import { DraftOrderRepository } from "../repositories/draft-order" import { DraftOrderRepository } from "../repositories/draft-order"
import { OrderRepository } from "../repositories/order" import { OrderRepository } from "../repositories/order"
import { PaymentRepository } from "../repositories/payment" import { PaymentRepository } from "../repositories/payment"
@@ -22,6 +28,7 @@ import EventBusService from "./event-bus"
import LineItemService from "./line-item" import LineItemService from "./line-item"
import ProductVariantService from "./product-variant" import ProductVariantService from "./product-variant"
import ShippingOptionService from "./shipping-option" import ShippingOptionService from "./shipping-option"
import { GenerateInputData } from "../types/line-item"
type InjectedDependencies = { type InjectedDependencies = {
manager: EntityManager manager: EntityManager
@@ -275,7 +282,7 @@ class DraftOrderService extends TransactionBaseService {
const cartServiceTx = const cartServiceTx =
this.cartService_.withTransaction(transactionManager) this.cartService_.withTransaction(transactionManager)
const createdCart = await cartServiceTx.create({ let createdCart = await cartServiceTx.create({
type: CartType.DRAFT_ORDER, type: CartType.DRAFT_ORDER,
...rawCart, ...rawCart,
}) })
@@ -297,64 +304,108 @@ class DraftOrderService extends TransactionBaseService {
const lineItemServiceTx = const lineItemServiceTx =
this.lineItemService_.withTransaction(transactionManager) this.lineItemService_.withTransaction(transactionManager)
for (const item of items || []) { const itemsToGenerate: GenerateInputData[] = []
const itemsToCreate: Partial<LineItem>[] = []
// prepare that for next steps
;(items ?? []).forEach((item) => {
if (item.variant_id) { if (item.variant_id) {
const line = await lineItemServiceTx.generate( itemsToGenerate.push({
item.variant_id, variantId: item.variant_id,
data.region_id,
item.quantity,
{
metadata: item?.metadata || {},
unit_price: item.unit_price,
}
)
await lineItemServiceTx.create({
...line,
cart_id: createdCart.id,
})
} else {
let price
if (typeof item.unit_price === `undefined` || item.unit_price < 0) {
price = 0
} else {
price = item.unit_price
}
// custom line items can be added to a draft order
await lineItemServiceTx.create({
cart_id: createdCart.id,
has_shipping: true,
title: item.title || "Custom item",
allow_discounts: false,
unit_price: price,
quantity: item.quantity, quantity: item.quantity,
metadata: item.metadata,
unit_price: item.unit_price,
}) })
return
} }
let price
if (!isDefined(item.unit_price) || item.unit_price < 0) {
price = 0
} else {
price = item.unit_price
}
itemsToCreate.push({
cart_id: createdCart.id,
has_shipping: true,
title: item.title || "Custom item",
allow_discounts: false,
unit_price: price,
quantity: item.quantity,
})
})
const promises: Promise<any>[] = []
// generate line item link to a variant
if (itemsToGenerate.length) {
const generatedLines = await lineItemServiceTx.generate(
itemsToGenerate,
{
region_id: data.region_id,
}
)
const toCreate = generatedLines.map((line) => ({
...line,
cart_id: createdCart.id,
}))
promises.push(lineItemServiceTx.create(toCreate))
} }
// custom line items can be added to a draft order
if (itemsToCreate.length) {
promises.push(lineItemServiceTx.create(itemsToCreate))
}
const shippingMethodToCreate: Partial<ShippingMethod>[] = []
shipping_methods.forEach((method) => {
if (isDefined(method.price)) {
shippingMethodToCreate.push({
shipping_option_id: method.option_id,
cart_id: createdCart.id,
price: method.price,
})
return
}
})
if (shippingMethodToCreate.length) {
await this.customShippingOptionService_
.withTransaction(transactionManager)
.create(shippingMethodToCreate)
}
createdCart = await cartServiceTx.retrieveWithTotals(createdCart.id, {
relations: [
"shipping_methods",
"shipping_methods.shipping_option",
"items",
"items.variant",
"items.variant.product",
"payment_sessions",
],
})
shipping_methods.forEach((method) => {
promises.push(
cartServiceTx.addShippingMethod(
createdCart,
method.option_id,
method.data
)
)
})
await Promise.all(promises)
if (discounts?.length) { if (discounts?.length) {
await cartServiceTx.update(createdCart.id, { discounts }) await cartServiceTx.update(createdCart.id, { discounts })
} }
for (const method of shipping_methods) {
if (typeof method.price !== "undefined") {
await this.customShippingOptionService_
.withTransaction(transactionManager)
.create({
shipping_option_id: method.option_id,
cart_id: createdCart.id,
price: method.price,
})
}
await cartServiceTx.addShippingMethod(
createdCart.id,
method.option_id,
method.data
)
}
return result return result
} }
) )
+13 -1
View File
@@ -215,9 +215,11 @@ class LineItemService extends TransactionBaseService {
quantity: quantity as number, quantity: quantity as number,
} }
: variantIdOrData : variantIdOrData
const resolvedContext = isString(variantIdOrData) const resolvedContext = isString(variantIdOrData)
? context ? context
: (regionIdOrContext as GenerateLineItemContext) : (regionIdOrContext as GenerateLineItemContext)
const regionId = ( const regionId = (
isString(variantIdOrData) isString(variantIdOrData)
? regionIdOrContext ? regionIdOrContext
@@ -228,6 +230,10 @@ class LineItemService extends TransactionBaseService {
Array.isArray(data) ? data : [data] Array.isArray(data) ? data : [data]
) as GenerateInputData[] ) as GenerateInputData[]
const resolvedDataMap = new Map(
resolvedData.map((d) => [d.variantId, d])
)
const variants = await this.productVariantService_.list( const variants = await this.productVariantService_.list(
{ {
id: resolvedData.map((d) => d.variantId), id: resolvedData.map((d) => d.variantId),
@@ -242,7 +248,11 @@ class LineItemService extends TransactionBaseService {
for (const variant of variants) { for (const variant of variants) {
variantsMap.set(variant.id, variant) variantsMap.set(variant.id, variant)
if (resolvedContext.unit_price == null) { const variantResolvedData = resolvedDataMap.get(variant.id)
if (
resolvedContext.unit_price == null &&
variantResolvedData?.unit_price == null
) {
variantIdsToCalculatePricingFor.push(variant.id) variantIdsToCalculatePricingFor.push(variant.id)
} }
} }
@@ -269,6 +279,8 @@ class LineItemService extends TransactionBaseService {
variantData.quantity, variantData.quantity,
{ {
...resolvedContext, ...resolvedContext,
unit_price: variantData.unit_price ?? resolvedContext.unit_price,
metadata: variantData.metadata ?? resolvedContext.metadata,
variantPricing, variantPricing,
} }
) )
+2
View File
@@ -3,6 +3,8 @@ import { CalculationContextData } from "./totals"
export type GenerateInputData = { export type GenerateInputData = {
variantId: string variantId: string
quantity: number quantity: number
metadata?: Record<string, unknown>
unit_price?: number
} }
export type GenerateLineItemContext = { export type GenerateLineItemContext = {