From 6131e571c072fc6f4af23eed93db53de44d93b88 Mon Sep 17 00:00:00 2001
From: Arthur <dufourarthur.perso@gmail.com>
Date: Sun, 18 Feb 2024 19:31:59 +0100
Subject: [PATCH 1/5] fix: remove notification from state on open (#789)

* fix: remove notification from state on open

* tests: add test for `removeNotificationFromState`
---
 src/components/NotificationRow.test.tsx |  5 ++--
 src/components/NotificationRow.tsx      |  4 +++
 src/context/App.tsx                     |  3 +++
 src/hooks/useNotifications.test.ts      | 36 +++++++++++++++++++++++++
 src/hooks/useNotifications.ts           | 16 +++++++++++
 5 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
index dcd601b16..064261881 100644
--- a/src/components/NotificationRow.test.tsx
+++ b/src/components/NotificationRow.test.tsx
@@ -31,7 +31,7 @@ describe('components/Notification.js', () => {
   });
 
   it('should open a notification in the browser', () => {
-    const markNotification = jest.fn();
+    const removeNotificationFromState = jest.fn();
 
     const props = {
       notification: mockedSingleNotification,
@@ -42,7 +42,7 @@ describe('components/Notification.js', () => {
       <AppContext.Provider
         value={{
           settings: { ...mockSettings, markAsDoneOnOpen: false },
-          markNotification,
+          removeNotificationFromState,
           accounts: mockAccounts,
         }}
       >
@@ -52,6 +52,7 @@ describe('components/Notification.js', () => {
 
     fireEvent.click(getByRole('main'));
     expect(shell.openExternal).toHaveBeenCalledTimes(1);
+    expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
   });
 
   it('should open a notification in browser & mark it as done', () => {
diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx
index 7f1dbc4a5..360df2840 100644
--- a/src/components/NotificationRow.tsx
+++ b/src/components/NotificationRow.tsx
@@ -23,6 +23,7 @@ export const NotificationRow: React.FC<IProps> = ({
   const {
     settings,
     accounts,
+    removeNotificationFromState,
     markNotification,
     markNotificationDone,
     unsubscribeNotification,
@@ -33,6 +34,9 @@ export const NotificationRow: React.FC<IProps> = ({
 
     if (settings.markAsDoneOnOpen) {
       markNotificationDone(notification.id, hostname);
+    } else {
+      // no need to mark as read, github does it by default when opening it
+      removeNotificationFromState(notification.id, hostname);
     }
   }, [settings]);
 
diff --git a/src/context/App.tsx b/src/context/App.tsx
index d009a1604..c9d466000 100644
--- a/src/context/App.tsx
+++ b/src/context/App.tsx
@@ -51,6 +51,7 @@ interface AppContextState {
   notifications: AccountNotifications[];
   isFetching: boolean;
   requestFailed: boolean;
+  removeNotificationFromState: (id: string, hostname: string) => void;
   fetchNotifications: () => Promise<void>;
   markNotification: (id: string, hostname: string) => Promise<void>;
   markNotificationDone: (id: string, hostname: string) => Promise<void>;
@@ -71,6 +72,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
     notifications,
     requestFailed,
     isFetching,
+    removeNotificationFromState,
     markNotification,
     markNotificationDone,
     unsubscribeNotification,
@@ -210,6 +212,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
         notifications,
         isFetching,
         requestFailed,
+        removeNotificationFromState,
         fetchNotifications: fetchNotificationsWithAccounts,
         markNotification: markNotificationWithAccounts,
         markNotificationDone: markNotificationDoneWithAccounts,
diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts
index c6f2b753d..ca789184e 100644
--- a/src/hooks/useNotifications.test.ts
+++ b/src/hooks/useNotifications.test.ts
@@ -269,6 +269,42 @@ describe('hooks/useNotifications.ts', () => {
     });
   });
 
+  describe('removeNotificationFromState', () => {
+    it('should remove a notification from state', async () => {
+      const notifications = [
+        { id: 1, title: 'This is a notification.' },
+        { id: 2, title: 'This is another one.' },
+      ];
+
+      nock('https://api.github.com')
+        .get('/notifications?participating=false')
+        .reply(200, notifications);
+
+      nock('https://github.gitify.io/api/v3')
+        .get('/notifications?participating=false')
+        .reply(200, notifications);
+
+      const { result } = renderHook(() => useNotifications(false));
+
+      act(() => {
+        result.current.fetchNotifications(mockAccounts, mockSettings);
+      });
+
+      await waitFor(() => {
+        expect(result.current.isFetching).toBe(false);
+      });
+
+      act(() => {
+        result.current.removeNotificationFromState(
+          result.current.notifications[0].notifications[0].id,
+          result.current.notifications[0].hostname,
+        );
+      });
+
+      expect(result.current.notifications[0].notifications.length).toBe(1);
+    });
+  });
+
   describe('markNotification', () => {
     const id = 'notification-123';
 
diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts
index 41875fb8b..08235acfb 100644
--- a/src/hooks/useNotifications.ts
+++ b/src/hooks/useNotifications.ts
@@ -19,6 +19,7 @@ import { removeNotifications } from '../utils/remove-notifications';
 
 interface NotificationsState {
   notifications: AccountNotifications[];
+  removeNotificationFromState: (id: string, hostname: string) => void;
   fetchNotifications: (
     accounts: AuthState,
     settings: SettingsState,
@@ -312,11 +313,26 @@ export const useNotifications = (colors: boolean): NotificationsState => {
     [notifications],
   );
 
+  const removeNotificationFromState = useCallback(
+    (id, hostname) => {
+      const updatedNotifications = removeNotification(
+        id,
+        notifications,
+        hostname,
+      );
+
+      setNotifications(updatedNotifications);
+      setTrayIconColor(updatedNotifications);
+    },
+    [notifications],
+  );
+
   return {
     isFetching,
     requestFailed,
     notifications,
 
+    removeNotificationFromState,
     fetchNotifications,
     markNotification,
     markNotificationDone,

From fd4d383622b6f8142d3323e6e65581251620fc0d Mon Sep 17 00:00:00 2001
From: Arthur <dufourarthur.perso@gmail.com>
Date: Sun, 18 Feb 2024 22:13:34 +0100
Subject: [PATCH 2/5] fix: init settings with `settings: null` (#787)

---
 src/context/App.test.tsx      | 4 ++--
 src/context/App.tsx           | 2 +-
 src/hooks/useNotifications.ts | 2 +-
 src/types.ts                  | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx
index 1c788a9ea..6f704f9f1 100644
--- a/src/context/App.test.tsx
+++ b/src/context/App.test.tsx
@@ -290,7 +290,7 @@ describe('context/App.tsx', () => {
         participating: true,
         playSound: true,
         showNotifications: true,
-        colors: false,
+        colors: null,
         markAsDoneOnOpen: false,
       },
     );
@@ -328,7 +328,7 @@ describe('context/App.tsx', () => {
         participating: false,
         playSound: true,
         showNotifications: true,
-        colors: false,
+        colors: null,
         markAsDoneOnOpen: false,
       },
     );
diff --git a/src/context/App.tsx b/src/context/App.tsx
index c9d466000..72978d937 100644
--- a/src/context/App.tsx
+++ b/src/context/App.tsx
@@ -36,7 +36,7 @@ export const defaultSettings: SettingsState = {
   showNotifications: true,
   openAtStartup: false,
   appearance: Appearance.SYSTEM,
-  colors: false,
+  colors: null,
   markAsDoneOnOpen: false,
 };
 
diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts
index 08235acfb..5634d2591 100644
--- a/src/hooks/useNotifications.ts
+++ b/src/hooks/useNotifications.ts
@@ -105,7 +105,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
                 ]
               : [...enterpriseNotifications];
 
-            if (!colors) {
+            if (colors === false) {
               setNotifications(data);
               triggerNativeNotifications(
                 notifications,
diff --git a/src/types.ts b/src/types.ts
index 5925a24f7..dc28c33a3 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -12,7 +12,7 @@ export interface SettingsState {
   showNotifications: boolean;
   openAtStartup: boolean;
   appearance: Appearance;
-  colors: boolean;
+  colors: boolean | null;
   markAsDoneOnOpen: boolean;
 }
 

From 2f925969c2abfc8463a89c79646bf2997076ec74 Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Sun, 18 Feb 2024 20:14:45 -0500
Subject: [PATCH 3/5] refactor: is enterprise host fn (#791)

---
 src/hooks/useNotifications.ts | 15 ++++++++-------
 src/utils/auth.ts             |  4 ++--
 src/utils/helpers.test.ts     | 13 +++++++++++++
 src/utils/helpers.ts          |  9 ++++++---
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts
index 5634d2591..20d1ae88e 100644
--- a/src/hooks/useNotifications.ts
+++ b/src/hooks/useNotifications.ts
@@ -8,6 +8,7 @@ import { apiRequestAuth } from '../utils/api-requests';
 import {
   getEnterpriseAccountToken,
   generateGitHubAPIUrl,
+  isEnterpriseHost,
 } from '../utils/helpers';
 import { removeNotification } from '../utils/remove-notification';
 import {
@@ -130,9 +131,9 @@ export const useNotifications = (colors: boolean): NotificationsState => {
                           ) {
                             return notification;
                           }
-                          const isEnterprise =
-                            accountNotifications.hostname !==
-                            Constants.DEFAULT_AUTH_OPTIONS.hostname;
+                          const isEnterprise = isEnterpriseHost(
+                            accountNotifications.hostname,
+                          );
                           const token = isEnterprise
                             ? getEnterpriseAccountToken(
                                 accountNotifications.hostname,
@@ -192,7 +193,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
     async (accounts, id, hostname) => {
       setIsFetching(true);
 
-      const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
+      const isEnterprise = isEnterpriseHost(hostname);
       const token = isEnterprise
         ? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
         : accounts.token;
@@ -225,7 +226,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
     async (accounts, id, hostname) => {
       setIsFetching(true);
 
-      const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
+      const isEnterprise = isEnterpriseHost(hostname);
       const token = isEnterprise
         ? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
         : accounts.token;
@@ -258,7 +259,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
     async (accounts, id, hostname) => {
       setIsFetching(true);
 
-      const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
+      const isEnterprise = isEnterpriseHost(hostname);
       const token = isEnterprise
         ? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
         : accounts.token;
@@ -284,7 +285,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
     async (accounts, repoSlug, hostname) => {
       setIsFetching(true);
 
-      const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
+      const isEnterprise = isEnterpriseHost(hostname);
       const token = isEnterprise
         ? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
         : accounts.token;
diff --git a/src/utils/auth.ts b/src/utils/auth.ts
index 96d83242f..f46ccc7ef 100644
--- a/src/utils/auth.ts
+++ b/src/utils/auth.ts
@@ -1,6 +1,6 @@
 import { BrowserWindow } from '@electron/remote';
 
-import { generateGitHubAPIUrl } from './helpers';
+import { generateGitHubAPIUrl, isEnterpriseHost } from './helpers';
 import { apiRequest, apiRequestAuth } from '../utils/api-requests';
 import { AuthResponse, AuthState, AuthTokenResponse } from '../types';
 import { Constants } from '../utils/constants';
@@ -114,7 +114,7 @@ export const addAccount = (
   hostname,
   user?: User,
 ): AuthState => {
-  if (hostname === Constants.DEFAULT_AUTH_OPTIONS.hostname) {
+  if (!isEnterpriseHost(hostname)) {
     return {
       ...accounts,
       token,
diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts
index f9a60b455..ebaa908e7 100644
--- a/src/utils/helpers.test.ts
+++ b/src/utils/helpers.test.ts
@@ -4,6 +4,7 @@ import {
   generateNotificationReferrerId,
   getCommentId,
   getLatestDiscussionCommentId,
+  isEnterpriseHost,
 } from './helpers';
 import {
   mockedSingleNotification,
@@ -23,6 +24,18 @@ const URL = {
 };
 
 describe('utils/helpers.ts', () => {
+  describe('isEnterpriseHost', () => {
+    it('should return true for enterprise host', () => {
+      expect(isEnterpriseHost('github.manos.im')).toBe(true);
+      expect(isEnterpriseHost('api.github.manos.im')).toBe(true);
+    });
+
+    it('should return false for non-enterprise host', () => {
+      expect(isEnterpriseHost('github.com')).toBe(false);
+      expect(isEnterpriseHost('api.github.com')).toBe(false);
+    });
+  });
+
   describe('generateNotificationReferrerId', () => {
     it('should generate the notification_referrer_id', () => {
       const referrerId = generateNotificationReferrerId(
diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts
index 708aac3af..e84fd3a56 100644
--- a/src/utils/helpers.ts
+++ b/src/utils/helpers.ts
@@ -15,8 +15,12 @@ export function getEnterpriseAccountToken(
   return accounts.find((obj) => obj.hostname === hostname).token;
 }
 
+export function isEnterpriseHost(hostname: string): boolean {
+  return !hostname.endsWith(Constants.DEFAULT_AUTH_OPTIONS.hostname);
+}
+
 export function generateGitHubAPIUrl(hostname) {
-  const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
+  const isEnterprise = isEnterpriseHost(hostname);
   return isEnterprise
     ? `https://${hostname}/api/v3/`
     : `https://api.${hostname}/`;
@@ -39,8 +43,7 @@ export function generateGitHubWebUrl(
   comment: string = '',
 ) {
   const { hostname } = new URL(url);
-  const isEnterprise =
-    hostname !== `api.${Constants.DEFAULT_AUTH_OPTIONS.hostname}`;
+  const isEnterprise = isEnterpriseHost(hostname);
 
   let newUrl: string = isEnterprise
     ? url.replace(`${hostname}/api/v3/repos`, hostname)

From 102c934539d0b228bfcf9d288e6171778817a4c3 Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Sun, 18 Feb 2024 20:15:08 -0500
Subject: [PATCH 4/5] test: add notification color coverage (#790)

---
 src/typesGithub.ts                            |  3 +++
 .../__snapshots__/github-api.test.ts.snap     | 21 +++++++++++++++++++
 src/utils/github-api.test.ts                  | 17 ++++++++++++++-
 src/utils/github-api.ts                       | 16 +++++++-------
 4 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/src/typesGithub.ts b/src/typesGithub.ts
index 3f4ac637f..98dfa7f51 100644
--- a/src/typesGithub.ts
+++ b/src/typesGithub.ts
@@ -28,8 +28,11 @@ export type IssueStateType =
   | 'completed'
   | 'reopened'
   | 'not_planned';
+
 export type PullRequestStateType = 'closed' | 'open' | 'merged' | 'draft';
+
 export type StateType = IssueStateType | PullRequestStateType;
+
 export type ViewerSubscription = 'IGNORED' | 'SUBSCRIBED' | 'UNSUBSCRIBED';
 
 export interface Notification {
diff --git a/src/utils/__snapshots__/github-api.test.ts.snap b/src/utils/__snapshots__/github-api.test.ts.snap
index 8b1b1033a..327ef366d 100644
--- a/src/utils/__snapshots__/github-api.test.ts.snap
+++ b/src/utils/__snapshots__/github-api.test.ts.snap
@@ -1,5 +1,26 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`./utils/github-api.ts should format the notification color 1`] = `"text-red-500"`;
+
+exports[`./utils/github-api.ts should format the notification color 2`] = `"text-purple-500"`;
+
+exports[`./utils/github-api.ts should format the notification color 3`] = `"text-gray-600"`;
+
+exports[`./utils/github-api.ts should format the notification color 4`] = `"text-purple-500"`;
+
+exports[`./utils/github-api.ts should format the notification color 5`] = `"text-gray-300"`;
+
+exports[`./utils/github-api.ts should format the notification color 6`] = `"text-green-500"`;
+
+exports[`./utils/github-api.ts should format the notification color 7`] = `"text-green-500"`;
+
+exports[`./utils/github-api.ts should format the notification color 8`] = `
+{
+  "description": "The reason for this notification is not supported by the app.",
+  "type": "Unknown",
+}
+`;
+
 exports[`./utils/github-api.ts should format the notification reason 1`] = `
 {
   "description": "You were assigned to the issue.",
diff --git a/src/utils/github-api.test.ts b/src/utils/github-api.test.ts
index fec6cca3b..25ccac95c 100644
--- a/src/utils/github-api.test.ts
+++ b/src/utils/github-api.test.ts
@@ -1,4 +1,8 @@
-import { formatReason, getNotificationTypeIcon } from './github-api';
+import {
+  formatReason,
+  getNotificationTypeIcon,
+  getNotificationTypeIconColor,
+} from './github-api';
 import { Reason, SubjectType } from '../typesGithub';
 
 describe('./utils/github-api.ts', () => {
@@ -62,4 +66,15 @@ describe('./utils/github-api.ts', () => {
       'QuestionIcon',
     );
   });
+
+  it('should format the notification color', () => {
+    expect(getNotificationTypeIconColor('closed')).toMatchSnapshot();
+    expect(getNotificationTypeIconColor('completed')).toMatchSnapshot();
+    expect(getNotificationTypeIconColor('draft')).toMatchSnapshot();
+    expect(getNotificationTypeIconColor('merged')).toMatchSnapshot();
+    expect(getNotificationTypeIconColor('not_planned')).toMatchSnapshot();
+    expect(getNotificationTypeIconColor('open')).toMatchSnapshot();
+    expect(getNotificationTypeIconColor('reopened')).toMatchSnapshot();
+    expect(formatReason('something_else_unknown' as Reason)).toMatchSnapshot();
+  });
 });
diff --git a/src/utils/github-api.ts b/src/utils/github-api.ts
index 462f39e78..ea1adca02 100644
--- a/src/utils/github-api.ts
+++ b/src/utils/github-api.ts
@@ -119,18 +119,18 @@ 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';
+    case 'merged':
+      return 'text-purple-500';
+    case 'not_planned':
+      return 'text-gray-300';
+    case 'open':
+      return 'text-green-500';
+    case 'reopened':
+      return 'text-green-500';
     default:
       return 'text-gray-300';
   }

From 7efe0d98dc020475ba4740bb733df6c494324d70 Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Mon, 19 Feb 2024 06:25:32 -0500
Subject: [PATCH 5/5] test: api request coverage (#792)

---
 .../__snapshots__/api-requests.test.ts.snap   | 18 ++++++++
 src/utils/api-requests.test.ts                | 41 +++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 src/utils/__snapshots__/api-requests.test.ts.snap
 create mode 100644 src/utils/api-requests.test.ts

diff --git a/src/utils/__snapshots__/api-requests.test.ts.snap b/src/utils/__snapshots__/api-requests.test.ts.snap
new file mode 100644
index 000000000..3283b8769
--- /dev/null
+++ b/src/utils/__snapshots__/api-requests.test.ts.snap
@@ -0,0 +1,18 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`apiRequest should make a request with the correct parameters 1`] = `
+{
+  "Accept": "application/json",
+  "Cache-Control": "no-cache",
+  "Content-Type": "application/json",
+}
+`;
+
+exports[`apiRequestAuth should make an authenticated request with the correct parameters 1`] = `
+{
+  "Accept": "application/json",
+  "Authorization": "token yourAuthToken",
+  "Cache-Control": "no-cache",
+  "Content-Type": "application/json",
+}
+`;
diff --git a/src/utils/api-requests.test.ts b/src/utils/api-requests.test.ts
new file mode 100644
index 000000000..635588258
--- /dev/null
+++ b/src/utils/api-requests.test.ts
@@ -0,0 +1,41 @@
+import axios from 'axios';
+import { apiRequest, apiRequestAuth } from './api-requests';
+
+jest.mock('axios');
+
+describe('apiRequest', () => {
+  it('should make a request with the correct parameters', async () => {
+    const url = 'https://example.com';
+    const method = 'get';
+    const data = { key: 'value' };
+
+    await apiRequest(url, method, data);
+
+    expect(axios).toHaveBeenCalledWith({
+      method,
+      url,
+      data,
+    });
+
+    expect(axios.defaults.headers.common).toMatchSnapshot();
+  });
+});
+
+describe('apiRequestAuth', () => {
+  it('should make an authenticated request with the correct parameters', async () => {
+    const url = 'https://example.com';
+    const method = 'get';
+    const token = 'yourAuthToken';
+    const data = { key: 'value' };
+
+    await apiRequestAuth(url, method, token, data);
+
+    expect(axios).toHaveBeenCalledWith({
+      method,
+      url,
+      data,
+    });
+
+    expect(axios.defaults.headers.common).toMatchSnapshot();
+  });
+});