Feat: add generic notification handler tests #572
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| import express from "express" | ||
| import request from "supertest" | ||
|
|
||
| import { NotificationOnEditHandler } from "@middleware/notificationOnEditHandler" | ||
|
|
||
| import UserSessionData from "@classes/UserSessionData" | ||
|
|
||
| import { | ||
| Notification, | ||
| Repo, | ||
| Reviewer, | ||
| ReviewMeta, | ||
| ReviewRequest, | ||
| ReviewRequestView, | ||
| Site, | ||
| SiteMember, | ||
| User, | ||
| Whitelist, | ||
| } from "@database/models" | ||
| import { generateRouterForUserWithSite } from "@fixtures/app" | ||
| import { | ||
| mockEmail, | ||
| mockIsomerUserId, | ||
| mockSiteName, | ||
| } 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" | ||
| const mockSiteMemberId = "1" | ||
|
|
||
| const mockGithubService = { | ||
| getPullRequest: jest.fn(), | ||
| getComments: jest.fn(), | ||
| } | ||
| const usersService = getUsersService(sequelize) | ||
| const reviewRequestService = new ReviewRequestService( | ||
| (mockGithubService as unknown) as typeof ReviewApi, | ||
| 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() | ||
| 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, | ||
alexanderleegs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // 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) | ||
|
There was a problem hiding this comment. nit: await has no effect There was a problem hiding this comment. Hmm it seems to be required in the test, probably because jest seems to conclude the test once the status code is received - I've left a comment explaining this in d6c1b3e |
||
| res.status(200).send(200) | ||
| }) | ||
| const userSessionData = new UserSessionData({ | ||
| isomerUserId: mockIsomerUserId, | ||
| email: mockEmail, | ||
| }) | ||
| const app = generateRouterForUserWithSite( | ||
| subrouter, | ||
| userSessionData, | ||
| mockSiteName | ||
| ) | ||
|
|
||
| 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(), | ||
| }) | ||
|
|
||
| // 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, | ||
| }) | ||
| 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: "mockSite2", | ||
| 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, | ||
| }) | ||
| }) | ||
|
|
||
| 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 | ||
| // 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) | ||
| }) | ||
| }) | ||
| }) | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, is the creation of two routers intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup - this is to simulate how this notification handler functions. The notification handler (represented by
subSubrouter) is inserted into ourauthenticatedSitessubrouter (represented bysubrouterhere), placed after any regular routes (represented by the/:siteName/testendpoint here) - this simulates our existing setup as closely as possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha could we rename this to
notificationHandlerRouterandauthenticatedSitesRouterinstead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops correction on the above explanation -
subSubrouterrepresents a router that the request is routed to and not the notification handler - the notification handler is just inserted in afterwards. As discussed offline - the names of the routers have been changed to make this clearer! - d6c1b3e