Skip to content

Commit 447a5dc

Browse files
mashiroafonsojramossetchy
authored
feat: improve enterprise login (#1112)
* Improve enterprise login * Discard changes to main.js --------- Co-authored-by: Afonso Jorge Ramos <afonsojorgeramos@gmail.com> Co-authored-by: Adam Setch <adam.setch@outlook.com>
1 parent 7d0fcd0 commit 447a5dc

File tree

6 files changed

+216
-35
lines changed

6 files changed

+216
-35
lines changed

src/routes/Settings.test.tsx

Lines changed: 96 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -450,25 +450,104 @@ describe('routes/Settings.tsx', () => {
450450
);
451451
});
452452

453-
it('should go to the login with oauth app route', async () => {
454-
await act(async () => {
455-
render(
456-
<AppContext.Provider
457-
value={{
458-
settings: mockSettings,
459-
accounts: mockAccounts,
460-
}}
461-
>
462-
<MemoryRouter>
463-
<SettingsRoute />
464-
</MemoryRouter>
465-
</AppContext.Provider>,
453+
describe('Login with Personal Access Token', () => {
454+
it('should show login with personal access token button if not logged in', async () => {
455+
await act(async () => {
456+
render(
457+
<AppContext.Provider
458+
value={{
459+
settings: mockSettings,
460+
accounts: { ...mockAccounts, token: null },
461+
}}
462+
>
463+
<MemoryRouter>
464+
<SettingsRoute />
465+
</MemoryRouter>
466+
</AppContext.Provider>,
467+
);
468+
});
469+
470+
expect(
471+
screen.getByTitle('Login with Personal Access Token').hidden,
472+
).toBe(false);
473+
474+
fireEvent.click(screen.getByTitle('Login with Personal Access Token'));
475+
expect(mockNavigate).toHaveBeenNthCalledWith(
476+
1,
477+
'/login-personal-access-token',
478+
{
479+
replace: true,
480+
},
466481
);
467482
});
468483

469-
fireEvent.click(screen.getByTitle('Login with OAuth App'));
470-
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app', {
471-
replace: true,
484+
it('should hide login with personal access token button if already logged in', async () => {
485+
await act(async () => {
486+
render(
487+
<AppContext.Provider
488+
value={{
489+
settings: mockSettings,
490+
accounts: { ...mockAccounts, token: '1234' },
491+
}}
492+
>
493+
<MemoryRouter>
494+
<SettingsRoute />
495+
</MemoryRouter>
496+
</AppContext.Provider>,
497+
);
498+
});
499+
500+
expect(
501+
screen.getByTitle('Login with Personal Access Token').hidden,
502+
).toBe(true);
503+
});
504+
});
505+
506+
describe('Login with OAuth App', () => {
507+
it('should show login with oauth app if not logged in', async () => {
508+
await act(async () => {
509+
render(
510+
<AppContext.Provider
511+
value={{
512+
settings: mockSettings,
513+
accounts: {
514+
...mockAccounts,
515+
enterpriseAccounts: [],
516+
},
517+
}}
518+
>
519+
<MemoryRouter>
520+
<SettingsRoute />
521+
</MemoryRouter>
522+
</AppContext.Provider>,
523+
);
524+
});
525+
526+
expect(screen.getByTitle('Login with OAuth App').hidden).toBe(false);
527+
528+
fireEvent.click(screen.getByTitle('Login with OAuth App'));
529+
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app', {
530+
replace: true,
531+
});
532+
});
533+
534+
it('should hide login with oauth app route if already logged in', async () => {
535+
await act(async () => {
536+
render(
537+
<AppContext.Provider
538+
value={{
539+
settings: mockSettings,
540+
accounts: mockAccounts,
541+
}}
542+
>
543+
<MemoryRouter>
544+
<SettingsRoute />
545+
</MemoryRouter>
546+
</AppContext.Provider>,
547+
);
548+
});
549+
550+
expect(screen.getByTitle('Login with OAuth App').hidden).toBe(true);
472551
});
473552
});
474553

@@ -490,7 +569,7 @@ describe('routes/Settings.tsx', () => {
490569
);
491570
});
492571

493-
fireEvent.click(screen.getByTitle('Logout from octocat'));
572+
fireEvent.click(screen.getByTitle('Logout'));
494573

495574
expect(logoutMock).toHaveBeenCalledTimes(1);
496575

src/routes/Settings.tsx

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
22
ArrowLeftIcon,
3-
PersonAddIcon,
3+
KeyIcon,
4+
PersonIcon,
5+
PlusIcon,
46
SignOutIcon,
57
XCircleIcon,
68
} from '@primer/octicons-react';
@@ -26,6 +28,10 @@ import {
2628
updateTrayTitle,
2729
} from '../utils/comms';
2830
import Constants from '../utils/constants';
31+
import {
32+
isOAuthAppLoggedIn,
33+
isPersonalAccessTokenLoggedIn,
34+
} from '../utils/helpers';
2935
import { setTheme } from '../utils/theme';
3036

3137
export const SettingsRoute: FC = () => {
@@ -81,6 +87,10 @@ export const SettingsRoute: FC = () => {
8187
ipcRenderer.send('app-quit');
8288
}, []);
8389

90+
const loginWithPersonalAccessToken = useCallback(() => {
91+
return navigate('/login-personal-access-token', { replace: true });
92+
}, []);
93+
8494
const loginWithOAuthApp = useCallback(() => {
8595
return navigate('/login-oauth-app', { replace: true });
8696
}, []);
@@ -290,26 +300,36 @@ export const SettingsRoute: FC = () => {
290300
Gitify v{appVersion}
291301
</button>
292302
<div>
303+
<button
304+
type="button"
305+
className={footerButtonClass}
306+
title="Login with Personal Access Token"
307+
onClick={loginWithPersonalAccessToken}
308+
hidden={isPersonalAccessTokenLoggedIn(accounts)}
309+
>
310+
<KeyIcon size={18} aria-label="Login with Personal Access Token" />
311+
<PlusIcon size={10} className="ml-1 mb-2" />
312+
</button>
313+
293314
<button
294315
type="button"
295316
className={footerButtonClass}
296317
title="Login with OAuth App"
297318
onClick={loginWithOAuthApp}
319+
hidden={isOAuthAppLoggedIn(accounts)}
298320
>
299-
<PersonAddIcon size={20} aria-label="Login with OAuth App" />
321+
<PersonIcon size={20} aria-label="Login with OAuth App" />
322+
<PlusIcon size={10} className="mb-2" />
300323
</button>
301324

302325
<button
303326
type="button"
304327
className={footerButtonClass}
305-
title={`Logout from ${accounts.user.login}`}
328+
title="Logout"
306329
role="button"
307330
onClick={logoutUser}
308331
>
309-
<SignOutIcon
310-
size={18}
311-
aria-label={`Logout from ${accounts.user.login}`}
312-
/>
332+
<SignOutIcon size={18} aria-label="Logout" />
313333
</button>
314334

315335
<button

src/routes/__snapshots__/Settings.test.tsx.snap

Lines changed: 55 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/utils/helpers.test.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,46 @@ import {
1212
generateGitHubWebUrl,
1313
generateNotificationReferrerId,
1414
isEnterpriseHost,
15-
isGitHubLoggedIn,
15+
isOAuthAppLoggedIn,
16+
isPersonalAccessTokenLoggedIn,
1617
} from './helpers';
1718

1819
describe('utils/helpers.ts', () => {
19-
describe('isGitHubLoggedIn', () => {
20+
describe('isPersonalAccessTokenLoggedIn', () => {
2021
it('logged in', () => {
21-
expect(isGitHubLoggedIn({ ...mockAccounts, token: '1234' })).toBe(true);
22+
expect(
23+
isPersonalAccessTokenLoggedIn({ ...mockAccounts, token: '1234' }),
24+
).toBe(true);
2225
});
2326

2427
it('logged out', () => {
25-
expect(isGitHubLoggedIn({ ...mockAccounts, token: null })).toBe(false);
28+
expect(
29+
isPersonalAccessTokenLoggedIn({ ...mockAccounts, token: null }),
30+
).toBe(false);
2631
});
2732
});
33+
34+
describe('isOAuthAppLoggedIn', () => {
35+
it('logged in', () => {
36+
expect(
37+
isOAuthAppLoggedIn({
38+
...mockAccounts,
39+
enterpriseAccounts: [{ hostname: 'github.gitify.io', token: '1234' }],
40+
}),
41+
).toBe(true);
42+
});
43+
44+
it('logged out', () => {
45+
expect(
46+
isOAuthAppLoggedIn({ ...mockAccounts, enterpriseAccounts: null }),
47+
).toBe(false);
48+
49+
expect(
50+
isOAuthAppLoggedIn({ ...mockAccounts, enterpriseAccounts: [] }),
51+
).toBe(false);
52+
});
53+
});
54+
2855
describe('isEnterpriseHost', () => {
2956
it('should return true for enterprise host', () => {
3057
expect(isEnterpriseHost('github.gitify.app')).toBe(true);

src/utils/helpers.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,14 @@ import {
1010
getWorkflowRunAttributes,
1111
} from './subject';
1212

13-
export function isGitHubLoggedIn(accounts: AuthState): boolean {
13+
export function isPersonalAccessTokenLoggedIn(accounts: AuthState): boolean {
1414
return accounts.token != null;
1515
}
1616

17+
export function isOAuthAppLoggedIn(accounts: AuthState): boolean {
18+
return accounts.enterpriseAccounts?.length > 0;
19+
}
20+
1721
export function getTokenForHost(hostname: string, accounts: AuthState): string {
1822
const isEnterprise = isEnterpriseHost(hostname);
1923

src/utils/notifications.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ipcRenderer } from 'electron';
22
import { Notification } from '../typesGitHub';
33
import {
44
getTokenForHost,
5-
isGitHubLoggedIn,
5+
isPersonalAccessTokenLoggedIn,
66
openInBrowser,
77
} from '../utils/helpers';
88
import { updateTrayIcon } from './comms';
@@ -110,7 +110,7 @@ export const raiseSoundNotification = () => {
110110
};
111111

112112
function getGitHubNotifications(accounts: AuthState, settings: SettingsState) {
113-
if (!isGitHubLoggedIn(accounts)) {
113+
if (!isPersonalAccessTokenLoggedIn(accounts)) {
114114
return;
115115
}
116116

0 commit comments

Comments
 (0)