From 74726893040a2454aa270650b2359a27bbc7535f Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 10:23:15 -0400 Subject: [PATCH 01/16] feat: new filters route --- src/__mocks__/state-mocks.ts | 1 + src/app.tsx | 9 +++ src/components/Sidebar.test.tsx | 45 +++++++++++-- src/components/Sidebar.tsx | 24 ++++++- src/context/App.test.tsx | 2 + src/context/App.tsx | 2 + src/routes/Filters.tsx | 116 ++++++++++++++++++++++++++++++++ src/types.ts | 7 +- src/utils/helpers.ts | 16 ++++- src/utils/notifications.ts | 8 +++ src/utils/reason.ts | 2 +- 11 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 src/routes/Filters.tsx diff --git a/src/__mocks__/state-mocks.ts b/src/__mocks__/state-mocks.ts index d41641901..2d2c0d5ef 100644 --- a/src/__mocks__/state-mocks.ts +++ b/src/__mocks__/state-mocks.ts @@ -86,6 +86,7 @@ export const mockSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, + filterReasons: null, }; export const mockState: GitifyState = { diff --git a/src/app.tsx b/src/app.tsx index 49af30b12..843b5ccc2 100644 --- a/src/app.tsx +++ b/src/app.tsx @@ -10,6 +10,7 @@ import { Loading } from './components/Loading'; import { Sidebar } from './components/Sidebar'; import { AppContext, AppProvider } from './context/App'; import { AccountsRoute } from './routes/Accounts'; +import { FiltersRoute } from './routes/Filters'; import { LoginRoute } from './routes/Login'; import { LoginWithOAuthApp } from './routes/LoginWithOAuthApp'; import { LoginWithPersonalAccessToken } from './routes/LoginWithPersonalAccessToken'; @@ -43,6 +44,14 @@ export const App = () => { } /> + + + + } + /> { it('should render itself & its children (logged out)', () => { const tree = render( @@ -62,6 +66,7 @@ describe('components/Sidebar.tsx', () => { value={{ isLoggedIn: true, notifications: [], + settings: mockSettings, fetchNotifications, status: 'success', }} @@ -83,6 +88,8 @@ describe('components/Sidebar.tsx', () => { value={{ isLoggedIn: true, notifications: [], + settings: mockSettings, + fetchNotifications, status: 'loading', }} @@ -102,7 +109,13 @@ describe('components/Sidebar.tsx', () => { describe('Settings', () => { it('go to the settings route', () => { render( - + @@ -114,7 +127,13 @@ describe('components/Sidebar.tsx', () => { it('go to the home if settings path already shown', () => { render( - + @@ -133,6 +152,7 @@ describe('components/Sidebar.tsx', () => { value={{ isLoggedIn: true, notifications: mockAccountNotifications, + settings: mockSettings, }} > @@ -155,6 +175,7 @@ describe('components/Sidebar.tsx', () => { value={{ isLoggedIn: true, notifications: mockAccountNotifications, + settings: mockSettings, }} > @@ -177,6 +198,7 @@ describe('components/Sidebar.tsx', () => { value={{ isLoggedIn: true, notifications: mockAccountNotifications, + settings: mockSettings, }} > @@ -195,7 +217,9 @@ describe('components/Sidebar.tsx', () => { const quitAppMock = jest.spyOn(comms, 'quitApp'); render( - + @@ -209,7 +233,9 @@ describe('components/Sidebar.tsx', () => { const openExternalLinkMock = jest.spyOn(comms, 'openExternalLink'); render( - + @@ -225,7 +251,13 @@ describe('components/Sidebar.tsx', () => { describe('should render the notifications icon', () => { it('when there are 0 notifications', () => { render( - + @@ -244,6 +276,7 @@ describe('components/Sidebar.tsx', () => { value={{ isLoggedIn: true, notifications: mockAccountNotifications, + settings: mockSettings, }} > diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index b85c3507a..177945499 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -1,5 +1,6 @@ import { BellIcon, + FilterIcon, GearIcon, GitPullRequestIcon, IssueOpenedIcon, @@ -11,6 +12,7 @@ import { useLocation, useNavigate } from 'react-router-dom'; import { AppContext } from '../context/App'; import { Size } from '../types'; import { quitApp } from '../utils/comms'; +import { getFilterCount } from '../utils/helpers'; import { openGitHubIssues, openGitHubNotifications, @@ -25,9 +27,17 @@ export const Sidebar: FC = () => { const navigate = useNavigate(); const location = useLocation(); - const { notifications, fetchNotifications, isLoggedIn, status } = + const { notifications, fetchNotifications, isLoggedIn, status, settings } = useContext(AppContext); + const toggleFilters = () => { + if (location.pathname.startsWith('/filters')) { + navigate('/', { replace: true }); + } else { + navigate('/filters'); + } + }; + const toggleSettings = () => { if (location.pathname.startsWith('/settings')) { navigate('/', { replace: true }); @@ -45,6 +55,10 @@ export const Sidebar: FC = () => { return getNotificationCount(notifications); }, [notifications]); + const filterCount = useMemo(() => { + return getFilterCount(settings); + }, [settings]); + return (
@@ -90,6 +104,14 @@ export const Sidebar: FC = () => { onClick={() => refreshNotifications()} /> + toggleFilters()} + /> + { showPills: true, keyboardShortcut: true, groupBy: 'REPOSITORY', + filterReasons: null, } as SettingsState, }); }); @@ -434,6 +435,7 @@ describe('context/App.tsx', () => { showPills: true, keyboardShortcut: true, groupBy: 'REPOSITORY', + filterReasons: null, } as SettingsState, }); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index 9ed3d2460..8d219d783 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -64,6 +64,7 @@ export const defaultSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, + filterReasons: null, }; interface AppContextState { @@ -128,6 +129,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { }, [ settings.participating, settings.showBots, + settings.filterReasons, settings.detailedNotifications, settings.delayNotificationState, auth.accounts.length, diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx new file mode 100644 index 000000000..078e23d80 --- /dev/null +++ b/src/routes/Filters.tsx @@ -0,0 +1,116 @@ +import { FilterRemoveIcon } from '@primer/octicons-react'; +import { type FC, useContext } from 'react'; +import { Header } from '../components/Header'; +import { Checkbox } from '../components/fields/Checkbox'; +import { AppContext, defaultSettings } from '../context/App'; +import { BUTTON_CLASS_NAME } from '../styles/gitify'; +import { Size } from '../types'; +import type { Reason } from '../typesGitHub'; +import { FORMATTED_REASONS, formatReason } from '../utils/reason'; + +export const FiltersRoute: FC = () => { + const { settings, updateSetting } = useContext(AppContext); + + const updateReasonFilter = (reason: Reason, checked: boolean) => { + let reasons: Reason[]; + + if (!settings.filterReasons) { + reasons = []; + } else { + reasons = settings.filterReasons.split(',') as Reason[]; + } + + if (checked) { + reasons.push(reason); + } else { + reasons = reasons.filter((r) => r !== reason); + } + + updateSetting('filterReasons', reasons.join(',')); + }; + + const shouldShowReason = (reason: Reason) => { + return settings.filterReasons.split(',').includes(reason); + }; + + const resetToDefaultFilters = () => { + updateSetting('filterReasons', defaultSettings.filterReasons); + updateSetting('showBots', defaultSettings.showBots); + }; + + return ( +
+
Filters
+
+
+ + Users + + + settings.detailedNotifications && + updateSetting('showBots', evt.target.checked) + } + disabled={!settings.detailedNotifications} + tooltip={ +
+
+ Show or hide notifications from Bot accounts, such as + @dependabot, @renovatebot, etc +
+
+ ⚠️ This setting requires{' '} + Detailed Notifications to be enabled. +
+
+ } + /> +
+ +
+ + Reason + + + Note: if no reasons are selected, all notifications will be shown. + + {Object.keys(FORMATTED_REASONS).map((reason: Reason) => { + return ( + + updateReasonFilter(reason, evt.target.checked) + } + tooltip={
{formatReason(reason).description}
} + /> + ); + })} +
+
+ +
+
+ +
+
+
+ ); +}; diff --git a/src/types.ts b/src/types.ts index 94aa95b8c..14a8ca48a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -53,7 +53,8 @@ export interface Account { export type SettingsState = AppearanceSettingsState & NotificationSettingsState & - SystemSettingsState; + SystemSettingsState & + FilterSettingsState; interface AppearanceSettingsState { theme: Theme; @@ -78,6 +79,10 @@ interface SystemSettingsState { keyboardShortcut: boolean; } +interface FilterSettingsState { + filterReasons: string | null; +} + export interface GitifyState { auth?: AuthState; settings?: SettingsState; diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 86ee2f5d5..2c7a21f48 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -1,5 +1,5 @@ import { formatDistanceToNowStrict, parseISO } from 'date-fns'; -import type { Hostname, Link } from '../types'; +import type { Hostname, Link, SettingsState } from '../types'; import type { Notification } from '../typesGitHub'; import { getHtmlUrl, getLatestDiscussion } from './api/client'; import type { PlatformType } from './auth/types'; @@ -166,3 +166,17 @@ export function formatNotificationUpdatedAt( return ''; } + +export function getFilterCount(settings: SettingsState): number { + let count = 0; + + if (settings.filterReasons) { + count += settings.filterReasons.split(',').length; + } + + if (!settings.showBots) { + count += 1; + } + + return count; +} diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index d65f06067..fdb631d66 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -184,6 +184,14 @@ export function filterNotifications( if (!settings.showBots && notification.subject?.user?.type === 'Bot') { return false; } + + if ( + settings.filterReasons && + !settings.filterReasons.split(',').includes(notification.reason) + ) { + return false; + } + return true; }); } diff --git a/src/utils/reason.ts b/src/utils/reason.ts index f8486ec0c..b14580ee1 100644 --- a/src/utils/reason.ts +++ b/src/utils/reason.ts @@ -1,7 +1,7 @@ import type { FormattedReason } from '../types'; import type { Reason } from '../typesGitHub'; -const FORMATTED_REASONS: Record = { +export const FORMATTED_REASONS: Record = { approval_requested: { title: 'Approval Requested', description: 'You were requested to review and approve a deployment.', From 1c55db1e74c632ba9efc091ff2888d15e0bf98df Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 10:39:27 -0400 Subject: [PATCH 02/16] feat: new filters route --- src/__mocks__/state-mocks.ts | 2 +- src/context/App.test.tsx | 4 ++-- src/context/App.tsx | 3 +-- src/types.ts | 2 +- src/utils/helpers.ts | 9 +++++++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/__mocks__/state-mocks.ts b/src/__mocks__/state-mocks.ts index 2d2c0d5ef..05e8e45bd 100644 --- a/src/__mocks__/state-mocks.ts +++ b/src/__mocks__/state-mocks.ts @@ -86,7 +86,7 @@ export const mockSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, - filterReasons: null, + filterReasons: '', }; export const mockState: GitifyState = { diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 8c9b7b471..ba269775f 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -381,7 +381,7 @@ describe('context/App.tsx', () => { showPills: true, keyboardShortcut: true, groupBy: 'REPOSITORY', - filterReasons: null, + filterReasons: '', } as SettingsState, }); }); @@ -435,7 +435,7 @@ describe('context/App.tsx', () => { showPills: true, keyboardShortcut: true, groupBy: 'REPOSITORY', - filterReasons: null, + filterReasons: '', } as SettingsState, }); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index 8d219d783..e1ab633a8 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -64,7 +64,7 @@ export const defaultSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, - filterReasons: null, + filterReasons: '', }; interface AppContextState { @@ -129,7 +129,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { }, [ settings.participating, settings.showBots, - settings.filterReasons, settings.detailedNotifications, settings.delayNotificationState, auth.accounts.length, diff --git a/src/types.ts b/src/types.ts index 14a8ca48a..0fe26cb4f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -80,7 +80,7 @@ interface SystemSettingsState { } interface FilterSettingsState { - filterReasons: string | null; + filterReasons: string; } export interface GitifyState { diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 2c7a21f48..89fc22b48 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -1,4 +1,5 @@ import { formatDistanceToNowStrict, parseISO } from 'date-fns'; +import { defaultSettings } from '../context/App'; import type { Hostname, Link, SettingsState } from '../types'; import type { Notification } from '../typesGitHub'; import { getHtmlUrl, getLatestDiscussion } from './api/client'; @@ -170,11 +171,15 @@ export function formatNotificationUpdatedAt( export function getFilterCount(settings: SettingsState): number { let count = 0; - if (settings.filterReasons) { + console.log('filter reasons', settings.filterReasons); + console.log('default settings', defaultSettings.filterReasons); + console.log('equal?', settings.filterReasons == ''); + + if (settings.filterReasons !== defaultSettings.filterReasons) { count += settings.filterReasons.split(',').length; } - if (!settings.showBots) { + if (settings.showBots !== defaultSettings.showBots) { count += 1; } From 6f2de40c0300b4446000d544a5d1c089c289bffc Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 11:03:49 -0400 Subject: [PATCH 03/16] fix lint --- src/utils/helpers.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 89fc22b48..796d8f475 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -171,10 +171,6 @@ export function formatNotificationUpdatedAt( export function getFilterCount(settings: SettingsState): number { let count = 0; - console.log('filter reasons', settings.filterReasons); - console.log('default settings', defaultSettings.filterReasons); - console.log('equal?', settings.filterReasons == ''); - if (settings.filterReasons !== defaultSettings.filterReasons) { count += settings.filterReasons.split(',').length; } From 40c7df0aaf7547504fd1bd20a8f16c79ae0ce865 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 11:06:48 -0400 Subject: [PATCH 04/16] add tests --- src/utils/helpers.test.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index ab4f45a2b..4e7aa1a44 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -1,7 +1,8 @@ import type { AxiosPromise, AxiosResponse } from 'axios'; import { mockPersonalAccessTokenAccount } from '../__mocks__/state-mocks'; -import type { Hostname, Link } from '../types'; +import { defaultSettings } from '../context/App'; +import type { Hostname, Link, SettingsState } from '../types'; import type { SubjectType } from '../typesGitHub'; import { mockGraphQLResponse, @@ -13,6 +14,7 @@ import { formatNotificationUpdatedAt, generateGitHubWebUrl, generateNotificationReferrerId, + getFilterCount, getPlatformFromHostname, isEnterpriseHost, } from './helpers'; @@ -500,4 +502,26 @@ describe('utils/helpers.ts', () => { }); }); }); + + describe('filter count', () => { + it('default filter settings', () => { + expect(getFilterCount(defaultSettings)).toBe(0); + }); + + it('non-default reason filters', () => { + const settings = { + ...defaultSettings, + filterReasons: 'subscribed,manual', + } as SettingsState; + expect(getFilterCount(settings)).toBe(2); + }); + + it('non-default bot filters', () => { + const settings = { + ...defaultSettings, + showBots: false, + } as SettingsState; + expect(getFilterCount(settings)).toBe(1); + }); + }); }); From e808629ed2384a094e0cfd1859aacacb4375d36e Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 11:17:51 -0400 Subject: [PATCH 05/16] add tests --- src/routes/Filters.test.tsx | 175 ++++ src/routes/Filters.tsx | 6 +- src/routes/Settings.test.tsx | 82 -- src/routes/Settings.tsx | 22 - .../__snapshots__/Filters.test.tsx.snap | 945 ++++++++++++++++++ .../__snapshots__/Settings.test.tsx.snap | 144 --- 6 files changed, 1123 insertions(+), 251 deletions(-) create mode 100644 src/routes/Filters.test.tsx create mode 100644 src/routes/__snapshots__/Filters.test.tsx.snap diff --git a/src/routes/Filters.test.tsx b/src/routes/Filters.test.tsx new file mode 100644 index 000000000..3641660bc --- /dev/null +++ b/src/routes/Filters.test.tsx @@ -0,0 +1,175 @@ +import { act, fireEvent, render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { mockAuth, mockSettings } from '../__mocks__/state-mocks'; +import { AppContext, defaultSettings } from '../context/App'; +import { FiltersRoute } from './Filters'; + +const mockNavigate = jest.fn(); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useNavigate: () => mockNavigate, +})); + +describe('routes/Filters.tsx', () => { + const updateSetting = jest.fn(); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('General', () => { + it('should render itself & its children', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + expect(screen.getByTestId('filters')).toMatchSnapshot(); + }); + + it('should go back by pressing the icon', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + fireEvent.click(screen.getByLabelText('Go Back')); + // TODO - add expects fetch notifications to be called. pending #1305 + expect(mockNavigate).toHaveBeenNthCalledWith(1, -1); + }); + }); + describe('Users section', () => { + it('should not be able to toggle the showBots checkbox when detailedNotifications is disabled', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + expect( + screen + .getByLabelText('Show notifications from Bot accounts') + .closest('input'), + ).toHaveProperty('disabled', true); + + // click the checkbox + fireEvent.click( + screen.getByLabelText('Show notifications from Bot accounts'), + ); + + // check if the checkbox is still unchecked + expect(updateSetting).not.toHaveBeenCalled(); + + expect( + screen.getByLabelText('Show notifications from Bot accounts').parentNode + .parentNode, + ).toMatchSnapshot(); + }); + + it('should be able to toggle the showBots checkbox when detailedNotifications is enabled', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + expect( + screen + .getByLabelText('Show notifications from Bot accounts') + .closest('input'), + ).toHaveProperty('disabled', false); + + // click the checkbox + fireEvent.click( + screen.getByLabelText('Show notifications from Bot accounts'), + ); + + // check if the checkbox is still unchecked + expect(updateSetting).toHaveBeenCalledWith('showBots', false); + + expect( + screen.getByLabelText('Show notifications from Bot accounts').parentNode + .parentNode, + ).toMatchSnapshot(); + }); + }); + + describe('Reasons section', () => {}); + + describe('Footer section', () => { + it('should reset filters', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + fireEvent.click(screen.getByTitle('Reset to default filters')); + + expect(updateSetting).toHaveBeenCalled(); + expect(updateSetting).toHaveBeenCalledWith( + 'showBots', + defaultSettings.showBots, + ); + expect(updateSetting).toHaveBeenCalledWith( + 'filterReasons', + defaultSettings.filterReasons, + ); + }); + }); +}); diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx index 078e23d80..b16b6ba69 100644 --- a/src/routes/Filters.tsx +++ b/src/routes/Filters.tsx @@ -39,7 +39,7 @@ export const FiltersRoute: FC = () => { }; return ( -
+
Filters
@@ -99,13 +99,13 @@ export const FiltersRoute: FC = () => { diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx index d6c2a2f23..3b5393760 100644 --- a/src/routes/Settings.test.tsx +++ b/src/routes/Settings.test.tsx @@ -263,88 +263,6 @@ describe('routes/Settings.tsx', () => { ); }); - it('should not be able to toggle the showBots checkbox when detailedNotifications is disabled', async () => { - await act(async () => { - render( - - - - - , - ); - }); - - expect( - screen - .getByLabelText('Show notifications from Bot accounts') - .closest('input'), - ).toHaveProperty('disabled', true); - - // click the checkbox - fireEvent.click( - screen.getByLabelText('Show notifications from Bot accounts'), - ); - - // check if the checkbox is still unchecked - expect(updateSetting).not.toHaveBeenCalled(); - - expect( - screen.getByLabelText('Show notifications from Bot accounts').parentNode - .parentNode, - ).toMatchSnapshot(); - }); - - it('should be able to toggle the showBots checkbox when detailedNotifications is enabled', async () => { - await act(async () => { - render( - - - - - , - ); - }); - - expect( - screen - .getByLabelText('Show notifications from Bot accounts') - .closest('input'), - ).toHaveProperty('disabled', false); - - // click the checkbox - fireEvent.click( - screen.getByLabelText('Show notifications from Bot accounts'), - ); - - // check if the checkbox is still unchecked - expect(updateSetting).toHaveBeenCalledWith('showBots', false); - - expect( - screen.getByLabelText('Show notifications from Bot accounts').parentNode - .parentNode, - ).toMatchSnapshot(); - }); - it('should toggle the markAsDoneOnOpen checkbox', async () => { await act(async () => { render( diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx index 5e6077605..eb2d00230 100644 --- a/src/routes/Settings.tsx +++ b/src/routes/Settings.tsx @@ -184,28 +184,6 @@ export const SettingsRoute: FC = () => {
} /> - - settings.detailedNotifications && - updateSetting('showBots', evt.target.checked) - } - disabled={!settings.detailedNotifications} - tooltip={ -
-
- Show or hide notifications from Bot accounts, such as - @dependabot, @renovatebot, etc -
-
- ⚠️ This setting requires{' '} - Detailed Notifications to be enabled. -
-
- } - /> +
+ +

+ Filters +

+
+
+
+ + Users + +
+
+
+ +
+
+ +
+
+
+
+
+ + Reason + + + Note: if no reasons are selected, all notifications will be shown. + +
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+ +
+
+ +
+
+
+
+
+
+
+ +
+
+
+`; + +exports[`routes/Filters.tsx Users section should be able to toggle the showBots checkbox when detailedNotifications is enabled 1`] = ` +
+
+ +
+
+ +
+
+`; + +exports[`routes/Filters.tsx Users section should not be able to toggle the showBots checkbox when detailedNotifications is disabled 1`] = ` +
+
+ +
+
+ +
+
+`; diff --git a/src/routes/__snapshots__/Settings.test.tsx.snap b/src/routes/__snapshots__/Settings.test.tsx.snap index 7dc308869..652050a1e 100644 --- a/src/routes/__snapshots__/Settings.test.tsx.snap +++ b/src/routes/__snapshots__/Settings.test.tsx.snap @@ -380,54 +380,6 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
-
-
-
- -
-
- -
-
-
@@ -734,99 +686,3 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
`; - -exports[`routes/Settings.tsx Notifications section should be able to toggle the showBots checkbox when detailedNotifications is enabled 1`] = ` -
-
- -
-
- -
-
-`; - -exports[`routes/Settings.tsx Notifications section should not be able to toggle the showBots checkbox when detailedNotifications is disabled 1`] = ` -
-
- -
-
- -
-
-`; From ac9a939a8c4e59d99dbd2f1fbbd928cb04866af4 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 11:26:44 -0400 Subject: [PATCH 06/16] add tests --- src/components/Sidebar.test.tsx | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/components/Sidebar.test.tsx b/src/components/Sidebar.test.tsx index 24f750762..31b950e5e 100644 --- a/src/components/Sidebar.test.tsx +++ b/src/components/Sidebar.test.tsx @@ -106,6 +106,44 @@ describe('components/Sidebar.tsx', () => { }); }); + describe('Filters', () => { + it('go to the filters route', () => { + render( + + + + + , + ); + fireEvent.click(screen.getByTitle('Filters')); + expect(mockNavigate).toHaveBeenCalledWith('/filters'); + }); + + it('go to the home if filters path already shown', () => { + render( + + + + + , + ); + fireEvent.click(screen.getByTitle('Filters')); + expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true }); + }); + }); + describe('Settings', () => { it('go to the settings route', () => { render( From cb0b6becab300c38094ff12a54d5751f22e3dbfa Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 11:30:17 -0400 Subject: [PATCH 07/16] add tests --- src/utils/notifications.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/utils/notifications.test.ts b/src/utils/notifications.test.ts index 5cf04ff16..31dea0ff9 100644 --- a/src/utils/notifications.test.ts +++ b/src/utils/notifications.test.ts @@ -187,5 +187,17 @@ describe('utils/notifications.ts', () => { expect(result.length).toBe(2); expect(result).toEqual(mockNotifications); }); + + it('should show bot notifications when set to true', async () => { + mockNotifications[0].reason = 'subscribed'; + mockNotifications[1].reason = 'manual'; + const result = filterNotifications(mockNotifications, { + ...mockSettings, + filterReasons: 'manual', + }); + + expect(result.length).toBe(1); + expect(result).toEqual([mockNotifications[1]]); + }); }); }); From 43449b0d177f03d304a1b06f5a1833066fff2cae Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 21 Jun 2024 11:34:51 -0400 Subject: [PATCH 08/16] add tests --- src/routes/Filters.test.tsx | 67 +++++++++++++- .../__snapshots__/Filters.test.tsx.snap | 92 +++++++++++++++++++ 2 files changed, 158 insertions(+), 1 deletion(-) diff --git a/src/routes/Filters.test.tsx b/src/routes/Filters.test.tsx index 3641660bc..26d12c7d9 100644 --- a/src/routes/Filters.test.tsx +++ b/src/routes/Filters.test.tsx @@ -139,7 +139,72 @@ describe('routes/Filters.tsx', () => { }); }); - describe('Reasons section', () => {}); + describe('Reasons section', () => { + it('should be able to toggle reason type - none already set', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + // click the checkbox + fireEvent.click(screen.getByLabelText('Mentioned')); + + // check if the checkbox is still unchecked + expect(updateSetting).toHaveBeenCalledWith('filterReasons', 'mention'); + + expect( + screen.getByLabelText('Mentioned').parentNode.parentNode, + ).toMatchSnapshot(); + }); + + it('should be able to toggle reason type - some filters already set', async () => { + await act(async () => { + render( + + + + + , + ); + }); + + // click the checkbox + fireEvent.click(screen.getByLabelText('Mentioned')); + + // check if the checkbox is still unchecked + expect(updateSetting).toHaveBeenCalledWith( + 'filterReasons', + 'security,mention', + ); + + expect( + screen.getByLabelText('Mentioned').parentNode.parentNode, + ).toMatchSnapshot(); + }); + }); describe('Footer section', () => { it('should reset filters', async () => { diff --git a/src/routes/__snapshots__/Filters.test.tsx.snap b/src/routes/__snapshots__/Filters.test.tsx.snap index 65e14870c..9adda0655 100644 --- a/src/routes/__snapshots__/Filters.test.tsx.snap +++ b/src/routes/__snapshots__/Filters.test.tsx.snap @@ -848,6 +848,98 @@ exports[`routes/Filters.tsx General should render itself & its children 1`] = ` `; +exports[`routes/Filters.tsx Reasons section should be able to toggle reason type - none already set 1`] = ` +
+
+ +
+
+ +
+
+`; + +exports[`routes/Filters.tsx Reasons section should be able to toggle reason type - some filters already set 1`] = ` +
+
+ +
+
+ +
+
+`; + exports[`routes/Filters.tsx Users section should be able to toggle the showBots checkbox when detailedNotifications is enabled 1`] = `
Date: Sat, 22 Jun 2024 08:18:45 -0400 Subject: [PATCH 09/16] rename tooltip --- src/routes/Filters.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx index b16b6ba69..fc7e9f204 100644 --- a/src/routes/Filters.tsx +++ b/src/routes/Filters.tsx @@ -62,8 +62,8 @@ export const FiltersRoute: FC = () => { @dependabot, @renovatebot, etc
- ⚠️ This setting requires{' '} - Detailed Notifications to be enabled. + ⚠️ This filter requires the{' '} + Detailed Notifications setting to be enabled.
} From d6d6fb423d8274ccf4397853eb323d431f7653ba Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 25 Jun 2024 06:08:05 -0400 Subject: [PATCH 10/16] call fetch on back --- src/components/Header.test.tsx | 6 ++---- src/components/Sidebar.test.tsx | 8 ++++---- src/routes/Filters.test.tsx | 4 +++- src/routes/Filters.tsx | 2 +- src/routes/Settings.test.tsx | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/components/Header.test.tsx b/src/components/Header.test.tsx index 81ad82dba..a381c8be8 100644 --- a/src/components/Header.test.tsx +++ b/src/components/Header.test.tsx @@ -26,8 +26,7 @@ describe('components/Header.tsx', () => { fireEvent.click(screen.getByLabelText('Go Back')); - expect(mockNavigate).toHaveBeenCalledTimes(1); - expect(mockNavigate).toHaveBeenCalledWith(-1); + expect(mockNavigate).toHaveBeenNthCalledWith(1, -1); }); it('should navigate back and fetch notifications', () => { @@ -43,8 +42,7 @@ describe('components/Header.tsx', () => { fireEvent.click(screen.getByLabelText('Go Back')); - expect(mockNavigate).toHaveBeenCalledTimes(1); - expect(mockNavigate).toHaveBeenCalledWith(-1); + expect(mockNavigate).toHaveBeenNthCalledWith(1, -1); expect(fetchNotifications).toHaveBeenCalledTimes(1); }); }); diff --git a/src/components/Sidebar.test.tsx b/src/components/Sidebar.test.tsx index 4b6e8787a..81e0b7b3a 100644 --- a/src/components/Sidebar.test.tsx +++ b/src/components/Sidebar.test.tsx @@ -247,7 +247,7 @@ describe('components/Sidebar.tsx', () => { , ); fireEvent.click(screen.getByTitle('Filters')); - expect(mockNavigate).toHaveBeenCalledWith('/filters'); + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/filters'); }); it('go to the home if filters path already shown', () => { @@ -265,7 +265,7 @@ describe('components/Sidebar.tsx', () => { , ); fireEvent.click(screen.getByTitle('Filters')); - expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true }); + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/', { replace: true }); }); }); @@ -287,7 +287,7 @@ describe('components/Sidebar.tsx', () => { fireEvent.click(screen.getByTitle('Settings')); - expect(mockNavigate).toHaveBeenCalledWith('/settings'); + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/settings'); }); it('go to the home if settings path already shown', () => { @@ -309,7 +309,7 @@ describe('components/Sidebar.tsx', () => { fireEvent.click(screen.getByTitle('Settings')); expect(fetchNotifications).toHaveBeenCalledTimes(1); - expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true }); + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/', { replace: true }); }); }); diff --git a/src/routes/Filters.test.tsx b/src/routes/Filters.test.tsx index 26d12c7d9..c38ad17fd 100644 --- a/src/routes/Filters.test.tsx +++ b/src/routes/Filters.test.tsx @@ -12,6 +12,7 @@ jest.mock('react-router-dom', () => ({ describe('routes/Filters.tsx', () => { const updateSetting = jest.fn(); + const fetchNotifications = jest.fn(); afterEach(() => { jest.clearAllMocks(); @@ -41,6 +42,7 @@ describe('routes/Filters.tsx', () => { value={{ auth: mockAuth, settings: mockSettings, + fetchNotifications, }} > @@ -51,7 +53,7 @@ describe('routes/Filters.tsx', () => { }); fireEvent.click(screen.getByLabelText('Go Back')); - // TODO - add expects fetch notifications to be called. pending #1305 + expect(fetchNotifications).toHaveBeenCalledTimes(1); expect(mockNavigate).toHaveBeenNthCalledWith(1, -1); }); }); diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx index fc7e9f204..ab0038988 100644 --- a/src/routes/Filters.tsx +++ b/src/routes/Filters.tsx @@ -40,7 +40,7 @@ export const FiltersRoute: FC = () => { return (
-
Filters
+
Filters
diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx index 0d3e769c4..974fef4a4 100644 --- a/src/routes/Settings.test.tsx +++ b/src/routes/Settings.test.tsx @@ -559,7 +559,7 @@ describe('routes/Settings.tsx', () => { }); fireEvent.click(screen.getByTitle('Accounts')); - expect(mockNavigate).toHaveBeenCalledWith('/accounts'); + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/accounts'); }); it('should quit the app', async () => { From 525002dced05c624d7ca9b8b9cb06164015c3157 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 25 Jun 2024 08:16:10 -0400 Subject: [PATCH 11/16] use array value instead of nasty string --- src/context/App.tsx | 8 +++----- src/routes/Settings.tsx | 4 ++-- src/types.ts | 2 ++ 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/context/App.tsx b/src/context/App.tsx index 0c0bf2a9d..d9ff3cd90 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -15,6 +15,7 @@ import { type GitifyError, GroupBy, type SettingsState, + type SettingsValue, type Status, Theme, } from '../types'; @@ -91,10 +92,7 @@ interface AppContextState { markRepoNotificationsDone: (notification: Notification) => Promise; settings: SettingsState; - updateSetting: ( - name: keyof SettingsState, - value: boolean | Theme | string | null, - ) => void; + updateSetting: (name: keyof SettingsState, value: SettingsValue) => void; } export const AppContext = createContext>({}); @@ -147,7 +145,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { }, [settings.keyboardShortcut]); const updateSetting = useCallback( - (name: keyof SettingsState, value: boolean | Theme) => { + (name: keyof SettingsState, value: SettingsValue) => { if (name === 'openAtStartup') { setAutoLaunch(value as boolean); } diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx index a7cd644fe..24c3d574e 100644 --- a/src/routes/Settings.tsx +++ b/src/routes/Settings.tsx @@ -72,7 +72,7 @@ export const SettingsRoute: FC = () => { { label: 'Dark', value: Theme.DARK }, ]} onChange={(evt) => { - updateSetting('theme', evt.target.value); + updateSetting('theme', evt.target.value as Theme); }} /> { { label: 'Date', value: GroupBy.DATE }, ]} onChange={(evt) => { - updateSetting('groupBy', evt.target.value); + updateSetting('groupBy', evt.target.value as GroupBy); }} /> Date: Tue, 25 Jun 2024 08:20:51 -0400 Subject: [PATCH 12/16] use array value instead of nasty string --- src/__mocks__/state-mocks.ts | 2 +- src/context/App.test.tsx | 4 ++-- src/context/App.tsx | 2 +- src/routes/Filters.test.tsx | 14 +++++++------- src/routes/Filters.tsx | 12 +++--------- src/types.ts | 8 ++++---- src/utils/helpers.test.ts | 2 +- src/utils/helpers.ts | 4 ++-- src/utils/notifications.test.ts | 2 +- src/utils/notifications.ts | 4 ++-- 10 files changed, 24 insertions(+), 30 deletions(-) diff --git a/src/__mocks__/state-mocks.ts b/src/__mocks__/state-mocks.ts index 05e8e45bd..eb54833b3 100644 --- a/src/__mocks__/state-mocks.ts +++ b/src/__mocks__/state-mocks.ts @@ -86,7 +86,7 @@ export const mockSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, - filterReasons: '', + filterReasons: [], }; export const mockState: GitifyState = { diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 12a33431b..38b3d3be9 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -373,7 +373,7 @@ describe('context/App.tsx', () => { showPills: true, keyboardShortcut: true, groupBy: 'REPOSITORY', - filterReasons: '', + filterReasons: [], } as SettingsState, }); }); @@ -427,7 +427,7 @@ describe('context/App.tsx', () => { showPills: true, keyboardShortcut: true, groupBy: 'REPOSITORY', - filterReasons: '', + filterReasons: [], } as SettingsState, }); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index d9ff3cd90..f92e3cfde 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -65,7 +65,7 @@ export const defaultSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, - filterReasons: '', + filterReasons: [], }; interface AppContextState { diff --git a/src/routes/Filters.test.tsx b/src/routes/Filters.test.tsx index c38ad17fd..431318b01 100644 --- a/src/routes/Filters.test.tsx +++ b/src/routes/Filters.test.tsx @@ -150,7 +150,7 @@ describe('routes/Filters.tsx', () => { auth: mockAuth, settings: { ...mockSettings, - filterReasons: '', + filterReasons: [], }, updateSetting, }} @@ -166,7 +166,7 @@ describe('routes/Filters.tsx', () => { fireEvent.click(screen.getByLabelText('Mentioned')); // check if the checkbox is still unchecked - expect(updateSetting).toHaveBeenCalledWith('filterReasons', 'mention'); + expect(updateSetting).toHaveBeenCalledWith('filterReasons', ['mention']); expect( screen.getByLabelText('Mentioned').parentNode.parentNode, @@ -181,7 +181,7 @@ describe('routes/Filters.tsx', () => { auth: mockAuth, settings: { ...mockSettings, - filterReasons: 'security', + filterReasons: ['security_alert'], }, updateSetting, }} @@ -197,10 +197,10 @@ describe('routes/Filters.tsx', () => { fireEvent.click(screen.getByLabelText('Mentioned')); // check if the checkbox is still unchecked - expect(updateSetting).toHaveBeenCalledWith( - 'filterReasons', - 'security,mention', - ); + expect(updateSetting).toHaveBeenCalledWith('filterReasons', [ + 'security_alert', + 'mention', + ]); expect( screen.getByLabelText('Mentioned').parentNode.parentNode, diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx index ab0038988..74d2d84b3 100644 --- a/src/routes/Filters.tsx +++ b/src/routes/Filters.tsx @@ -12,13 +12,7 @@ export const FiltersRoute: FC = () => { const { settings, updateSetting } = useContext(AppContext); const updateReasonFilter = (reason: Reason, checked: boolean) => { - let reasons: Reason[]; - - if (!settings.filterReasons) { - reasons = []; - } else { - reasons = settings.filterReasons.split(',') as Reason[]; - } + let reasons: Reason[] = settings.filterReasons; if (checked) { reasons.push(reason); @@ -26,11 +20,11 @@ export const FiltersRoute: FC = () => { reasons = reasons.filter((r) => r !== reason); } - updateSetting('filterReasons', reasons.join(',')); + updateSetting('filterReasons', reasons); }; const shouldShowReason = (reason: Reason) => { - return settings.filterReasons.split(',').includes(reason); + return settings.filterReasons.includes(reason); }; const resetToDefaultFilters = () => { diff --git a/src/types.ts b/src/types.ts index e3ee87143..a4d209f4d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,6 +1,6 @@ import type { OcticonProps } from '@primer/octicons-react'; import type { FC } from 'react'; -import type { Notification } from './typesGitHub'; +import type { Notification, Reason } from './typesGitHub'; import type { AuthMethod, EnterpriseAccount, @@ -51,7 +51,7 @@ export interface Account { user: GitifyUser | null; } -export type SettingsValue = boolean | Theme | GroupBy | string[]; +export type SettingsValue = boolean | Theme | GroupBy | Reason[]; export type SettingsState = AppearanceSettingsState & NotificationSettingsState & @@ -68,7 +68,6 @@ interface AppearanceSettingsState { interface NotificationSettingsState { participating: boolean; showNotifications: boolean; - showBots: boolean; markAsDoneOnOpen: boolean; delayNotificationState: boolean; groupBy: GroupBy; @@ -82,7 +81,8 @@ interface SystemSettingsState { } interface FilterSettingsState { - filterReasons: string; + showBots: boolean; + filterReasons: Reason[]; } export interface GitifyState { diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index 4e7aa1a44..aab9e1dd3 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -511,7 +511,7 @@ describe('utils/helpers.ts', () => { it('non-default reason filters', () => { const settings = { ...defaultSettings, - filterReasons: 'subscribed,manual', + filterReasons: ['subscribed', 'manual'], } as SettingsState; expect(getFilterCount(settings)).toBe(2); }); diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 796d8f475..a1f0745c6 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -171,8 +171,8 @@ export function formatNotificationUpdatedAt( export function getFilterCount(settings: SettingsState): number { let count = 0; - if (settings.filterReasons !== defaultSettings.filterReasons) { - count += settings.filterReasons.split(',').length; + if (settings.filterReasons.length !== defaultSettings.filterReasons.length) { + count += settings.filterReasons.length; } if (settings.showBots !== defaultSettings.showBots) { diff --git a/src/utils/notifications.test.ts b/src/utils/notifications.test.ts index 31dea0ff9..cd0b76f59 100644 --- a/src/utils/notifications.test.ts +++ b/src/utils/notifications.test.ts @@ -193,7 +193,7 @@ describe('utils/notifications.ts', () => { mockNotifications[1].reason = 'manual'; const result = filterNotifications(mockNotifications, { ...mockSettings, - filterReasons: 'manual', + filterReasons: ['manual'], }); expect(result.length).toBe(1); diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index fdb631d66..509c7736c 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -186,8 +186,8 @@ export function filterNotifications( } if ( - settings.filterReasons && - !settings.filterReasons.split(',').includes(notification.reason) + settings.filterReasons.length > 0 && + !settings.filterReasons.includes(notification.reason) ) { return false; } From 6a7429cd7da624da404b7fdc4076f7dab8443a52 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 28 Jun 2024 08:18:45 -0700 Subject: [PATCH 13/16] update footer for reset to default filters --- src/routes/Filters.tsx | 2 +- src/routes/__snapshots__/Filters.test.tsx.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx index 74d2d84b3..9231aafe8 100644 --- a/src/routes/Filters.tsx +++ b/src/routes/Filters.tsx @@ -89,6 +89,7 @@ export const FiltersRoute: FC = () => {
+
diff --git a/src/routes/__snapshots__/Filters.test.tsx.snap b/src/routes/__snapshots__/Filters.test.tsx.snap index 9adda0655..0248de054 100644 --- a/src/routes/__snapshots__/Filters.test.tsx.snap +++ b/src/routes/__snapshots__/Filters.test.tsx.snap @@ -820,6 +820,7 @@ exports[`routes/Filters.tsx General should render itself & its children 1`] = `
+
From 5e80d0afbe3a89b0fdaeb32393dd79189c5f6286 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 2 Jul 2024 08:36:14 -0800 Subject: [PATCH 14/16] refactor: use hideBots instead of showBots --- src/__mocks__/state-mocks.ts | 2 +- src/context/App.test.tsx | 50 +++++++++++++++++-- src/context/App.tsx | 16 +++++- src/routes/Filters.test.tsx | 41 +++++++-------- src/routes/Filters.tsx | 28 +++++------ .../__snapshots__/Filters.test.tsx.snap | 42 ++++++++-------- src/types.ts | 2 +- src/utils/helpers.test.ts | 2 +- src/utils/helpers.ts | 2 +- src/utils/notifications.test.ts | 10 ++-- src/utils/notifications.ts | 2 +- 11 files changed, 120 insertions(+), 77 deletions(-) diff --git a/src/__mocks__/state-mocks.ts b/src/__mocks__/state-mocks.ts index eb54833b3..68278bc84 100644 --- a/src/__mocks__/state-mocks.ts +++ b/src/__mocks__/state-mocks.ts @@ -75,7 +75,7 @@ export const mockSettings: SettingsState = { participating: false, playSound: true, showNotifications: true, - showBots: true, + hideBots: false, showNotificationsCountInTray: false, openAtStartup: false, theme: Theme.SYSTEM, diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 38b3d3be9..0a9f368a3 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -9,7 +9,7 @@ import * as comms from '../utils/comms'; import Constants from '../utils/constants'; import * as notifications from '../utils/notifications'; import * as storage from '../utils/storage'; -import { AppContext, AppProvider } from './App'; +import { AppContext, AppProvider, defaultSettings } from './App'; jest.mock('../hooks/useNotifications'); @@ -327,6 +327,14 @@ describe('context/App.tsx', () => { }); describe('settings methods', () => { + const fetchNotificationsMock = jest.fn(); + + beforeEach(() => { + (useNotifications as jest.Mock).mockReturnValue({ + fetchNotifications: fetchNotificationsMock, + }); + }); + it('should call updateSetting', async () => { const saveStateMock = jest .spyOn(storage, 'saveState') @@ -362,7 +370,7 @@ describe('context/App.tsx', () => { participating: true, playSound: true, showNotifications: true, - showBots: true, + hideBots: false, showNotificationsCountInTray: false, openAtStartup: false, theme: 'SYSTEM', @@ -416,7 +424,7 @@ describe('context/App.tsx', () => { participating: false, playSound: true, showNotifications: true, - showBots: true, + hideBots: false, showNotificationsCountInTray: false, openAtStartup: true, theme: 'SYSTEM', @@ -431,5 +439,41 @@ describe('context/App.tsx', () => { } as SettingsState, }); }); + + it('should clear filters back to default', async () => { + const saveStateMock = jest + .spyOn(storage, 'saveState') + .mockImplementation(jest.fn()); + + const TestComponent = () => { + const { clearFilters } = useContext(AppContext); + + return ( + + ); + }; + + const { getByText } = customRender(); + + act(() => { + fireEvent.click(getByText('Test Case')); + }); + + expect(saveStateMock).toHaveBeenCalledWith({ + auth: { + accounts: [], + enterpriseAccounts: [], + token: null, + user: null, + } as AuthState, + settings: { + ...mockSettings, + hideBots: defaultSettings.hideBots, + filterReasons: defaultSettings.filterReasons, + }, + }); + }); }); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index f92e3cfde..986ea2a28 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -50,11 +50,15 @@ const defaultAuth: AuthState = { user: null, }; +export const defaultFilters = { + hideBots: false, + filterReasons: [], +}; + export const defaultSettings: SettingsState = { participating: false, playSound: true, showNotifications: true, - showBots: true, showNotificationsCountInTray: false, openAtStartup: false, theme: Theme.SYSTEM, @@ -65,7 +69,7 @@ export const defaultSettings: SettingsState = { showPills: true, keyboardShortcut: true, groupBy: GroupBy.REPOSITORY, - filterReasons: [], + ...defaultFilters, }; interface AppContextState { @@ -93,6 +97,7 @@ interface AppContextState { settings: SettingsState; updateSetting: (name: keyof SettingsState, value: SettingsValue) => void; + clearFilters: () => void; } export const AppContext = createContext>({}); @@ -157,6 +162,12 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { [auth, settings], ); + const clearFilters = useCallback(() => { + const newSettings = { ...settings, ...defaultFilters }; + setSettings(newSettings); + saveState({ auth, settings: newSettings }); + }, [auth]); + const isLoggedIn = useMemo(() => { return auth.accounts.length > 0; }, [auth]); @@ -289,6 +300,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { settings, updateSetting, + clearFilters, }} > {children} diff --git a/src/routes/Filters.test.tsx b/src/routes/Filters.test.tsx index 431318b01..0ecb07154 100644 --- a/src/routes/Filters.test.tsx +++ b/src/routes/Filters.test.tsx @@ -1,7 +1,7 @@ import { act, fireEvent, render, screen } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; import { mockAuth, mockSettings } from '../__mocks__/state-mocks'; -import { AppContext, defaultSettings } from '../context/App'; +import { AppContext } from '../context/App'; import { FiltersRoute } from './Filters'; const mockNavigate = jest.fn(); @@ -12,6 +12,7 @@ jest.mock('react-router-dom', () => ({ describe('routes/Filters.tsx', () => { const updateSetting = jest.fn(); + const clearFilters = jest.fn(); const fetchNotifications = jest.fn(); afterEach(() => { @@ -58,7 +59,7 @@ describe('routes/Filters.tsx', () => { }); }); describe('Users section', () => { - it('should not be able to toggle the showBots checkbox when detailedNotifications is disabled', async () => { + it('should not be able to toggle the hideBots checkbox when detailedNotifications is disabled', async () => { await act(async () => { render( { settings: { ...mockSettings, detailedNotifications: false, - showBots: true, + hideBots: false, }, updateSetting, }} @@ -81,25 +82,25 @@ describe('routes/Filters.tsx', () => { expect( screen - .getByLabelText('Show notifications from Bot accounts') + .getByLabelText('Hide notifications from Bot accounts') .closest('input'), ).toHaveProperty('disabled', true); // click the checkbox fireEvent.click( - screen.getByLabelText('Show notifications from Bot accounts'), + screen.getByLabelText('Hide notifications from Bot accounts'), ); // check if the checkbox is still unchecked expect(updateSetting).not.toHaveBeenCalled(); expect( - screen.getByLabelText('Show notifications from Bot accounts').parentNode + screen.getByLabelText('Hide notifications from Bot accounts').parentNode .parentNode, ).toMatchSnapshot(); }); - it('should be able to toggle the showBots checkbox when detailedNotifications is enabled', async () => { + it('should be able to toggle the hideBots checkbox when detailedNotifications is enabled', async () => { await act(async () => { render( { settings: { ...mockSettings, detailedNotifications: true, - showBots: true, + hideBots: false, }, updateSetting, }} @@ -122,20 +123,20 @@ describe('routes/Filters.tsx', () => { expect( screen - .getByLabelText('Show notifications from Bot accounts') + .getByLabelText('Hide notifications from Bot accounts') .closest('input'), ).toHaveProperty('disabled', false); // click the checkbox fireEvent.click( - screen.getByLabelText('Show notifications from Bot accounts'), + screen.getByLabelText('Hide notifications from Bot accounts'), ); // check if the checkbox is still unchecked - expect(updateSetting).toHaveBeenCalledWith('showBots', false); + expect(updateSetting).toHaveBeenCalledWith('hideBots', true); expect( - screen.getByLabelText('Show notifications from Bot accounts').parentNode + screen.getByLabelText('Hide notifications from Bot accounts').parentNode .parentNode, ).toMatchSnapshot(); }); @@ -209,14 +210,14 @@ describe('routes/Filters.tsx', () => { }); describe('Footer section', () => { - it('should reset filters', async () => { + it('should clear filters', async () => { await act(async () => { render( @@ -226,17 +227,9 @@ describe('routes/Filters.tsx', () => { ); }); - fireEvent.click(screen.getByTitle('Reset to default filters')); + fireEvent.click(screen.getByTitle('Clear filters')); - expect(updateSetting).toHaveBeenCalled(); - expect(updateSetting).toHaveBeenCalledWith( - 'showBots', - defaultSettings.showBots, - ); - expect(updateSetting).toHaveBeenCalledWith( - 'filterReasons', - defaultSettings.filterReasons, - ); + expect(clearFilters).toHaveBeenCalled(); }); }); }); diff --git a/src/routes/Filters.tsx b/src/routes/Filters.tsx index 9231aafe8..4f46f863b 100644 --- a/src/routes/Filters.tsx +++ b/src/routes/Filters.tsx @@ -2,14 +2,14 @@ import { FilterRemoveIcon } from '@primer/octicons-react'; import { type FC, useContext } from 'react'; import { Header } from '../components/Header'; import { Checkbox } from '../components/fields/Checkbox'; -import { AppContext, defaultSettings } from '../context/App'; +import { AppContext } from '../context/App'; import { BUTTON_CLASS_NAME } from '../styles/gitify'; import { Size } from '../types'; import type { Reason } from '../typesGitHub'; import { FORMATTED_REASONS, formatReason } from '../utils/reason'; export const FiltersRoute: FC = () => { - const { settings, updateSetting } = useContext(AppContext); + const { settings, clearFilters, updateSetting } = useContext(AppContext); const updateReasonFilter = (reason: Reason, checked: boolean) => { let reasons: Reason[] = settings.filterReasons; @@ -27,11 +27,6 @@ export const FiltersRoute: FC = () => { return settings.filterReasons.includes(reason); }; - const resetToDefaultFilters = () => { - updateSetting('filterReasons', defaultSettings.filterReasons); - updateSetting('showBots', defaultSettings.showBots); - }; - return (
Filters
@@ -41,19 +36,19 @@ export const FiltersRoute: FC = () => { Users settings.detailedNotifications && - updateSetting('showBots', evt.target.checked) + updateSetting('hideBots', evt.target.checked) } disabled={!settings.detailedNotifications} tooltip={
- Show or hide notifications from Bot accounts, such as - @dependabot, @renovatebot, etc + Hide notifications from GitHub Bot accounts, such as + @dependabot, @renovate, @netlify, etc
⚠️ This filter requires the{' '} @@ -94,14 +89,15 @@ export const FiltersRoute: FC = () => {
diff --git a/src/routes/__snapshots__/Filters.test.tsx.snap b/src/routes/__snapshots__/Filters.test.tsx.snap index 0248de054..4bbea5a2c 100644 --- a/src/routes/__snapshots__/Filters.test.tsx.snap +++ b/src/routes/__snapshots__/Filters.test.tsx.snap @@ -57,9 +57,8 @@ exports[`routes/Filters.tsx General should render itself & its children 1`] = ` class="flex h-5 items-center" >
@@ -68,13 +67,13 @@ exports[`routes/Filters.tsx General should render itself & its children 1`] = ` >
@@ -940,7 +940,7 @@ exports[`routes/Filters.tsx Reasons section should be able to toggle reason type
`; -exports[`routes/Filters.tsx Users section should be able to toggle the showBots checkbox when detailedNotifications is enabled 1`] = ` +exports[`routes/Filters.tsx Users section should be able to toggle the hideBots checkbox when detailedNotifications is enabled 1`] = `
@@ -948,9 +948,8 @@ exports[`routes/Filters.tsx Users section should be able to toggle the showBots class="flex h-5 items-center" >
@@ -959,13 +958,13 @@ exports[`routes/Filters.tsx Users section should be able to toggle the showBots >