-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Feat/generic notification creation (#523)
* Feat: move notifications router to authenticated subrouter * Feat: modify create notification arguments * Feat: add notification middleware to handle edit notifications * Fix: allow next on routeHandler * Feat: add creatnotification middleware to authenticatesSitesRouter * Chore: add notification types * Feat: add notification changes when reviews are modified * Fix: swap order of subrouters * Fix: review request router dependencies * Fix: time period for updating notification * Fix: updating createdAt * Fix: tests * Chore: swap to promise.all * fix: rebase errors * Fix: add jsdoc * Feat: add link to notificationHandler * Fix: swap to promise.all and add links * fix(review): fixed triggering event for request approved notif * chore(notifications.spec): added correct ports * Fix: update message on update notification * Fix: notification integration tests * Feat: add generic notification handler tests (#572) * Feat: add generic notification handler tests * Fix: router initialisation and cleanup * fix: update github actions to use runInBand * Fix: update tests * Fix: reset database tables before integration tests * Chore: modify imports * Nit: test comment spelling Co-authored-by: Kishore <[email protected]> * Nit: comments and change var names Co-authored-by: Kishore <[email protected]> Co-authored-by: seaerchin <[email protected]> Co-authored-by: Kishore <[email protected]>
- Loading branch information
1 parent
ac2a5c9
commit 6717bd0
Showing
28 changed files
with
702 additions
and
162 deletions.
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,239 @@ | ||
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 router = express() | ||
const dummySubrouter = express() | ||
dummySubrouter.get("/:siteName/test", async (req, res, next) => | ||
// Dummy subrouter | ||
next() | ||
) | ||
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 | ||
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) | ||
}) | ||
const userSessionData = new UserSessionData({ | ||
isomerUserId: mockIsomerUserId, | ||
email: mockEmail, | ||
}) | ||
const app = generateRouterForUserWithSite(router, 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) | ||
}) | ||
}) | ||
}) |
Oops, something went wrong.