diff --git a/.changeset/metal-kangaroos-occur.md b/.changeset/metal-kangaroos-occur.md new file mode 100644 index 0000000000..7e6031df91 --- /dev/null +++ b/.changeset/metal-kangaroos-occur.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +feat(medusa): Performance improvements of listing shipping options diff --git a/packages/medusa/src/api/routes/store/shipping-options/__tests__/list-shipping-options.js b/packages/medusa/src/api/routes/store/shipping-options/__tests__/list-shipping-options.js index 672740f805..ae64176c9c 100644 --- a/packages/medusa/src/api/routes/store/shipping-options/__tests__/list-shipping-options.js +++ b/packages/medusa/src/api/routes/store/shipping-options/__tests__/list-shipping-options.js @@ -23,13 +23,7 @@ describe("GET /store/shipping-options", () => { expect(CartServiceMock.retrieveWithTotals).toHaveBeenCalledWith( IdMap.getId("emptyCart"), { - relations: [ - "region", - "items", - "items.adjustments", - "items.variant", - "items.variant.product", - ], + relations: ["items.variant", "items.variant.product"], } ) }) diff --git a/packages/medusa/src/api/routes/store/shipping-options/list-shipping-options.ts b/packages/medusa/src/api/routes/store/shipping-options/list-shipping-options.ts index 100fcf27e9..2b16c3be32 100644 --- a/packages/medusa/src/api/routes/store/shipping-options/list-shipping-options.ts +++ b/packages/medusa/src/api/routes/store/shipping-options/list-shipping-options.ts @@ -56,13 +56,7 @@ export default async (req, res) => { ) const cart = await cartService.retrieveWithTotals(cart_id, { - relations: [ - "region", - "items", - "items.adjustments", - "items.variant", - "items.variant.product", - ], + relations: ["items.variant", "items.variant.product"], }) const options = await shippingProfileService.fetchCartOptions(cart) diff --git a/packages/medusa/src/interfaces/transaction-base-service.ts b/packages/medusa/src/interfaces/transaction-base-service.ts index 2ef4534df4..5f9e914b0c 100644 --- a/packages/medusa/src/interfaces/transaction-base-service.ts +++ b/packages/medusa/src/interfaces/transaction-base-service.ts @@ -16,13 +16,11 @@ export abstract class TransactionBaseService { } const cloned = new (this.constructor as any)( - { - ...this.__container__, - manager: transactionManager, - }, + this.__container__, this.__configModule__ ) + cloned.manager_ = transactionManager cloned.transactionManager_ = transactionManager return cloned diff --git a/packages/medusa/src/services/__tests__/shipping-profile.js b/packages/medusa/src/services/__tests__/shipping-profile.js index 42b5a2e040..c694721411 100644 --- a/packages/medusa/src/services/__tests__/shipping-profile.js +++ b/packages/medusa/src/services/__tests__/shipping-profile.js @@ -1,4 +1,4 @@ -import { IdMap, MockRepository, MockManager } from "medusa-test-utils" +import { IdMap, MockManager, MockRepository } from "medusa-test-utils" import ShippingProfileService from "../shipping-profile" describe("ShippingProfileService", () => { @@ -188,7 +188,7 @@ describe("ShippingProfileService", () => { }, ]) }), - validateCartOption: jest.fn().mockImplementation((s) => s), + validateCartOption: jest.fn().mockImplementation(async (s) => s), withTransaction: function () { return this }, diff --git a/packages/medusa/src/services/fulfillment-provider.ts b/packages/medusa/src/services/fulfillment-provider.ts index 6960a510d8..b7609bc182 100644 --- a/packages/medusa/src/services/fulfillment-provider.ts +++ b/packages/medusa/src/services/fulfillment-provider.ts @@ -8,7 +8,6 @@ import { FulfillmentProvider, LineItem, Order, - Return, ShippingMethod, ShippingOption, } from "../models" @@ -150,7 +149,11 @@ class FulfillmentProviderService extends TransactionBaseService { cart?: Order | Cart ): Promise { const provider = this.retrieveProvider(option.provider_id) - return provider.calculatePrice(option.data, data, cart) as unknown as number + return (await provider.calculatePrice( + option.data, + data, + cart + )) as unknown as number } async validateOption(option: ShippingOption): Promise { diff --git a/packages/medusa/src/services/pricing.ts b/packages/medusa/src/services/pricing.ts index aed79bd305..fb8cdddbb2 100644 --- a/packages/medusa/src/services/pricing.ts +++ b/packages/medusa/src/services/pricing.ts @@ -432,7 +432,9 @@ class PricingService extends TransactionBaseService { if ("automatic_taxes" in context) { pricingContext = context } else { - pricingContext = await this.collectPricingContext(context) + pricingContext = + (context as PricingContext) ?? + (await this.collectPricingContext(context)) } let shippingOptionRates: TaxServiceRate[] = [] @@ -470,14 +472,12 @@ class PricingService extends TransactionBaseService { ) const totalInclTax = includesTax ? price : price + taxAmount - const result: PricedShippingOption = { + return { ...shippingOption, price_incl_tax: totalInclTax, tax_rates: shippingOptionRates, tax_amount: taxAmount, } - - return result } /** @@ -508,29 +508,26 @@ class PricingService extends TransactionBaseService { }) ) - return await Promise.all( - shippingOptions.map(async (shippingOption) => { - const pricingContext = contexts.find( - (c) => c.region_id === shippingOption.region_id - ) + const shippingOptionPricingPromises: Promise[] = [] - if (!pricingContext) { - throw new MedusaError( - MedusaError.Types.UNEXPECTED_STATE, - "Could not find pricing context for shipping option" - ) - } + shippingOptions.map(async (shippingOption) => { + const pricingContext = contexts.find( + (c) => c.region_id === shippingOption.region_id + ) - const shippingOptionPricing = await this.getShippingOptionPricing( - shippingOption, - pricingContext.context + if (!pricingContext) { + throw new MedusaError( + MedusaError.Types.UNEXPECTED_STATE, + "Could not find pricing context for shipping option" ) - return { - ...shippingOption, - ...shippingOptionPricing, - } - }) - ) + } + + shippingOptionPricingPromises.push( + this.getShippingOptionPricing(shippingOption, pricingContext.context) + ) + }) + + return await Promise.all(shippingOptionPricingPromises) } } diff --git a/packages/medusa/src/services/shipping-option.ts b/packages/medusa/src/services/shipping-option.ts index 5824992359..c7a60db717 100644 --- a/packages/medusa/src/services/shipping-option.ts +++ b/packages/medusa/src/services/shipping-option.ts @@ -15,9 +15,9 @@ import { ShippingOptionRequirementRepository } from "../repositories/shipping-op import { ExtendedFindConfig, FindConfig, Selector } from "../types/common" import { CreateShippingMethodDto, + CreateShippingOptionInput, ShippingMethodUpdate, UpdateShippingOptionInput, - CreateShippingOptionInput, } from "../types/shipping-options" import { buildQuery, isDefined, setMetadata } from "../utils" import FulfillmentProviderService from "./fulfillment-provider" @@ -370,7 +370,7 @@ class ShippingOptionService extends TransactionBaseService { ) } - const amount = (option.includes_tax ? cart.total : cart.subtotal) as number + const amount = option.includes_tax ? cart.total! : cart.subtotal! const requirementResults: boolean[] = option.requirements.map( (requirement) => { @@ -385,7 +385,7 @@ class ShippingOptionService extends TransactionBaseService { } ) - if (!requirementResults.every(Boolean)) { + if (requirementResults.some((requirement) => !requirement)) { throw new MedusaError( MedusaError.Types.NOT_ALLOWED, "The Cart does not satisfy the shipping option's requirements" diff --git a/packages/medusa/src/services/shipping-profile.ts b/packages/medusa/src/services/shipping-profile.ts index e9e98848a6..eb5c6a04f3 100644 --- a/packages/medusa/src/services/shipping-profile.ts +++ b/packages/medusa/src/services/shipping-profile.ts @@ -3,6 +3,7 @@ import { EntityManager } from "typeorm" import { TransactionBaseService } from "../interfaces" import { Cart, + CustomShippingOption, ShippingOption, ShippingProfile, ShippingProfileType, @@ -422,10 +423,14 @@ class ShippingProfileService extends TransactionBaseService { // if there are custom shipping options associated with the cart, return cart shipping options with custom price if (hasCustomShippingOptions) { + const customShippingOptionsMap = new Map() + + customShippingOptions.forEach((option) => { + customShippingOptionsMap.set(option.shipping_option_id, option) + }) + return rawOpts.map((so) => { - const customOption = customShippingOptions.find( - (cso) => cso.shipping_option_id === so.id - ) + const customOption = customShippingOptionsMap.get(so.id) return { ...so, @@ -434,24 +439,16 @@ class ShippingProfileService extends TransactionBaseService { }) as ShippingOption[] } - const options = await Promise.all( - rawOpts.map(async (so) => { - try { - const option = await this.shippingOptionService_ + return ( + await Promise.all( + rawOpts.map(async (so) => { + return await this.shippingOptionService_ .withTransaction(manager) .validateCartOption(so, cart) - if (option) { - return option - } - return null - } catch (err) { - // if validateCartOption fails it means the option is not valid - return null - } - }) - ) - - return options.filter(Boolean) as ShippingOption[] + .catch(() => null) // if validateCartOption fails it means the option is not valid + }) + ) + ).filter((option): option is ShippingOption => !!option) }) } @@ -461,16 +458,15 @@ class ShippingProfileService extends TransactionBaseService { * @return a list of product ids */ protected getProfilesInCart(cart: Cart): string[] { - return cart.items.reduce((acc, next) => { - // We may have line items that are not associated with a product - if (next.variant && next.variant.product) { - if (!acc.includes(next.variant.product.profile_id)) { - acc.push(next.variant.product.profile_id) - } - } + const profileIds = new Set() - return acc - }, [] as string[]) + cart.items.forEach((item) => { + if (item.variant?.product) { + profileIds.add(item.variant.product.profile_id) + } + }) + + return [...profileIds] } } diff --git a/packages/medusa/src/services/store.ts b/packages/medusa/src/services/store.ts index eb1644154b..c4401c9805 100644 --- a/packages/medusa/src/services/store.ts +++ b/packages/medusa/src/services/store.ts @@ -34,12 +34,7 @@ class StoreService extends TransactionBaseService { currencyRepository, eventBusService, }: InjectedDependencies) { - super({ - manager, - storeRepository, - currencyRepository, - eventBusService, - }) + super(arguments[0]) this.manager_ = manager this.storeRepository_ = storeRepository