Fix/add validators (#1197)

* chore: add validators * fix: tests * fix: add test * chore: enforce 6 length otp
isomerpages · Mar 11, 2024 · a7284e5 · a7284e5
1 parent 555b89b
commit a7284e5
Show file tree

Hide file tree

Showing 14 changed files with 270 additions and 70 deletions.
diff --git a/src/fixtures/sessionData.ts b/src/fixtures/sessionData.ts
@@ -21,7 +21,7 @@ import {
 export const mockAccessToken = "mockAccessToken"
 export const mockGithubId = "mockGithubId"
 export const mockIsomerUserId = "1"
-export const mockEmail = "mockEmail"
+export const mockEmail = "mockEmail@email.com"
 export const mockTreeSha = "mockTreeSha"
 export const mockCurrentCommitSha = "mockCurrentCommitSha"
 export const mockSiteName = "mockSiteName"

diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts
@@ -975,9 +975,11 @@ describe("Review Requests Integration Tests", () => {
       )
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
-      )
+      const actual = await request(app)
+        .post(`/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`)
+        .send({
+          reviewers: [MOCK_USER_EMAIL_THREE],
+        })
 
       // Assert
       expect(actual.statusCode).toEqual(404)
@@ -992,7 +994,11 @@ describe("Review Requests Integration Tests", () => {
       )
 
       // Act
-      const actual = await request(app).post(`/${MOCK_REPO_NAME_ONE}/123456`)
+      const actual = await request(app)
+        .post(`/${MOCK_REPO_NAME_ONE}/123456`)
+        .send({
+          reviewers: [MOCK_USER_EMAIL_THREE],
+        })
 
       // Assert
       expect(actual.statusCode).toEqual(404)
@@ -1007,9 +1013,11 @@ describe("Review Requests Integration Tests", () => {
       )
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
-      )
+      const actual = await request(app)
+        .post(`/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`)
+        .send({
+          reviewers: [MOCK_USER_EMAIL_THREE],
+        })
 
       // Assert
       expect(actual.statusCode).toEqual(403)
@@ -1078,88 +1086,88 @@ describe("Review Requests Integration Tests", () => {
       })
     })
 
-    it("should close the review request successfully", async () => {
+    it("should return 404 if site is not found", async () => {
       // Arrange
       const app = generateRouterForUserWithSite(
         subrouter,
         MOCK_USER_SESSION_DATA_ONE,
-        MOCK_REPO_NAME_ONE
+        MOCK_REPO_NAME_TWO
       )
-      mockGenericAxios.patch.mockResolvedValueOnce(null)
 
       // Act
       const actual = await request(app).delete(
-        `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
+        `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
       )
 
       // Assert
-      expect(actual.statusCode).toEqual(200)
+      expect(actual.statusCode).toEqual(404)
     })
 
-    it("should return 404 if site is not found", async () => {
+    it("should return 404 if the review request is not found", async () => {
       // Arrange
       const app = generateRouterForUserWithSite(
         subrouter,
         MOCK_USER_SESSION_DATA_ONE,
-        MOCK_REPO_NAME_TWO
+        MOCK_REPO_NAME_ONE
       )
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
-      )
+      const actual = await request(app).delete(`/${MOCK_REPO_NAME_ONE}/123456`)
 
       // Assert
       expect(actual.statusCode).toEqual(404)
     })
 
-    it("should return 404 if the review request is not found", async () => {
+    it("should return 403 if user is not the original requestor", async () => {
       // Arrange
       const app = generateRouterForUserWithSite(
         subrouter,
-        MOCK_USER_SESSION_DATA_ONE,
+        MOCK_USER_SESSION_DATA_TWO,
         MOCK_REPO_NAME_ONE
       )
 
       // Act
-      const actual = await request(app).post(`/${MOCK_REPO_NAME_ONE}/123456`)
+      const actual = await request(app).delete(
+        `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
+      )
 
       // Assert
-      expect(actual.statusCode).toEqual(404)
+      expect(actual.statusCode).toEqual(403)
     })
 
-    it("should return 403 if user is not the original requestor", async () => {
+    it("should return 403 if the user is not a valid site member", async () => {
       // Arrange
       const app = generateRouterForUserWithSite(
         subrouter,
-        MOCK_USER_SESSION_DATA_TWO,
+        MOCK_USER_SESSION_DATA_THREE,
         MOCK_REPO_NAME_ONE
       )
 
       // Act
-      const actual = await request(app).post(
+      const actual = await request(app).delete(
         `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
       )
 
       // Assert
       expect(actual.statusCode).toEqual(403)
     })
 
-    it("should return 403 if the user is not a valid site member", async () => {
+    it("should close the review request successfully", async () => {
       // Arrange
       const app = generateRouterForUserWithSite(
         subrouter,
-        MOCK_USER_SESSION_DATA_THREE,
+        MOCK_USER_SESSION_DATA_ONE,
         MOCK_REPO_NAME_ONE
       )
+      mockGenericAxios.patch.mockResolvedValueOnce(null)
 
       // Act
-      const actual = await request(app).post(
+      const actual = await request(app).delete(
         `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`
       )
 
       // Assert
-      expect(actual.statusCode).toEqual(403)
+      expect(actual.statusCode).toEqual(200)
     })
   })
 
@@ -1871,9 +1879,11 @@ describe("Review Requests Integration Tests", () => {
       mockGenericAxios.post.mockResolvedValueOnce(null)
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments`
-      )
+      const actual = await request(app)
+        .post(
+          `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments`
+        )
+        .send({ message: "blahblah" })
 
       // Assert
       expect(actual.statusCode).toEqual(200)
@@ -1888,9 +1898,11 @@ describe("Review Requests Integration Tests", () => {
       )
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments`
-      )
+      const actual = await request(app)
+        .post(
+          `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments`
+        )
+        .send({ message: "blahblah" })
 
       // Assert
       expect(actual.statusCode).toEqual(404)
@@ -1905,9 +1917,11 @@ describe("Review Requests Integration Tests", () => {
       )
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments`
-      )
+      const actual = await request(app)
+        .post(
+          `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments`
+        )
+        .send({ message: "blahblah" })
 
       // Assert
       expect(actual.statusCode).toEqual(404)
@@ -1922,9 +1936,9 @@ describe("Review Requests Integration Tests", () => {
       )
 
       // Act
-      const actual = await request(app).post(
-        `/${MOCK_REPO_NAME_ONE}/123456/comments`
-      )
+      const actual = await request(app)
+        .post(`/${MOCK_REPO_NAME_ONE}/123456/comments`)
+        .send({ message: "blahblah" })
 
       // Assert
       expect(actual.statusCode).toEqual(404)

diff --git a/src/integration/Users.spec.ts b/src/integration/Users.spec.ts
@@ -40,7 +40,7 @@ const subrouter = express()
 // that allows us to set this properties also
 subrouter.use((req, res, next) => {
   const userSessionData = new UserSessionData({
-    isomerUserId: req.body.userId,
+    isomerUserId: mockIsomerUserId,
     githubId: req.body.githubId,
     email: req.body.email,
   })
@@ -223,7 +223,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/email/verifyOtp").send({
         email: mockValidEmail,
         otp,
-        userId: mockIsomerUserId,
       })
       const updatedUser = await User.findOne({
         where: {
@@ -252,7 +251,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/email/verifyOtp").send({
         email: mockValidEmail,
         otp: wrongOtp,
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -274,7 +272,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/email/verifyOtp").send({
         email: mockValidEmail,
         otp: "",
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -296,7 +293,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/email/verifyOtp").send({
         email: mockValidEmail,
         otp: undefined,
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -326,7 +322,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/email/verifyOtp").send({
         email: mockValidEmail,
         otp,
-        userId: mockIsomerUserId,
       })
       const oldOtp = otp
 
@@ -342,7 +337,6 @@ describe("Users Router", () => {
       const newActual = await request(app).post("/email/verifyOtp").send({
         email: mockValidEmail,
         otp: oldOtp,
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -366,7 +360,6 @@ describe("Users Router", () => {
         const actual = await request(app).post("/email/verifyOtp").send({
           email: mockValidEmail,
           otp: mockInvalidOtp,
-          userId: mockIsomerUserId,
         })
         const otpEntry = await Otp.findOne({
           where: { email: mockValidEmail },
@@ -400,7 +393,6 @@ describe("Users Router", () => {
         await request(app).post("/email/verifyOtp").send({
           email: mockValidEmail,
           otp: mockInvalidOtp,
-          userId: mockIsomerUserId,
         })
       }
 
@@ -502,7 +494,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/mobile/verifyOtp").send({
         mobile: mockValidNumber,
         otp,
-        userId: mockIsomerUserId,
       })
       const updatedUser = await User.findOne({
         where: {
@@ -529,13 +520,34 @@ describe("Users Router", () => {
       const actual = await request(app).post("/mobile/verifyOtp").send({
         mobile: mockValidNumber,
         otp: wrongOtp,
-        userId: mockIsomerUserId,
       })
 
       // Assert
       expect(actual.statusCode).toBe(expected)
     })
 
+    it("should return 400 when the request body format is wrong", async () => {
+      // Arrange
+      const expected = 400
+      const otp = "123456"
+      mockAxios.post.mockResolvedValueOnce(200)
+      await User.create({ id: mockIsomerUserId })
+      await request(app).post("/mobile/otp").send({
+        mobile: mockValidNumber,
+      })
+
+      // Act
+      const actual = await request(app)
+        .post("/mobile/verifyOtp")
+        .send({
+          mobile: [mockValidNumber, "98765432"],
+          otp,
+        })
+
+      // Assert
+      expect(actual.statusCode).toBe(expected)
+    })
+
     it("should return 400 when there is no otp", async () => {
       // Arrange
       const expected = 400
@@ -549,7 +561,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/mobile/verifyOtp").send({
         mobile: mockValidNumber,
         otp: "",
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -569,7 +580,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/mobile/verifyOtp").send({
         mobile: mockValidNumber,
         otp: undefined,
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -594,7 +604,6 @@ describe("Users Router", () => {
       const actual = await request(app).post("/mobile/verifyOtp").send({
         mobile: mockValidNumber,
         otp,
-        userId: mockIsomerUserId,
       })
       const oldOtp = otp
 
@@ -610,7 +619,6 @@ describe("Users Router", () => {
       const newActual = await request(app).post("/mobile/verifyOtp").send({
         mobile: mockValidNumber,
         otp: oldOtp,
-        userId: mockIsomerUserId,
       })
 
       // Assert
@@ -632,7 +640,6 @@ describe("Users Router", () => {
         const actual = await request(app).post("/mobile/verifyOtp").send({
           mobile: mockValidNumber,
           otp: mockInvalidOtp,
-          userId: mockIsomerUserId,
         })
         const otpEntry = await Otp.findOne({
           where: { mobileNumber: mockValidNumber },
@@ -664,7 +671,6 @@ describe("Users Router", () => {
         await request(app).post("/mobile/verifyOtp").send({
           mobile: mockValidNumber,
           otp: mockInvalidOtp,
-          userId: mockIsomerUserId,
         })
       }