Skip to content

Commit

Permalink
feat: improve enterprise login (#1112)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people committed May 22, 2024
1 parent 7d0fcd0 commit 447a5dc
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 35 deletions.
113 changes: 96 additions & 17 deletions src/routes/Settings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

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

fireEvent.click(screen.getByTitle('Logout from octocat'));
fireEvent.click(screen.getByTitle('Logout'));

expect(logoutMock).toHaveBeenCalledTimes(1);

Expand Down
34 changes: 27 additions & 7 deletions src/routes/Settings.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
ArrowLeftIcon,
PersonAddIcon,
KeyIcon,
PersonIcon,
PlusIcon,
SignOutIcon,
XCircleIcon,
} from '@primer/octicons-react';
Expand All @@ -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 = () => {
Expand Down Expand Up @@ -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 });
}, []);
Expand Down Expand Up @@ -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
Expand Down
59 changes: 55 additions & 4 deletions src/routes/__snapshots__/Settings.test.tsx.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 31 additions & 4 deletions src/utils/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/utils/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ipcRenderer } from 'electron';
import { Notification } from '../typesGitHub';
import {
getTokenForHost,
isGitHubLoggedIn,
isPersonalAccessTokenLoggedIn,
openInBrowser,
} from '../utils/helpers';
import { updateTrayIcon } from './comms';
Expand Down Expand Up @@ -110,7 +110,7 @@ export const raiseSoundNotification = () => {
};

function getGitHubNotifications(accounts: AuthState, settings: SettingsState) {
if (!isGitHubLoggedIn(accounts)) {
if (!isPersonalAccessTokenLoggedIn(accounts)) {
return;
}

Expand Down

0 comments on commit 447a5dc

Please sign in to comment.