diff --git a/.changeset/lazy-lemons-occur.md b/.changeset/lazy-lemons-occur.md new file mode 100644 index 0000000000..69293fd2c5 --- /dev/null +++ b/.changeset/lazy-lemons-occur.md @@ -0,0 +1,5 @@ +--- +"@medusajs/promotion": patch +--- + +Chore(): Further promotions pre filtering improvements diff --git a/integration-tests/http/__tests__/cart/store/cart.spec.ts b/integration-tests/http/__tests__/cart/store/cart.spec.ts index bc883b10c1..3305d18a20 100644 --- a/integration-tests/http/__tests__/cart/store/cart.spec.ts +++ b/integration-tests/http/__tests__/cart/store/cart.spec.ts @@ -140,7 +140,7 @@ medusaIntegrationTestRunner({ currency_code: "usd", target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: "in", values: [product.id], }, @@ -2571,7 +2571,7 @@ medusaIntegrationTestRunner({ apply_to_quantity: 1, target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: PromotionRuleOperator.IN, values: [product.id], }, @@ -4353,7 +4353,7 @@ medusaIntegrationTestRunner({ currency_code: "eur", target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: "in", values: [product.id], }, diff --git a/integration-tests/modules/__tests__/cart/store/add-promotions-to-cart.spec.ts b/integration-tests/modules/__tests__/cart/store/add-promotions-to-cart.spec.ts index ac3181dc68..994a97d515 100644 --- a/integration-tests/modules/__tests__/cart/store/add-promotions-to-cart.spec.ts +++ b/integration-tests/modules/__tests__/cart/store/add-promotions-to-cart.spec.ts @@ -82,7 +82,7 @@ medusaIntegrationTestRunner({ currency_code: "usd", target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: "eq", values: "prod_mat", }, diff --git a/integration-tests/modules/__tests__/cart/store/carts.spec.ts b/integration-tests/modules/__tests__/cart/store/carts.spec.ts index ef0900de0b..98434e0685 100644 --- a/integration-tests/modules/__tests__/cart/store/carts.spec.ts +++ b/integration-tests/modules/__tests__/cart/store/carts.spec.ts @@ -1418,7 +1418,7 @@ medusaIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: "eq", values: [product.id], }, diff --git a/integration-tests/modules/__tests__/cart/store/remove-promotions-from-cart.spec.ts b/integration-tests/modules/__tests__/cart/store/remove-promotions-from-cart.spec.ts index 06e597adf4..2bfea51f11 100644 --- a/integration-tests/modules/__tests__/cart/store/remove-promotions-from-cart.spec.ts +++ b/integration-tests/modules/__tests__/cart/store/remove-promotions-from-cart.spec.ts @@ -60,7 +60,7 @@ medusaIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: "eq", values: "prod_tshirt", }, @@ -83,7 +83,7 @@ medusaIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_id", + attribute: "items.product_id", operator: "eq", values: "prod_tshirt", }, diff --git a/integration-tests/modules/__tests__/order/draft-order.spec.ts b/integration-tests/modules/__tests__/order/draft-order.spec.ts index a4a0c7d081..553b312dc6 100644 --- a/integration-tests/modules/__tests__/order/draft-order.spec.ts +++ b/integration-tests/modules/__tests__/order/draft-order.spec.ts @@ -469,7 +469,7 @@ medusaIntegrationTestRunner({ max_quantity: 100, target_rules: [ { - attribute: "variant_id", + attribute: "items.variant_id", operator: "in", values: [product.variants[0].id, product_2.variants[0].id], }, @@ -877,7 +877,7 @@ medusaIntegrationTestRunner({ max_quantity: 100, target_rules: [ { - attribute: "variant_id", + attribute: "items.variant_id", operator: "in", values: [product.variants[0].id], }, diff --git a/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/compute-actions.spec.ts b/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/compute-actions.spec.ts index f826b70396..6ebac6856a 100644 --- a/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/compute-actions.spec.ts +++ b/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/compute-actions.spec.ts @@ -25,6 +25,113 @@ moduleIntegrationTestRunner({ await createCampaigns(MikroOrmWrapper.forkManager()) }) + it("edge case: should only return promotions with no rules when context has no attributes", async () => { + // Create promotions with different rule configurations + const promotionsToCreate: CreatePromotionDTO[] = [ + // Promotion with no rules - should be returned + { + code: "NO_RULES_PROMO", + is_automatic: true, + rules: [], // No global rules + application_method: { + type: "fixed", + target_type: "items", + allocation: "each", + max_quantity: 100000, + value: 100, + target_rules: [], // No target rules + }, + type: "standard", + status: PromotionStatus.ACTIVE, + campaign_id: "campaign-id-1", + }, + // Promotion with global rules - should NOT be returned + { + code: "WITH_GLOBAL_RULES", + is_automatic: true, + rules: [ + { + attribute: "customer.id", + operator: "eq", + values: ["some_customer"], + }, + ], + application_method: { + type: "fixed", + target_type: "items", + allocation: "each", + max_quantity: 100000, + value: 100, + target_rules: [], + }, + type: "standard", + status: PromotionStatus.ACTIVE, + campaign_id: "campaign-id-1", + }, + // Promotion with target rules - should NOT be returned + { + code: "WITH_TARGET_RULES", + is_automatic: true, + rules: [], + application_method: { + type: "fixed", + target_type: "items", + allocation: "each", + max_quantity: 100000, + value: 100, + target_rules: [ + { + attribute: "items.product.id", + operator: "eq", + values: ["some_product"], + }, + ], + }, + type: "standard", + status: PromotionStatus.ACTIVE, + campaign_id: "campaign-id-1", + }, + ] + + const promotions = await service.createPromotions(promotionsToCreate) + const noRulePromotion = promotions.find( + (p) => p.code === "NO_RULES_PROMO" + )! + + // Spy on the internal promotion service to verify prefiltering + let prefilterCallCount = 0 + let prefilteredPromotions: any[] = [] + const originalPromotionServiceList = (service as any).promotionService_ + .list + + ;(service as any).promotionService_.list = async (...args: any[]) => { + const result = await originalPromotionServiceList.bind( + (service as any).promotionService_ + )(...args) + + if (prefilterCallCount === 0) { + prefilteredPromotions = result + } + prefilterCallCount++ + + // Return nothing to not trigger future context checks + return [] + } + + // Empty context - no attributes at all, should trigger noRulesSubquery + const emptyContext = {} + + const actions = await service.computeActions([], emptyContext as any) + + ;(service as any).promotionService_.list = originalPromotionServiceList + + // Should only return the promotion with no rules + expect(prefilteredPromotions).toHaveLength(1) + expect(prefilteredPromotions[0].id).toBe(noRulePromotion.id) + + expect(actions).toHaveLength(0) // No items to apply to + }) + it("should prefilter promotions by applicable rules", async () => { // 1. Promotion with NO rules (should always apply if automatic) await createDefaultPromotion(service, { @@ -39,7 +146,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt0"], // Only applies to product 0 }, @@ -66,7 +173,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt1"], // Only applies to product 1 }, @@ -114,7 +221,7 @@ moduleIntegrationTestRunner({ value: 250, // Different value to distinguish target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt9"], // Only applies to product 9 }, @@ -141,7 +248,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt0"], }, @@ -168,7 +275,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt0"], }, @@ -195,7 +302,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt0"], }, @@ -326,6 +433,88 @@ moduleIntegrationTestRunner({ ]) }) + it("should exclude promotions with rules for attributes not in context", async () => { + // Create promotion with mixed attributes - some in context, some not + await createDefaultPromotion(service, { + code: "MIXED_ATTRIBUTES_PROMO", + is_automatic: true, + rules: [ + { + attribute: "customer.id", + operator: "eq", + values: ["customer1"], // This matches context + }, + { + attribute: "special_flag", // This attribute NOT in context + operator: "eq", + values: ["premium"], + }, + ], + application_method: { + type: "fixed", + target_type: "items", + allocation: "each", + max_quantity: 100000, + value: 100, + }, + }) + + // Create promotion with only context attributes - should be included + await createDefaultPromotion(service, { + code: "CONTEXT_ONLY_PROMO", + is_automatic: true, + rules: [ + { + attribute: "customer.id", + operator: "eq", + values: ["customer1"], + }, + ], + application_method: { + type: "fixed", + target_type: "items", + allocation: "each", + max_quantity: 100000, + value: 50, + }, + }) + + // Spy on promotion service to capture prefiltered results + let prefilteredPromotions: any[] = [] + const originalPromotionServiceList = (service as any).promotionService_ + .list + + ;(service as any).promotionService_.list = async (...args: any[]) => { + const result = await originalPromotionServiceList.bind( + (service as any).promotionService_ + )(...args) + prefilteredPromotions = result + return result + } + + // Context with limited attributes + const testContext = { + customer: { id: "customer1" }, + items: [ + { + id: "item1", + quantity: 1, + subtotal: 100, + product: { id: "prod1" }, + }, + ], + } + + await service.computeActions([], testContext as any) + + // Restore original method + ;(service as any).promotionService_.list = originalPromotionServiceList + + // Should only include promotion with context-only attributes + expect(prefilteredPromotions).toHaveLength(1) + expect(prefilteredPromotions[0].code).toBe("CONTEXT_ONLY_PROMO") + }) + it("should handle prefiltering of many automatic promotions targetting customers and regions with only one that is relevant", async () => { const promotionToCreate: CreatePromotionDTO[] = [] // I ve also tested with 20k and the compute actions takes 200/300ms @@ -484,7 +673,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt0"], // Only applies to product 0 }, @@ -577,7 +766,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -679,7 +868,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -790,7 +979,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -815,7 +1004,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -901,7 +1090,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -926,7 +1115,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1005,7 +1194,7 @@ moduleIntegrationTestRunner({ max_quantity: 5, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1064,7 +1253,7 @@ moduleIntegrationTestRunner({ max_quantity: 5, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1122,7 +1311,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1198,7 +1387,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1223,7 +1412,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1311,7 +1500,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt"], }, @@ -1330,7 +1519,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt"], }, @@ -1393,7 +1582,7 @@ moduleIntegrationTestRunner({ max_quantity: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1418,7 +1607,7 @@ moduleIntegrationTestRunner({ max_quantity: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1497,7 +1686,7 @@ moduleIntegrationTestRunner({ max_quantity: 5, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1550,7 +1739,7 @@ moduleIntegrationTestRunner({ max_quantity: 5, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1611,7 +1800,7 @@ moduleIntegrationTestRunner({ value: 400, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1687,7 +1876,7 @@ moduleIntegrationTestRunner({ value: 400, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1762,7 +1951,7 @@ moduleIntegrationTestRunner({ value: 30, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1787,7 +1976,7 @@ moduleIntegrationTestRunner({ value: 50, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1879,7 +2068,7 @@ moduleIntegrationTestRunner({ value: 1000, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1904,7 +2093,7 @@ moduleIntegrationTestRunner({ value: 50, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -1982,7 +2171,7 @@ moduleIntegrationTestRunner({ value: 1500, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2034,7 +2223,7 @@ moduleIntegrationTestRunner({ value: 500, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2092,7 +2281,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2168,7 +2357,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2243,7 +2432,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2267,7 +2456,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2354,7 +2543,7 @@ moduleIntegrationTestRunner({ value: 50, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt"], }, @@ -2372,7 +2561,7 @@ moduleIntegrationTestRunner({ value: 50, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt"], }, @@ -2446,7 +2635,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_tshirt"], }, @@ -2519,7 +2708,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2543,7 +2732,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2635,7 +2824,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2687,7 +2876,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -2748,7 +2937,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -2822,7 +3011,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -2896,7 +3085,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -2960,7 +3149,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -2985,7 +3174,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3067,7 +3256,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3092,7 +3281,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3168,7 +3357,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3217,7 +3406,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3272,7 +3461,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3346,7 +3535,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3420,7 +3609,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3484,7 +3673,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3510,7 +3699,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3598,7 +3787,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3623,7 +3812,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3711,7 +3900,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3760,7 +3949,7 @@ moduleIntegrationTestRunner({ max_quantity: 2, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3816,7 +4005,7 @@ moduleIntegrationTestRunner({ value: 200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3889,7 +4078,7 @@ moduleIntegrationTestRunner({ value: 200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3961,7 +4150,7 @@ moduleIntegrationTestRunner({ value: 200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -3985,7 +4174,7 @@ moduleIntegrationTestRunner({ value: 200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4072,7 +4261,7 @@ moduleIntegrationTestRunner({ value: 1000, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4096,7 +4285,7 @@ moduleIntegrationTestRunner({ value: 200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4171,7 +4360,7 @@ moduleIntegrationTestRunner({ value: 1200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4219,7 +4408,7 @@ moduleIntegrationTestRunner({ value: 1200, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4273,7 +4462,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4346,7 +4535,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4418,7 +4607,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4442,7 +4631,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4529,7 +4718,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4553,7 +4742,7 @@ moduleIntegrationTestRunner({ value: 10, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4629,7 +4818,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -4682,7 +4871,7 @@ moduleIntegrationTestRunner({ value: 100, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -5083,7 +5272,7 @@ moduleIntegrationTestRunner({ max_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_cotton"], }, @@ -5176,7 +5365,7 @@ moduleIntegrationTestRunner({ max_quantity: undefined, target_rules: [ { - attribute: "shipping_option.id", + attribute: "shipping_methods.shipping_option.id", operator: "in", values: ["express", "standard"], }, @@ -5309,14 +5498,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_tshirt"], }, ], buy_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_sweater"], }, @@ -5403,14 +5592,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 4, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_tshirt"], }, ], buy_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_sweater"], }, @@ -5491,14 +5680,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_tshirt"], }, ], buy_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_sweater"], }, @@ -5592,14 +5781,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, target_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_not-found"], }, ], buy_rules: [ { - attribute: "product_category.id", + attribute: "items.product_category.id", operator: "eq", values: ["catg_sweater"], }, @@ -5633,14 +5822,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 2, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -5699,14 +5888,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 2, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -5834,14 +6023,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 2, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -5866,14 +6055,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 2, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -6078,14 +6267,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 50, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -6110,14 +6299,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 10, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -6176,14 +6365,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 50, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, @@ -6208,14 +6397,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 10, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, ], buy_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [product1], }, diff --git a/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/promotion.spec.ts b/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/promotion.spec.ts index 82d2f73403..c8ad3cc9e3 100644 --- a/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/promotion.spec.ts +++ b/packages/modules/promotion/integration-tests/__tests__/services/promotion-module/promotion.spec.ts @@ -266,7 +266,7 @@ moduleIntegrationTestRunner({ const createdPromotion = await createDefaultPromotion(service, { rules: [ { - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: ["VIP", "top100"], }, @@ -285,7 +285,7 @@ moduleIntegrationTestRunner({ type: "standard", rules: [ expect.objectContaining({ - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: expect.arrayContaining([ expect.objectContaining({ @@ -305,7 +305,7 @@ moduleIntegrationTestRunner({ const createdPromotion = await createDefaultPromotion(service, { rules: [ { - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "eq", values: "VIP", }, @@ -324,7 +324,7 @@ moduleIntegrationTestRunner({ type: "standard", rules: [ expect.objectContaining({ - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "eq", values: expect.arrayContaining([ expect.objectContaining({ @@ -469,14 +469,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, buy_rules: [ { - attribute: "product_collection.id", + attribute: "items.product_collection.id", operator: "eq", values: ["pcol_towel"], }, ], target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_mat"], }, @@ -498,14 +498,14 @@ moduleIntegrationTestRunner({ allocation: "each", buy_rules: [ { - attribute: "product_collection.id", + attribute: "items.product_collection.id", operator: "eq", values: ["pcol_towel"], }, ], target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: ["prod_mat"], }, @@ -528,14 +528,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, buy_rules: [ { - attribute: "product_collection.id", + attribute: "items.product_collection.id", operator: "eq", values: ["pcol_towel"], }, ], target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: "prod_mat", }, @@ -551,14 +551,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, target_rules: [ expect.objectContaining({ - attribute: "product.id", + attribute: "items.product.id", operator: "eq", values: [expect.objectContaining({ value: "prod_mat" })], }), ], buy_rules: [ expect.objectContaining({ - attribute: "product_collection.id", + attribute: "items.product_collection.id", operator: "eq", values: [expect.objectContaining({ value: "pcol_towel" })], }), @@ -980,7 +980,7 @@ moduleIntegrationTestRunner({ it("should successfully add rules to a promotion", async () => { const promotionRules = await service.addPromotionRules(promotion.id, [ { - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: ["VIP", "top100"], }, @@ -989,7 +989,7 @@ moduleIntegrationTestRunner({ expect(promotionRules).toEqual([ expect.objectContaining({ id: promotionRules[0].id, - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: expect.arrayContaining([ expect.objectContaining({ value: "VIP" }), @@ -1041,7 +1041,7 @@ moduleIntegrationTestRunner({ promotion.id, [ { - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: ["VIP", "top100"], }, @@ -1051,7 +1051,7 @@ moduleIntegrationTestRunner({ expect(promotionRules).toEqual([ expect.objectContaining({ id: promotionRules[0].id, - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: expect.arrayContaining([ expect.objectContaining({ value: "VIP" }), @@ -1075,14 +1075,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, buy_rules: [ { - attribute: "product_collection.id", + attribute: "items.product_collection.id", operator: "eq", values: ["pcol_towel"], }, ], target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "in", values: ["prod_1", "prod_2"], }, @@ -1125,7 +1125,7 @@ moduleIntegrationTestRunner({ promotion.id, [ { - attribute: "product.id", + attribute: "items.product.id", operator: "in", values: ["prod_3", "prod_4"], }, @@ -1135,7 +1135,7 @@ moduleIntegrationTestRunner({ expect(promotionRules).toEqual([ expect.objectContaining({ id: promotionRules[0].id, - attribute: "product.id", + attribute: "items.product.id", operator: "in", values: expect.arrayContaining([ expect.objectContaining({ value: "prod_3" }), @@ -1153,7 +1153,7 @@ moduleIntegrationTestRunner({ promotion = await createDefaultPromotion(service, { rules: [ { - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: ["VIP", "top100"], }, @@ -1219,7 +1219,7 @@ moduleIntegrationTestRunner({ application_method: { target_rules: [ { - attribute: "customer_group_id", + attribute: "customer.customer_group.id", operator: "in", values: ["VIP", "top100"], }, @@ -1295,14 +1295,14 @@ moduleIntegrationTestRunner({ buy_rules_min_quantity: 1, target_rules: [ { - attribute: "product.id", + attribute: "items.product.id", operator: "in", values: ["prod_1", "prod_2"], }, ], buy_rules: [ { - attribute: "product_collection", + attribute: "items.product_collection.id", operator: "eq", values: ["pcol_towel"], }, diff --git a/packages/modules/promotion/src/migrations/.snapshot-medusa-promotion.json b/packages/modules/promotion/src/migrations/.snapshot-medusa-promotion.json index a14640fb00..480f092ff6 100644 --- a/packages/modules/promotion/src/migrations/.snapshot-medusa-promotion.json +++ b/packages/modules/promotion/src/migrations/.snapshot-medusa-promotion.json @@ -850,6 +850,15 @@ "unique": false, "expression": "CREATE INDEX IF NOT EXISTS \"IDX_promotion_rule_attribute_operator\" ON \"promotion_rule\" (attribute, operator) WHERE deleted_at IS NULL" }, + { + "keyName": "IDX_promotion_rule_attribute_operator_id", + "columnNames": [], + "composite": false, + "constraint": false, + "primary": false, + "unique": false, + "expression": "CREATE INDEX IF NOT EXISTS \"IDX_promotion_rule_attribute_operator_id\" ON \"promotion_rule\" (operator, attribute, id) WHERE deleted_at IS NULL" + }, { "keyName": "promotion_rule_pkey", "columnNames": [ diff --git a/packages/modules/promotion/src/migrations/Migration20250919122137.ts b/packages/modules/promotion/src/migrations/Migration20250919122137.ts new file mode 100644 index 0000000000..39807f6f4c --- /dev/null +++ b/packages/modules/promotion/src/migrations/Migration20250919122137.ts @@ -0,0 +1,13 @@ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20250919122137 extends Migration { + + override async up(): Promise { + this.addSql(`CREATE INDEX IF NOT EXISTS "IDX_promotion_rule_attribute_operator_id" ON "promotion_rule" (operator, attribute, id) WHERE deleted_at IS NULL;`); + } + + override async down(): Promise { + this.addSql(`drop index if exists "IDX_promotion_rule_attribute_operator_id";`); + } + +} diff --git a/packages/modules/promotion/src/models/promotion-rule.ts b/packages/modules/promotion/src/models/promotion-rule.ts index 1d3b2c172a..e2ce52b137 100644 --- a/packages/modules/promotion/src/models/promotion-rule.ts +++ b/packages/modules/promotion/src/models/promotion-rule.ts @@ -36,6 +36,12 @@ const PromotionRule = model unique: false, where: "deleted_at IS NULL", }, + { + name: "IDX_promotion_rule_attribute_operator_id", + on: ["operator", "attribute", "id"], + unique: false, + where: "deleted_at IS NULL", + }, ]) .cascades({ delete: ["values"], diff --git a/packages/modules/promotion/src/utils/compute-actions/build-promotion-rule-query-filter-from-context.ts b/packages/modules/promotion/src/utils/compute-actions/build-promotion-rule-query-filter-from-context.ts index 6c3fe1e0ff..4505e154d7 100644 --- a/packages/modules/promotion/src/utils/compute-actions/build-promotion-rule-query-filter-from-context.ts +++ b/packages/modules/promotion/src/utils/compute-actions/build-promotion-rule-query-filter-from-context.ts @@ -69,7 +69,6 @@ export async function buildPromotionRuleQueryFilterFromContext( }) }) - // count the number of attributes in the map const numberOfAttributes = attributeValueMap.size if (numberOfAttributes > 10) { const manager = (sharedContext.transactionManager ?? @@ -96,18 +95,9 @@ export async function buildPromotionRuleQueryFilterFromContext( }) } - // Build conditions for a NOT EXISTS subquery to exclude promotions with unsatisfiable rules const sqlConditions: string[] = [] - // First, check for rules where the attribute doesn't exist in context at all - // These rules can never be satisfied - sqlConditions.push( - `pr.attribute NOT IN (${Array.from(attributeValueMap.keys()) - .map((attr) => `'${attr.replace(/'/g, "''")}'`) - .join(",")})` - ) - - // Then, for attributes that exist in context, check if the values don't satisfy the rules + // For each attribute, check if the values don't satisfy the rules attributeValueMap.forEach((valueSet, attribute) => { const values = Array.from(valueSet) const stringValues = values @@ -121,7 +111,6 @@ export async function buildPromotionRuleQueryFilterFromContext( }) .filter((v) => v !== null) as number[] - // Escape attribute name to prevent SQL injection const escapedAttribute = `'${attribute.replace(/'/g, "''")}'` // For 'in' and 'eq' operators - rule is unsatisfiable if NO rule values overlap with context @@ -165,17 +154,25 @@ export async function buildPromotionRuleQueryFilterFromContext( // Handle the case where context has no attributes at all, it means // that any promotion that have a rule cant be satisfied by the context if (attributeValueMap.size === 0) { - // If context has no attributes, exclude all promotions that have any rules - const notExistsSubquery = (alias: string) => + // If context has no attributes, exclude all promotions that have any rules (promotion rules, target rules, or buy rules) + const noRulesSubquery = (alias: string) => ` - NOT EXISTS ( - SELECT 1 FROM promotion_promotion_rule ppr - WHERE ppr.promotion_id = ${alias}.id + ${alias}.id NOT IN ( + SELECT DISTINCT ppr.promotion_id + FROM promotion_promotion_rule ppr + UNION + SELECT DISTINCT am.promotion_id + FROM promotion_application_method am + JOIN application_method_target_rules amtr ON am.id = amtr.application_method_id + UNION + SELECT DISTINCT am2.promotion_id + FROM promotion_application_method am2 + JOIN application_method_buy_rules ambr ON am2.id = ambr.application_method_id ) `.trim() return { - [raw((alias) => notExistsSubquery(alias))]: true, + [raw((alias) => noRulesSubquery(alias))]: true, } } @@ -188,18 +185,69 @@ export async function buildPromotionRuleQueryFilterFromContext( return null } - const notExistsSubquery = (alias: string) => + const attributeKeys = Array.from(attributeValueMap.keys()) + .map((attr) => `'${attr.replace(/'/g, "''")}'`) + .join(",") + + // Use anti-join approach for better performance than NOT IN + const antiJoinSubquery = (alias: string) => ` NOT EXISTS ( - SELECT 1 FROM promotion_promotion_rule ppr - JOIN promotion_rule pr ON ppr.promotion_rule_id = pr.id - LEFT JOIN promotion_rule_value prv ON prv.promotion_rule_id = pr.id - WHERE ppr.promotion_id = ${alias}.id - AND (${joinedConditions}) + SELECT 1 FROM ( + -- Promotions with rules for attributes not in context + SELECT ppr.promotion_id + FROM promotion_promotion_rule ppr + JOIN promotion_rule pr ON ppr.promotion_rule_id = pr.id + WHERE pr.attribute NOT IN (${attributeKeys}) + + UNION + + SELECT am.promotion_id + FROM promotion_application_method am + JOIN application_method_target_rules amtr ON am.id = amtr.application_method_id + JOIN promotion_rule pr ON amtr.promotion_rule_id = pr.id + WHERE pr.attribute NOT IN (${attributeKeys}) + + UNION + + SELECT am2.promotion_id + FROM promotion_application_method am2 + JOIN application_method_buy_rules ambr ON am2.id = ambr.application_method_id + JOIN promotion_rule pr ON ambr.promotion_rule_id = pr.id + WHERE pr.attribute NOT IN (${attributeKeys}) + + UNION + + -- Promotions with unsatisfiable rules for context attributes + SELECT ppr.promotion_id + FROM promotion_promotion_rule ppr + JOIN promotion_rule pr ON ppr.promotion_rule_id = pr.id + LEFT JOIN promotion_rule_value prv ON prv.promotion_rule_id = pr.id + WHERE pr.attribute IN (${attributeKeys}) AND (${joinedConditions}) + + UNION + + SELECT am.promotion_id + FROM promotion_application_method am + JOIN application_method_target_rules amtr ON am.id = amtr.application_method_id + JOIN promotion_rule pr ON amtr.promotion_rule_id = pr.id + LEFT JOIN promotion_rule_value prv ON prv.promotion_rule_id = pr.id + WHERE pr.attribute IN (${attributeKeys}) AND (${joinedConditions}) + + UNION + + SELECT am2.promotion_id + FROM promotion_application_method am2 + JOIN application_method_buy_rules ambr ON am2.id = ambr.application_method_id + JOIN promotion_rule pr ON ambr.promotion_rule_id = pr.id + LEFT JOIN promotion_rule_value prv ON prv.promotion_rule_id = pr.id + WHERE pr.attribute IN (${attributeKeys}) AND (${joinedConditions}) + ) excluded_promotions + WHERE excluded_promotions.promotion_id = ${alias}.id ) `.trim() return { - [raw((alias) => notExistsSubquery(alias))]: true, + [raw((alias) => antiJoinSubquery(alias))]: true, } }