From b4a6c34853f487405c655096c8b4f7e38d8fb141 Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Wed, 25 Feb 2026 20:52:03 +0200 Subject: [PATCH] fix: use per-review comment fetching for Forgejo compatibility Forgejo doesn't have the flat GET /pulls/{number}/comments endpoint (returns 404). Replace pulls.listReviewComments() with a 2-phase approach: fetch reviews first, then pulls.listCommentsForReview() per-review in parallel. Updates tests to mock the new per-review API. Co-Authored-By: Claude Opus 4.6 --- src/forgejo/data/fetcher.ts | 36 +++++++++++++++++++++--------- test/forgejo-data-fetcher.test.ts | 37 +++++++++++++++++++------------ 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/forgejo/data/fetcher.ts b/src/forgejo/data/fetcher.ts index 2fd7bc8..4169aa7 100644 --- a/src/forgejo/data/fetcher.ts +++ b/src/forgejo/data/fetcher.ts @@ -210,14 +210,16 @@ export async function fetchForgejoData({ let rawReviews: GitHubReview[] = []; if (isPR) { - // Fetch PR details, files, commits, comments, reviews, and inline comments in parallel + // Phase 1: Fetch PR details, files, commits, comments, and reviews in parallel. + // Note: Forgejo doesn't have a flat /pulls/{number}/comments endpoint for + // review comments (returns 404). Instead, fetch review comments per-review + // in phase 2 after we have the review IDs. const [ prResponse, filesResponse, commitsResponse, commentsResponse, reviewsResponse, - reviewCommentsResponse, ] = await Promise.all([ octokits.rest.pulls.get({ owner, repo, pull_number: number }), octokits.rest.pulls.listFiles({ @@ -244,23 +246,35 @@ export async function fetchForgejoData({ pull_number: number, per_page: 100, }), - octokits.rest.pulls.listReviewComments({ - owner, - repo, - pull_number: number, - per_page: 100, - }), ]); const pr = prResponse.data; const files = filesResponse.data; const commits = commitsResponse.data; + // Phase 2: Fetch inline comments for each review in parallel. + // Forgejo uses GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments + const reviewCommentArrays = await Promise.all( + reviewsResponse.data.map(async (review) => { + try { + const { data } = await octokits.rest.pulls.listCommentsForReview({ + owner, + repo, + pull_number: number, + review_id: review.id, + }); + return data; + } catch { + // Some reviews (e.g. pending) may not have accessible comments + return []; + } + }), + ); + const allReviewCommentData = reviewCommentArrays.flat(); + // Map REST responses to internal types rawComments = mapRestComments(commentsResponse.data); - const mappedInlineComments = mapRestReviewComments( - reviewCommentsResponse.data, - ); + const mappedInlineComments = mapRestReviewComments(allReviewCommentData); rawReviews = mapRestReviews(reviewsResponse.data, mappedInlineComments); // Map REST file objects to GitHubFile type diff --git a/test/forgejo-data-fetcher.test.ts b/test/forgejo-data-fetcher.test.ts index f20cb20..aeeb494 100644 --- a/test/forgejo-data-fetcher.test.ts +++ b/test/forgejo-data-fetcher.test.ts @@ -29,11 +29,25 @@ function createMockOctokits({ filesData?: Record[]; commitsData?: Record[]; commentsData?: Record[]; - reviewsData?: Record[]; - reviewCommentsData?: Record[]; + reviewsData?: (Record & { id?: number })[]; + reviewCommentsData?: (Record & { + pull_request_review_id?: number | null; + })[]; issueData?: Record; userData?: Record; } = {}) { + // Build per-review mock: listCommentsForReview returns comments matching + // the requested review_id (Forgejo doesn't have a flat review-comments endpoint). + const allReviewComments = reviewCommentsData ?? []; + const listCommentsForReviewMock = jest + .fn() + .mockImplementation((params: { review_id: number }) => { + const matching = allReviewComments.filter( + (c) => c.pull_request_review_id === params.review_id, + ); + return Promise.resolve({ data: matching }); + }); + return { rest: { pulls: { @@ -41,9 +55,7 @@ function createMockOctokits({ listFiles: jest.fn().mockResolvedValue({ data: filesData ?? [] }), listCommits: jest.fn().mockResolvedValue({ data: commitsData ?? [] }), listReviews: jest.fn().mockResolvedValue({ data: reviewsData ?? [] }), - listReviewComments: jest - .fn() - .mockResolvedValue({ data: reviewCommentsData ?? [] }), + listCommentsForReview: listCommentsForReviewMock, }, issues: { get: jest.fn().mockResolvedValue({ data: issueData ?? {} }), @@ -114,7 +126,7 @@ const mockIssueRestResponse = { describe("fetchForgejoData", () => { describe("PR data fetching", () => { - it("should fetch PR details, files, commits, comments, reviews, and inline comments in parallel", async () => { + it("should fetch PR details, files, commits, comments, and reviews in parallel then per-review comments", async () => { const mockOctokits = createMockOctokits({ prData: mockPrRestResponse, filesData: mockFilesRestResponse, @@ -128,7 +140,7 @@ describe("fetchForgejoData", () => { isPR: true, }); - // Verify all six REST calls were made + // Verify phase 1 REST calls (parallel) expect(mockOctokits.rest.pulls.get).toHaveBeenCalledWith({ owner: "test-owner", repo: "test-repo", @@ -158,12 +170,7 @@ describe("fetchForgejoData", () => { pull_number: 42, per_page: 100, }); - expect(mockOctokits.rest.pulls.listReviewComments).toHaveBeenCalledWith({ - owner: "test-owner", - repo: "test-repo", - pull_number: 42, - per_page: 100, - }); + // Phase 2: per-review comment fetching (no reviews in this test → no calls) }); it("should map PR fields to GitHubPullRequest type", async () => { @@ -412,7 +419,9 @@ describe("fetchForgejoData", () => { expect(mockOctokits.rest.pulls.listFiles).not.toHaveBeenCalled(); expect(mockOctokits.rest.pulls.listCommits).not.toHaveBeenCalled(); expect(mockOctokits.rest.pulls.listReviews).not.toHaveBeenCalled(); - expect(mockOctokits.rest.pulls.listReviewComments).not.toHaveBeenCalled(); + expect( + mockOctokits.rest.pulls.listCommentsForReview, + ).not.toHaveBeenCalled(); }); it("should map issue fields to GitHubIssue type", async () => {