Skip to content

Commit

Permalink
GitHub: use files API with pagination
Browse files Browse the repository at this point in the history
  • Loading branch information
fabianehlert committed Apr 26, 2024
1 parent faa3d5d commit bf158b8
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<!-- Your comment below this -->

- Adds a log when you run on GitHub Actions without being a pull_request - [@orta]
- GitHub: Move to 'List pull request files' API with pagination support [@fabianehlert]

<!-- Your comment above this -->

Expand Down
63 changes: 58 additions & 5 deletions source/platforms/github/GitHubAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ export type APIToken = string

const limit = pLimit(25)

// Structure of files returned by the 'List pull request files' API
export interface GitHubFile {
filename: string
patch: string
}

// Note that there are parts of this class which don't seem to be
// used by Danger, they are exposed for Peril support.

Expand Down Expand Up @@ -329,13 +335,60 @@ export class GitHubAPI {
})
}

getPullRequestDiff = async () => {
getPullRequestFiles = async (page: number = 1): Promise<GitHubFile[]> => {
const repo = this.repoMetadata.repoSlug
const prID = this.repoMetadata.pullRequestID
const res = await this.get(`repos/${repo}/pulls/${prID}`, {
Accept: "application/vnd.github.v3.diff",
})
return res.ok ? res.text() : ""
const perPage = 100
const url = `repos/${repo}/pulls/${prID}/files?page=${page}&per_page=${perPage}`

try {
const response = await this.get(url, {
Accept: "application/vnd.github.v3.diff",
})
const data = await response.json()

if (!response.ok) {
throw new Error(`GitHub 'List pull request files' API returned an error: ${data.message}`)
}

// Check for pagination using the Link header
const linkHeader = response.headers.get("Link")
const hasNextPage = linkHeader && linkHeader.includes('rel="next"')

// If there's a next page, recursively fetch it and concatenate the results
if (hasNextPage) {
const nextPageNumber = page + 1
const nextPageFiles = await this.getPullRequestFiles(nextPageNumber)
return [...data, ...nextPageFiles]
}

return data as GitHubFile[]
} catch (error) {
console.error("Failed to fetch GitHub pull request files:", error)
throw error
}
}

getPullRequestDiff = async (): Promise<string> => {
// This is a hack to get the file patch into a format that parse-diff accepts
// as the GitHub API for listing pull request files is missing file names in the patch.
function prefixedPatch(file: GitHubFile): string {
return `
diff --git a/${file.filename} b/${file.filename}
--- a/${file.filename}
+++ b/${file.filename}
${file.patch}
`
}

try {
const files = await this.getPullRequestFiles()
const diff = files.map(prefixedPatch).join("\n")
return diff
} catch (error) {
console.error("Failed to fetch pull request diff:", error)
return ""
}
}

getFileContents = async (path: string, repoSlug: string, ref: string): Promise<any> => {
Expand Down
57 changes: 34 additions & 23 deletions source/platforms/github/_tests/_github_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { FakeCI } from "../../../ci_source/providers/Fake"
import { GitHubAPI } from "../GitHubAPI"
import { requestWithFixturedJSON } from "../../_tests/_github.test"
import { GitHubUser } from "../../../dsl/GitHubDSL"
import { GitHubFile } from "../GitHubAPI"

const fetchJSON = (api: any, params: any): Promise<any> => {
return Promise.resolve({
Expand All @@ -25,19 +26,6 @@ const fetchErrorJSON = (api: any, params: any): Promise<any> => {
})
}

const fetchText = (api: any, params: any): Promise<any> => {
return Promise.resolve({
ok: true,
text: () =>
Promise.resolve(
JSON.stringify({
api,
...params,
})
),
})
}

const fetch = (api: any, params: any): Promise<any> => {
return Promise.resolve({
api,
Expand Down Expand Up @@ -86,17 +74,40 @@ describe("API testing", () => {
})

it("getPullRequestDiff", async () => {
api.getPullRequestInfo = await requestWithFixturedJSON("github_pr.json")
api.fetch = fetchText
let text = await api.getPullRequestDiff()
expect(JSON.parse(text)).toMatchObject({
api: "https://api.github.com/repos/artsy/emission/pulls/1",
headers: {
Authorization: "token ABCDE",
Accept: "application/vnd.github.v3.diff",
"Content-Type": "application/json",
},
api.fetch = jest.fn().mockReturnValue({
ok: true,
headers: new Headers({}),
json: jest
.fn()
.mockImplementation(() =>
Promise.resolve<GitHubFile[]>(JSON.parse('[{"filename": "file.txt", "patch": "+ hello"}]'))
),
})

let diff = await api.getPullRequestDiff()

let expected = `
diff --git a/file.txt b/file.txt
--- a/file.txt
+++ b/file.txt
+ hello
`

expect(diff).toEqual(expected)

expect(api.fetch).toHaveBeenCalledWith(
"https://api.github.com/repos/artsy/emission/pulls/1/files?page=1&per_page=100",
{
body: null,
headers: {
Accept: "application/vnd.github.v3.diff",
Authorization: "token ABCDE",
"Content-Type": "application/json",
},
method: "GET",
},
undefined
)
})

it("getDangerCommentIDs ignores comments not marked as generated", async () => {
Expand Down

0 comments on commit bf158b8

Please sign in to comment.