From f66dfacacdd132a69a525f4e952536e23eccaa0f Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 16 Nov 2022 16:08:47 +0800 Subject: [PATCH 1/8] Feat: add generic notification handler tests --- .../NotificationOnEditHandler.spec.ts | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 src/integration/NotificationOnEditHandler.spec.ts diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts new file mode 100644 index 000000000..8ad05449b --- /dev/null +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -0,0 +1,221 @@ +import express from "express" +import request from "supertest" + +import { + Notification, + Repo, + Reviewer, + ReviewMeta, + ReviewRequest, + ReviewRequestView, + Site, + SiteMember, + User, + Whitelist, +} from "@database/models" +import { generateRouter } from "@fixtures/app" +import UserSessionData from "@root/classes/UserSessionData" +import { + mockEmail, + mockIsomerUserId, + mockSiteName, +} from "@root/fixtures/sessionData" +import { NotificationOnEditHandler } from "@root/middleware/notificationOnEditHandler" +import { GitHubService } from "@root/services/db/GitHubService" +import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/ConfigYmlService" +import CollaboratorsService from "@root/services/identity/CollaboratorsService" +import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" +import SitesService from "@root/services/identity/SitesService" +import ReviewRequestService from "@root/services/review/ReviewRequestService" +import { getUsersService, notificationsService } from "@services/identity" +import { sequelize } from "@tests/database" + +const mockSiteId = "1" +const mockSiteMemberId = "1" + +const mockGithubService = { + getPullRequest: jest.fn(), + getComments: jest.fn(), +} +const usersService = getUsersService(sequelize) +const reviewRequestService = new ReviewRequestService( + (mockGithubService as unknown) as GitHubService, + User, + ReviewRequest, + Reviewer, + ReviewMeta, + ReviewRequestView +) +const sitesService = new SitesService({ + siteRepository: Site, + gitHubService: (mockGithubService as unknown) as GitHubService, + configYmlService: (jest.fn() as unknown) as ConfigYmlService, + usersService, + isomerAdminsService: (jest.fn() as unknown) as IsomerAdminsService, + reviewRequestService, +}) +const collaboratorsService = new CollaboratorsService({ + siteRepository: Site, + siteMemberRepository: SiteMember, + sitesService, + usersService, + whitelist: Whitelist, +}) + +const notificationsHandler = new NotificationOnEditHandler({ + reviewRequestService, + collaboratorsService, + sitesService, + notificationsService, +}) + +// Set up express with defaults and use the router under test +const subrouter = express() +// As we set certain properties on res.locals when the user signs in using github +// In order to do integration testing, we must expose a middleware +// that allows us to set this properties also +subrouter.use((req, res, next) => { + const userSessionData = new UserSessionData({ + isomerUserId: mockIsomerUserId, + email: mockEmail, + }) + res.locals.userSessionData = userSessionData + next() +}) +const subSubrouter = express() +subSubrouter.get("/:siteName/test", async (req, res, next) => + // Dummy subrouter + next() +) +subrouter.use(subSubrouter) + +// We handle the test slightly diferently - jest interprets the end of the test as when the response is sent, +// but we normally create a notification after this response, due to the position of the middleware +// the solution to get tests working is to send a response only after the notification middleware +subrouter.use(async (req, res, next) => { + await notificationsHandler.createNotification(req as any, res as any, next) + res.status(200).send(200) +}) +const app = generateRouter(subrouter) + +describe("Notifications Router", () => { + const mockAdditionalUserId = "2" + const mockAdditionalSiteId = "2" + const mockAdditionalSiteMemberId = "2" + const mockAnotherSiteMemberId = "3" + + beforeAll(async () => { + // Mock github service return + mockGithubService.getPullRequest.mockResolvedValue({ + title: "title", + body: "body", + changed_files: [], + created_at: new Date(), + }) + // Set up User and Site table entries + await User.create({ + id: mockIsomerUserId, + }) + await User.create({ + id: mockAdditionalUserId, + }) + await Site.create({ + id: mockSiteId, + name: mockSiteName, + apiTokenName: "token", + jobStatus: "READY", + siteStatus: "LAUNCHED", + creatorId: mockIsomerUserId, + }) + await SiteMember.create({ + userId: mockIsomerUserId, + siteId: mockSiteId, + role: "ADMIN", + id: mockSiteMemberId, + }) + await Repo.create({ + name: mockSiteName, + url: "url", + siteId: mockSiteId, + }) + await SiteMember.create({ + userId: mockAdditionalUserId, + siteId: mockSiteId, + role: "ADMIN", + id: mockAdditionalSiteMemberId, + }) + await Site.create({ + id: mockAdditionalSiteId, + name: mockSiteName, + apiTokenName: "token", + jobStatus: "READY", + siteStatus: "LAUNCHED", + creatorId: mockIsomerUserId, + }) + await SiteMember.create({ + userId: mockIsomerUserId, + siteId: mockAdditionalSiteId, + role: "ADMIN", + id: mockAnotherSiteMemberId, + }) + await Repo.create({ + name: `${mockSiteName}2`, + url: "url", + siteId: mockAdditionalSiteId, + }) + }) + describe("createNotification handler", () => { + afterEach(async () => { + // Clean up so that different tests using + // the same notifications don't interfere with each other + await Notification.sync({ force: true }) + await ReviewMeta.sync({ force: true }) + await ReviewRequest.sync({ force: true }) + }) + it("should create a new notification when called", async () => { + // Arrange + await ReviewRequest.create({ + id: 1, + requestorId: mockIsomerUserId, + siteId: mockSiteId, + reviewStatus: "OPEN", + }) + await ReviewMeta.create({ + reviewId: 1, + pullRequestNumber: 1, + reviewLink: "test", + }) + mockGithubService.getComments.mockResolvedValueOnce([]) + + // Act + await request(app).get(`/${mockSiteName}/test`) + + // Assert + // Notification should be sent to all site members other than the creator + expect( + ( + await Notification.findAll({ + where: { + userId: mockAdditionalUserId, + siteId: mockSiteId, + siteMemberId: mockAdditionalSiteMemberId, + firstReadTime: null, + }, + }) + ).length + ).toEqual(1) + expect( + ( + await Notification.findAll({ + where: { + userId: mockIsomerUserId, + siteId: mockSiteId, + siteMemberId: mockSiteMemberId, + firstReadTime: null, + }, + }) + ).length + ).toEqual(0) + }) + }) +}) From a69770d462ba8fd0d0f8e7ac242de7df238c79f1 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 22 Nov 2022 17:52:03 +0800 Subject: [PATCH 2/8] Fix: router initialisation and cleanup --- .../NotificationOnEditHandler.spec.ts | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index 8ad05449b..7948f402c 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -13,7 +13,7 @@ import { User, Whitelist, } from "@database/models" -import { generateRouter } from "@fixtures/app" +import { generateRouterForUserWithSite } from "@fixtures/app" import UserSessionData from "@root/classes/UserSessionData" import { mockEmail, @@ -71,17 +71,6 @@ const notificationsHandler = new NotificationOnEditHandler({ // Set up express with defaults and use the router under test const subrouter = express() -// As we set certain properties on res.locals when the user signs in using github -// In order to do integration testing, we must expose a middleware -// that allows us to set this properties also -subrouter.use((req, res, next) => { - const userSessionData = new UserSessionData({ - isomerUserId: mockIsomerUserId, - email: mockEmail, - }) - res.locals.userSessionData = userSessionData - next() -}) const subSubrouter = express() subSubrouter.get("/:siteName/test", async (req, res, next) => // Dummy subrouter @@ -96,7 +85,15 @@ subrouter.use(async (req, res, next) => { await notificationsHandler.createNotification(req as any, res as any, next) res.status(200).send(200) }) -const app = generateRouter(subrouter) +const userSessionData = new UserSessionData({ + isomerUserId: mockIsomerUserId, + email: mockEmail, +}) +const app = generateRouterForUserWithSite( + subrouter, + userSessionData, + mockSiteName +) describe("Notifications Router", () => { const mockAdditionalUserId = "2" @@ -164,6 +161,14 @@ describe("Notifications Router", () => { siteId: mockAdditionalSiteId, }) }) + + afterAll(async () => { + await SiteMember.sync({ force: true }) + await Site.sync({ force: true }) + await User.sync({ force: true }) + await Repo.sync({ force: true }) + }) + describe("createNotification handler", () => { afterEach(async () => { // Clean up so that different tests using From 2ba4cd45db57e726261f58b0c33745f536aa320b Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 30 Nov 2022 17:41:53 +0800 Subject: [PATCH 3/8] fix: update github actions to use runInBand --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b0bba564c..a15d91b15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,7 +71,7 @@ jobs: key: ${{ runner.OS }}-node-${{ hashFiles('**/package-lock.json') }} - run: npm ci - run: npm run dev:services - - run: . .env.test && npx jest + - run: . .env.test && npx jest --runInBand - run: docker compose down gatekeep: From 893d255f0381e849b23be6e97f8575024c57816c Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 5 Jan 2023 17:11:34 +0800 Subject: [PATCH 4/8] Fix: update tests --- .../NotificationOnEditHandler.spec.ts | 5 +- src/integration/Notifications.spec.ts | 3 +- src/integration/Reviews.spec.ts | 9 ++-- .../v2/authenticated/__tests__/review.spec.ts | 50 +++++++++++++++++-- 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index 7948f402c..ed66a041e 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -27,6 +27,7 @@ import CollaboratorsService from "@root/services/identity/CollaboratorsService" import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" import SitesService from "@root/services/identity/SitesService" import ReviewRequestService from "@root/services/review/ReviewRequestService" +import * as ReviewApi from "@services/db/review" import { getUsersService, notificationsService } from "@services/identity" import { sequelize } from "@tests/database" @@ -39,7 +40,7 @@ const mockGithubService = { } const usersService = getUsersService(sequelize) const reviewRequestService = new ReviewRequestService( - (mockGithubService as unknown) as GitHubService, + (mockGithubService as unknown) as typeof ReviewApi, User, ReviewRequest, Reviewer, @@ -143,7 +144,7 @@ describe("Notifications Router", () => { }) await Site.create({ id: mockAdditionalSiteId, - name: mockSiteName, + name: "mockSite2", apiTokenName: "token", jobStatus: "READY", siteStatus: "LAUNCHED", diff --git a/src/integration/Notifications.spec.ts b/src/integration/Notifications.spec.ts index 2eabb919c..412270a9b 100644 --- a/src/integration/Notifications.spec.ts +++ b/src/integration/Notifications.spec.ts @@ -39,6 +39,7 @@ import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/Co import CollaboratorsService from "@root/services/identity/CollaboratorsService" import SitesService from "@root/services/identity/SitesService" import ReviewRequestService from "@root/services/review/ReviewRequestService" +import * as ReviewApi from "@services/db/review" import { getIdentityAuthService, getUsersService, @@ -58,7 +59,7 @@ const identityAuthService = getIdentityAuthService(gitHubService) const usersService = getUsersService(sequelize) const configYmlService = new ConfigYmlService({ gitHubService }) const reviewRequestService = new ReviewRequestService( - gitHubService, + (gitHubService as unknown) as typeof ReviewApi, User, ReviewRequest, Reviewer, diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index 8876b6e7e..cf5ddf44b 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -7,6 +7,7 @@ import { SitesRouter as _SitesRouter } from "@routes/v2/authenticated/sites" import { IsomerAdmin, + Notification, Repo, Reviewer, ReviewMeta, @@ -70,8 +71,9 @@ import { import { ReviewRequestStatus } from "@root/constants" import { ReviewRequestDto } from "@root/types/dto/review" import { GitHubService } from "@services/db/GitHubService" +import * as ReviewApi from "@services/db/review" import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" -import { getUsersService } from "@services/identity" +import { getUsersService, notificationsService } from "@services/identity" import CollaboratorsService from "@services/identity/CollaboratorsService" import IsomerAdminsService from "@services/identity/IsomerAdminsService" import SitesService from "@services/identity/SitesService" @@ -83,7 +85,7 @@ const configYmlService = new ConfigYmlService({ gitHubService }) const usersService = getUsersService(sequelize) const isomerAdminsService = new IsomerAdminsService({ repository: IsomerAdmin }) const reviewRequestService = new ReviewRequestService( - gitHubService, + (gitHubService as unknown) as typeof ReviewApi, User, ReviewRequest, Reviewer, @@ -110,7 +112,8 @@ const ReviewsRouter = new _ReviewsRouter( reviewRequestService, usersService, sitesService, - collaboratorsService + collaboratorsService, + notificationsService ) const reviewsSubrouter = ReviewsRouter.getRouter() const subrouter = express() diff --git a/src/routes/v2/authenticated/__tests__/review.spec.ts b/src/routes/v2/authenticated/__tests__/review.spec.ts index c860e44a7..602fc9f59 100644 --- a/src/routes/v2/authenticated/__tests__/review.spec.ts +++ b/src/routes/v2/authenticated/__tests__/review.spec.ts @@ -1,15 +1,18 @@ import express from "express" import request from "supertest" +import RequestNotFoundError from "@errors/RequestNotFoundError" + import { attachReadRouteHandlerWrapper } from "@middleware/routeHandler" import { ReviewsRouter as _ReviewsRouter } from "@routes/v2/authenticated/review" import { generateRouterForDefaultUserWithSite } from "@fixtures/app" +import { mockUserId } from "@fixtures/identity" +import { MOCK_USER_EMAIL_ONE, MOCK_USER_EMAIL_TWO } from "@fixtures/users" import { CollaboratorRoles } from "@root/constants" -import RequestNotFoundError from "@root/errors/RequestNotFoundError" -import { MOCK_USER_EMAIL_ONE, MOCK_USER_EMAIL_TWO } from "@root/fixtures/users" import CollaboratorsService from "@services/identity/CollaboratorsService" +import NotificationsService from "@services/identity/NotificationsService" import SitesService from "@services/identity/SitesService" import UsersService from "@services/identity/UsersService" import ReviewRequestService from "@services/review/ReviewRequestService" @@ -48,11 +51,16 @@ describe("Review Requests Router", () => { list: jest.fn(), } + const mockNotificationsService = { + create: jest.fn(), + } + const ReviewsRouter = new _ReviewsRouter( (mockReviewRequestService as unknown) as ReviewRequestService, (mockIdentityUsersService as unknown) as UsersService, (mockSitesService as unknown) as SitesService, - (mockCollaboratorsService as unknown) as CollaboratorsService + (mockCollaboratorsService as unknown) as CollaboratorsService, + (mockNotificationsService as unknown) as NotificationsService ) const subrouter = express() @@ -169,11 +177,13 @@ describe("Review Requests Router", () => { SiteMember: { role: CollaboratorRoles.Admin, }, + id: mockUserId, }, ]) mockReviewRequestService.createReviewRequest.mockResolvedValueOnce( mockPullRequestNumber ) + mockNotificationsService.create.mockResolvedValueOnce([]) // Act const response = await request(app) @@ -196,6 +206,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.createReviewRequest ).toHaveBeenCalledTimes(1) + expect(mockNotificationsService.create).toHaveBeenCalledTimes(1) }) it("should return 404 if the site does not exist", async () => { @@ -216,6 +227,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.createReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 404 if user is not a site collaborator", async () => { @@ -237,6 +249,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.createReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 400 if no reviewers are provided", async () => { @@ -263,6 +276,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.createReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 400 if provided reviewer is not an admin", async () => { @@ -291,6 +305,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.createReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) }) @@ -570,6 +585,7 @@ describe("Review Requests Router", () => { SiteMember: { role: CollaboratorRoles.Admin, }, + id: mockUserId, }, ]) mockReviewRequestService.updateReviewRequest.mockResolvedValueOnce( @@ -773,6 +789,7 @@ describe("Review Requests Router", () => { it("should return 200 with the review request successfully marked as approved", async () => { // Arrange const mockReviewRequest = { reviewers: [{ email: MOCK_USER_EMAIL_ONE }] } + const mockReviewer = "reviewer@test.gov.sg" mockSitesService.getBySiteName.mockResolvedValueOnce("site") mockReviewRequestService.getReviewRequest.mockResolvedValueOnce( mockReviewRequest @@ -780,6 +797,15 @@ describe("Review Requests Router", () => { mockReviewRequestService.approveReviewRequest.mockResolvedValueOnce( undefined ) + mockCollaboratorsService.list.mockResolvedValueOnce([ + { + email: mockReviewer, + SiteMember: { + role: CollaboratorRoles.Admin, + }, + id: mockUserId, + }, + ]) // Act const response = await request(app).post(`/mockSite/review/12345/approve`) @@ -791,6 +817,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.approveReviewRequest ).toHaveBeenCalledTimes(1) + expect(mockNotificationsService.create).toHaveBeenCalledTimes(1) }) it("should return 404 if the site does not exist", async () => { @@ -807,6 +834,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.approveReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 404 if the review request does not exist", async () => { @@ -826,6 +854,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.approveReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 403 if the user is not a reviewer", async () => { @@ -846,6 +875,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.approveReviewRequest ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) }) @@ -1097,6 +1127,7 @@ describe("Review Requests Router", () => { it("should return 200 with the review request closed successfully", async () => { // Arrange const mockReviewRequest = { requestor: { email: MOCK_USER_EMAIL_ONE } } + const mockReviewer = "reviewer@test.gov.sg" mockSitesService.getBySiteName.mockResolvedValueOnce("site") mockReviewRequestService.getReviewRequest.mockResolvedValueOnce( mockReviewRequest @@ -1107,6 +1138,15 @@ describe("Review Requests Router", () => { mockReviewRequestService.deleteAllReviewRequestViews.mockResolvedValueOnce( undefined ) + mockCollaboratorsService.list.mockResolvedValueOnce([ + { + email: mockReviewer, + SiteMember: { + role: CollaboratorRoles.Admin, + }, + id: mockUserId, + }, + ]) // Act const response = await request(app).delete(`/mockSite/review/12345`) @@ -1121,6 +1161,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.deleteAllReviewRequestViews ).toHaveBeenCalledTimes(1) + expect(mockNotificationsService.create).toHaveBeenCalledTimes(1) }) it("should return 404 if the site does not exist", async () => { @@ -1138,6 +1179,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.deleteAllReviewRequestViews ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 404 if the review request does not exist", async () => { @@ -1158,6 +1200,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.deleteAllReviewRequestViews ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) it("should return 404 if the user is not the requestor of the review request", async () => { @@ -1179,6 +1222,7 @@ describe("Review Requests Router", () => { expect( mockReviewRequestService.deleteAllReviewRequestViews ).not.toHaveBeenCalled() + expect(mockNotificationsService.create).not.toHaveBeenCalled() }) }) From 5e865d24e14ea1e921efa300d6c6fb9a127faba5 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 5 Jan 2023 17:12:05 +0800 Subject: [PATCH 5/8] Fix: reset database tables before integration tests --- src/integration/NotificationOnEditHandler.spec.ts | 12 ++++++++++++ src/integration/Notifications.spec.ts | 9 +++++++++ src/integration/Reviews.spec.ts | 2 ++ 3 files changed, 23 insertions(+) diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index ed66a041e..cbebc2633 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -110,6 +110,18 @@ describe("Notifications Router", () => { changed_files: [], created_at: new Date(), }) + + // 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 }) + await SiteMember.sync({ force: true }) + await Notification.sync({ force: true }) + await ReviewMeta.sync({ force: true }) + await ReviewRequest.sync({ force: true }) + // Set up User and Site table entries await User.create({ id: mockIsomerUserId, diff --git a/src/integration/Notifications.spec.ts b/src/integration/Notifications.spec.ts index 412270a9b..79b822be5 100644 --- a/src/integration/Notifications.spec.ts +++ b/src/integration/Notifications.spec.ts @@ -110,6 +110,15 @@ describe("Notifications Router", () => { const MOCK_ANOTHER_SITE_MEMBER_ID = "3" 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 }) + await SiteMember.sync({ force: true }) + await Notification.sync({ force: true }) + // Set up User and Site table entries await User.create({ id: mockIsomerUserId, diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index cf5ddf44b..7179bafef 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -139,6 +139,8 @@ describe("Review Requests Integration Tests", () => { await Site.sync({ force: true }) await Repo.sync({ force: true }) await SiteMember.sync({ force: true }) + await Notification.sync({ force: true }) + await ReviewMeta.sync({ force: true }) await User.create(MOCK_USER_DBENTRY_ONE) await User.create(MOCK_USER_DBENTRY_TWO) From a312155119e01da5fa108c602ea02295baa1a2cf Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 5 Jan 2023 17:14:53 +0800 Subject: [PATCH 6/8] Chore: modify imports --- .../NotificationOnEditHandler.spec.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index cbebc2633..bffe9606a 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -1,6 +1,10 @@ import express from "express" import request from "supertest" +import { NotificationOnEditHandler } from "@middleware/notificationOnEditHandler" + +import UserSessionData from "@classes/UserSessionData" + import { Notification, Repo, @@ -14,21 +18,19 @@ import { Whitelist, } from "@database/models" import { generateRouterForUserWithSite } from "@fixtures/app" -import UserSessionData from "@root/classes/UserSessionData" import { mockEmail, mockIsomerUserId, mockSiteName, -} from "@root/fixtures/sessionData" -import { NotificationOnEditHandler } from "@root/middleware/notificationOnEditHandler" -import { GitHubService } from "@root/services/db/GitHubService" -import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/ConfigYmlService" -import CollaboratorsService from "@root/services/identity/CollaboratorsService" -import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" -import SitesService from "@root/services/identity/SitesService" -import ReviewRequestService from "@root/services/review/ReviewRequestService" +} from "@fixtures/sessionData" +import { GitHubService } from "@services/db/GitHubService" import * as ReviewApi from "@services/db/review" +import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" import { getUsersService, notificationsService } 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 mockSiteId = "1" From f717000dfe632128ae50640edd57cb3ea608d2ea Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Mon, 9 Jan 2023 10:08:43 +0800 Subject: [PATCH 7/8] Nit: test comment spelling Co-authored-by: Kishore <42832651+kishore03109@users.noreply.github.com> --- src/integration/NotificationOnEditHandler.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index bffe9606a..917052a42 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -81,7 +81,7 @@ subSubrouter.get("/:siteName/test", async (req, res, next) => ) subrouter.use(subSubrouter) -// We handle the test slightly diferently - jest interprets the end of the test as when the response is sent, +// We handle the test slightly differently - jest interprets the end of the test as when the response is sent, // but we normally create a notification after this response, due to the position of the middleware // the solution to get tests working is to send a response only after the notification middleware subrouter.use(async (req, res, next) => { From d6c1b3ea7776f186aa2a0391bf8357a87c5c6028 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Wed, 11 Jan 2023 17:44:14 +0800 Subject: [PATCH 8/8] Nit: comments and change var names --- .../NotificationOnEditHandler.spec.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index 917052a42..24d208925 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -73,18 +73,20 @@ const notificationsHandler = new NotificationOnEditHandler({ }) // Set up express with defaults and use the router under test -const subrouter = express() -const subSubrouter = express() -subSubrouter.get("/:siteName/test", async (req, res, next) => +const router = express() +const dummySubrouter = express() +dummySubrouter.get("/:siteName/test", async (req, res, next) => // Dummy subrouter next() ) -subrouter.use(subSubrouter) +router.use(dummySubrouter) // We handle the test slightly differently - jest interprets the end of the test as when the response is sent, // but we normally create a notification after this response, due to the position of the middleware // the solution to get tests working is to send a response only after the notification middleware -subrouter.use(async (req, res, next) => { +router.use(async (req, res, next) => { + // Inserts notification handler after all other subrouters + // Needs to be awaited so jest doesn't exit prematurely upon receiving response status await notificationsHandler.createNotification(req as any, res as any, next) res.status(200).send(200) }) @@ -92,11 +94,7 @@ const userSessionData = new UserSessionData({ isomerUserId: mockIsomerUserId, email: mockEmail, }) -const app = generateRouterForUserWithSite( - subrouter, - userSessionData, - mockSiteName -) +const app = generateRouterForUserWithSite(router, userSessionData, mockSiteName) describe("Notifications Router", () => { const mockAdditionalUserId = "2"