From ac09b3cbeffc11c76b8e5972ee25688b17e5fa58 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Wed, 10 Sep 2025 12:20:31 +0200 Subject: [PATCH] Chore(): localised improvement to promotion module (#13446) **What** - Remove overserialization withing transaction and rely more on internal service or protected method instead which does not serialize and work with Infered entity type - Slightly rework loop complexity Overall, this gives a good spare of resources and time spent for serialization --- .changeset/smart-numbers-build.md | 5 + packages/modules/promotion/package.json | 2 +- .../src/services/promotion-module.ts | 123 +++++++++++------- .../src/utils/compute-actions/buy-get.ts | 10 +- .../src/utils/compute-actions/line-items.ts | 14 +- .../utils/compute-actions/shipping-methods.ts | 21 ++- .../src/utils/compute-actions/usage.ts | 4 +- .../src/utils/validations/promotion-rule.ts | 15 ++- .../integration-tests/__tests__/index.spec.ts | 2 +- 9 files changed, 129 insertions(+), 67 deletions(-) create mode 100644 .changeset/smart-numbers-build.md diff --git a/.changeset/smart-numbers-build.md b/.changeset/smart-numbers-build.md new file mode 100644 index 0000000000..c96b63c79f --- /dev/null +++ b/.changeset/smart-numbers-build.md @@ -0,0 +1,5 @@ +--- +"@medusajs/promotion": patch +--- + +Chore/localised improvement to promotion module diff --git a/packages/modules/promotion/package.json b/packages/modules/promotion/package.json index bc39a587ad..bc5f991b58 100644 --- a/packages/modules/promotion/package.json +++ b/packages/modules/promotion/package.json @@ -29,7 +29,7 @@ "resolve:aliases": "tsc --showConfig -p tsconfig.json > tsconfig.resolved.json && tsc-alias -p tsconfig.resolved.json && rimraf tsconfig.resolved.json", "build": "rimraf dist && tsc --build && npm run resolve:aliases", "test": "jest --runInBand --passWithNoTests --bail --forceExit -- src", - "test:integration": "jest --forceExit ", + "test:integration": "jest --forceExit -- integration-tests/__tests__/**/*.spec.ts", "migration:initial": "MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:create --initial", "migration:create": "MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:create", "migration:up": "MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:up", diff --git a/packages/modules/promotion/src/services/promotion-module.ts b/packages/modules/promotion/src/services/promotion-module.ts index 09b03d383a..97d6c02688 100644 --- a/packages/modules/promotion/src/services/promotion-module.ts +++ b/packages/modules/promotion/src/services/promotion-module.ts @@ -139,11 +139,26 @@ export default class PromotionModuleService } @InjectManager() - listActivePromotions( + async listActivePromotions( filters?: FilterablePromotionProps, config?: FindConfig, sharedContext?: Context ): Promise { + const activePromotions = await this.listActivePromotions_( + filters, + config, + sharedContext + ) + + return this.baseRepository_.serialize(activePromotions) + } + + @InjectManager() + protected async listActivePromotions_( + filters?: FilterablePromotionProps, + config?: FindConfig, + @MedusaContext() sharedContext?: Context + ): Promise[]> { // Ensure we share the same now date across all filters const now = new Date() const activeFilters = { @@ -170,7 +185,11 @@ export default class PromotionModuleService ], } - return this.listPromotions(activeFilters, config, sharedContext) + return await this.promotionService_.list( + activeFilters, + config, + sharedContext + ) } @InjectTransactionManager() @@ -185,7 +204,7 @@ export default class PromotionModuleService const campaignBudgetMap = new Map() const promotionCodeUsageMap = new Map() - const existingPromotions = await this.listActivePromotions( + const existingPromotions = await this.listActivePromotions_( { code: promotionCodes }, { relations: ["campaign", "campaign.budget"] }, sharedContext @@ -200,9 +219,10 @@ export default class PromotionModuleService } } - const existingPromotionsMap = new Map( - existingPromotions.map((promotion) => [promotion.code!, promotion]) - ) + const existingPromotionsMap = new Map< + string, + InferEntityType + >(existingPromotions.map((promotion) => [promotion.code!, promotion])) for (let computedAction of computedActions) { const promotion = existingPromotionsMap.get(computedAction.code) @@ -293,7 +313,7 @@ export default class PromotionModuleService const promotionCodeUsageMap = new Map() const campaignBudgetMap = new Map() - const existingPromotions = await this.listActivePromotions( + const existingPromotions = await this.listActivePromotions_( { code: computedActions .map((computedAction) => computedAction.code) @@ -312,9 +332,10 @@ export default class PromotionModuleService } } - const existingPromotionsMap = new Map( - existingPromotions.map((promotion) => [promotion.code!, promotion]) - ) + const existingPromotionsMap = new Map< + string, + InferEntityType + >(existingPromotions.map((promotion) => [promotion.code!, promotion])) for (let computedAction of computedActions) { const promotion = existingPromotionsMap.get(computedAction.code) @@ -433,27 +454,19 @@ export default class PromotionModuleService const methodIdPromoValueMap = new Map() - const automaticPromotions = preventAutoPromotions - ? [] - : await this.listActivePromotions( - { is_automatic: true }, - { select: ["code"] }, - sharedContext - ) - - const automaticPromotionCodes = automaticPromotions.map((p) => p.code!) - const promotionCodesToApply = [ - ...promotionCodes, - ...automaticPromotionCodes, - ...appliedCodes, - ] + const promotionCodesToApply = [...promotionCodes, ...appliedCodes] const uniquePromotionCodes = Array.from(new Set(promotionCodesToApply)) - const promotions = await this.listActivePromotions( - { code: uniquePromotionCodes }, + const queryFilter = preventAutoPromotions + ? { code: uniquePromotionCodes } + : { + $or: [{ code: uniquePromotionCodes }, { is_automatic: true }], + } + + const promotions = await this.listActivePromotions_( + queryFilter, { - take: null, order: { application_method: { value: "DESC" } }, relations: [ "application_method", @@ -470,9 +483,18 @@ export default class PromotionModuleService sharedContext ) - const existingPromotionsMap = new Map( - promotions.map((promotion) => [promotion.code!, promotion]) - ) + const existingPromotionsMap = new Map< + string, + InferEntityType + >(promotions.map((promotion) => [promotion.code!, promotion])) + + const automaticPromotionCodes: string[] = [] + + for (const promotion of promotions) { + if (promotion.is_automatic) { + automaticPromotionCodes.push(promotion.code!) + } + } for (const [code, adjustments] of codeAdjustmentMap.entries()) { for (const adjustment of adjustments.items) { @@ -492,11 +514,14 @@ export default class PromotionModuleService } } + const promotionCodeSet = new Set(promotionCodes) + const automaticPromotionCodeSet = new Set(automaticPromotionCodes) + const sortedPromotionsToApply = promotions .filter( (p) => - promotionCodes.includes(p.code!) || - automaticPromotionCodes.includes(p.code!) + promotionCodeSet.has(p.code!) || + automaticPromotionCodeSet.has(p.code!) ) .sort(ComputeActionUtils.sortByBuyGetType) @@ -511,15 +536,15 @@ export default class PromotionModuleService for (const promotionToApply of sortedPromotionsToApply) { const promotion = existingPromotionsMap.get(promotionToApply.code!)! + if (!promotion.application_method) { + continue + } + const { application_method: applicationMethod, rules: promotionRules = [], } = promotion - if (!applicationMethod) { - continue - } - const isCurrencyCodeValid = !isPresent(applicationMethod.currency_code) || applicationContext.currency_code === applicationMethod.currency_code @@ -1040,15 +1065,24 @@ export default class PromotionModuleService ) { const promotionRuleIds = data.map((d) => d.id) - const promotionRules = await this.listPromotionRules( + const promotionRules = await this.promotionRuleService_.list( { id: promotionRuleIds }, { relations: ["values"] }, sharedContext ) + const existingPromotionRuleIds: string[] = [] + const promotionRulesMap: Map = + new Map() + + for (const promotionRule of promotionRules) { + existingPromotionRuleIds.push(promotionRule.id) + promotionRulesMap.set(promotionRule.id, promotionRule) + } + const invalidRuleId = arrayDifference( deduplicate(promotionRuleIds), - promotionRules.map((pr) => pr.id) + existingPromotionRuleIds ) if (invalidRuleId.length) { @@ -1058,10 +1092,6 @@ export default class PromotionModuleService ) } - const promotionRulesMap = new Map( - promotionRules.map((pr) => [pr.id, pr]) - ) - const rulesToUpdate: PromotionTypes.UpdatePromotionRuleDTO[] = [] const ruleValueIdsToDelete: string[] = [] const ruleValuesToCreate: CreatePromotionRuleValueDTO[] = [] @@ -1489,15 +1519,16 @@ export default class PromotionModuleService const updateBudgetData: UpdateCampaignBudgetDTO[] = [] const createBudgetData: CreateCampaignBudgetDTO[] = [] - const existingCampaigns = await this.listCampaigns( + const existingCampaigns = await this.campaignService_.list( { id: campaignIds }, { relations: ["budget"] }, sharedContext ) - const existingCampaignsMap = new Map( - existingCampaigns.map((campaign) => [campaign.id, campaign]) - ) + const existingCampaignsMap = new Map< + string, + InferEntityType + >(existingCampaigns.map((campaign) => [campaign.id, campaign])) for (const updateCampaignData of data) { const { budget: budgetData, ...campaignData } = updateCampaignData diff --git a/packages/modules/promotion/src/utils/compute-actions/buy-get.ts b/packages/modules/promotion/src/utils/compute-actions/buy-get.ts index 496d4a92c9..7d2af2a35e 100644 --- a/packages/modules/promotion/src/utils/compute-actions/buy-get.ts +++ b/packages/modules/promotion/src/utils/compute-actions/buy-get.ts @@ -1,6 +1,7 @@ import { BigNumberInput, ComputeActionItemLine, + InferEntityType, PromotionTypes, } from "@medusajs/framework/types" import { @@ -12,6 +13,7 @@ import { } from "@medusajs/framework/utils" import { areRulesValidForContext } from "../validations" import { computeActionForBudgetExceeded } from "./usage" +import { Promotion } from "@models" export type EligibleItem = { item_id: string @@ -23,7 +25,7 @@ function sortByPrice(a: ComputeActionItemLine, b: ComputeActionItemLine) { } function isValidPromotionContext( - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, itemsContext: ComputeActionItemLine[] ): boolean { if (!itemsContext) { @@ -52,7 +54,7 @@ function isValidPromotionContext( } function normalizePromotionApplicationConfiguration( - promotion: PromotionTypes.PromotionDTO + promotion: PromotionTypes.PromotionDTO | InferEntityType ) { const minimumBuyQuantity = MathBN.convert( promotion.application_method?.buy_rules_min_quantity ?? 0 @@ -275,7 +277,7 @@ function applyPromotionToTargetItems( targetItems: EligibleItem[], itemIdPromotionAmountMap: Map, methodIdPromoValueMap: Map, - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, itemsMap: Map, applicationConfig: PromotionConfig ): { @@ -456,7 +458,7 @@ function filterItemsByPromotionRules( } export function getComputedActionsForBuyGet( - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, itemsContext: ComputeActionItemLine[], methodIdPromoValueMap: Map, eligibleBuyItemMap: Map, diff --git a/packages/modules/promotion/src/utils/compute-actions/line-items.ts b/packages/modules/promotion/src/utils/compute-actions/line-items.ts index 685871de42..ca43dccfbb 100644 --- a/packages/modules/promotion/src/utils/compute-actions/line-items.ts +++ b/packages/modules/promotion/src/utils/compute-actions/line-items.ts @@ -1,4 +1,9 @@ -import { ApplicationMethodAllocationValues, BigNumberInput, PromotionTypes, } from "@medusajs/framework/types" +import { + ApplicationMethodAllocationValues, + BigNumberInput, + InferEntityType, + PromotionTypes, +} from "@medusajs/framework/types" import { ApplicationMethodAllocation, ApplicationMethodTargetType, @@ -10,6 +15,7 @@ import { } from "@medusajs/framework/utils" import { areRulesValidForContext } from "../validations" import { computeActionForBudgetExceeded } from "./usage" +import { Promotion } from "@models" function validateContext( contextKey: string, @@ -24,7 +30,7 @@ function validateContext( } export function getComputedActionsForItems( - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, items: PromotionTypes.ComputeActionContext[TargetType.ITEMS], appliedPromotionsMap: Map, allocationOverride?: ApplicationMethodAllocationValues @@ -40,7 +46,7 @@ export function getComputedActionsForItems( } function applyPromotionToItems( - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, items: PromotionTypes.ComputeActionContext[TargetType.ITEMS], appliedPromotionsMap: Map, allocationOverride?: ApplicationMethodAllocationValues @@ -149,7 +155,7 @@ function getValidItemsForPromotion( items: | PromotionTypes.ComputeActionContext[TargetType.ITEMS] | PromotionTypes.ComputeActionContext[TargetType.SHIPPING_METHODS], - promotion: PromotionTypes.PromotionDTO + promotion: PromotionTypes.PromotionDTO | InferEntityType ) { if (!items?.length || !promotion?.application_method) { return [] diff --git a/packages/modules/promotion/src/utils/compute-actions/shipping-methods.ts b/packages/modules/promotion/src/utils/compute-actions/shipping-methods.ts index b496eb51fd..999faa341b 100644 --- a/packages/modules/promotion/src/utils/compute-actions/shipping-methods.ts +++ b/packages/modules/promotion/src/utils/compute-actions/shipping-methods.ts @@ -1,4 +1,8 @@ -import { BigNumberInput, PromotionTypes } from "@medusajs/framework/types" +import { + BigNumberInput, + InferEntityType, + PromotionTypes, +} from "@medusajs/framework/types" import { ApplicationMethodAllocation, ApplicationMethodTargetType, @@ -9,9 +13,10 @@ import { } from "@medusajs/framework/utils" import { areRulesValidForContext } from "../validations" import { computeActionForBudgetExceeded } from "./usage" +import { Promotion } from "@models" export function getComputedActionsForShippingMethods( - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, shippingMethodApplicationContext: PromotionTypes.ComputeActionContext[ApplicationMethodTargetType.SHIPPING_METHODS], methodIdPromoValueMap: Map ): PromotionTypes.ComputeActions[] { @@ -47,7 +52,7 @@ export function getComputedActionsForShippingMethods( } export function applyPromotionToShippingMethods( - promotion: PromotionTypes.PromotionDTO, + promotion: PromotionTypes.PromotionDTO | InferEntityType, shippingMethods: PromotionTypes.ComputeActionContext[ApplicationMethodTargetType.SHIPPING_METHODS], methodIdPromoValueMap: Map ): PromotionTypes.ComputeActions[] { @@ -126,10 +131,16 @@ export function applyPromotionToShippingMethods( const appliedPromoValue = methodIdPromoValueMap.get(method.id) ?? 0 const applicableTotal = MathBN.sub(method.subtotal, appliedPromoValue) - let applicablePromotionValue = MathBN.mult(MathBN.div(applicableTotal, totalApplicableValue), promotionValue) + let applicablePromotionValue = MathBN.mult( + MathBN.div(applicableTotal, totalApplicableValue), + promotionValue + ) if (applicationMethod?.type === ApplicationMethodType.PERCENTAGE) { - applicablePromotionValue = MathBN.mult(MathBN.div(promotionValue, 100), applicableTotal) + applicablePromotionValue = MathBN.mult( + MathBN.div(promotionValue, 100), + applicableTotal + ) } const amount = MathBN.min(applicablePromotionValue, applicableTotal) diff --git a/packages/modules/promotion/src/utils/compute-actions/usage.ts b/packages/modules/promotion/src/utils/compute-actions/usage.ts index 0396e967e0..60271060da 100644 --- a/packages/modules/promotion/src/utils/compute-actions/usage.ts +++ b/packages/modules/promotion/src/utils/compute-actions/usage.ts @@ -1,6 +1,7 @@ import { BigNumberInput, CampaignBudgetExceededAction, + InferEntityType, PromotionDTO, } from "@medusajs/framework/types" import { @@ -8,9 +9,10 @@ import { ComputedActions, MathBN, } from "@medusajs/framework/utils" +import { Promotion } from "@models" export function computeActionForBudgetExceeded( - promotion: PromotionDTO, + promotion: PromotionDTO | InferEntityType, amount: BigNumberInput ): CampaignBudgetExceededAction | void { const campaignBudget = promotion.campaign?.budget diff --git a/packages/modules/promotion/src/utils/validations/promotion-rule.ts b/packages/modules/promotion/src/utils/validations/promotion-rule.ts index a71a5141af..064cf2af7c 100644 --- a/packages/modules/promotion/src/utils/validations/promotion-rule.ts +++ b/packages/modules/promotion/src/utils/validations/promotion-rule.ts @@ -1,5 +1,6 @@ import { ApplicationMethodTargetTypeValues, + InferEntityType, PromotionRuleDTO, PromotionRuleOperatorValues, } from "@medusajs/framework/types" @@ -12,6 +13,7 @@ import { isString, pickValueFromObject, } from "@medusajs/framework/utils" +import { PromotionRule } from "@models" import { CreatePromotionRuleDTO } from "@types" export function validatePromotionRuleAttributes( @@ -51,7 +53,7 @@ export function validatePromotionRuleAttributes( } export function areRulesValidForContext( - rules: PromotionRuleDTO[], + rules: PromotionRuleDTO[] | InferEntityType[], context: Record, contextScope: ApplicationMethodTargetTypeValues ): boolean { @@ -63,14 +65,17 @@ export function areRulesValidForContext( const isShippingScope = contextScope === ApplicationMethodTargetType.SHIPPING_METHODS - return rules.every((rule) => { + return ("initialized" in rules ? [...rules] : rules).every((rule) => { if (!rule.attribute || !rule.values?.length) { return false } - const validRuleValues = rule.values - .filter((v) => isString(v.value)) - .map((v) => v.value as string) + const validRuleValues: string[] = [] + for (const value of rule.values) { + if (isString(value.value)) { + validRuleValues.push(value.value as string) + } + } if (!validRuleValues.length) { return false diff --git a/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts b/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts index 2b17d5fb60..f005582a39 100644 --- a/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts +++ b/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts @@ -539,7 +539,7 @@ moduleIntegrationTestRunner({ throwOnError: false, }) - await setTimeout(1000) + await setTimeout(4000) expect(lastExepectHaveBeenCalledTimes).toEqual(1) })