From d4e565b0b92b85818f9e49abc23e80129b215328 Mon Sep 17 00:00:00 2001
From: Riqwan Thamir <rmthamir@gmail.com>
Date: Tue, 6 Dec 2022 14:41:53 +0100
Subject: [PATCH] fix: regions are changed from an optional to required field

A discount should always be associated to atleast 1 region - we validate it against the region in the cart service.

Adding a request parameter validation to pass in atleast one regionID in an array of regions will ensure that regionID will be validated as a part of the discount service and fail if its not a valid region ID.
---
 .../discounts/__tests__/create-discount.js    | 49 +++++++++++++++++++
 .../routes/admin/discounts/create-discount.ts |  8 +--
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/packages/medusa/src/api/routes/admin/discounts/__tests__/create-discount.js b/packages/medusa/src/api/routes/admin/discounts/__tests__/create-discount.js
index c858562dd9..fa315a1e44 100644
--- a/packages/medusa/src/api/routes/admin/discounts/__tests__/create-discount.js
+++ b/packages/medusa/src/api/routes/admin/discounts/__tests__/create-discount.js
@@ -16,6 +16,7 @@ describe("POST /admin/discounts", () => {
             value: 10,
             allocation: "total",
           },
+          regions: [IdMap.getId("region-france")],
           starts_at: "02/02/2021 13:45",
           ends_at: "03/14/2021 04:30",
         },
@@ -41,6 +42,7 @@ describe("POST /admin/discounts", () => {
           value: 10,
           allocation: "total",
         },
+        regions: [IdMap.getId("region-france")],
         starts_at: new Date("02/02/2021 13:45"),
         ends_at: new Date("03/14/2021 04:30"),
         is_disabled: false,
@@ -63,6 +65,7 @@ describe("POST /admin/discounts", () => {
             value: 10,
             allocation: "total",
           },
+          regions: [IdMap.getId("region-france")],
           starts_at: "02/02/2021 13:45",
           is_dynamic: true,
           valid_duration: "PaMT2D",
@@ -100,6 +103,7 @@ describe("POST /admin/discounts", () => {
             value: 10,
             allocation: "total",
           },
+          regions: [IdMap.getId("region-france")],
           starts_at: "02/02/2021 13:45",
           is_dynamic: true,
           valid_duration: "P1Y2M03DT04H05M",
@@ -126,6 +130,7 @@ describe("POST /admin/discounts", () => {
           value: 10,
           allocation: "total",
         },
+        regions: [IdMap.getId("region-france")],
         starts_at: new Date("02/02/2021 13:45"),
         is_disabled: false,
         is_dynamic: true,
@@ -146,6 +151,7 @@ describe("POST /admin/discounts", () => {
             value: 10,
             allocation: "total",
           },
+          regions: [IdMap.getId("region-france")],
         },
         adminSession: {
           jwt: {
@@ -186,6 +192,7 @@ describe("POST /admin/discounts", () => {
               },
             ],
           },
+          regions: [IdMap.getId("region-france")],
           starts_at: "02/02/2021 13:45",
           is_dynamic: true,
           valid_duration: "P1Y2M03DT04H05M",
@@ -222,6 +229,7 @@ describe("POST /admin/discounts", () => {
             value: 10,
             allocation: "total",
           },
+          regions: [IdMap.getId("region-france")],
           ends_at: "02/02/2021",
           starts_at: "03/14/2021",
         },
@@ -259,6 +267,7 @@ describe("POST /admin/discounts", () => {
             value: 10,
             allocation: "total",
           },
+          regions: [IdMap.getId("region-france")],
           starts_at: "03/14/2021 14:30",
         },
         adminSession: {
@@ -284,8 +293,48 @@ describe("POST /admin/discounts", () => {
           value: 10,
           allocation: "total",
         },
+        regions: [IdMap.getId("region-france")],
         starts_at: new Date("03/14/2021 14:30"),
       })
     })
   })
+
+  describe("unsuccessful creation with invalid regions", () => {
+    let subject
+
+    beforeAll(async () => {
+      jest.clearAllMocks()
+
+      subject = await request("POST", "/admin/discounts", {
+        payload: {
+          code: "TEST",
+          rule: {
+            description: "Test",
+            type: "fixed",
+            value: 10,
+            allocation: "total",
+          },
+        },
+        adminSession: {
+          jwt: {
+            userId: IdMap.getId("admin_user"),
+          },
+        },
+      })
+    })
+
+    it("returns 400", () => {
+      expect(subject.status).toEqual(400)
+    })
+
+    it("returns error", () => {
+      expect(subject.body.message).toEqual(
+        `each value in regions must be a string, regions must be an array`
+      )
+    })
+
+    it("does not call service create", () => {
+      expect(DiscountServiceMock.create).toHaveBeenCalledTimes(0)
+    })
+  })
 })
diff --git a/packages/medusa/src/api/routes/admin/discounts/create-discount.ts b/packages/medusa/src/api/routes/admin/discounts/create-discount.ts
index 30ede6452b..37da3037a2 100644
--- a/packages/medusa/src/api/routes/admin/discounts/create-discount.ts
+++ b/packages/medusa/src/api/routes/admin/discounts/create-discount.ts
@@ -42,6 +42,7 @@ import { FindParams } from "../../../../types/common"
  *         required:
  *           - code
  *           - rule
+ *           - regions
  *         properties:
  *           code:
  *             type: string
@@ -151,6 +152,7 @@ import { FindParams } from "../../../../types/common"
  *           value: 10,
  *           allocation: AllocationType.ITEM
  *         },
+ *         regions: ['reg_XXXXXXXX'],
  *         is_dynamic: false,
  *         is_disabled: false
  *       })
@@ -169,7 +171,8 @@ import { FindParams } from "../../../../types/common"
  *              "type": "fixed",
  *              "value": 10,
  *              "allocation": "item"
- *           }
+ *           },
+ *           "regions": ['reg_XXXXXXXX']
  *       }'
  * security:
  *   - api_token: []
@@ -257,9 +260,8 @@ export class AdminPostDiscountsReq {
   usage_limit?: number
 
   @IsArray()
-  @IsOptional()
   @IsString({ each: true })
-  regions?: string[]
+  regions: string[]
 
   @IsObject()
   @IsOptional()