From e87fb022b69b333c0cd7edfdf75741f01244c765 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Fri, 12 May 2023 17:15:32 +0100 Subject: [PATCH 1/7] chore: add required types --- src/__mocks__/mock-state.ts | 1 + src/__mocks__/mockedData.ts | 1 + src/types.ts | 1 + src/typesGithub.ts | 10 ++++++++++ 4 files changed, 13 insertions(+) diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts index 3b2f00ced..91043e1a9 100644 --- a/src/__mocks__/mock-state.ts +++ b/src/__mocks__/mock-state.ts @@ -19,4 +19,5 @@ export const mockSettings: SettingsState = { markOnClick: false, openAtStartup: false, appearance: Appearance.SYSTEM, + colors: false, }; diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts index c18d0184a..f1afdc857 100644 --- a/src/__mocks__/mockedData.ts +++ b/src/__mocks__/mockedData.ts @@ -26,6 +26,7 @@ export const mockedSingleNotification: Notification = { url: 'https://api.github.com/repos/manosim/notifications-test/issues/1', latest_comment_url: 'https://api.github.com/repos/manosim/notifications-test/issues/comments/302888448', type: 'Issue', + state: 'open', }, repository: { id: 57216596, diff --git a/src/types.ts b/src/types.ts index 1d472051d..d3e881b9e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -13,6 +13,7 @@ export interface SettingsState { markOnClick: boolean; openAtStartup: boolean; appearance: Appearance; + colors: boolean; } export enum Appearance { diff --git a/src/typesGithub.ts b/src/typesGithub.ts index 1ff4e7810..521d45346 100644 --- a/src/typesGithub.ts +++ b/src/typesGithub.ts @@ -21,6 +21,15 @@ export type SubjectType = | 'Release' | 'RepositoryVulnerabilityAlert'; +export type IssueStateType = + | 'closed' + | 'open' + | 'completed' + | 'reopened' + | 'not_planned'; +export type PullRequestStateType = 'closed' | 'open' | 'merged' | 'draft'; +export type StateType = IssueStateType | PullRequestStateType; + export interface Notification { id: string; unread: boolean; @@ -112,6 +121,7 @@ export interface Owner { export interface Subject { title: string; url?: string; + state: StateType; latest_comment_url?: string; type: SubjectType; } From ecc53ebdb3986cc0a42cafae5acbf4a90d7198e3 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Fri, 12 May 2023 17:16:18 +0100 Subject: [PATCH 2/7] feat: get color based on state --- src/utils/github-api.ts | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/utils/github-api.ts b/src/utils/github-api.ts index 73737520f..f6c500d73 100644 --- a/src/utils/github-api.ts +++ b/src/utils/github-api.ts @@ -1,4 +1,4 @@ -import { Reason, SubjectType } from '../typesGithub'; +import { Reason, StateType, SubjectType } from '../typesGithub'; import * as Octicons from '@primer/octicons-react'; // prettier-ignore @@ -74,3 +74,24 @@ export function getNotificationTypeIcon( return Octicons.QuestionIcon; } } + +export function getNotificationTypeIconColor(state: StateType): string { + switch (state) { + case 'closed': + return 'text-red-500'; + case 'open': + return 'text-green-500'; + case 'merged': + return 'text-purple-500'; + case 'reopened': + return 'text-green-500'; + case 'not_planned': + return 'text-gray-300'; + case 'completed': + return 'text-purple-500'; + case 'draft': + return 'text-gray-600'; + default: + return 'text-gray-300'; + } +} From 2d8bd9b9be2b7f146486d33e69110f1b43fcbe4f Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Fri, 12 May 2023 17:17:23 +0100 Subject: [PATCH 3/7] feat: add color setting --- src/context/App.test.tsx | 2 ++ src/context/App.tsx | 4 ++- src/routes/Settings.tsx | 6 +++++ .../__snapshots__/Settings.test.tsx.snap | 25 +++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 158388cf8..cb12bd9aa 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -253,6 +253,7 @@ describe('context/App.tsx', () => { participating: true, playSound: true, showNotifications: true, + colors: true, } ); }); @@ -289,6 +290,7 @@ describe('context/App.tsx', () => { participating: false, playSound: true, showNotifications: true, + colors: true, } ); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index f3191fd3b..7e933421e 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -36,6 +36,7 @@ export const defaultSettings: SettingsState = { markOnClick: false, openAtStartup: false, appearance: Appearance.SYSTEM, + colors: true, }; interface AppContextState { @@ -71,7 +72,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { markNotification, unsubscribeNotification, markRepoNotifications, - } = useNotifications(); + } = useNotifications(settings.colors); useEffect(() => { restoreSettings(); @@ -160,6 +161,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { if (existing.settings) { setSettings({ ...defaultSettings, ...existing.settings }); + return existing.settings; } }, []); diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx index e15e2316a..28ee76e35 100644 --- a/src/routes/Settings.tsx +++ b/src/routes/Settings.tsx @@ -71,6 +71,12 @@ export const SettingsRoute: React.FC = () => { }} /> + updateSetting('colors', evt.target.checked)} + /> +
+
+ +
+
+ +
+
From de46b210af878d86b3369dc1bbd603dcee33fe11 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Fri, 12 May 2023 17:34:07 +0100 Subject: [PATCH 4/7] feat: state fetching for PRs and issues --- src/hooks/useNotifications.test.ts | 125 ++++++++++++++++++++++++----- src/hooks/useNotifications.ts | 75 ++++++++++++++--- 2 files changed, 173 insertions(+), 27 deletions(-) diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index 73f12c42e..ad81dff0e 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -29,7 +29,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200, notifications); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -59,7 +59,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400, { message }); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -92,7 +92,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200, notifications); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -118,7 +118,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400, { message: 'Oops! Something went wrong.' }); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -149,7 +149,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200, notifications); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -173,7 +173,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400, { message: 'Oops! Something went wrong.' }); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -185,6 +185,95 @@ describe('hooks/useNotifications.ts', () => { expect(result.current.requestFailed).toBe(true); }); }); + + describe('with colors', () => { + it('should fetch notifications with success - with colors', async () => { + const accounts: AuthState = { + ...mockAccounts, + enterpriseAccounts: [], + user: mockedUser, + }; + + const notifications = [ + { + id: 1, + title: 'This is a notification.', + subject: { type: 'Issue', url: 'https://api.github.com/1' }, + }, + { + id: 2, + title: 'A merged PR.', + subject: { type: 'PullRequest', url: 'https://api.github.com/2' }, + }, + { + id: 3, + title: 'A closed PR.', + subject: { type: 'PullRequest', url: 'https://api.github.com/3' }, + }, + { + id: 4, + title: 'A draft PR.', + subject: { type: 'PullRequest', url: 'https://api.github.com/4' }, + }, + { + id: 5, + title: 'A draft PR.', + subject: { type: 'PullRequest', url: 'https://api.github.com/5' }, + }, + ]; + + nock('https://api.github.com') + .get('/notifications?participating=false') + .reply(200, notifications); + + nock('https://api.github.com').get('/1').reply(200, { state: 'open' }); + nock('https://api.github.com') + .get('/2') + .reply(200, { state: 'closed', merged: true }); + nock('https://api.github.com') + .get('/3') + .reply(200, { state: 'closed', merged: false }); + nock('https://api.github.com') + .get('/4') + .reply(200, { state: 'open', draft: false }); + nock('https://api.github.com') + .get('/5') + .reply(200, { state: 'open', draft: true }); + + const { result, waitForNextUpdate } = renderHook(() => + useNotifications(true) + ); + + act(() => { + result.current.fetchNotifications(accounts, { + ...mockSettings, + colors: true, + }); + }); + + expect(result.current.isFetching).toBe(true); + + await waitForNextUpdate(); + + expect(result.current.notifications[0].hostname).toBe('github.com'); + expect(result.current.notifications[0].notifications.length).toBe(5); + expect( + result.current.notifications[0].notifications[0].subject.state + ).toBe('open'); + expect( + result.current.notifications[0].notifications[1].subject.state + ).toBe('merged'); + expect( + result.current.notifications[0].notifications[2].subject.state + ).toBe('closed'); + expect( + result.current.notifications[0].notifications[3].subject.state + ).toBe('open'); + expect( + result.current.notifications[0].notifications[4].subject.state + ).toBe('draft'); + }); + }); }); describe('markNotification', () => { @@ -200,7 +289,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -218,7 +307,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -241,7 +330,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -259,7 +348,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -292,7 +381,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200); const { result, waitForValueToChange } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -318,7 +407,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400); const { result, waitForValueToChange } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -349,7 +438,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200); const { result, waitForValueToChange } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -375,7 +464,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400); const { result, waitForValueToChange } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -404,7 +493,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -422,7 +511,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -445,7 +534,7 @@ describe('hooks/useNotifications.ts', () => { .reply(200); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { @@ -463,7 +552,7 @@ describe('hooks/useNotifications.ts', () => { .reply(400); const { result, waitForNextUpdate } = renderHook(() => - useNotifications() + useNotifications(false) ); act(() => { diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 88e670cb2..c2a3e8112 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -3,6 +3,7 @@ import { parse } from 'url'; import { useCallback, useState } from 'react'; import { AccountNotifications, AuthState, SettingsState } from '../types'; +import { Notification } from '../typesGithub'; import { apiRequestAuth } from '../utils/api-requests'; import { getEnterpriseAccountToken, @@ -41,7 +42,7 @@ interface NotificationsState { requestFailed: boolean; } -export const useNotifications = (): NotificationsState => { +export const useNotifications = (colors: boolean): NotificationsState => { const [isFetching, setIsFetching] = useState(false); const [requestFailed, setRequestFailed] = useState(false); const [notifications, setNotifications] = useState( @@ -96,14 +97,70 @@ export const useNotifications = (): NotificationsState => { ] : [...enterpriseNotifications]; - triggerNativeNotifications( - notifications, - data, - settings, - accounts.user - ); - setNotifications(data); - setIsFetching(false); + if (!colors) { + setNotifications(data); + triggerNativeNotifications( + notifications, + data, + settings, + accounts.user + ); + setIsFetching(false); + return; + } + axios + .all( + data.map(async (accountNotifications) => { + return { + hostname: accountNotifications.hostname, + notifications: await axios.all( + accountNotifications.notifications.map( + async (notification: Notification) => { + if ( + notification.subject.type !== 'PullRequest' && + notification.subject.type !== 'Issue' + ) { + return notification; + } + + const cardinalData = ( + await apiRequestAuth( + notification.subject.url, + 'GET', + accounts.token + ) + ).data; + + const state = + cardinalData.state === 'closed' + ? cardinalData.state_reason || + (cardinalData.merged && 'merged') || + 'closed' + : (cardinalData.draft && 'draft') || 'open'; + + return { + ...notification, + subject: { + ...notification.subject, + state, + }, + }; + } + ) + ), + }; + }) + ) + .then((parsedNotifications) => { + setNotifications(parsedNotifications); + triggerNativeNotifications( + notifications, + parsedNotifications, + settings, + accounts.user + ); + setIsFetching(false); + }); }) ) .catch(() => { From 8630295e471f29f9ee2be22690eb7e0dafe8d887 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Fri, 12 May 2023 17:34:38 +0100 Subject: [PATCH 5/7] feat: color switching --- src/components/NotificationRow.tsx | 10 ++++++++-- .../__snapshots__/NotificationRow.test.tsx.snap | 2 +- src/routes/__snapshots__/Notifications.test.tsx.snap | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index eb23e75d5..c129e5afb 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -4,7 +4,11 @@ import React, { useCallback, useContext } from 'react'; import { formatDistanceToNow, parseISO } from 'date-fns'; import { CheckIcon, MuteIcon } from '@primer/octicons-react'; -import { formatReason, getNotificationTypeIcon } from '../utils/github-api'; +import { + formatReason, + getNotificationTypeIcon, + getNotificationTypeIconColor, +} from '../utils/github-api'; import { generateGitHubWebUrl } from '../utils/helpers'; import { Notification } from '../typesGithub'; import { AppContext } from '../context/App'; @@ -50,13 +54,15 @@ export const NotificationRow: React.FC = ({ const reason = formatReason(notification.reason); const NotificationIcon = getNotificationTypeIcon(notification.subject.type); + const iconColor = + settings.colors && getNotificationTypeIconColor(notification.subject.state); const updatedAt = formatDistanceToNow(parseISO(notification.updated_at), { addSuffix: true, }); return (
-
+
diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap index e9728a966..15fd3fb4d 100644 --- a/src/components/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap @@ -5,7 +5,7 @@ exports[`components/Notification.js should render itself & its children 1`] = ` className="flex space-x-2 p-2 bg-white dark:bg-gray-dark dark:text-white hover:bg-gray-100 dark:hover:bg-gray-darker border-b border-gray-100 dark:border-gray-darker" >
Date: Thu, 14 Sep 2023 03:11:32 +0100 Subject: [PATCH 6/7] style: prettify files --- src/routes/Notifications.tsx | 14 ++++++++------ src/utils/github-api.ts | 7 ++++--- src/utils/notifications.test.ts | 9 +++++---- src/utils/notifications.ts | 5 ++--- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/routes/Notifications.tsx b/src/routes/Notifications.tsx index 247aa5695..d028744a1 100644 --- a/src/routes/Notifications.tsx +++ b/src/routes/Notifications.tsx @@ -8,17 +8,19 @@ import { Oops } from '../components/Oops'; export const NotificationsRoute: React.FC = (props) => { const { notifications, requestFailed } = useContext(AppContext); - const hasMultipleAccounts = useMemo(() => notifications.length > 1, [ - notifications, - ]); + const hasMultipleAccounts = useMemo( + () => notifications.length > 1, + [notifications] + ); const notificationsCount = useMemo( () => notifications.reduce((memo, acc) => memo + acc.notifications.length, 0), [notifications] ); - const hasNotifications = useMemo(() => notificationsCount > 0, [ - notificationsCount, - ]); + const hasNotifications = useMemo( + () => notificationsCount > 0, + [notificationsCount] + ); if (requestFailed) { return ; diff --git a/src/utils/github-api.ts b/src/utils/github-api.ts index f6c500d73..09ba269e3 100644 --- a/src/utils/github-api.ts +++ b/src/utils/github-api.ts @@ -18,9 +18,10 @@ const DESCRIPTIONS = { UNKNOWN: 'The reason for this notification is not supported by the app.', }; -export function formatReason( - reason: Reason -): { type: string; description: string } { +export function formatReason(reason: Reason): { + type: string; + description: string; +} { // prettier-ignore switch (reason) { case 'assign': diff --git a/src/utils/notifications.test.ts b/src/utils/notifications.test.ts index 86bea2f3a..89b75ff56 100644 --- a/src/utils/notifications.test.ts +++ b/src/utils/notifications.test.ts @@ -104,10 +104,11 @@ describe('utils/notifications.ts', () => { it('should click on a native notification (with 1 notification)', () => { spyOn(comms, 'openExternalLink'); - const nativeNotification: Notification = notificationsHelpers.raiseNativeNotification( - [mockedGithubNotifications[0]], - mockedUser.id - ); + const nativeNotification: Notification = + notificationsHelpers.raiseNativeNotification( + [mockedGithubNotifications[0]], + mockedUser.id + ); nativeNotification.onclick(null); const notif = mockedGithubNotifications[0]; diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 5e9a4e12c..54d58ecce 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -31,9 +31,8 @@ export const triggerNativeNotifications = ( return account.notifications; } - const accountPreviousNotificationsIds = accountPreviousNotifications.notifications.map( - (item) => item.id - ); + const accountPreviousNotificationsIds = + accountPreviousNotifications.notifications.map((item) => item.id); const accountNewNotifications = account.notifications.filter((item) => { return !accountPreviousNotificationsIds.includes(`${item.id}`); From 0f428c4ac4ed33b127b4429260ffcfa5b0bccb2f Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 14 Sep 2023 15:46:44 +0100 Subject: [PATCH 7/7] chore: push color fix --- src/components/NotificationRow.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 3420e3ee8..7b88093ff 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -7,7 +7,6 @@ import { getNotificationTypeIcon, getNotificationTypeIconColor, } from '../utils/github-api'; -import { generateGitHubWebUrl } from '../utils/helpers'; import { openInBrowser } from '../utils/helpers'; import { Notification } from '../typesGithub'; import { AppContext } from '../context/App'; @@ -46,15 +45,17 @@ export const NotificationRow: React.FC = ({ const reason = formatReason(notification.reason); const NotificationIcon = getNotificationTypeIcon(notification.subject.type); - const iconColor = - settings.colors && getNotificationTypeIconColor(notification.subject.state); + const iconColor = getNotificationTypeIconColor(notification.subject.state); + const realIconColor = settings + ? (settings.colors && iconColor) || '' + : iconColor; const updatedAt = formatDistanceToNow(parseISO(notification.updated_at), { addSuffix: true, }); return (
-
+