Repository: gitify-app/gitify
Pull request #1070 (2024-04-29 16:40:19)

refactor: simplify useNotifications
🧼 consolidate response handling for both github cloud and github server accounts.


Patch:
diff --git a/src/__mocks__/mockedData.ts b/src/__mocks__/mockedData.ts
index 24428a97..27ed7d6b 100644
--- a/src/__mocks__/mockedData.ts
+++ b/src/__mocks__/mockedData.ts
@@ -70,7 +70,7 @@ export const mockedCommenterUser: User = {
 };
 
 export const mockedSingleNotification: Notification = {
-  hostname: Constants.DEFAULT_AUTH_OPTIONS.hostname,
+  hostname: Constants.GITHUB_API_BASE_URL,
   id: '138661096',
   unread: true,
   reason: 'subscribed',
@@ -249,7 +249,7 @@ export const mockedGitHubNotifications = [
     url: 'https://api.github.com/notifications/threads/148827438',
     subscription_url:
       'https://api.github.com/notifications/threads/148827438/subscription',
-    hostname: Constants.DEFAULT_AUTH_OPTIONS.hostname,
+    hostname: Constants.GITHUB_API_BASE_URL,
   } as Notification,
 ];
 
@@ -359,7 +359,7 @@ export const mockedEnterpriseNotifications = [
     url: 'https://github.gitify.io/api/v3/notifications/threads/3',
     subscription_url:
       'https://github.gitify.io/api/v3/notifications/threads/3/subscription',
-    hostname: Constants.DEFAULT_AUTH_OPTIONS.hostname,
+    hostname: Constants.GITHUB_API_BASE_URL,
   } as Notification,
 ];
 
diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts
index 6e4eab85..80e13dc6 100644
--- a/src/hooks/useNotifications.ts
+++ b/src/hooks/useNotifications.ts
@@ -106,43 +106,24 @@ export const useNotifications = (): NotificationsState => {
         getGitHubNotifications(),
         ...getEnterpriseNotifications(),
       ])
-        .then(([gitHubNotifications, ...entAccNotifications]) => {
-          const enterpriseNotifications = entAccNotifications.map(
-            (accountNotifications) => {
+        .then(([...responses]) => {
+          const accountsNotifications: AccountNotifications[] = responses
+            .filter((response) => !!response)
+            .map((accountNotifications) => {
               const { hostname } = new URL(accountNotifications.config.url);
               return {
                 hostname,
                 notifications: accountNotifications.data.map(
-                  (notification: Notification) => {
-                    return {
-                      ...notification,
-                      hostname: hostname,
-                    };
-                  },
+                  (notification: Notification) => ({
+                    ...notification,
+                    hostname,
+                  }),
                 ),
               };
-            },
-          );
-
-          const data = isGitHubLoggedIn(accounts)
-            ? [
-                ...enterpriseNotifications,
-                {
-                  hostname: Constants.DEFAULT_AUTH_OPTIONS.hostname,
-                  notifications: gitHubNotifications.data.map(
-                    (notification: Notification) => {
-                      return {
-                        ...notification,
-                        hostname: Constants.DEFAULT_AUTH_OPTIONS.hostname,
-                      };
-                    },
-                  ),
-                },
-              ]
-            : [...enterpriseNotifications];
+            });
 
           Promise.all(
-            data.map(async (accountNotifications) => {
+            accountsNotifications.map(async (accountNotifications) => {
               return {
                 hostname: accountNotifications.hostname,
                 notifications: await Promise.all<Notification>(
diff --git a/src/typesGitHub.ts b/src/typesGitHub.ts
index 164033fe..4e5327d0 100644
--- a/src/typesGitHub.ts
+++ b/src/typesGitHub.ts
@@ -79,7 +79,8 @@ export interface Notification {
   repository: Repository;
   url: string;
   subscription_url: string;
-  hostname: string; // This is not in the GitHub API, but we add it to the type to make it easier to work with
+  // TODO - rename this to apiBaseUrl
+  hostname: string; // This is not in the official GitHub API. We add this to make notification interactions easier.
 }
 
 export type UserDetails = User & UserProfile;


Test patch:
diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts
index 018ea1b8..0ac0ae07 100644
--- a/src/hooks/useNotifications.test.ts
+++ b/src/hooks/useNotifications.test.ts
@@ -43,10 +43,13 @@ describe('hooks/useNotifications.ts', () => {
           expect(result.current.status).toBe('success');
         });
 
-        expect(result.current.notifications[0].hostname).toBe(
+        expect(result.current.notifications[0].hostname).toBe('api.github.com');
+        expect(result.current.notifications[0].notifications.length).toBe(2);
+
+        expect(result.current.notifications[1].hostname).toBe(
           'github.gitify.io',
         );
-        expect(result.current.notifications[1].hostname).toBe('github.com');
+        expect(result.current.notifications[1].notifications.length).toBe(2);
       });
 
       it('should fetch notifications with failures - github.com & enterprise', async () => {
@@ -117,11 +120,12 @@ describe('hooks/useNotifications.ts', () => {
         });
 
         await waitFor(() => {
-          expect(result.current.notifications[0].hostname).toBe(
-            'github.gitify.io',
-          );
+          expect(result.current.status).toBe('success');
         });
 
+        expect(result.current.notifications[0].hostname).toBe(
+          'github.gitify.io',
+        );
         expect(result.current.notifications[0].notifications.length).toBe(2);
       });
 
@@ -179,9 +183,10 @@ describe('hooks/useNotifications.ts', () => {
         });
 
         await waitFor(() => {
-          expect(result.current.notifications[0].hostname).toBe('github.com');
+          expect(result.current.status).toBe('success');
         });
 
+        expect(result.current.notifications[0].hostname).toBe('api.github.com');
         expect(result.current.notifications[0].notifications.length).toBe(2);
       });
 
@@ -379,7 +384,7 @@ describe('hooks/useNotifications.ts', () => {
           expect(result.current.status).toBe('success');
         });
 
-        expect(result.current.notifications[0].hostname).toBe('github.com');
+        expect(result.current.notifications[0].hostname).toBe('api.github.com');
         expect(result.current.notifications[0].notifications.length).toBe(6);
       });
     });
@@ -459,7 +464,7 @@ describe('hooks/useNotifications.ts', () => {
           expect(result.current.status).toBe('success');
         });
 
-        expect(result.current.notifications[0].hostname).toBe('github.com');
+        expect(result.current.notifications[0].hostname).toBe('api.github.com');
         expect(result.current.notifications[0].notifications.length).toBe(1);
       });
     });
diff --git a/src/routes/__snapshots__/Notifications.test.tsx.snap b/src/routes/__snapshots__/Notifications.test.tsx.snap
index 343e755d..1b3221ce 100644
--- a/src/routes/__snapshots__/Notifications.test.tsx.snap
+++ b/src/routes/__snapshots__/Notifications.test.tsx.snap
@@ -99,7 +99,7 @@ exports[`routes/Notifications.tsx should render itself & its children (show acco
     notifications={
       [
         {
-          "hostname": "github.com",
+          "hostname": "https://api.github.com",
           "id": "138661096",
           "last_read_at": "2017-05-20T17:06:51Z",
           "reason": "subscribed",
@@ -189,7 +189,7 @@ exports[`routes/Notifications.tsx should render itself & its children (show acco
           "url": "https://api.github.com/notifications/threads/138661096",
         },
         {
-          "hostname": "github.com",
+          "hostname": "https://api.github.com",
           "id": "148827438",
           "last_read_at": "2017-05-20T16:59:03Z",
           "reason": "author",
@@ -248,7 +248,7 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti
     notifications={
       [
         {
-          "hostname": "github.com",
+          "hostname": "https://api.github.com",
           "id": "138661096",
           "last_read_at": "2017-05-20T17:06:51Z",
           "reason": "subscribed",
@@ -338,7 +338,7 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti
           "url": "https://api.github.com/notifications/threads/138661096",
         },
         {
-          "hostname": "github.com",
+          "hostname": "https://api.github.com",
           "id": "148827438",
           "last_read_at": "2017-05-20T16:59:03Z",
           "reason": "author",
@@ -433,7 +433,7 @@ exports[`routes/Notifications.tsx should render itself & its children (with noti
           "url": "https://github.gitify.io/api/v3/notifications/threads/4",
         },
         {
-          "hostname": "github.com",
+          "hostname": "https://api.github.com",
           "id": "3",
           "last_read_at": "2017-05-20T14:20:55Z",
           "reason": "subscribed",