From 6cabe6006988a9882e9c755e1707972db538248c Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 18 Mar 2024 12:08:43 -0400 Subject: [PATCH 1/3] feat: show user --- src/__mocks__/mockedData.ts | 66 ++++++++ src/components/NotificationRow.tsx | 4 + src/hooks/useNotifications.test.ts | 11 ++ src/hooks/useNotifications.ts | 10 +- src/typesGithub.ts | 13 +- src/utils/helpers.ts | 15 +- src/utils/{state.test.ts => subject.test.ts} | 152 ++++++++++++++----- src/utils/{state.ts => subject.ts} | 94 +++++++++--- 8 files changed, 289 insertions(+), 76 deletions(-) rename src/utils/{state.test.ts => subject.test.ts} (73%) rename src/utils/{state.ts => subject.ts} (59%) diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts index 59f046f34..f57450b81 100644 --- a/src/__mocks__/mockedData.ts +++ b/src/__mocks__/mockedData.ts @@ -299,6 +299,9 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2215656, createdAt: '2022-02-20T18:33:39Z', + author: { + login: 'comment-user', + }, replies: { nodes: [], }, @@ -306,6 +309,9 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2217789, createdAt: '2022-02-21T03:30:42Z', + author: { + login: 'comment-user', + }, replies: { nodes: [], }, @@ -313,11 +319,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2223243, createdAt: '2022-02-21T18:26:27Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2232922, createdAt: '2022-02-23T00:57:58Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -325,6 +337,9 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2232921, createdAt: '2022-02-23T00:57:49Z', + author: { + login: 'comment-user', + }, replies: { nodes: [], }, @@ -332,11 +347,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2258799, createdAt: '2022-02-27T01:22:20Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2300902, createdAt: '2022-03-05T17:43:52Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -344,11 +365,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2297637, createdAt: '2022-03-04T20:39:44Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2300893, createdAt: '2022-03-05T17:41:04Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -356,11 +383,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2299763, createdAt: '2022-03-05T11:05:42Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2300895, createdAt: '2022-03-05T17:41:44Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -379,6 +412,9 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2215656, createdAt: '2022-02-20T18:33:39Z', + author: { + login: 'comment-user', + }, replies: { nodes: [], }, @@ -386,6 +422,9 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2217789, createdAt: '2022-02-21T03:30:42Z', + author: { + login: 'comment-user', + }, replies: { nodes: [], }, @@ -393,11 +432,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2223243, createdAt: '2022-02-21T18:26:27Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2232922, createdAt: '2022-02-23T00:57:58Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -405,6 +450,9 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2232921, createdAt: '2022-02-23T00:57:49Z', + author: { + login: 'comment-user', + }, replies: { nodes: [], }, @@ -412,11 +460,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2258799, createdAt: '2022-02-27T01:22:20Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2300902, createdAt: '2022-03-05T17:43:52Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -424,11 +478,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2297637, createdAt: '2022-03-04T20:39:44Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2300893, createdAt: '2022-03-05T17:41:04Z', + author: { + login: 'reply-user', + }, }, ], }, @@ -436,11 +496,17 @@ export const mockedGraphQLResponse: GraphQLSearch = { databaseId: 2299763, createdAt: '2022-03-05T11:05:42Z', + author: { + login: 'comment-user', + }, replies: { nodes: [ { databaseId: 2300895, createdAt: '2022-03-05T17:41:44Z', + author: { + login: 'reply-user', + }, }, ], }, diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 8f884cef5..d24ce1483 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -61,6 +61,9 @@ export const NotificationRow: React.FC = ({ const updatedAt = formatDistanceToNow(parseISO(notification.updated_at), { addSuffix: true, }); + const updatedBy = notification.subject.user + ? ` by ${notification.subject.user}` + : ''; const notificationTitle = formatForDisplay([ notification.subject.state, notification.subject.type, @@ -87,6 +90,7 @@ export const NotificationRow: React.FC = ({
{reason.type} - Updated{' '} {updatedAt} + {updatedBy}
diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index 4b020c8f3..0d7da8a03 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -219,6 +219,7 @@ describe('hooks/useNotifications.ts', () => { title: 'This is an Issue.', type: 'Issue', url: 'https://api.github.com/3', + latest_comment_url: 'https://api.github.com/3/comments', }, }, { @@ -227,6 +228,7 @@ describe('hooks/useNotifications.ts', () => { title: 'This is a Pull Request.', type: 'PullRequest', url: 'https://api.github.com/4', + latest_comment_url: 'https://api.github.com/4/comments', }, }, { @@ -262,6 +264,9 @@ describe('hooks/useNotifications.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: null, isAnswered: true, + comments: { + nodes: [], // TODO - this should be replaced with data + }, }, ], }, @@ -270,9 +275,15 @@ describe('hooks/useNotifications.ts', () => { nock('https://api.github.com') .get('/3') .reply(200, { state: 'closed', merged: true }); + nock('https://api.github.com') + .get('/3/comments') + .reply(200, { user: { login: 'some-user' } }); nock('https://api.github.com') .get('/4') .reply(200, { state: 'closed', merged: false }); + nock('https://api.github.com') + .get('/4/comments') + .reply(200, { user: { login: 'some-user' } }); nock('https://api.github.com') .get('/5') .reply(200, { state: 'open', draft: false }); diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 82ca9f4df..72f43459c 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -17,7 +17,7 @@ import { } from '../utils/notifications'; import Constants from '../utils/constants'; import { removeNotifications } from '../utils/remove-notifications'; -import { getNotificationState } from '../utils/state'; +import { getGitifySubjectDetails } from '../utils/subject'; interface NotificationsState { notifications: AccountNotifications[]; @@ -141,16 +141,14 @@ export const useNotifications = (colors: boolean): NotificationsState => { ) : accounts.token; - const notificationState = await getNotificationState( - notification, - token, - ); + const additionalSubjectDetails = + await getGitifySubjectDetails(notification, token); return { ...notification, subject: { ...notification.subject, - state: notificationState, + ...additionalSubjectDetails, }, }; }, diff --git a/src/typesGithub.ts b/src/typesGithub.ts index d2654a8d5..49f3354f0 100644 --- a/src/typesGithub.ts +++ b/src/typesGithub.ts @@ -157,14 +157,21 @@ export interface Owner { site_admin: boolean; } -export interface Subject { +export type Subject = GitHubSubject & GitifySubject; + +interface GitHubSubject { title: string; url: string | null; - state?: StateType; // This is not in the GitHub API, but we add it to the type to make it easier to work with latest_comment_url: string | null; type: SubjectType; } +// This is not in the GitHub API, but we add it to the type to make it easier to work with +export interface GitifySubject { + state?: StateType; + user?: string; +} + export interface PullRequest { url: string; id: number; @@ -262,6 +269,7 @@ export interface DiscussionSearchResultNode { export interface DiscussionCommentNode { databaseId: string | number; createdAt: string; + author: { login: string }; replies: { nodes: DiscussionSubcommentNode[]; }; @@ -270,6 +278,7 @@ export interface DiscussionCommentNode { export interface DiscussionSubcommentNode { databaseId: string | number; createdAt: string; + author: { login: string }; } export interface CheckSuiteAttributes { diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 62a72a746..8786a19fc 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -11,7 +11,7 @@ import { import { apiRequestAuth } from '../utils/api-requests'; import { openExternalLink } from '../utils/comms'; import { Constants } from './constants'; -import { getWorkflowRunAttributes, getCheckSuiteAttributes } from './state'; +import { getWorkflowRunAttributes, getCheckSuiteAttributes } from './subject'; export function getEnterpriseAccountToken( hostname: string, @@ -126,7 +126,7 @@ async function getDiscussionUrl( ): Promise { let url = `${notification.repository.html_url}/discussions`; - const discussion = await fetchDiscussion(notification, token, true); + const discussion = await fetchDiscussion(notification, token); if (discussion) { url = discussion.url; @@ -147,12 +147,10 @@ async function getDiscussionUrl( export async function fetchDiscussion( notification: Notification, token: string, - includeComments: boolean, ): Promise { const response: GraphQLSearch = await apiRequestAuth(`https://api.github.com/graphql`, 'POST', token, { query: `query fetchDiscussions( - $includeComments: Boolean!, $queryStatement: String!, $type: SearchType!, $firstDiscussions: Int, @@ -167,14 +165,20 @@ export async function fetchDiscussion( stateReason isAnswered url - comments(last: $lastComments) @include(if: $includeComments){ + comments(last: $lastComments){ nodes { databaseId createdAt + author { + login + } replies(last: $firstReplies) { nodes { databaseId createdAt + author { + login + } } } } @@ -194,7 +198,6 @@ export async function fetchDiscussion( firstDiscussions: 10, lastComments: 100, firstReplies: 1, - includeComments: includeComments, }, }); diff --git a/src/utils/state.test.ts b/src/utils/subject.test.ts similarity index 73% rename from src/utils/state.test.ts rename to src/utils/subject.test.ts index 89a62318b..e6700bd2f 100644 --- a/src/utils/state.test.ts +++ b/src/utils/subject.test.ts @@ -5,11 +5,11 @@ import { mockAccounts } from '../__mocks__/mock-state'; import { mockedSingleNotification } from '../__mocks__/mockedData'; import { getCheckSuiteAttributes, - getDiscussionState, - getIssueState, - getPullRequestState, + getGitifySubjectForDiscussion, + getGitifySubjectForIssue, + getGitifySubjectForPullRequest, getWorkflowRunAttributes, -} from './state'; +} from './subject'; describe('utils/state.ts', () => { beforeEach(() => { // axios will default to using the XHR adapter which can't be intercepted @@ -156,18 +156,22 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: null, isAnswered: true, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('ANSWERED'); + expect(result.state).toBe('ANSWERED'); + expect(result.user).toBe(null); }); it('duplicate discussion state', async () => { @@ -190,18 +194,22 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: 'DUPLICATE', isAnswered: false, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('DUPLICATE'); + expect(result.state).toBe('DUPLICATE'); + expect(result.user).toBe(null); }); it('open discussion state', async () => { @@ -224,18 +232,22 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: null, isAnswered: false, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('OPEN'); + expect(result.state).toBe('OPEN'); + expect(result.user).toBe(null); }); it('outdated discussion state', async () => { @@ -258,18 +270,22 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: 'OUTDATED', isAnswered: false, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('OUTDATED'); + expect(result.state).toBe('OUTDATED'); + expect(result.user).toBe(null); }); it('reopened discussion state', async () => { @@ -292,18 +308,22 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: 'REOPENED', isAnswered: false, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('REOPENED'); + expect(result.state).toBe('REOPENED'); + expect(result.user).toBe(null); }); it('resolved discussion state', async () => { @@ -326,18 +346,22 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: 'RESOLVED', isAnswered: false, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('RESOLVED'); + expect(result.state).toBe('RESOLVED'); + expect(result.user).toBe(null); }); it('filtered response by subscribed', async () => { @@ -360,24 +384,31 @@ describe('utils/state.ts', () => { viewerSubscription: 'SUBSCRIBED', stateReason: null, isAnswered: false, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, { title: 'This is a discussion', viewerSubscription: 'IGNORED', stateReason: null, isAnswered: true, + comments: { + nodes: [], //TODO - Update this to have real data + }, }, ], }, }, }); - const result = await getDiscussionState( + const result = await getGitifySubjectForDiscussion( mockNotification, mockAccounts.token, ); - expect(result).toBe('OPEN'); + expect(result.state).toBe('OPEN'); + expect(result.user).toBe(null); }); it('handles unknown or missing results', async () => {}); @@ -389,12 +420,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'open' }); - const result = await getIssueState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForIssue( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('open'); + expect(result.state).toBe('open'); + expect(result.user).toBe('some-user'); }); it('closed issue state', async () => { @@ -402,12 +438,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'closed' }); - const result = await getIssueState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForIssue( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('closed'); + expect(result.state).toBe('closed'); + expect(result.user).toBe('some-user'); }); it('completed issue state', async () => { @@ -415,12 +456,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'closed', state_reason: 'completed' }); - const result = await getIssueState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForIssue( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('completed'); + expect(result.state).toBe('completed'); + expect(result.user).toBe('some-user'); }); it('not_planned issue state', async () => { @@ -428,12 +474,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'open', state_reason: 'not_planned' }); - const result = await getIssueState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForIssue( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('not_planned'); + expect(result.state).toBe('not_planned'); + expect(result.user).toBe('some-user'); }); it('reopened issue state', async () => { @@ -441,12 +492,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'open', state_reason: 'reopened' }); - const result = await getIssueState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForIssue( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('reopened'); + expect(result.state).toBe('reopened'); + expect(result.user).toBe('some-user'); }); }); @@ -456,12 +512,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'closed', draft: false, merged: false }); - const result = await getPullRequestState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForPullRequest( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('closed'); + expect(result.state).toBe('closed'); + expect(result.user).toBe('some-user'); }); it('draft pull request state', async () => { @@ -469,12 +530,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'open', draft: true, merged: false }); - const result = await getPullRequestState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForPullRequest( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('draft'); + expect(result.state).toBe('draft'); + expect(result.user).toBe('some-user'); }); it('merged pull request state', async () => { @@ -482,12 +548,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'open', draft: false, merged: true }); - const result = await getPullRequestState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForPullRequest( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('merged'); + expect(result.state).toBe('merged'); + expect(result.user).toBe('some-user'); }); it('open pull request state', async () => { @@ -495,12 +566,17 @@ describe('utils/state.ts', () => { .get('/repos/manosim/notifications-test/issues/1') .reply(200, { state: 'open', draft: false, merged: false }); - const result = await getPullRequestState( + nock('https://api.github.com') + .get('/repos/manosim/notifications-test/issues/comments/302888448') + .reply(200, { user: { login: 'some-user' } }); + + const result = await getGitifySubjectForPullRequest( mockedSingleNotification, mockAccounts.token, ); - expect(result).toBe('open'); + expect(result.state).toBe('open'); + expect(result.user).toBe('some-user'); }); }); @@ -516,8 +592,8 @@ describe('utils/state.ts', () => { const result = getWorkflowRunAttributes(mockNotification); - expect(result.user).toBe('some-user'); expect(result.status).toBe('waiting'); + expect(result.user).toBe('some-user'); }); it('unknown workflow run state', async () => { @@ -532,8 +608,8 @@ describe('utils/state.ts', () => { const result = getWorkflowRunAttributes(mockNotification); - expect(result.user).toBe('some-user'); expect(result.status).toBeNull(); + expect(result.user).toBe('some-user'); }); it('unhandled workflow run title', async () => { diff --git a/src/utils/state.ts b/src/utils/subject.ts similarity index 59% rename from src/utils/state.ts rename to src/utils/subject.ts index 0e750573f..1caec5963 100644 --- a/src/utils/state.ts +++ b/src/utils/subject.ts @@ -3,32 +3,30 @@ import { CheckSuiteAttributes, CheckSuiteStatus, DiscussionStateType, + GitifySubject, Issue, - IssueStateReasonType, - IssueStateType, Notification, PullRequest, PullRequestStateType, - StateType, WorkflowRunAttributes, } from '../typesGithub'; import { apiRequestAuth } from './api-requests'; -export async function getNotificationState( +export async function getGitifySubjectDetails( notification: Notification, token: string, -): Promise { +): Promise { switch (notification.subject.type) { case 'CheckSuite': - return getCheckSuiteAttributes(notification)?.status; + return getGitifySubjectForCheckSuite(notification); case 'Discussion': - return await getDiscussionState(notification, token); + return await getGitifySubjectForDiscussion(notification, token); case 'Issue': - return await getIssueState(notification, token); + return await getGitifySubjectForIssue(notification, token); case 'PullRequest': - return await getPullRequestState(notification, token); + return await getGitifySubjectForPullRequest(notification, token); case 'WorkflowRun': - return getWorkflowRunAttributes(notification)?.status; + return getGitifySubjectForWorkflowRun(notification); default: return null; } @@ -78,51 +76,99 @@ function getCheckSuiteStatus(statusDisplayName: string): CheckSuiteStatus { } } -export async function getDiscussionState( +export function getGitifySubjectForCheckSuite( + notification: Notification, +): GitifySubject { + return { + state: getCheckSuiteAttributes(notification)?.status, + user: null, + }; +} + +export async function getGitifySubjectForDiscussion( notification: Notification, token: string, -): Promise { - const discussion = await fetchDiscussion(notification, token, false); +): Promise { + const discussion = await fetchDiscussion(notification, token); + let discussionState: DiscussionStateType = 'OPEN'; if (discussion) { if (discussion.isAnswered) { - return 'ANSWERED'; + discussionState = 'ANSWERED'; } if (discussion.stateReason) { - return discussion.stateReason; + discussionState = discussion.stateReason; } } - return 'OPEN'; + let discussionUser = null; + console.log(discussion); + const firstComment = discussion?.comments?.nodes[0]; + if (firstComment) { + const firstReply = firstComment?.replies?.nodes[0]; + + discussionUser = firstReply + ? firstReply.author.login + : firstComment.author.login; + } + + return { + state: discussionState, + user: discussionUser, + }; } -export async function getIssueState( +export async function getGitifySubjectForIssue( notification: Notification, token: string, -): Promise { +): Promise { const issue: Issue = ( await apiRequestAuth(notification.subject.url, 'GET', token) ).data; - return issue.state_reason ?? issue.state; + const issueComment: Issue = ( + await apiRequestAuth(notification.subject.latest_comment_url, 'GET', token) + ).data; + + return { + state: issue.state_reason ?? issue.state, + user: issueComment.user.login, + }; } -export async function getPullRequestState( +export async function getGitifySubjectForPullRequest( notification: Notification, token: string, -): Promise { +): Promise { const pr: PullRequest = ( await apiRequestAuth(notification.subject.url, 'GET', token) ).data; + const prComment: PullRequest = ( + await apiRequestAuth(notification.subject.latest_comment_url, 'GET', token) + ).data; + + let prState: PullRequestStateType = pr.state; if (pr.merged) { - return 'merged'; + prState = 'merged'; } else if (pr.draft) { - return 'draft'; + prState = 'draft'; } - return pr.state; + return { + state: prState, + user: prComment.user.login, + }; +} + +export function getGitifySubjectForWorkflowRun( + notification: Notification, +): GitifySubject { + return { + state: getWorkflowRunAttributes(notification)?.status, + user: null, + }; } /** From 7ef008de2fc9aa9d0c05750209ea41c386178ed2 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 18 Mar 2024 13:59:18 -0400 Subject: [PATCH 2/3] feat: show notification user --- src/components/NotificationRow.tsx | 13 +++-- .../NotificationRow.test.tsx.snap | 11 +++-- src/hooks/useNotifications.test.ts | 17 ++++++- src/utils/helpers.ts | 9 ++-- src/utils/subject.test.ts | 6 +-- src/utils/subject.ts | 49 ++++++++++++------- 6 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index d24ce1483..17c553c8a 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -64,6 +64,7 @@ export const NotificationRow: React.FC = ({ const updatedBy = notification.subject.user ? ` by ${notification.subject.user}` : ''; + const updatedLabel = `Updated ${updatedAt}${updatedBy}`; const notificationTitle = formatForDisplay([ notification.subject.state, notification.subject.type, @@ -83,14 +84,16 @@ export const NotificationRow: React.FC = ({ onClick={() => pressTitle()} role="main" > -
+
{notification.subject.title}
-
- {reason.type} - Updated{' '} - {updatedAt} - {updatedBy} +
+ {reason.type} -{' '} + {updatedLabel}
diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap index 3dfac90dc..0f107e1fd 100644 --- a/src/components/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap @@ -42,20 +42,25 @@ exports[`components/Notification.js should render itself & its children 1`] = ` >
I am a robot and this is a test!
Subscribed - - Updated + - - in over 3 years + + Updated in over 3 years +
{ search: { nodes: [ { - title: 'This is an answered discussion', + title: 'This is a Discussion.', viewerSubscription: 'SUBSCRIBED', stateReason: null, isAnswered: true, + url: 'https://github.com/manosim/notifications-test/discussions/612', comments: { - nodes: [], // TODO - this should be replaced with data + nodes: [ + { + databaseId: 2297637, + createdAt: '2022-03-04T20:39:44Z', + author: { + login: 'comment-user', + }, + replies: { + nodes: [], + }, + }, + ], }, }, ], }, }, }); + nock('https://api.github.com') .get('/3') .reply(200, { state: 'closed', merged: true }); diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 8786a19fc..6da0b2525 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -7,6 +7,7 @@ import { PullRequest, Issue, IssueComments, + DiscussionSubcommentNode, } from '../typesGithub'; import { apiRequestAuth } from '../utils/api-requests'; import { openExternalLink } from '../utils/comms'; @@ -136,7 +137,7 @@ async function getDiscussionUrl( let latestCommentId: string | number; if (comments?.length) { - latestCommentId = getLatestDiscussionCommentId(comments); + latestCommentId = getLatestDiscussionComment(comments)?.databaseId; url += `#discussioncomment-${latestCommentId}`; } } @@ -214,13 +215,13 @@ export async function fetchDiscussion( return discussions[0]; } -export function getLatestDiscussionCommentId( +export function getLatestDiscussionComment( comments: DiscussionCommentNode[], -) { +): DiscussionSubcommentNode | null { return comments .flatMap((comment) => comment.replies.nodes) .concat([comments.at(-1)]) - .reduce((a, b) => (a.createdAt > b.createdAt ? a : b))?.databaseId; + .reduce((a, b) => (a.createdAt > b.createdAt ? a : b)); } export async function generateGitHubWebUrl( diff --git a/src/utils/subject.test.ts b/src/utils/subject.test.ts index e6700bd2f..d2eb357cf 100644 --- a/src/utils/subject.test.ts +++ b/src/utils/subject.test.ts @@ -135,7 +135,7 @@ describe('utils/state.ts', () => { }); }); - describe('getDiscussionState', () => { + describe('getGitifySubjectForDiscussion', () => { it('answered discussion state', async () => { const mockNotification = { ...mockedSingleNotification, @@ -414,7 +414,7 @@ describe('utils/state.ts', () => { it('handles unknown or missing results', async () => {}); }); - describe('getIssueState', () => { + describe('getGitifySubjectForIssue', () => { it('open issue state', async () => { nock('https://api.github.com') .get('/repos/manosim/notifications-test/issues/1') @@ -506,7 +506,7 @@ describe('utils/state.ts', () => { }); }); - describe('getPullRequestState', () => { + describe('getGitifySubjectForPullRequest', () => { it('closed pull request state', async () => { nock('https://api.github.com') .get('/repos/manosim/notifications-test/issues/1') diff --git a/src/utils/subject.ts b/src/utils/subject.ts index 1caec5963..c58f54fd6 100644 --- a/src/utils/subject.ts +++ b/src/utils/subject.ts @@ -1,10 +1,11 @@ -import { fetchDiscussion } from './helpers'; +import { fetchDiscussion, getLatestDiscussionComment } from './helpers'; import { CheckSuiteAttributes, CheckSuiteStatus, DiscussionStateType, GitifySubject, Issue, + IssueComments, Notification, PullRequest, PullRequestStateType, @@ -102,15 +103,11 @@ export async function getGitifySubjectForDiscussion( } } + let comments = discussion.comments.nodes; let discussionUser = null; - console.log(discussion); - const firstComment = discussion?.comments?.nodes[0]; - if (firstComment) { - const firstReply = firstComment?.replies?.nodes[0]; - - discussionUser = firstReply - ? firstReply.author.login - : firstComment.author.login; + + if (comments?.length) { + discussionUser = getLatestDiscussionComment(comments)?.author.login; } return { @@ -127,13 +124,22 @@ export async function getGitifySubjectForIssue( await apiRequestAuth(notification.subject.url, 'GET', token) ).data; - const issueComment: Issue = ( - await apiRequestAuth(notification.subject.latest_comment_url, 'GET', token) - ).data; + let issueCommentUser = null; + if (notification.subject.latest_comment_url) { + const issueComment: IssueComments = ( + await apiRequestAuth( + notification.subject.latest_comment_url, + 'GET', + token, + ) + ).data; + + issueCommentUser = issueComment.user.login; + } return { state: issue.state_reason ?? issue.state, - user: issueComment.user.login, + user: issueCommentUser, }; } @@ -145,9 +151,18 @@ export async function getGitifySubjectForPullRequest( await apiRequestAuth(notification.subject.url, 'GET', token) ).data; - const prComment: PullRequest = ( - await apiRequestAuth(notification.subject.latest_comment_url, 'GET', token) - ).data; + let prCommentUser = null; + if (notification.subject.latest_comment_url) { + const prComment: IssueComments = ( + await apiRequestAuth( + notification.subject.latest_comment_url, + 'GET', + token, + ) + ).data; + + prCommentUser = prComment.user.login; + } let prState: PullRequestStateType = pr.state; if (pr.merged) { @@ -158,7 +173,7 @@ export async function getGitifySubjectForPullRequest( return { state: prState, - user: prComment.user.login, + user: prCommentUser, }; } From 6807b88f1532abaf9829a86b7c60b13c4e8bc076 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 18 Mar 2024 14:10:36 -0400 Subject: [PATCH 3/3] revert --- src/components/NotificationRow.tsx | 5 +---- src/components/__snapshots__/NotificationRow.test.tsx.snap | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 17c553c8a..619036530 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -84,10 +84,7 @@ export const NotificationRow: React.FC = ({ onClick={() => pressTitle()} role="main" > -
+
{notification.subject.title}
diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap index 0f107e1fd..ac12f9075 100644 --- a/src/components/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap @@ -42,7 +42,6 @@ exports[`components/Notification.js should render itself & its children 1`] = ` >
I am a robot and this is a test!