feat(pricing,types,utils): Move calculate pricing query to a repository + rule type validation (#5294)

.changeset/breezy-elephants-talk.md

@@ -0,0 +1,7 @@
+---
+"@medusajs/pricing": patch
+"@medusajs/utils": patch
+"@medusajs/types": patch
+---
+
+feat(pricing,types,utils): Move calculate pricing query to a repository + rule type validation

packages/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts

@@ -343,7 +343,7 @@ describe("PricingModule Service - Calculate Price", () => {
       )
 
       expect(result).rejects.toThrow(
-        "currency_code is a required input in the pricing context"
+        "Method calculatePrices requires currency_code in the pricing context"
       )
 
       result = service.calculatePrices(
@@ -352,7 +352,7 @@ describe("PricingModule Service - Calculate Price", () => {
       )
 
       expect(result).rejects.toThrow(
-        "currency_code is a required input in the pricing context"
+        "Method calculatePrices requires currency_code in the pricing context"
       )
     })
 

packages/pricing/integration-tests/__tests__/services/rule-type/index.spec.ts

@@ -251,5 +251,24 @@ describe("RuleType Service", () => {
         })
       )
     })
+
+    it("should throw an error when using one of the reserved keywords", async () => {
+      let error
+
+      try {
+        await service.create([
+          {
+            name: "Test Rule",
+            rule_attribute: "currency_code",
+          },
+        ])
+      } catch (e) {
+        error = e
+      }
+
+      expect(error.message).toEqual(
+        "Can't create rule_attribute with reserved keywords [quantity, currency_code, price_list_id] - currency_code"
+      )
+    })
   })
 })

packages/pricing/src/loaders/container.ts

@@ -26,8 +26,12 @@ export default async ({
       defaultServices.PriceSetMoneyAmountRulesService
     ).singleton(),
     priceRuleService: asClass(defaultServices.PriceRuleService).singleton(),
-    priceSetRuleTypeService: asClass(defaultServices.PriceSetRuleTypeService).singleton(),
-    priceSetMoneyAmountService : asClass(defaultServices.PriceSetMoneyAmountService).singleton(),
+    priceSetRuleTypeService: asClass(
+      defaultServices.PriceSetRuleTypeService
+    ).singleton(),
+    priceSetMoneyAmountService: asClass(
+      defaultServices.PriceSetMoneyAmountService
+    ).singleton(),
   })
 
   if (customRepositories) {
@@ -44,6 +48,9 @@ export default async ({
 function loadDefaultRepositories({ container }) {
   container.register({
     baseRepository: asClass(defaultRepositories.BaseRepository).singleton(),
+    pricingRepository: asClass(
+      defaultRepositories.PricingRepository
+    ).singleton(),
     currencyRepository: asClass(
       defaultRepositories.CurrencyRepository
     ).singleton(),

packages/pricing/src/repositories/index.ts

@@ -6,4 +6,5 @@ export { PriceSetRepository } from "./price-set"
 export { PriceSetMoneyAmountRepository } from "./price-set-money-amount"
 export { PriceSetMoneyAmountRulesRepository } from "./price-set-money-amount-rules"
 export { PriceSetRuleTypeRepository } from "./price-set-rule-type"
+export { PricingRepository } from "./pricing"
 export { RuleTypeRepository } from "./rule-type"

packages/pricing/src/repositories/pricing.ts

@@ -0,0 +1,116 @@
+import {
+  CalculatedPriceSetDTO,
+  Context,
+  PricingContext,
+  PricingFilters,
+} from "@medusajs/types"
+import { MedusaError, MikroOrmBase } from "@medusajs/utils"
+import { SqlEntityManager } from "@mikro-orm/postgresql"
+import { PricingRepositoryService } from "../types"
+
+export class PricingRepository
+  extends MikroOrmBase
+  implements PricingRepositoryService
+{
+  protected readonly manager_: SqlEntityManager
+
+  constructor({ manager }: { manager: SqlEntityManager }) {
+    // @ts-ignore
+    // eslint-disable-next-line prefer-rest-params
+    super(...arguments)
+
+    this.manager_ = manager
+  }
+
+  async calculatePrices(
+    pricingFilters: PricingFilters,
+    pricingContext: PricingContext = { context: {} },
+    sharedContext: Context = {}
+  ): Promise<CalculatedPriceSetDTO[]> {
+    const manager = this.getActiveManager<SqlEntityManager>()
+    const knex = manager.getKnex()
+    const context = pricingContext.context || {}
+
+    // Quantity is used to scope money amounts based on min_quantity and max_quantity.
+    // We should potentially think of reserved words in pricingContext that can't be used in rules
+    // or have a separate pricing options that accept things like quantity, price_list_id and other
+    // pricing module features
+    const quantity = context.quantity
+    delete context.quantity
+
+    // Currency code here is a required param.
+    const currencyCode = context.currency_code
+    delete context.currency_code
+
+    if (!currencyCode) {
+      throw new MedusaError(
+        MedusaError.Types.INVALID_DATA,
+        `Method calculatePrices requires currency_code in the pricing context`
+      )
+    }
+
+    const isContextPresent = Object.entries(context).length || !!currencyCode
+
+    // Only if the context is present do we need to query the database.
+    // We don't get anything from the db otherwise.
+    if (!isContextPresent) {
+      return []
+    }
+
+    // Gets all the price set money amounts where rules match for each of the contexts
+    // that the price set is configured for
+    const psmaSubQueryKnex = knex({
+      psma: "price_set_money_amount",
+    })
+      .select({
+        id: "psma.id",
+        price_set_id: "psma.price_set_id",
+        money_amount_id: "psma.money_amount_id",
+        number_rules: "psma.number_rules",
+      })
+      .leftJoin("price_set_money_amount as psma1", "psma1.id", "psma.id")
+      .leftJoin("price_rule as pr", "pr.price_set_money_amount_id", "psma.id")
+      .leftJoin("rule_type as rt", "rt.id", "pr.rule_type_id")
+      .orderBy("number_rules", "desc")
+      .orWhere("psma1.number_rules", "=", 0)
+      .groupBy("psma.id")
+      .having(knex.raw("count(DISTINCT rt.rule_attribute) = psma.number_rules"))
+
+    for (const [key, value] of Object.entries(context)) {
+      psmaSubQueryKnex.orWhere({
+        "rt.rule_attribute": key,
+        "pr.value": value,
+      })
+    }
+
+    const priceSetQueryKnex = knex({
+      ps: "price_set",
+    })
+      .select({
+        id: "ps.id",
+        amount: "ma.amount",
+        min_quantity: "ma.min_quantity",
+        max_quantity: "ma.max_quantity",
+        currency_code: "ma.currency_code",
+        default_priority: "rt.default_priority",
+        number_rules: "psma.number_rules",
+      })
+      .join(psmaSubQueryKnex.as("psma"), "psma.price_set_id", "ps.id")
+      .join("money_amount as ma", "ma.id", "psma.money_amount_id")
+      .leftJoin("price_rule as pr", "pr.price_set_money_amount_id", "psma.id")
+      .leftJoin("rule_type as rt", "rt.id", "pr.rule_type_id")
+      .whereIn("ps.id", pricingFilters.id)
+      .andWhere("ma.currency_code", "=", currencyCode)
+      .orderBy([
+        { column: "number_rules", order: "desc" },
+        { column: "default_priority", order: "desc" },
+      ])
+
+    if (quantity) {
+      priceSetQueryKnex.where("ma.min_quantity", "<=", quantity)
+      priceSetQueryKnex.andWhere("ma.max_quantity", ">=", quantity)
+    }
+
+    return await priceSetQueryKnex
+  }
+}

packages/pricing/src/repositories/rule-type.ts

@@ -4,7 +4,7 @@ import {
   DAL,
   UpdateRuleTypeDTO,
 } from "@medusajs/types"
-import { DALUtils, MedusaError } from "@medusajs/utils"
+import { DALUtils, MedusaError, validateRuleAttributes } from "@medusajs/utils"
 import {
   LoadStrategy,
   FilterQuery as MikroFilterQuery,
@@ -72,6 +72,8 @@ export class RuleTypeRepository extends DALUtils.MikroOrmBaseRepository {
     data: CreateRuleTypeDTO[],
     context: Context = {}
   ): Promise<RuleType[]> {
+    validateRuleAttributes(data.map((d) => d.rule_attribute))
+
     const manager = this.getActiveManager<SqlEntityManager>(context)
 
     const ruleTypes = data.map((ruleTypeData) => {
@@ -87,6 +89,8 @@ export class RuleTypeRepository extends DALUtils.MikroOrmBaseRepository {
     data: UpdateRuleTypeDTO[],
     context: Context = {}
   ): Promise<RuleType[]> {
+    validateRuleAttributes(data.map((d) => d.rule_attribute))
+
     const manager = this.getActiveManager<SqlEntityManager>(context)
     const ruleTypeIds = data.map((ruleType) => ruleType.id)
     const existingRuleTypes = await this.find(

packages/pricing/src/services/pricing-module.ts

@@ -34,8 +34,6 @@ import {
   RuleTypeService,
 } from "@services"
 
-import { EntityManager } from "@mikro-orm/postgresql"
-
 import {
   InjectManager,
   InjectTransactionManager,
@@ -47,9 +45,11 @@ import {
 
 import { AddPricesDTO } from "@medusajs/types"
 import { joinerConfig } from "../joiner-config"
+import { PricingRepositoryService } from "../types"
 
 type InjectedDependencies = {
   baseRepository: DAL.RepositoryService
+  pricingRepository: PricingRepositoryService
   currencyService: CurrencyService<any>
   moneyAmountService: MoneyAmountService<any>
   priceSetService: PriceSetService<any>
@@ -72,6 +72,7 @@ export default class PricingModuleService<
 > implements PricingTypes.IPricingModuleService
 {
   protected baseRepository_: DAL.RepositoryService
+  protected readonly pricingRepository_: PricingRepositoryService
   protected readonly currencyService_: CurrencyService<TCurrency>
   protected readonly moneyAmountService_: MoneyAmountService<TMoneyAmount>
   protected readonly ruleTypeService_: RuleTypeService<TRuleType>
@@ -84,6 +85,7 @@ export default class PricingModuleService<
   constructor(
     {
       baseRepository,
+      pricingRepository,
       moneyAmountService,
       currencyService,
       ruleTypeService,
@@ -96,6 +98,7 @@ export default class PricingModuleService<
     protected readonly moduleDeclaration: InternalModuleDeclaration
   ) {
     this.baseRepository_ = baseRepository
+    this.pricingRepository_ = pricingRepository
     this.currencyService_ = currencyService
     this.moneyAmountService_ = moneyAmountService
     this.ruleTypeService_ = ruleTypeService
@@ -117,89 +120,12 @@ export default class PricingModuleService<
     pricingContext: PricingContext = { context: {} },
     @MedusaContext() sharedContext: Context = {}
   ): Promise<PricingTypes.CalculatedPriceSetDTO> {
-    const manager = sharedContext.manager as EntityManager
-    const knex = manager.getKnex()
-
-    // Keeping this whole logic raw in here for now as they will undergo
-    // some changes, will abstract them out once we have a final version
-    const context = pricingContext.context || {}
-
-    // Quantity is used to scope money amounts based on min_quantity and max_quantity.
-    // We should potentially think of reserved words in pricingContext that can't be used in rules
-    // or have a separate pricing options that accept things like quantity, price_list_id and other
-    // pricing module features
-    const quantity = context.quantity
-    delete context.quantity
-
-    // Currency code here is a required param.
-    const currencyCode = context.currency_code
-    delete context.currency_code
-
-    if (!currencyCode) {
-      throw new MedusaError(
-        MedusaError.Types.INVALID_DATA,
-        `currency_code is a required input in the pricing context`
-      )
-    }
-
-    // Gets all the price set money amounts where rules match for each of the contexts
-    // that the price set is configured for
-    const psmaSubQueryKnex = knex({
-      psma: "price_set_money_amount",
-    })
-      .select({
-        id: "psma.id",
-        price_set_id: "psma.price_set_id",
-        money_amount_id: "psma.money_amount_id",
-        number_rules: "psma.number_rules",
-      })
-      .leftJoin("price_set_money_amount as psma1", "psma1.id", "psma.id")
-      .leftJoin("price_rule as pr", "pr.price_set_money_amount_id", "psma.id")
-      .leftJoin("rule_type as rt", "rt.id", "pr.rule_type_id")
-      .orderBy("number_rules", "desc")
-      .orWhere("psma1.number_rules", "=", 0)
-      .groupBy("psma.id")
-      .having(knex.raw("count(DISTINCT rt.rule_attribute) = psma.number_rules"))
-
-    for (const [key, value] of Object.entries(context)) {
-      psmaSubQueryKnex.orWhere({
-        "rt.rule_attribute": key,
-        "pr.value": value,
-      })
-    }
-
-    const priceSetQueryKnex = knex({
-      ps: "price_set",
-    })
-      .select({
-        id: "ps.id",
-        amount: "ma.amount",
-        min_quantity: "ma.min_quantity",
-        max_quantity: "ma.max_quantity",
-        currency_code: "ma.currency_code",
-        default_priority: "rt.default_priority",
-        number_rules: "psma.number_rules",
-      })
-      .join(psmaSubQueryKnex.as("psma"), "psma.price_set_id", "ps.id")
-      .join("money_amount as ma", "ma.id", "psma.money_amount_id")
-      .leftJoin("price_rule as pr", "pr.price_set_money_amount_id", "psma.id")
-      .leftJoin("rule_type as rt", "rt.id", "pr.rule_type_id")
-      .whereIn("ps.id", pricingFilters.id)
-      .andWhere("ma.currency_code", "=", currencyCode)
-      .orderBy([
-        { column: "number_rules", order: "desc" },
-        { column: "default_priority", order: "desc" },
-      ])
-
-    if (quantity) {
-      priceSetQueryKnex.where("ma.min_quantity", "<=", quantity)
-      priceSetQueryKnex.andWhere("ma.max_quantity", ">=", quantity)
-    }
-
-    const isContextPresent = Object.entries(context).length || !!currencyCode
-    // Only if the context is present do we need to query the database.
-    const queryBuilderResults = isContextPresent ? await priceSetQueryKnex : []
-    const pricesSetPricesMap = groupBy(queryBuilderResults, "id")
+    const results = await this.pricingRepository_.calculatePrices(
+      pricingFilters,
+      pricingContext,
+      sharedContext
+    )
+    const pricesSetPricesMap = groupBy(results, "id")
 
     const calculatedPrices = pricingFilters.id.map(
       (priceSetId: string): PricingTypes.CalculatedPriceSetDTO => {

packages/pricing/src/types/index.ts

@@ -3,3 +3,5 @@ import { Logger } from "@medusajs/types"
 export type InitializeModuleInjectableDependencies = {
   logger?: Logger
 }
+
+export * from "./repositories"

packages/pricing/src/types/repositories/index.ts

@@ -0,0 +1 @@
+export * from "./pricing"

packages/pricing/src/types/repositories/pricing.ts

@@ -0,0 +1,14 @@
+import {
+  CalculatedPriceSetDTO,
+  Context,
+  PricingContext,
+  PricingFilters,
+} from "@medusajs/types"
+
+export interface PricingRepositoryService {
+  calculatePrices(
+    pricingFilters: PricingFilters,
+    pricingContext: PricingContext,
+    context: Context
+  ): Promise<CalculatedPriceSetDTO[]>
+}

packages/utils/src/dal/mikro-orm/mikro-orm-repository.ts

@@ -6,14 +6,14 @@ import {
 } from "@medusajs/types"
 import { isString } from "../../common"
 import { MedusaContext } from "../../decorators"
-import { buildQuery, InjectTransactionManager } from "../../modules-sdk"
+import { InjectTransactionManager, buildQuery } from "../../modules-sdk"
 import {
   getSoftDeletedCascadedEntitiesIdsMappedBy,
   transactionWrapper,
 } from "../utils"
 import { mikroOrmSerializer, mikroOrmUpdateDeletedAtRecursively } from "./utils"
 
-class MikroOrmBase<T = any> {
+export class MikroOrmBase<T = any> {
   protected readonly manager_: any
 
   protected constructor({ manager }) {

packages/utils/src/index.ts

@@ -5,6 +5,7 @@ export * from "./decorators"
 export * from "./event-bus"
 export * from "./feature-flags"
 export * from "./modules-sdk"
+export * from "./pricing"
 export * from "./product"
 export * from "./search"
 export * from "./shipping"

packages/utils/src/pricing/__tests__/get-invalid-rule-attributes.spec.ts

@@ -0,0 +1,17 @@
+import { getInvalidRuleAttributes } from ".."
+
+describe("getInvalidRuleAttributes", function () {
+  it("should return list of rule attributes that matches reserved keywords", function () {
+    let result = getInvalidRuleAttributes(["shouldnotmatch"])
+    expect(result).toEqual([])
+
+    result = getInvalidRuleAttributes(["currency_code", "shouldnotmatch"])
+    expect(result).toEqual(["currency_code"])
+
+    result = getInvalidRuleAttributes(["currency_code", "price_list_id"])
+    expect(result).toEqual(["currency_code", "price_list_id"])
+
+    result = getInvalidRuleAttributes(["shouldnotmatch", "quantity"])
+    expect(result).toEqual(["quantity"])
+  })
+})

packages/utils/src/pricing/__tests__/validate-rule-attributes.spec.ts

@@ -0,0 +1,28 @@
+import { validateRuleAttributes } from ".."
+
+describe("validateRuleAttributes", function () {
+  it("should return void if there are no validation errors", function () {
+    let result = validateRuleAttributes(["shouldpasswithouterrors"])
+
+    expect(result).toEqual(undefined)
+  })
+
+  it("should throw an error if one of the array strings matches a reserved keyword", function () {
+    let error
+
+    try {
+      validateRuleAttributes([
+        "currency_code",
+        "shouldnotbepresent",
+        "quantity",
+        "price_list_id",
+      ])
+    } catch (e) {
+      error = e
+    }
+
+    expect(error.message).toEqual(
+      "Can't create rule_attribute with reserved keywords [quantity, currency_code, price_list_id] - quantity, currency_code, price_list_id"
+    )
+  })
+})

packages/utils/src/pricing/index.ts

@@ -0,0 +1,38 @@
+import { MedusaError } from "../common"
+
+export const ReservedPricingRuleAttributes = [
+  "quantity",
+  "currency_code",
+  "price_list_id",
+]
+
+type RuleAttributeInput = string | undefined
+
+export const getInvalidRuleAttributes = (
+  ruleAttributes: RuleAttributeInput[]
+): string[] => {
+  const invalidRuleAttributes: string[] = []
+
+  for (const attribute of ReservedPricingRuleAttributes) {
+    if (ruleAttributes.indexOf(attribute) > -1) {
+      invalidRuleAttributes.push(attribute)
+    }
+  }
+
+  return invalidRuleAttributes
+}
+
+export const validateRuleAttributes = (
+  ruleAttributes: RuleAttributeInput[]
+): void => {
+  const invalidRuleAttributes = getInvalidRuleAttributes(ruleAttributes)
+
+  if (invalidRuleAttributes.length) {
+    throw new MedusaError(
+      MedusaError.Types.INVALID_DATA,
+      `Can't create rule_attribute with reserved keywords [${ReservedPricingRuleAttributes.join(
+        ", "
+      )}] - ${invalidRuleAttributes.join(", ")}`
+    )
+  }
+}