diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts index e7a4dad1e..e4c916072 100644 --- a/src/__mocks__/mockedData.ts +++ b/src/__mocks__/mockedData.ts @@ -72,6 +72,20 @@ export const mockedGitHubNotifications: Notification[] = [ 'https://avatars.githubusercontent.com/u/133795385?s=200&v=4', type: 'User', }, + reviews: [ + { + state: 'APPROVED', + users: ['octocat'], + }, + { + state: 'CHANGES_REQUESTED', + users: ['gitify-app'], + }, + { + state: 'PENDING', + users: ['gitify-user'], + }, + ], }, repository: { id: 57216596, @@ -198,6 +212,7 @@ export const mockedGitHubNotifications: Notification[] = [ latest_comment_url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302885965', type: 'Issue', + reviews: null, }, repository: { id: 57216596, @@ -255,6 +270,7 @@ export const mockedEnterpriseNotifications: Notification[] = [ latest_comment_url: 'https://github.gitify.io/api/v3/repos/myorg/notifications-test/releases/3', type: 'Release', + reviews: null, }, repository: { id: 1, @@ -308,6 +324,7 @@ export const mockedEnterpriseNotifications: Notification[] = [ latest_comment_url: 'https://github.gitify.io/api/v3/repos/myorg/notifications-test/issues/comments/21', type: 'PullRequest', + reviews: null, }, repository: { id: 1, diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index 44718c46c..3703b110e 100644 --- a/src/components/NotificationRow.test.tsx +++ b/src/components/NotificationRow.test.tsx @@ -207,6 +207,7 @@ describe('components/NotificationRow.tsx', () => { avatar_url: 'https://avatars.githubusercontent.com/u/123456789?v=4', type: 'User' as UserType, }, + reviews: null, }, }, hostname: 'github.com', diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 410115059..75eaa3994 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -20,6 +20,7 @@ import { formatForDisplay, openInBrowser } from '../utils/helpers'; import { getNotificationTypeIcon, getNotificationTypeIconColor, + getPullRequestReviewIcon, } from '../utils/icons'; import { formatReason } from '../utils/reason'; @@ -69,13 +70,14 @@ export const NotificationRow: FC = ({ notification, hostname }) => { const reason = formatReason(notification.reason); const NotificationIcon = getNotificationTypeIcon(notification.subject); const iconColor = getNotificationTypeIconColor(notification.subject); + const updatedAt = formatDistanceToNow(parseISO(notification.updated_at), { addSuffix: true, }); - const updatedLabel = notification.subject.user ? `${notification.subject.user.login} updated ${updatedAt}` : `Updated ${updatedAt}`; + const notificationTitle = formatForDisplay([ notification.subject.state, notification.subject.type, @@ -131,6 +133,28 @@ export const NotificationRow: FC = ({ notification, hostname }) => { {reason.title} {updatedAt} + {notification.subject.reviews + ? notification.subject.reviews.map((review) => { + const icon = getPullRequestReviewIcon(review); + if (!icon) { + return null; + } + + return ( + + + + ); + }) + : null} diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap index 640cc496f..e9830cdf9 100644 --- a/src/components/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap @@ -79,6 +79,60 @@ exports[`components/NotificationRow.tsx should render itself & its children 1`] > over 6 years ago + + + + + + + + + + @@ -267,6 +321,60 @@ exports[`components/NotificationRow.tsx should render itself & its children with > over 6 years ago + + + + + + + + + + diff --git a/src/routes/__snapshots__/Notifications.test.tsx.snap b/src/routes/__snapshots__/Notifications.test.tsx.snap index 16d2d9b62..887334d78 100644 --- a/src/routes/__snapshots__/Notifications.test.tsx.snap +++ b/src/routes/__snapshots__/Notifications.test.tsx.snap @@ -172,6 +172,26 @@ exports[`routes/Notifications.tsx should render itself & its children (show acco }, "subject": { "latest_comment_url": "https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302888448", + "reviews": [ + { + "state": "APPROVED", + "users": [ + "octocat", + ], + }, + { + "state": "CHANGES_REQUESTED", + "users": [ + "gitify-app", + ], + }, + { + "state": "PENDING", + "users": [ + "gitify-user", + ], + }, + ], "state": "open", "title": "I am a robot and this is a test!", "type": "Issue", @@ -223,6 +243,7 @@ exports[`routes/Notifications.tsx should render itself & its children (show acco }, "subject": { "latest_comment_url": "https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302885965", + "reviews": null, "title": "Improve the UI", "type": "Issue", "url": "https://api.github.com/repos/gitify-app/notifications-test/issues/4", @@ -321,6 +342,26 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti }, "subject": { "latest_comment_url": "https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302888448", + "reviews": [ + { + "state": "APPROVED", + "users": [ + "octocat", + ], + }, + { + "state": "CHANGES_REQUESTED", + "users": [ + "gitify-app", + ], + }, + { + "state": "PENDING", + "users": [ + "gitify-user", + ], + }, + ], "state": "open", "title": "I am a robot and this is a test!", "type": "Issue", @@ -372,6 +413,7 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti }, "subject": { "latest_comment_url": "https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302885965", + "reviews": null, "title": "Improve the UI", "type": "Issue", "url": "https://api.github.com/repos/gitify-app/notifications-test/issues/4", @@ -424,6 +466,7 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti }, "subject": { "latest_comment_url": "https://github.gitify.io/api/v3/repos/myorg/notifications-test/releases/3", + "reviews": null, "title": "Release 0.0.1", "type": "Release", "url": "https://github.gitify.io/api/v3/repos/myorg/notifications-test/releases/3", @@ -468,6 +511,7 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti }, "subject": { "latest_comment_url": "https://github.gitify.io/api/v3/repos/myorg/notifications-test/issues/comments/21", + "reviews": null, "title": "Bump Version", "type": "PullRequest", "url": "https://github.gitify.io/api/v3/repos/myorg/notifications-test/pulls/4", diff --git a/src/types.ts b/src/types.ts index 001fac7a3..f361b4735 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,3 +1,5 @@ +import type { OcticonProps } from '@primer/octicons-react'; +import type { FC } from 'react'; import type { Notification } from './typesGitHub'; export interface AuthState { @@ -101,4 +103,11 @@ export enum IconColor { GREEN = 'text-green-500', PURPLE = 'text-purple-500', RED = 'text-red-500', + YELLOW = 'text-yellow-500 dark:text-yellow-300', } + +export type PullRequestApprovalIcon = { + type: FC; + color: IconColor; + description: string; +}; diff --git a/src/typesGitHub.ts b/src/typesGitHub.ts index 19fce0567..43a9a0d2f 100644 --- a/src/typesGitHub.ts +++ b/src/typesGitHub.ts @@ -76,6 +76,23 @@ export type CheckSuiteStatus = | 'timed_out' | 'waiting'; +export type PullRequestReviewState = + | 'APPROVED' + | 'CHANGES_REQUESTED' + | 'COMMENTED' + | 'DISMISSED' + | 'PENDING'; + +export type PullRequestReviewAuthorAssociation = + | 'COLLABORATOR' + | 'CONTRIBUTOR' + | 'FIRST_TIMER' + | 'FIRST_TIME_CONTRIBUTOR' + | 'MANNEQUIN' + | 'MEMBER' + | 'NONE' + | 'OWNER'; + export interface Notification { id: string; unread: boolean; @@ -241,6 +258,7 @@ interface GitHubSubject { export interface GitifySubject { state?: StateType; user?: SubjectUser; + reviews?: GitifyPullRequestReview[]; } export interface PullRequest { @@ -281,6 +299,24 @@ export interface PullRequest { changed_files: number; } +export interface GitifyPullRequestReview { + state: PullRequestReviewState; + users: string[]; +} + +export interface PullRequestReview { + id: number; + node_id: string; + user: User; + body: string; + state: PullRequestReviewState; + html_url: string; + pull_request_url: string; + author_association: PullRequestReviewAuthorAssociation; + submitted_at: string; + commit_id: string; +} + export interface Commit { sha: string; node_id: string; diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index 0849f7e7b..686829192 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -8,6 +8,7 @@ import type { Notification, NotificationThreadSubscription, PullRequest, + PullRequestReview, Release, RootHypermediaLinks, UserDetails, @@ -193,6 +194,18 @@ export function getPullRequest( return apiRequestAuth(url, 'GET', token); } +/** + * Lists all reviews for a specified pull request. The list of reviews returns in chronological order. + * + * Endpoint documentation: https://docs.github.com/en/rest/pulls/reviews#list-reviews-for-a-pull-request + */ +export function getPullRequestReviews( + url: string, + token: string, +): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} + /** * Gets a public release with the specified release ID. * diff --git a/src/utils/icons.test.ts b/src/utils/icons.test.ts index 15fb35651..7f1f7747f 100644 --- a/src/utils/icons.test.ts +++ b/src/utils/icons.test.ts @@ -1,5 +1,16 @@ -import type { StateType, Subject, SubjectType } from '../typesGitHub'; -import { getNotificationTypeIcon, getNotificationTypeIconColor } from './icons'; +import { CheckIcon, CommentIcon, FileDiffIcon } from '@primer/octicons-react'; +import { IconColor } from '../types'; +import type { + GitifyPullRequestReview, + StateType, + Subject, + SubjectType, +} from '../typesGitHub'; +import { + getNotificationTypeIcon, + getNotificationTypeIconColor, + getPullRequestReviewIcon, +} from './icons'; describe('utils/icons.ts', () => { describe('getNotificationTypeIcon - should get the notification type icon', () => { @@ -248,6 +259,98 @@ describe('utils/icons.ts', () => { ).toMatchSnapshot(); }); }); + + describe('getPullRequestReviewIcon', () => { + let mockReviewSingleReviewer: GitifyPullRequestReview; + let mockReviewMultipleReviewer: GitifyPullRequestReview; + beforeEach(() => { + mockReviewSingleReviewer = { + state: 'APPROVED', + users: ['user1'], + }; + mockReviewMultipleReviewer = { + state: 'APPROVED', + users: ['user1', 'user2'], + }; + }); + + it('approved', () => { + mockReviewSingleReviewer.state = 'APPROVED'; + mockReviewMultipleReviewer.state = 'APPROVED'; + + expect(getPullRequestReviewIcon(mockReviewSingleReviewer)).toEqual({ + type: CheckIcon, + color: IconColor.GREEN, + description: 'user1 approved these changes', + }); + + expect(getPullRequestReviewIcon(mockReviewMultipleReviewer)).toEqual({ + type: CheckIcon, + color: IconColor.GREEN, + description: 'user1, user2 approved these changes', + }); + }); + + it('changes requested', () => { + mockReviewSingleReviewer.state = 'CHANGES_REQUESTED'; + mockReviewMultipleReviewer.state = 'CHANGES_REQUESTED'; + + expect(getPullRequestReviewIcon(mockReviewSingleReviewer)).toEqual({ + type: FileDiffIcon, + color: IconColor.RED, + description: 'user1 requested changes', + }); + + expect(getPullRequestReviewIcon(mockReviewMultipleReviewer)).toEqual({ + type: FileDiffIcon, + color: IconColor.RED, + description: 'user1, user2 requested changes', + }); + }); + + it('commented', () => { + mockReviewSingleReviewer.state = 'COMMENTED'; + mockReviewMultipleReviewer.state = 'COMMENTED'; + + expect(getPullRequestReviewIcon(mockReviewSingleReviewer)).toEqual({ + type: CommentIcon, + color: IconColor.YELLOW, + description: 'user1 left review comments', + }); + + expect(getPullRequestReviewIcon(mockReviewMultipleReviewer)).toEqual({ + type: CommentIcon, + color: IconColor.YELLOW, + description: 'user1, user2 left review comments', + }); + }); + + it('dismissed', () => { + mockReviewSingleReviewer.state = 'DISMISSED'; + mockReviewMultipleReviewer.state = 'DISMISSED'; + + expect(getPullRequestReviewIcon(mockReviewSingleReviewer)).toEqual({ + type: CommentIcon, + color: IconColor.GRAY, + description: 'user1 review has been dismissed', + }); + + expect(getPullRequestReviewIcon(mockReviewMultipleReviewer)).toEqual({ + type: CommentIcon, + color: IconColor.GRAY, + description: 'user1, user2 reviews have been dismissed', + }); + }); + + it('pending', () => { + mockReviewSingleReviewer.state = 'PENDING'; + mockReviewMultipleReviewer.state = 'PENDING'; + + expect(getPullRequestReviewIcon(mockReviewSingleReviewer)).toBeNull(); + + expect(getPullRequestReviewIcon(mockReviewMultipleReviewer)).toBeNull(); + }); + }); }); function createSubjectMock(mocks: { diff --git a/src/utils/icons.ts b/src/utils/icons.ts index 609421457..b85864e2c 100644 --- a/src/utils/icons.ts +++ b/src/utils/icons.ts @@ -2,9 +2,11 @@ import { AlertIcon, CheckIcon, CommentDiscussionIcon, + CommentIcon, DiscussionClosedIcon, DiscussionDuplicateIcon, DiscussionOutdatedIcon, + FileDiffIcon, GitCommitIcon, GitMergeIcon, GitPullRequestClosedIcon, @@ -24,8 +26,8 @@ import { XIcon, } from '@primer/octicons-react'; import type { FC } from 'react'; -import { IconColor } from '../types'; -import type { Subject } from '../typesGitHub'; +import { IconColor, type PullRequestApprovalIcon } from '../types'; +import type { GitifyPullRequestReview, Subject } from '../typesGitHub'; export function getNotificationTypeIcon(subject: Subject): FC { switch (subject.type) { @@ -111,3 +113,40 @@ export function getNotificationTypeIconColor(subject: Subject): IconColor { return IconColor.GRAY; } } + +export function getPullRequestReviewIcon( + review: GitifyPullRequestReview, +): PullRequestApprovalIcon | null { + const descriptionPrefix = review.users.join(', '); + + switch (review.state) { + case 'APPROVED': + return { + type: CheckIcon, + color: IconColor.GREEN, + description: `${descriptionPrefix} approved these changes`, + }; + case 'CHANGES_REQUESTED': + return { + type: FileDiffIcon, + color: IconColor.RED, + description: `${descriptionPrefix} requested changes`, + }; + case 'COMMENTED': + return { + type: CommentIcon, + color: IconColor.YELLOW, + description: `${descriptionPrefix} left review comments`, + }; + case 'DISMISSED': + return { + type: CommentIcon, + color: IconColor.GRAY, + description: `${descriptionPrefix} ${ + review.users.length > 1 ? 'reviews have' : 'review has' + } been dismissed`, + }; + default: + return null; + } +} diff --git a/src/utils/subject.test.ts b/src/utils/subject.test.ts index cb2798604..560252ba3 100644 --- a/src/utils/subject.test.ts +++ b/src/utils/subject.test.ts @@ -17,6 +17,7 @@ import type { import { getCheckSuiteAttributes, getGitifySubjectDetails, + getLatestReviewForReviewers, getWorkflowRunAttributes, } from './subject'; @@ -630,6 +631,10 @@ describe('utils/subject.ts', () => { .get('/repos/gitify-app/notifications-test/issues/comments/302888448') .reply(200, { user: mockCommenter }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + const result = await getGitifySubjectDetails( mockNotification, mockAccounts.token, @@ -643,6 +648,7 @@ describe('utils/subject.ts', () => { avatar_url: mockCommenter.avatar_url, type: mockCommenter.type, }, + reviews: null, }); }); @@ -660,6 +666,10 @@ describe('utils/subject.ts', () => { .get('/repos/gitify-app/notifications-test/issues/comments/302888448') .reply(200, { user: mockCommenter }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + const result = await getGitifySubjectDetails( mockNotification, mockAccounts.token, @@ -673,6 +683,7 @@ describe('utils/subject.ts', () => { avatar_url: mockCommenter.avatar_url, type: mockCommenter.type, }, + reviews: null, }); }); @@ -690,6 +701,10 @@ describe('utils/subject.ts', () => { .get('/repos/gitify-app/notifications-test/issues/comments/302888448') .reply(200, { user: mockCommenter }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + const result = await getGitifySubjectDetails( mockNotification, mockAccounts.token, @@ -703,6 +718,7 @@ describe('utils/subject.ts', () => { avatar_url: mockCommenter.avatar_url, type: mockCommenter.type, }, + reviews: null, }); }); @@ -720,6 +736,10 @@ describe('utils/subject.ts', () => { .get('/repos/gitify-app/notifications-test/issues/comments/302888448') .reply(200, { user: mockCommenter }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + const result = await getGitifySubjectDetails( mockNotification, mockAccounts.token, @@ -733,6 +753,7 @@ describe('utils/subject.ts', () => { avatar_url: mockCommenter.avatar_url, type: mockCommenter.type, }, + reviews: null, }); }); @@ -749,6 +770,10 @@ describe('utils/subject.ts', () => { user: mockAuthor, }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + const result = await getGitifySubjectDetails( mockNotification, mockAccounts.token, @@ -762,6 +787,7 @@ describe('utils/subject.ts', () => { avatar_url: mockAuthor.avatar_url, type: mockAuthor.type, }, + reviews: null, }); }); @@ -777,6 +803,10 @@ describe('utils/subject.ts', () => { user: mockAuthor, }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + const result = await getGitifySubjectDetails( mockNotification, mockAccounts.token, @@ -790,6 +820,74 @@ describe('utils/subject.ts', () => { avatar_url: mockAuthor.avatar_url, type: mockAuthor.type, }, + reviews: null, + }); + }); + + describe('Pull Request Reviews - Latest Reviews By Reviewer', () => { + it('returns latest review state per reviewer', async () => { + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, [ + { + user: { + login: 'reviewer-1', + }, + state: 'REQUESTED_CHANGES', + }, + { + user: { + login: 'reviewer-2', + }, + state: 'COMMENTED', + }, + { + user: { + login: 'reviewer-1', + }, + state: 'APPROVED', + }, + { + user: { + login: 'reviewer-3', + }, + state: 'APPROVED', + }, + ]); + + const result = await getLatestReviewForReviewers( + mockNotification, + mockAccounts.token, + ); + + expect(result).toEqual([ + { state: 'APPROVED', users: ['reviewer-3', 'reviewer-1'] }, + { state: 'COMMENTED', users: ['reviewer-2'] }, + ]); + }); + + it('handles no PR reviews yet', async () => { + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/1/reviews') + .reply(200, []); + + const result = await getLatestReviewForReviewers( + mockNotification, + mockAccounts.token, + ); + + expect(result).toBeNull(); + }); + + it('returns null when not a PR notification', async () => { + mockNotification.subject.type = 'Issue'; + + const result = await getLatestReviewForReviewers( + mockNotification, + mockAccounts.token, + ); + + expect(result).toBeNull(); }); }); }); diff --git a/src/utils/subject.ts b/src/utils/subject.ts index a172ad6bb..3a94b9574 100644 --- a/src/utils/subject.ts +++ b/src/utils/subject.ts @@ -2,8 +2,10 @@ import type { CheckSuiteAttributes, CheckSuiteStatus, DiscussionStateType, + GitifyPullRequestReview, GitifySubject, Notification, + PullRequestReview, PullRequestStateType, SubjectUser, User, @@ -15,6 +17,7 @@ import { getIssue, getIssueOrPullRequestComment, getPullRequest, + getPullRequestReviews, getRelease, } from './api/client'; import { fetchDiscussion, getLatestDiscussionComment } from './helpers'; @@ -237,6 +240,8 @@ async function getGitifySubjectForPullRequest( prCommentUser = prComment.user; } + const reviews = await getLatestReviewForReviewers(notification, token); + return { state: prState, user: { @@ -245,9 +250,62 @@ async function getGitifySubjectForPullRequest( avatar_url: prCommentUser?.avatar_url ?? pr.user.avatar_url, type: prCommentUser?.type ?? pr.user.type, }, + reviews: reviews, }; } +export async function getLatestReviewForReviewers( + notification: Notification, + token: string, +): Promise | null { + if (notification.subject.type !== 'PullRequest') { + return null; + } + + const prReviews = await getPullRequestReviews( + `${notification.subject.url}/reviews`, + token, + ); + + if (!prReviews.data.length) { + return null; + } + + // Find the most recent review for each reviewer + const latestReviews: PullRequestReview[] = []; + for (const prReview of prReviews.data.reverse()) { + const reviewerFound = latestReviews.find( + (review) => review.user.login === prReview.user.login, + ); + + if (!reviewerFound) { + latestReviews.push(prReview); + } + } + + // Group by the review state + const reviewers: GitifyPullRequestReview[] = []; + for (const prReview of latestReviews) { + const reviewerFound = reviewers.find( + (review) => review.state === prReview.state, + ); + + if (!reviewerFound) { + reviewers.push({ + state: prReview.state, + users: [prReview.user.login], + }); + } else { + reviewerFound.users.push(prReview.user.login); + } + } + + // Sort reviews by state for consistent order when rendering + return reviewers.sort((a, b) => { + return a.state.localeCompare(b.state); + }); +} + async function getGitifySubjectForRelease( notification: Notification, token: string,