Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use platform version for github enterprise server feature availability (mark as done) #1424

Merged
merged 10 commits into from
Aug 5, 2024
Merged
27 changes: 16 additions & 11 deletions src/components/NotificationRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import { AppContext } from '../context/App';
import { Opacity, Size } from '../types';
import type { Notification } from '../typesGitHub';
import { cn } from '../utils/cn';
import { formatForDisplay } from '../utils/helpers';
import {
formatForDisplay,
isMarkAsDoneFeatureSupported,
} from '../utils/helpers';
import {
getNotificationTypeIcon,
getNotificationTypeIconColor,
Expand Down Expand Up @@ -126,16 +129,18 @@ export const NotificationRow: FC<INotificationRow> = ({

{!animateExit && (
<HoverGroup>
<InteractionButton
title="Mark as Done"
icon={CheckIcon}
size={Size.MEDIUM}
onClick={() => {
setAnimateExit(!settings.delayNotificationState);
setShowAsRead(settings.delayNotificationState);
markNotificationDone(notification);
}}
/>
{isMarkAsDoneFeatureSupported(notification.account) && (
<InteractionButton
title="Mark as Done"
icon={CheckIcon}
size={Size.MEDIUM}
onClick={() => {
setAnimateExit(!settings.delayNotificationState);
setShowAsRead(settings.delayNotificationState);
markNotificationDone(notification);
}}
/>
)}
<InteractionButton
title="Mark as Read"
icon={ReadIcon}
Expand Down
27 changes: 15 additions & 12 deletions src/components/RepositoryNotifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { AppContext } from '../context/App';
import { Opacity, Size } from '../types';
import type { Notification } from '../typesGitHub';
import { cn } from '../utils/cn';
import { isMarkAsDoneFeatureSupported } from '../utils/helpers';
import { openRepository } from '../utils/links';
import { HoverGroup } from './HoverGroup';
import { NotificationRow } from './NotificationRow';
Expand Down Expand Up @@ -80,18 +81,20 @@ export const RepositoryNotifications: FC<IRepositoryNotifications> = ({

{!animateExit && (
<HoverGroup>
<InteractionButton
title="Mark Repository as Done"
icon={CheckIcon}
size={Size.MEDIUM}
onClick={(event: MouseEvent<HTMLElement>) => {
// Don't trigger onClick of parent element.
event.stopPropagation();
setAnimateExit(!settings.delayNotificationState);
setShowAsRead(settings.delayNotificationState);
markRepoNotificationsDone(repoNotifications[0]);
}}
/>
{isMarkAsDoneFeatureSupported(repoNotifications[0].account) && (
<InteractionButton
title="Mark Repository as Done"
icon={CheckIcon}
size={Size.MEDIUM}
onClick={(event: MouseEvent<HTMLElement>) => {
// Don't trigger onClick of parent element.
event.stopPropagation();
setAnimateExit(!settings.delayNotificationState);
setShowAsRead(settings.delayNotificationState);
markRepoNotificationsDone(repoNotifications[0]);
}}
/>
)}
<InteractionButton
title="Mark Repository as Read"
icon={ReadIcon}
Expand Down
6 changes: 6 additions & 0 deletions src/components/settings/NotificationSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ export const NotificationSettings: FC = () => {
onChange={(evt) =>
updateSetting('markAsDoneOnOpen', evt.target.checked)
}
tooltip={
<div>
<strong>Mark as Done</strong> feature is supported in GitHub Cloud
and GitHub Enterprise Server 3.13 or later.
</div>
}
/>
<Checkbox
name="delayNotificationState"
Expand Down
7 changes: 7 additions & 0 deletions src/context/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,13 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
);
return existing.settings;
}

for (const account of auth.accounts.filter(
(a) => a.platform === 'GitHub Enterprise Server',
)) {
const res = await headNotifications(account.hostname, account.token);
account.version = res.headers['x-github-enterprise-version'];
}
}, []);

const fetchNotificationsWithAccounts = useCallback(
Expand Down
13 changes: 8 additions & 5 deletions src/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
markRepositoryNotificationsAsRead,
} from '../utils/api/client';
import { getAccountUUID } from '../utils/auth/utils';
import { isMarkAsDoneFeatureSupported } from '../utils/helpers';
import {
getAllNotifications,
setTrayIconColor,
Expand Down Expand Up @@ -120,11 +121,13 @@ export const useNotifications = (): NotificationsState => {
setStatus('loading');

try {
await markNotificationThreadAsDone(
notification.id,
notification.account.hostname,
notification.account.token,
);
if (isMarkAsDoneFeatureSupported(notification.account)) {
await markNotificationThreadAsDone(
notification.id,
notification.account.hostname,
notification.account.token,
);
}

const updatedNotifications = removeNotification(
state.settings,
Expand Down
20 changes: 20 additions & 0 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.

1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export type Status = 'loading' | 'success' | 'error';
export interface Account {
method: AuthMethod;
platform: PlatformType;
version?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should comment this explaining what it is?

hostname: Hostname;
token: Token;
user: GitifyUser | null;
Expand Down
12 changes: 6 additions & 6 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.

6 changes: 3 additions & 3 deletions src/utils/api/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('utils/api/client.ts', () => {
expect(axios.defaults.headers.common).toMatchSnapshot();
});

it('should mark notification thread as read- enterprise', async () => {
it('should mark notification thread as read - enterprise', async () => {
await markNotificationThreadAsRead(
mockThreadId,
mockEnterpriseHostname,
Expand Down Expand Up @@ -169,7 +169,7 @@ describe('utils/api/client.ts', () => {
expect(axios.defaults.headers.common).toMatchSnapshot();
});

it('should mark notification thread as done- enterprise', async () => {
it('should mark notification thread as done - enterprise', async () => {
await markNotificationThreadAsDone(
mockThreadId,
mockEnterpriseHostname,
Expand Down Expand Up @@ -203,7 +203,7 @@ describe('utils/api/client.ts', () => {
expect(axios.defaults.headers.common).toMatchSnapshot();
});

it('should ignore notification thread subscription- enterprise', async () => {
it('should ignore notification thread subscription - enterprise', async () => {
await ignoreNotificationThreadSubscription(
mockThreadId,
mockEnterpriseHostname,
Expand Down
3 changes: 2 additions & 1 deletion src/utils/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ export function markNotificationThreadAsRead(
* Marks a thread as "done." Marking a thread as "done" is equivalent to marking a
* notification in your notification inbox on GitHub as done.
*
* NOTE: This was added to GitHub Enterprise Server in version 3.13 or later.
*
* Endpoint documentation: https://docs.github.com/en/rest/activity/notifications#mark-a-thread-as-done
*/
export function markNotificationThreadAsDone(
Expand All @@ -99,7 +101,6 @@ export function markNotificationThreadAsDone(
): AxiosPromise<void> {
const url = getGitHubAPIBaseUrl(hostname);
url.pathname += `notifications/threads/${threadId}`;

return apiRequestAuth(url.toString() as Link, 'DELETE', token, {});
}

Expand Down
49 changes: 48 additions & 1 deletion src/utils/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { AxiosPromise, AxiosResponse } from 'axios';
import { mockPersonalAccessTokenAccount } from '../__mocks__/state-mocks';
import {
mockGitHubCloudAccount,
mockGitHubEnterpriseServerAccount,
mockPersonalAccessTokenAccount,
} from '../__mocks__/state-mocks';

import { defaultSettings } from '../context/App';
import type { Hostname, Link, SettingsState } from '../types';
Expand All @@ -17,6 +21,7 @@ import {
getFilterCount,
getPlatformFromHostname,
isEnterpriseServerHost,
isMarkAsDoneFeatureSupported,
} from './helpers';

describe('utils/helpers.ts', () => {
Expand Down Expand Up @@ -56,6 +61,48 @@ describe('utils/helpers.ts', () => {
});
});

describe('isMarkAsDoneFeatureSupported', () => {
it('should return true for GitHub Cloud', () => {
expect(isMarkAsDoneFeatureSupported(mockGitHubCloudAccount)).toBe(true);
});

it('should return false for GitHub Enterprise Server < v3.13', () => {
const account = {
...mockGitHubEnterpriseServerAccount,
version: '3.12.0',
};

expect(isMarkAsDoneFeatureSupported(account)).toBe(false);
});

it('should return true for GitHub Enterprise Server >= v3.13', () => {
const account = {
...mockGitHubEnterpriseServerAccount,
version: '3.13.3',
};

expect(isMarkAsDoneFeatureSupported(account)).toBe(true);
});

it('should return false for GitHub Enterprise Server when partial version available', () => {
const account = {
...mockGitHubEnterpriseServerAccount,
version: '3',
};

expect(isMarkAsDoneFeatureSupported(account)).toBe(false);
});

it('should return false for GitHub Enterprise Server when no version available', () => {
const account = {
...mockGitHubEnterpriseServerAccount,
version: null,
};

expect(isMarkAsDoneFeatureSupported(account)).toBe(false);
});
});

describe('generateNotificationReferrerId', () => {
it('should generate the notification_referrer_id', () => {
const referrerId = generateNotificationReferrerId(mockSingleNotification);
Expand Down
16 changes: 15 additions & 1 deletion src/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { formatDistanceToNowStrict, parseISO } from 'date-fns';
import { defaultSettings } from '../context/App';
import type { Hostname, Link, SettingsState } from '../types';
import type { Account, Hostname, Link, SettingsState } from '../types';
import type { Notification } from '../typesGitHub';
import { getHtmlUrl, getLatestDiscussion } from './api/client';
import type { PlatformType } from './auth/types';
Expand All @@ -21,6 +21,20 @@ export function isEnterpriseServerHost(hostname: Hostname): boolean {
return !hostname.endsWith(Constants.DEFAULT_AUTH_OPTIONS.hostname);
}

export function isMarkAsDoneFeatureSupported(account: Account): boolean {
if (isEnterpriseServerHost(account.hostname)) {
// Support for "Mark as Done" was added to GitHub Enterprise Server in v3.13 or newer
if (account.version) {
const version = account?.version.split('.').map(Number);
return version[0] >= 3 && version[1] >= 13;
}

return false;
}

return true;
}

export function generateNotificationReferrerId(
notification: Notification,
): string {
Expand Down