Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(medusa): Performance improvement of listing shipping options #2632

Merged
merged 8 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/metal-kangaroos-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@medusajs/medusa": patch
---

feat(medusa): Performance improvements of listing shipping options
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
}
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions packages/medusa/src/interfaces/transaction-base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/medusa/src/services/__tests__/shipping-profile.js
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -188,7 +188,7 @@ describe("ShippingProfileService", () => {
},
])
}),
validateCartOption: jest.fn().mockImplementation((s) => s),
validateCartOption: jest.fn().mockImplementation(async (s) => s),
withTransaction: function () {
return this
},
Expand Down
7 changes: 5 additions & 2 deletions packages/medusa/src/services/fulfillment-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
FulfillmentProvider,
LineItem,
Order,
Return,
ShippingMethod,
ShippingOption,
} from "../models"
Expand Down Expand Up @@ -150,7 +149,11 @@ class FulfillmentProviderService extends TransactionBaseService {
cart?: Order | Cart
): Promise<number> {
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<boolean> {
Expand Down
45 changes: 21 additions & 24 deletions packages/medusa/src/services/pricing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = []
Expand Down Expand Up @@ -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
}

/**
Expand Down Expand Up @@ -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<PricedShippingOption>[] = []

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)
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/medusa/src/services/shipping-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) => {
Expand All @@ -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"
Expand Down
50 changes: 23 additions & 27 deletions packages/medusa/src/services/shipping-profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { EntityManager } from "typeorm"
import { TransactionBaseService } from "../interfaces"
import {
Cart,
CustomShippingOption,
ShippingOption,
ShippingProfile,
ShippingProfileType,
Expand Down Expand Up @@ -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<string, CustomShippingOption>()

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,
Expand All @@ -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)
})
}

Expand All @@ -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<string>()

cart.items.forEach((item) => {
if (item.variant?.product) {
profileIds.add(item.variant.product.profile_id)
}
})

return acc
}, [] as string[])
return [...profileIds]
}
}

Expand Down
7 changes: 1 addition & 6 deletions packages/medusa/src/services/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down