From b93e81f450d32095ab946ae3d5f07daf60e1f211 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Fri, 21 Oct 2022 15:30:14 +0800 Subject: [PATCH] feat(rr): services + routes (#519) * feat(services): add initial services for rr * feat(types): add github types * feat(reviewrequestservice): add features * chore(review): wip for review routes * feat(types): add new types * feat(reviewrequestservice): add impl for computing sha mappings * feat(usersservice): add nwe method to user service to retrieve site admins * feat(review): add route for creating review request * refactor(collaboratorsservice): refactor method api for clarity * feat(types): add more types * refactor(collaborators): fix typings and add more steuff to return * chore(routes): update authenticated routes * chore(review): refactor to use collaborators service * refactor(reviewrequestservice): update methods * chore(server): add init code * refactor(reviewmeta): updat eto use belongs to * feat(types): add more types * feat(requestnotfounderror): add new error type * feat(review): add methods for listing review requests and retrieval of rr * refactor(types/dto): update review types * refactor(rrservice): update enum type * feat(rrservice): add new method to merge rr * feat(review): add new route to merge rr * fix(collaborators): remove erroneous destructuring * fix(routes/review): add siteId prop * chore(review dto): add status * chore(requestrequestservice): remove old comment * fix(reviewrequestservice): changed some stuff to be optional * refactor(rr service): split retrieving db/github view into 2 methods * feat(rr service): add methods to close/approve rr * refactor(rr service): refactor merge rr method * chore(collaboratorsservice): remove extra typecasts * feat(rr): add new endpoint to update rr * chore(types): minor cleanup * feat(rr routes): add new routes for close and approve pr * chore(review): update to userwithsitesesiontoken * refactor(reviewrequestservice): migrate api calls into own file * refactor(authenticated): shift review router dpes to init function * fix(index): fixed faulty init * refactor(reviewrequestservice): add site to reviewreq object * fix(review.ts): add explicit bearer token to api call * refactor(rrservice): refactor to retrieve user from db * chore(settingsservice): remove extra console log * chore(github): remove extra `patch` property * chore(review): add logging * fix(server): update imports from rebase * Chore: Update src/routes/v2/authenticated/review.ts Co-authored-by: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> * Chore: Update src/routes/v2/authenticated/review.ts Co-authored-by: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> * feat(rr status): add rr status enum * chore(rr): update db model * refactor(rrservice): updat to use enum * chore(rrservice): fix commennt * chore(review): update error codes * chore(usersservice): rename hasAccess to getSiteMember * chore(usersservice): update method name * feat(rr): allow updating of admins (#539) * chore(server): add init code * chore(dto): removed trailing space on folder name * refactor(reviewrequestservice): update to remove title/desc from update api * chore(collaborators): update import * chore(routes/review): updaterr api Co-authored-by: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> --- src/constants/constants.ts | 7 + src/database/models/ReviewMeta.ts | 5 +- src/database/models/ReviewRequest.ts | 5 +- src/database/models/Reviewers.ts | 10 +- src/errors/RequestNotFoundError.ts | 10 + src/routes/v2/authenticated/collaborators.ts | 12 +- src/routes/v2/authenticated/index.js | 11 +- src/routes/v2/authenticated/review.ts | 697 ++++++++++++++++++ src/server.js | 18 + .../configServices/SettingsService.js | 1 - src/services/db/GitHubService.js | 39 +- src/services/db/review.ts | 85 +++ src/services/identity/CollaboratorsService.ts | 26 +- src/services/identity/UsersService.ts | 28 +- .../AuthorizationMiddlewareService.ts | 5 +- src/services/review/ReviewRequestService.ts | 347 +++++++++ src/types/dto/error.ts | 3 + src/types/dto/review.ts | 46 ++ src/types/error.ts | 5 + src/types/github.ts | 83 +++ src/types/request.ts | 2 +- src/types/review.ts | 15 + 22 files changed, 1415 insertions(+), 45 deletions(-) create mode 100644 src/errors/RequestNotFoundError.ts create mode 100644 src/routes/v2/authenticated/review.ts create mode 100644 src/services/db/review.ts create mode 100644 src/services/review/ReviewRequestService.ts create mode 100644 src/types/dto/error.ts create mode 100644 src/types/dto/review.ts create mode 100644 src/types/github.ts create mode 100644 src/types/review.ts diff --git a/src/constants/constants.ts b/src/constants/constants.ts index bf3d68414..4e1ead50d 100644 --- a/src/constants/constants.ts +++ b/src/constants/constants.ts @@ -20,6 +20,13 @@ export enum CollaboratorRoles { Contributor = "CONTRIBUTOR", } +export enum ReviewRequestStatus { + Approved = "APPROVED", + Open = "OPEN", + Merged = "MERGED", + Closed = "CLOSED", +} + export const E2E_ISOMER_ID = "-1" export const E2E_TEST_EMAIL = "test@e2e" export const E2E_TEST_CONTACT = "12345678" diff --git a/src/database/models/ReviewMeta.ts b/src/database/models/ReviewMeta.ts index 58d2eda89..84212e68d 100644 --- a/src/database/models/ReviewMeta.ts +++ b/src/database/models/ReviewMeta.ts @@ -4,6 +4,7 @@ import { Model, Table, ForeignKey, + BelongsTo, } from "sequelize-typescript" import { ReviewRequest } from "./ReviewRequest" @@ -24,9 +25,11 @@ export class ReviewMeta extends Model { reviewerId!: number @ForeignKey(() => ReviewRequest) - @Column reviewId!: number + @BelongsTo(() => ReviewRequest) + reviewRequest!: ReviewRequest + @Column({ allowNull: false, type: DataType.BIGINT, diff --git a/src/database/models/ReviewRequest.ts b/src/database/models/ReviewRequest.ts index fac7c01d1..22bd5b737 100644 --- a/src/database/models/ReviewRequest.ts +++ b/src/database/models/ReviewRequest.ts @@ -11,6 +11,7 @@ import { import { Site } from "@database/models/Site" import { User } from "@database/models/User" +import { ReviewRequestStatus } from "@root/constants" import { Reviewer } from "./Reviewers" import { ReviewMeta } from "./ReviewMeta" @@ -54,9 +55,9 @@ export class ReviewRequest extends Model { @Column({ allowNull: false, defaultValue: "OPEN", - type: DataType.ENUM("OPEN", "APPROVED", "MERGED", "CLOSED"), + type: DataType.ENUM(...Object.values(ReviewRequestStatus)), }) - reviewStatus!: "OPEN" | "APPROVED" | "MERGED" | "CLOSED" + reviewStatus!: ReviewRequestStatus @BelongsToMany(() => User, { onUpdate: "CASCADE", diff --git a/src/database/models/Reviewers.ts b/src/database/models/Reviewers.ts index 357fb6fe6..198ec3a82 100644 --- a/src/database/models/Reviewers.ts +++ b/src/database/models/Reviewers.ts @@ -1,12 +1,4 @@ -import { - Column, - CreatedAt, - DataType, - ForeignKey, - Model, - Table, - UpdatedAt, -} from "sequelize-typescript" +import { Column, ForeignKey, Model, Table } from "sequelize-typescript" import { User } from "@database/models/User" diff --git a/src/errors/RequestNotFoundError.ts b/src/errors/RequestNotFoundError.ts new file mode 100644 index 000000000..6dcd7513b --- /dev/null +++ b/src/errors/RequestNotFoundError.ts @@ -0,0 +1,10 @@ +import { NotFoundError } from "./NotFoundError" + +export default class RequestNotFoundError extends NotFoundError { + constructor(message = "The specified review request could not be found!") { + super() + Error.captureStackTrace(this, this.constructor) + this.name = this.constructor.name + this.message = message + } +} diff --git a/src/routes/v2/authenticated/collaborators.ts b/src/routes/v2/authenticated/collaborators.ts index f19df674a..c2e7b386f 100644 --- a/src/routes/v2/authenticated/collaborators.ts +++ b/src/routes/v2/authenticated/collaborators.ts @@ -9,6 +9,7 @@ import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" import { BaseIsomerError } from "@root/errors/BaseError" import { attachSiteHandler } from "@root/middleware" import { RequestHandler } from "@root/types" +import { UserDto } from "@root/types/dto/review" import CollaboratorsService from "@services/identity/CollaboratorsService" interface CollaboratorsRouterProps { @@ -71,18 +72,23 @@ export class CollaboratorsRouter { } listCollaborators: RequestHandler< + { siteName: string }, + { collaborators: UserDto[] }, never, unknown, - never, - { siteName: string }, { userWithSiteSessionData: UserWithSiteSessionData } > = async (req, res) => { const { siteName } = req.params const { userWithSiteSessionData } = res.locals - const collaborators = await this.collaboratorsService.list( + const rawCollaborators = await this.collaboratorsService.list( siteName, userWithSiteSessionData.isomerUserId ) + const collaborators = rawCollaborators.map((collaborator) => ({ + ...collaborator.toJSON(), + email: collaborator.email || "", + role: collaborator.SiteMember.role, + })) return res.status(200).json({ collaborators }) } diff --git a/src/routes/v2/authenticated/index.js b/src/routes/v2/authenticated/index.js index 69b5ad44c..d3015f411 100644 --- a/src/routes/v2/authenticated/index.js +++ b/src/routes/v2/authenticated/index.js @@ -1,3 +1,5 @@ +import { attachSiteHandler } from "@root/middleware" + const express = require("express") const { @@ -17,6 +19,7 @@ const getAuthenticatedSubrouter = ({ isomerAdminsService, collaboratorsService, authorizationMiddleware, + reviewRouter, }) => { const netlifyTomlService = new NetlifyTomlService() @@ -42,7 +45,13 @@ const getAuthenticatedSubrouter = ({ "/sites/:siteName/collaborators", collaboratorsRouter.getRouter() ) - authenticatedSubrouter.use("/sites", sitesV2Router.getRouter()) + const baseSitesV2Router = sitesV2Router.getRouter() + const sitesRouterWithReviewRequest = baseSitesV2Router.use( + "/:siteName/review", + attachSiteHandler, + reviewRouter.getRouter() + ) + authenticatedSubrouter.use("/sites", sitesRouterWithReviewRequest) authenticatedSubrouter.use("/user", usersRouter.getRouter()) authenticatedSubrouter.use("/netlify-toml", netlifyTomlV2Router.getRouter()) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts new file mode 100644 index 000000000..2e146e496 --- /dev/null +++ b/src/routes/v2/authenticated/review.ts @@ -0,0 +1,697 @@ +import autoBind from "auto-bind" +import express from "express" +import _ from "lodash" + +import logger from "@logger/logger" + +import { + attachReadRouteHandlerWrapper, + attachWriteRouteHandlerWrapper, +} from "@middleware/routeHandler" + +import UserSessionData from "@classes/UserSessionData" +import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" + +import { CollaboratorRoles } from "@root/constants" +import CollaboratorsService from "@root/services/identity/CollaboratorsService" +import SitesService from "@root/services/identity/SitesService" +import UsersService from "@root/services/identity/UsersService" +import { isIsomerError, RequestHandler } from "@root/types" +import { ResponseErrorBody } from "@root/types/dto/error" +import { + DashboardReviewRequestDto, + EditedItemDto, + UpdateReviewRequestDto, + ReviewRequestDto, +} from "@root/types/dto/review" +import ReviewRequestService from "@services/review/ReviewRequestService" +// eslint-disable-next-line import/prefer-default-export +export class ReviewsRouter { + private readonly reviewRequestService + + private readonly identityUsersService + + private readonly sitesService + + private readonly collaboratorsService + + constructor( + reviewRequestService: ReviewRequestService, + identityUsersService: UsersService, + sitesService: SitesService, + collaboratorsService: CollaboratorsService + ) { + this.reviewRequestService = reviewRequestService + this.identityUsersService = identityUsersService + this.sitesService = sitesService + this.collaboratorsService = collaboratorsService + + autoBind(this) + } + + compareDiff: RequestHandler< + { siteName: string }, + { items: EditedItemDto[] }, + unknown, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that user exists. + // Having session data is proof that this user exists + // as otherwise, they would be rejected by our middleware + const { userWithSiteSessionData } = res.locals + const { siteName } = req.params + + // Check if they have access to site + const possibleSiteMember = this.identityUsersService.getSiteMember( + userWithSiteSessionData.isomerUserId, + siteName + ) + + if (!possibleSiteMember) { + return res.status(404).send() + } + + const files = await this.reviewRequestService.compareDiff( + userWithSiteSessionData + ) + + return res.status(200).json({ items: files }) + } + + createReviewRequest: RequestHandler< + { siteName: string }, + { pullRequestNumber: number } | ResponseErrorBody, + { reviewers: string[]; title: string; description: string }, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName } = req.params + const site = await this.sitesService.getBySiteName(siteName) + const { userWithSiteSessionData } = res.locals + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "createReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(404).send({ + message: + "Please ensure that the site you are requesting a review for 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 site admin + const role = await this.collaboratorsService.getRole( + siteName, + userWithSiteSessionData.isomerUserId + ) + + if (!role || role !== CollaboratorRoles.Admin) { + logger.error({ + message: + "User attempted to create review request with invalid permissions", + method: "createReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(404).send({ + message: "Only admins can request reviews!", + }) + } + + const admin = await this.identityUsersService.findByEmail( + userWithSiteSessionData.email + ) + const { reviewers, title, description } = req.body + + // Step 3: Check if reviewers are admins of repo + // Check if number of requested reviewers > 0 + if (reviewers.length === 0) { + res.status(400).json({ + message: "Please ensure that you have selected at least 1 reviewer!", + }) + } + const reviewersMap: Record = {} + + // May we repent for writing such code in production. + reviewers.forEach((email) => { + reviewersMap[email] = true + }) + + const collaborators = await this.collaboratorsService.list( + siteName, + userWithSiteSessionData.isomerUserId + ) + + // Filter to get admins, + // then ensure that they have been requested for review + const admins = collaborators + .filter( + (collaborator) => + collaborator.SiteMember.role === CollaboratorRoles.Admin + ) + .filter((collaborator) => reviewersMap[collaborator.email || ""]) + + const areAllReviewersAdmin = admins.length === reviewers.length + if (!areAllReviewersAdmin) { + return res.status(400).send({ + message: "Please ensure that all requested reviewers are admins!", + }) + } + + // Step 4: Create RR + const pullRequestNumber = await this.reviewRequestService.createReviewRequest( + userWithSiteSessionData, + admins, + // NOTE: Safe assertion as we first retrieve the role + // and assert that the user is an admin of said site. + // This guarantees that the user exists in our database. + admin!, + site, + title, + description + ) + + return res.status(200).send({ + pullRequestNumber, + }) + } + + listReviews: RequestHandler< + { siteName: string }, + { reviews: DashboardReviewRequestDto[] } | ResponseErrorBody, + never, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName } = req.params + const site = await this.sitesService.getBySiteName(siteName) + const { userWithSiteSessionData } = res.locals + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "listReviews", + 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 view review request", + method: "listReviews", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(404).send({ + message: "Only collaborators of a site can view reviews!", + }) + } + + // Step 3: Fetch data and return + const reviews = await this.reviewRequestService.listReviewRequest( + userWithSiteSessionData, + site + ) + + return res.status(200).json({ + reviews, + }) + } + + getReviewRequest: RequestHandler< + { siteName: string; requestId: number }, + { reviewRequest: ReviewRequestDto } | ResponseErrorBody, + never, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName, requestId } = req.params + const { userWithSiteSessionData } = res.locals + const site = await this.sitesService.getBySiteName(siteName) + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "getReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + 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", + method: "getReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).send({ + message: "Only collaborators of a site can view reviews!", + }) + } + + const possibleReviewRequest = await this.reviewRequestService.getFullReviewRequest( + userWithSiteSessionData, + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "getReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(possibleReviewRequest.status).send({ + message: possibleReviewRequest.message, + }) + } + + return res.status(200).json({ reviewRequest: possibleReviewRequest }) + } + + updateReviewRequest: RequestHandler< + { siteName: string; requestId: number }, + ResponseErrorBody, + UpdateReviewRequestDto, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName, requestId } = req.params + const { userWithSiteSessionData } = res.locals + const site = await this.sitesService.getBySiteName(siteName) + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "updateReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).send({ + message: "Please ensure that the site exists!", + }) + } + + // Step 2: Retrieve review request + const possibleReviewRequest = await this.reviewRequestService.getReviewRequest( + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "updateReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).json({ message: possibleReviewRequest.message }) + } + + // Step 3: Check that the user updating is the requestor + const { requestor } = possibleReviewRequest + if (requestor.email !== userWithSiteSessionData.email) { + logger.error({ + message: "Insufficient permissions to update review request", + method: "updateReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(403).json({ + message: "Only requestors can update the review request!", + }) + } + + // Step 4: Check that all new reviewers are admins of the site + const { reviewers } = req.body + const collaborators = await this.collaboratorsService.list(siteName) + const collaboratorMappings = Object.fromEntries( + reviewers.map((reviewer) => [reviewer, true]) + ) + const verifiedReviewers = collaborators.filter( + (collaborator) => + collaborator.SiteMember.role === CollaboratorRoles.Admin && + // NOTE: We check for existence of email on the user - since this + // is an identity feature, we assume that **all** users calling this endpoint + // will have a valid email (guaranteed by our modal) + collaborator.email && + !!collaboratorMappings[collaborator.email] + ) + + if (verifiedReviewers.length !== reviewers.length) { + return res.status(400).json({ + message: + "Please ensure that all requested reviewers are admins of the site!", + }) + } + + // Step 5: Update the rr with the appropriate details + await this.reviewRequestService.updateReviewRequest(possibleReviewRequest, { + reviewers: verifiedReviewers, + }) + + return res.status(200).send() + } + + mergeReviewRequest: RequestHandler< + { siteName: string; requestId: number }, + ResponseErrorBody, + never, + unknown, + { userSessionData: UserSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName, requestId } = req.params + const { userSessionData } = res.locals + const site = await this.sitesService.getBySiteName(siteName) + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "mergeReviewRequest", + meta: { + userId: userSessionData.isomerUserId, + email: userSessionData.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, + userSessionData.isomerUserId + ) + + if (!role) { + logger.error({ + message: "Insufficient permissions to merge review request", + method: "mergeReviewRequest", + meta: { + userId: userSessionData.isomerUserId, + email: userSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).send({ + message: "Only collaborators of a site can view reviews!", + }) + } + + // Step 3: Retrieve review request + const possibleReviewRequest = await this.reviewRequestService.getReviewRequest( + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "mergeReviewRequest", + meta: { + userId: userSessionData.isomerUserId, + email: userSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).json({ message: possibleReviewRequest.message }) + } + + // Step 4: Merge review request + // NOTE: We are not checking for existence of PR + // as the underlying Github API returns 404 if + // the requested review could not be found. + await this.reviewRequestService.mergeReviewRequest(possibleReviewRequest) + return res.status(200).send() + } + + approveReviewRequest: RequestHandler< + { siteName: string; requestId: number }, + ResponseErrorBody, + never, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName, requestId } = req.params + const { userWithSiteSessionData } = res.locals + const site = await this.sitesService.getBySiteName(siteName) + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "approveReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(404).send({ + message: "Please ensure that the site exists!", + }) + } + + // Step 3: Retrieve review request + const possibleReviewRequest = await this.reviewRequestService.getReviewRequest( + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "approveReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).send({ + message: "Please ensure that the site exists!", + }) + } + + // Step 4: Check if the user is a reviewer of the RR + const { reviewers } = possibleReviewRequest + const isReviewer = _.some( + reviewers, + (user) => user.email === userWithSiteSessionData.email + ) + + if (!isReviewer) { + logger.error({ + message: "Insufficient permissions to approve review request", + method: "approveReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(403).send({ + message: "Please ensure that you are a reviewer of the review request!", + }) + } + + // Step 5: Approve review request + // NOTE: We are not checking for existence of PR + // as the underlying Github API returns 404 if + // the requested review could not be found. + await this.reviewRequestService.approveReviewRequest(possibleReviewRequest) + return res.status(200).send() + } + + closeReviewRequest: RequestHandler< + { siteName: string; requestId: number }, + ResponseErrorBody, + never, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + // Step 1: Check that the site exists + const { siteName, requestId } = req.params + const { userWithSiteSessionData } = res.locals + const site = await this.sitesService.getBySiteName(siteName) + + if (!site) { + logger.error({ + message: "Invalid site requested", + method: "closeReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) + return res.status(404).send({ + message: "Please ensure that the site exists!", + }) + } + + // Step 3: Retrieve review request + const possibleReviewRequest = await this.reviewRequestService.getReviewRequest( + site, + requestId + ) + + if (isIsomerError(possibleReviewRequest)) { + logger.error({ + message: "Invalid review request requested", + method: "closeReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res + .status(possibleReviewRequest.status) + .json({ message: possibleReviewRequest.message }) + } + + // Step 4: Check if the user is the requestor + const { requestor } = possibleReviewRequest + const isRequestor = requestor.email === userWithSiteSessionData.email + if (!isRequestor) { + logger.error({ + message: "Insufficient permissions to close review request", + method: "closeReviewRequest", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + requestId, + }, + }) + return res.status(404).json({ + message: "Only the requestor can close the Review Request!", + }) + } + + // Step 5: Close review request + // NOTE: We are not checking for existence of PR + // as the underlying Github API returns 404 if + // the requested review could not be found. + await this.reviewRequestService.closeReviewRequest(possibleReviewRequest) + return res.status(200).send() + } + + getRouter() { + const router = express.Router({ mergeParams: true }) + + router.get("/compare", attachReadRouteHandlerWrapper(this.compareDiff)) + router.post( + "/request", + attachWriteRouteHandlerWrapper(this.createReviewRequest) + ) + router.get("/summary", attachReadRouteHandlerWrapper(this.listReviews)) + router.get( + "/:requestId", + attachReadRouteHandlerWrapper(this.getReviewRequest) + ) + router.post( + "/:requestId/merge", + attachWriteRouteHandlerWrapper(this.mergeReviewRequest) + ) + router.post( + "/:requestId/approve", + attachReadRouteHandlerWrapper(this.approveReviewRequest) + ) + router.post( + "/:requestId", + attachWriteRouteHandlerWrapper(this.updateReviewRequest) + ) + router.delete( + "/:requestId", + attachReadRouteHandlerWrapper(this.closeReviewRequest) + ) + + return router + } +} diff --git a/src/server.js b/src/server.js index 0ceba1153..f7e4b44aa 100644 --- a/src/server.js +++ b/src/server.js @@ -37,12 +37,14 @@ import QueueService from "@services/identity/QueueService" import ReposService from "@services/identity/ReposService" import SitesService from "@services/identity/SitesService" import InfraService from "@services/infra/InfraService" +import ReviewRequestService from "@services/review/ReviewRequestService" import { apiLogger } from "./middleware/apiLogger" import { AuthorizationMiddleware } from "./middleware/authorization" import getAuthenticatedSubrouterV1 from "./routes/v1/authenticated" import getAuthenticatedSitesSubrouterV1 from "./routes/v1/authenticatedSites" import getAuthenticatedSubrouter from "./routes/v2/authenticated" +import { ReviewsRouter } from "./routes/v2/authenticated/review" import getAuthenticatedSitesSubrouter from "./routes/v2/authenticatedSites" import CollaboratorsService from "./services/identity/CollaboratorsService" import LaunchClient from "./services/identity/LaunchClient" @@ -135,6 +137,14 @@ const collaboratorsService = new CollaboratorsService({ usersService, whitelist: Whitelist, }) +const reviewRequestService = new ReviewRequestService( + gitHubService, + User, + ReviewRequest, + Reviewer, + ReviewMeta +) + const authenticationMiddleware = getAuthenticationMiddleware() const authorizationMiddleware = getAuthorizationMiddleware({ identityAuthService, @@ -143,6 +153,12 @@ const authorizationMiddleware = getAuthorizationMiddleware({ collaboratorsService, }) +const reviewRouter = new ReviewsRouter( + reviewRequestService, + usersService, + sitesService, + collaboratorsService +) const authenticatedSubrouterV1 = getAuthenticatedSubrouterV1({ authenticationMiddleware, usersService, @@ -164,7 +180,9 @@ const authenticatedSubrouterV2 = getAuthenticatedSubrouter({ isomerAdminsService, collaboratorsService, authorizationMiddleware, + reviewRouter, }) + const authenticatedSitesSubrouterV2 = getAuthenticatedSitesSubrouter({ authorizationMiddleware, authenticationMiddleware, diff --git a/src/services/configServices/SettingsService.js b/src/services/configServices/SettingsService.js index 8228cfcc2..1678d6c91 100644 --- a/src/services/configServices/SettingsService.js +++ b/src/services/configServices/SettingsService.js @@ -55,7 +55,6 @@ class SettingsService { updatedNavigationContent, } ) { - console.log(sessionData) if (!_.isEmpty(updatedConfigContent)) { const mergedConfigContent = this.mergeUpdatedData( config.content, diff --git a/src/services/db/GitHubService.js b/src/services/db/GitHubService.js index 567326178..fd4e91fa8 100644 --- a/src/services/db/GitHubService.js +++ b/src/services/db/GitHubService.js @@ -1,7 +1,5 @@ const { Base64 } = require("js-base64") -const validateStatus = require("@utils/axios-utils") - const BRANCH_REF = "staging" const { @@ -11,11 +9,48 @@ const { const { NotFoundError } = require("@errors/NotFoundError") const { UnprocessableError } = require("@errors/UnprocessableError") +const validateStatus = require("@utils/axios-utils") + +const ReviewApi = require("./review") + class GitHubService { constructor({ axiosInstance }) { this.axiosInstance = axiosInstance } + getCommitDiff(siteName, base, head) { + return ReviewApi.getCommitDiff(siteName, base, head) + } + + createPullRequest(siteName, title, description) { + return ReviewApi.createPullRequest(siteName, title, description) + } + + getPullRequest(siteName, pullRequestNumber) { + return ReviewApi.getPullRequest(siteName, pullRequestNumber) + } + + updatePullRequest(siteName, pullRequestNumber, title, description) { + return ReviewApi.updatePullRequest( + siteName, + pullRequestNumber, + title, + description + ) + } + + closeReviewRequest(siteName, pullRequestNumber) { + return ReviewApi.closeReviewRequest(siteName, pullRequestNumber) + } + + mergePullRequest(siteName, pullRequestNumber) { + return ReviewApi.mergePullRequest(siteName, pullRequestNumber) + } + + approvePullRequest(siteName, pullRequestNumber) { + return ReviewApi.approvePullRequest(siteName, pullRequestNumber) + } + getFilePath({ siteName, fileName, directoryName }) { if (!directoryName) return `${siteName}/contents/${encodeURIComponent(fileName)}` diff --git a/src/services/db/review.ts b/src/services/db/review.ts new file mode 100644 index 000000000..1236be72b --- /dev/null +++ b/src/services/db/review.ts @@ -0,0 +1,85 @@ +import { RawFileChangeInfo, Commit, RawPullRequest } from "@root/types/github" + +import { isomerRepoAxiosInstance as axiosInstance } from "../api/AxiosInstance" + +const { E2E_TEST_GH_TOKEN } = process.env + +export const getCommitDiff = async ( + siteName: string, + base = "master", + head = "staging" +) => + axiosInstance + .get<{ files: RawFileChangeInfo[]; commits: Commit[] }>( + `${siteName}/compare/${base}...${head}` + ) + .then(({ data }) => data) + +export const createPullRequest = ( + siteName: string, + title: string, + description?: string, + base = "master", + head = "staging" +) => + axiosInstance + .post<{ number: number }>( + `${siteName}/pulls`, + // NOTE: only create body if a valid description is given + { title, base, head, ...(description && { body: description }) } + ) + .then(({ data }) => data.number) + +export const getPullRequest = (siteName: string, pullRequestNumber: number) => + axiosInstance + .get(`${siteName}/pulls/${pullRequestNumber}`) + .then(({ data }) => data) + +export const updatePullRequest = ( + siteName: string, + pullRequestNumber: number, + title: string, + description?: string +) => + axiosInstance.patch( + `${siteName}/pulls/${pullRequestNumber}`, + // NOTE: only create body if a valid description is given + { title, ...(description !== undefined && { body: description }) } + ) + +export const closeReviewRequest = ( + siteName: string, + pullRequestNumber: number +) => + axiosInstance.patch( + `${siteName}/pulls/${pullRequestNumber}`, + // NOTE: only create body if a valid description is given + { state: "closed" } + ) + +export const mergePullRequest = (siteName: string, pullRequestNumber: number) => + axiosInstance.put(`${siteName}/pulls/${pullRequestNumber}/merge`) + +export const approvePullRequest = ( + siteName: string, + pullRequestNumber: number +) => + axiosInstance.post( + `${siteName}/pulls/${pullRequestNumber}/reviews`, + { + event: "APPROVE", + }, + { + headers: { + // NOTE: This is currently done because + // we have a lock on the master branch + // and github requires an approval from + // *another* account that is not the creator + // of the pull request. + // This is a temporary workaround until we + // write a migration script to remove the lock on master. + // TODO!: Remove this + Authorization: `token ${E2E_TEST_GH_TOKEN}`, + }, + } + ) diff --git a/src/services/identity/CollaboratorsService.ts b/src/services/identity/CollaboratorsService.ts index 00ca5b0ae..e00e6448c 100644 --- a/src/services/identity/CollaboratorsService.ts +++ b/src/services/identity/CollaboratorsService.ts @@ -105,23 +105,12 @@ class CollaboratorsService { collaborators, [ // Prioritize Admins over Contributors - ( - collaborator: User & { - SiteMember: SiteMember - } - ) => collaborator.SiteMember.role === CollaboratorRoles.Admin, + (collaborator) => + collaborator.SiteMember.role === CollaboratorRoles.Admin, // Prioritize elements where the userId matches the requesterId (i.e. "you") - ( - collaborator: User & { - SiteMember: SiteMember - } - ) => collaborator.id.toString() === requesterId, + (collaborator) => collaborator.id.toString() === requesterId, // Prioritize the user that has not logged in for the longest time - ( - collaborator: User & { - SiteMember: SiteMember - } - ) => collaborator.lastLoggedIn, + (collaborator) => collaborator.lastLoggedIn, ], ["desc", "desc", "asc"] ) @@ -222,7 +211,10 @@ class CollaboratorsService { }) } - getRole = async (siteName: string, userId: string) => { + getRole = async ( + siteName: string, + userId: string + ): Promise => { const site = await this.siteRepository.findOne({ where: { name: siteName }, include: [ @@ -236,7 +228,7 @@ class CollaboratorsService { ], }) - return (site?.site_members?.[0]?.SiteMember?.role as string | null) ?? null + return site?.site_members?.[0]?.SiteMember?.role ?? null } getStatistics = async (siteName: string) => { diff --git a/src/services/identity/UsersService.ts b/src/services/identity/UsersService.ts index f5ee89cb3..2af2782ba 100644 --- a/src/services/identity/UsersService.ts +++ b/src/services/identity/UsersService.ts @@ -2,7 +2,7 @@ import { Op, ModelStatic } from "sequelize" import { Sequelize } from "sequelize-typescript" import { RequireAtLeastOne } from "type-fest" -import { Repo, Site, SiteMember, User, Whitelist } from "@database/models" +import { Repo, Site, User, Whitelist, SiteMember } from "@database/models" import SmsClient from "@services/identity/SmsClient" import TotpGenerator from "@services/identity/TotpGenerator" import MailClient from "@services/utilServices/MailClient" @@ -59,8 +59,8 @@ class UsersService { return this.repository.findOne({ where: { githubId } }) } - async hasAccessToSite(userId: string, siteName: string): Promise { - const siteMember = await this.repository.findOne({ + async getSiteMember(userId: string, siteName: string): Promise { + return this.repository.findOne({ where: { id: userId }, include: [ { @@ -79,8 +79,28 @@ class UsersService { }, ], }) + } - return !!siteMember + async getSiteAdmin(userId: string, siteName: string) { + return this.repository.findOne({ + where: { id: userId, role: "ADMIN" }, + include: [ + { + model: SiteMember, + as: "site_members", + required: true, + include: [ + { + model: Repo, + required: true, + where: { + name: siteName, + }, + }, + ], + }, + ], + }) } async findSitesByUserId( diff --git a/src/services/middlewareServices/AuthorizationMiddlewareService.ts b/src/services/middlewareServices/AuthorizationMiddlewareService.ts index a2579fbc5..3715324bb 100644 --- a/src/services/middlewareServices/AuthorizationMiddlewareService.ts +++ b/src/services/middlewareServices/AuthorizationMiddlewareService.ts @@ -1,4 +1,4 @@ -import { NotFoundError } from "@errors/NotFoundError" +import logger from "@logger/logger" import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" @@ -9,9 +9,6 @@ import CollaboratorsService from "@services/identity/CollaboratorsService" import IsomerAdminsService from "@services/identity/IsomerAdminsService" import UsersService from "@services/identity/UsersService" -// Import logger -const logger = require("@logger/logger") - interface AuthorizationMiddlewareServiceProps { identityAuthService: AuthService usersService: UsersService diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts new file mode 100644 index 000000000..418bf9afd --- /dev/null +++ b/src/services/review/ReviewRequestService.ts @@ -0,0 +1,347 @@ +import { ModelStatic } from "sequelize" + +import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" + +import { Reviewer } from "@database/models/Reviewers" +import { ReviewMeta } from "@database/models/ReviewMeta" +import { ReviewRequest } from "@database/models/ReviewRequest" +import { ReviewRequestStatus } from "@root/constants" +import { Site } from "@root/database/models/Site" +import { User } from "@root/database/models/User" +import RequestNotFoundError from "@root/errors/RequestNotFoundError" +import { + DashboardReviewRequestDto, + EditedItemDto, + FileType, + ReviewRequestDto, +} from "@root/types/dto/review" +import { Commit, fromGithubCommitMessage } from "@root/types/github" +import { RequestChangeInfo } from "@root/types/review" +import * as ReviewApi from "@services/db/review" + +/** + * NOTE: This class does not belong as a subset of GitHub service. + * This is because GitHub service exists to operate on _files_ + * whereas this operates on pull requests. + * + * Perhaps we could rename Github service into GitHubFile service + * and this into GitHubPullRequest service to make the distinction obvious. + * + * Separately, this also allows us to add typings into this service. + */ +export default class ReviewRequestService { + private readonly apiService: typeof ReviewApi + + private readonly repository: ModelStatic + + private readonly users: ModelStatic + + private readonly reviewers: ModelStatic + + private readonly reviewMeta: ModelStatic + + constructor( + apiService: typeof ReviewApi, + users: ModelStatic, + repository: ModelStatic, + reviewers: ModelStatic, + reviewMeta: ModelStatic + ) { + this.apiService = apiService + this.users = users + this.repository = repository + this.reviewers = reviewers + this.reviewMeta = reviewMeta + } + + compareDiff = async ( + sessionData: UserWithSiteSessionData + ): Promise => { + // Step 1: Get the site name + const { siteName } = sessionData + + // Step 2: Get the list of changed files using Github's API + // Refer here for details; https://docs.github.com/en/rest/commits/commits#compare-two-commits + // Note that we need a triple dot (...) between base and head refs + const { files, commits } = await this.apiService.getCommitDiff(siteName) + + const mappings = await this.computeShaMappings(commits) + + return files.map(({ filename, contents_url }) => { + const fullPath = filename.split("/") + const items = contents_url.split("?ref=") + // NOTE: We have to compute sha this way rather than + // taking the file sha. + // This is because the sha present on the file is + // a checksum of the files contents. + // And the actual commit sha is given by the ref param + const sha = items[items.length - 1] + + return { + type: this.computeFileType(filename), + // NOTE: The string is guaranteed to be non-empty + // and hence this should exist. + name: fullPath.pop() || "", + // NOTE: pop alters in place + path: fullPath, + url: this.computeFileUrl(filename, siteName), + lastEditedBy: mappings[sha]?.author || "Unknown user", + lastEditedTime: mappings[sha]?.unixTime || 0, + } + }) + } + + // TODO + computeFileType = (filename: string): FileType[] => ["page"] + + computeFileUrl = (filename: string, siteName: string) => "www.google.com" + + computeShaMappings = async ( + commits: Commit[] + ): Promise> => { + const mappings: Record = {} + + // NOTE: commits from github are capped at 300. + // This implies that there might possibly be some files + // whose commit isn't being returned. + await Promise.all( + commits.map(async ({ commit, sha }) => { + const { userId } = fromGithubCommitMessage(commit.message) + const author = await this.users.findByPk(userId) + const lastChangedTime = new Date(commit.author.date).getTime() + mappings[sha] = { + author: author?.email || commit.author.name, + unixTime: lastChangedTime, + } + }) + ) + return mappings + } + + createReviewRequest = async ( + sessionData: UserWithSiteSessionData, + reviewers: User[], + requestor: User, + site: Site, + title: string, + description?: string + ): Promise => { + const { siteName } = sessionData + // Step 1: Create an actual pull request on Github + // From head -> base + const pullRequestNumber = await this.apiService.createPullRequest( + siteName, + title, + description + ) + + // Step 2: Only update internal model state once PR is created + const reviewRequest = await this.repository.create({ + requestorId: requestor.id, + siteId: site.id, + }) + await Promise.all( + reviewers.map(({ id }) => + this.reviewers.create({ + requestId: reviewRequest.id, + reviewerId: id, + }) + ) + ) + + await this.reviewMeta.create({ + reviewId: reviewRequest.id, + pullRequestNumber, + reviewLink: `cms.isomer.gov.sg/sites/${siteName}/review/${pullRequestNumber}`, + }) + + return pullRequestNumber + } + + listReviewRequest = async ( + { siteName }: UserWithSiteSessionData, + site: Site + ): Promise => { + // Find all review requests associated with the site + const requests = await this.repository.findAll({ + where: { + siteId: site.id, + }, + include: [ + { + model: ReviewMeta, + as: "reviewMeta", + }, + { + model: User, + as: "requestor", + }, + ], + }) + + // NOTE: This has a max of 30 pull requests + // and returns only open pull requests. + return Promise.all( + requests.map(async (req) => { + const { pullRequestNumber } = req.reviewMeta + // NOTE: We explicitly destructure as the raw data + // contains ALOT more than these fields, which we want to + // discard to lower retrieval times for FE + const { + title, + body, + changed_files, + created_at, + } = await this.apiService.getPullRequest(siteName, pullRequestNumber) + + return { + id: pullRequestNumber, + author: req.requestor.email || "Unknown user", + status: req.reviewStatus, + title, + description: body || "", + changedFiles: changed_files, + createdAt: new Date(created_at).getTime(), + // TODO! + newComments: 0, + // TODO! + firstView: false, + } + }) + ) + } + + getReviewRequest = async (site: Site, pullRequestNumber: number) => { + const possibleReviewRequest = await this.repository.findOne({ + where: { + siteId: site.id, + }, + include: [ + { + model: ReviewMeta, + as: "reviewMeta", + where: { + pullRequestNumber, + }, + }, + { + model: User, + as: "requestor", + }, + { + model: User, + as: "reviewers", + }, + { + model: Site, + }, + ], + }) + + if (!possibleReviewRequest) { + return new RequestNotFoundError() + } + + return possibleReviewRequest + } + + getFullReviewRequest = async ( + userWithSiteSessionData: UserWithSiteSessionData, + site: Site, + pullRequestNumber: number + ): Promise => { + const { siteName } = userWithSiteSessionData + const review = await this.repository.findOne({ + where: { + siteId: site.id, + }, + include: [ + { + model: ReviewMeta, + as: "reviewMeta", + where: { + pullRequestNumber, + }, + }, + { + model: User, + as: "requestor", + }, + { + model: User, + as: "reviewers", + }, + { + model: Site, + }, + ], + }) + + // As the db stores github's PR # and (siteName, prNumber) + // should be a unique identifier for a specific review request, + // unable to find a RR with the tuple implies that said RR does not exist. + // This could happen when the user queries for an existing PR that is on github, + // but created prior to this feature rolling out. + if (!review) { + return new RequestNotFoundError() + } + + // NOTE: We explicitly destructure as the raw data + // contains ALOT more than these fields, which we want to + // discard to lower retrieval times for FE + const { title, created_at } = await this.apiService.getPullRequest( + siteName, + pullRequestNumber + ) + + const changedItems = await this.compareDiff(userWithSiteSessionData) + + return { + reviewUrl: review.reviewMeta.reviewLink, + title, + status: review.reviewStatus, + requestor: review.requestor.email || "", + reviewers: review.reviewers.map(({ email }) => email || ""), + reviewRequestedTime: new Date(created_at).getTime(), + changedItems, + } + } + + updateReviewRequest = async ( + reviewRequest: ReviewRequest, + { reviewers }: RequestChangeInfo + ) => { + // Update db state with new reviewers + reviewRequest.$set("reviewers", reviewers) + await reviewRequest.save() + } + + // NOTE: The semantics of our reviewing system is slightly different from github. + // The approval is tied to the request, rather than the user. + approveReviewRequest = async (reviewRequest: ReviewRequest) => { + reviewRequest.reviewStatus = ReviewRequestStatus.Approved + await reviewRequest.save() + } + + closeReviewRequest = async (reviewRequest: ReviewRequest) => { + const siteName = reviewRequest.site.name + const { pullRequestNumber } = reviewRequest.reviewMeta + await this.apiService.closeReviewRequest(siteName, pullRequestNumber) + + reviewRequest.reviewStatus = ReviewRequestStatus.Closed + await reviewRequest.save() + } + + mergeReviewRequest = async ( + reviewRequest: ReviewRequest + ): Promise => { + const siteName = reviewRequest.site.name + const { pullRequestNumber } = reviewRequest.reviewMeta + + await this.apiService.approvePullRequest(siteName, pullRequestNumber) + await this.apiService.mergePullRequest(siteName, pullRequestNumber) + + reviewRequest.reviewStatus = ReviewRequestStatus.Merged + return reviewRequest.save() + } +} diff --git a/src/types/dto/error.ts b/src/types/dto/error.ts new file mode 100644 index 000000000..9ffe2c556 --- /dev/null +++ b/src/types/dto/error.ts @@ -0,0 +1,3 @@ +export interface ResponseErrorBody { + message: string +} diff --git a/src/types/dto/review.ts b/src/types/dto/review.ts new file mode 100644 index 000000000..ffffc2a52 --- /dev/null +++ b/src/types/dto/review.ts @@ -0,0 +1,46 @@ +import { CollaboratorRoles } from "@constants/constants" + +export type ReviewRequestStatus = "OPEN" | "APPROVED" | "MERGED" | "CLOSED" + +export type FileType = "page" | "nav" | "setting" | "file" | "image" + +export interface EditedItemDto { + type: FileType[] + name: string + path: string[] + url: string + lastEditedBy: string + lastEditedTime: number +} + +export interface UserDto { + email: string + role: CollaboratorRoles +} + +export type DashboardReviewRequestDto = { + id: number + title: string + description: string + author: string + status: ReviewRequestStatus + changedFiles: number + // TODO! + newComments: number + firstView: boolean + createdAt: number // Unix timestamp +} + +export interface ReviewRequestDto { + reviewUrl: string + title: string + requestor: string + reviewers: string[] + reviewRequestedTime: number + status: ReviewRequestStatus + changedItems: EditedItemDto[] +} + +export interface UpdateReviewRequestDto { + reviewers: string[] +} diff --git a/src/types/error.ts b/src/types/error.ts index 86de13942..7fe1e9c9b 100644 --- a/src/types/error.ts +++ b/src/types/error.ts @@ -1,2 +1,7 @@ +import { BaseIsomerError } from "@root/errors/BaseError" + // eslint-disable-next-line import/prefer-default-export export const isError = (e: unknown): e is Error => e instanceof Error + +export const isIsomerError = (e: unknown): e is BaseIsomerError => + isError(e) && !!(e as BaseIsomerError).isIsomerError diff --git a/src/types/github.ts b/src/types/github.ts new file mode 100644 index 000000000..1ec755fb7 --- /dev/null +++ b/src/types/github.ts @@ -0,0 +1,83 @@ +// NOTE: Types here are with reference to: +// https://docs.github.com/en/rest/commits/commits#compare-two-commits + +export type FileChangeStatus = + | "added" + | "removed" + | "modified" + | "renamed" + | "copied" + | "changed" + | "unchanged" + +export interface Author { + name: string + email: string + date: string +} + +export interface RawCommit { + url: string + author: Author + // NOTE: message is assumed to have a JSON structure with + // the field `email` existing. + // Moreover, this field is assumed to point to the + // author of the commit. + message: string +} + +export interface Commit { + url: string + sha: string + commit: RawCommit +} + +export interface RawFileChangeInfo { + sha: string + filename: string + status: FileChangeStatus + additions: number + deletions: number + changes: number + // eslint-disable-next-line camelcase + blob_url: string + // eslint-disable-next-line camelcase + raw_url: string + // eslint-disable-next-line camelcase + contents_url: string +} + +export interface IsomerCommitMessage { + message: string + fileName: string + userId: string +} + +/** + * NOTE: Properties can be undefined and caller should validate/give sane default. + * + * This should happen as our current format is not backward compat + * as this implies we rewrite all existing commit messages to have this format. + * We should instead default to the one existing on Github. + */ +export const fromGithubCommitMessage = ( + message: string +): Partial => { + try { + const parsed = JSON.parse(message) + return { + message: parsed.message, + fileName: parsed.filename, + userId: parsed.userId, + } + } catch { + return {} + } +} + +export interface RawPullRequest { + title: string + body: string + changed_files: number + created_at: string +} diff --git a/src/types/request.ts b/src/types/request.ts index 66fd1feb3..22d34e903 100644 --- a/src/types/request.ts +++ b/src/types/request.ts @@ -5,5 +5,5 @@ export type RequestHandler< ResBody = unknown, ReqBody = unknown, ReqQuery = unknown, - Locals = Record + Locals extends Record = Record > = ExpressHandler diff --git a/src/types/review.ts b/src/types/review.ts new file mode 100644 index 000000000..cd78f0112 --- /dev/null +++ b/src/types/review.ts @@ -0,0 +1,15 @@ +import type { User } from "@root/database/models/User" + +import { RawFileChangeInfo } from "./github" + +export interface FileChangeInfo + extends Pick< + RawFileChangeInfo, + "additions" | "deletions" | "changes" | "status" | "filename" + > { + rawUrl: string +} + +export interface RequestChangeInfo { + reviewers: User[] +}