From ac37728e12ff7b57598e67ac7f74a9c0b5bb20de Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 17:45:08 +0800 Subject: [PATCH 01/14] fix: force tests to run sequentially --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4610054c4..d80be5863 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "start": "node -r ts-node/register/transpile-only -r tsconfig-paths/register build/server.js", "dev:services": "docker compose up -d", "dev": "source .env && ts-node-dev --respawn src/server.js", - "test": "source .env.test && jest", + "test": "source .env.test && jest --runInBand", "release": "npm version $npm_config_isomer_update && git push --tags", "lint": "npx eslint .", "lint-fix": "eslint --ignore-path .gitignore . --fix", From b07a745940f864eba3919f9051742e2baa01f043 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 17:45:44 +0800 Subject: [PATCH 02/14] fix: reset the state of the tables before running the tests --- src/integration/Sites.spec.ts | 5 +++++ src/integration/Users.spec.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/integration/Sites.spec.ts b/src/integration/Sites.spec.ts index f77d1ee2f..f616a62c6 100644 --- a/src/integration/Sites.spec.ts +++ b/src/integration/Sites.spec.ts @@ -109,6 +109,11 @@ describe("Sites Router", () => { describe("/", () => { beforeAll(async () => { + await User.sync({ force: true }) + await Site.sync({ force: true }) + await Repo.sync({ force: true }) + await SiteMember.sync({ force: true }) + // Set up User and Site table entries await User.create({ id: mockIsomerUserId, diff --git a/src/integration/Users.spec.ts b/src/integration/Users.spec.ts index cb638e3a1..326c3bd56 100644 --- a/src/integration/Users.spec.ts +++ b/src/integration/Users.spec.ts @@ -55,6 +55,11 @@ const extractMobileOtp = (mobileBody: string): string => mobileBody.slice(12, 12 + 6) describe("Users Router", () => { + beforeAll(async () => { + await User.sync({ force: true }) + await Whitelist.sync({ force: true }) + }) + afterEach(() => { jest.resetAllMocks() mockAxios.reset() From 2b8baa4d1246f7a9660a616c8f34467c4ce20a1c Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 17:46:29 +0800 Subject: [PATCH 03/14] feat: add fixtures used for review requests --- src/fixtures/github.ts | 138 +++++++++++++++++++++++++++++++++++++++++ src/fixtures/review.ts | 14 +++-- src/fixtures/sites.ts | 127 +++++++++++++++++++++++++++++++++++++ src/fixtures/users.ts | 46 ++++++++++++++ 4 files changed, 320 insertions(+), 5 deletions(-) create mode 100644 src/fixtures/github.ts diff --git a/src/fixtures/github.ts b/src/fixtures/github.ts new file mode 100644 index 000000000..7710a4d0f --- /dev/null +++ b/src/fixtures/github.ts @@ -0,0 +1,138 @@ +import { + Commit, + RawComment, + RawFileChangeInfo, + RawPullRequest, +} from "@root/types/github" + +import { MOCK_USER_ID_ONE, MOCK_USER_ID_TWO } from "./users" + +export const MOCK_GITHUB_USER_NAME_ONE = "isomergithub1" +export const MOCK_GITHUB_USER_NAME_TWO = "isomergithub2" + +export const MOCK_GITHUB_USER_EMAIL_ONE = + "111718653+isomergithub1@users.noreply.github.com" +export const MOCK_GITHUB_USER_EMAIL_TWO = + "111725612+isomergithub2@users.noreply.github.com" + +export const MOCK_GITHUB_PULL_REQUEST_NUMBER = 251 + +// This is one set of commits and file changes which should be used together +export const MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA = + "a15a7c8b23324f680cd7c5011ca763e36d350f41" +export const MOCK_GITHUB_COMMIT_DATE_ONE = "2022-10-12T06:31:05Z" +export const MOCK_GITHUB_COMMIT_DATE_TWO = "2022-10-13T05:39:43Z" +export const MOCK_GITHUB_COMMIT_DATE_THREE = "2022-11-07T16:32:08Z" +export const MOCK_GITHUB_FILENAME_ALPHA_ONE = "index.md" +export const MOCK_GITHUB_FILEPATH_ALPHA_ONE = "" +export const MOCK_GITHUB_FILENAME_ALPHA_TWO = "Example Title 22.md" +export const MOCK_GITHUB_FILEPATH_ALPHA_TWO = "pages/" +export const MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_ONE = `Update file: ${MOCK_GITHUB_FILENAME_ALPHA_ONE}` +export const MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_TWO = `Update file: ${MOCK_GITHUB_FILENAME_ALPHA_TWO}` +export const MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_ONE = { + message: MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_ONE, + fileName: MOCK_GITHUB_FILENAME_ALPHA_ONE, + userId: MOCK_USER_ID_ONE, +} +export const MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_TWO = { + message: MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_TWO, + fileName: MOCK_GITHUB_FILENAME_ALPHA_TWO, + userId: MOCK_USER_ID_ONE, +} +export const MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_THREE = { + message: MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_TWO, + fileName: MOCK_GITHUB_FILENAME_ALPHA_TWO, + userId: MOCK_USER_ID_TWO, +} + +export const MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE: RawFileChangeInfo = { + sha: "66804d21ba86f1a193c31714bc15e388c2013a57", + filename: MOCK_GITHUB_FILENAME_ALPHA_ONE, + status: "modified", + additions: 1, + deletions: 2, + changes: 3, + blob_url: `https://github.com/isomerpages/a-test-v4/blob/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/index.md`, + raw_url: `https://github.com/isomerpages/a-test-v4/raw/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/index.md`, + contents_url: `https://api.github.com/repos/isomerpages/a-test-v4/contents/index.md?ref=${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, +} +export const MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO: RawFileChangeInfo = { + sha: "f04f18eaa8d31fffc9f8cf5020b1f6a765ac225f", + filename: `${MOCK_GITHUB_FILEPATH_ALPHA_TWO}/${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, + status: "modified", + additions: 13, + deletions: 2, + changes: 15, + blob_url: `https://github.com/isomerpages/a-test-v4/blob/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/pages%2FExample%20Title%2022.md`, + raw_url: `https://github.com/isomerpages/a-test-v4/raw/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/pages%2FExample%20Title%2022.md`, + contents_url: `https://api.github.com/repos/isomerpages/a-test-v4/contents/pages%2FExample%20Title%2022.md?ref=${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, +} + +export const MOCK_GITHUB_COMMIT_ALPHA_ONE: Commit = { + url: + "https://api.github.com/repos/isomerpages/a-test-v4/commits/a79525f0d188880b965053bc0df25a041b476fad", + sha: "a79525f0d188880b965053bc0df25a041b476fad", + commit: { + url: + "https://api.github.com/repos/isomerpages/a-test-v4/git/commits/a79525f0d188880b965053bc0df25a041b476fad", + author: { + name: MOCK_GITHUB_USER_NAME_ONE, + email: MOCK_GITHUB_USER_EMAIL_ONE, + date: MOCK_GITHUB_COMMIT_DATE_ONE, + }, + message: JSON.stringify(MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_ONE), + }, +} +export const MOCK_GITHUB_COMMIT_ALPHA_TWO: Commit = { + url: + "https://api.github.com/repos/isomerpages/a-test-v4/commits/ad2b13184f8ee1030636c304737941146bd67f4d", + sha: "ad2b13184f8ee1030636c304737941146bd67f4d", + commit: { + url: + "https://api.github.com/repos/isomerpages/a-test-v4/git/commits/ad2b13184f8ee1030636c304737941146bd67f4d", + author: { + name: MOCK_GITHUB_USER_NAME_TWO, + email: MOCK_GITHUB_USER_EMAIL_TWO, + date: MOCK_GITHUB_COMMIT_DATE_TWO, + }, + message: JSON.stringify(MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_TWO), + }, +} +export const MOCK_GITHUB_COMMIT_ALPHA_THREE: Commit = { + url: `https://api.github.com/repos/isomerpages/a-test-v4/commits/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, + sha: MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA, + commit: { + url: `https://api.github.com/repos/isomerpages/a-test-v4/git/commits/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, + author: { + name: MOCK_GITHUB_USER_NAME_ONE, + email: MOCK_GITHUB_USER_EMAIL_ONE, + date: MOCK_GITHUB_COMMIT_DATE_THREE, + }, + message: JSON.stringify(MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_THREE), + }, +} +// end of set + +export const MOCK_GITHUB_COMMENT_BODY_ONE = "Comment 1" +export const MOCK_GITHUB_COMMENT_BODY_TWO = "Comment 2" + +export const MOCK_GITHUB_COMMENT_OBJECT_ONE = { + message: MOCK_GITHUB_COMMENT_BODY_ONE, + fileName: MOCK_GITHUB_FILENAME_ALPHA_ONE, + userId: MOCK_USER_ID_ONE, +} + +export const MOCK_GITHUB_COMMENT_OBJECT_TWO = { + message: MOCK_GITHUB_COMMENT_BODY_TWO, + fileName: MOCK_GITHUB_FILENAME_ALPHA_TWO, + userId: MOCK_USER_ID_TWO, +} + +export const MOCK_GITHUB_RAWCOMMENT_ONE: RawComment = { + body: JSON.stringify(MOCK_GITHUB_COMMENT_OBJECT_ONE), + created_at: MOCK_GITHUB_COMMIT_DATE_ONE, +} +export const MOCK_GITHUB_RAWCOMMENT_TWO: RawComment = { + body: JSON.stringify(MOCK_GITHUB_COMMENT_OBJECT_TWO), + created_at: MOCK_GITHUB_COMMIT_DATE_THREE, +} diff --git a/src/fixtures/review.ts b/src/fixtures/review.ts index f533dad4c..dbedadd48 100644 --- a/src/fixtures/review.ts +++ b/src/fixtures/review.ts @@ -2,7 +2,7 @@ import { Attributes } from "sequelize/types" import { ReviewRequestStatus } from "@root/constants" import { ReviewRequest, ReviewRequestView } from "@root/database/models" -import { Commit } from "@root/types/github" +import { Commit, RawPullRequest } from "@root/types/github" import { mockCollaboratorAdmin1, @@ -56,10 +56,14 @@ export const MOCK_PULL_REQUEST_COMMIT_TWO: Commit = { }, } -export const MOCK_PULL_REQUEST_ONE = { - title: "Pull Request 1", - body: "Pull Request 1 Description", - changed_files: 3, +export const MOCK_PULL_REQUEST_TITLE_ONE = "Pull Request 1" +export const MOCK_PULL_REQUEST_BODY_ONE = "Pull Request 1 Description" +export const MOCK_PULL_REQUEST_CHANGED_FILES_ONE = 3 + +export const MOCK_PULL_REQUEST_ONE: RawPullRequest = { + title: MOCK_PULL_REQUEST_TITLE_ONE, + body: MOCK_PULL_REQUEST_BODY_ONE, + changed_files: MOCK_PULL_REQUEST_CHANGED_FILES_ONE, created_at: MOCK_GITHUB_DATE_ONE, } diff --git a/src/fixtures/sites.ts b/src/fixtures/sites.ts index 5b282c0b4..cd2aaeab4 100644 --- a/src/fixtures/sites.ts +++ b/src/fixtures/sites.ts @@ -1,2 +1,129 @@ +import { Attributes } from "sequelize/types" + +import { Deployment, Repo, Site, SiteMember } from "@database/models" +import { CollaboratorRoles, JobStatus, SiteStatus } from "@root/constants" + +import { + MOCK_USER_ID_FOUR, + MOCK_USER_ID_ONE, + MOCK_USER_ID_THREE, + MOCK_USER_ID_TWO, +} from "./users" + +export const MOCK_SITE_ID_ONE = 1 +export const MOCK_SITE_ID_TWO = 2 + export const MOCK_SITE_NAME_ONE = "test-site-one" export const MOCK_SITE_NAME_TWO = "test-site-two" + +export const MOCK_SITE_DATE_ONE = new Date("2022-09-23T00:00:00Z") +export const MOCK_SITE_DATE_TWO = new Date("2022-09-25T00:00:00Z") + +export const MOCK_REPO_URL_ONE = "https://github.com/example/repo-one" +export const MOCK_REPO_URL_TWO = "https://github.com/example/repo-two" + +export const MOCK_DEPLOYMENT_PROD_URL_ONE = + "https://master.gibberishone.amplifyapp.com" +export const MOCK_DEPLOYMENT_PROD_URL_TWO = + "https://master.gibberishtwo.amplifyapp.com" + +export const MOCK_DEPLOYMENT_STAGING_URL_ONE = + "https://staging.gibberishone.amplifyapp.com" +export const MOCK_DEPLOYMENT_STAGING_URL_TWO = + "https://staging.gibberishtwo.amplifyapp.com" + +export const MOCK_SITE_DBENTRY_ONE: Attributes = { + id: MOCK_SITE_ID_ONE, + name: MOCK_SITE_NAME_ONE, + apiTokenName: "unused", + siteStatus: SiteStatus.Launched, + jobStatus: JobStatus.Ready, + creatorId: MOCK_USER_ID_ONE, + createdAt: MOCK_SITE_DATE_ONE, + updatedAt: MOCK_SITE_DATE_ONE, +} + +export const MOCK_SITE_DBENTRY_TWO: Attributes = { + id: MOCK_SITE_ID_TWO, + name: MOCK_SITE_NAME_TWO, + apiTokenName: "unused", + siteStatus: SiteStatus.Launched, + jobStatus: JobStatus.Ready, + creatorId: MOCK_USER_ID_TWO, + createdAt: MOCK_SITE_DATE_TWO, + updatedAt: MOCK_SITE_DATE_TWO, +} + +export const MOCK_REPO_DBENTRY_ONE: Attributes = { + id: 1, + name: MOCK_SITE_NAME_ONE, + url: MOCK_REPO_URL_ONE, + siteId: MOCK_SITE_ID_ONE, + createdAt: MOCK_SITE_DATE_ONE, + updatedAt: MOCK_SITE_DATE_ONE, +} + +export const MOCK_REPO_DBENTRY_TWO: Attributes = { + id: 2, + name: MOCK_SITE_NAME_TWO, + url: MOCK_REPO_URL_TWO, + siteId: MOCK_SITE_ID_TWO, + createdAt: MOCK_SITE_DATE_TWO, + updatedAt: MOCK_SITE_DATE_TWO, +} + +export const MOCK_DEPLOYMENT_DBENTRY_ONE: Attributes = { + id: 1, + siteId: MOCK_SITE_ID_ONE, + productionUrl: MOCK_DEPLOYMENT_PROD_URL_ONE, + stagingUrl: MOCK_DEPLOYMENT_STAGING_URL_ONE, + createdAt: MOCK_SITE_DATE_ONE, + updatedAt: MOCK_SITE_DATE_ONE, + hostingId: "1", +} + +export const MOCK_DEPLOYMENT_DBENTRY_TWO: Attributes = { + id: 2, + siteId: MOCK_SITE_ID_TWO, + productionUrl: MOCK_DEPLOYMENT_PROD_URL_TWO, + stagingUrl: MOCK_DEPLOYMENT_STAGING_URL_TWO, + createdAt: MOCK_SITE_DATE_TWO, + updatedAt: MOCK_SITE_DATE_TWO, + hostingId: "1", +} + +export const MOCK_SITEMEMBER_DBENTRY_ONE: Attributes = { + id: 1, + userId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + role: CollaboratorRoles.Admin, + createdAt: MOCK_SITE_DATE_ONE, + updatedAt: MOCK_SITE_DATE_ONE, +} + +export const MOCK_SITEMEMBER_DBENTRY_TWO: Attributes = { + id: 2, + userId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + role: CollaboratorRoles.Contributor, + createdAt: MOCK_SITE_DATE_ONE, + updatedAt: MOCK_SITE_DATE_ONE, +} + +export const MOCK_SITEMEMBER_DBENTRY_THREE: Attributes = { + id: 3, + userId: MOCK_USER_ID_THREE, + siteId: MOCK_SITE_ID_TWO, + role: CollaboratorRoles.Admin, + createdAt: MOCK_SITE_DATE_TWO, + updatedAt: MOCK_SITE_DATE_TWO, +} + +export const MOCK_SITEMEMBER_DBENTRY_FOUR: Attributes = { + id: 4, + userId: MOCK_USER_ID_FOUR, + siteId: MOCK_SITE_ID_TWO, + role: CollaboratorRoles.Contributor, + createdAt: MOCK_SITE_DATE_TWO, + updatedAt: MOCK_SITE_DATE_TWO, +} diff --git a/src/fixtures/users.ts b/src/fixtures/users.ts index 4212b4b34..079c3c966 100644 --- a/src/fixtures/users.ts +++ b/src/fixtures/users.ts @@ -1,3 +1,7 @@ +import { Attributes } from "sequelize/types" + +import { User } from "@database/models" + export const MOCK_USER_ID_ONE = 1 export const MOCK_USER_ID_TWO = 2 export const MOCK_USER_ID_THREE = 3 @@ -7,3 +11,45 @@ export const MOCK_USER_EMAIL_ONE = "one@test.gov.sg" export const MOCK_USER_EMAIL_TWO = "two@test.gov.sg" export const MOCK_USER_EMAIL_THREE = "three@test.gov.sg" export const MOCK_USER_EMAIL_FOUR = "four@test.gov.sg" + +export const MOCK_USER_DATE_ONE = new Date("2022-08-23T00:00:00Z") +export const MOCK_USER_DATE_TWO = new Date("2022-08-25T00:00:00Z") +export const MOCK_USER_DATE_THREE = new Date("2022-08-27T00:00:00Z") +export const MOCK_USER_DATE_FOUR = new Date("2022-08-29T00:00:00Z") + +export const MOCK_USER_LAST_LOGIN_ONE = new Date("2022-09-12T00:00:00Z") +export const MOCK_USER_LAST_LOGIN_TWO = new Date("2022-09-14T00:00:00Z") +export const MOCK_USER_LAST_LOGIN_THREE = new Date("2022-09-16T00:00:00Z") +export const MOCK_USER_LAST_LOGIN_FOUR = new Date("2022-09-18T00:00:00Z") + +export const MOCK_USER_DBENTRY_ONE: Attributes = { + id: MOCK_USER_ID_ONE, + email: MOCK_USER_EMAIL_ONE, + lastLoggedIn: MOCK_USER_LAST_LOGIN_ONE, + createdAt: MOCK_USER_DATE_ONE, + updatedAt: MOCK_USER_DATE_ONE, +} + +export const MOCK_USER_DBENTRY_TWO: Attributes = { + id: MOCK_USER_ID_TWO, + email: MOCK_USER_EMAIL_TWO, + lastLoggedIn: MOCK_USER_LAST_LOGIN_TWO, + createdAt: MOCK_USER_DATE_TWO, + updatedAt: MOCK_USER_DATE_TWO, +} + +export const MOCK_USER_DBENTRY_THREE: Attributes = { + id: MOCK_USER_ID_THREE, + email: MOCK_USER_EMAIL_THREE, + lastLoggedIn: MOCK_USER_LAST_LOGIN_THREE, + createdAt: MOCK_USER_DATE_THREE, + updatedAt: MOCK_USER_DATE_THREE, +} + +export const MOCK_USER_DBENTRY_FOUR: Attributes = { + id: MOCK_USER_ID_FOUR, + email: MOCK_USER_EMAIL_FOUR, + lastLoggedIn: MOCK_USER_LAST_LOGIN_FOUR, + createdAt: MOCK_USER_DATE_FOUR, + updatedAt: MOCK_USER_DATE_FOUR, +} From c3ed0e7d62ea6fe409c3653b9de4663dce6803ba Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 17:46:51 +0800 Subject: [PATCH 04/14] fix: await results from IdentityService --- src/routes/v2/authenticated/review.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 6411433e2..7b48687e3 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -64,7 +64,7 @@ export class ReviewsRouter { const { siteName } = req.params // Check if they have access to site - const possibleSiteMember = this.identityUsersService.getSiteMember( + const possibleSiteMember = await this.identityUsersService.getSiteMember( userWithSiteSessionData.isomerUserId, siteName ) From 971ffb0e06e62598a8367918d2308a3011aaa5ba Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 17:47:12 +0800 Subject: [PATCH 05/14] tests: add partial integration tests for review requests --- src/integration/Reviews.spec.ts | 503 ++++++++++++++++++++++++++++++++ 1 file changed, 503 insertions(+) create mode 100644 src/integration/Reviews.spec.ts diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts new file mode 100644 index 000000000..47040ed21 --- /dev/null +++ b/src/integration/Reviews.spec.ts @@ -0,0 +1,503 @@ +import express from "express" +import mockAxios from "jest-mock-axios" +import request from "supertest" + +import { ReviewsRouter as _ReviewsRouter } from "@routes/v2/authenticated/review" +import { SitesRouter as _SitesRouter } from "@routes/v2/authenticated/sites" + +import { + IsomerAdmin, + Repo, + Reviewer, + ReviewMeta, + ReviewRequest, + ReviewRequestView, + Site, + SiteMember, + User, + Whitelist, +} from "@database/models" +import { generateRouterForUserWithSite } from "@fixtures/app" +import { + MOCK_USER_SESSION_DATA_ONE, + MOCK_USER_SESSION_DATA_THREE, + MOCK_USER_SESSION_DATA_TWO, +} from "@fixtures/sessionData" +import { + MOCK_REPO_DBENTRY_ONE, + MOCK_SITEMEMBER_DBENTRY_ONE, + MOCK_SITEMEMBER_DBENTRY_TWO, + MOCK_SITE_DATE_ONE, + MOCK_SITE_DBENTRY_ONE, + MOCK_SITE_ID_ONE, + MOCK_SITE_NAME_ONE, + MOCK_SITE_NAME_TWO, +} from "@fixtures/sites" +import { + MOCK_USER_DBENTRY_ONE, + MOCK_USER_DBENTRY_THREE, + MOCK_USER_DBENTRY_TWO, + MOCK_USER_EMAIL_ONE, + MOCK_USER_EMAIL_TWO, + MOCK_USER_ID_ONE, + MOCK_USER_ID_TWO, +} from "@fixtures/users" +import { ReviewRequestStatus } from "@root/constants" +import { + MOCK_GITHUB_COMMIT_ALPHA_ONE, + MOCK_GITHUB_COMMIT_ALPHA_THREE, + MOCK_GITHUB_COMMIT_ALPHA_TWO, + MOCK_GITHUB_COMMIT_DATE_THREE, + MOCK_GITHUB_FILENAME_ALPHA_ONE, + MOCK_GITHUB_FILENAME_ALPHA_TWO, + MOCK_GITHUB_FILEPATH_ALPHA_TWO, + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE, + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO, + MOCK_GITHUB_PULL_REQUEST_NUMBER, + MOCK_GITHUB_RAWCOMMENT_ONE, + MOCK_GITHUB_RAWCOMMENT_TWO, +} from "@root/fixtures/github" +import { MOCK_GITHUB_DATE_ONE } from "@root/fixtures/identity" +import { + MOCK_PULL_REQUEST_BODY_ONE, + MOCK_PULL_REQUEST_CHANGED_FILES_ONE, + MOCK_PULL_REQUEST_ONE, + MOCK_PULL_REQUEST_TITLE_ONE, +} from "@root/fixtures/review" +import { GitHubService } from "@services/db/GitHubService" +import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" +import { getUsersService } from "@services/identity" +import CollaboratorsService from "@services/identity/CollaboratorsService" +import IsomerAdminsService from "@services/identity/IsomerAdminsService" +import SitesService from "@services/identity/SitesService" +import ReviewRequestService from "@services/review/ReviewRequestService" +import { sequelize } from "@tests/database" + +const gitHubService = new GitHubService({ axiosInstance: mockAxios.create() }) +const configYmlService = new ConfigYmlService({ gitHubService }) +const usersService = getUsersService(sequelize) +const isomerAdminsService = new IsomerAdminsService({ repository: IsomerAdmin }) +const reviewRequestService = new ReviewRequestService( + gitHubService, + User, + ReviewRequest, + Reviewer, + ReviewMeta, + ReviewRequestView +) +const sitesService = new SitesService({ + siteRepository: Site, + gitHubService, + configYmlService, + usersService, + isomerAdminsService, + reviewRequestService, +}) +const collaboratorsService = new CollaboratorsService({ + siteRepository: Site, + siteMemberRepository: SiteMember, + sitesService, + usersService, + whitelist: Whitelist, +}) + +const ReviewsRouter = new _ReviewsRouter( + reviewRequestService, + usersService, + sitesService, + collaboratorsService +) +const reviewsSubrouter = ReviewsRouter.getRouter() +const subrouter = express() +subrouter.use("/:siteName", reviewsSubrouter) + +const mockGenericAxios = mockAxios.create() + +describe("Review Requests Router", () => { + beforeAll(async () => { + // NOTE: Because SitesService uses an axios instance, + // we need to mock the axios instance using es5 named exports + // to ensure that the calls for .get() on the instance + // will actually return a value and not fail. + jest.mock("../services/api/AxiosInstance.ts", () => ({ + __esModule: true, // this property makes it work + genericGitHubAxiosInstance: mockGenericAxios, + })) + await User.sync({ force: true }) + await Site.sync({ force: true }) + await Repo.sync({ force: true }) + await SiteMember.sync({ force: true }) + + await User.create(MOCK_USER_DBENTRY_ONE) + await User.create(MOCK_USER_DBENTRY_TWO) + await User.create(MOCK_USER_DBENTRY_THREE) + await Site.create(MOCK_SITE_DBENTRY_ONE) + await Repo.create(MOCK_REPO_DBENTRY_ONE) + await SiteMember.create(MOCK_SITEMEMBER_DBENTRY_ONE) + await SiteMember.create(MOCK_SITEMEMBER_DBENTRY_TWO) + }) + + afterAll(async () => { + await SiteMember.destroy({ + where: { + siteId: MOCK_SITE_ID_ONE, + }, + }) + await User.destroy({ + where: { + id: MOCK_USER_ID_ONE, + }, + }) + await User.destroy({ + where: { + id: MOCK_USER_ID_TWO, + }, + }) + await Repo.destroy({ + where: { + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Site.destroy({ + where: { + id: MOCK_SITE_ID_ONE, + }, + }) + }) + + describe("/compare", () => { + it("should get GitHub diff response for a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_SITE_NAME_ONE + ) + mockGenericAxios.get.mockResolvedValueOnce({ + data: { + files: [ + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE, + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO, + ], + commits: [ + MOCK_GITHUB_COMMIT_ALPHA_ONE, + MOCK_GITHUB_COMMIT_ALPHA_TWO, + MOCK_GITHUB_COMMIT_ALPHA_THREE, + ], + }, + }) + const expected = { + items: [ + { + type: ["page"], + name: MOCK_GITHUB_FILENAME_ALPHA_ONE, + path: [], + url: "www.google.com", + lastEditedBy: MOCK_USER_EMAIL_TWO, // TODO: This should be MOCK_USER_EMAIL_ONE + lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + }, + { + type: ["page"], + name: MOCK_GITHUB_FILENAME_ALPHA_TWO, + path: MOCK_GITHUB_FILEPATH_ALPHA_TWO.split("/"), + url: "www.google.com", + lastEditedBy: MOCK_USER_EMAIL_TWO, + lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + }, + ], + } + + // Act + const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/compare`) + + // Assert + expect(actual.statusCode).toEqual(200) + expect(actual.body).toMatchObject(expected) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_SITE_NAME_ONE + ) + + // Act + const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/compare`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/request", () => { + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should successfully create a review request when valid inputs are provided", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_SITE_NAME_ONE + ) + const mockPullRequest = { + reviewers: [MOCK_USER_EMAIL_ONE], + title: "Fake title", + description: "Fake description", + } + const expected = { + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + } + mockGenericAxios.post.mockResolvedValueOnce({ + data: { + number: MOCK_GITHUB_PULL_REQUEST_NUMBER, + }, + }) + + // Act + const actual = await request(app) + .post(`/${MOCK_SITE_NAME_ONE}/request`) + .send(mockPullRequest) + + // Assert + expect(actual.body).toMatchObject(expected) + expect(actual.statusCode).toEqual(200) + const actualReviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }, + }) + const actualReviewer = await Reviewer.findOne({ + where: { + requestId: actualReviewRequest?.id, + reviewerId: MOCK_USER_ID_ONE, + }, + }) + const actualReviewMeta = await ReviewMeta.findOne({ + where: { + reviewId: actualReviewRequest?.id, + }, + }) + expect(actualReviewRequest).not.toBeNull() + expect(actualReviewer).not.toBeNull() + expect(actualReviewMeta).not.toBeNull() + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_SITE_NAME_TWO + ) + const mockPullRequest = { + reviewers: [MOCK_USER_EMAIL_TWO], + title: "Fake title", + description: "Fake description", + } + + // Act + const actual = await request(app) + .post(`/${MOCK_SITE_NAME_TWO}/request`) + .send(mockPullRequest) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_SITE_NAME_ONE + ) + const mockPullRequest = { + reviewers: [MOCK_USER_EMAIL_TWO], + title: "Fake title", + description: "Fake description", + } + + // Act + const actual = await request(app) + .post(`/${MOCK_SITE_NAME_ONE}/request`) + .send(mockPullRequest) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 400 if no reviewers are provided", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_SITE_NAME_ONE + ) + const mockPullRequest = { + reviewers: [], + title: "Fake title", + description: "Fake description", + } + + // Act + const actual = await request(app) + .post(`/${MOCK_SITE_NAME_ONE}/request`) + .send(mockPullRequest) + + // Assert + expect(actual.statusCode).toEqual(400) + }) + + it("should return 400 if selected reviewers are not admins", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_SITE_NAME_ONE + ) + const mockPullRequest = { + reviewers: [MOCK_USER_EMAIL_TWO], + title: "Fake title", + description: "Fake description", + } + + // Act + const actual = await request(app) + .post(`/${MOCK_SITE_NAME_ONE}/request`) + .send(mockPullRequest) + + // Assert + expect(actual.statusCode).toEqual(400) + }) + }) + + describe("/summary", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_SITE_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should get the summary of all existing review requests", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_SITE_NAME_ONE + ) + mockGenericAxios.get.mockResolvedValueOnce({ + data: MOCK_PULL_REQUEST_ONE, + }) + mockGenericAxios.get.mockResolvedValueOnce({ + data: [MOCK_GITHUB_RAWCOMMENT_ONE, MOCK_GITHUB_RAWCOMMENT_TWO], + }) + const expected = { + reviews: [ + { + id: String(MOCK_GITHUB_PULL_REQUEST_NUMBER), + author: MOCK_USER_EMAIL_ONE, + status: ReviewRequestStatus.Open, + title: MOCK_PULL_REQUEST_TITLE_ONE, + description: MOCK_PULL_REQUEST_BODY_ONE, + changedFiles: MOCK_PULL_REQUEST_CHANGED_FILES_ONE, + createdAt: new Date(MOCK_GITHUB_DATE_ONE).getTime(), + newComments: 2, + firstView: true, + }, + ], + } + + // Act + const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/summary`) + + // Assert + expect(actual.statusCode).toEqual(200) + expect(actual.body).toMatchObject(expected) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_SITE_NAME_TWO + ) + + // Act + const actual = await request(app).get(`/${MOCK_SITE_NAME_TWO}/summary`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_SITE_NAME_ONE + ) + + // Act + const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/summary`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/viewed", () => {}) + + describe("/:requestId", () => {}) + + describe("/:requestId/viewed", () => {}) + + describe("/:requestId/merge", () => {}) + + describe("/:requestId/approve", () => {}) + + describe("/:requestId/comments", () => {}) + + describe("/:requestId/comments/viewedComments", () => {}) +}) From d043f837797cfd53cb7b66c6cdab4246a0840b9b Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 18:14:55 +0800 Subject: [PATCH 06/14] chore: switch site name to use repo name instead --- src/fixtures/app.ts | 4 +-- src/fixtures/sites.ts | 15 ++++++---- src/integration/Reviews.spec.ts | 49 ++++++++++++++++----------------- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/fixtures/app.ts b/src/fixtures/app.ts index e3f5dcaa7..ae9ff7c97 100644 --- a/src/fixtures/app.ts +++ b/src/fixtures/app.ts @@ -16,7 +16,7 @@ import { mockGithubSessionData, MOCK_USER_SESSION_DATA_ONE, } from "./sessionData" -import { MOCK_SITE_NAME_ONE } from "./sites" +import { MOCK_REPO_NAME_ONE } from "./sites" /** * @deprecated @@ -93,7 +93,7 @@ const attachDefaultUserSessionDataWithSite: RequestHandler< } > = attachUserSessionDataWithSite( MOCK_USER_SESSION_DATA_ONE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) /** diff --git a/src/fixtures/sites.ts b/src/fixtures/sites.ts index cd2aaeab4..0aa44f3af 100644 --- a/src/fixtures/sites.ts +++ b/src/fixtures/sites.ts @@ -13,12 +13,15 @@ import { export const MOCK_SITE_ID_ONE = 1 export const MOCK_SITE_ID_TWO = 2 -export const MOCK_SITE_NAME_ONE = "test-site-one" -export const MOCK_SITE_NAME_TWO = "test-site-two" +export const MOCK_SITE_NAME_ONE = "Human readable site name one" +export const MOCK_SITE_NAME_TWO = "Human readable site name two" export const MOCK_SITE_DATE_ONE = new Date("2022-09-23T00:00:00Z") export const MOCK_SITE_DATE_TWO = new Date("2022-09-25T00:00:00Z") +export const MOCK_REPO_NAME_ONE = "repo-name-test-one" +export const MOCK_REPO_NAME_TWO = "repo-name-test-two" + export const MOCK_REPO_URL_ONE = "https://github.com/example/repo-one" export const MOCK_REPO_URL_TWO = "https://github.com/example/repo-two" @@ -34,7 +37,7 @@ export const MOCK_DEPLOYMENT_STAGING_URL_TWO = export const MOCK_SITE_DBENTRY_ONE: Attributes = { id: MOCK_SITE_ID_ONE, - name: MOCK_SITE_NAME_ONE, + name: MOCK_REPO_NAME_ONE, apiTokenName: "unused", siteStatus: SiteStatus.Launched, jobStatus: JobStatus.Ready, @@ -45,7 +48,7 @@ export const MOCK_SITE_DBENTRY_ONE: Attributes = { export const MOCK_SITE_DBENTRY_TWO: Attributes = { id: MOCK_SITE_ID_TWO, - name: MOCK_SITE_NAME_TWO, + name: MOCK_REPO_NAME_TWO, apiTokenName: "unused", siteStatus: SiteStatus.Launched, jobStatus: JobStatus.Ready, @@ -56,7 +59,7 @@ export const MOCK_SITE_DBENTRY_TWO: Attributes = { export const MOCK_REPO_DBENTRY_ONE: Attributes = { id: 1, - name: MOCK_SITE_NAME_ONE, + name: MOCK_REPO_NAME_ONE, url: MOCK_REPO_URL_ONE, siteId: MOCK_SITE_ID_ONE, createdAt: MOCK_SITE_DATE_ONE, @@ -65,7 +68,7 @@ export const MOCK_REPO_DBENTRY_ONE: Attributes = { export const MOCK_REPO_DBENTRY_TWO: Attributes = { id: 2, - name: MOCK_SITE_NAME_TWO, + name: MOCK_REPO_NAME_TWO, url: MOCK_REPO_URL_TWO, siteId: MOCK_SITE_ID_TWO, createdAt: MOCK_SITE_DATE_TWO, diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index 47040ed21..a82d2c1dc 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -27,11 +27,10 @@ import { MOCK_REPO_DBENTRY_ONE, MOCK_SITEMEMBER_DBENTRY_ONE, MOCK_SITEMEMBER_DBENTRY_TWO, - MOCK_SITE_DATE_ONE, MOCK_SITE_DBENTRY_ONE, MOCK_SITE_ID_ONE, - MOCK_SITE_NAME_ONE, - MOCK_SITE_NAME_TWO, + MOCK_REPO_NAME_ONE, + MOCK_REPO_NAME_TWO, } from "@fixtures/sites" import { MOCK_USER_DBENTRY_ONE, @@ -171,7 +170,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_ONE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) mockGenericAxios.get.mockResolvedValueOnce({ data: { @@ -199,7 +198,7 @@ describe("Review Requests Router", () => { { type: ["page"], name: MOCK_GITHUB_FILENAME_ALPHA_TWO, - path: MOCK_GITHUB_FILEPATH_ALPHA_TWO.split("/"), + path: MOCK_GITHUB_FILEPATH_ALPHA_TWO.split("/").filter((x) => x), url: "www.google.com", lastEditedBy: MOCK_USER_EMAIL_TWO, lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), @@ -208,7 +207,7 @@ describe("Review Requests Router", () => { } // Act - const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/compare`) + const actual = await request(app).get(`/${MOCK_REPO_NAME_ONE}/compare`) // Assert expect(actual.statusCode).toEqual(200) @@ -220,11 +219,11 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_THREE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) // Act - const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/compare`) + const actual = await request(app).get(`/${MOCK_REPO_NAME_ONE}/compare`) // Assert expect(actual.statusCode).toEqual(404) @@ -249,7 +248,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_TWO, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) const mockPullRequest = { reviewers: [MOCK_USER_EMAIL_ONE], @@ -267,7 +266,7 @@ describe("Review Requests Router", () => { // Act const actual = await request(app) - .post(`/${MOCK_SITE_NAME_ONE}/request`) + .post(`/${MOCK_REPO_NAME_ONE}/request`) .send(mockPullRequest) // Assert @@ -300,7 +299,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_ONE, - MOCK_SITE_NAME_TWO + MOCK_REPO_NAME_TWO ) const mockPullRequest = { reviewers: [MOCK_USER_EMAIL_TWO], @@ -310,7 +309,7 @@ describe("Review Requests Router", () => { // Act const actual = await request(app) - .post(`/${MOCK_SITE_NAME_TWO}/request`) + .post(`/${MOCK_REPO_NAME_TWO}/request`) .send(mockPullRequest) // Assert @@ -322,7 +321,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_THREE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) const mockPullRequest = { reviewers: [MOCK_USER_EMAIL_TWO], @@ -332,7 +331,7 @@ describe("Review Requests Router", () => { // Act const actual = await request(app) - .post(`/${MOCK_SITE_NAME_ONE}/request`) + .post(`/${MOCK_REPO_NAME_ONE}/request`) .send(mockPullRequest) // Assert @@ -344,7 +343,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_TWO, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) const mockPullRequest = { reviewers: [], @@ -354,7 +353,7 @@ describe("Review Requests Router", () => { // Act const actual = await request(app) - .post(`/${MOCK_SITE_NAME_ONE}/request`) + .post(`/${MOCK_REPO_NAME_ONE}/request`) .send(mockPullRequest) // Assert @@ -366,7 +365,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_ONE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) const mockPullRequest = { reviewers: [MOCK_USER_EMAIL_TWO], @@ -376,7 +375,7 @@ describe("Review Requests Router", () => { // Act const actual = await request(app) - .post(`/${MOCK_SITE_NAME_ONE}/request`) + .post(`/${MOCK_REPO_NAME_ONE}/request`) .send(mockPullRequest) // Assert @@ -403,7 +402,7 @@ describe("Review Requests Router", () => { await ReviewMeta.create({ reviewId: reviewRequest?.id, pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, - reviewLink: `cms.isomer.gov.sg/sites/${MOCK_SITE_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, }) }) @@ -424,7 +423,7 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_ONE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) mockGenericAxios.get.mockResolvedValueOnce({ data: MOCK_PULL_REQUEST_ONE, @@ -449,7 +448,7 @@ describe("Review Requests Router", () => { } // Act - const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/summary`) + const actual = await request(app).get(`/${MOCK_REPO_NAME_ONE}/summary`) // Assert expect(actual.statusCode).toEqual(200) @@ -461,11 +460,11 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_ONE, - MOCK_SITE_NAME_TWO + MOCK_REPO_NAME_TWO ) // Act - const actual = await request(app).get(`/${MOCK_SITE_NAME_TWO}/summary`) + const actual = await request(app).get(`/${MOCK_REPO_NAME_TWO}/summary`) // Assert expect(actual.statusCode).toEqual(404) @@ -476,11 +475,11 @@ describe("Review Requests Router", () => { const app = generateRouterForUserWithSite( subrouter, MOCK_USER_SESSION_DATA_THREE, - MOCK_SITE_NAME_ONE + MOCK_REPO_NAME_ONE ) // Act - const actual = await request(app).get(`/${MOCK_SITE_NAME_ONE}/summary`) + const actual = await request(app).get(`/${MOCK_REPO_NAME_ONE}/summary`) // Assert expect(actual.statusCode).toEqual(404) From a027adf938c42c4eff7c80d8ac8290c2fe406186 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 22 Nov 2022 18:15:19 +0800 Subject: [PATCH 07/14] chore: switch to using variable for file name --- src/fixtures/github.ts | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/fixtures/github.ts b/src/fixtures/github.ts index 7710a4d0f..0adefcfb6 100644 --- a/src/fixtures/github.ts +++ b/src/fixtures/github.ts @@ -1,9 +1,4 @@ -import { - Commit, - RawComment, - RawFileChangeInfo, - RawPullRequest, -} from "@root/types/github" +import { Commit, RawComment, RawFileChangeInfo } from "@root/types/github" import { MOCK_USER_ID_ONE, MOCK_USER_ID_TWO } from "./users" @@ -25,8 +20,14 @@ export const MOCK_GITHUB_COMMIT_DATE_TWO = "2022-10-13T05:39:43Z" export const MOCK_GITHUB_COMMIT_DATE_THREE = "2022-11-07T16:32:08Z" export const MOCK_GITHUB_FILENAME_ALPHA_ONE = "index.md" export const MOCK_GITHUB_FILEPATH_ALPHA_ONE = "" +export const MOCK_GITHUB_FULL_FILEPATH_ALPHA_ONE = encodeURIComponent( + MOCK_GITHUB_FILENAME_ALPHA_ONE +) export const MOCK_GITHUB_FILENAME_ALPHA_TWO = "Example Title 22.md" export const MOCK_GITHUB_FILEPATH_ALPHA_TWO = "pages/" +export const MOCK_GITHUB_FULL_FILEPATH_ALPHA_TWO = encodeURIComponent( + MOCK_GITHUB_FILEPATH_ALPHA_TWO + MOCK_GITHUB_FILENAME_ALPHA_TWO +) export const MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_ONE = `Update file: ${MOCK_GITHUB_FILENAME_ALPHA_ONE}` export const MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_TWO = `Update file: ${MOCK_GITHUB_FILENAME_ALPHA_TWO}` export const MOCK_GITHUB_COMMIT_MESSAGE_OBJECT_ALPHA_ONE = { @@ -52,20 +53,20 @@ export const MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE: RawFileChangeInfo = { additions: 1, deletions: 2, changes: 3, - blob_url: `https://github.com/isomerpages/a-test-v4/blob/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/index.md`, - raw_url: `https://github.com/isomerpages/a-test-v4/raw/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/index.md`, - contents_url: `https://api.github.com/repos/isomerpages/a-test-v4/contents/index.md?ref=${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, + blob_url: `https://github.com/isomerpages/a-test-v4/blob/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/${MOCK_GITHUB_FULL_FILEPATH_ALPHA_ONE}`, + raw_url: `https://github.com/isomerpages/a-test-v4/raw/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/${MOCK_GITHUB_FULL_FILEPATH_ALPHA_ONE}`, + contents_url: `https://api.github.com/repos/isomerpages/a-test-v4/contents/${MOCK_GITHUB_FULL_FILEPATH_ALPHA_ONE}?ref=${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, } export const MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO: RawFileChangeInfo = { sha: "f04f18eaa8d31fffc9f8cf5020b1f6a765ac225f", - filename: `${MOCK_GITHUB_FILEPATH_ALPHA_TWO}/${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, + filename: `${MOCK_GITHUB_FILEPATH_ALPHA_TWO}${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, status: "modified", additions: 13, deletions: 2, changes: 15, - blob_url: `https://github.com/isomerpages/a-test-v4/blob/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/pages%2FExample%20Title%2022.md`, - raw_url: `https://github.com/isomerpages/a-test-v4/raw/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/pages%2FExample%20Title%2022.md`, - contents_url: `https://api.github.com/repos/isomerpages/a-test-v4/contents/pages%2FExample%20Title%2022.md?ref=${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, + blob_url: `https://github.com/isomerpages/a-test-v4/blob/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/${MOCK_GITHUB_FULL_FILEPATH_ALPHA_TWO}`, + raw_url: `https://github.com/isomerpages/a-test-v4/raw/${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}/${MOCK_GITHUB_FULL_FILEPATH_ALPHA_TWO}`, + contents_url: `https://api.github.com/repos/isomerpages/a-test-v4/contents/${MOCK_GITHUB_FULL_FILEPATH_ALPHA_TWO}?ref=${MOCK_GITHUB_COMMIT_SHA_LATEST_ALPHA}`, } export const MOCK_GITHUB_COMMIT_ALPHA_ONE: Commit = { From cbad9c797f39af3af663f24659e3220f80426cb3 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 29 Nov 2022 15:20:07 +0800 Subject: [PATCH 08/14] chore: standardise the response codes for non-happy paths --- src/routes/v2/authenticated/review.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 7b48687e3..7d3e61fb3 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -282,7 +282,7 @@ export class ReviewsRouter { ) if (!role) { - return res.status(400).send({ + return res.status(404).send({ message: "User is not a collaborator of this site!", }) } @@ -325,7 +325,7 @@ export class ReviewsRouter { ) if (!role) { - return res.status(400).send({ + return res.status(404).send({ message: "User is not a collaborator of this site!", }) } @@ -783,7 +783,7 @@ export class ReviewsRouter { ) if (!role) { - return res.status(400).send({ + return res.status(404).send({ message: "User is not a collaborator of this site!", }) } From 1bec74e94d45efda14e7b9de1667e01a34c7ee33 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 29 Nov 2022 15:20:42 +0800 Subject: [PATCH 09/14] fix: add additional sanity checks for comments-related features --- src/routes/v2/authenticated/review.ts | 119 +++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 4 deletions(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 7d3e61fb3..96a070ba8 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -725,7 +725,53 @@ export class ReviewsRouter { }) } - // Step 2: Retrieve comments + // Step 2: Check that user exists. + // Having session data is proof that this user exists + // as otherwise, they would be rejected by our middleware + // Check if they are a collaborator + const role = await this.collaboratorsService.getRole( + siteName, + userWithSiteSessionData.isomerUserId + ) + + if (!role) { + logger.error({ + message: "Insufficient permissions to retrieve review request comments", + method: "getComments", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).send({ + message: + "Only collaborators of a site can view review request comments!", + }) + } + + // Step 3: Retrieve review request + const possibleReviewRequest = await this.reviewRequestService.getReviewRequest( + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "getComments", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).json({ message: possibleReviewRequest.message }) + } + + // Step 4: Retrieve comments const comments = await this.reviewRequestService.getComments( userWithSiteSessionData, site, @@ -737,21 +783,86 @@ export class ReviewsRouter { createComment: RequestHandler< { siteName: string; requestId: number }, - string, + ResponseErrorBody, { message: string }, unknown, { userWithSiteSessionData: UserWithSiteSessionData } > = async (req, res) => { - const { requestId } = req.params + const { siteName, requestId } = req.params const { message } = req.body const { userWithSiteSessionData } = res.locals + + // Step 1: Check that the site exists + const site = await this.sitesService.getBySiteName(siteName) + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "createComment", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(404).send({ + message: "Please ensure that the site exists!", + }) + } + + // Step 2: Check that user exists. + // Having session data is proof that this user exists + // as otherwise, they would be rejected by our middleware + // Check if they are a collaborator + const role = await this.collaboratorsService.getRole( + siteName, + userWithSiteSessionData.isomerUserId + ) + + if (!role) { + logger.error({ + message: "Insufficient permissions to retrieve review request comments", + method: "createComment", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).send({ + message: + "Only collaborators of a site can view review request comments!", + }) + } + + // Step 3: Retrieve review request + const possibleReviewRequest = await this.reviewRequestService.getReviewRequest( + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "createComment", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).json({ message: possibleReviewRequest.message }) + } + + // Step 4: Create comment await this.reviewRequestService.createComment( userWithSiteSessionData, requestId, message ) - return res.status(200).send("OK") + return res.status(200).send() } markReviewRequestCommentsAsViewed: RequestHandler< From 4d05e800fbb479a690798787b5634f12a80cd5c2 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 29 Nov 2022 15:21:05 +0800 Subject: [PATCH 10/14] tests: add remaining integration tests for review requests --- src/integration/Reviews.spec.ts | 1383 ++++++++++++++++++++++++++++++- 1 file changed, 1376 insertions(+), 7 deletions(-) diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index a82d2c1dc..01fbc4162 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -31,21 +31,26 @@ import { MOCK_SITE_ID_ONE, MOCK_REPO_NAME_ONE, MOCK_REPO_NAME_TWO, + MOCK_SITE_ID_TWO, } from "@fixtures/sites" import { MOCK_USER_DBENTRY_ONE, MOCK_USER_DBENTRY_THREE, MOCK_USER_DBENTRY_TWO, MOCK_USER_EMAIL_ONE, + MOCK_USER_EMAIL_THREE, MOCK_USER_EMAIL_TWO, MOCK_USER_ID_ONE, MOCK_USER_ID_TWO, } from "@fixtures/users" import { ReviewRequestStatus } from "@root/constants" import { + MOCK_GITHUB_COMMENT_BODY_ONE, + MOCK_GITHUB_COMMENT_BODY_TWO, MOCK_GITHUB_COMMIT_ALPHA_ONE, MOCK_GITHUB_COMMIT_ALPHA_THREE, MOCK_GITHUB_COMMIT_ALPHA_TWO, + MOCK_GITHUB_COMMIT_DATE_ONE, MOCK_GITHUB_COMMIT_DATE_THREE, MOCK_GITHUB_FILENAME_ALPHA_ONE, MOCK_GITHUB_FILENAME_ALPHA_TWO, @@ -63,6 +68,7 @@ import { MOCK_PULL_REQUEST_ONE, MOCK_PULL_REQUEST_TITLE_ONE, } from "@root/fixtures/review" +import { ReviewRequestDto } from "@root/types/dto/review" import { GitHubService } from "@services/db/GitHubService" import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" import { getUsersService } from "@services/identity" @@ -486,17 +492,1380 @@ describe("Review Requests Router", () => { }) }) - describe("/viewed", () => {}) + describe("/viewed", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }) + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_TWO, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await ReviewRequestView.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should mark all existing review requests as viewed for the user", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Pre-requisite checks + const countViews = await ReviewRequestView.count({ + where: { + userId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(countViews).toEqual(0) + const countAnotherUserViews = await ReviewRequestView.count({ + where: { + userId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(countAnotherUserViews).toEqual(0) + + // Act + const actual = await request(app).post(`/${MOCK_REPO_NAME_ONE}/viewed`) + + // Assert + expect(actual.statusCode).toEqual(200) + const countViewsAfter = await ReviewRequestView.count({ + where: { + userId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(countViewsAfter).toEqual(2) + const countAnotherUserViewsAfter = await ReviewRequestView.count({ + where: { + userId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(countAnotherUserViewsAfter).toEqual(0) + const countTotalViewsAfter = await ReviewRequestView.count({ + where: {}, + }) + expect(countTotalViewsAfter).toEqual(2) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).post(`/${MOCK_REPO_NAME_TWO}/viewed`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post(`/${MOCK_REPO_NAME_ONE}/viewed`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId GET", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should return the full details of a review request", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + mockGenericAxios.get.mockResolvedValueOnce({ + data: MOCK_PULL_REQUEST_ONE, + }) + mockGenericAxios.get.mockResolvedValueOnce({ + data: { + files: [ + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE, + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO, + ], + commits: [ + MOCK_GITHUB_COMMIT_ALPHA_ONE, + MOCK_GITHUB_COMMIT_ALPHA_TWO, + MOCK_GITHUB_COMMIT_ALPHA_THREE, + ], + }, + }) + const expected: ReviewRequestDto = { + reviewUrl: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + title: MOCK_PULL_REQUEST_TITLE_ONE, + status: ReviewRequestStatus.Open, + requestor: MOCK_USER_EMAIL_ONE, + reviewers: [MOCK_USER_EMAIL_TWO], + reviewRequestedTime: new Date(MOCK_GITHUB_DATE_ONE).getTime(), + changedItems: [ + { + type: ["page"], + name: MOCK_GITHUB_FILENAME_ALPHA_ONE, + path: [], + url: "www.google.com", + lastEditedBy: MOCK_USER_EMAIL_TWO, // TODO: This should be MOCK_USER_EMAIL_ONE + lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + }, + { + type: ["page"], + name: MOCK_GITHUB_FILENAME_ALPHA_TWO, + path: MOCK_GITHUB_FILEPATH_ALPHA_TWO.split("/").filter((x) => x), + url: "www.google.com", + lastEditedBy: MOCK_USER_EMAIL_TWO, + lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + }, + ], + } + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + expect(actual.body).toEqual({ reviewRequest: expected }) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).get(`/${MOCK_REPO_NAME_ONE}/123456`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId POST", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should update the review request successfully", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Pre-requisite checks + const reviewerCount = await Reviewer.count({ + where: {}, + }) + expect(reviewerCount).toEqual(0) + + // Act + const actual = await request(app) + .post(`/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`) + .send({ + reviewers: [MOCK_USER_EMAIL_ONE], + }) + + // Assert + expect(actual.statusCode).toEqual(200) + const reviewerCountAfter = await Reviewer.count({ + where: {}, + }) + expect(reviewerCountAfter).toEqual(1) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if the review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post(`/${MOCK_REPO_NAME_ONE}/123456`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 403 if user is not the original requestor", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(403) + }) + + it("should return 400 if provided reviewers are not admins of the site", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + 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(400) + }) + }) + + describe("/:requestId DELETE", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewRequestView.create({ + reviewRequestId: reviewRequest?.id, + userId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequestView.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should close the review request successfully", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + mockGenericAxios.patch.mockResolvedValueOnce(null) + + // Act + const actual = await request(app).delete( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + 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_ONE + ) + + // Act + const actual = await request(app).post(`/${MOCK_REPO_NAME_ONE}/123456`) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 403 if user is not the original requestor", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${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 () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}` + ) + + // Assert + expect(actual.statusCode).toEqual(403) + }) + }) + + describe("/:requestId/viewed", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }) + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_TWO, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await ReviewRequestView.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should mark the review request as viewed for the user", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Pre-requisite checks + const countViews = await ReviewRequestView.count({ + where: {}, + }) + expect(countViews).toEqual(0) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/viewed` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + const countViewsAfter = await ReviewRequestView.count({ + where: { + userId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(countViewsAfter).toEqual(1) + const countAnotherUserViewsAfter = await ReviewRequestView.count({ + where: { + userId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(countAnotherUserViewsAfter).toEqual(0) + const countTotalViewsAfter = await ReviewRequestView.count({ + where: {}, + }) + expect(countTotalViewsAfter).toEqual(1) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/viewed` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/viewed` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/123456/viewed` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId/merge", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + await ReviewRequestView.create({ + reviewRequestId: reviewRequest?.id, + siteId: MOCK_SITE_ID_ONE, + userId: MOCK_USER_ID_ONE, + }) + await ReviewRequestView.create({ + reviewRequestId: reviewRequest?.id, + siteId: MOCK_SITE_ID_ONE, + userId: MOCK_USER_ID_TWO, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should merge the pull request successfully", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + mockGenericAxios.post.mockResolvedValueOnce(null) + mockGenericAxios.put.mockResolvedValueOnce(null) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/merge` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(reviewRequest?.reviewStatus).toEqual(ReviewRequestStatus.Merged) + const countViews = await ReviewRequestView.count({ + where: {}, + }) + expect(countViews).toEqual(0) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/merge` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/merge` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/123456/merge` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId/approve POST", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should allow the reviewer to approve the pull request", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(reviewRequest?.reviewStatus).toEqual(ReviewRequestStatus.Approved) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/123456/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 403 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(403) + }) + + it("should return 403 if site member is not a reviewer", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(403) + }) + }) + + describe("/:requestId/approve DELETE", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + reviewStatus: ReviewRequestStatus.Approved, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should allow the reviewer to unapprove the pull request", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).delete( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(reviewRequest?.reviewStatus).toEqual(ReviewRequestStatus.Open) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).delete( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).delete( + `/${MOCK_REPO_NAME_ONE}/123456/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if the user is not a reviewer of the RR", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_ONE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).delete( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if the user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).delete( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId/comments GET", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await Reviewer.create({ + requestId: reviewRequest?.id, + reviewerId: MOCK_USER_ID_TWO, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await Reviewer.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should retrieve the comments for the review request", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + mockGenericAxios.get.mockResolvedValueOnce({ + data: [MOCK_GITHUB_RAWCOMMENT_ONE, MOCK_GITHUB_RAWCOMMENT_TWO], + }) + const expected = [ + { + user: MOCK_USER_EMAIL_ONE, + message: MOCK_GITHUB_COMMENT_BODY_ONE, + createdAt: new Date(MOCK_GITHUB_COMMIT_DATE_ONE).getTime(), + isRead: false, + }, + { + user: MOCK_USER_EMAIL_TWO, + message: MOCK_GITHUB_COMMENT_BODY_TWO, + createdAt: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + isRead: false, + }, + ] + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + expect(actual.body).toEqual(expected) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_TWO + ) + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).get( + `/${MOCK_REPO_NAME_ONE}/123456/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId/comments POST", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + }) + + it("should create a new comment for a review request", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + mockGenericAxios.post.mockResolvedValueOnce(null) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_TWO + ) - describe("/:requestId", () => {}) + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/123456/comments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) + + describe("/:requestId/comments/viewedComments", () => { + beforeAll(async () => { + await ReviewRequest.create({ + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }) + const reviewRequest = await ReviewRequest.findOne({ + where: { + requestorId: MOCK_USER_ID_ONE, + siteId: MOCK_SITE_ID_ONE, + }, + }) + await ReviewMeta.create({ + reviewId: reviewRequest?.id, + pullRequestNumber: MOCK_GITHUB_PULL_REQUEST_NUMBER, + reviewLink: `cms.isomer.gov.sg/sites/${MOCK_REPO_NAME_ONE}/review/${MOCK_GITHUB_PULL_REQUEST_NUMBER}`, + }) + + // Avoid race conditions when checking between expected and actual date values + jest.useFakeTimers("modern") + jest.setSystemTime(new Date(MOCK_GITHUB_COMMIT_DATE_ONE).getTime()) + }) + + afterAll(async () => { + await ReviewMeta.destroy({ + where: {}, + }) + await ReviewRequestView.destroy({ + where: {}, + }) + await ReviewRequest.destroy({ + where: {}, + }) + jest.useRealTimers() + }) + + it("should update last viewed timestamp when the user views the review request", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) + + // Pre-requisite checks + const countViews = await ReviewRequestView.count({ + where: {}, + }) + expect(countViews).toEqual(0) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments/viewedComments` + ) + + // Assert + expect(actual.statusCode).toEqual(200) + const reviewRequestView = await ReviewRequestView.findOne({ + where: { + userId: MOCK_USER_ID_TWO, + siteId: MOCK_SITE_ID_ONE, + }, + }) + expect(reviewRequestView?.lastViewedAt).toEqual( + new Date(MOCK_GITHUB_COMMIT_DATE_ONE) + ) + }) + + it("should return 404 if site is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_TWO + ) - describe("/:requestId/viewed", () => {}) + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_TWO}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments/viewedComments` + ) + + // Assert + expect(actual.statusCode).toEqual(404) + }) + + it("should return 404 if user is not a valid site member", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_THREE, + MOCK_REPO_NAME_ONE + ) + + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/comments/viewedComments` + ) - describe("/:requestId/merge", () => {}) + // Assert + expect(actual.statusCode).toEqual(404) + }) - describe("/:requestId/approve", () => {}) + it("should return 404 if review request is not found", async () => { + // Arrange + const app = generateRouterForUserWithSite( + subrouter, + MOCK_USER_SESSION_DATA_TWO, + MOCK_REPO_NAME_ONE + ) - describe("/:requestId/comments", () => {}) + // Act + const actual = await request(app).post( + `/${MOCK_REPO_NAME_ONE}/123456/comments/viewedComments` + ) - describe("/:requestId/comments/viewedComments", () => {}) + // Assert + expect(actual.statusCode).toEqual(404) + }) + }) }) From 7354e91bd10aa13d45df96bca4c44dd41240b431 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 29 Nov 2022 16:34:58 +0800 Subject: [PATCH 11/14] fix: add methods to generate router for default values --- src/fixtures/app.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/fixtures/app.ts b/src/fixtures/app.ts index ae9ff7c97..96bd6fec3 100644 --- a/src/fixtures/app.ts +++ b/src/fixtures/app.ts @@ -140,3 +140,17 @@ export const generateRouterForUserWithSite = ( app.use(router) return generateFinalRouter(app) } + +export const generateRouterForDefaultUser = (router: Express) => { + const app = express() + app.use(attachDefaultUserSessionData) + app.use(router) + return generateFinalRouter(app) +} + +export const generateRouterForDefaultUserWithSite = (router: Express) => { + const app = express() + app.use(attachDefaultUserSessionDataWithSite) + app.use(router) + return generateFinalRouter(app) +} From ed6af067526e93ea5628d5a587d7d9facb4d4f14 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Fri, 2 Dec 2022 10:25:07 +0800 Subject: [PATCH 12/14] chore: rename test to be more reflective of its purpose --- src/integration/Reviews.spec.ts | 50 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index 01fbc4162..230b60cb9 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -18,6 +18,30 @@ import { Whitelist, } from "@database/models" import { generateRouterForUserWithSite } from "@fixtures/app" +import { + MOCK_GITHUB_COMMENT_BODY_ONE, + MOCK_GITHUB_COMMENT_BODY_TWO, + MOCK_GITHUB_COMMIT_ALPHA_ONE, + MOCK_GITHUB_COMMIT_ALPHA_THREE, + MOCK_GITHUB_COMMIT_ALPHA_TWO, + MOCK_GITHUB_COMMIT_DATE_ONE, + MOCK_GITHUB_COMMIT_DATE_THREE, + MOCK_GITHUB_FILENAME_ALPHA_ONE, + MOCK_GITHUB_FILENAME_ALPHA_TWO, + MOCK_GITHUB_FILEPATH_ALPHA_TWO, + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE, + MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO, + MOCK_GITHUB_PULL_REQUEST_NUMBER, + MOCK_GITHUB_RAWCOMMENT_ONE, + MOCK_GITHUB_RAWCOMMENT_TWO, +} from "@fixtures/github" +import { MOCK_GITHUB_DATE_ONE } from "@fixtures/identity" +import { + MOCK_PULL_REQUEST_BODY_ONE, + MOCK_PULL_REQUEST_CHANGED_FILES_ONE, + MOCK_PULL_REQUEST_ONE, + MOCK_PULL_REQUEST_TITLE_ONE, +} from "@fixtures/review" import { MOCK_USER_SESSION_DATA_ONE, MOCK_USER_SESSION_DATA_THREE, @@ -44,30 +68,6 @@ import { MOCK_USER_ID_TWO, } from "@fixtures/users" import { ReviewRequestStatus } from "@root/constants" -import { - MOCK_GITHUB_COMMENT_BODY_ONE, - MOCK_GITHUB_COMMENT_BODY_TWO, - MOCK_GITHUB_COMMIT_ALPHA_ONE, - MOCK_GITHUB_COMMIT_ALPHA_THREE, - MOCK_GITHUB_COMMIT_ALPHA_TWO, - MOCK_GITHUB_COMMIT_DATE_ONE, - MOCK_GITHUB_COMMIT_DATE_THREE, - MOCK_GITHUB_FILENAME_ALPHA_ONE, - MOCK_GITHUB_FILENAME_ALPHA_TWO, - MOCK_GITHUB_FILEPATH_ALPHA_TWO, - MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_ONE, - MOCK_GITHUB_FILE_CHANGE_INFO_ALPHA_TWO, - MOCK_GITHUB_PULL_REQUEST_NUMBER, - MOCK_GITHUB_RAWCOMMENT_ONE, - MOCK_GITHUB_RAWCOMMENT_TWO, -} from "@root/fixtures/github" -import { MOCK_GITHUB_DATE_ONE } from "@root/fixtures/identity" -import { - MOCK_PULL_REQUEST_BODY_ONE, - MOCK_PULL_REQUEST_CHANGED_FILES_ONE, - MOCK_PULL_REQUEST_ONE, - MOCK_PULL_REQUEST_TITLE_ONE, -} from "@root/fixtures/review" import { ReviewRequestDto } from "@root/types/dto/review" import { GitHubService } from "@services/db/GitHubService" import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" @@ -118,7 +118,7 @@ subrouter.use("/:siteName", reviewsSubrouter) const mockGenericAxios = mockAxios.create() -describe("Review Requests Router", () => { +describe("Review Requests Integration Tests", () => { beforeAll(async () => { // NOTE: Because SitesService uses an axios instance, // we need to mock the axios instance using es5 named exports From f3b8eeaa3a0e5bbfc4059b27aa56c3b58c8d3d08 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Fri, 2 Dec 2022 15:43:07 +0800 Subject: [PATCH 13/14] fix: add missing return and await when updating database --- src/routes/v2/authenticated/review.ts | 2 +- src/services/review/ReviewRequestService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 96a070ba8..2f64d5a46 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -141,7 +141,7 @@ export class ReviewsRouter { // Step 3: Check if reviewers are admins of repo // Check if number of requested reviewers > 0 if (reviewers.length === 0) { - res.status(400).json({ + return res.status(400).json({ message: "Please ensure that you have selected at least 1 reviewer!", }) } diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index 6613a51fb..3a3777311 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -529,7 +529,7 @@ export default class ReviewRequestService { { reviewers }: RequestChangeInfo ) => { // Update db state with new reviewers - reviewRequest.$set("reviewers", reviewers) + await reviewRequest.$set("reviewers", reviewers) await reviewRequest.save() } From f83091b9e023970ce64fb95dd5c512aa122df241 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 6 Dec 2022 16:07:56 +0800 Subject: [PATCH 14/14] chore: add comment on the need for Table.sync --- src/integration/Reviews.spec.ts | 4 ++++ src/integration/Sites.spec.ts | 3 +++ src/integration/Users.spec.ts | 3 +++ 3 files changed, 10 insertions(+) diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index 230b60cb9..8876b6e7e 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -128,6 +128,10 @@ describe("Review Requests Integration Tests", () => { __esModule: true, // this property makes it work genericGitHubAxiosInstance: mockGenericAxios, })) + + // We need to force the relevant tables to start from a clean slate + // Otherwise, some tests may fail due to the auto-incrementing IDs + // not starting from 1 await User.sync({ force: true }) await Site.sync({ force: true }) await Repo.sync({ force: true }) diff --git a/src/integration/Sites.spec.ts b/src/integration/Sites.spec.ts index f616a62c6..8bb86e77e 100644 --- a/src/integration/Sites.spec.ts +++ b/src/integration/Sites.spec.ts @@ -109,6 +109,9 @@ describe("Sites Router", () => { describe("/", () => { beforeAll(async () => { + // We need to force the relevant tables to start from a clean slate + // Otherwise, some tests may fail due to the auto-incrementing IDs + // not starting from 1 await User.sync({ force: true }) await Site.sync({ force: true }) await Repo.sync({ force: true }) diff --git a/src/integration/Users.spec.ts b/src/integration/Users.spec.ts index 326c3bd56..6634c4ad8 100644 --- a/src/integration/Users.spec.ts +++ b/src/integration/Users.spec.ts @@ -56,6 +56,9 @@ const extractMobileOtp = (mobileBody: string): string => describe("Users Router", () => { beforeAll(async () => { + // We need to force the relevant tables to start from a clean slate + // Otherwise, some tests may fail due to the auto-incrementing IDs + // not starting from 1 await User.sync({ force: true }) await Whitelist.sync({ force: true }) })