From fadfb4ecaef2fad1ab2d9925664aa50755e4eb3b Mon Sep 17 00:00:00 2001 From: jordan-ae Date: Wed, 6 Nov 2024 18:28:01 +0100 Subject: [PATCH 1/2] fix: use paginate and use correct delay tolerance --- src/handlers/shared/start.ts | 7 +++-- src/utils/issue.ts | 61 ++++++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/handlers/shared/start.ts b/src/handlers/shared/start.ts index 5c54157e..2a88dddc 100644 --- a/src/handlers/shared/start.ts +++ b/src/handlers/shared/start.ts @@ -156,15 +156,16 @@ async function fetchUserIds(context: Context, username: string[]) { } async function handleTaskLimitChecks(username: string, context: Context, logger: Context["logger"], sender: string) { - const openedPullRequests = await getAvailableOpenedPullRequests(context, username); + const { approved, changes } = await getAvailableOpenedPullRequests(context, username); const assignedIssues = await getAssignedIssues(context, username); const { limit } = await getUserRoleAndTaskLimit(context, username); + const adjustedAssignedIssues = assignedIssues.length - approved.length + changes.length; // check for max and enforce max - if (Math.abs(assignedIssues.length - openedPullRequests.length) >= limit) { + if (Math.abs(adjustedAssignedIssues) >= limit) { logger.error(username === sender ? "You have reached your max task limit" : `${username} has reached their max task limit`, { assignedIssues: assignedIssues.length, - openedPullRequests: openedPullRequests.length, + openedPullRequests: adjustedAssignedIssues, limit, }); diff --git a/src/utils/issue.ts b/src/utils/issue.ts index 4c008979..0e81ef23 100644 --- a/src/utils/issue.ts +++ b/src/utils/issue.ts @@ -176,7 +176,7 @@ export async function addAssignees(context: Context, issueNo: number, assignees: 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}`, + q: `org:${payload.repository.owner.login} author:${username} state:${state} is:pr`, per_page: 100, order: "desc", sort: "created", @@ -224,6 +224,24 @@ export async function getAllPullRequestReviews(context: Context, pullNumber: num } } +async function getReviewRequestsTimeline(context: Context, pullNumber: number, owner: string, repo: string) { + try { + const events = (await context.octokit.paginate(`GET /repos/${owner}/${repo}/issues/${pullNumber}/timeline`, { + owner, + repo, + issue_number: pullNumber, + })) as { + created_at: string | number | Date; + event: string; + }[]; + + return events.filter((event: { event: string }) => event.event === "review_requested" || event.event === "review_request_removed"); + } catch (error) { + console.error("Error fetching review request timeline events:", error); + return []; + } +} + export function getOwnerRepoFromHtmlUrl(url: string) { const parts = url.split("/"); if (parts.length < 5) { @@ -237,10 +255,11 @@ export function getOwnerRepoFromHtmlUrl(url: string) { export async function getAvailableOpenedPullRequests(context: Context, username: string) { const { reviewDelayTolerance } = context.config; - if (!reviewDelayTolerance) return []; + if (!reviewDelayTolerance) return { approved: [], changes: [] }; const openedPullRequests = await getOpenedPullRequestsForUser(context, username); - const result: (typeof openedPullRequests)[number][] = []; + const approved = [] as unknown[]; + const changes = [] as unknown[]; for (let i = 0; openedPullRequests && i < openedPullRequests.length; i++) { const openedPullRequest = openedPullRequests[i]; @@ -248,18 +267,42 @@ export async function getAvailableOpenedPullRequests(context: Context, username: const { owner, repo } = getOwnerRepoFromHtmlUrl(openedPullRequest.html_url); const reviews = await getAllPullRequestReviews(context, openedPullRequest.number, owner, repo); - if (reviews.length > 0) { - const approvedReviews = reviews.find((review) => review.state === "APPROVED"); - if (approvedReviews) { - result.push(openedPullRequest); + // Determine the latest review state + const latestReview = reviews[reviews.length - 1]; + const latestReviewState = latestReview?.state; + + if (latestReviewState === "APPROVED") { + approved.push(openedPullRequest); + continue; + } + + if (latestReviewState === "CHANGES_REQUESTED") { + changes.push(openedPullRequest); + + // Track the time of the last "CHANGES_REQUESTED"s + const lastChangesRequestedTime = latestReview.submitted_at ? new Date(latestReview.submitted_at).getTime() : null; + + // Fetch timeline or comments to check if reviewer has been re-requested + const reviewRequests = await getReviewRequestsTimeline(context, openedPullRequest.number, owner, repo); + + // Find if any review request was made after the last changes requested + const isReviewRequestedAfterChanges = lastChangesRequestedTime + ? reviewRequests.some((request) => new Date(request.created_at).getTime() > lastChangesRequestedTime) + : false; + + if (isReviewRequestedAfterChanges) { + approved.push(openedPullRequest); + changes.pop(); + continue; } } if (reviews.length === 0 && new Date().getTime() - new Date(openedPullRequest.created_at).getTime() >= getTimeValue(reviewDelayTolerance)) { - result.push(openedPullRequest); + approved.push(openedPullRequest); } } - return result; + + return { approved, changes }; } export function getTimeValue(timeString: string): number { From 4ee96e9974345f65ba2e7d3221133b77455b824f Mon Sep 17 00:00:00 2001 From: jordan-ae Date: Thu, 7 Nov 2024 18:18:10 +0100 Subject: [PATCH 2/2] feat: check if user has marked all conversation as resolved --- src/types/payload.ts | 12 ++++++++++ src/utils/issue.ts | 54 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/types/payload.ts b/src/types/payload.ts index c6992bbe..3f166696 100644 --- a/src/types/payload.ts +++ b/src/types/payload.ts @@ -8,6 +8,18 @@ export type TimelineEvents = RestEndpointMethodTypes["issues"]["listEventsForTim export type Assignee = Issue["assignee"]; export type GitHubIssueSearch = RestEndpointMethodTypes["search"]["issuesAndPullRequests"]["response"]["data"]; +export type PullRequestConversationsResponse = { + repository: { + pullRequest: { + reviewThreads: { + nodes: { + isResolved: boolean; + }[]; + }; + }; + }; +}; + export type Sender = { login: string; id: number }; export const ISSUE_TYPE = { diff --git a/src/utils/issue.ts b/src/utils/issue.ts index 0e81ef23..dd0636f6 100644 --- a/src/utils/issue.ts +++ b/src/utils/issue.ts @@ -2,7 +2,7 @@ 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 { GitHubIssueSearch, PullRequestConversationsResponse, Review } from "../types/payload"; import { getLinkedPullRequests, GetLinkedResults } from "./get-linked-prs"; import { getAllPullRequestsFallback, getAssignedIssuesFallback } from "./get-pull-requests-fallback"; @@ -226,22 +226,51 @@ export async function getAllPullRequestReviews(context: Context, pullNumber: num async function getReviewRequestsTimeline(context: Context, pullNumber: number, owner: string, repo: string) { try { - const events = (await context.octokit.paginate(`GET /repos/${owner}/${repo}/issues/${pullNumber}/timeline`, { + const events = await context.octokit.paginate(context.octokit.rest.issues.listEventsForTimeline, { owner, repo, issue_number: pullNumber, - })) as { - created_at: string | number | Date; - event: string; - }[]; - - return events.filter((event: { event: string }) => event.event === "review_requested" || event.event === "review_request_removed"); + per_page: 100, + }); + return events + .filter( + ( + event + ): event is { + created_at: string | number | Date; + event: string; + } => typeof event.event === "string" + ) + .filter((event) => event.event === "review_requested" || event.event === "review_request_removed"); } catch (error) { console.error("Error fetching review request timeline events:", error); return []; } } +async function getPullRequestConversations(context: Context, owner: string, repo: string, pullNumber: number) { + const query = ` + query($owner: String!, $repo: String!, $pullNumber: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pullNumber) { + reviewThreads(first: 100) { + nodes { + isResolved + } + } + } + } + } + `; + + const params = { owner, repo, pullNumber }; + const result: PullRequestConversationsResponse = await context.octokit.graphql(query, { + ...params, + }); + + return result.repository.pullRequest.reviewThreads.nodes; +} + export function getOwnerRepoFromHtmlUrl(url: string) { const parts = url.split("/"); if (parts.length < 5) { @@ -271,11 +300,20 @@ export async function getAvailableOpenedPullRequests(context: Context, username: const latestReview = reviews[reviews.length - 1]; const latestReviewState = latestReview?.state; + // Check if all conversations have been reviewed + const conversations = await getPullRequestConversations(context, owner, repo, openedPullRequest.number); + const isResolved = conversations.every((thread: { isResolved: unknown }) => thread.isResolved); + if (latestReviewState === "APPROVED") { approved.push(openedPullRequest); continue; } + if (isResolved) { + approved.push(openedPullRequest); + continue; + } + if (latestReviewState === "CHANGES_REQUESTED") { changes.push(openedPullRequest);