feat(filters): split into two stages (base, detailed) to reduce api calls

src/renderer/utils/notifications/filters/filter.test.ts

@@ -2,7 +2,11 @@ import { partialMockNotification } from '../../../__mocks__/partial-mocks';
 import { mockSettings } from '../../../__mocks__/state-mocks';
 import { defaultSettings } from '../../../context/App';
 import type { Link, SettingsState } from '../../../types';
-import { filterNotifications, hasAnyFiltersSet } from './filter';
+import {
+  filterBaseNotifications,
+  filterDetailedNotifications,
+  hasAnyFiltersSet,
+} from './filter';
 
 describe('renderer/utils/notifications/filters/filter.ts', () => {
   afterEach(() => {
@@ -33,87 +37,91 @@ describe('renderer/utils/notifications/filters/filter.ts', () => {
       }),
     ];
 
-    it('should ignore user type, handle filters and state filters if detailed notifications not enabled', async () => {
-      const result = filterNotifications(mockNotifications, {
-        ...mockSettings,
-        detailedNotifications: false,
-        filterUserTypes: ['Bot'],
-        filterIncludeHandles: ['github-user'],
-        filterExcludeHandles: ['github-bot'],
-        filterStates: ['merged'],
+    describe('filterBaseNotifications', () => {
+      it('should filter notifications by subject type when provided', async () => {
+        mockNotifications[0].subject.type = 'Issue';
+        mockNotifications[1].subject.type = 'PullRequest';
+        const result = filterBaseNotifications(mockNotifications, {
+          ...mockSettings,
+          filterSubjectTypes: ['Issue'],
+        });
+
+        expect(result.length).toBe(1);
+        expect(result).toEqual([mockNotifications[0]]);
       });
 
-      expect(result.length).toBe(2);
-      expect(result).toEqual(mockNotifications);
-    });
+      it('should filter notifications by reasons when provided', async () => {
+        mockNotifications[0].reason = 'subscribed';
+        mockNotifications[1].reason = 'manual';
+        const result = filterBaseNotifications(mockNotifications, {
+          ...mockSettings,
+          filterReasons: ['manual'],
+        });
 
-    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[1]]);
       });
-
-      expect(result.length).toBe(1);
-      expect(result).toEqual([mockNotifications[1]]);
     });
 
-    it('should filter notifications that match include user handle', async () => {
-      const result = filterNotifications(mockNotifications, {
-        ...mockSettings,
-        detailedNotifications: true,
-        filterIncludeHandles: ['github-user'],
+    describe('filterDetailedNotifications', () => {
+      it('should ignore user type, handle filters and state filters if detailed notifications not enabled', async () => {
+        const result = filterDetailedNotifications(mockNotifications, {
+          ...mockSettings,
+          detailedNotifications: false,
+          filterUserTypes: ['Bot'],
+          filterIncludeHandles: ['github-user'],
+          filterExcludeHandles: ['github-bot'],
+          filterStates: ['merged'],
+        });
+
+        expect(result.length).toBe(2);
+        expect(result).toEqual(mockNotifications);
       });
 
-      expect(result.length).toBe(1);
-      expect(result).toEqual([mockNotifications[0]]);
-    });
+      it('should filter notifications by user type provided', async () => {
+        const result = filterDetailedNotifications(mockNotifications, {
+          ...mockSettings,
+          detailedNotifications: true,
+          filterUserTypes: ['Bot'],
+        });
 
-    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[1]]);
       });
 
-      expect(result.length).toBe(1);
-      expect(result).toEqual([mockNotifications[0]]);
-    });
+      it('should filter notifications that match include user handle', async () => {
+        const result = filterDetailedNotifications(mockNotifications, {
+          ...mockSettings,
+          detailedNotifications: true,
+          filterIncludeHandles: ['github-user'],
+        });
 
-    it('should filter notifications by subject type when provided', async () => {
-      mockNotifications[0].subject.type = 'Issue';
-      mockNotifications[1].subject.type = 'PullRequest';
-      const result = filterNotifications(mockNotifications, {
-        ...mockSettings,
-        filterSubjectTypes: ['Issue'],
+        expect(result.length).toBe(1);
+        expect(result).toEqual([mockNotifications[0]]);
       });
 
-      expect(result.length).toBe(1);
-      expect(result).toEqual([mockNotifications[0]]);
-    });
+      it('should filter notifications that match exclude user handle', async () => {
+        const result = filterDetailedNotifications(mockNotifications, {
+          ...mockSettings,
+          detailedNotifications: true,
+          filterExcludeHandles: ['github-bot'],
+        });
 
-    it('should filter notifications by state when provided', async () => {
-      mockNotifications[0].subject.state = 'open';
-      mockNotifications[1].subject.state = 'closed';
-      const result = filterNotifications(mockNotifications, {
-        ...mockSettings,
-        filterStates: ['closed'],
+        expect(result.length).toBe(1);
+        expect(result).toEqual([mockNotifications[0]]);
       });
 
-      expect(result.length).toBe(1);
-      expect(result).toEqual([mockNotifications[1]]);
-    });
+      it('should filter notifications by state when provided', async () => {
+        mockNotifications[0].subject.state = 'open';
+        mockNotifications[1].subject.state = 'closed';
+        const result = filterDetailedNotifications(mockNotifications, {
+          ...mockSettings,
+          filterStates: ['closed'],
+        });
 
-    it('should filter notifications by reasons when provided', 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]]);
       });
-
-      expect(result.length).toBe(1);
-      expect(result).toEqual([mockNotifications[1]]);
     });
   });
 

src/renderer/utils/notifications/filters/filter.ts

@@ -10,7 +10,34 @@ import {
   userTypeFilter,
 } from '.';
 
-export function filterNotifications(
+export function filterBaseNotifications(
+  notifications: Notification[],
+  settings: SettingsState,
+): Notification[] {
+  return notifications.filter((notification) => {
+    let passesFilters = true;
+
+    if (subjectTypeFilter.hasFilters(settings)) {
+      passesFilters =
+        passesFilters &&
+        settings.filterSubjectTypes.some((subjectType) =>
+          subjectTypeFilter.filterNotification(notification, subjectType),
+        );
+    }
+
+    if (reasonFilter.hasFilters(settings)) {
+      passesFilters =
+        passesFilters &&
+        settings.filterReasons.some((reason) =>
+          reasonFilter.filterNotification(notification, reason),
+        );
+    }
+
+    return passesFilters;
+  });
+}
+
+export function filterDetailedNotifications(
   notifications: Notification[],
   settings: SettingsState,
 ): Notification[] {
@@ -51,22 +78,6 @@ export function filterNotifications(
       }
     }
 
-    if (subjectTypeFilter.hasFilters(settings)) {
-      passesFilters =
-        passesFilters &&
-        settings.filterSubjectTypes.some((subjectType) =>
-          subjectTypeFilter.filterNotification(notification, subjectType),
-        );
-    }
-
-    if (reasonFilter.hasFilters(settings)) {
-      passesFilters =
-        passesFilters &&
-        settings.filterReasons.some((reason) =>
-          reasonFilter.filterNotification(notification, reason),
-        );
-    }
-
     return passesFilters;
   });
 }

src/renderer/utils/notifications/notifications.ts

@@ -1,11 +1,18 @@
 import { logError, logWarn } from '../../../shared/logger';
-import type { AccountNotifications, GitifyState } from '../../types';
+import type {
+  AccountNotifications,
+  GitifyState,
+  SettingsState,
+} from '../../types';
 import type { GitifySubject, Notification } from '../../typesGitHub';
 import { listNotificationsForAuthenticatedUser } from '../api/client';
 import { determineFailureType } from '../api/errors';
 import { updateTrayIcon } from '../comms';
 import { getGitifySubjectDetails } from '../subject';
-import { filterNotifications } from './filters/filter';
+import {
+  filterBaseNotifications,
+  filterDetailedNotifications,
+} from './filters/filter';
 
 export function setTrayIconColor(notifications: AccountNotifications[]) {
   const allNotificationsCount = getNotificationCount(notifications);
@@ -49,9 +56,20 @@ export async function getAllNotifications(
             account: accountNotifications.account,
           }));
 
-          notifications = await enrichNotifications(notifications, state);
+          notifications = filterBaseNotifications(
+            notifications,
+            state.settings,
+          );
 
-          notifications = filterNotifications(notifications, state.settings);
+          notifications = await enrichNotifications(
+            notifications,
+            state.settings,
+          );
+
+          notifications = filterDetailedNotifications(
+            notifications,
+            state.settings,
+          );
 
           return {
             account: accountNotifications.account,
@@ -79,9 +97,9 @@ export async function getAllNotifications(
 
 export async function enrichNotifications(
   notifications: Notification[],
-  state: GitifyState,
+  settings: SettingsState,
 ): Promise<Notification[]> {
-  if (!state.settings.detailedNotifications) {
+  if (!settings.detailedNotifications) {
     return notifications;
   }