From 5b349b227e2e7cfc14c0a0fdb14966f334b6e89c Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Wed, 5 Feb 2025 07:10:43 -0500 Subject: [PATCH 01/13] feat: filter by handles Signed-off-by: Adam Setch --- src/renderer/__mocks__/state-mocks.ts | 4 +- src/renderer/components/AllRead.test.tsx | 1 - src/renderer/components/AllRead.tsx | 2 +- src/renderer/components/Sidebar.tsx | 2 +- .../notifications/NotificationFooter.tsx | 4 +- src/renderer/context/App.test.tsx | 4 +- src/renderer/context/App.tsx | 30 ++- src/renderer/routes/Filters.test.tsx | 156 ++++++----- src/renderer/routes/Filters.tsx | 242 ++++++++++++++---- src/renderer/types.ts | 14 +- .../utils/__snapshots__/reason.test.ts.snap | 224 ++++++++++++++++ src/renderer/utils/filters.test.ts | 78 +++--- src/renderer/utils/filters.ts | 29 --- src/renderer/utils/notifications/filter.ts | 22 -- .../{ => filters}/filter.test.ts | 40 +-- .../utils/notifications/filters/filter.ts | 51 ++++ .../utils/notifications/filters/handles.ts | 17 ++ .../utils/notifications/filters/reason.ts | 28 ++ .../utils/notifications/filters/userType.ts | 64 +++++ .../utils/notifications/notifications.ts | 2 +- src/renderer/utils/reason.test.ts | 38 +-- src/renderer/utils/reason.ts | 8 +- 22 files changed, 777 insertions(+), 283 deletions(-) delete mode 100644 src/renderer/utils/filters.ts delete mode 100644 src/renderer/utils/notifications/filter.ts rename src/renderer/utils/notifications/{ => filters}/filter.test.ts (58%) create mode 100644 src/renderer/utils/notifications/filters/filter.ts create mode 100644 src/renderer/utils/notifications/filters/handles.ts create mode 100644 src/renderer/utils/notifications/filters/reason.ts create mode 100644 src/renderer/utils/notifications/filters/userType.ts diff --git a/src/renderer/__mocks__/state-mocks.ts b/src/renderer/__mocks__/state-mocks.ts index 315050f43..7905387d8 100644 --- a/src/renderer/__mocks__/state-mocks.ts +++ b/src/renderer/__mocks__/state-mocks.ts @@ -106,7 +106,9 @@ const mockSystemSettings: SystemSettingsState = { }; const mockFilters: FilterSettingsState = { - hideBots: false, + filterUserTypes: [], + filterIncludeHandles: [], + filterExcludeHandles: [], filterReasons: [], }; diff --git a/src/renderer/components/AllRead.test.tsx b/src/renderer/components/AllRead.test.tsx index 23afec8c1..d608d9098 100644 --- a/src/renderer/components/AllRead.test.tsx +++ b/src/renderer/components/AllRead.test.tsx @@ -35,7 +35,6 @@ describe('renderer/components/AllRead.tsx', () => { settings: { ...mockSettings, filterReasons: ['author'], - hideBots: true, }, }} > diff --git a/src/renderer/components/AllRead.tsx b/src/renderer/components/AllRead.tsx index 71af304eb..4109d4d33 100644 --- a/src/renderer/components/AllRead.tsx +++ b/src/renderer/components/AllRead.tsx @@ -2,7 +2,7 @@ import { type FC, useContext, useMemo } from 'react'; import { AppContext } from '../context/App'; import { Constants } from '../utils/constants'; -import { hasAnyFiltersSet } from '../utils/filters'; +import { hasAnyFiltersSet } from '../utils/notifications/filters/filter'; import { EmojiSplash } from './layout/EmojiSplash'; interface IAllRead { diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index 0c5de1d2c..49c511401 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -16,12 +16,12 @@ import { APPLICATION } from '../../shared/constants'; import { AppContext } from '../context/App'; import { quitApp } from '../utils/comms'; import { Constants } from '../utils/constants'; -import { hasAnyFiltersSet } from '../utils/filters'; import { openGitHubIssues, openGitHubNotifications, openGitHubPulls, } from '../utils/links'; +import { hasAnyFiltersSet } from '../utils/notifications/filters/filter'; import { getNotificationCount } from '../utils/notifications/notifications'; import { LogoIcon } from './icons/LogoIcon'; diff --git a/src/renderer/components/notifications/NotificationFooter.tsx b/src/renderer/components/notifications/NotificationFooter.tsx index dba9c20ac..f5c9b25bb 100644 --- a/src/renderer/components/notifications/NotificationFooter.tsx +++ b/src/renderer/components/notifications/NotificationFooter.tsx @@ -6,7 +6,7 @@ import { Opacity, Size } from '../../types'; import type { Notification } from '../../typesGitHub'; import { cn } from '../../utils/cn'; import { openUserProfile } from '../../utils/links'; -import { formatReason } from '../../utils/reason'; +import { getReasonDetails } from '../../utils/reason'; import { AvatarWithFallback } from '../avatars/AvatarWithFallback'; import { MetricGroup } from '../metrics/MetricGroup'; @@ -17,7 +17,7 @@ interface INotificationFooter { export const NotificationFooter: FC = ({ notification, }: INotificationFooter) => { - const reason = formatReason(notification.reason); + const reason = getReasonDetails(notification.reason); return ( { } as AuthState, settings: { ...mockSettings, - hideBots: defaultSettings.hideBots, + filterUserTypes: defaultSettings.filterUserTypes, + filterIncludeHandles: defaultSettings.filterIncludeHandles, + filterExcludeHandles: defaultSettings.filterExcludeHandles, filterReasons: defaultSettings.filterReasons, }, }); diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 68973bcd5..21a1ad3b6 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -19,6 +19,7 @@ import { type AppearanceSettingsState, type AuthState, type FilterSettingsState, + type FilterValue, type GitifyError, GroupBy, type NotificationSettingsState, @@ -97,7 +98,9 @@ const defaultSystemSettings: SystemSettingsState = { }; export const defaultFilters: FilterSettingsState = { - hideBots: false, + filterUserTypes: [], + filterIncludeHandles: [], + filterExcludeHandles: [], filterReasons: [], }; @@ -129,6 +132,11 @@ interface AppContextState { clearFilters: () => void; resetSettings: () => void; updateSetting: (name: keyof SettingsState, value: SettingsValue) => void; + updateFilter: ( + name: keyof FilterSettingsState, + value: FilterValue, + checked: boolean, + ) => void; } export const AppContext = createContext>({}); @@ -164,7 +172,13 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { // biome-ignore lint/correctness/useExhaustiveDependencies: We only want fetchNotifications to be called for particular state changes useEffect(() => { fetchNotifications({ auth, settings }); - }, [auth.accounts, settings.filterReasons, settings.hideBots]); + }, [ + auth.accounts, + settings.filterUserTypes, + settings.filterIncludeHandles, + settings.filterExcludeHandles, + settings.filterReasons, + ]); useInterval(() => { fetchNotifications({ auth, settings }); @@ -225,6 +239,17 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { [auth, settings], ); + const updateFilter = useCallback( + (name: keyof FilterSettingsState, value: FilterValue, checked: boolean) => { + const updatedFilters = checked + ? [...settings[name], value] + : settings[name].filter((item) => item !== value); + + updateSetting(name, updatedFilters); + }, + [updateSetting, settings], + ); + const isLoggedIn = useMemo(() => { return hasAccounts(auth); }, [auth]); @@ -356,6 +381,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { clearFilters, resetSettings, updateSetting, + updateFilter, }} > {children} diff --git a/src/renderer/routes/Filters.test.tsx b/src/renderer/routes/Filters.test.tsx index 52502dae6..ddcb9773c 100644 --- a/src/renderer/routes/Filters.test.tsx +++ b/src/renderer/routes/Filters.test.tsx @@ -65,89 +65,79 @@ describe('renderer/routes/Filters.tsx', () => { }); describe('Users section', () => { - it('should not be able to toggle the hideBots checkbox when detailedNotifications is disabled', async () => { - await act(async () => { - render( - - - - - , - ); - }); - - expect( - screen - .getByLabelText('Hide notifications from Bot accounts') - .closest('input'), - ).toHaveProperty('disabled', true); - - // click the checkbox - fireEvent.click( - screen.getByLabelText('Hide notifications from Bot accounts'), - ); - - // check if the checkbox is still unchecked - expect(updateSetting).not.toHaveBeenCalled(); - - expect( - screen.getByLabelText('Hide notifications from Bot accounts').parentNode - .parentNode, - ).toMatchSnapshot(); - }); - - it('should be able to toggle the hideBots checkbox when detailedNotifications is enabled', async () => { - await act(async () => { - render( - - - - - , - ); - }); - - expect( - screen - .getByLabelText('Hide notifications from Bot accounts') - .closest('input'), - ).toHaveProperty('disabled', false); - - // click the checkbox - fireEvent.click( - screen.getByLabelText('Hide notifications from Bot accounts'), - ); - - // check if the checkbox is still unchecked - expect(updateSetting).toHaveBeenCalledWith('hideBots', true); - - expect( - screen.getByLabelText('Hide notifications from Bot accounts').parentNode - .parentNode, - ).toMatchSnapshot(); - }); + // it('should not be able to toggle the hideBots checkbox when detailedNotifications is disabled', async () => { + // await act(async () => { + // render( + // + // + // + // + // , + // ); + // }); + // expect( + // screen + // .getByLabelText('Hide notifications from Bot accounts') + // .closest('input'), + // ).toHaveProperty('disabled', true); + // // click the checkbox + // fireEvent.click( + // screen.getByLabelText('Hide notifications from Bot accounts'), + // ); + // // check if the checkbox is still unchecked + // expect(updateSetting).not.toHaveBeenCalled(); + // expect( + // screen.getByLabelText('Hide notifications from Bot accounts').parentNode + // .parentNode, + // ).toMatchSnapshot(); + // }); + // it('should be able to toggle the hideBots checkbox when detailedNotifications is enabled', async () => { + // await act(async () => { + // render( + // + // + // + // + // , + // ); + // }); + // expect( + // screen + // .getByLabelText('Hide notifications from Bot accounts') + // .closest('input'), + // ).toHaveProperty('disabled', false); + // // click the checkbox + // fireEvent.click( + // screen.getByLabelText('Hide notifications from Bot accounts'), + // ); + // // check if the checkbox is still unchecked + // expect(updateSetting).toHaveBeenCalledWith('hideBots', true); + // expect( + // screen.getByLabelText('Hide notifications from Bot accounts').parentNode + // .parentNode, + // ).toMatchSnapshot(); + // }); }); describe('Reasons section', () => { diff --git a/src/renderer/routes/Filters.tsx b/src/renderer/routes/Filters.tsx index b0445ecad..391aec4fc 100644 --- a/src/renderer/routes/Filters.tsx +++ b/src/renderer/routes/Filters.tsx @@ -1,12 +1,22 @@ -import { type FC, useContext } from 'react'; +import { type FC, useContext, useState } from 'react'; import { + CheckCircleFillIcon, FeedPersonIcon, FilterIcon, FilterRemoveIcon, + MentionIcon, + NoEntryFillIcon, NoteIcon, } from '@primer/octicons-react'; -import { Box, Button, Stack, Text, Tooltip } from '@primer/react'; +import { + Box, + Button, + Stack, + Text, + TextInputWithTokens, + Tooltip, +} from '@primer/react'; import { Checkbox } from '../components/fields/Checkbox'; import { Contents } from '../components/layout/Contents'; @@ -15,28 +25,102 @@ import { Footer } from '../components/primitives/Footer'; import { Header } from '../components/primitives/Header'; import { Title } from '../components/primitives/Title'; import { AppContext } from '../context/App'; -import type { Reason } from '../typesGitHub'; -import { getNonBotFilterCount, getReasonFilterCount } from '../utils/filters'; -import { FORMATTED_REASONS, formatReason } from '../utils/reason'; +import { IconColor } from '../types'; +import type { Reason, UserType } from '../typesGitHub'; +import { + getReasonFilterCount, + isReasonFilterSet, +} from '../utils/notifications/filters/reason'; +import { + FILTERS_USER_TYPES, + getUserTypeDetails, + getUserTypeFilterCount, + isUserTypeFilterSet, +} from '../utils/notifications/filters/userType'; +import { FORMATTED_REASONS, getReasonDetails } from '../utils/reason'; + +type InputToken = { + id: number; + text: string; +}; + +const tokenEvents = ['Enter', 'Tab', ' ', ',']; export const FiltersRoute: FC = () => { - const { settings, clearFilters, updateSetting, notifications } = + const { settings, clearFilters, updateFilter, notifications } = useContext(AppContext); - const updateReasonFilter = (reason: Reason, checked: boolean) => { - let reasons: Reason[] = [...settings.filterReasons]; + const mapValuesToTokens = (values: string[]): InputToken[] => { + return values.map((value, index) => ({ + id: index, + text: value, + })); + }; + + const [includeHandles, setIncludeHandles] = useState( + mapValuesToTokens(settings.filterIncludeHandles), + ); - if (checked) { - reasons.push(reason); - } else { - reasons = reasons.filter((r) => r !== reason); + const removeIncludeHandleToken = (tokenId: string | number) => { + const value = includeHandles.find((v) => v.id === tokenId)?.text || ''; + updateFilter('filterIncludeHandles', value, false); + + setIncludeHandles(includeHandles.filter((v) => v.id !== tokenId)); + }; + + const includeHandlesKeyDown = ( + event: React.KeyboardEvent, + ) => { + const value = (event.target as HTMLInputElement).value.trim(); + + if ( + tokenEvents.includes(event.key) && + !includeHandles.some((v) => v.text === value) && + value.length > 0 + ) { + event.preventDefault(); + + setIncludeHandles([ + ...includeHandles, + { id: includeHandles.length, text: value }, + ]); + updateFilter('filterIncludeHandles', value, true); + + (event.target as HTMLInputElement).value = ''; } + }; - updateSetting('filterReasons', reasons); + const [excludeHandles, setExcludeHandles] = useState( + mapValuesToTokens(settings.filterExcludeHandles), + ); + + const removeExcludeHandleToken = (tokenId: string | number) => { + const value = includeHandles.find((v) => v.id === tokenId)?.text || ''; + updateFilter('filterExcludeHandles', value, false); + + setExcludeHandles(excludeHandles.filter((v) => v.id !== tokenId)); }; - const isReasonFilterSet = (reason: Reason) => { - return settings.filterReasons.includes(reason); + const excludeHandlesKeyDown = ( + event: React.KeyboardEvent, + ) => { + const value = (event.target as HTMLInputElement).value.trim(); + + if ( + tokenEvents.includes(event.key) && + !includeHandles.some((v) => v.text === value) && + value.length > 0 + ) { + event.preventDefault(); + + setExcludeHandles([ + ...excludeHandles, + { id: excludeHandles.length, text: value }, + ]); + updateFilter('filterExcludeHandles', value, false); + + (event.target as HTMLInputElement).value = ''; + } }; return ( @@ -47,57 +131,105 @@ export const FiltersRoute: FC = () => {
- Users - - - settings.detailedNotifications && - updateSetting('hideBots', evt.target.checked) - } - disabled={!settings.detailedNotifications} - tooltip={ - - - Hide notifications from GitHub Bot accounts, such as - @dependabot, @renovate, @netlify, etc - - - ⚠️ This filter requires the{' '} - Detailed Notifications setting to be - enabled. - - - } - counter={getNonBotFilterCount(notifications)} - /> + User Type + + + {Object.keys(FILTERS_USER_TYPES).map((userType: UserType) => { + const userTypeDetails = getUserTypeDetails(userType); + const userTypeTitle = userTypeDetails.title; + const userTypeDescription = userTypeDetails.description; + const isUserTypeChecked = isUserTypeFilterSet(settings, userType); + const userTypeCount = getUserTypeFilterCount( + notifications, + userType, + ); + + return ( + + updateFilter( + 'filterUserTypes', + userType, + evt.target.checked, + ) + } + tooltip={{userTypeDescription}} + counter={userTypeCount} + /> + ); + })} +
- Reason - - - Note: If no reasons are selected, all notifications will be shown. - - + Handles + + + + + + Include: + + + + + + + + + Exclude: + + + + + +
+ +
+ Reason {Object.keys(FORMATTED_REASONS).map((reason: Reason) => { - const reasonTitle = formatReason(reason).title; - const isReasonChecked = isReasonFilterSet(reason); - const reasonDescription = formatReason(reason).description; + const reasonDetails = getReasonDetails(reason); + const reasonTitle = reasonDetails.title; + const reasonDescription = reasonDetails.description; + const isReasonChecked = isReasonFilterSet(settings, reason); const reasonCount = getReasonFilterCount(notifications, reason); return ( - updateReasonFilter(reason, evt.target.checked) + updateFilter('filterReasons', reason, evt.target.checked) } tooltip={{reasonDescription}} counter={reasonCount} @@ -112,7 +244,11 @@ export const FiltersRoute: FC = () => {
+
- - + +
+
+ +

+ Handles +

+
+
+
- -
- +
+ + + Include: + +
+
+ +
+
+ +
+
+
-
-
- -
- +
+ + + Exclude: + +
+
+ +
+
+ +
+
+
-
- - +
+
+
- + +

+ Reason +

+
- +
- -
- + +
+ +
-
-
- -
- + +
+ +
-
-
- -
- + +
+ +
-
-
- -
- + +
+ +
-
-
- -
- + +
+ +
-
-
- -
- +
-
-
- - + Invitation Received + +
+ +
+
- + +
+ +
- -
- -
- + +
+ +
-
-
- -
- + +
+ +
-
-
- -
- + +
+ +
-
- -
- -
-
- - - -
-
- -`; - -exports[`renderer/routes/Filters.tsx Reasons section should be able to toggle reason type - none already set 1`] = ` -
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
-`; - -exports[`renderer/routes/Filters.tsx Reasons section should be able to toggle reason type - some filters already set 1`] = ` -
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- -
-
-
- - -
- +
+
+
+ + +
+ +
+
+
+ + +
+ +
+
+
+ + +
+ +
+
+
+ + +
+ +
+
+
+ - -`; - -exports[`renderer/routes/Filters.tsx Users section should be able to toggle the hideBots checkbox when detailedNotifications is enabled 1`] = ` -
- -
-
- -

- Users -

-
-
-
- -
- -
-
-
-`; - -exports[`renderer/routes/Filters.tsx Users section should not be able to toggle the hideBots checkbox when detailedNotifications is disabled 1`] = ` -
- -
- -
- -
- - -
- + + + + + + Clear filters + + + +
-
+ `; diff --git a/src/renderer/utils/__snapshots__/reason.test.ts.snap b/src/renderer/utils/__snapshots__/reason.test.ts.snap index 84d998abe..726de77f3 100644 --- a/src/renderer/utils/__snapshots__/reason.test.ts.snap +++ b/src/renderer/utils/__snapshots__/reason.test.ts.snap @@ -1,117 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 1`] = ` -{ - "description": "You were requested to review and approve a deployment.", - "title": "Approval Requested", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 2`] = ` -{ - "description": "You were assigned to the issue.", - "title": "Assigned", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 3`] = ` -{ - "description": "You created the thread.", - "title": "Authored", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 4`] = ` -{ - "description": "A GitHub Actions workflow run was triggered for your repository.", - "title": "Workflow Run Completed", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 5`] = ` -{ - "description": "You commented on the thread.", - "title": "Commented", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 6`] = ` -{ - "description": "You accepted an invitation to contribute to the repository.", - "title": "Invitation Received", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 7`] = ` -{ - "description": "You subscribed to the thread (via an issue or pull request).", - "title": "Updated", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 8`] = ` -{ - "description": "Organization members have requested to enable a feature such as Draft Pull Requests or Copilot.", - "title": "Member Feature Requested", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 9`] = ` -{ - "description": "You were specifically @mentioned in the content.", - "title": "Mentioned", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 10`] = ` -{ - "description": "You, or a team you're a member of, were requested to review a pull request.", - "title": "Review Requested", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 11`] = ` -{ - "description": "You were credited for contributing to a security advisory.", - "title": "Security Advisory Credit Received", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 12`] = ` -{ - "description": "GitHub discovered a security vulnerability in your repository.", - "title": "Security Alert Received", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 13`] = ` -{ - "description": "You changed the thread state (for example, closing an issue or merging a pull request).", - "title": "State Changed", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 14`] = ` -{ - "description": "You're watching the repository.", - "title": "Updated", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 15`] = ` -{ - "description": "You were on a team that was mentioned.", - "title": "Team Mentioned", -} -`; - -exports[`renderer/utils/reason.ts formatReason - should format the notification reason 16`] = ` -{ - "description": "The reason for this notification is not supported by the app.", - "title": "Unknown", -} -`; - exports[`renderer/utils/reason.ts getReasonDetails - should get details for notification reason 1`] = ` { "description": "You were requested to review and approve a deployment.", @@ -223,115 +111,3 @@ exports[`renderer/utils/reason.ts getReasonDetails - should get details for noti "title": "Unknown", } `; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 1`] = ` -{ - "description": "You were requested to review and approve a deployment.", - "title": "Approval Requested", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 2`] = ` -{ - "description": "You were assigned to the issue.", - "title": "Assigned", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 3`] = ` -{ - "description": "You created the thread.", - "title": "Authored", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 4`] = ` -{ - "description": "A GitHub Actions workflow run was triggered for your repository.", - "title": "Workflow Run Completed", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 5`] = ` -{ - "description": "You commented on the thread.", - "title": "Commented", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 6`] = ` -{ - "description": "You accepted an invitation to contribute to the repository.", - "title": "Invitation Received", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 7`] = ` -{ - "description": "You subscribed to the thread (via an issue or pull request).", - "title": "Updated", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 8`] = ` -{ - "description": "Organization members have requested to enable a feature such as Draft Pull Requests or Copilot.", - "title": "Member Feature Requested", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 9`] = ` -{ - "description": "You were specifically @mentioned in the content.", - "title": "Mentioned", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 10`] = ` -{ - "description": "You, or a team you're a member of, were requested to review a pull request.", - "title": "Review Requested", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 11`] = ` -{ - "description": "You were credited for contributing to a security advisory.", - "title": "Security Advisory Credit Received", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 12`] = ` -{ - "description": "GitHub discovered a security vulnerability in your repository.", - "title": "Security Alert Received", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 13`] = ` -{ - "description": "You changed the thread state (for example, closing an issue or merging a pull request).", - "title": "State Changed", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 14`] = ` -{ - "description": "You're watching the repository.", - "title": "Updated", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 15`] = ` -{ - "description": "You were on a team that was mentioned.", - "title": "Team Mentioned", -} -`; - -exports[`renderer/utils/reason.ts getReasonDetails - should get the notification reason 16`] = ` -{ - "description": "The reason for this notification is not supported by the app.", - "title": "Unknown", -} -`; diff --git a/src/renderer/utils/filters.test.ts b/src/renderer/utils/filters.test.ts deleted file mode 100644 index cc8c65c72..000000000 --- a/src/renderer/utils/filters.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -// import { mockSingleAccountNotifications } from '../__mocks__/notifications-mocks'; -// import { defaultSettings } from '../context/App'; -// import type { SettingsState } from '../types'; -// import { -// getNonBotFilterCount, -// getReasonFilterCount, -// hasAnyFiltersSet, -// } from './filters'; - -// describe('renderer/utils/filters.ts', () => { -// describe('has filters', () => { -// it('default filter settings', () => { -// expect(hasAnyFiltersSet(defaultSettings)).toBe(false); -// }); - -// it('non-default reason filters', () => { -// const settings = { -// ...defaultSettings, -// filterReasons: ['subscribed', 'manual'], -// } as SettingsState; -// expect(hasAnyFiltersSet(settings)).toBe(true); -// }); - -// it('non-default bot filters', () => { -// const settings = { -// ...defaultSettings, -// hideBots: true, -// } as SettingsState; -// expect(hasAnyFiltersSet(settings)).toBe(true); -// }); -// }); - -// describe('filter counts', () => { -// it('hideBots', () => { -// expect(getNonBotFilterCount(mockSingleAccountNotifications)).toBe(1); -// }); - -// it('reason', () => { -// expect( -// getReasonFilterCount(mockSingleAccountNotifications, 'subscribed'), -// ).toBe(1); -// }); -// }); -// }); diff --git a/src/renderer/utils/notifications/filters/filter.test.ts b/src/renderer/utils/notifications/filters/filter.test.ts index b11632804..4c2c3d58a 100644 --- a/src/renderer/utils/notifications/filters/filter.test.ts +++ b/src/renderer/utils/notifications/filters/filter.test.ts @@ -1,7 +1,8 @@ import { partialMockNotification } from '../../../__mocks__/partial-mocks'; import { mockSettings } from '../../../__mocks__/state-mocks'; -import type { Link } from '../../../types'; -import { filterNotifications } from './filter'; +import { defaultSettings } from '../../../context/App'; +import type { Link, SettingsState } from '../../../types'; +import { filterNotifications, hasAnyFiltersSet } from './filter'; describe('renderer/utils/notifications/filters/filter.ts', () => { afterEach(() => { @@ -13,7 +14,7 @@ describe('renderer/utils/notifications/filters/filter.ts', () => { partialMockNotification({ title: 'User authored notification', user: { - login: 'user', + login: 'github-user', html_url: 'https://github.com/user' as Link, avatar_url: 'https://avatars.githubusercontent.com/u/133795385?s=200&v=4' as Link, @@ -23,7 +24,7 @@ describe('renderer/utils/notifications/filters/filter.ts', () => { partialMockNotification({ title: 'Bot authored notification', user: { - login: 'bot', + login: 'github-bot', html_url: 'https://github.com/bot' as Link, avatar_url: 'https://avatars.githubusercontent.com/u/133795385?s=200&v=4' as Link, @@ -32,25 +33,51 @@ describe('renderer/utils/notifications/filters/filter.ts', () => { }), ]; - // it('should hide bot notifications when set to true', async () => { - // const result = filterNotifications(mockNotifications, { - // ...mockSettings, - // hideBots: true, - // }); + it('should ignore user type or handle filters if detailed notifications not enabled', async () => { + const result = filterNotifications(mockNotifications, { + ...mockSettings, + detailedNotifications: false, + filterUserTypes: ['Bot'], + filterIncludeHandles: ['github-user'], + filterExcludeHandles: ['github-bot'], + }); + + expect(result.length).toBe(2); + expect(result).toEqual(mockNotifications); + }); + + it('should filter notifications by user type provided', async () => { + const result = filterNotifications(mockNotifications, { + ...mockSettings, + detailedNotifications: true, + filterUserTypes: ['Bot'], + }); - // expect(result.length).toBe(1); - // expect(result).toEqual([mockNotifications[0]]); - // }); + expect(result.length).toBe(1); + expect(result).toEqual([mockNotifications[1]]); + }); - // it('should show bot notifications when set to false', async () => { - // const result = filterNotifications(mockNotifications, { - // ...mockSettings, - // hideBots: false, - // }); + it('should filter notifications that match include user handle', async () => { + const result = filterNotifications(mockNotifications, { + ...mockSettings, + detailedNotifications: true, + filterIncludeHandles: ['github-user'], + }); - // expect(result.length).toBe(2); - // expect(result).toEqual(mockNotifications); - // }); + expect(result.length).toBe(1); + expect(result).toEqual([mockNotifications[0]]); + }); + + it('should filter notifications that match exclude user handle', async () => { + const result = filterNotifications(mockNotifications, { + ...mockSettings, + detailedNotifications: true, + filterExcludeHandles: ['github-bot'], + }); + + expect(result.length).toBe(1); + expect(result).toEqual([mockNotifications[0]]); + }); it('should filter notifications by reasons when provided', async () => { mockNotifications[0].reason = 'subscribed'; @@ -64,4 +91,42 @@ describe('renderer/utils/notifications/filters/filter.ts', () => { expect(result).toEqual([mockNotifications[1]]); }); }); + + describe('has filters', () => { + it('default filter settings', () => { + expect(hasAnyFiltersSet(defaultSettings)).toBe(false); + }); + + it('non-default user type filters', () => { + const settings = { + ...defaultSettings, + filterUserTypes: ['Bot'], + } as SettingsState; + expect(hasAnyFiltersSet(settings)).toBe(true); + }); + + it('non-default user handle includes filters', () => { + const settings = { + ...defaultSettings, + filterIncludeHandles: ['gitify'], + } as SettingsState; + expect(hasAnyFiltersSet(settings)).toBe(true); + }); + + it('non-default user handle excludes filters', () => { + const settings = { + ...defaultSettings, + filterExcludeHandles: ['gitify'], + } as SettingsState; + expect(hasAnyFiltersSet(settings)).toBe(true); + }); + + it('non-default reason filters', () => { + const settings = { + ...defaultSettings, + filterReasons: ['subscribed', 'manual'], + } as SettingsState; + expect(hasAnyFiltersSet(settings)).toBe(true); + }); + }); }); diff --git a/src/renderer/utils/notifications/filters/filter.ts b/src/renderer/utils/notifications/filters/filter.ts index dbdcf2c6e..a14698709 100644 --- a/src/renderer/utils/notifications/filters/filter.ts +++ b/src/renderer/utils/notifications/filters/filter.ts @@ -13,22 +13,24 @@ export function filterNotifications( settings: SettingsState, ): Notification[] { return notifications.filter((notification) => { - if (hasUserTypeFilters(settings)) { - return settings.filterUserTypes.some((userType) => - filterNotificationByUserType(notification, userType), - ); - } + if (settings.detailedNotifications) { + if (hasUserTypeFilters(settings)) { + return settings.filterUserTypes.some((userType) => + filterNotificationByUserType(notification, userType), + ); + } - if (hasIncludeHandleFilters(settings)) { - return settings.filterIncludeHandles.some((handle) => - filterNotificationByHandle(notification, handle), - ); - } + if (hasIncludeHandleFilters(settings)) { + return settings.filterIncludeHandles.some((handle) => + filterNotificationByHandle(notification, handle), + ); + } - if (hasExcludeHandleFilters(settings)) { - return !settings.filterExcludeHandles.some((handle) => - filterNotificationByHandle(notification, handle), - ); + if (hasExcludeHandleFilters(settings)) { + return !settings.filterExcludeHandles.some((handle) => + filterNotificationByHandle(notification, handle), + ); + } } if (hasReasonFilters(settings)) { diff --git a/src/renderer/utils/notifications/filters/handles.ts b/src/renderer/utils/notifications/filters/handles.ts index 4807b27fb..2fe743d29 100644 --- a/src/renderer/utils/notifications/filters/handles.ts +++ b/src/renderer/utils/notifications/filters/handles.ts @@ -13,5 +13,5 @@ export function filterNotificationByHandle( notification: Notification, handleName: string, ): boolean { - return notification.subject?.user.login === handleName; + return notification.subject?.user?.login === handleName; } diff --git a/src/renderer/utils/notifications/filters/userType.ts b/src/renderer/utils/notifications/filters/userType.ts index 79e7af762..d2c23034c 100644 --- a/src/renderer/utils/notifications/filters/userType.ts +++ b/src/renderer/utils/notifications/filters/userType.ts @@ -8,16 +8,13 @@ import type { Notification, UserType } from '../../../typesGitHub'; export const FILTERS_USER_TYPES: Record = { User: { title: 'User', - description: 'Notifications by users', }, Bot: { title: 'Bot', - description: - 'Notifications by Bot accounts such as @dependabot, @renovate, @netlify, etc', + description: 'Bot accounts such as @dependabot, @renovate, @netlify, etc', }, Organization: { title: 'Organization', - description: 'Notifications from Organizations', }, } as Partial> as Record; From f0654456e628cc23ab385f1d4bd85af3c9200a88 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Thu, 6 Feb 2025 09:02:08 -0500 Subject: [PATCH 06/13] feat: filter by handles Signed-off-by: Adam Setch --- src/renderer/components/filters/UserHandleFilter.tsx | 11 ++++++----- src/renderer/context/App.tsx | 2 +- src/renderer/types.ts | 4 +++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/renderer/components/filters/UserHandleFilter.tsx b/src/renderer/components/filters/UserHandleFilter.tsx index 492ee1ea6..a8a0ee4c2 100644 --- a/src/renderer/components/filters/UserHandleFilter.tsx +++ b/src/renderer/components/filters/UserHandleFilter.tsx @@ -8,7 +8,7 @@ import { NoEntryFillIcon, } from '@primer/octicons-react'; import { AppContext } from '../../context/App'; -import { IconColor } from '../../types'; +import { IconColor, type UserHandle } from '../../types'; import { hasExcludeHandleFilters, hasIncludeHandleFilters, @@ -26,6 +26,7 @@ const tokenEvents = ['Enter', 'Tab', ' ', ',']; export const UserHandleFilter: FC = () => { const { updateFilter, settings } = useContext(AppContext); + // biome-ignore lint/correctness/useExhaustiveDependencies: we only want to run this effect on handle filter changes useEffect(() => { if (!hasIncludeHandleFilters(settings)) { setIncludeHandles([]); @@ -49,7 +50,7 @@ export const UserHandleFilter: FC = () => { const removeIncludeHandleToken = (tokenId: string | number) => { const value = includeHandles.find((v) => v.id === tokenId)?.text || ''; - updateFilter('filterIncludeHandles', value, false); + updateFilter('filterIncludeHandles', value as UserHandle, false); setIncludeHandles(includeHandles.filter((v) => v.id !== tokenId)); }; @@ -70,7 +71,7 @@ export const UserHandleFilter: FC = () => { ...includeHandles, { id: includeHandles.length, text: value }, ]); - updateFilter('filterIncludeHandles', value, true); + updateFilter('filterIncludeHandles', value as UserHandle, true); (event.target as HTMLInputElement).value = ''; } @@ -82,7 +83,7 @@ export const UserHandleFilter: FC = () => { const removeExcludeHandleToken = (tokenId: string | number) => { const value = excludeHandles.find((v) => v.id === tokenId)?.text || ''; - updateFilter('filterExcludeHandles', value, false); + updateFilter('filterExcludeHandles', value as UserHandle, false); setExcludeHandles(excludeHandles.filter((v) => v.id !== tokenId)); }; @@ -103,7 +104,7 @@ export const UserHandleFilter: FC = () => { ...excludeHandles, { id: excludeHandles.length, text: value }, ]); - updateFilter('filterExcludeHandles', value, true); + updateFilter('filterExcludeHandles', value as UserHandle, true); (event.target as HTMLInputElement).value = ''; } diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 21a1ad3b6..9630303d2 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -245,7 +245,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { ? [...settings[name], value] : settings[name].filter((item) => item !== value); - updateSetting(name, updatedFilters); + updateSetting(name, updatedFilters as FilterValue[]); }, [updateSetting, settings], ); diff --git a/src/renderer/types.ts b/src/renderer/types.ts index 6be69e073..83178d95a 100644 --- a/src/renderer/types.ts +++ b/src/renderer/types.ts @@ -25,6 +25,8 @@ export type Hostname = Branded; export type Link = Branded; +export type UserHandle = Branded; + export type Status = 'loading' | 'success' | 'error'; export interface Account { @@ -45,7 +47,7 @@ export type SettingsValue = | Theme | FilterValue[]; -export type FilterValue = Reason | UserType | string; +export type FilterValue = Reason | UserType | UserHandle; export type SettingsState = AppearanceSettingsState & NotificationSettingsState & From 2721a0d4e28dceb2c03307d2c76a432a4e488e50 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Thu, 6 Feb 2025 09:43:11 -0500 Subject: [PATCH 07/13] Update src/renderer/utils/notifications/filters/userType.ts Co-authored-by: Afonso Jorge Ramos --- src/renderer/utils/notifications/filters/userType.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/utils/notifications/filters/userType.ts b/src/renderer/utils/notifications/filters/userType.ts index d2c23034c..5c94fc769 100644 --- a/src/renderer/utils/notifications/filters/userType.ts +++ b/src/renderer/utils/notifications/filters/userType.ts @@ -38,9 +38,9 @@ export function getUserTypeFilterCount( userType: UserType, ) { return notifications.reduce( - (memo, acc) => - memo + - acc.notifications.filter((n) => filterNotificationByUserType(n, userType)) + (sum, account) => + sum + + account.notifications.filter((n) => filterNotificationByUserType(n, userType)) .length, 0, ); From 0f25bd56d0f6c3f147f0ba7ce104dceab5080e2c Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Thu, 6 Feb 2025 09:43:23 -0500 Subject: [PATCH 08/13] Update src/renderer/utils/notifications/filters/reason.ts Co-authored-by: Afonso Jorge Ramos --- src/renderer/utils/notifications/filters/reason.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/utils/notifications/filters/reason.ts b/src/renderer/utils/notifications/filters/reason.ts index 492f81bf2..34a7fe6f3 100644 --- a/src/renderer/utils/notifications/filters/reason.ts +++ b/src/renderer/utils/notifications/filters/reason.ts @@ -14,8 +14,8 @@ export function getReasonFilterCount( reason: Reason, ) { return notifications.reduce( - (memo, acc) => - memo + acc.notifications.filter((n) => n.reason === reason).length, + (sum, account) => + sum + account.notifications.filter((n) => n.reason === reason).length, 0, ); } From 74b708feb1fc561e3097aed941e3647ede0ae865 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Thu, 6 Feb 2025 09:45:30 -0500 Subject: [PATCH 09/13] Merge branch 'main' into feat/filter-by-handle Signed-off-by: Adam Setch --- src/renderer/routes/Filters.test.tsx | 76 ---------------------------- 1 file changed, 76 deletions(-) diff --git a/src/renderer/routes/Filters.test.tsx b/src/renderer/routes/Filters.test.tsx index 09d8cc19e..982fb9e62 100644 --- a/src/renderer/routes/Filters.test.tsx +++ b/src/renderer/routes/Filters.test.tsx @@ -63,82 +63,6 @@ describe('renderer/routes/Filters.tsx', () => { }); }); - describe('Users section', () => { - // it('should not be able to toggle the hideBots checkbox when detailedNotifications is disabled', async () => { - // await act(async () => { - // render( - // - // - // - // - // , - // ); - // }); - // expect( - // screen - // .getByLabelText('Hide notifications from Bot accounts') - // .closest('input'), - // ).toHaveProperty('disabled', true); - // // click the checkbox - // fireEvent.click( - // screen.getByLabelText('Hide notifications from Bot accounts'), - // ); - // // check if the checkbox is still unchecked - // expect(updateSetting).not.toHaveBeenCalled(); - // expect( - // screen.getByLabelText('Hide notifications from Bot accounts').parentNode - // .parentNode, - // ).toMatchSnapshot(); - // }); - // it('should be able to toggle the hideBots checkbox when detailedNotifications is enabled', async () => { - // await act(async () => { - // render( - // - // - // - // - // , - // ); - // }); - // expect( - // screen - // .getByLabelText('Hide notifications from Bot accounts') - // .closest('input'), - // ).toHaveProperty('disabled', false); - // // click the checkbox - // fireEvent.click( - // screen.getByLabelText('Hide notifications from Bot accounts'), - // ); - // // check if the checkbox is still unchecked - // expect(updateSetting).toHaveBeenCalledWith('hideBots', true); - // expect( - // screen.getByLabelText('Hide notifications from Bot accounts').parentNode - // .parentNode, - // ).toMatchSnapshot(); - // }); - }); - describe('Footer section', () => { it('should clear filters', async () => { await act(async () => { From ecdc356886f1081d6131439c7bde1d29d65414a6 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Thu, 6 Feb 2025 09:49:19 -0500 Subject: [PATCH 10/13] Merge branch 'main' into feat/filter-by-handle Signed-off-by: Adam Setch --- .../__snapshots__/AllRead.test.tsx.snap | 24 +- .../__snapshots__/Oops.test.tsx.snap | 24 +- .../__snapshots__/Sidebar.test.tsx.snap | 108 +- .../AvatarWithFallback.test.tsx.snap | 52 +- .../__snapshots__/Checkbox.test.tsx.snap | 40 +- .../__snapshots__/RadioGroup.test.tsx.snap | 24 +- .../__snapshots__/Centered.test.tsx.snap | 8 +- .../__snapshots__/EmojiSplash.test.tsx.snap | 24 +- .../__snapshots__/MetricGroup.test.tsx.snap | 400 +-- .../__snapshots__/MetricPill.test.tsx.snap | 20 +- .../AccountNotifications.test.tsx.snap | 490 ++-- .../NotificationFooter.test.tsx.snap | 236 +- .../NotificationHeader.test.tsx.snap | 48 +- .../NotificationRow.test.tsx.snap | 280 +- .../RepositoryNotifications.test.tsx.snap | 172 +- .../__snapshots__/Footer.test.tsx.snap | 8 +- .../__snapshots__/Header.test.tsx.snap | 16 +- .../__snapshots__/HoverButton.test.tsx.snap | 4 +- .../__snapshots__/HoverGroup.test.tsx.snap | 4 +- .../__snapshots__/Title.test.tsx.snap | 8 +- .../SettingsFooter.test.tsx.snap | 8 +- .../__snapshots__/Accounts.test.tsx.snap | 498 ++-- .../__snapshots__/Filters.test.tsx.snap | 2400 +---------------- .../routes/__snapshots__/Login.test.tsx.snap | 64 +- .../LoginWithOAuthApp.test.tsx.snap | 172 +- ...LoginWithPersonalAccessToken.test.tsx.snap | 160 +- .../__snapshots__/Settings.test.tsx.snap | 120 +- 27 files changed, 1567 insertions(+), 3845 deletions(-) diff --git a/src/renderer/components/__snapshots__/AllRead.test.tsx.snap b/src/renderer/components/__snapshots__/AllRead.test.tsx.snap index 851456cc8..89d98c835 100644 --- a/src/renderer/components/__snapshots__/AllRead.test.tsx.snap +++ b/src/renderer/components/__snapshots__/AllRead.test.tsx.snap @@ -6,7 +6,7 @@ exports[`renderer/components/AllRead.tsx should render itself & its children - n "baseElement":
, "container":
, "container":
, "container":
, "container":