From 46f682090156107a6855dd67d941a6566273131d Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 18 Oct 2022 13:34:19 +0800 Subject: [PATCH 01/12] Feat: add methods for retrieving comments from github --- src/services/db/GitHubService.js | 8 ++++++ src/services/db/review.ts | 42 +++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/services/db/GitHubService.js b/src/services/db/GitHubService.js index fd4e91fa8..58ed1eb04 100644 --- a/src/services/db/GitHubService.js +++ b/src/services/db/GitHubService.js @@ -51,6 +51,14 @@ class GitHubService { return ReviewApi.approvePullRequest(siteName, pullRequestNumber) } + async getComments(siteName, pullRequestNumber) { + return ReviewApi.getComments(siteName, pullRequestNumber) + } + + async createComment(siteName, pullRequestNumber, user, message) { + return ReviewApi.createComment(siteName, pullRequestNumber, user, message) + } + 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 index 1236be72b..c12b0f1cc 100644 --- a/src/services/db/review.ts +++ b/src/services/db/review.ts @@ -1,4 +1,9 @@ -import { RawFileChangeInfo, Commit, RawPullRequest } from "@root/types/github" +import { + RawFileChangeInfo, + Commit, + RawPullRequest, + RawComment, +} from "@root/types/github" import { isomerRepoAxiosInstance as axiosInstance } from "../api/AxiosInstance" @@ -83,3 +88,38 @@ export const approvePullRequest = ( }, } ) + +export const getComments = async ( + siteName: string, + pullRequestNumber: number +) => { + const rawComments = await axiosInstance + .get(`${siteName}/issues/${pullRequestNumber}/comments`) + .then(({ data }) => data) + return rawComments.map((rawComment) => { + const commentData = JSON.parse(rawComment.body) + const { user, message } = commentData + return { + user, + message, + createdAt: rawComment.created_at, + } + }) +} + +export const createComment = async ( + siteName: string, + pullRequestNumber: number, + user: string, + message: string +) => { + const stringifiedMessage = JSON.stringify({ + user, + message, + }) + return axiosInstance.post( + `${siteName}/issues/${pullRequestNumber}/comments`, + // NOTE: only create body if a valid description is given + { body: stringifiedMessage } + ) +} From fb5629d70d3ca4f4bb524cb93cf51178d39a50e0 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 18 Oct 2022 13:34:29 +0800 Subject: [PATCH 02/12] Chore: add types --- src/types/dto/review.ts | 7 +++++++ src/types/github.ts | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/src/types/dto/review.ts b/src/types/dto/review.ts index ffffc2a52..75f8261d2 100644 --- a/src/types/dto/review.ts +++ b/src/types/dto/review.ts @@ -44,3 +44,10 @@ export interface ReviewRequestDto { export interface UpdateReviewRequestDto { reviewers: string[] } + +export interface CommentItem { + user: string + createdAt: number + message: string + isRead: boolean +} diff --git a/src/types/github.ts b/src/types/github.ts index 1ec755fb7..1f0c57038 100644 --- a/src/types/github.ts +++ b/src/types/github.ts @@ -81,3 +81,8 @@ export interface RawPullRequest { changed_files: number created_at: string } + +export interface RawComment { + body: string + created_at: string +} From 2578696fd0bcefc70ead35682891bfc15068ec95 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 18 Oct 2022 13:35:17 +0800 Subject: [PATCH 03/12] Feat: add comments methods to reviewRequestService --- src/services/review/ReviewRequestService.ts | 64 +++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index e4fa863c7..3cd449666 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -12,6 +12,7 @@ import { Site } from "@root/database/models/Site" import { User } from "@root/database/models/User" import RequestNotFoundError from "@root/errors/RequestNotFoundError" import { + CommentItem, DashboardReviewRequestDto, EditedItemDto, FileType, @@ -455,4 +456,67 @@ export default class ReviewRequestService { reviewRequest.reviewStatus = ReviewRequestStatus.Merged return reviewRequest.save() } + + createComment = async ( + sessionData: UserWithSiteSessionData, + pullRequestNumber: number, + message: string + ) => { + const { siteName, email } = sessionData + + return this.apiService.createComment( + siteName, + pullRequestNumber, + email, + message + ) + } + + getComments = async ( + sessionData: UserWithSiteSessionData, + site: Site, + pullRequestNumber: number + ): Promise => { + const { siteName, isomerUserId: userId } = sessionData + + // Find all review requests associated with the site + const comments = await this.apiService.getComments( + siteName, + pullRequestNumber + ) + + const requestsView = await this.reviewRequestView.findOne({ + where: { + siteId: site.id, + userId, + }, + include: [ + { + model: ReviewRequest, + required: true, + include: [ + { + model: ReviewMeta, + required: true, + where: { + pullRequestNumber, + }, + }, + ], + }, + ], + }) + + const viewedTime = requestsView ? new Date(requestsView.lastViewedAt) : null + + return comments.map((comment) => { + const createdTime = new Date(comment.createdAt) + return { + user: comment.user, + message: comment.message, + createdAt: createdTime.getTime(), + isRead: viewedTime ? createdTime < viewedTime : false, + } + }) + } } From 797c0a7ea83e3c07d26cf8b747bf8a2055452ba5 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 18 Oct 2022 13:35:44 +0800 Subject: [PATCH 04/12] Feat: add comments routes --- src/routes/v2/authenticated/review.ts | 57 ++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 37415f269..7e66c5fd0 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -19,6 +19,7 @@ import UsersService from "@root/services/identity/UsersService" import { isIsomerError, RequestHandler } from "@root/types" import { ResponseErrorBody } from "@root/types/dto/error" import { + CommentItem, DashboardReviewRequestDto, EditedItemDto, UpdateReviewRequestDto, @@ -634,6 +635,52 @@ export class ReviewsRouter { return res.status(200).send() } + getComments: RequestHandler< + { siteName: string; requestId: number }, + CommentItem[] | ResponseErrorBody, + never, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + const { siteName, requestId } = req.params + const { userWithSiteSessionData } = res.locals + // Step 1: Check that the site exists + const site = await this.sitesService.getBySiteName(siteName) + if (!site) { + return res.status(404).send({ + message: "Please ensure that the site exists!", + }) + } + + // Step 2: Retrieve comments + const comments = await this.reviewRequestService.getComments( + userWithSiteSessionData, + site, + requestId + ) + + return res.status(200).json(comments) + } + + createComment: RequestHandler< + { siteName: string; requestId: number }, + string | ResponseErrorBody, + { message: string }, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + const { requestId } = req.params + const { message } = req.body + const { userWithSiteSessionData } = res.locals + await this.reviewRequestService.createComment( + userWithSiteSessionData, + requestId, + message + ) + + return res.status(200).send("OK") + } + markReviewRequestCommentsAsViewed: RequestHandler< { siteName: string; requestId: number }, string | ResponseErrorBody, @@ -794,8 +841,16 @@ export class ReviewsRouter { "/:requestId/approve", attachReadRouteHandlerWrapper(this.approveReviewRequest) ) + router.get( + "/:requestId/comments", + attachWriteRouteHandlerWrapper(this.getComments) + ) + router.post( + "/:requestId/comments", + attachWriteRouteHandlerWrapper(this.createComment) + ) router.post( - "/:requestId/viewedComments", + "/:requestId/comments/viewedComments", attachWriteRouteHandlerWrapper(this.markReviewRequestCommentsAsViewed) ) router.post( From a479d2294dc4d44eb942498362b2e6339c933cb8 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Sat, 22 Oct 2022 20:05:48 +0800 Subject: [PATCH 05/12] fix: check for properly formatted comments --- src/services/db/review.ts | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/services/db/review.ts b/src/services/db/review.ts index c12b0f1cc..cdf881708 100644 --- a/src/services/db/review.ts +++ b/src/services/db/review.ts @@ -1,3 +1,5 @@ +import _ from "lodash" + import { RawFileChangeInfo, Commit, @@ -96,15 +98,22 @@ export const getComments = async ( const rawComments = await axiosInstance .get(`${siteName}/issues/${pullRequestNumber}/comments`) .then(({ data }) => data) - return rawComments.map((rawComment) => { - const commentData = JSON.parse(rawComment.body) - const { user, message } = commentData - return { - user, - message, - createdAt: rawComment.created_at, - } - }) + return _.compact( + rawComments.map((rawComment) => { + try { + const commentData = JSON.parse(rawComment.body) + const { user, message } = commentData + return { + user, + message, + createdAt: rawComment.created_at, + } + } catch (e) { + // Not properly formatted comment, ignore + return null + } + }) + ) } export const createComment = async ( From 886deb1e5d59886e0a4942270a9b7b632612a453 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 27 Oct 2022 09:48:47 +0800 Subject: [PATCH 06/12] Chore: remove incorrect comments --- src/services/db/review.ts | 1 - src/services/review/ReviewRequestService.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/services/db/review.ts b/src/services/db/review.ts index cdf881708..0006b7e62 100644 --- a/src/services/db/review.ts +++ b/src/services/db/review.ts @@ -128,7 +128,6 @@ export const createComment = async ( }) return axiosInstance.post( `${siteName}/issues/${pullRequestNumber}/comments`, - // NOTE: only create body if a valid description is given { body: stringifiedMessage } ) } diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index 3cd449666..d29b9c2de 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -479,7 +479,6 @@ export default class ReviewRequestService { ): Promise => { const { siteName, isomerUserId: userId } = sessionData - // Find all review requests associated with the site const comments = await this.apiService.getComments( siteName, pullRequestNumber From 8f8bbb7e163fe141a0d84ec20d433dfb5c99e0a1 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 27 Oct 2022 09:49:17 +0800 Subject: [PATCH 07/12] Fix: remove error return type --- src/routes/v2/authenticated/review.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 7e66c5fd0..2fa69e1d0 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -637,7 +637,7 @@ export class ReviewsRouter { getComments: RequestHandler< { siteName: string; requestId: number }, - CommentItem[] | ResponseErrorBody, + CommentItem[], never, unknown, { userWithSiteSessionData: UserWithSiteSessionData } From fe7463b9b9d54ab5ebdfbbe64504493d86cc2c96 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 27 Oct 2022 09:49:35 +0800 Subject: [PATCH 08/12] Fix: add logging if site not found --- src/routes/v2/authenticated/review.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 2fa69e1d0..5f9e2cf29 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -647,6 +647,15 @@ export class ReviewsRouter { // Step 1: Check that the site exists const site = await this.sitesService.getBySiteName(siteName) if (!site) { + logger.error({ + message: "Invalid site requested", + method: "getComments", + meta: { + userId: userWithSiteSessionData.isomerUserId, + email: userWithSiteSessionData.email, + siteName, + }, + }) return res.status(404).send({ message: "Please ensure that the site exists!", }) From 2667ea51f960aa4231b8a4a8d9f535709fc0e0ac Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 27 Oct 2022 12:10:53 +0800 Subject: [PATCH 09/12] Feat: swap use of email in github commit to userid --- src/services/db/review.ts | 24 ++++++------- src/services/review/ReviewRequestService.ts | 37 +++++++++++++++------ 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/services/db/review.ts b/src/services/db/review.ts index 0006b7e62..16f9467ee 100644 --- a/src/services/db/review.ts +++ b/src/services/db/review.ts @@ -5,6 +5,7 @@ import { Commit, RawPullRequest, RawComment, + fromGithubCommitMessage, } from "@root/types/github" import { isomerRepoAxiosInstance as axiosInstance } from "../api/AxiosInstance" @@ -100,17 +101,14 @@ export const getComments = async ( .then(({ data }) => data) return _.compact( rawComments.map((rawComment) => { - try { - const commentData = JSON.parse(rawComment.body) - const { user, message } = commentData - return { - user, - message, - createdAt: rawComment.created_at, - } - } catch (e) { - // Not properly formatted comment, ignore - return null + const commentData = fromGithubCommitMessage(rawComment.body) + if (_.isEmpty(commentData)) return null // Will be filtered out by _.compact + const { userId, message } = commentData + if (!userId || !message) return null // Will be filtered out by _.compact + return { + userId, + message, + createdAt: rawComment.created_at, } }) ) @@ -119,11 +117,11 @@ export const getComments = async ( export const createComment = async ( siteName: string, pullRequestNumber: number, - user: string, + userId: string, message: string ) => { const stringifiedMessage = JSON.stringify({ - user, + userId, message, }) return axiosInstance.post( diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index d29b9c2de..e89469a82 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -126,6 +126,29 @@ export default class ReviewRequestService { return mappings } + computeCommentDataMappings = async ( + comments: { + userId: string + message: string + createdAt: string + }[], + viewedTime: Date | null + ) => { + const mappings = await Promise.all( + comments.map(async ({ userId, message, createdAt }) => { + const createdTime = new Date(createdAt) + const author = await this.users.findByPk(userId) + return { + user: author?.email || "", + message, + createdAt: createdTime.getTime(), + isRead: viewedTime ? createdTime < viewedTime : false, + } + }) + ) + return mappings + } + createReviewRequest = async ( sessionData: UserWithSiteSessionData, reviewers: User[], @@ -462,12 +485,12 @@ export default class ReviewRequestService { pullRequestNumber: number, message: string ) => { - const { siteName, email } = sessionData + const { siteName, isomerUserId } = sessionData return this.apiService.createComment( siteName, pullRequestNumber, - email, + isomerUserId, message ) } @@ -508,14 +531,6 @@ export default class ReviewRequestService { const viewedTime = requestsView ? new Date(requestsView.lastViewedAt) : null - return comments.map((comment) => { - const createdTime = new Date(comment.createdAt) - return { - user: comment.user, - message: comment.message, - createdAt: createdTime.getTime(), - isRead: viewedTime ? createdTime < viewedTime : false, - } - }) + return this.computeCommentDataMappings(comments, viewedTime) } } From d60c399848f15ddc5465da95e048103f03902053 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Sun, 30 Oct 2022 19:51:55 +0800 Subject: [PATCH 10/12] Fix: response type --- src/routes/v2/authenticated/review.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 5f9e2cf29..1a87c027b 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -637,7 +637,7 @@ export class ReviewsRouter { getComments: RequestHandler< { siteName: string; requestId: number }, - CommentItem[], + CommentItem[] | ResponseErrorBody, never, unknown, { userWithSiteSessionData: UserWithSiteSessionData } @@ -673,7 +673,7 @@ export class ReviewsRouter { createComment: RequestHandler< { siteName: string; requestId: number }, - string | ResponseErrorBody, + string, { message: string }, unknown, { userWithSiteSessionData: UserWithSiteSessionData } From bd98768916087899e514211645d91dc655a219ba Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Sun, 30 Oct 2022 19:56:16 +0800 Subject: [PATCH 11/12] Fix: rename method and add github comment type --- src/services/review/ReviewRequestService.ts | 11 ++++------- src/types/dto/review.ts | 6 ++++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index e89469a82..cb8835856 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -16,6 +16,7 @@ import { DashboardReviewRequestDto, EditedItemDto, FileType, + GithubCommentData, ReviewRequestDto, } from "@root/types/dto/review" import { isIsomerError } from "@root/types/error" @@ -126,12 +127,8 @@ export default class ReviewRequestService { return mappings } - computeCommentDataMappings = async ( - comments: { - userId: string - message: string - createdAt: string - }[], + computeCommentData = async ( + comments: GithubCommentData[], viewedTime: Date | null ) => { const mappings = await Promise.all( @@ -531,6 +528,6 @@ export default class ReviewRequestService { const viewedTime = requestsView ? new Date(requestsView.lastViewedAt) : null - return this.computeCommentDataMappings(comments, viewedTime) + return this.computeCommentData(comments, viewedTime) } } diff --git a/src/types/dto/review.ts b/src/types/dto/review.ts index 75f8261d2..2890b2f42 100644 --- a/src/types/dto/review.ts +++ b/src/types/dto/review.ts @@ -51,3 +51,9 @@ export interface CommentItem { message: string isRead: boolean } + +export interface GithubCommentData { + userId: string + message: string + createdAt: string +} From 367cfd92840a1aa9563c7cd19a0f0f8e991c1e5b Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Tue, 1 Nov 2022 15:49:43 +0800 Subject: [PATCH 12/12] fix: compute the number of new comments to show (#549) * fix: compute the number of new comments to show * chore: adjust naming of variable and structure of code * chore: split getting number of new comments into 2 lines --- src/services/review/ReviewRequestService.ts | 24 +++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index cb8835856..345cbc779 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -234,6 +234,27 @@ export default class ReviewRequestService { }, })) + // It is a new comment to the user if any of the following + // conditions satisfy: + // 1. The review request views table does not contain a record + // for the user and the review request. + // 2. The review request views table contains a record for that + // user and review request, but the lastViewedAt entry is NULL. + // 3. The review request views table contains a record in the + // lastViewedAt entry, and the comment has a timestamp greater + // than the one stored in the database. + const allComments = await this.getComments( + sessionData, + site, + pullRequestNumber + ) + const countNewComments = await Promise.all( + allComments.map(async (value) => value.isRead) + ).then((arr) => { + const readComments = arr.filter((isRead) => !!isRead) + return readComments.length + }) + return { id: pullRequestNumber, author: req.requestor.email || "Unknown user", @@ -242,8 +263,7 @@ export default class ReviewRequestService { description: body || "", changedFiles: changed_files, createdAt: new Date(created_at).getTime(), - // TODO! - newComments: 0, + newComments: countNewComments, firstView: isFirstView, } })