Skip to content

Commit

Permalink
Merge multiple accept headers, instead of having them conflicting
Browse files Browse the repository at this point in the history
  • Loading branch information
orta committed Aug 27, 2017
1 parent cd0a24e commit 32de218
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 4 deletions.
5 changes: 4 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

### Master

- Fixed #348 invalid json response body error - felipesabino
### 2.0.0-alpha.12

- Fixed #348 invalid json response body error on generating a diff - felipesabino
- Potential fix for ^ that works with Peril also - orta

### 2.0.0-alpha.11

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "danger",
"version": "2.0.0-alpha.11",
"version": "2.0.0-alpha.12",
"description": "Unit tests for Team Culture",
"main": "distribution/danger.js",
"typings": "distribution/danger.d.ts",
Expand Down
9 changes: 8 additions & 1 deletion source/platforms/github/GitHubAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export class GitHubAPI {
const prJSON = await this.getPullRequestInfo()
const diffURL = prJSON["diff_url"]
const res = await this.get(diffURL, {
accept: "application/vnd.github.v3.diff",
Accept: "application/vnd.github.v3.diff",
})

return res.ok ? res.text() : ""
Expand Down Expand Up @@ -287,13 +287,20 @@ export class GitHubAPI {
const baseUrl = process.env["DANGER_GITHUB_API_BASE_URL"] || "https://api.github.com"
const url = containsBase ? path : `${baseUrl}/${path}`

let customAccept = {}
if (headers.Accept && this.additionalHeaders.Accept) {
// We need to merge the accepts which are comma separated according to the HTML spec
// e.g. https://gist.github.com/LTe/5270348
customAccept = { Accept: `${this.additionalHeaders.Accept}, ${headers.Accept}` }
}
return this.fetch(url, {
method: method,
body: body,
headers: {
"Content-Type": "application/json",
...headers,
...this.additionalHeaders,
...customAccept,
},
})
}
Expand Down
20 changes: 19 additions & 1 deletion source/platforms/github/_tests/_github_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe("API testing", () => {
api: "https://github.com/artsy/emission/pull/327.diff",
headers: {
Authorization: "token ABCDE",
accept: "application/vnd.github.v3.diff",
Accept: "application/vnd.github.v3.diff",
"Content-Type": "application/json",
},
})
Expand Down Expand Up @@ -110,6 +110,24 @@ describe("Peril", () => {
})
})

it("Merges two Accept headers", async () => {
api.additionalHeaders = { Accept: "application/vnd.github.machine-man-preview+json" }

const request = await api.get("user", {
Accept: "application/vnd.github.v3.diff",
})

expect(api.fetch).toHaveBeenCalledWith("https://api.github.com/user", {
body: {},
headers: {
Accept: "application/vnd.github.machine-man-preview+json, application/vnd.github.v3.diff",
Authorization: "token ABCDE",
"Content-Type": "application/json",
},
method: "GET",
})
})

describe("Allows setting DANGER_GITHUB_APP env variable", () => {
beforeEach(() => {
process.env.DANGER_GITHUB_APP = "1"
Expand Down

0 comments on commit 32de218

Please sign in to comment.