From e25a532029af3dac668509f5044523934b861b48 Mon Sep 17 00:00:00 2001 From: Adam Setch <adam.setch@outlook.com> Date: Mon, 5 Aug 2024 05:10:54 -0400 Subject: [PATCH] feat: use platform version for github enterprise server feature availability (mark as done) (#1424) * feat: mark as done for old github enterprise server instances * feat: mark as done for old github enterprise server instances * feat: mark as done for old github enterprise server instances * feat: mark as done for old github enterprise server instances * feat: fetch enterprise server platform version on load and use for feature availability * feat: fetch enterprise server platform version on load and use for feature availability * Merge branch 'main' into feat/mark-as-done-server --- src/components/NotificationRow.tsx | 27 +++++----- src/components/RepositoryNotifications.tsx | 27 +++++----- .../settings/NotificationSettings.tsx | 6 +++ src/context/App.tsx | 7 +++ src/hooks/useNotifications.ts | 13 +++-- .../__snapshots__/Settings.test.tsx.snap | 20 ++++++++ src/types.ts | 1 + .../api/__snapshots__/client.test.ts.snap | 12 ++--- src/utils/api/client.test.ts | 6 +-- src/utils/api/client.ts | 3 +- src/utils/helpers.test.ts | 49 ++++++++++++++++++- src/utils/helpers.ts | 16 +++++- 12 files changed, 147 insertions(+), 40 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index c69f29d79..1f0752f22 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -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, @@ -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} diff --git a/src/components/RepositoryNotifications.tsx b/src/components/RepositoryNotifications.tsx index 27b50ad49..c4aaab81c 100644 --- a/src/components/RepositoryNotifications.tsx +++ b/src/components/RepositoryNotifications.tsx @@ -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'; @@ -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} diff --git a/src/components/settings/NotificationSettings.tsx b/src/components/settings/NotificationSettings.tsx index f9fac93e5..1a7096285 100644 --- a/src/components/settings/NotificationSettings.tsx +++ b/src/components/settings/NotificationSettings.tsx @@ -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" diff --git a/src/context/App.tsx b/src/context/App.tsx index 90cf29a1d..67857be5e 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -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( diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 76415d384..a5f385ab2 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -13,6 +13,7 @@ import { markRepositoryNotificationsAsRead, } from '../utils/api/client'; import { getAccountUUID } from '../utils/auth/utils'; +import { isMarkAsDoneFeatureSupported } from '../utils/helpers'; import { getAllNotifications, setTrayIconColor, @@ -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, diff --git a/src/routes/__snapshots__/Settings.test.tsx.snap b/src/routes/__snapshots__/Settings.test.tsx.snap index e5b4f9869..54e98764e 100644 --- a/src/routes/__snapshots__/Settings.test.tsx.snap +++ b/src/routes/__snapshots__/Settings.test.tsx.snap @@ -486,6 +486,26 @@ exports[`routes/Settings.tsx should render itself & its children 1`] = ` for="markAsDoneOnOpen" > Mark as done on open + <span + aria-label="tooltip-markAsDoneOnOpen" + class="relative" + id="tooltip-markAsDoneOnOpen" + > + <svg + aria-hidden="true" + class="ml-1 text-blue-500" + fill="currentColor" + focusable="false" + height="16" + style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;" + viewBox="0 0 16 16" + width="16" + > + <path + d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8Zm8-6.5a6.5 6.5 0 1 0 0 13 6.5 6.5 0 0 0 0-13ZM6.92 6.085h.001a.749.749 0 1 1-1.342-.67c.169-.339.436-.701.849-.977C6.845 4.16 7.369 4 8 4a2.756 2.756 0 0 1 1.637.525c.503.377.863.965.863 1.725 0 .448-.115.83-.329 1.15-.205.307-.47.513-.692.662-.109.072-.22.138-.313.195l-.006.004a6.24 6.24 0 0 0-.26.16.952.952 0 0 0-.276.245.75.75 0 0 1-1.248-.832c.184-.264.42-.489.692-.661.103-.067.207-.132.313-.195l.007-.004c.1-.061.182-.11.258-.161a.969.969 0 0 0 .277-.245C8.96 6.514 9 6.427 9 6.25a.612.612 0 0 0-.262-.525A1.27 1.27 0 0 0 8 5.5c-.369 0-.595.09-.74.187a1.01 1.01 0 0 0-.34.398ZM9 11a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z" + /> + </svg> + </span> </label> </div> </div> diff --git a/src/types.ts b/src/types.ts index e7cac771f..09ac84a8b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -46,6 +46,7 @@ export type Status = 'loading' | 'success' | 'error'; export interface Account { method: AuthMethod; platform: PlatformType; + version?: string; hostname: Hostname; token: Token; user: GitifyUser | null; diff --git a/src/utils/api/__snapshots__/client.test.ts.snap b/src/utils/api/__snapshots__/client.test.ts.snap index 7f0db3198..dc98f15e5 100644 --- a/src/utils/api/__snapshots__/client.test.ts.snap +++ b/src/utils/api/__snapshots__/client.test.ts.snap @@ -36,7 +36,7 @@ exports[`utils/api/client.ts headNotifications should fetch notifications head - } `; -exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription - github 1`] = ` +exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription - enterprise 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", @@ -45,7 +45,7 @@ exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore } `; -exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription- enterprise 1`] = ` +exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription - github 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", @@ -72,7 +72,7 @@ exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list n } `; -exports[`utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done - github 1`] = ` +exports[`utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done - enterprise 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", @@ -81,7 +81,7 @@ exports[`utils/api/client.ts markNotificationThreadAsDone should mark notificati } `; -exports[`utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done- enterprise 1`] = ` +exports[`utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done - github 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", @@ -90,7 +90,7 @@ exports[`utils/api/client.ts markNotificationThreadAsDone should mark notificati } `; -exports[`utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read - github 1`] = ` +exports[`utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read - enterprise 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", @@ -99,7 +99,7 @@ exports[`utils/api/client.ts markNotificationThreadAsRead should mark notificati } `; -exports[`utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read- enterprise 1`] = ` +exports[`utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read - github 1`] = ` { "Accept": "application/json", "Authorization": "token token-123-456", diff --git a/src/utils/api/client.test.ts b/src/utils/api/client.test.ts index 614758c16..edf0b0325 100644 --- a/src/utils/api/client.test.ts +++ b/src/utils/api/client.test.ts @@ -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, @@ -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, @@ -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, diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index c558b24bb..d3b358376 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -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( @@ -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, {}); } diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index 805c740b4..0cdbc7b72 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -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'; @@ -17,6 +21,7 @@ import { getFilterCount, getPlatformFromHostname, isEnterpriseServerHost, + isMarkAsDoneFeatureSupported, } from './helpers'; describe('utils/helpers.ts', () => { @@ -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); diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index a0724bfe3..10094b417 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -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'; @@ -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 {