Skip to content

Commit

Permalink
refactor(auth): remove repo scopes check (#1127)
Browse files Browse the repository at this point in the history
  • Loading branch information
setchy committed May 21, 2024
1 parent 4f9fb2e commit 638c0b8
Show file tree
Hide file tree
Showing 7 changed files with 3 additions and 217 deletions.
54 changes: 1 addition & 53 deletions src/routes/Settings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,7 @@ describe('routes/Settings.tsx', () => {
expect(updateSetting).toHaveBeenCalledWith('theme', 'LIGHT');
});

it('should be able to enable detailed notifications', async () => {
jest.spyOn(apiRequests, 'apiRequestAuth').mockResolvedValue({
headers: {
'x-oauth-scopes': Constants.AUTH_SCOPE.join(', '),
},
} as unknown as AxiosResponse);

it('should toggle detailed notifications checkbox', async () => {
await act(async () => {
render(
<AppContext.Provider
Expand All @@ -121,52 +115,6 @@ describe('routes/Settings.tsx', () => {
expect(updateSetting).toHaveBeenCalledTimes(1);
expect(updateSetting).toHaveBeenCalledWith('detailedNotifications', true);
});

it('should not be able to enable detailed notifications due to missing scope', async () => {
jest.spyOn(apiRequests, 'apiRequestAuth').mockResolvedValue({
headers: {
'x-oauth-scopes': 'read:user, notifications',
},
} as unknown as AxiosResponse);

await act(async () => {
render(
<AppContext.Provider
value={{
settings: mockSettings,
accounts: mockAccounts,
updateSetting,
}}
>
<MemoryRouter>
<SettingsRoute />
</MemoryRouter>
</AppContext.Provider>,
);
});

expect(
screen
.getByLabelText('Detailed notifications (requires repo scope)')
.closest('input'),
).toHaveProperty('disabled', true);

// click the checkbox
fireEvent.click(
screen.getByLabelText('Detailed notifications (requires repo scope)'),
);

// check if the checkbox is still unchecked
expect(updateSetting).not.toHaveBeenCalled();
expect(
screen.getByLabelText('Detailed notifications (requires repo scope)'),
).not.toBe('checked');

expect(
screen.getByLabelText('Detailed notifications (requires repo scope)')
.parentNode.parentNode,
).toMatchSnapshot();
});
});

describe('Notifications section', () => {
Expand Down
23 changes: 2 additions & 21 deletions src/routes/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
useCallback,
useContext,
useEffect,
useMemo,
useState,
} from 'react';
import { useNavigate } from 'react-router-dom';
Expand All @@ -21,7 +20,6 @@ import { Checkbox } from '../components/fields/Checkbox';
import { RadioGroup } from '../components/fields/RadioGroup';
import { AppContext } from '../context/App';
import { Theme } from '../types';
import { getRootHypermediaLinks } from '../utils/api/client';
import {
openExternalLink,
updateTrayIcon,
Expand All @@ -37,7 +35,6 @@ export const SettingsRoute: FC = () => {
const [isLinux, setIsLinux] = useState<boolean>(false);
const [isMacOS, setIsMacOS] = useState<boolean>(false);
const [appVersion, setAppVersion] = useState<string | null>(null);
const [repoScope, setRepoScope] = useState<boolean>(false);

const openGitHubReleaseNotes = useCallback((version) => {
openExternalLink(
Expand Down Expand Up @@ -73,18 +70,6 @@ export const SettingsRoute: FC = () => {
});
}, []);

useMemo(() => {
(async () => {
const response = await getRootHypermediaLinks(
Constants.DEFAULT_AUTH_OPTIONS.hostname,
accounts.token,
);

if (response.headers['x-oauth-scopes'].includes('repo'))
setRepoScope(true);
})();
}, [accounts.token]);

const logoutUser = useCallback(() => {
logout();
navigate(-1);
Expand Down Expand Up @@ -145,15 +130,11 @@ export const SettingsRoute: FC = () => {
/>
<Checkbox
name="detailedNotifications"
label={`Detailed notifications${
!repoScope ? ' (requires repo scope)' : ''
}`}
checked={repoScope && settings.detailedNotifications}
label="Detailed notifications"
checked={settings.detailedNotifications}
onChange={(evt) =>
repoScope &&
updateSetting('detailedNotifications', evt.target.checked)
}
disabled={!repoScope}
tooltip={
<div>
<div className="pb-3">
Expand Down
49 changes: 0 additions & 49 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: 0 additions & 35 deletions src/typesGitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,38 +498,3 @@ export interface NotificationThreadSubscription {
url: string;
thread_url: string;
}

export interface RootHypermediaLinks {
current_user_url: string;
current_user_authorizations_html_url: string;
authorizations_url: string;
code_search_url: string;
commit_search_url: string;
emails_url: string;
emojis_url: string;
events_url: string;
feeds_url: string;
followers_url: string;
following_url: string;
gists_url: string;
hub_url: string;
issue_search_url: string;
issues_url: string;
keys_url: string;
notifications_url: string;
organization_url: string;
organization_repositories_url: string;
organization_teams_url: string;
public_gists_url: string;
rate_limit_url: string;
repository_url: string;
repository_search_url: string;
current_user_repositories_url: string;
starred_url: string;
starred_gists_url: string;
topic_search_url: string;
user_url: string;
user_organizations_url: string;
user_repositories_url: string;
user_search_url: string;
}
18 changes: 0 additions & 18 deletions src/utils/api/__snapshots__/client.test.ts.snap

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

27 changes: 0 additions & 27 deletions src/utils/api/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { SettingsState } from '../../types';
import {
getAuthenticatedUser,
getHtmlUrl,
getRootHypermediaLinks,
headNotifications,
ignoreNotificationThreadSubscription,
listNotificationsForAuthenticatedUser,
Expand All @@ -26,32 +25,6 @@ describe('utils/api/client.ts', () => {
jest.resetAllMocks();
});

describe('getRootHypermediaLinks', () => {
it('should fetch root hypermedia links - github', async () => {
await getRootHypermediaLinks(mockGitHubHostname, mockToken);

expect(axios).toHaveBeenCalledWith({
url: 'https://api.github.com/',
method: 'GET',
data: {},
});

expect(axios.defaults.headers.common).toMatchSnapshot();
});

it('should fetch root hypermedia links - enterprise', async () => {
await getRootHypermediaLinks(mockEnterpriseHostname, mockToken);

expect(axios).toHaveBeenCalledWith({
url: 'https://example.com/api/v3/',
method: 'GET',
data: {},
});

expect(axios.defaults.headers.common).toMatchSnapshot();
});
});

describe('getAuthenticatedUser', () => {
it('should fetch authenticated user - github', async () => {
await getAuthenticatedUser(mockGitHubHostname, mockToken);
Expand Down
14 changes: 0 additions & 14 deletions src/utils/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type {
PullRequest,
PullRequestReview,
Release,
RootHypermediaLinks,
UserDetails,
} from '../../typesGitHub';
import { apiRequestAuth } from './request';
Expand All @@ -22,19 +21,6 @@ import { QUERY_SEARCH_DISCUSSIONS } from './graphql/discussions';
import { formatAsGitHubSearchSyntax } from './graphql/utils';
import { getGitHubAPIBaseUrl, getGitHubGraphQLUrl } from './utils';

/**
* Get Hypermedia links to resources accessible in GitHub's REST API
*
* Endpoint documentation: https://docs.github.com/en/rest/meta/meta#github-api-root
*/
export function getRootHypermediaLinks(
hostname: string,
token: string,
): AxiosPromise<RootHypermediaLinks> {
const url = getGitHubAPIBaseUrl(hostname);
return apiRequestAuth(url.toString(), 'GET', token);
}

/**
* Get the authenticated user
*
Expand Down

0 comments on commit 638c0b8

Please sign in to comment.