From 464fd9cd9569707247b633fce0fe35ba988eef8a Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 25 Mar 2024 08:09:09 -0400 Subject: [PATCH 1/3] refactor: make explicit the different user types --- src/__mocks__/mockedData.ts | 5 +-- src/components/NotificationRow.tsx | 2 +- src/types.ts | 10 ++++- src/typesGithub.ts | 62 ++++++++++++++++++++++++++++-- src/utils/auth.ts | 12 ++++-- src/utils/subject.test.ts | 24 ++++++------ src/utils/subject.ts | 21 +++++++--- 7 files changed, 106 insertions(+), 30 deletions(-) diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts index f57450b81..27ec5fa65 100644 --- a/src/__mocks__/mockedData.ts +++ b/src/__mocks__/mockedData.ts @@ -1,8 +1,7 @@ -import { AccountNotifications, EnterpriseAccount } from '../types'; +import { AccountNotifications, EnterpriseAccount, GitifyUser } from '../types'; import { Notification, Repository, - User, GraphQLSearch, DiscussionSearchResultNode, } from '../typesGithub'; @@ -14,7 +13,7 @@ export const mockedEnterpriseAccounts: EnterpriseAccount[] = [ }, ]; -export const mockedUser: User = { +export const mockedUser: GitifyUser = { login: 'octocat', name: 'Mona Lisa Octocat', id: 123456789, diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 17c553c8a..54a42474d 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -62,7 +62,7 @@ export const NotificationRow: React.FC = ({ addSuffix: true, }); const updatedBy = notification.subject.user - ? ` by ${notification.subject.user}` + ? ` by ${notification.subject.user.login}` : ''; const updatedLabel = `Updated ${updatedAt}${updatedBy}`; const notificationTitle = formatForDisplay([ diff --git a/src/types.ts b/src/types.ts index dc28c33a3..5e0eee68b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,9 +1,9 @@ -import { Notification, User } from './typesGithub'; +import { Notification } from './typesGithub'; export interface AuthState { token?: string; enterpriseAccounts: EnterpriseAccount[]; - user: User | null; + user: GitifyUser | null; } export interface SettingsState { @@ -56,3 +56,9 @@ export interface AuthTokenResponse { hostname: string; token: string; } + +export interface GitifyUser { + login: string; + name: string; + id: number; +} diff --git a/src/typesGithub.ts b/src/typesGithub.ts index 8649b543d..a85884419 100644 --- a/src/typesGithub.ts +++ b/src/typesGithub.ts @@ -81,10 +81,66 @@ export interface Notification { subscription_url: string; } +export interface UserProfile { + name: string; + company: string; + blog: string; + location: string; + email: string; + hireable: string; + bio: string; + twitter_username: string; + public_repos: number; + public_gists: number; + followers: number; + following: number; + created_at: string; + updated_at: string; + private_gists: number; + total_private_repos: number; + owned_private_repos: number; + disk_usage: number; + collaborators: number; + two_factor_authentication: boolean; + plan: Plan; +} +export interface Plan { + name: string; + space: number; + private_repos: number; + collaborators: number; +} + export interface User { login: string; name: string; id: number; + node_id: string; + avatar_url: string; + gravatar_url: string; + url: string; + html_url: string; + followers_url: string; + following_url: string; + gists_url: string; + starred_url: string; + subscriptions_url: string; + organizations_url: string; + repos_url: string; + events_url: string; + received_events_url: string; + type: UserType; + site_admin: boolean; +} + +export type UserType = 'User' | 'Bot'; + +export interface SubjectUser { + login: string; +} + +export interface DiscussionAuthor { + login: string; } export interface Repository { @@ -169,7 +225,7 @@ interface GitHubSubject { // 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; + user?: SubjectUser; } export interface PullRequest { @@ -285,7 +341,7 @@ export interface DiscussionSearchResultNode { export interface DiscussionCommentNode { databaseId: string | number; createdAt: string; - author: { login: string }; + author: DiscussionAuthor; replies: { nodes: DiscussionSubcommentNode[]; }; @@ -294,7 +350,7 @@ export interface DiscussionCommentNode { export interface DiscussionSubcommentNode { databaseId: string | number; createdAt: string; - author: { login: string }; + author: DiscussionAuthor; } export interface CheckSuiteAttributes { diff --git a/src/utils/auth.ts b/src/utils/auth.ts index f46ccc7ef..91dca49fb 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -2,9 +2,13 @@ import { BrowserWindow } from '@electron/remote'; import { generateGitHubAPIUrl, isEnterpriseHost } from './helpers'; import { apiRequest, apiRequestAuth } from '../utils/api-requests'; -import { AuthResponse, AuthState, AuthTokenResponse } from '../types'; +import { + AuthResponse, + AuthState, + AuthTokenResponse, + GitifyUser, +} from '../types'; import { Constants } from '../utils/constants'; -import { User } from '../typesGithub'; export const authGitHub = ( authOptions = Constants.DEFAULT_AUTH_OPTIONS, @@ -76,7 +80,7 @@ export const authGitHub = ( export const getUserData = async ( token: string, hostname: string, -): Promise => { +): Promise => { const response = await apiRequestAuth( `${generateGitHubAPIUrl(hostname)}user`, 'GET', @@ -112,7 +116,7 @@ export const addAccount = ( accounts: AuthState, token, hostname, - user?: User, + user?: GitifyUser, ): AuthState => { if (!isEnterpriseHost(hostname)) { return { diff --git a/src/utils/subject.test.ts b/src/utils/subject.test.ts index 657f4e3e0..2ad6dc879 100644 --- a/src/utils/subject.test.ts +++ b/src/utils/subject.test.ts @@ -435,7 +435,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('open'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('closed issue state', async () => { @@ -453,7 +453,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('closed'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('completed issue state', async () => { @@ -475,7 +475,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('completed'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('not_planned issue state', async () => { @@ -497,7 +497,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('not_planned'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('reopened issue state', async () => { @@ -519,7 +519,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('reopened'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('handle issues without latest_comment_url', async () => { @@ -544,7 +544,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('open'); - expect(result.user).toBe('some-user'); + expect(result.user).toEqual({ login: 'some-user' }); }); }); @@ -577,7 +577,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('closed'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('draft pull request state', async () => { @@ -600,7 +600,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('draft'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('merged pull request state', async () => { @@ -623,7 +623,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('merged'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('open pull request state', async () => { @@ -646,7 +646,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('open'); - expect(result.user).toBe('some-commenter'); + expect(result.user).toEqual({ login: 'some-commenter' }); }); it('handle pull request without latest_comment_url', async () => { @@ -671,7 +671,7 @@ describe('utils/subject.ts', () => { ); expect(result.state).toBe('open'); - expect(result.user).toBe('some-user'); + expect(result.user).toEqual({ login: 'some-user' }); }); }); }); @@ -698,7 +698,7 @@ describe('utils/subject.ts', () => { mockAccounts.token, ); - expect(result.user).toBe('some-user'); + expect(result.user).toEqual({ login: 'some-user' }); }); }); diff --git a/src/utils/subject.ts b/src/utils/subject.ts index 10ce5ae31..e651c1c14 100644 --- a/src/utils/subject.ts +++ b/src/utils/subject.ts @@ -107,8 +107,13 @@ async function getGitifySubjectForDiscussion( } } - const discussionUser = getLatestDiscussionComment(discussion.comments.nodes) - ?.author.login; + const latestDiscussionComment = getLatestDiscussionComment( + discussion.comments.nodes, + ); + let discussionUser = null; + if (latestDiscussionComment) { + discussionUser = latestDiscussionComment.author; + } return { state: discussionState, @@ -128,7 +133,9 @@ async function getGitifySubjectForIssue( return { state: issue.state_reason ?? issue.state, - user: issueCommentUser?.login ?? issue.user.login, + user: { + login: issueCommentUser?.login ?? issue.user.login, + }, }; } @@ -151,7 +158,9 @@ async function getGitifySubjectForPullRequest( return { state: prState, - user: prCommentUser?.login ?? pr.user.login, + user: { + login: prCommentUser?.login ?? pr.user.login, + }, }; } @@ -163,7 +172,9 @@ async function getGitifySubjectForRelease( return { state: null, - user: releaseCommentUser.login, + user: { + login: releaseCommentUser.login, + }, }; } From b18420a8384c4a05b519a6fbd7cb1aa7a89dd444 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 25 Mar 2024 08:14:11 -0400 Subject: [PATCH 2/3] refactor: make explicit the different user types --- src/typesGithub.ts | 2 ++ src/utils/auth.ts | 15 +++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/typesGithub.ts b/src/typesGithub.ts index a85884419..ad9838ba6 100644 --- a/src/typesGithub.ts +++ b/src/typesGithub.ts @@ -81,6 +81,8 @@ export interface Notification { subscription_url: string; } +export type UserDetails = User & UserProfile; + export interface UserProfile { name: string; company: string; diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 91dca49fb..394abc314 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -9,6 +9,7 @@ import { GitifyUser, } from '../types'; import { Constants } from '../utils/constants'; +import { UserDetails } from '../typesGithub'; export const authGitHub = ( authOptions = Constants.DEFAULT_AUTH_OPTIONS, @@ -81,16 +82,14 @@ export const getUserData = async ( token: string, hostname: string, ): Promise => { - const response = await apiRequestAuth( - `${generateGitHubAPIUrl(hostname)}user`, - 'GET', - token, - ); + const response: UserDetails = ( + await apiRequestAuth(`${generateGitHubAPIUrl(hostname)}user`, 'GET', token) + ).data; return { - id: response.data.id, - login: response.data.login, - name: response.data.name, + id: response.id, + login: response.login, + name: response.name, }; }; From e36dc4b1a28023658b610d3d95925fffaa2d6cc8 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 25 Mar 2024 08:28:22 -0400 Subject: [PATCH 3/3] refactor: make explicit the different user types --- src/typesGithub.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/typesGithub.ts b/src/typesGithub.ts index ad9838ba6..863933257 100644 --- a/src/typesGithub.ts +++ b/src/typesGithub.ts @@ -131,12 +131,10 @@ export interface User { repos_url: string; events_url: string; received_events_url: string; - type: UserType; + type: string; site_admin: boolean; } -export type UserType = 'User' | 'Bot'; - export interface SubjectUser { login: string; }