From 3d5ef3587398e41024f7012a4cb8b196b2b847c1 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 24 Nov 2025 09:49:06 -0500 Subject: [PATCH 1/2] refactor: account notifications Signed-off-by: Adam Setch --- src/renderer/context/App.tsx | 25 +++---- src/renderer/hooks/useNotifications.ts | 30 +++++++- src/renderer/utils/errors.ts | 16 +++-- .../utils/notifications/filters/reason.ts | 4 +- .../utils/notifications/filters/state.ts | 4 +- .../notifications/filters/subjectType.ts | 4 +- .../utils/notifications/filters/types.ts | 2 +- .../utils/notifications/filters/userType.ts | 4 +- .../utils/notifications/notifications.ts | 30 ++++---- src/renderer/utils/notifications/remove.ts | 8 +-- .../utils/notifications/utils.test.ts | 70 +++++++++---------- src/renderer/utils/notifications/utils.ts | 8 +-- 12 files changed, 114 insertions(+), 91 deletions(-) diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 542644aed..28f56cc9c 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -119,30 +119,23 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { const { setColorMode, setDayScheme, setNightScheme } = useTheme(); const { + status, + globalError, + notifications, + notificationCount, + unreadNotificationCount, + hasNotifications, + hasUnreadNotifications, + fetchNotifications, removeAccountNotifications, - status, - globalError, + markNotificationsAsRead, markNotificationsAsDone, unsubscribeNotification, } = useNotifications(); - const notificationCount = getNotificationCount(notifications); - - const unreadNotificationCount = getUnreadNotificationCount(notifications); - - const hasNotifications = useMemo( - () => notificationCount > 0, - [notificationCount], - ); - - const hasUnreadNotifications = useMemo( - () => unreadNotificationCount > 0, - [unreadNotificationCount], - ); - const refreshAllAccounts = useCallback(() => { if (!auth.accounts.length) { return; diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index e8076bb14..c636d1b25 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -1,4 +1,4 @@ -import { useCallback, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import type { Account, @@ -20,7 +20,11 @@ import { import { isMarkAsDoneFeatureSupported } from '../utils/features'; import { rendererLogError } from '../utils/logger'; import { raiseNativeNotification } from '../utils/notifications/native'; -import { getAllNotifications } from '../utils/notifications/notifications'; +import { + getAllNotifications, + getNotificationCount, + getUnreadNotificationCount, +} from '../utils/notifications/notifications'; import { removeNotificationsForAccount } from '../utils/notifications/remove'; import { raiseSoundNotification } from '../utils/notifications/sound'; import { getNewNotifications } from '../utils/notifications/utils'; @@ -30,6 +34,10 @@ interface NotificationsState { globalError: GitifyError; notifications: AccountNotifications[]; + notificationCount: number; + unreadNotificationCount: number; + hasNotifications: boolean; + hasUnreadNotifications: boolean; fetchNotifications: (state: GitifyState) => Promise; removeAccountNotifications: (account: Account) => Promise; @@ -56,6 +64,20 @@ export const useNotifications = (): NotificationsState => { [], ); + const notificationCount = getNotificationCount(notifications); + + const unreadNotificationCount = getUnreadNotificationCount(notifications); + + const hasNotifications = useMemo( + () => notificationCount > 0, + [notificationCount], + ); + + const hasUnreadNotifications = useMemo( + () => unreadNotificationCount > 0, + [unreadNotificationCount], + ); + const removeAccountNotifications = useCallback( async (account: Account) => { setStatus('loading'); @@ -224,6 +246,10 @@ export const useNotifications = (): NotificationsState => { globalError, notifications, + notificationCount, + unreadNotificationCount, + hasNotifications, + hasUnreadNotifications, fetchNotifications, removeAccountNotifications, diff --git a/src/renderer/utils/errors.ts b/src/renderer/utils/errors.ts index 0f83c5b4e..0929ec22d 100644 --- a/src/renderer/utils/errors.ts +++ b/src/renderer/utils/errors.ts @@ -35,22 +35,24 @@ export const Errors: Record = { * Check if all accounts have errors */ export function doesAllAccountsHaveErrors( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], ) { return ( - notifications.length > 0 && - notifications.every((account) => account.error !== null) + accountNotifications.length > 0 && + accountNotifications.every((account) => account.error !== null) ); } /** * Check if all account errors are the same */ -export function areAllAccountErrorsSame(notifications: AccountNotifications[]) { - if (notifications.length === 0) { +export function areAllAccountErrorsSame( + accountNotifications: AccountNotifications[], +) { + if (accountNotifications.length === 0) { return true; } - const firstError = notifications[0].error; - return notifications.every((account) => account.error === firstError); + const firstError = accountNotifications[0].error; + return accountNotifications.every((account) => account.error === firstError); } diff --git a/src/renderer/utils/notifications/filters/reason.ts b/src/renderer/utils/notifications/filters/reason.ts index 8983c99b0..7cf825acc 100644 --- a/src/renderer/utils/notifications/filters/reason.ts +++ b/src/renderer/utils/notifications/filters/reason.ts @@ -25,10 +25,10 @@ export const reasonFilter: Filter = { }, getFilterCount( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], reason: Reason, ): number { - return notifications.reduce( + return accountNotifications.reduce( (sum, account) => sum + account.notifications.filter((n) => this.filterNotification(n, reason)) diff --git a/src/renderer/utils/notifications/filters/state.ts b/src/renderer/utils/notifications/filters/state.ts index 72db25248..9ed75f98a 100644 --- a/src/renderer/utils/notifications/filters/state.ts +++ b/src/renderer/utils/notifications/filters/state.ts @@ -46,10 +46,10 @@ export const stateFilter: Filter = { }, getFilterCount( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], stateType: FilterStateType, ): number { - return notifications.reduce( + return accountNotifications.reduce( (sum, account) => sum + account.notifications.filter((n) => diff --git a/src/renderer/utils/notifications/filters/subjectType.ts b/src/renderer/utils/notifications/filters/subjectType.ts index d6efb0978..4e444c078 100644 --- a/src/renderer/utils/notifications/filters/subjectType.ts +++ b/src/renderer/utils/notifications/filters/subjectType.ts @@ -57,10 +57,10 @@ export const subjectTypeFilter: Filter = { }, getFilterCount( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], subjectType: SubjectType, ): number { - return notifications.reduce( + return accountNotifications.reduce( (sum, account) => sum + account.notifications.filter((n) => diff --git a/src/renderer/utils/notifications/filters/types.ts b/src/renderer/utils/notifications/filters/types.ts index ac7609d4b..438fda917 100644 --- a/src/renderer/utils/notifications/filters/types.ts +++ b/src/renderer/utils/notifications/filters/types.ts @@ -16,7 +16,7 @@ export interface Filter { isFilterSet(settings: SettingsState, type: T): boolean; - getFilterCount(notifications: AccountNotifications[], type: T): number; + getFilterCount(accountNotifications: AccountNotifications[], type: T): number; filterNotification(notification: Notification, type: T): boolean; } diff --git a/src/renderer/utils/notifications/filters/userType.ts b/src/renderer/utils/notifications/filters/userType.ts index 5565ef04d..8c712127d 100644 --- a/src/renderer/utils/notifications/filters/userType.ts +++ b/src/renderer/utils/notifications/filters/userType.ts @@ -37,10 +37,10 @@ export const userTypeFilter: Filter = { }, getFilterCount( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], userType: UserType, ): number { - return notifications.reduce( + return accountNotifications.reduce( (sum, account) => sum + account.notifications.filter((n) => diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 86d7f256c..7780af27c 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -17,11 +17,13 @@ import { createNotificationHandler } from './handlers'; /** * Get the count of notifications across all accounts. * - * @param notifications - The account notifications to check. + * @param accountNotifications - The account notifications to check. * @returns The count of all notifications. */ -export function getNotificationCount(notifications: AccountNotifications[]) { - return notifications.reduce( +export function getNotificationCount( + accountNotifications: AccountNotifications[], +) { + return accountNotifications.reduce( (sum, account) => sum + account.notifications.length, 0, ); @@ -30,13 +32,13 @@ export function getNotificationCount(notifications: AccountNotifications[]) { /** * Get the count of notifications across all accounts. * - * @param notifications - The account notifications to check. + * @param accountNotifications - The account notifications to check. * @returns The count of unread notifications. */ export function getUnreadNotificationCount( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], ) { - return notifications.reduce( + return accountNotifications.reduce( (sum, account) => sum + account.notifications.filter((n) => n.unread === true).length, 0, @@ -64,7 +66,7 @@ function getNotifications(state: GitifyState) { export async function getAllNotifications( state: GitifyState, ): Promise { - const notifications: AccountNotifications[] = await Promise.all( + const accountNotifications: AccountNotifications[] = await Promise.all( getNotifications(state) .filter((response) => !!response) .map(async (accountNotifications) => { @@ -113,9 +115,9 @@ export async function getAllNotifications( ); // Set the order property for the notifications - stabilizeNotificationsOrder(notifications, state.settings); + stabilizeNotificationsOrder(accountNotifications, state.settings); - return notifications; + return accountNotifications; } export async function enrichNotifications( @@ -178,23 +180,23 @@ export async function enrichNotification( * Assign an order property to each notification to stabilize how they are displayed * during notification interaction events (mark as read, mark as done, etc.) * - * @param notifications + * @param accountNotifications * @param settings */ export function stabilizeNotificationsOrder( - notifications: AccountNotifications[], + accountNotifications: AccountNotifications[], settings: SettingsState, ) { let orderIndex = 0; - for (const account of notifications) { + for (const account of accountNotifications) { const flattenedNotifications = getFlattenedNotificationsByRepo( account.notifications, settings, ); - for (const n of flattenedNotifications) { - n.order = orderIndex++; + for (const notification of flattenedNotifications) { + notification.order = orderIndex++; } } } diff --git a/src/renderer/utils/notifications/remove.ts b/src/renderer/utils/notifications/remove.ts index e1e140811..ad430fcdd 100644 --- a/src/renderer/utils/notifications/remove.ts +++ b/src/renderer/utils/notifications/remove.ts @@ -11,17 +11,17 @@ export function removeNotificationsForAccount( account: Account, settings: SettingsState, notificationsToRemove: Notification[], - allNotifications: AccountNotifications[], + accountNotifications: AccountNotifications[], ): AccountNotifications[] { if (notificationsToRemove.length === 0) { - return allNotifications; + return accountNotifications; } const notificationIDsToRemove = new Set( - notificationsToRemove.map((n) => n.id), + notificationsToRemove.map((notification) => notification.id), ); - return allNotifications.map((accountNotifications) => + return accountNotifications.map((accountNotifications) => getAccountUUID(account) === getAccountUUID(accountNotifications.account) ? { ...accountNotifications, diff --git a/src/renderer/utils/notifications/utils.test.ts b/src/renderer/utils/notifications/utils.test.ts index 647f0187b..d7d2bf089 100644 --- a/src/renderer/utils/notifications/utils.test.ts +++ b/src/renderer/utils/notifications/utils.test.ts @@ -9,7 +9,7 @@ import { getNewNotifications } from './utils'; describe('renderer/utils/notifications/utils.ts', () => { describe('getNewNotifications', () => { it('returns all notifications when previous is empty', () => { - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -28,7 +28,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('returns empty array when new is empty', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -36,7 +36,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const result = getNewNotifications(previousNotifications, []); + const result = getNewNotifications(previousAccountNotifications, []); expect(result).toHaveLength(0); }); @@ -48,7 +48,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('returns only new notifications, filtering out existing ones', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -59,7 +59,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -72,8 +72,8 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(2); @@ -81,7 +81,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('returns empty array when all notifications already exist', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -93,7 +93,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -106,15 +106,15 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(0); }); it('handles multiple accounts correctly', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -127,7 +127,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -147,8 +147,8 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(2); @@ -156,7 +156,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('treats new account as having all new notifications', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -164,7 +164,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -181,8 +181,8 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(2); @@ -190,7 +190,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('handles account with no notifications', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -198,7 +198,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [], @@ -207,15 +207,15 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(0); }); it('preserves notification order from input', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -223,7 +223,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -236,8 +236,8 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(3); @@ -245,7 +245,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('handles removed account gracefully', () => { - const previousNotifications: AccountNotifications[] = [ + const previousAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [createMockNotificationForRepoName('1', 'some/repo')], @@ -258,7 +258,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -271,8 +271,8 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(1); @@ -280,9 +280,9 @@ describe('renderer/utils/notifications/utils.ts', () => { }); it('handles multiple new notifications across multiple accounts', () => { - const previousNotifications: AccountNotifications[] = []; + const previousAccountNotifications: AccountNotifications[] = []; - const newNotifications: AccountNotifications[] = [ + const newAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [ @@ -302,8 +302,8 @@ describe('renderer/utils/notifications/utils.ts', () => { ]; const result = getNewNotifications( - previousNotifications, - newNotifications, + previousAccountNotifications, + newAccountNotifications, ); expect(result).toHaveLength(4); diff --git a/src/renderer/utils/notifications/utils.ts b/src/renderer/utils/notifications/utils.ts index a9b8cf160..dea4705f7 100644 --- a/src/renderer/utils/notifications/utils.ts +++ b/src/renderer/utils/notifications/utils.ts @@ -6,11 +6,11 @@ import { getAccountUUID } from '../auth/utils'; * Find notifications that exist in newNotifications but not in previousNotifications */ export function getNewNotifications( - previousNotifications: AccountNotifications[], - newNotifications: AccountNotifications[], + previousAccountNotifications: AccountNotifications[], + newAccountNotifications: AccountNotifications[], ): Notification[] { - return newNotifications.flatMap((accountNotifications) => { - const accountPreviousNotifications = previousNotifications.find( + return newAccountNotifications.flatMap((accountNotifications) => { + const accountPreviousNotifications = previousAccountNotifications.find( (item) => getAccountUUID(item.account) === getAccountUUID(accountNotifications.account), From 894acccf117c60a76cd0dcb5748bafa418eed41e Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 24 Nov 2025 09:52:45 -0500 Subject: [PATCH 2/2] refactor: account notifications Signed-off-by: Adam Setch --- src/renderer/context/App.tsx | 4 ---- src/renderer/utils/notifications/utils.test.ts | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 28f56cc9c..a2e308f62 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -47,10 +47,6 @@ import { setUseAlternateIdleIcon, setUseUnreadActiveIcon, } from '../utils/comms'; -import { - getNotificationCount, - getUnreadNotificationCount, -} from '../utils/notifications/notifications'; import { clearState, loadState, saveState } from '../utils/storage'; import { DEFAULT_DAY_COLOR_SCHEME, diff --git a/src/renderer/utils/notifications/utils.test.ts b/src/renderer/utils/notifications/utils.test.ts index d7d2bf089..1721dfdbd 100644 --- a/src/renderer/utils/notifications/utils.test.ts +++ b/src/renderer/utils/notifications/utils.test.ts @@ -21,7 +21,7 @@ describe('renderer/utils/notifications/utils.ts', () => { }, ]; - const result = getNewNotifications([], newNotifications); + const result = getNewNotifications([], newAccountNotifications); expect(result).toHaveLength(3); expect(result.map((n) => n.id)).toEqual(['1', '2', '3']);