Merge pull request #71 from gentlementlegen/fix/gql

fix: pull-request fetch for private user profiles

.gitignore

@@ -15,4 +15,5 @@ junit.xml
 cypress/screenshots
 script.ts
 .wrangler
-test-dashboard.md
+test-dashboard.md
+*.env.json

package.json

@@ -17,7 +17,7 @@
     "knip-ci": "knip --no-exit-code --reporter json --config .github/knip.ts",
     "prepare": "husky install",
     "test": "jest --setupFiles dotenv/config --coverage",
-    "worker": "wrangler dev --env dev --port 4001"
+    "worker": "wrangler dev --env dev --port 4000"
   },
   "keywords": [
     "typescript",
@@ -30,6 +30,7 @@
     "@octokit/graphql-schema": "15.25.0",
     "@octokit/plugin-paginate-graphql": "5.2.2",
     "@octokit/rest": "20.1.1",
+    "@octokit/types": "^13.6.1",
     "@octokit/webhooks": "13.2.7",
     "@sinclair/typebox": "^0.32.5",
     "@supabase/supabase-js": "2.42.0",

src/plugin.ts

@@ -31,9 +31,9 @@ export async function startStopTask(inputs: PluginInputs, env: Env) {
       case "issues.assigned":
         return await userSelfAssign(context as Context<"issues.assigned">);
       case "pull_request.opened":
-        return await userPullRequest(context as Context<"pull_request.edited">);
-      case "pull_request.edited":
         return await userPullRequest(context as Context<"pull_request.opened">);
+      case "pull_request.edited":
+        return await userPullRequest(context as Context<"pull_request.edited">);
       case "issues.unassigned":
         return await userUnassigned(context);
       default:

src/utils/get-pull-requests-fallback.ts

@@ -0,0 +1,91 @@
+import { RestEndpointMethodTypes } from "@octokit/rest";
+import type { Endpoints } from "@octokit/types";
+import { Context } from "../types";
+
+function isHttpError(error: unknown): error is { status: number; message: string } {
+  return typeof error === "object" && error !== null && "status" in error && "message" in error;
+}
+
+/**
+ * Fetches all open pull requests within a specified organization created by a particular user.
+ * This method is slower than using a search query, but should work even if the user has his activity set to private.
+ */
+export async function getAllPullRequestsFallback(
+  context: Context,
+  state: Endpoints["GET /repos/{owner}/{repo}/pulls"]["parameters"]["state"],
+  username: string
+) {
+  const { octokit, logger } = context;
+  const organization = context.payload.repository.owner.login;
+
+  try {
+    const repositories = await octokit.paginate(octokit.rest.repos.listForOrg, {
+      org: organization,
+      per_page: 100,
+      type: "all",
+    });
+
+    const allPrs: RestEndpointMethodTypes["pulls"]["list"]["response"]["data"] = [];
+
+    const tasks = repositories.map(async (repo) => {
+      try {
+        const prs = await octokit.paginate(octokit.rest.pulls.list, {
+          owner: organization,
+          repo: repo.name,
+          state,
+          per_page: 100,
+        });
+        const userPrs = prs.filter((pr) => pr.user?.login === username);
+        allPrs.push(...userPrs);
+      } catch (error) {
+        if (isHttpError(error) && (error.status === 404 || error.status === 403)) {
+          logger.error(`Could not find pull requests for repository ${repo.url}, skipping: ${error}`);
+          return;
+        }
+        logger.fatal("Failed to fetch pull requests for repository", { error: error as Error });
+        throw error;
+      }
+    });
+
+    await Promise.all(tasks);
+
+    return allPrs;
+  } catch (error) {
+    logger.fatal("Failed to fetch pull requests for organization", { error: error as Error });
+    throw error;
+  }
+}
+
+export async function getAssignedIssuesFallback(context: Context, username: string) {
+  const org = context.payload.repository.owner.login;
+  const assignedIssues = [];
+
+  try {
+    const repositories = await context.octokit.paginate(context.octokit.rest.repos.listForOrg, {
+      org,
+      type: "all",
+      per_page: 100,
+    });
+
+    for (const repo of repositories) {
+      const issues = await context.octokit.paginate(context.octokit.rest.issues.listForRepo, {
+        owner: org,
+        repo: repo.name,
+        assignee: username,
+        state: "open",
+        per_page: 100,
+      });
+
+      assignedIssues.push(
+        ...issues.filter(
+          (issue) =>
+            issue.pull_request === undefined && (issue.assignee?.login === username || issue.assignees?.some((assignee) => assignee.login === username))
+        )
+      );
+    }
+
+    return assignedIssues;
+  } catch (err: unknown) {
+    throw new Error(context.logger.error("Fetching assigned issues failed!", { error: err as Error }).logMessage.raw);
+  }
+}

src/utils/issue.ts

@@ -1,20 +1,22 @@
 import { RestEndpointMethodTypes } from "@octokit/rest";
+import { Endpoints } from "@octokit/types";
 import ms from "ms";
 import { Context } from "../types/context";
 import { GitHubIssueSearch, Review } from "../types/payload";
 import { getLinkedPullRequests, GetLinkedResults } from "./get-linked-prs";
+import { getAllPullRequestsFallback, getAssignedIssuesFallback } from "./get-pull-requests-fallback";
 
 export function isParentIssue(body: string) {
   const parentPattern = /-\s+\[( |x)\]\s+#\d+/;
   return body.match(parentPattern);
 }
 
-export async function getAssignedIssues(context: Context, username: string): Promise<GitHubIssueSearch["items"]> {
+export async function getAssignedIssues(context: Context, username: string) {
   const payload = context.payload;
 
   try {
     return await context.octokit
-      .paginate(context.octokit.search.issuesAndPullRequests, {
+      .paginate(context.octokit.rest.search.issuesAndPullRequests, {
         q: `org:${payload.repository.owner.login} assignee:${username} is:open is:issue`,
         per_page: 100,
         order: "desc",
@@ -25,8 +27,9 @@ export async function getAssignedIssues(context: Context, username: string): Pro
           return issue.state === "open" && (issue.assignee?.login === username || issue.assignees?.some((assignee) => assignee.login === username));
         })
       );
-  } catch (err: unknown) {
-    throw new Error(context.logger.error("Fetching assigned issues failed!", { error: err as Error }).logMessage.raw);
+  } catch (err) {
+    context.logger.info("Will try re-fetching assigned issues...", { error: err as Error });
+    return getAssignedIssuesFallback(context, username);
   }
 }
 
@@ -170,7 +173,7 @@ export async function addAssignees(context: Context, issueNo: number, assignees:
   await confirmMultiAssignment(context, issueNo, assignees);
 }
 
-export async function getAllPullRequests(context: Context, state: "open" | "closed" | "all" = "open", username: string) {
+async function getAllPullRequests(context: Context, state: Endpoints["GET /repos/{owner}/{repo}/pulls"]["parameters"]["state"] = "open", username: string) {
   const { payload } = context;
   const query: RestEndpointMethodTypes["search"]["issuesAndPullRequests"]["parameters"] = {
     q: `org:${payload.repository.owner.login} author:${username} state:${state}`,
@@ -180,19 +183,32 @@ export async function getAllPullRequests(context: Context, state: "open" | "clos
   };
 
   try {
-    return (await context.octokit.paginate(context.octokit.search.issuesAndPullRequests, query)) as GitHubIssueSearch["items"];
+    return (await context.octokit.paginate(context.octokit.rest.search.issuesAndPullRequests, query)) as GitHubIssueSearch["items"];
   } catch (err: unknown) {
     throw new Error(context.logger.error("Fetching all pull requests failed!", { error: err as Error, query }).logMessage.raw);
   }
 }
 
+export async function getAllPullRequestsWithRetry(
+  context: Context,
+  state: Endpoints["GET /repos/{owner}/{repo}/pulls"]["parameters"]["state"],
+  username: string
+) {
+  try {
+    return await getAllPullRequests(context, state, username);
+  } catch (error) {
+    context.logger.info("Will retry re-fetching all pull requests...", { error: error as Error });
+    return getAllPullRequestsFallback(context, state, username);
+  }
+}
+
 export async function getAllPullRequestReviews(context: Context, pullNumber: number, owner: string, repo: string) {
   const {
     config: { rolesWithReviewAuthority },
   } = context;
   try {
     return (
-      await context.octokit.paginate(context.octokit.pulls.listReviews, {
+      await context.octokit.paginate(context.octokit.rest.pulls.listReviews, {
         owner,
         repo,
         pull_number: pullNumber,
@@ -219,11 +235,12 @@ export async function getAvailableOpenedPullRequests(context: Context, username:
   const { reviewDelayTolerance } = context.config;
   if (!reviewDelayTolerance) return [];
 
-  const openedPullRequests = await getOpenedPullRequests(context, username);
-  const result = [] as typeof openedPullRequests;
+  const openedPullRequests = await getOpenedPullRequestsForUser(context, username);
+  const result: (typeof openedPullRequests)[number][] = [];
 
-  for (let i = 0; i < openedPullRequests.length; i++) {
+  for (let i = 0; openedPullRequests && i < openedPullRequests.length; i++) {
     const openedPullRequest = openedPullRequests[i];
+    if (!openedPullRequest) continue;
     const { owner, repo } = getOwnerRepoFromHtmlUrl(openedPullRequest.html_url);
     const reviews = await getAllPullRequestReviews(context, openedPullRequest.number, owner, repo);
 
@@ -251,9 +268,8 @@ export function getTimeValue(timeString: string): number {
   return timeValue;
 }
 
-async function getOpenedPullRequests(context: Context, username: string): Promise<ReturnType<typeof getAllPullRequests>> {
-  const prs = await getAllPullRequests(context, "open", username);
-  return prs.filter((pr) => pr.pull_request && pr.state === "open");
+async function getOpenedPullRequestsForUser(context: Context, username: string): Promise<ReturnType<typeof getAllPullRequestsWithRetry>> {
+  return getAllPullRequestsWithRetry(context, "open", username);
 }
 
 /**

tests/fallbacks.test.ts

@@ -0,0 +1,48 @@
+import { RestEndpointMethodTypes } from "@octokit/rest";
+import { Logs } from "@ubiquity-os/ubiquity-os-logger";
+import { Context } from "../src/types/context";
+import { getAllPullRequestsWithRetry } from "../src/utils/issue";
+
+const username = "private-user";
+
+const mockPullRequestData = [
+  { id: 1, number: 123, state: "open", user: { login: username } },
+  { id: 2, number: 124, state: "open", user: { login: "public-user" } },
+] as unknown as RestEndpointMethodTypes["pulls"]["list"]["response"]["data"];
+
+const mockOctokit = {
+  paginate: jest.fn().mockResolvedValue(mockPullRequestData),
+  rest: {
+    pulls: {
+      list: jest.fn().mockResolvedValue(mockPullRequestData),
+    },
+    repos: {
+      listForOrg: jest.fn().mockResolvedValue(mockPullRequestData),
+    },
+  },
+};
+
+const context: Context = {
+  eventName: "pull_request",
+  payload: {
+    repository: {
+      name: "test-repo",
+      owner: {
+        login: "test-owner",
+      },
+    },
+  },
+  octokit: mockOctokit as unknown as Context["octokit"],
+  logger: new Logs("debug"),
+  adapters: {},
+} as unknown as Context;
+
+describe("getAllPullRequestsWithRetry", () => {
+  it("should return pull requests even if user information is private", async () => {
+    const pullRequests = await getAllPullRequestsWithRetry(context, "all", username);
+    expect(pullRequests).toHaveLength(2);
+    expect(pullRequests[0].user?.login).toBe(username);
+    expect(pullRequests[1].user?.login).toBe(username);
+    console.log(pullRequests);
+  });
+});