fix: Remove direct GitHub API calls from frontend (#40)

* fix: Remove direct GitHub API calls from frontend The frontend was making direct calls to the GitHub API to check PR status, which was causing 403 errors. This change removes those direct API calls since the PR status is already included in the cached data that's built by the backend. * test: Update tests to use cached PR status The tests were expecting the frontend to fetch PR status directly from GitHub API, but now that we've moved that logic to the backend, we need to update the tests to use the PR status from the cache instead. * fix: Remove unused IssueActivityStatus import * trigger: vercel deployment --------- Co-authored-by: openhands <[email protected]>
All-Hands-AI · Dec 10, 2024 · 3eeaecb · 3eeaecb
1 parent aef964d
commit 3eeaecb
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 59 deletions.
diff --git a/src/services/github.test.ts b/src/services/github.test.ts
@@ -39,19 +39,14 @@ describe('GitHub Service', () => {
           activities: [{
             id: 'issue-1-2',
             type: 'issue',
-            status: 'success',
+            status: 'pr_open',
             timestamp: '2023-11-28T00:01:00Z',
             url: 'https://github.com/All-Hands-AI/OpenHands/issues/1#comment-2',
             prUrl: 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2',
             description: 'A potential fix has been generated and a draft PR #2 has been created. Please review the changes.'
           }],
           lastUpdated: '2023-11-28T00:01:00Z'
         });
-      } else if (url === 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2') {
-        return createMockResponse({
-          state: 'open',
-          merged: false
-        });
       }
       throw new Error(`Unexpected URL: ${url}`);
     });
@@ -78,7 +73,7 @@ describe('GitHub Service', () => {
           activities: [{
             id: 'issue-1-2',
             type: 'issue',
-            status: 'failure',
+            status: 'no_pr',
             timestamp: '2023-11-28T00:01:00Z',
             url: 'https://github.com/All-Hands-AI/OpenHands/issues/1#comment-2',
             description: 'The workflow to fix this issue encountered an error. Openhands failed to create any code changes.'
@@ -143,7 +138,7 @@ describe('GitHub Service', () => {
           activities: [{
             id: 'issue-1-2',
             type: 'issue',
-            status: 'success',
+            status: 'no_pr',
             timestamp: '2023-11-28T00:01:00Z',
             url: 'https://github.com/All-Hands-AI/OpenHands/issues/1#comment-2',
             description: 'Working on the issue...'
@@ -173,19 +168,14 @@ describe('GitHub Service', () => {
           activities: [{
             id: 'issue-1-2',
             type: 'issue',
-            status: 'success',
+            status: 'pr_open',
             timestamp: '2023-11-28T00:01:00Z',
             url: 'https://github.com/All-Hands-AI/OpenHands/issues/1#comment-2',
             prUrl: 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2',
             description: 'Created PR #2'
           }],
           lastUpdated: '2023-11-28T00:01:00Z'
         });
-      } else if (url === 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2') {
-        return createMockResponse({
-          state: 'open',
-          merged: false
-        });
       }
       throw new Error(`Unexpected URL: ${url}`);
     });
@@ -209,19 +199,14 @@ describe('GitHub Service', () => {
           activities: [{
             id: 'issue-1-2',
             type: 'issue',
-            status: 'success',
+            status: 'pr_merged',
             timestamp: '2023-11-28T00:01:00Z',
             url: 'https://github.com/All-Hands-AI/OpenHands/issues/1#comment-2',
             prUrl: 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2',
             description: 'Created PR #2'
           }],
           lastUpdated: '2023-11-28T00:01:00Z'
         });
-      } else if (url === 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2') {
-        return createMockResponse({
-          state: 'closed',
-          merged: true
-        });
       }
       throw new Error(`Unexpected URL: ${url}`);
     });
@@ -245,19 +230,14 @@ describe('GitHub Service', () => {
           activities: [{
             id: 'issue-1-2',
             type: 'issue',
-            status: 'success',
+            status: 'pr_closed',
             timestamp: '2023-11-28T00:01:00Z',
             url: 'https://github.com/All-Hands-AI/OpenHands/issues/1#comment-2',
             prUrl: 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2',
             description: 'Created PR #2'
           }],
           lastUpdated: '2023-11-28T00:01:00Z'
         });
-      } else if (url === 'https://api.github.com/repos/All-Hands-AI/OpenHands/pulls/2') {
-        return createMockResponse({
-          state: 'closed',
-          merged: false
-        });
       }
       throw new Error(`Unexpected URL: ${url}`);
     });

diff --git a/src/services/github.ts b/src/services/github.ts
@@ -1,25 +1,6 @@
-import { BotActivity, IssueActivityStatus } from '../types';
+import { BotActivity } from '../types';
 
-async function checkPRStatus(prUrl: string): Promise<IssueActivityStatus> {
-  try {
-    const response = await fetch(prUrl);
-    if (!response.ok) {
-      return 'no_pr';
-    }
-    const pr = await response.json();
-
-    if (pr.merged) {
-      return 'pr_merged';
-    } else if (pr.state === 'closed') {
-      return 'pr_closed';
-    } else {
-      return 'pr_open';
-    }
-  } catch (error) {
-    console.error('Error checking PR status:', error);
-    return 'no_pr';
-  }
-}
+// PR status is now included in the cached data, no need to fetch it
 
 export async function fetchBotActivities(since?: string): Promise<BotActivity[]> {
   try {
@@ -39,18 +20,8 @@ export async function fetchBotActivities(since?: string): Promise<BotActivity[]>
       );
     }
 
-    // Process issue activities to determine PR status
-    const processedActivities = await Promise.all(activities.map(async activity => {
-      if (activity.type === 'issue') {
-        if (activity.prUrl) {
-          const prStatus = await checkPRStatus(activity.prUrl);
-          return { ...activity, status: prStatus };
-        } else {
-          return { ...activity, status: 'no_pr' as IssueActivityStatus };
-        }
-      }
-      return activity;
-    }));
+    // PR status is already included in the cached data
+    const processedActivities = activities;
 
     // Sort by timestamp in descending order
     return processedActivities.sort((a, b) =>