Skip to content

Commit

Permalink
feat: add review request views API endpoint and functions (#532)
Browse files Browse the repository at this point in the history
* feat: add review request views API endpoint and functions

* fix: adjust to use Promise.all to allow concurrent creations

* chore: adjust naming of variable to be more reflective of state

* Expose new API endpoint to update lastViewedAt timestamp
  • Loading branch information
dcshzj authored Oct 21, 2022
1 parent 3df3f0d commit 5bf25d9
Show file tree
Hide file tree
Showing 2 changed files with 230 additions and 4 deletions.
115 changes: 115 additions & 0 deletions src/routes/v2/authenticated/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,49 @@ export class ReviewsRouter {
})
}

markAllReviewRequestsAsViewed: RequestHandler<
{ siteName: string },
string | 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)

if (!site) {
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
const { userWithSiteSessionData } = res.locals

// Check if they are a collaborator
const role = await this.collaboratorsService.getRole(
siteName,
userWithSiteSessionData.isomerUserId
)

if (!role) {
return res.status(400).send({
message: "User is not a collaborator of this site!",
})
}

// Step 3: Update all review requests for the site as viewed
await this.reviewRequestService.markAllReviewRequestsAsViewed(
userWithSiteSessionData,
site
)

return res.status(200).json()
}

getReviewRequest: RequestHandler<
{ siteName: string; requestId: number },
{ reviewRequest: ReviewRequestDto } | ResponseErrorBody,
Expand Down Expand Up @@ -504,6 +547,11 @@ export class ReviewsRouter {
// as the underlying Github API returns 404 if
// the requested review could not be found.
await this.reviewRequestService.mergeReviewRequest(possibleReviewRequest)

// Step 5: Clean up the review request view records
// The error is discarded as we are guaranteed to have a review request
await this.reviewRequestService.deleteAllReviewRequestViews(site, requestId)

return res.status(200).send()
}

Expand Down Expand Up @@ -586,6 +634,60 @@ export class ReviewsRouter {
return res.status(200).send()
}

markReviewRequestCommentsAsViewed: RequestHandler<
{ siteName: string; requestId: number },
string | ResponseErrorBody,
never,
unknown,
{ userWithSiteSessionData: UserWithSiteSessionData }
> = async (req, res) => {
// Step 1: Check that the site exists
const { siteName, requestId } = req.params
const site = await this.sitesService.getBySiteName(siteName)

if (!site) {
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
const { userWithSiteSessionData } = res.locals

// Check if they are a collaborator
const role = await this.collaboratorsService.getRole(
siteName,
userWithSiteSessionData.isomerUserId
)

if (!role) {
return res.status(400).send({
message: "User is not a collaborator of this site!",
})
}

// Step 3: Retrieve review request
const possibleReviewRequest = await this.reviewRequestService.getReviewRequest(
site,
requestId
)

if (isIsomerError(possibleReviewRequest)) {
return res.status(404).json({ message: possibleReviewRequest.message })
}

// Step 4: Update user's last viewed timestamp for the review request
await this.reviewRequestService.updateReviewRequestLastViewedAt(
userWithSiteSessionData,
site,
possibleReviewRequest
)

return res.status(200).json()
}

closeReviewRequest: RequestHandler<
{ siteName: string; requestId: number },
ResponseErrorBody,
Expand Down Expand Up @@ -659,6 +761,11 @@ export class ReviewsRouter {
// as the underlying Github API returns 404 if
// the requested review could not be found.
await this.reviewRequestService.closeReviewRequest(possibleReviewRequest)

// Step 6: Clean up the review request view records
// The error is discarded as we are guaranteed to have a review request
await this.reviewRequestService.deleteAllReviewRequestViews(site, requestId)

return res.status(200).send()
}

Expand All @@ -671,6 +778,10 @@ export class ReviewsRouter {
attachWriteRouteHandlerWrapper(this.createReviewRequest)
)
router.get("/summary", attachReadRouteHandlerWrapper(this.listReviews))
router.post(
"/viewed",
attachWriteRouteHandlerWrapper(this.markAllReviewRequestsAsViewed)
)
router.get(
"/:requestId",
attachReadRouteHandlerWrapper(this.getReviewRequest)
Expand All @@ -683,6 +794,10 @@ export class ReviewsRouter {
"/:requestId/approve",
attachReadRouteHandlerWrapper(this.approveReviewRequest)
)
router.post(
"/:requestId/viewedComments",
attachWriteRouteHandlerWrapper(this.markReviewRequestCommentsAsViewed)
)
router.post(
"/:requestId",
attachWriteRouteHandlerWrapper(this.updateReviewRequest)
Expand Down
119 changes: 115 additions & 4 deletions src/services/review/ReviewRequestService.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from "lodash"
import { ModelStatic } from "sequelize"

import UserWithSiteSessionData from "@classes/UserWithSiteSessionData"
Expand All @@ -6,6 +7,7 @@ import { Reviewer } from "@database/models/Reviewers"
import { ReviewMeta } from "@database/models/ReviewMeta"
import { ReviewRequest } from "@database/models/ReviewRequest"
import { ReviewRequestStatus } from "@root/constants"
import { ReviewRequestView } from "@root/database/models"
import { Site } from "@root/database/models/Site"
import { User } from "@root/database/models/User"
import RequestNotFoundError from "@root/errors/RequestNotFoundError"
Expand All @@ -15,6 +17,7 @@ import {
FileType,
ReviewRequestDto,
} from "@root/types/dto/review"
import { isIsomerError } from "@root/types/error"
import { Commit, fromGithubCommitMessage } from "@root/types/github"
import { RequestChangeInfo } from "@root/types/review"
import * as ReviewApi from "@services/db/review"
Expand All @@ -40,18 +43,22 @@ export default class ReviewRequestService {

private readonly reviewMeta: ModelStatic<ReviewMeta>

private readonly reviewRequestView: ModelStatic<ReviewRequestView>

constructor(
apiService: typeof ReviewApi,
users: ModelStatic<User>,
repository: ModelStatic<ReviewRequest>,
reviewers: ModelStatic<Reviewer>,
reviewMeta: ModelStatic<ReviewMeta>
reviewMeta: ModelStatic<ReviewMeta>,
reviewRequestView: ModelStatic<ReviewRequestView>
) {
this.apiService = apiService
this.users = users
this.repository = repository
this.reviewers = reviewers
this.reviewMeta = reviewMeta
this.reviewRequestView = reviewRequestView
}

compareDiff = async (
Expand Down Expand Up @@ -159,9 +166,11 @@ export default class ReviewRequestService {
}

listReviewRequest = async (
{ siteName }: UserWithSiteSessionData,
sessionData: UserWithSiteSessionData,
site: Site
): Promise<DashboardReviewRequestDto[]> => {
const { siteName, isomerUserId: userId } = sessionData

// Find all review requests associated with the site
const requests = await this.repository.findAll({
where: {
Expand Down Expand Up @@ -194,6 +203,16 @@ export default class ReviewRequestService {
created_at,
} = await this.apiService.getPullRequest(siteName, pullRequestNumber)

// It is the user's first view if the review request views table
// does not contain a record for the user and the review request
const isFirstView = !(await this.reviewRequestView.count({
where: {
reviewId: req.id,
siteId: site.id,
userId,
},
}))

return {
id: pullRequestNumber,
author: req.requestor.email || "Unknown user",
Expand All @@ -204,13 +223,105 @@ export default class ReviewRequestService {
createdAt: new Date(created_at).getTime(),
// TODO!
newComments: 0,
// TODO!
firstView: false,
firstView: isFirstView,
}
})
)
}

markAllReviewRequestsAsViewed = async (
sessionData: UserWithSiteSessionData,
site: Site
): Promise<void> => {
const { isomerUserId: userId } = sessionData

const requestsViewed = await this.reviewRequestView.findAll({
where: {
siteId: site.id,
userId,
},
})

const allActiveRequests = await this.repository.findAll({
where: {
siteId: site.id,
// NOTE: Closed and merged review requests would not have an
// entry in the review request views table
reviewStatus: ["OPEN", "APPROVED"],
},
})

const requestIdsViewed = requestsViewed.map(
(request) => request.reviewRequestId
)
const allActiveRequestIds = allActiveRequests.map((request) => request.id)
const requestIdsToMarkAsViewed = _.difference(
allActiveRequestIds,
requestIdsViewed
)

await Promise.all(
// Using map here to allow creations to be done concurrently
// But we do not actually need the result of the view creation
requestIdsToMarkAsViewed.map(
async (requestId) =>
await this.reviewRequestView.create({
reviewRequestId: requestId,
siteId: site.id,
userId,
// This field represents the user opening the review request
// itself, which the user has not done so yet at this stage.
lastViewedAt: null,
})
)
)
}

updateReviewRequestLastViewedAt = async (
sessionData: UserWithSiteSessionData,
site: Site,
reviewRequest: ReviewRequest
): Promise<void | RequestNotFoundError> => {
const { isomerUserId: userId } = sessionData
const { id: reviewRequestId } = reviewRequest

await this.reviewRequestView.update(
{
lastViewedAt: new Date(),
},
{
where: {
reviewRequestId,
siteId: site.id,
userId,
},
}
)
}

deleteAllReviewRequestViews = async (
site: Site,
pullRequestNumber: number
): Promise<void | RequestNotFoundError> => {
const possibleReviewRequest = await this.getReviewRequest(
site,
pullRequestNumber
)

if (isIsomerError(possibleReviewRequest)) {
return possibleReviewRequest
}

const { id: reviewRequestId } = possibleReviewRequest

await this.reviewRequestView.destroy({
where: {
reviewRequestId,
siteId: site.id,
},
})
}

getReviewRequest = async (site: Site, pullRequestNumber: number) => {
const possibleReviewRequest = await this.repository.findOne({
where: {
Expand Down

0 comments on commit 5bf25d9

Please sign in to comment.