fix(medusa): Prevent discount type updates (#1584)

integration-tests/api/__tests__/admin/discount.js

@@ -634,7 +634,6 @@ describe("/admin/discounts", () => {
           {
             rule: {
               id: createdRule.id,
-              type: createdRule.type,
               value: createdRule.value,
               allocation: createdRule.allocation,
               conditions: [
@@ -657,24 +656,26 @@ describe("/admin/discounts", () => {
         })
 
       expect(updated.status).toEqual(200)
-      expect(updated.data.discount.rule.conditions).toEqual(expect.arrayContaining([
+      expect(updated.data.discount.rule.conditions).toEqual(
-        expect.objectContaining({
+        expect.arrayContaining([
-          type: "products",
+          expect.objectContaining({
-          operator: "not_in",
+            type: "products",
-          products: expect.arrayContaining([
+            operator: "not_in",
-            expect.objectContaining({
+            products: expect.arrayContaining([
-              id: product.id,
+              expect.objectContaining({
-            }),
+                id: product.id,
-            expect.objectContaining({
+              }),
-              id: anotherProduct.id,
+              expect.objectContaining({
-            }),
+                id: anotherProduct.id,
-          ]),
+              }),
-        }),
+            ]),
-        expect.objectContaining({
+          }),
-          type: "product_types",
+          expect.objectContaining({
-          operator: "not_in",
+            type: "product_types",
-        }),
+            operator: "not_in",
-      ]))
+          }),
+        ])
+      )
     })
 
     it("fails to add condition on rule with existing comb. of type and operator", async () => {
@@ -728,9 +729,7 @@ describe("/admin/discounts", () => {
             {
               rule: {
                 id: createdRule.id,
-                type: createdRule.type,
                 value: createdRule.value,
-                allocation: createdRule.allocation,
                 conditions: [
                   {
                     products: [anotherProduct.id],
@@ -841,7 +840,6 @@ describe("/admin/discounts", () => {
             {
               rule: {
                 id: createdRule.id,
-                type: createdRule.type,
                 value: createdRule.value,
                 allocation: createdRule.allocation,
                 conditions: [
@@ -922,6 +920,121 @@ describe("/admin/discounts", () => {
       )
     })
 
+    it("creates a discount and fails to update it because attempting type update", async () => {
+      const api = useApi()
+
+      const response = await api
+        .post(
+          "/admin/discounts",
+          {
+            code: "HELLOWORLD",
+            rule: {
+              description: "test",
+              type: "percentage",
+              value: 10,
+              allocation: "total",
+            },
+            usage_limit: 10,
+          },
+          {
+            headers: {
+              Authorization: "Bearer test_token",
+            },
+          }
+        )
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+      expect(response.data.discount).toEqual(
+        expect.objectContaining({
+          code: "HELLOWORLD",
+          usage_limit: 10,
+        })
+      )
+
+      await api
+        .post(
+          `/admin/discounts/${response.data.discount.id}`,
+          {
+            usage_limit: 20,
+            rule: {
+              id: response.data.discount.rule.id,
+              type: "free_shipping",
+            },
+          },
+          {
+            headers: {
+              Authorization: "Bearer test_token",
+            },
+          }
+        )
+        .catch((err) => {
+          expect(err.response.status).toEqual(400)
+          expect(err.response.data.message).toEqual(
+            "property type should not exist"
+          )
+        })
+    })
+
+    it("creates a discount and fails to update it because attempting is_dynamic update", async () => {
+      const api = useApi()
+
+      const response = await api
+        .post(
+          "/admin/discounts",
+          {
+            code: "HELLOWORLD",
+            is_dynamic: true,
+            rule: {
+              description: "test",
+              type: "percentage",
+              value: 10,
+              allocation: "total",
+            },
+            usage_limit: 10,
+          },
+          {
+            headers: {
+              Authorization: "Bearer test_token",
+            },
+          }
+        )
+        .catch((err) => {
+          console.log(err)
+        })
+
+      expect(response.status).toEqual(200)
+      expect(response.data.discount).toEqual(
+        expect.objectContaining({
+          code: "HELLOWORLD",
+          usage_limit: 10,
+          is_dynamic: true,
+        })
+      )
+
+      await api
+        .post(
+          `/admin/discounts/${response.data.discount.id}`,
+          {
+            usage_limit: 20,
+            is_dynamic: false,
+          },
+          {
+            headers: {
+              Authorization: "Bearer test_token",
+            },
+          }
+        )
+        .catch((err) => {
+          expect(err.response.status).toEqual(400)
+          expect(err.response.data.message).toEqual(
+            "property is_dynamic should not exist"
+          )
+        })
+    })
+
     it("automatically sets the code to an uppercase string on update", async () => {
       const api = useApi()
 

packages/medusa/src/api/routes/admin/discounts/__tests__/create-discount.js

@@ -161,7 +161,7 @@ describe("POST /admin/discounts", () => {
 
     it("returns error", () => {
       expect(subject.body.message).toEqual(
-        `type should not be empty, type must be a string`
+        `Invalid rule type, must be one of "fixed", "percentage" or "free_shipping"`
       )
     })
   })

packages/medusa/src/api/routes/admin/discounts/__tests__/update-discount.js

@@ -16,7 +16,6 @@ describe("POST /admin/discounts", () => {
             code: "10TOTALOFF",
             rule: {
               id: "1234",
-              type: "fixed",
               value: 10,
               allocation: "total",
             },
@@ -42,7 +41,6 @@ describe("POST /admin/discounts", () => {
           code: "10TOTALOFF",
           rule: {
             id: "1234",
-            type: "fixed",
             value: 10,
             allocation: "total",
           },
@@ -64,12 +62,9 @@ describe("POST /admin/discounts", () => {
             code: "10TOTALOFF",
             rule: {
               id: "1234",
-              type: "fixed",
               value: 10,
-              allocation: "total",
             },
             starts_at: "02/02/2021 13:45",
-            is_dynamic: true,
             valid_duration: "PaMT2D",
           },
           adminSession: {
@@ -92,60 +87,6 @@ describe("POST /admin/discounts", () => {
     })
   })
 
-  describe("successful update with dynamic discount", () => {
-    let subject
-
-    beforeAll(async () => {
-      jest.clearAllMocks()
-      subject = await request(
-        "POST",
-        `/admin/discounts/${IdMap.getId("total10")}`,
-        {
-          payload: {
-            code: "10TOTALOFF",
-            rule: {
-              id: "1234",
-              type: "fixed",
-              value: 10,
-              allocation: "total",
-            },
-            starts_at: "02/02/2021 13:45",
-            is_dynamic: true,
-            valid_duration: "P1Y2M03DT04H05M",
-          },
-          adminSession: {
-            jwt: {
-              userId: IdMap.getId("admin_user"),
-            },
-          },
-        }
-      )
-    })
-
-    it("returns 200", () => {
-      expect(subject.status).toEqual(200)
-    })
-
-    it("calls service update", () => {
-      expect(DiscountServiceMock.update).toHaveBeenCalledTimes(1)
-      expect(DiscountServiceMock.update).toHaveBeenCalledWith(
-        IdMap.getId("total10"),
-        {
-          code: "10TOTALOFF",
-          rule: {
-            id: "1234",
-            type: "fixed",
-            value: 10,
-            allocation: "total",
-          },
-          starts_at: new Date("02/02/2021 13:45"),
-          is_dynamic: true,
-          valid_duration: "P1Y2M03DT04H05M",
-        }
-      )
-    })
-  })
-
   describe("fails on invalid date intervals", () => {
     let subject
 
@@ -159,7 +100,6 @@ describe("POST /admin/discounts", () => {
             code: "10TOTALOFF",
             rule: {
               id: "1234",
-              type: "fixed",
               value: 10,
               allocation: "total",
             },

packages/medusa/src/api/routes/admin/discounts/create-discount.ts

@@ -3,6 +3,7 @@ import {
   IsArray,
   IsBoolean,
   IsDate,
+  IsEnum,
   IsNotEmpty,
   IsNumber,
   IsObject,
@@ -12,6 +13,7 @@ import {
   ValidateNested,
 } from "class-validator"
 import { defaultAdminDiscountsFields, defaultAdminDiscountsRelations } from "."
+import { AllocationType, DiscountRuleType } from "../../../../models"
 import { Discount } from "../../../../models/discount"
 import { DiscountConditionOperator } from "../../../../models/discount-condition"
 import DiscountService from "../../../../services/discount"
@@ -157,16 +159,18 @@ export class AdminPostDiscountsDiscountRule {
   @IsOptional()
   description?: string
 
-  @IsString()
+  @IsEnum(DiscountRuleType, {
-  @IsNotEmpty()
+    message: `Invalid rule type, must be one of "fixed", "percentage" or "free_shipping"`,
-  type: string
+  })
+  type: DiscountRuleType
 
   @IsNumber()
   value: number
 
-  @IsString()
+  @IsEnum(AllocationType, {
-  @IsNotEmpty()
+    message: `Invalid allocation type, must be one of "total" or "item"`,
-  allocation: string
+  })
+  allocation: AllocationType
 
   @IsOptional()
   @IsArray()

packages/medusa/src/api/routes/admin/discounts/get-discount-by-code.ts

@@ -1,5 +1,9 @@
-import { defaultAdminDiscountsRelations } from "."
+import { IsOptional, IsString } from "class-validator"
+import { defaultAdminDiscountsFields, defaultAdminDiscountsRelations } from "."
+import { Discount } from "../../../../models"
 import DiscountService from "../../../../services/discount"
+import { getRetrieveConfig } from "../../../../utils/get-query-config"
+import { validator } from "../../../../utils/validator"
 /**
  * @oas [get] /discounts/code/{code}
  * operationId: "GetDiscountsDiscountCode"
@@ -23,11 +27,30 @@ import DiscountService from "../../../../services/discount"
 export default async (req, res) => {
   const { code } = req.params
 
-  const discountService: DiscountService = req.scope.resolve("discountService")
+  const validated = await validator(
-  const discount = await discountService.retrieveByCode(
+    AdminGetDiscountsDiscountCodeParams,
-    code,
+    req.query
-    defaultAdminDiscountsRelations
   )
 
+  const config = getRetrieveConfig<Discount>(
+    defaultAdminDiscountsFields,
+    defaultAdminDiscountsRelations,
+    validated?.fields?.split(",") as (keyof Discount)[],
+    validated?.expand?.split(",")
+  )
+
+  const discountService: DiscountService = req.scope.resolve("discountService")
+  const discount = await discountService.retrieveByCode(code, config)
+
   res.status(200).json({ discount })
 }
+
+export class AdminGetDiscountsDiscountCodeParams {
+  @IsOptional()
+  @IsString()
+  expand?: string
+
+  @IsOptional()
+  @IsString()
+  fields?: string
+}

packages/medusa/src/api/routes/admin/discounts/update-discount.ts

@@ -3,6 +3,7 @@ import {
   IsArray,
   IsBoolean,
   IsDate,
+  IsEnum,
   IsNotEmpty,
   IsNumber,
   IsObject,
@@ -12,6 +13,7 @@ import {
   ValidateNested,
 } from "class-validator"
 import { defaultAdminDiscountsFields, defaultAdminDiscountsRelations } from "."
+import { AllocationType } from "../../../../models"
 import { Discount } from "../../../../models/discount"
 import { DiscountConditionOperator } from "../../../../models/discount-condition"
 import DiscountService from "../../../../services/discount"
@@ -37,9 +39,6 @@ import { IsISO8601Duration } from "../../../../utils/validators/iso8601-duration
  *           code:
  *             type: string
  *             description: A unique code that will be used to redeem the Discount
- *           is_dynamic:
- *             type: string
- *             description: Whether the Discount should have multiple instances of itself, each with a different code. This can be useful for automatically generated codes that all have to follow a common set of rules.
  *           rule:
  *             description: The Discount Rule that defines how Discounts are calculated
  *             oneOf:
@@ -109,10 +108,6 @@ export class AdminPostDiscountsDiscountReq {
   @Type(() => AdminUpdateDiscountRule)
   rule?: AdminUpdateDiscountRule
 
-  @IsBoolean()
-  @IsOptional()
-  is_dynamic?: boolean
-
   @IsBoolean()
   @IsOptional()
   is_disabled?: boolean
@@ -156,16 +151,15 @@ export class AdminUpdateDiscountRule {
   @IsOptional()
   description?: string
 
-  @IsString()
-  @IsNotEmpty()
-  type: string
-
   @IsNumber()
-  value: number
+  @IsOptional()
+  value?: number
 
-  @IsString()
+  @IsOptional()
-  @IsNotEmpty()
+  @IsEnum(AllocationType, {
-  allocation: string
+    message: `Invalid allocation type, must be one of "total" or "item"`,
+  })
+  allocation?: AllocationType
 
   @IsOptional()
   @IsArray()

packages/medusa/src/services/__tests__/discount.js

@@ -217,7 +217,6 @@ describe("DiscountService", () => {
           code: "10%OFF",
           is_dynamic: false,
         },
-        relations: [],
       })
     })
   })
@@ -232,7 +231,9 @@ describe("DiscountService", () => {
         }),
     })
 
-    const discountRuleRepository = MockRepository({})
+    const discountRuleRepository = MockRepository({
+      create: (values) => values,
+    })
 
     const regionService = {
       retrieve: () => {

packages/medusa/src/services/cart.ts

@@ -1046,7 +1046,7 @@ class CartService extends TransactionBaseService<CartService> {
       async (transactionManager: EntityManager) => {
         const discount = await this.discountService_
           .withTransaction(transactionManager)
-          .retrieveByCode(discountCode, ["rule", "regions"])
+          .retrieveByCode(discountCode, { relations: ["rule", "regions"] })
 
         await this.discountService_
           .withTransaction(transactionManager)

packages/medusa/src/services/discount.ts

@@ -1,8 +1,14 @@
 import { parse, toSeconds } from "iso8601-duration"
 import { isEmpty, omit } from "lodash"
-import { MedusaError, Validator } from "medusa-core-utils"
+import { MedusaError } from "medusa-core-utils"
 import { BaseService } from "medusa-interfaces"
-import { Brackets, EntityManager, ILike, SelectQueryBuilder } from "typeorm"
+import {
+  Brackets,
+  DeepPartial,
+  EntityManager,
+  ILike,
+  SelectQueryBuilder,
+} from "typeorm"
 import {
   EventBusService,
   ProductService,
@@ -24,9 +30,11 @@ import { GiftCardRepository } from "../repositories/gift-card"
 import { FindConfig } from "../types/common"
 import {
   CreateDiscountInput,
+  CreateDiscountRuleInput,
   CreateDynamicDiscountInput,
   FilterableDiscountProps,
   UpdateDiscountInput,
+  UpdateDiscountRuleInput,
 } from "../types/discount"
 import { isFuture, isPast } from "../utils/date-helpers"
 import { formatException } from "../utils/exception-formatter"
@@ -127,35 +135,17 @@ class DiscountService extends BaseService {
    * @param {DiscountRule} discountRule - the discount rule to create
    * @return {Promise} the result of the create operation
    */
-  validateDiscountRule_(discountRule): DiscountRule {
+  validateDiscountRule_<T extends { type: DiscountRuleType; value: number }>(
-    const schema = Validator.object().keys({
+    discountRule: T
-      id: Validator.string().optional(),
+  ): T {
-      description: Validator.string().optional(),
+    if (discountRule.type === "percentage" && discountRule.value > 100) {
-      type: Validator.string().required(),
-      value: Validator.number().min(0).required(),
-      allocation: Validator.string().required(),
-      created_at: Validator.date().optional(),
-      updated_at: Validator.date().allow(null).optional(),
-      deleted_at: Validator.date().allow(null).optional(),
-      metadata: Validator.object().allow(null).optional(),
-    })
-
-    const { value, error } = schema.validate(discountRule)
-    if (error) {
-      throw new MedusaError(
-        MedusaError.Types.INVALID_DATA,
-        error.details[0].message
-      )
-    }
-
-    if (value.type === "percentage" && value.value > 100) {
       throw new MedusaError(
         MedusaError.Types.INVALID_DATA,
         "Discount value above 100 is not allowed when type is percentage"
       )
     }
 
-    return value
+    return discountRule
   }
 
   /**
@@ -228,16 +218,15 @@ class DiscountService extends BaseService {
    * @return {Promise} the result of the create operation
    */
   async create(discount: CreateDiscountInput): Promise<Discount> {
-    return this.atomicPhase_(async (manager) => {
+    return this.atomicPhase_(async (manager: EntityManager) => {
       const discountRepo = manager.getCustomRepository(this.discountRepository_)
       const ruleRepo = manager.getCustomRepository(this.discountRuleRepository_)
 
       const conditions = discount.rule?.conditions
 
       const ruleToCreate = omit(discount.rule, ["conditions"])
-      discount.rule = ruleToCreate
+      const validatedRule: Omit<CreateDiscountRuleInput, "conditions"> =
-
+        this.validateDiscountRule_(ruleToCreate)
-      const validatedRule = this.validateDiscountRule_(discount.rule)
 
       if (
         discount?.regions &&
@@ -258,13 +247,16 @@ class DiscountService extends BaseService {
           )
         }
 
-        const discountRule = await ruleRepo.create(validatedRule)
+        const discountRule = ruleRepo.create(validatedRule)
         const createdDiscountRule = await ruleRepo.save(discountRule)
 
         discount.code = discount.code!.toUpperCase()
-        discount.rule = createdDiscountRule
 
-        const created = await discountRepo.create(discount)
+        const created: Discount = discountRepo.create(
+          discount as DeepPartial<Discount>
+        )
+        created.rule = createdDiscountRule
+
         const result = await discountRepo.save(created)
 
         if (conditions?.length) {
@@ -315,27 +307,26 @@ class DiscountService extends BaseService {
   /**
    * Gets a discount by discount code.
    * @param {string} discountCode - discount code of discount to retrieve
-   * @param {array} relations - list of relations
+   * @param {Object} config - the config object containing query settings
    * @return {Promise<Discount>} the discount document
    */
   async retrieveByCode(
     discountCode: string,
-    relations: string[] = []
+    config: FindConfig<Discount> = {}
   ): Promise<Discount> {
     const discountRepo = this.manager_.getCustomRepository(
       this.discountRepository_
     )
 
-    let discount = await discountRepo.findOne({
+    let query = this.buildQuery_(
-      where: { code: discountCode.toUpperCase(), is_dynamic: false },
+      { code: discountCode, is_dynamic: false },
-      relations,
+      config
-    })
+    )
+    let discount = await discountRepo.findOne(query)
 
     if (!discount) {
-      discount = await discountRepo.findOne({
+      query = this.buildQuery_({ code: discountCode, is_dynamic: true }, config)
-        where: { code: discountCode.toUpperCase(), is_dynamic: true },
+      discount = await discountRepo.findOne(query)
-        relations,
-      })
 
       if (!discount) {
         throw new MedusaError(
@@ -359,7 +350,12 @@ class DiscountService extends BaseService {
     update: UpdateDiscountInput
   ): Promise<Discount> {
     return this.atomicPhase_(async (manager) => {
-      const discountRepo = manager.getCustomRepository(this.discountRepository_)
+      const discountRepo: DiscountRepository = manager.getCustomRepository(
+        this.discountRepository_
+      )
+      const ruleRepo: DiscountRuleRepository = manager.getCustomRepository(
+        this.discountRuleRepository_
+      )
 
       const discount = await this.retrieve(discountId, {
         relations: ["rule"],
@@ -411,7 +407,21 @@ class DiscountService extends BaseService {
       }
 
       if (rule) {
-        discount.rule = this.validateDiscountRule_(ruleToUpdate)
+        const ruleUpdate: Omit<UpdateDiscountRuleInput, "conditions"> = rule
+
+        if (rule.value) {
+          this.validateDiscountRule_({
+            value: rule.value,
+            type: discount.rule.type,
+          })
+        }
+
+        const updatedRule = ruleRepo.create({
+          ...discount.rule,
+          ...ruleUpdate,
+        })
+
+        discount.rule = updatedRule
       }
 
       for (const key of Object.keys(rest).filter(

packages/medusa/src/types/discount.ts

@@ -117,9 +117,9 @@ export type UpsertDiscountConditionInput = {
 
 export type CreateDiscountRuleInput = {
   description?: string
-  type: string
+  type: DiscountRuleType
   value: number
-  allocation: string
+  allocation: AllocationType
   conditions?: UpsertDiscountConditionInput[]
 }
 
@@ -139,16 +139,14 @@ export type CreateDiscountInput = {
 export type UpdateDiscountRuleInput = {
   id: string
   description?: string
-  type: string
+  value?: number
-  value: number
+  allocation?: AllocationType
-  allocation: string
   conditions?: UpsertDiscountConditionInput[]
 }
 
 export type UpdateDiscountInput = {
   code?: string
   rule?: UpdateDiscountRuleInput
-  is_dynamic?: boolean
   is_disabled?: boolean
   starts_at?: Date
   ends_at?: Date | null