diff --git a/src/renderer/components/notifications/AccountNotifications.tsx b/src/renderer/components/notifications/AccountNotifications.tsx index 7d8c3376f..c1f919519 100644 --- a/src/renderer/components/notifications/AccountNotifications.tsx +++ b/src/renderer/components/notifications/AccountNotifications.tsx @@ -13,6 +13,10 @@ import { openGitHubIssues, openGitHubPulls, } from '../../utils/links'; +import { + groupNotificationsByRepository, + isGroupByRepository, +} from '../../utils/notifications/group'; import { AllRead } from '../AllRead'; import { AvatarWithFallback } from '../avatars/AvatarWithFallback'; import { Oops } from '../Oops'; @@ -31,28 +35,26 @@ interface IAccountNotifications { export const AccountNotifications: FC = ( props: IAccountNotifications, ) => { - const { account, showAccountHeader, notifications } = props; + const { account, showAccountHeader, error, notifications } = props; const { settings } = useContext(AppContext); const [showAccountNotifications, setShowAccountNotifications] = useState(true); - const groupedNotifications = Object.values( - notifications.reduce( - (acc: { [key: string]: Notification[] }, notification) => { - const key = notification.repository.full_name; - if (!acc[key]) { - acc[key] = []; - } - - acc[key].push(notification); - return acc; - }, - {}, - ), + const sortedNotifications = useMemo( + () => [...notifications].sort((a, b) => a.order - b.order), + [notifications], ); + const groupedNotifications = useMemo(() => { + const map = groupNotificationsByRepository([ + { account, error, notifications: sortedNotifications }, + ]); + + return Array.from(map.values()); + }, [account, error, sortedNotifications]); + const hasNotifications = useMemo( () => notifications.length > 0, [notifications], @@ -68,8 +70,6 @@ export const AccountNotifications: FC = ( 'account', ); - const isGroupByRepository = settings.groupBy === 'REPOSITORY'; - return ( <> {showAccountHeader && ( @@ -130,7 +130,7 @@ export const AccountNotifications: FC = ( <> {props.error && } {!hasNotifications && !props.error && } - {isGroupByRepository + {isGroupByRepository(settings) ? Object.values(groupedNotifications).map((repoNotifications) => { const repoSlug = repoNotifications[0].repository.full_name; @@ -142,7 +142,7 @@ export const AccountNotifications: FC = ( /> ); }) - : notifications.map((notification) => ( + : sortedNotifications.map((notification) => ( { return ( - {notifications.map((accountNotifications) => ( - - ))} + {notifications.map((accountNotification) => { + return ( + + ); + })} ); diff --git a/src/renderer/typesGitHub.ts b/src/renderer/typesGitHub.ts index d38ff2c27..a3c7f8e35 100644 --- a/src/renderer/typesGitHub.ts +++ b/src/renderer/typesGitHub.ts @@ -112,6 +112,7 @@ export interface GitHubNotification { // Note: This is not in the official GitHub API. We add this to make notification interactions easier. export interface GitifyNotification { account: Account; + order: number; } export type UserDetails = User & UserProfile; diff --git a/src/renderer/utils/api/__mocks__/response-mocks.ts b/src/renderer/utils/api/__mocks__/response-mocks.ts index 6aa43e4a9..2b79c20d3 100644 --- a/src/renderer/utils/api/__mocks__/response-mocks.ts +++ b/src/renderer/utils/api/__mocks__/response-mocks.ts @@ -45,6 +45,7 @@ export const mockNotificationUser: User = { export const mockGitHubNotifications: Notification[] = [ { account: mockGitHubCloudAccount, + order: 0, id: '138661096', unread: true, reason: 'subscribed', @@ -197,6 +198,7 @@ export const mockGitHubNotifications: Notification[] = [ }, { account: mockGitHubCloudAccount, + order: 1, id: '148827438', unread: true, reason: 'author', @@ -260,6 +262,7 @@ export const mockGitHubNotifications: Notification[] = [ export const mockEnterpriseNotifications: Notification[] = [ { account: mockGitHubEnterpriseServerAccount, + order: 0, id: '3', unread: true, reason: 'subscribed', @@ -316,6 +319,7 @@ export const mockEnterpriseNotifications: Notification[] = [ }, { account: mockGitHubEnterpriseServerAccount, + order: 1, id: '4', unread: true, reason: 'subscribed', diff --git a/src/renderer/utils/notifications/group.ts b/src/renderer/utils/notifications/group.ts new file mode 100644 index 000000000..0c118071a --- /dev/null +++ b/src/renderer/utils/notifications/group.ts @@ -0,0 +1,50 @@ +import type { AccountNotifications, SettingsState } from '../../types'; +import type { Notification } from '../../typesGitHub'; + +/** + * Returns true when settings say to group by repository. + */ +export function isGroupByRepository(settings: SettingsState) { + return settings.groupBy === 'REPOSITORY'; +} + +/** + * Group notifications by repository.full_name preserving first-seen repo order. + * Returns a Map where keys are repo full_names and values are arrays of notifications. + */ +export function groupNotificationsByRepository( + accounts: AccountNotifications[], +): Map { + const repoGroups = new Map(); + + for (const account of accounts) { + for (const notification of account.notifications) { + const repo = notification.repository?.full_name ?? ''; + const group = repoGroups.get(repo); + + if (group) { + group.push(notification); + } else { + repoGroups.set(repo, [notification]); + } + } + } + + return repoGroups; +} + +/** + * Returns a flattened, ordered Notification[] according to repository-first-seen order + * (when grouped) or the natural account->notification order otherwise. + */ +export function getFlattenedNotificationsByRepo( + accounts: AccountNotifications[], + settings: SettingsState, +): Notification[] { + if (isGroupByRepository(settings)) { + const repoGroups = groupNotificationsByRepository(accounts); + return Array.from(repoGroups.values()).flat(); + } + + return accounts.flatMap((a) => a.notifications); +} diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 1aef06d0b..62b92cb0c 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -12,6 +12,7 @@ import { filterBaseNotifications, filterDetailedNotifications, } from './filters/filter'; +import { getFlattenedNotificationsByRepo } from './group'; import { createNotificationHandler } from './handlers'; export function setTrayIconColor(notifications: AccountNotifications[]) { @@ -90,6 +91,9 @@ export async function getAllNotifications( }), ); + // Set the order property for the notifications + stabilizeNotificationsOrder(notifications, state.settings); + return notifications; } @@ -141,3 +145,26 @@ 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 settings + */ +export function stabilizeNotificationsOrder( + notifications: AccountNotifications[], + settings: SettingsState, +) { + const flattenedNotifications = getFlattenedNotificationsByRepo( + notifications, + settings, + ); + + let orderIndex = 0; + + for (const n of flattenedNotifications) { + n.order = orderIndex++; + } +}