From 324b4ab438662f44de495ffe4d9137677a032a00 Mon Sep 17 00:00:00 2001 From: Riqwan Thamir Date: Thu, 28 Nov 2024 21:48:00 +0100 Subject: [PATCH] feat(pricing, types): add price rule operators to price calculations (#10350) what: - adds price rule operators when doing price calculations - rules now accepts a key where the value can be an array of objects `({ operator: string, value: number })` - validation for available types of operator and value to be a number ``` await service.createPriceSets({ prices: [ { amount: 50, currency_code: "usd", rules: { region_id: "de", cart_total: [ { operator: "gte", value: 400 }, { operator: "lte", value: 500 }, ] }, }, ] }) ``` - price calculations will now account for the operators - lte, gte, lt, gt when the price context is a number RESOLVES CMRC-747 --- .changeset/kind-moons-attack.md | 6 + .../types/src/pricing/common/price-set.ts | 9 +- .../__fixtures__/price-rule/index.ts | 1 + .../__fixtures__/price-rule/operators.ts | 39 ++++ .../pricing-module/calculate-price.spec.ts | 175 ++++++++++++++++++ .../services/pricing-module/price-set.spec.ts | 131 ++++++++++++- .../pricing/src/repositories/pricing.ts | 88 ++++++--- .../pricing/src/services/pricing-module.ts | 46 ++++- 8 files changed, 462 insertions(+), 33 deletions(-) create mode 100644 .changeset/kind-moons-attack.md create mode 100644 packages/modules/pricing/integration-tests/__fixtures__/price-rule/operators.ts diff --git a/.changeset/kind-moons-attack.md b/.changeset/kind-moons-attack.md new file mode 100644 index 0000000000000..a3817d77b8eb9 --- /dev/null +++ b/.changeset/kind-moons-attack.md @@ -0,0 +1,6 @@ +--- +"@medusajs/pricing": patch +"@medusajs/types": patch +--- + +feat(pricing, types): add price rule operators to price calculations diff --git a/packages/core/types/src/pricing/common/price-set.ts b/packages/core/types/src/pricing/common/price-set.ts index 76ca0f8012b35..ae393c23a669e 100644 --- a/packages/core/types/src/pricing/common/price-set.ts +++ b/packages/core/types/src/pricing/common/price-set.ts @@ -7,6 +7,7 @@ import { MoneyAmountDTO, UpdateMoneyAmountDTO, } from "./money-amount" +import { PricingRuleOperatorValues } from "./price-rule" export interface PricingRepositoryService { calculatePrices( @@ -206,6 +207,11 @@ export interface CalculatedPriceSet { } } +export interface RuleWithOperator { + operator: PricingRuleOperatorValues + value: number +} + /** * @interface * @@ -214,7 +220,8 @@ export interface CalculatedPriceSet { * Each key of the object is a the attribute, and its value * is the values of the rule. */ -export interface CreatePriceSetPriceRules extends Record {} +export interface CreatePriceSetPriceRules + extends Record {} /** * @interface diff --git a/packages/modules/pricing/integration-tests/__fixtures__/price-rule/index.ts b/packages/modules/pricing/integration-tests/__fixtures__/price-rule/index.ts index c4578f16ee6ee..2aa5927088479 100644 --- a/packages/modules/pricing/integration-tests/__fixtures__/price-rule/index.ts +++ b/packages/modules/pricing/integration-tests/__fixtures__/price-rule/index.ts @@ -5,6 +5,7 @@ import { SqlEntityManager } from "@mikro-orm/postgresql" import { defaultPriceRuleData } from "./data" export * from "./data" +export * from "./operators" export async function createPriceRules( manager: SqlEntityManager, diff --git a/packages/modules/pricing/integration-tests/__fixtures__/price-rule/operators.ts b/packages/modules/pricing/integration-tests/__fixtures__/price-rule/operators.ts new file mode 100644 index 0000000000000..5f1e457639f02 --- /dev/null +++ b/packages/modules/pricing/integration-tests/__fixtures__/price-rule/operators.ts @@ -0,0 +1,39 @@ +import { RuleWithOperator } from "@medusajs/types" + +export const withOperator = ( + border, + min = 400, + max = 800 +): RuleWithOperator[] => { + if (border === "betweenEquals") { + return [ + { operator: "gte", value: min }, + { operator: "lte", value: max }, + ] + } else if (border === "between") { + return [ + { operator: "gt", value: min }, + { operator: "lt", value: max }, + ] + } else if (border === "excludingMin") { + return [ + { operator: "gt", value: min }, + { operator: "lte", value: max }, + ] + } else if (border === "excludingMax") { + return [ + { operator: "gte", value: min }, + { operator: "lt", value: max }, + ] + } else if (border === "gt") { + return [{ operator: "gt", value: min }] + } else if (border === "lt") { + return [{ operator: "lt", value: min }] + } else if (border === "lte") { + return [{ operator: "lte", value: min }] + } else if (border === "gte") { + return [{ operator: "gte", value: min }] + } else { + return [] + } +} diff --git a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts index 12958606f72b6..e1f4ab4e5dc7e 100644 --- a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts +++ b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/calculate-price.spec.ts @@ -10,6 +10,7 @@ import { PriceListType, } from "@medusajs/framework/utils" import { moduleIntegrationTestRunner } from "@medusajs/test-utils" +import { withOperator } from "../../../__fixtures__/price-rule" import { seedPriceData } from "../../../__fixtures__/seed-price-data" jest.setTimeout(30000) @@ -1857,6 +1858,180 @@ moduleIntegrationTestRunner({ }) }) }) + + describe("calculatePrices", () => { + let priceSet1 + + it("should return accurate prices when using custom price rule operators", async () => { + priceSet1 = await service.createPriceSets({ + prices: [ + { + amount: 50, + currency_code: "usd", + rules: { + region_id: "de", + cart_total: withOperator("between", 300, 400), + }, + }, + { + amount: 100, + currency_code: "usd", + rules: { + region_id: "de", + cart_total: withOperator("betweenEquals", 400, 500), + }, + }, + { + amount: 150, + currency_code: "usd", + rules: { + region_id: "de", + cart_total: withOperator("excludingMin", 500, 600), + }, + }, + { + amount: 200, + currency_code: "usd", + rules: { + region_id: "de", + cart_total: withOperator("excludingMax", 600, 700), + }, + }, + ], + }) + + let priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 350, + }, + } + ) + + expect(priceSetsResult).toEqual([ + expect.objectContaining({ + is_calculated_price_price_list: false, + is_calculated_price_tax_inclusive: false, + calculated_amount: 50, + raw_calculated_amount: { + value: "50", + precision: 20, + }, + is_original_price_price_list: false, + is_original_price_tax_inclusive: false, + original_amount: 50, + raw_original_amount: { + value: "50", + precision: 20, + }, + currency_code: "usd", + calculated_price: expect.objectContaining({ + id: expect.any(String), + price_list_id: null, + price_list_type: null, + min_quantity: null, + max_quantity: null, + }), + original_price: { + id: expect.any(String), + price_list_id: null, + price_list_type: null, + min_quantity: null, + max_quantity: null, + }, + }), + ]) + + priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 300, + }, + } + ) + + expect(priceSetsResult).toEqual([]) + + priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 400, + }, + } + ) + + expect(priceSetsResult).toEqual([ + expect.objectContaining({ calculated_amount: 100 }), + ]) + + priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 500, + }, + } + ) + + expect(priceSetsResult).toEqual([ + expect.objectContaining({ calculated_amount: 100 }), + ]) + + priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 501, + }, + } + ) + + expect(priceSetsResult).toEqual([ + expect.objectContaining({ calculated_amount: 150 }), + ]) + + priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 601, + }, + } + ) + + expect(priceSetsResult).toEqual([ + expect.objectContaining({ calculated_amount: 200 }), + ]) + + priceSetsResult = await service.calculatePrices( + { id: [priceSet1.id] }, + { + context: { + currency_code: "usd", + region_id: "de", + cart_total: 900, + }, + } + ) + + expect(priceSetsResult).toEqual([]) + }) + }) }) }, }) diff --git a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts index 9a2b193d2d627..a983f7d1797e3 100644 --- a/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts +++ b/packages/modules/pricing/integration-tests/__tests__/services/pricing-module/price-set.spec.ts @@ -619,6 +619,119 @@ moduleIntegrationTestRunner({ ) }) + it("should create price set with prices including rule operators", async () => { + const [priceSet] = await service.createPriceSets([ + { + prices: [ + { + amount: 100, + currency_code: "USD", + rules: { + region_id: "1", + custom_rule: [ + { + operator: "gt", + value: 500, + }, + { + operator: "lt", + value: 1000, + }, + ], + }, + }, + { + amount: 150, + currency_code: "USD", + }, + ], + }, + ]) + + expect(priceSet.prices).toHaveLength(2) + expect(priceSet).toEqual( + expect.objectContaining({ + prices: expect.arrayContaining([ + expect.objectContaining({ + amount: 100, + currency_code: "USD", + price_rules: expect.arrayContaining([ + expect.objectContaining({ + attribute: "region_id", + operator: "eq", + value: "1", + }), + expect.objectContaining({ + attribute: "custom_rule", + operator: "gt", + value: "500", + }), + expect.objectContaining({ + attribute: "custom_rule", + operator: "lt", + value: "1000", + }), + ]), + }), + expect.objectContaining({ + amount: 150, + currency_code: "USD", + }), + ]), + }) + ) + }) + + it("should throw error when creating price with invalid rules", async () => { + let error = await service + .createPriceSets([ + { + prices: [ + { + amount: 100, + currency_code: "USD", + rules: { + custom_rule: [ + { + operator: "unknown" as any, + value: 500, + }, + ], + }, + }, + ], + }, + ]) + .catch((e) => e) + + expect(error.message).toBe( + "operator should be one of gte, lte, gt, lt, eq" + ) + + error = await service + .createPriceSets([ + { + prices: [ + { + amount: 100, + currency_code: "USD", + rules: { + custom_rule: [ + { + operator: "gt", + value: "string" as any, + }, + ], + }, + }, + ], + }, + ]) + .catch((e) => e) + + expect(error.message).toBe("value should be a number") + }) + it("should create a priceSet successfully", async () => { await service.createPriceSets([ { @@ -786,7 +899,9 @@ moduleIntegrationTestRunner({ { amount: 100, currency_code: "USD", - rules: { region_id: "123" }, + rules: { + region_id: "123", + }, }, ], }, @@ -799,7 +914,10 @@ moduleIntegrationTestRunner({ { amount: 200, currency_code: "USD", - rules: { region_id: "123" }, + rules: { + region_id: "123", + test: [{ value: 500, operator: "gte" }], + }, }, ], }, @@ -812,6 +930,10 @@ moduleIntegrationTestRunner({ expect( priceSet.prices?.sort((a: any, b: any) => a.amount - b.amount) ).toEqual([ + expect.objectContaining({ + amount: 100, + currency_code: "USD", + }), expect.objectContaining({ amount: 200, currency_code: "USD", @@ -820,6 +942,11 @@ moduleIntegrationTestRunner({ attribute: "region_id", value: "123", }), + expect.objectContaining({ + attribute: "test", + operator: "gte", + value: "500", + }), ], }), expect.objectContaining({ diff --git a/packages/modules/pricing/src/repositories/pricing.ts b/packages/modules/pricing/src/repositories/pricing.ts index ef2636c542ad1..1b6f47a4b8653 100644 --- a/packages/modules/pricing/src/repositories/pricing.ts +++ b/packages/modules/pricing/src/repositories/pricing.ts @@ -11,7 +11,7 @@ import { PricingFilters, PricingRepositoryService, } from "@medusajs/framework/types" -import { SqlEntityManager } from "@mikro-orm/postgresql" +import { Knex, SqlEntityManager } from "@mikro-orm/postgresql" export class PricingRepository extends MikroOrmBase @@ -58,7 +58,7 @@ export class PricingRepository return [] } - // Gets all the price set money amounts where rules match for each of the contexts + // Gets all the prices where rules match for each of the contexts // that the price set is configured for const priceSubQueryKnex = knex({ price: "price", @@ -90,7 +90,7 @@ export class PricingRepository .groupBy("price.id", "pl.id") .having( knex.raw( - "count(DISTINCT pr.attribute) = price.rules_count AND price.price_list_id IS NULL" + "count(pr.attribute) = price.rules_count AND price.price_list_id IS NULL" ) ) .orHaving( @@ -99,17 +99,57 @@ export class PricingRepository ) ) - priceSubQueryKnex.orWhere((q) => { - const nullPLq = q.whereNull("price.price_list_id") - nullPLq.andWhere((q) => { - for (const [key, value] of Object.entries(context)) { - q.orWhere({ - "pr.attribute": key, - "pr.value": value, - }) - } - q.orWhere("price.rules_count", "=", 0) - }) + const buildOperatorQueries = ( + operatorGroupBuilder: Knex.QueryBuilder, + value + ) => { + operatorGroupBuilder + .where((operatorBuilder) => { + operatorBuilder + .where("pr.operator", "gte") + .whereRaw("? >= pr.value::numeric", [value]) + }) + .orWhere((operatorBuilder) => { + operatorBuilder + .where("pr.operator", "gt") + .whereRaw("? > pr.value::numeric", [value]) + }) + .orWhere((operatorBuilder) => { + operatorBuilder + .where("pr.operator", "lt") + .whereRaw("? < pr.value::numeric", [value]) + }) + .orWhere((operatorBuilder) => { + operatorBuilder + .where("pr.operator", "lte") + .whereRaw("? <= pr.value::numeric", [value]) + }) + .orWhere((operatorBuilder) => { + operatorBuilder + .where("pr.operator", "eq") + .whereRaw("? = pr.value::numeric", [value]) + }) + } + + priceSubQueryKnex.orWhere((priceBuilder) => { + priceBuilder + .whereNull("price.price_list_id") + .andWhere((withoutPriceListBuilder) => { + for (const [key, value] of Object.entries(context)) { + withoutPriceListBuilder.orWhere((orBuilder) => { + orBuilder.where("pr.attribute", key) + + if (typeof value === "number") { + orBuilder.where((operatorGroupBuilder) => { + buildOperatorQueries(operatorGroupBuilder, value) + }) + } else { + orBuilder.where({ "pr.value": value }) + } + }) + } + withoutPriceListBuilder.orWhere("price.rules_count", "=", 0) + }) }) priceSubQueryKnex.orWhere((q) => { @@ -132,9 +172,7 @@ export class PricingRepository .andWhere(function () { this.andWhere(function () { for (const [key, value] of Object.entries(context)) { - this.orWhere({ - "plr.attribute": key, - }) + this.orWhere({ "plr.attribute": key }) this.where( "plr.value", "@>", @@ -146,14 +184,20 @@ export class PricingRepository }) this.andWhere(function () { - this.andWhere(function () { + this.andWhere((contextBuilder) => { for (const [key, value] of Object.entries(context)) { - this.orWhere({ - "pr.attribute": key, - "pr.value": value, + contextBuilder.orWhere((orBuilder) => { + orBuilder.where("pr.attribute", key) + + if (typeof value === "number") { + buildOperatorQueries(orBuilder, value) + } else { + orBuilder.where({ "pr.value": value }) + } }) } - this.andWhere("price.rules_count", ">", 0) + + contextBuilder.andWhere("price.rules_count", ">", 0) }) this.orWhere("price.rules_count", "=", 0) }) diff --git a/packages/modules/pricing/src/services/pricing-module.ts b/packages/modules/pricing/src/services/pricing-module.ts index bd29e038969ab..e7fa9824014bc 100644 --- a/packages/modules/pricing/src/services/pricing-module.ts +++ b/packages/modules/pricing/src/services/pricing-module.ts @@ -15,6 +15,7 @@ import { PricingContext, PricingFilters, PricingRepositoryService, + PricingRuleOperatorValues, PricingTypes, UpsertPricePreferenceDTO, UpsertPriceSetDTO, @@ -33,6 +34,7 @@ import { MedusaError, ModulesSdkUtils, PriceListType, + PricingRuleOperator, promiseAll, removeNullish, simpleHash, @@ -596,20 +598,48 @@ export default class PricingModuleService data?.forEach((price) => { const cleanRules = price.rules ? removeNullish(price.rules) : {} - const ruleEntries = Object.entries(cleanRules) - const rules = ruleEntries.map(([attribute, value]) => { - return { - attribute, - value, - } - }) + const ruleOperators: PricingRuleOperatorValues[] = + Object.values(PricingRuleOperator) + + const rules = Object.entries(cleanRules) + .map(([attribute, value]) => { + if (Array.isArray(value)) { + return value.map((customRule) => { + if (!ruleOperators.includes(customRule.operator)) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `operator should be one of ${ruleOperators.join(", ")}` + ) + } + + if (typeof customRule.value !== "number") { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `value should be a number` + ) + } + + return { + attribute, + operator: customRule.operator, + value: customRule.value, + } + }) + } + + return { + attribute, + value, + } + }) + .flat(1) const hasRulesInput = isPresent(price.rules) const entry = { ...price, price_list_id: priceListId, price_rules: hasRulesInput ? rules : undefined, - rules_count: hasRulesInput ? ruleEntries.length : undefined, + rules_count: hasRulesInput ? rules.length : undefined, } as ServiceTypes.UpsertPriceDTO delete (entry as CreatePricesDTO).rules