Repository: gitify-app/gitify
Pull request #1112 (2024-05-14 12:53:40)

feat: improve enterprise login
Fixed the following issues with GitHub Enterprise.

1. Settings now support logging in with pat, or logging in with oauth app, after the initial login.
2. Fixed a bug that caused the settings page to go blank when logged in only to github enterprise. Closes https://github.com/gitify-app/gitify/issues/1130
3. ~Multiple logins to github enterprise are now possible with a personal token and the github app.~ planned to 5.7.x



setchy: thank for your patience @mashiro.  fyi - i'll push an update to this feature branch shortly to help move this enhancement along.  watch this space 😃 
setchy: Updates pushed.  @mashiro @afonsojramos - appreciate some feedback.  

the Settings changes now support logging in with pat, or logging in with oauth app, after the initial login.

it is still constrained to at-most one of each.  in v5.7.x we'll focus on adding account views and a new account type model to support adding any number of each. 💪 
mashiro: @setchy Looks good!

Patch:
diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx
index 910569b0..d4503f48 100644
--- a/src/routes/Settings.tsx
+++ b/src/routes/Settings.tsx
@@ -1,6 +1,8 @@
 import {
   ArrowLeftIcon,
-  PersonAddIcon,
+  KeyIcon,
+  PersonIcon,
+  PlusIcon,
   SignOutIcon,
   XCircleIcon,
 } from '@primer/octicons-react';
@@ -26,6 +28,10 @@ import {
   updateTrayTitle,
 } from '../utils/comms';
 import Constants from '../utils/constants';
+import {
+  isOAuthAppLoggedIn,
+  isPersonalAccessTokenLoggedIn,
+} from '../utils/helpers';
 import { setTheme } from '../utils/theme';
 
 export const SettingsRoute: FC = () => {
@@ -81,6 +87,10 @@ export const SettingsRoute: FC = () => {
     ipcRenderer.send('app-quit');
   }, []);
 
+  const loginWithPersonalAccessToken = useCallback(() => {
+    return navigate('/login-personal-access-token', { replace: true });
+  }, []);
+
   const loginWithOAuthApp = useCallback(() => {
     return navigate('/login-oauth-app', { replace: true });
   }, []);
@@ -290,26 +300,36 @@ export const SettingsRoute: FC = () => {
           Gitify v{appVersion}
         </button>
         <div>
+          <button
+            type="button"
+            className={footerButtonClass}
+            title="Login with Personal Access Token"
+            onClick={loginWithPersonalAccessToken}
+            hidden={isPersonalAccessTokenLoggedIn(accounts)}
+          >
+            <KeyIcon size={18} aria-label="Login with Personal Access Token" />
+            <PlusIcon size={10} className="ml-1 mb-2" />
+          </button>
+
           <button
             type="button"
             className={footerButtonClass}
             title="Login with OAuth App"
             onClick={loginWithOAuthApp}
+            hidden={isOAuthAppLoggedIn(accounts)}
           >
-            <PersonAddIcon size={20} aria-label="Login with OAuth App" />
+            <PersonIcon size={20} aria-label="Login with OAuth App" />
+            <PlusIcon size={10} className="mb-2" />
           </button>
 
           <button
             type="button"
             className={footerButtonClass}
-            title={`Logout from ${accounts.user.login}`}
+            title="Logout"
             role="button"
             onClick={logoutUser}
           >
-            <SignOutIcon
-              size={18}
-              aria-label={`Logout from ${accounts.user.login}`}
-            />
+            <SignOutIcon size={18} aria-label="Logout" />
           </button>
 
           <button
diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts
index 7a8a1ee5..7101c063 100644
--- a/src/utils/helpers.ts
+++ b/src/utils/helpers.ts
@@ -10,10 +10,14 @@ import {
   getWorkflowRunAttributes,
 } from './subject';
 
-export function isGitHubLoggedIn(accounts: AuthState): boolean {
+export function isPersonalAccessTokenLoggedIn(accounts: AuthState): boolean {
   return accounts.token != null;
 }
 
+export function isOAuthAppLoggedIn(accounts: AuthState): boolean {
+  return accounts.enterpriseAccounts?.length > 0;
+}
+
 export function getTokenForHost(hostname: string, accounts: AuthState): string {
   const isEnterprise = isEnterpriseHost(hostname);
 
diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts
index 142ec359..9b6b3409 100644
--- a/src/utils/notifications.ts
+++ b/src/utils/notifications.ts
@@ -2,7 +2,7 @@ import { ipcRenderer } from 'electron';
 import { Notification } from '../typesGitHub';
 import {
   getTokenForHost,
-  isGitHubLoggedIn,
+  isPersonalAccessTokenLoggedIn,
   openInBrowser,
 } from '../utils/helpers';
 import { updateTrayIcon } from './comms';
@@ -110,7 +110,7 @@ export const raiseSoundNotification = () => {
 };
 
 function getGitHubNotifications(accounts: AuthState, settings: SettingsState) {
-  if (!isGitHubLoggedIn(accounts)) {
+  if (!isPersonalAccessTokenLoggedIn(accounts)) {
     return;
   }
 


Test patch:
diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx
index 3484ef88..64ee49c9 100644
--- a/src/routes/Settings.test.tsx
+++ b/src/routes/Settings.test.tsx
@@ -450,25 +450,104 @@ describe('routes/Settings.tsx', () => {
       );
     });
 
-    it('should go to the login with oauth app route', async () => {
-      await act(async () => {
-        render(
-          <AppContext.Provider
-            value={{
-              settings: mockSettings,
-              accounts: mockAccounts,
-            }}
-          >
-            <MemoryRouter>
-              <SettingsRoute />
-            </MemoryRouter>
-          </AppContext.Provider>,
+    describe('Login with Personal Access Token', () => {
+      it('should show login with personal access token button if not logged in', async () => {
+        await act(async () => {
+          render(
+            <AppContext.Provider
+              value={{
+                settings: mockSettings,
+                accounts: { ...mockAccounts, token: null },
+              }}
+            >
+              <MemoryRouter>
+                <SettingsRoute />
+              </MemoryRouter>
+            </AppContext.Provider>,
+          );
+        });
+
+        expect(
+          screen.getByTitle('Login with Personal Access Token').hidden,
+        ).toBe(false);
+
+        fireEvent.click(screen.getByTitle('Login with Personal Access Token'));
+        expect(mockNavigate).toHaveBeenNthCalledWith(
+          1,
+          '/login-personal-access-token',
+          {
+            replace: true,
+          },
         );
       });
 
-      fireEvent.click(screen.getByTitle('Login with OAuth App'));
-      expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app', {
-        replace: true,
+      it('should hide login with personal access token button if already logged in', async () => {
+        await act(async () => {
+          render(
+            <AppContext.Provider
+              value={{
+                settings: mockSettings,
+                accounts: { ...mockAccounts, token: '1234' },
+              }}
+            >
+              <MemoryRouter>
+                <SettingsRoute />
+              </MemoryRouter>
+            </AppContext.Provider>,
+          );
+        });
+
+        expect(
+          screen.getByTitle('Login with Personal Access Token').hidden,
+        ).toBe(true);
+      });
+    });
+
+    describe('Login with OAuth App', () => {
+      it('should show login with oauth app if not logged in', async () => {
+        await act(async () => {
+          render(
+            <AppContext.Provider
+              value={{
+                settings: mockSettings,
+                accounts: {
+                  ...mockAccounts,
+                  enterpriseAccounts: [],
+                },
+              }}
+            >
+              <MemoryRouter>
+                <SettingsRoute />
+              </MemoryRouter>
+            </AppContext.Provider>,
+          );
+        });
+
+        expect(screen.getByTitle('Login with OAuth App').hidden).toBe(false);
+
+        fireEvent.click(screen.getByTitle('Login with OAuth App'));
+        expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app', {
+          replace: true,
+        });
+      });
+
+      it('should hide login with oauth app route if already logged in', async () => {
+        await act(async () => {
+          render(
+            <AppContext.Provider
+              value={{
+                settings: mockSettings,
+                accounts: mockAccounts,
+              }}
+            >
+              <MemoryRouter>
+                <SettingsRoute />
+              </MemoryRouter>
+            </AppContext.Provider>,
+          );
+        });
+
+        expect(screen.getByTitle('Login with OAuth App').hidden).toBe(true);
       });
     });
 
@@ -490,7 +569,7 @@ describe('routes/Settings.tsx', () => {
         );
       });
 
-      fireEvent.click(screen.getByTitle('Logout from octocat'));
+      fireEvent.click(screen.getByTitle('Logout'));
 
       expect(logoutMock).toHaveBeenCalledTimes(1);
 
diff --git a/src/routes/__snapshots__/Settings.test.tsx.snap b/src/routes/__snapshots__/Settings.test.tsx.snap
index 212b0003..c0eb68f2 100644
--- a/src/routes/__snapshots__/Settings.test.tsx.snap
+++ b/src/routes/__snapshots__/Settings.test.tsx.snap
@@ -513,12 +513,49 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
     <div>
       <button
         class="hover:text-gray-500 py-1 px-2 my-1 mx-2 focus:outline-none"
+        hidden=""
+        title="Login with Personal Access Token"
+        type="button"
+      >
+        <svg
+          aria-label="Login with Personal Access Token"
+          class="octicon octicon-key"
+          fill="currentColor"
+          focusable="false"
+          height="18"
+          role="img"
+          style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+          viewBox="0 0 16 16"
+          width="18"
+        >
+          <path
+            d="M10.5 0a5.499 5.499 0 1 1-1.288 10.848l-.932.932a.749.749 0 0 1-.53.22H7v.75a.749.749 0 0 1-.22.53l-.5.5a.749.749 0 0 1-.53.22H5v.75a.749.749 0 0 1-.22.53l-.5.5a.749.749 0 0 1-.53.22h-2A1.75 1.75 0 0 1 0 14.25v-2c0-.199.079-.389.22-.53l4.932-4.932A5.5 5.5 0 0 1 10.5 0Zm-4 5.5c-.001.431.069.86.205 1.269a.75.75 0 0 1-.181.768L1.5 12.56v1.69c0 .138.112.25.25.25h1.69l.06-.06v-1.19a.75.75 0 0 1 .75-.75h1.19l.06-.06v-1.19a.75.75 0 0 1 .75-.75h1.19l1.023-1.025a.75.75 0 0 1 .768-.18A4 4 0 1 0 6.5 5.5ZM11 6a1 1 0 1 1 0-2 1 1 0 0 1 0 2Z"
+          />
+        </svg>
+        <svg
+          aria-hidden="true"
+          class="ml-1 mb-2"
+          fill="currentColor"
+          focusable="false"
+          height="10"
+          style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+          viewBox="0 0 16 16"
+          width="10"
+        >
+          <path
+            d="M7.75 2a.75.75 0 0 1 .75.75V7h4.25a.75.75 0 0 1 0 1.5H8.5v4.25a.75.75 0 0 1-1.5 0V8.5H2.75a.75.75 0 0 1 0-1.5H7V2.75A.75.75 0 0 1 7.75 2Z"
+          />
+        </svg>
+      </button>
+      <button
+        class="hover:text-gray-500 py-1 px-2 my-1 mx-2 focus:outline-none"
+        hidden=""
         title="Login with OAuth App"
         type="button"
       >
         <svg
           aria-label="Login with OAuth App"
-          class="octicon octicon-person-add"
+          class="octicon octicon-person"
           fill="currentColor"
           focusable="false"
           height="20"
@@ -528,18 +565,32 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
           width="20"
         >
           <path
-            d="M7.9 8.548h-.001a5.528 5.528 0 0 1 3.1 4.659.75.75 0 1 1-1.498.086A4.01 4.01 0 0 0 5.5 9.5a4.01 4.01 0 0 0-4.001 3.793.75.75 0 1 1-1.498-.085 5.527 5.527 0 0 1 3.1-4.66 3.5 3.5 0 1 1 4.799 0ZM13.25 0a.75.75 0 0 1 .75.75V2h1.25a.75.75 0 0 1 0 1.5H14v1.25a.75.75 0 0 1-1.5 0V3.5h-1.25a.75.75 0 0 1 0-1.5h1.25V.75a.75.75 0 0 1 .75-.75ZM5.5 4a2 2 0 1 0-.001 3.999A2 2 0 0 0 5.5 4Z"
+            d="M10.561 8.073a6.005 6.005 0 0 1 3.432 5.142.75.75 0 1 1-1.498.07 4.5 4.5 0 0 0-8.99 0 .75.75 0 0 1-1.498-.07 6.004 6.004 0 0 1 3.431-5.142 3.999 3.999 0 1 1 5.123 0ZM10.5 5a2.5 2.5 0 1 0-5 0 2.5 2.5 0 0 0 5 0Z"
+          />
+        </svg>
+        <svg
+          aria-hidden="true"
+          class="mb-2"
+          fill="currentColor"
+          focusable="false"
+          height="10"
+          style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+          viewBox="0 0 16 16"
+          width="10"
+        >
+          <path
+            d="M7.75 2a.75.75 0 0 1 .75.75V7h4.25a.75.75 0 0 1 0 1.5H8.5v4.25a.75.75 0 0 1-1.5 0V8.5H2.75a.75.75 0 0 1 0-1.5H7V2.75A.75.75 0 0 1 7.75 2Z"
           />
         </svg>
       </button>
       <button
         class="hover:text-gray-500 py-1 px-2 my-1 mx-2 focus:outline-none"
         role="button"
-        title="Logout from octocat"
+        title="Logout"
         type="button"
       >
         <svg
-          aria-label="Logout from octocat"
+          aria-label="Logout"
           class="octicon octicon-sign-out"
           fill="currentColor"
           focusable="false"
diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts
index 1adbdfcb..d22a6411 100644
--- a/src/utils/helpers.test.ts
+++ b/src/utils/helpers.test.ts
@@ -12,19 +12,46 @@ import {
   generateGitHubWebUrl,
   generateNotificationReferrerId,
   isEnterpriseHost,
-  isGitHubLoggedIn,
+  isOAuthAppLoggedIn,
+  isPersonalAccessTokenLoggedIn,
 } from './helpers';
 
 describe('utils/helpers.ts', () => {
-  describe('isGitHubLoggedIn', () => {
+  describe('isPersonalAccessTokenLoggedIn', () => {
     it('logged in', () => {
-      expect(isGitHubLoggedIn({ ...mockAccounts, token: '1234' })).toBe(true);
+      expect(
+        isPersonalAccessTokenLoggedIn({ ...mockAccounts, token: '1234' }),
+      ).toBe(true);
     });
 
     it('logged out', () => {
-      expect(isGitHubLoggedIn({ ...mockAccounts, token: null })).toBe(false);
+      expect(
+        isPersonalAccessTokenLoggedIn({ ...mockAccounts, token: null }),
+      ).toBe(false);
     });
   });
+
+  describe('isOAuthAppLoggedIn', () => {
+    it('logged in', () => {
+      expect(
+        isOAuthAppLoggedIn({
+          ...mockAccounts,
+          enterpriseAccounts: [{ hostname: 'github.gitify.io', token: '1234' }],
+        }),
+      ).toBe(true);
+    });
+
+    it('logged out', () => {
+      expect(
+        isOAuthAppLoggedIn({ ...mockAccounts, enterpriseAccounts: null }),
+      ).toBe(false);
+
+      expect(
+        isOAuthAppLoggedIn({ ...mockAccounts, enterpriseAccounts: [] }),
+      ).toBe(false);
+    });
+  });
+
   describe('isEnterpriseHost', () => {
     it('should return true for enterprise host', () => {
       expect(isEnterpriseHost('github.gitify.app')).toBe(true);