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

@@ -0,0 +1,5 @@
+---
+"@medusajs/promotion": patch
+---
+
+Chore/localised improvement to promotion module

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",

packages/modules/promotion/src/services/promotion-module.ts

@@ -139,11 +139,26 @@ export default class PromotionModuleService
   }
 
   @InjectManager()
-  listActivePromotions(
+  async listActivePromotions(
     filters?: FilterablePromotionProps,
     config?: FindConfig<PromotionDTO>,
     sharedContext?: Context
   ): Promise<PromotionDTO[]> {
+    const activePromotions = await this.listActivePromotions_(
+      filters,
+      config,
+      sharedContext
+    )
+
+    return this.baseRepository_.serialize<PromotionDTO[]>(activePromotions)
+  }
+
+  @InjectManager()
+  protected async listActivePromotions_(
+    filters?: FilterablePromotionProps,
+    config?: FindConfig<PromotionDTO>,
+    @MedusaContext() sharedContext?: Context
+  ): Promise<InferEntityType<typeof Promotion>[]> {
     // 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<string, UpdateCampaignBudgetDTO>()
     const promotionCodeUsageMap = new Map<string, boolean>()
 
-    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<string, PromotionTypes.PromotionDTO>(
+    const existingPromotionsMap = new Map<
-      existingPromotions.map((promotion) => [promotion.code!, promotion])
+      string,
-    )
+      InferEntityType<typeof Promotion>
+    >(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<string, boolean>()
     const campaignBudgetMap = new Map<string, UpdateCampaignBudgetDTO>()
 
-    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<string, PromotionTypes.PromotionDTO>(
+    const existingPromotionsMap = new Map<
-      existingPromotions.map((promotion) => [promotion.code!, promotion])
+      string,
-    )
+      InferEntityType<typeof Promotion>
+    >(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<string, number>()
 
-    const automaticPromotions = preventAutoPromotions
+    const promotionCodesToApply = [...promotionCodes, ...appliedCodes]
-      ? []
-      : await this.listActivePromotions(
-          { is_automatic: true },
-          { select: ["code"] },
-          sharedContext
-        )
-
-    const automaticPromotionCodes = automaticPromotions.map((p) => p.code!)
-    const promotionCodesToApply = [
-      ...promotionCodes,
-      ...automaticPromotionCodes,
-      ...appliedCodes,
-    ]
 
     const uniquePromotionCodes = Array.from(new Set(promotionCodesToApply))
 
-    const promotions = await this.listActivePromotions(
+    const queryFilter = preventAutoPromotions
-      { code: uniquePromotionCodes },
+      ? { 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<string, PromotionTypes.PromotionDTO>(
+    const existingPromotionsMap = new Map<
-      promotions.map((promotion) => [promotion.code!, promotion])
+      string,
-    )
+      InferEntityType<typeof Promotion>
+    >(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<string>(promotionCodes)
+    const automaticPromotionCodeSet = new Set<string>(automaticPromotionCodes)
+
     const sortedPromotionsToApply = promotions
       .filter(
         (p) =>
-          promotionCodes.includes(p.code!) ||
+          promotionCodeSet.has(p.code!) ||
-          automaticPromotionCodes.includes(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<string, PromotionTypes.PromotionRuleDTO> =
+      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<string, PromotionTypes.PromotionRuleDTO>(
-      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<string, PromotionTypes.CampaignDTO>(
+    const existingCampaignsMap = new Map<
-      existingCampaigns.map((campaign) => [campaign.id, campaign])
+      string,
-    )
+      InferEntityType<typeof Campaign>
+    >(existingCampaigns.map((campaign) => [campaign.id, campaign]))
 
     for (const updateCampaignData of data) {
       const { budget: budgetData, ...campaignData } = updateCampaignData

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<typeof Promotion>,
   itemsContext: ComputeActionItemLine[]
 ): boolean {
   if (!itemsContext) {
@@ -52,7 +54,7 @@ function isValidPromotionContext(
 }
 
 function normalizePromotionApplicationConfiguration(
-  promotion: PromotionTypes.PromotionDTO
+  promotion: PromotionTypes.PromotionDTO | InferEntityType<typeof Promotion>
 ) {
   const minimumBuyQuantity = MathBN.convert(
     promotion.application_method?.buy_rules_min_quantity ?? 0
@@ -275,7 +277,7 @@ function applyPromotionToTargetItems(
   targetItems: EligibleItem[],
   itemIdPromotionAmountMap: Map<string, BigNumberInput>,
   methodIdPromoValueMap: Map<string, BigNumberInput>,
-  promotion: PromotionTypes.PromotionDTO,
+  promotion: PromotionTypes.PromotionDTO | InferEntityType<typeof Promotion>,
   itemsMap: Map<string, ComputeActionItemLine>,
   applicationConfig: PromotionConfig
 ): {
@@ -456,7 +458,7 @@ function filterItemsByPromotionRules(
 }
 
 export function getComputedActionsForBuyGet(
-  promotion: PromotionTypes.PromotionDTO,
+  promotion: PromotionTypes.PromotionDTO | InferEntityType<typeof Promotion>,
   itemsContext: ComputeActionItemLine[],
   methodIdPromoValueMap: Map<string, BigNumberInput>,
   eligibleBuyItemMap: Map<string, EligibleItem[]>,

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<typeof Promotion>,
   items: PromotionTypes.ComputeActionContext[TargetType.ITEMS],
   appliedPromotionsMap: Map<string, number>,
   allocationOverride?: ApplicationMethodAllocationValues
@@ -40,7 +46,7 @@ export function getComputedActionsForItems(
 }
 
 function applyPromotionToItems(
-  promotion: PromotionTypes.PromotionDTO,
+  promotion: PromotionTypes.PromotionDTO | InferEntityType<typeof Promotion>,
   items: PromotionTypes.ComputeActionContext[TargetType.ITEMS],
   appliedPromotionsMap: Map<string, BigNumberInput>,
   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<typeof Promotion>
 ) {
   if (!items?.length || !promotion?.application_method) {
     return []

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<typeof Promotion>,
   shippingMethodApplicationContext: PromotionTypes.ComputeActionContext[ApplicationMethodTargetType.SHIPPING_METHODS],
   methodIdPromoValueMap: Map<string, number>
 ): PromotionTypes.ComputeActions[] {
@@ -47,7 +52,7 @@ export function getComputedActionsForShippingMethods(
 }
 
 export function applyPromotionToShippingMethods(
-  promotion: PromotionTypes.PromotionDTO,
+  promotion: PromotionTypes.PromotionDTO | InferEntityType<typeof Promotion>,
   shippingMethods: PromotionTypes.ComputeActionContext[ApplicationMethodTargetType.SHIPPING_METHODS],
   methodIdPromoValueMap: Map<string, BigNumberInput>
 ): 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)

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<typeof Promotion>,
   amount: BigNumberInput
 ): CampaignBudgetExceededAction | void {
   const campaignBudget = promotion.campaign?.budget

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<typeof PromotionRule>[],
   context: Record<string, any>,
   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
+    const validRuleValues: string[] = []
-      .filter((v) => isString(v.value))
+    for (const value of rule.values) {
-      .map((v) => v.value as string)
+      if (isString(value.value)) {
+        validRuleValues.push(value.value as string)
+      }
+    }
 
     if (!validRuleValues.length) {
       return false

packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts

@@ -539,7 +539,7 @@ moduleIntegrationTestRunner<IWorkflowEngineService>({
             throwOnError: false,
           })
 
-          await setTimeout(1000)
+          await setTimeout(4000)
 
           expect(lastExepectHaveBeenCalledTimes).toEqual(1)
         })