diff --git a/.changeset/breezy-elephants-talk.md b/.changeset/breezy-elephants-talk.md new file mode 100644 index 0000000000..6453cb746a --- /dev/null +++ b/.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 diff --git a/packages/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts b/packages/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts index 3efba8ad60..0dc4414cf9 100644 --- a/packages/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts +++ b/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" ) }) diff --git a/packages/pricing/integration-tests/__tests__/services/rule-type/index.spec.ts b/packages/pricing/integration-tests/__tests__/services/rule-type/index.spec.ts index 2d1e3be1c2..5a39254ef0 100644 --- a/packages/pricing/integration-tests/__tests__/services/rule-type/index.spec.ts +++ b/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" + ) + }) }) }) diff --git a/packages/pricing/src/loaders/container.ts b/packages/pricing/src/loaders/container.ts index 8d7ba0dc72..c8b304eec5 100644 --- a/packages/pricing/src/loaders/container.ts +++ b/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(), diff --git a/packages/pricing/src/repositories/index.ts b/packages/pricing/src/repositories/index.ts index 7e34dce023..d0ca202ac1 100644 --- a/packages/pricing/src/repositories/index.ts +++ b/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" diff --git a/packages/pricing/src/repositories/pricing.ts b/packages/pricing/src/repositories/pricing.ts new file mode 100644 index 0000000000..f2766b7efa --- /dev/null +++ b/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 { + const manager = this.getActiveManager() + 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 + } +} diff --git a/packages/pricing/src/repositories/rule-type.ts b/packages/pricing/src/repositories/rule-type.ts index 7cab1bf8ff..e7714582a7 100644 --- a/packages/pricing/src/repositories/rule-type.ts +++ b/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 { + validateRuleAttributes(data.map((d) => d.rule_attribute)) + const manager = this.getActiveManager(context) const ruleTypes = data.map((ruleTypeData) => { @@ -87,6 +89,8 @@ export class RuleTypeRepository extends DALUtils.MikroOrmBaseRepository { data: UpdateRuleTypeDTO[], context: Context = {} ): Promise { + validateRuleAttributes(data.map((d) => d.rule_attribute)) + const manager = this.getActiveManager(context) const ruleTypeIds = data.map((ruleType) => ruleType.id) const existingRuleTypes = await this.find( diff --git a/packages/pricing/src/services/pricing-module.ts b/packages/pricing/src/services/pricing-module.ts index daa782b92b..4528a67fa2 100644 --- a/packages/pricing/src/services/pricing-module.ts +++ b/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 moneyAmountService: MoneyAmountService priceSetService: PriceSetService @@ -72,6 +72,7 @@ export default class PricingModuleService< > implements PricingTypes.IPricingModuleService { protected baseRepository_: DAL.RepositoryService + protected readonly pricingRepository_: PricingRepositoryService protected readonly currencyService_: CurrencyService protected readonly moneyAmountService_: MoneyAmountService protected readonly ruleTypeService_: RuleTypeService @@ -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 { - 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 => { diff --git a/packages/pricing/src/types/index.ts b/packages/pricing/src/types/index.ts index 0f252977b0..99a6ccd6e7 100644 --- a/packages/pricing/src/types/index.ts +++ b/packages/pricing/src/types/index.ts @@ -3,3 +3,5 @@ import { Logger } from "@medusajs/types" export type InitializeModuleInjectableDependencies = { logger?: Logger } + +export * from "./repositories" diff --git a/packages/pricing/src/types/repositories/index.ts b/packages/pricing/src/types/repositories/index.ts new file mode 100644 index 0000000000..d4f24d5a82 --- /dev/null +++ b/packages/pricing/src/types/repositories/index.ts @@ -0,0 +1 @@ +export * from "./pricing" diff --git a/packages/pricing/src/types/repositories/pricing.ts b/packages/pricing/src/types/repositories/pricing.ts new file mode 100644 index 0000000000..bd8f6e5be1 --- /dev/null +++ b/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 +} diff --git a/packages/utils/src/dal/mikro-orm/mikro-orm-repository.ts b/packages/utils/src/dal/mikro-orm/mikro-orm-repository.ts index 979b717d84..11596a9825 100644 --- a/packages/utils/src/dal/mikro-orm/mikro-orm-repository.ts +++ b/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 { +export class MikroOrmBase { protected readonly manager_: any protected constructor({ manager }) { diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 50bceb4955..e0e0870d5d 100644 --- a/packages/utils/src/index.ts +++ b/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" diff --git a/packages/utils/src/pricing/__tests__/get-invalid-rule-attributes.spec.ts b/packages/utils/src/pricing/__tests__/get-invalid-rule-attributes.spec.ts new file mode 100644 index 0000000000..2febb0d55d --- /dev/null +++ b/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"]) + }) +}) diff --git a/packages/utils/src/pricing/__tests__/validate-rule-attributes.spec.ts b/packages/utils/src/pricing/__tests__/validate-rule-attributes.spec.ts new file mode 100644 index 0000000000..994886023b --- /dev/null +++ b/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" + ) + }) +}) diff --git a/packages/utils/src/pricing/index.ts b/packages/utils/src/pricing/index.ts new file mode 100644 index 0000000000..28ab69825a --- /dev/null +++ b/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(", ")}` + ) + } +}