Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Danger to find its own inline comment with an exact ID match #1287

Merged
merged 1 commit into from Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 9 additions & 10 deletions source/platforms/github/GitHubAPI.ts
Expand Up @@ -89,10 +89,10 @@ export class GitHubAPI {
this.d(`User ID: ${userID}`)
this.d(`Looking at ${allComments.length} comments for ${dangerIDMessage}`)
return allComments
.filter(comment => comment.body.includes(dangerIDMessage)) // does it contain the right danger ID?
.filter(comment => comment.user.id === userID) // Does it have the right user ID?
.filter(comment => comment.body.includes("Generated by")) // Does it look like a danger message?
.map(comment => comment.id) // only return IDs
.filter((comment) => comment.body.includes(dangerIDMessage)) // does it contain the right danger ID?
.filter((comment) => comment.user.id === userID) // Does it have the right user ID?
.filter((comment) => comment.body.includes("Generated by")) // Does it look like a danger message?
.map((comment) => comment.id) // only return IDs
}

updateCommentWithID = async (id: string, comment: string): Promise<any> => {
Expand Down Expand Up @@ -289,9 +289,7 @@ export class GitHubAPI {
page = getNextPageFromLinkHeader(response)
} else {
this.d(
`getPullRequestCommits:: Failed to get response while traverse page ${page} with ${
response.status
}, bailing rest of pages if exists`
`getPullRequestCommits:: Failed to get response while traverse page ${page} with ${response.status}, bailing rest of pages if exists`
)
page = -1
}
Expand All @@ -317,13 +315,14 @@ export class GitHubAPI {
const userID = await this.getUserID()
const repo = this.repoMetadata.repoSlug
const prID = this.repoMetadata.pullRequestID
const dangerIDMessage = dangerIDToString(dangerID)
return await this.getAllOfResource(`repos/${repo}/pulls/${prID}/comments`).then((v: GitHubIssueComment[]) => {
return v
.filter(Boolean)
.map(i => {
return { ...i, ownedByDanger: i.user.id == userID && i.body.includes(dangerID) }
.map((i) => {
return { ...i, ownedByDanger: i.user.id == userID && i.body.includes(dangerIDMessage) }
})
.filter(i => i.ownedByDanger)
.filter((i) => i.ownedByDanger)
})
}

Expand Down
10 changes: 5 additions & 5 deletions source/platforms/github/_tests/_github_api.test.ts
Expand Up @@ -100,7 +100,7 @@ describe("API testing", () => {

it("getDangerCommentIDs ignores comments not marked as generated", async () => {
api.getAllOfResource = await requestWithFixturedJSON("github_inline_comments_with_danger.json")
api.getUserID = () => new Promise<number>(r => r(20229914))
api.getUserID = () => new Promise<number>((r) => r(20229914))

const commentIDs = await api.getDangerCommentIDs("default")

Expand All @@ -109,19 +109,19 @@ describe("API testing", () => {

it("getPullRequestInlineComment gets only comments for given DangerId", async () => {
api.getAllOfResource = await requestWithFixturedJSON("github_inline_comments_with_danger.json")
api.getUserID = () => new Promise<number>(r => r(20229914))
api.getUserID = () => new Promise<number>((r) => r(20229914))

const comments = await api.getPullRequestInlineComments("danger-id-default")
const comments = await api.getPullRequestInlineComments("default")

expect(comments.length).toEqual(1)
expect(comments[0].ownedByDanger).toBeTruthy()
})

it("getPullRequestInlineComment doesn't get comments as the DangerId is different", async () => {
api.getAllOfResource = await requestWithFixturedJSON("github_inline_comments_with_danger.json")
api.getUserID = () => new Promise<number>(r => r(123))
api.getUserID = () => new Promise<number>((r) => r(123))

const comments = await api.getPullRequestInlineComments("danger-id-default")
const comments = await api.getPullRequestInlineComments("default")

expect(comments.length).toEqual(0)
})
Expand Down