fix(promotion): eval conditions for rules are made accurate (#10915)

what:

- fixes eval conditions for promotion rules

RESOLVES CMRC-851

.changeset/five-plants-cheat.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/promotion": patch
+---
+
+fix(promotion): eval conditions for rules are corrected

integration-tests/http/__fixtures__/product.ts

@@ -0,0 +1,65 @@
+import { ProductStatus } from "@medusajs/utils"
+
+export const medusaTshirtProduct = {
+  title: "Medusa T-Shirt",
+  handle: "t-shirt",
+  status: ProductStatus.PUBLISHED,
+  options: [
+    {
+      title: "Size",
+      values: ["S"],
+    },
+    {
+      title: "Color",
+      values: ["Black", "White"],
+    },
+  ],
+  variants: [
+    {
+      title: "S / Black",
+      sku: "SHIRT-S-BLACK",
+      options: {
+        Size: "S",
+        Color: "Black",
+      },
+      manage_inventory: false,
+      prices: [
+        {
+          amount: 1500,
+          currency_code: "usd",
+        },
+        {
+          amount: 1500,
+          currency_code: "eur",
+        },
+        {
+          amount: 1300,
+          currency_code: "dkk",
+        },
+      ],
+    },
+    {
+      title: "S / White",
+      sku: "SHIRT-S-WHITE",
+      options: {
+        Size: "S",
+        Color: "White",
+      },
+      manage_inventory: false,
+      prices: [
+        {
+          amount: 1500,
+          currency_code: "usd",
+        },
+        {
+          amount: 1500,
+          currency_code: "eur",
+        },
+        {
+          amount: 1300,
+          currency_code: "dkk",
+        },
+      ],
+    },
+  ],
+}

integration-tests/http/__tests__/cart/store/cart.spec.ts

@@ -3,7 +3,6 @@ import {
   Modules,
   PriceListStatus,
   PriceListType,
-  ProductStatus,
   PromotionRuleOperator,
   PromotionStatus,
   PromotionType,
@@ -15,6 +14,7 @@ import {
 } from "../../../../helpers/create-admin-user"
 import { setupTaxStructure } from "../../../../modules/__tests__/fixtures"
 import { createAuthenticatedCustomer } from "../../../../modules/helpers/create-authenticated-customer"
+import { medusaTshirtProduct } from "../../../__fixtures__/product"
 
 jest.setTimeout(100000)
 
@@ -30,70 +30,6 @@ const shippingAddressData = {
   postal_code: "94016",
 }
 
-const productData = {
-  title: "Medusa T-Shirt",
-  handle: "t-shirt",
-  status: ProductStatus.PUBLISHED,
-  options: [
-    {
-      title: "Size",
-      values: ["S"],
-    },
-    {
-      title: "Color",
-      values: ["Black", "White"],
-    },
-  ],
-  variants: [
-    {
-      title: "S / Black",
-      sku: "SHIRT-S-BLACK",
-      options: {
-        Size: "S",
-        Color: "Black",
-      },
-      manage_inventory: false,
-      prices: [
-        {
-          amount: 1500,
-          currency_code: "usd",
-        },
-        {
-          amount: 1500,
-          currency_code: "eur",
-        },
-        {
-          amount: 1300,
-          currency_code: "dkk",
-        },
-      ],
-    },
-    {
-      title: "S / White",
-      sku: "SHIRT-S-WHITE",
-      options: {
-        Size: "S",
-        Color: "White",
-      },
-      manage_inventory: false,
-      prices: [
-        {
-          amount: 1500,
-          currency_code: "usd",
-        },
-        {
-          amount: 1500,
-          currency_code: "eur",
-        },
-        {
-          amount: 1300,
-          currency_code: "dkk",
-        },
-      ],
-    },
-  ],
-}
-
 medusaIntegrationTestRunner({
   env,
   testSuite: ({ dbConnection, getContainer, api }) => {
@@ -150,8 +86,9 @@ medusaIntegrationTestRunner({
           )
         ).data.region
 
-        product = (await api.post("/admin/products", productData, adminHeaders))
+        product = (
-          .data.product
+          await api.post("/admin/products", medusaTshirtProduct, adminHeaders)
+        ).data.product
 
         salesChannel = (
           await api.post(

integration-tests/http/__tests__/promotions/admin/promotions.spec.ts

@@ -1,6 +1,11 @@
 import { medusaIntegrationTestRunner } from "@medusajs/test-utils"
 import { PromotionStatus, PromotionType } from "@medusajs/utils"
-import { createAdminUser } from "../../../../helpers/create-admin-user"
+import {
+  createAdminUser,
+  generatePublishableKey,
+  generateStoreHeaders,
+} from "../../../../helpers/create-admin-user"
+import { medusaTshirtProduct } from "../../../__fixtures__/product"
 
 jest.setTimeout(50000)
 
@@ -219,6 +224,23 @@ medusaIntegrationTestRunner({
           )
         })
 
+        it("should throw error when an incorrect status is passed", async () => {
+          const { response } = await api
+            .post(
+              `/admin/promotions`,
+              { ...standardPromotionPayload, status: "does-not-exist" },
+              adminHeaders
+            )
+            .catch((e) => e)
+
+          expect(response.status).toEqual(400)
+          expect(response.data).toEqual({
+            type: "invalid_data",
+            message:
+              "Invalid request: Expected: 'draft, active, inactive' for field 'status', but got: 'does-not-exist'",
+          })
+        })
+
         it("should create a standard promotion successfully", async () => {
           const response = await api.post(
             `/admin/promotions`,
@@ -466,20 +488,133 @@ medusaIntegrationTestRunner({
           )
         })
 
-        it("should throw error when an incorrect status is passed", async () => {
+        describe("with cart", () => {
-          const { response } = await api
+          it("should add promotion to cart only when gte rule matches", async () => {
-            .post(
+            const publishableKey = await generatePublishableKey(appContainer)
-              `/admin/promotions`,
+            const storeHeaders = generateStoreHeaders({ publishableKey })
-              { ...standardPromotionPayload, status: "does-not-exist" },
+
+            const salesChannel = (
+              await api.post(
+                "/admin/sales-channels",
+                { name: "Webshop", description: "channel" },
                 adminHeaders
               )
-            .catch((e) => e)
+            ).data.sales_channel
 
-          expect(response.status).toEqual(400)
+            const region = (
-          expect(response.data).toEqual({
+              await api.post(
-            type: "invalid_data",
+                "/admin/regions",
-            message:
+                { name: "US", currency_code: "usd", countries: ["us"] },
-              "Invalid request: Expected: 'draft, active, inactive' for field 'status', but got: 'does-not-exist'",
+                adminHeaders
+              )
+            ).data.region
+
+            const product = (
+              await api.post(
+                "/admin/products",
+                medusaTshirtProduct,
+                adminHeaders
+              )
+            ).data.product
+
+            const cart = (
+              await api.post(
+                `/store/carts`,
+                {
+                  currency_code: "usd",
+                  sales_channel_id: salesChannel.id,
+                  region_id: region.id,
+                  items: [{ variant_id: product.variants[0].id, quantity: 1 }],
+                  promo_codes: [promotion.code],
+                },
+                storeHeaders
+              )
+            ).data.cart
+
+            const response = await api.post(
+              `/admin/promotions`,
+              {
+                code: "TEST",
+                type: PromotionType.STANDARD,
+                status: PromotionStatus.ACTIVE,
+                is_automatic: true,
+                application_method: {
+                  target_type: "items",
+                  type: "fixed",
+                  allocation: "each",
+                  currency_code: "USD",
+                  value: 100,
+                  max_quantity: 100,
+                },
+                rules: [
+                  {
+                    attribute: "subtotal",
+                    operator: "gte",
+                    values: "2000",
+                  },
+                ],
+              },
+              adminHeaders
+            )
+
+            expect(response.status).toEqual(200)
+            expect(response.data.promotion).toEqual(
+              expect.objectContaining({
+                id: expect.any(String),
+                code: "TEST",
+                type: "standard",
+                is_automatic: true,
+                application_method: expect.objectContaining({
+                  value: 100,
+                  max_quantity: 100,
+                  type: "fixed",
+                  target_type: "items",
+                  allocation: "each",
+                  target_rules: [],
+                }),
+                rules: [
+                  expect.objectContaining({
+                    operator: "gte",
+                    attribute: "subtotal",
+                    values: expect.arrayContaining([
+                      expect.objectContaining({ value: "2000" }),
+                    ]),
+                  }),
+                ],
+              })
+            )
+
+            const cartWithPromotion1 = (
+              await api.post(
+                `/store/carts/${cart.id}`,
+                { promo_codes: [promotion.code] },
+                storeHeaders
+              )
+            ).data.cart
+
+            expect(cartWithPromotion1).toEqual(
+              expect.objectContaining({
+                promotions: [],
+              })
+            )
+
+            const cartWithPromotion2 = (
+              await api.post(
+                `/store/carts/${cart.id}/line-items`,
+                { variant_id: product.variants[0].id, quantity: 40 },
+                storeHeaders
+              )
+            ).data.cart
+            console.log("cartWithPromotion2 -- ", cartWithPromotion2.promotions)
+            expect(cartWithPromotion2).toEqual(
+              expect.objectContaining({
+                promotions: [
+                  expect.objectContaining({
+                    code: response.data.promotion.code,
+                  }),
+                ],
+              })
+            )
           })
         })
       })

packages/modules/promotion/integration-tests/__tests__/services/promotion-module/evaluate-rule-value-condition.spec.ts

@@ -0,0 +1,84 @@
+import { Modules } from "@medusajs/framework/utils"
+import { moduleIntegrationTestRunner } from "@medusajs/test-utils"
+import { evaluateRuleValueCondition } from "../../../../src/utils/validations/promotion-rule"
+
+moduleIntegrationTestRunner({
+  moduleName: Modules.PROMOTION,
+  testSuite: () => {
+    describe("evaluateRuleValueCondition", () => {
+      const testFunc = evaluateRuleValueCondition
+
+      describe("eq", () => {
+        const operator = "eq"
+
+        it("should evaluate conditions accurately", async () => {
+          expect(testFunc(["2"], operator, [2])).toEqual(true)
+          expect(testFunc(["2"], operator, ["2"])).toEqual(true)
+          expect(testFunc(["2"], operator, ["22"])).toEqual(false)
+        })
+      })
+
+      describe("ne", () => {
+        const operator = "ne"
+
+        it("should evaluate conditions accurately", async () => {
+          expect(testFunc(["2"], operator, [2])).toEqual(false)
+          expect(testFunc(["2"], operator, ["2"])).toEqual(false)
+          expect(testFunc(["2"], operator, ["22"])).toEqual(true)
+        })
+      })
+
+      describe("gt", () => {
+        const operator = "gt"
+
+        it("should evaluate conditions accurately", async () => {
+          expect(testFunc(["2"], operator, [1])).toEqual(false)
+          expect(testFunc(["2"], operator, ["1"])).toEqual(false)
+          expect(testFunc(["2"], operator, [2])).toEqual(false)
+          expect(testFunc(["2"], operator, ["2"])).toEqual(false)
+          expect(testFunc(["2"], operator, ["22"])).toEqual(true)
+          expect(testFunc(["2"], operator, [22])).toEqual(true)
+        })
+      })
+
+      describe("gte", () => {
+        const operator = "gte"
+
+        it("should evaluate conditions accurately", async () => {
+          expect(testFunc(["2"], operator, [1])).toEqual(false)
+          expect(testFunc(["2"], operator, ["1"])).toEqual(false)
+          expect(testFunc(["2"], operator, [2])).toEqual(true)
+          expect(testFunc(["2"], operator, ["2"])).toEqual(true)
+          expect(testFunc(["2"], operator, ["22"])).toEqual(true)
+          expect(testFunc(["2"], operator, [22])).toEqual(true)
+        })
+      })
+
+      describe("lt", () => {
+        const operator = "lt"
+
+        it("should evaluate conditions accurately", async () => {
+          expect(testFunc([1], operator, ["2"])).toEqual(false)
+          expect(testFunc(["1"], operator, ["2"])).toEqual(false)
+          expect(testFunc([2], operator, ["2"])).toEqual(false)
+          expect(testFunc(["2"], operator, ["2"])).toEqual(false)
+          expect(testFunc(["22"], operator, ["2"])).toEqual(true)
+          expect(testFunc([22], operator, ["2"])).toEqual(true)
+        })
+      })
+
+      describe("lte", () => {
+        const operator = "lte"
+
+        it("should evaluate conditions accurately", async () => {
+          expect(testFunc([1], operator, ["2"])).toEqual(false)
+          expect(testFunc(["1"], operator, ["2"])).toEqual(false)
+          expect(testFunc([2], operator, ["2"])).toEqual(true)
+          expect(testFunc(["2"], operator, ["2"])).toEqual(true)
+          expect(testFunc(["22"], operator, ["2"])).toEqual(true)
+          expect(testFunc([22], operator, ["2"])).toEqual(true)
+        })
+      })
+    })
+  },
+})

packages/modules/promotion/src/utils/validations/promotion-rule.ts

@@ -5,6 +5,7 @@ import {
 } from "@medusajs/framework/types"
 import {
   ApplicationMethodTargetType,
+  MathBN,
   MedusaError,
   PromotionRuleOperator,
   isPresent,
@@ -109,7 +110,7 @@ function fetchRuleAttributeForContext(
 export function evaluateRuleValueCondition(
   ruleValues: string[],
   operator: string,
-  ruleValuesToCheck: string[] | string
+  ruleValuesToCheck: (string | number)[] | (string | number)
 ) {
   if (!Array.isArray(ruleValuesToCheck)) {
     ruleValuesToCheck = [ruleValuesToCheck]
@@ -119,29 +120,37 @@ export function evaluateRuleValueCondition(
     return false
   }
 
-  return ruleValuesToCheck.every((ruleValueToCheck: string) => {
+  return ruleValuesToCheck.every((ruleValueToCheck: string | number) => {
     if (operator === "in" || operator === "eq") {
-      return ruleValues.some((ruleValue) => ruleValue === ruleValueToCheck)
+      return ruleValues.some((ruleValue) => ruleValue === `${ruleValueToCheck}`)
     }
 
     if (operator === "ne") {
-      return ruleValues.some((ruleValue) => ruleValue !== ruleValueToCheck)
+      return ruleValues.some((ruleValue) => ruleValue !== `${ruleValueToCheck}`)
     }
 
     if (operator === "gt") {
-      return ruleValues.some((ruleValue) => ruleValue > ruleValueToCheck)
+      return ruleValues.some((ruleValue) =>
+        MathBN.convert(ruleValueToCheck).gt(MathBN.convert(ruleValue))
+      )
     }
 
     if (operator === "gte") {
-      return ruleValues.some((ruleValue) => ruleValue >= ruleValueToCheck)
+      return ruleValues.some((ruleValue) =>
+        MathBN.convert(ruleValueToCheck).gte(MathBN.convert(ruleValue))
+      )
     }
 
     if (operator === "lt") {
-      return ruleValues.some((ruleValue) => ruleValue < ruleValueToCheck)
+      return ruleValues.some((ruleValue) =>
+        MathBN.convert(ruleValueToCheck).lt(MathBN.convert(ruleValue))
+      )
     }
 
     if (operator === "lte") {
-      return ruleValues.some((ruleValue) => ruleValue <= ruleValueToCheck)
+      return ruleValues.some((ruleValue) =>
+        MathBN.convert(ruleValueToCheck).lte(MathBN.convert(ruleValue))
+      )
     }
 
     return false