fix(api): split merged graphql enrichment into batches to limit errors (#2555)

Signed-off-by: Adam Setch <adam.setch@outlook.com>

Author: setchy

src/renderer/constants.ts

@@ -18,6 +18,7 @@ export const Constants = {
 
   GITHUB_API_BASE_URL: 'https://api.github.com',
   GITHUB_API_GRAPHQL_URL: 'https://api.github.com/graphql',
+  GITHUB_API_MERGE_BATCH_SIZE: 100,
 
   ALL_READ_EMOJIS: ['🎉', '🎊', '🥳', '👏', '🙌', '😎', '🏖️', '🚀', '✨', '🏆'],
 

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

@@ -12,6 +12,8 @@ import {
 } from '../../__mocks__/notifications-mocks';
 import { mockSettings } from '../../__mocks__/state-mocks';
 
+import { Constants } from '../../constants';
+
 import {
   type AccountNotifications,
   type GitifyNotification,
@@ -22,6 +24,7 @@ import {
 } from '../../types';
 
 import * as logger from '../../utils/logger';
+import * as apiClient from '../api/client';
 import {
   enrichNotification,
   enrichNotifications,
@@ -190,5 +193,68 @@ describe('renderer/utils/notifications/notifications.ts', () => {
 
       expect(result).toEqual([]);
     });
+
+    it('should batch notifications by GITHUB_API_MERGE_BATCH_SIZE', async () => {
+      const fetchNotificationDetailsForListSpy = jest
+        .spyOn(apiClient, 'fetchNotificationDetailsForList')
+        .mockResolvedValue(new Map());
+
+      const notificationCount = Constants.GITHUB_API_MERGE_BATCH_SIZE * 2.5;
+      const notifications = Array.from({ length: notificationCount }, (_, i) =>
+        mockPartialGitifyNotification({
+          title: `Notification ${i}`,
+          type: 'Issue',
+          url: `https://api.github.com/repos/gitify-app/notifications-test/issues/${i}` as Link,
+        }),
+      ) as GitifyNotification[];
+
+      const settings: SettingsState = {
+        ...mockSettings,
+        detailedNotifications: true,
+      };
+
+      await enrichNotifications(notifications, settings);
+
+      // Should be called 3 times: batches of 100, 100, 50
+      expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(3);
+
+      // Verify batch sizes
+      expect(fetchNotificationDetailsForListSpy.mock.calls[0][0]).toHaveLength(
+        Constants.GITHUB_API_MERGE_BATCH_SIZE,
+      );
+      expect(fetchNotificationDetailsForListSpy.mock.calls[1][0]).toHaveLength(
+        Constants.GITHUB_API_MERGE_BATCH_SIZE,
+      );
+      expect(fetchNotificationDetailsForListSpy.mock.calls[2][0]).toHaveLength(
+        50,
+      );
+    });
+
+    it('should handle single batch of notifications', async () => {
+      const fetchNotificationDetailsForListSpy = jest
+        .spyOn(apiClient, 'fetchNotificationDetailsForList')
+        .mockResolvedValue(new Map());
+
+      const notifications = Array.from({ length: 50 }, (_, i) =>
+        mockPartialGitifyNotification({
+          title: `Notification ${i}`,
+          type: 'Issue',
+          url: `https://api.github.com/repos/gitify-app/notifications-test/issues/${i}` as Link,
+        }),
+      ) as GitifyNotification[];
+
+      const settings: SettingsState = {
+        ...mockSettings,
+        detailedNotifications: true,
+      };
+
+      await enrichNotifications(notifications, settings);
+
+      // Should be called once for single batch
+      expect(fetchNotificationDetailsForListSpy).toHaveBeenCalledTimes(1);
+      expect(fetchNotificationDetailsForListSpy.mock.calls[0][0]).toHaveLength(
+        50,
+      );
+    });
   });
 });

src/renderer/utils/notifications/notifications.ts

@@ -1,3 +1,5 @@
+import { Constants } from '../../constants';
+
 import type {
   AccountNotifications,
   GitifyNotification,
@@ -141,6 +143,13 @@ export async function getAllNotifications(
   return accountNotifications;
 }
 
+/**
+ * Enrich notification details
+ *
+ * @param notifications All Gitify inbox notifications
+ * @param settings
+ * @returns
+ */
 export async function enrichNotifications(
   notifications: GitifyNotification[],
   settings: SettingsState,
@@ -149,20 +158,7 @@ export async function enrichNotifications(
     return notifications;
   }
 
-  // Build and fetch merged details via client; returns per-notification results
-  let mergedResults: Map<
-    GitifyNotification,
-    FetchMergedDetailsTemplateQuery['repository']
-  > = new Map();
-  try {
-    mergedResults = await fetchNotificationDetailsForList(notifications);
-  } catch (err) {
-    rendererLogError(
-      'enrichNotifications',
-      'Failed to fetch merged notification details',
-      err,
-    );
-  }
+  const mergedResults = await fetchNotificationDetailsInBatches(notifications);
 
   const enrichedNotifications = await Promise.all(
     notifications.map(async (notification: GitifyNotification) => {
@@ -174,6 +170,54 @@ export async function enrichNotifications(
   return enrichedNotifications;
 }
 
+/**
+ * Fetch notification details in batches to avoid overwhelming the API.
+ *
+ * @param notifications - The notifications to fetch details for.
+ * @returns A map of notifications to their repository details.
+ */
+async function fetchNotificationDetailsInBatches(
+  notifications: GitifyNotification[],
+): Promise<
+  Map<GitifyNotification, FetchMergedDetailsTemplateQuery['repository']>
+> {
+  const mergedResults: Map<
+    GitifyNotification,
+    FetchMergedDetailsTemplateQuery['repository']
+  > = new Map();
+
+  const batchSize = Constants.GITHUB_API_MERGE_BATCH_SIZE;
+
+  for (
+    let batchStart = 0;
+    batchStart < notifications.length;
+    batchStart += batchSize
+  ) {
+    const batchIndex = Math.floor(batchStart / batchSize) + 1;
+    const batchNotifications = notifications.slice(
+      batchStart,
+      batchStart + batchSize,
+    );
+
+    try {
+      const batchResults =
+        await fetchNotificationDetailsForList(batchNotifications);
+
+      for (const [notification, repository] of batchResults) {
+        mergedResults.set(notification, repository);
+      }
+    } catch (err) {
+      rendererLogError(
+        'fetchNotificationDetailsInBatches',
+        `Failed to fetch merged notification details for batch ${batchIndex}`,
+        err,
+      );
+    }
+  }
+
+  return mergedResults;
+}
+
 /**
  * Enrich a notification with additional details.
  *