asyavee/medusa-store
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

chore(): Further improve promotions computation (#13556)

Browse Source 
* chore(): Further improve promotions

* chore(): Further improve promotions

* chore(): Further improve promotions

* chore(): Further improve promotions

* chore(): Further improve promotions

* Create lazy-lemons-occur.md

* chore(): Further improve promotions

* WIP

* fix

* improve:

* fix attribute tests

* fix tests

* union
This commit is contained in:
Adrien de Peretti
2025-09-19 21:34:35 +02:00
committed by GitHub
parent cb716856b6
commit 4c1c1dd4c0
12 changed files with 431 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/lazy-lemons-occur.md
+5
View File
@@ -0,0 +1,5 @@
---
"@medusajs/promotion": patch
---
Chore(): Further promotions pre filtering improvements
integration-tests/http/__tests__/cart/store/cart.spec.ts
+3 -3
View File
@@ -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],
},
integration-tests/modules/__tests__/cart/store/add-promotions-to-cart.spec.ts
+1 -1
View File
@@ -82,7 +82,7 @@ medusaIntegrationTestRunner({
currency_code: "usd",
target_rules: [
{
attribute: "product_id",
attribute: "items.product_id",
operator: "eq",
values: "prod_mat",
},
integration-tests/modules/__tests__/cart/store/carts.spec.ts
+1 -1
View File
@@ -1418,7 +1418,7 @@ medusaIntegrationTestRunner({
max_quantity: 1,
target_rules: [
{
attribute: "product_id",
attribute: "items.product_id",
operator: "eq",
values: [product.id],
},
integration-tests/modules/__tests__/cart/store/remove-promotions-from-cart.spec.ts
+2 -2
View File
@@ -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",
},
integration-tests/modules/__tests__/order/draft-order.spec.ts
+2 -2
View File
@@ -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],
},
packages/modules/promotion/integration-tests/__tests__/services/promotion-module/compute-actions.spec.ts
+292 -103
View File
File diff suppressed because it is too large Load Diff
packages/modules/promotion/integration-tests/__tests__/services/promotion-module/promotion.spec.ts
+24 -24
View File
@@ -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"],
},
packages/modules/promotion/src/migrations/.snapshot-medusa-promotion.json
+9
View File
@@ -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": [
packages/modules/promotion/src/migrations/Migration20250919122137.ts
+13
View File
@@ -0,0 +1,13 @@
import { Migration } from '@mikro-orm/migrations';
export class Migration20250919122137 extends Migration {
override async up(): Promise<void> {
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<void> {
this.addSql(`drop index if exists "IDX_promotion_rule_attribute_operator_id";`);
}
}
packages/modules/promotion/src/models/promotion-rule.ts
+6
View File
@@ -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"],
packages/modules/promotion/src/utils/compute-actions/build-promotion-rule-query-filter-from-context.ts
+73 -25
View File
@@ -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,
}
}