From 0eb5b4c1ad939b322076d07ef895f2e11e7c65cc Mon Sep 17 00:00:00 2001 From: Scott Talbot Date: Thu, 27 Dec 2018 19:33:35 +1100 Subject: [PATCH 1/2] Loosen GitHub comment ids to strings --- source/platforms/github/GitHubAPI.ts | 6 +++--- source/platforms/github/_tests/_github_api.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/source/platforms/github/GitHubAPI.ts b/source/platforms/github/GitHubAPI.ts index a9d12c3b6..a25a96879 100644 --- a/source/platforms/github/GitHubAPI.ts +++ b/source/platforms/github/GitHubAPI.ts @@ -83,7 +83,7 @@ export class GitHubAPI { // The above is the API for Platform - getDangerCommentIDs = async (dangerID: string): Promise => { + getDangerCommentIDs = async (dangerID: string): Promise => { const userID = await this.getUserID() const allComments: any[] = await this.getPullRequestComments() const dangerIDMessage = dangerIDToString(dangerID) @@ -96,7 +96,7 @@ export class GitHubAPI { .map(comment => comment.id) // only return IDs } - updateCommentWithID = async (id: number, comment: string): Promise => { + updateCommentWithID = async (id: string, comment: string): Promise => { const repo = this.repoMetadata.repoSlug const res = await this.patch( `repos/${repo}/issues/comments/${id}`, @@ -109,7 +109,7 @@ export class GitHubAPI { return res.json() } - deleteCommentWithID = async (id: number): Promise => { + deleteCommentWithID = async (id: string): Promise => { const repo = this.repoMetadata.repoSlug const res = await this.api(`repos/${repo}/issues/comments/${id}`, {}, null, "DELETE") diff --git a/source/platforms/github/_tests/_github_api.test.ts b/source/platforms/github/_tests/_github_api.test.ts index c90255ac5..f12056f23 100644 --- a/source/platforms/github/_tests/_github_api.test.ts +++ b/source/platforms/github/_tests/_github_api.test.ts @@ -79,7 +79,7 @@ describe("API testing", () => { api.fetch = fetch api.patch = jest.fn(() => ({ json: jest.fn() })) - await api.updateCommentWithID(123, "Hello!") + await api.updateCommentWithID("123", "Hello!") expect(api.patch).toHaveBeenCalledWith("repos/artsy/emission/issues/comments/123", {}, { body: "Hello!" }) }) @@ -234,7 +234,7 @@ describe("API testing", () => { it("deleteCommentWithID", async () => { api.fetch = jest.fn().mockReturnValue({ status: 204 }) - await api.deleteCommentWithID(123) + await api.deleteCommentWithID("123") expect(api.fetch).toHaveBeenCalledWith( "https://api.github.com/repos/artsy/emission/issues/comments/123", From dbc961540590e4401ee7317004a12eb621e347c3 Mon Sep 17 00:00:00 2001 From: Scott Talbot Date: Thu, 27 Dec 2018 20:21:31 +1100 Subject: [PATCH 2/2] Introduce a type for GitHub issue comments --- source/dsl/GitHubDSL.ts | 17 +++++++++++++++++ source/platforms/github/GitHubAPI.ts | 17 ++++++++--------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/source/dsl/GitHubDSL.ts b/source/dsl/GitHubDSL.ts index 5ebae7aaf..a6feb4a8b 100644 --- a/source/dsl/GitHubDSL.ts +++ b/source/dsl/GitHubDSL.ts @@ -136,6 +136,23 @@ export interface GitHubIssueLabel { color: string } +export interface GitHubIssueComment { + /** + * UUID for the comment + */ + id: string + + /** + * The User who made the comment + */ + user: GitHubUser + + /** + * Textual representation of comment + */ + body: string +} + // This is `danger.github.pr` /** diff --git a/source/platforms/github/GitHubAPI.ts b/source/platforms/github/GitHubAPI.ts index a25a96879..e5e49c1d1 100644 --- a/source/platforms/github/GitHubAPI.ts +++ b/source/platforms/github/GitHubAPI.ts @@ -4,11 +4,10 @@ import * as node_fetch from "node-fetch" import parse from "parse-link-header" import pLimit from "p-limit" -import { GitHubPRDSL, GitHubUser } from "../../dsl/GitHubDSL" +import { GitHubPRDSL, GitHubIssueComment, GitHubUser } from "../../dsl/GitHubDSL" import { dangerIDToString } from "../../runner/templates/githubIssueTemplate" import { api as fetch } from "../../api/fetch" -import { Comment } from "../platform" import { RepoMetaData } from "../../dsl/BitBucketServerDSL" import { CheckOptions } from "./comms/checks/resultsToCheck" @@ -85,7 +84,7 @@ export class GitHubAPI { getDangerCommentIDs = async (dangerID: string): Promise => { const userID = await this.getUserID() - const allComments: any[] = await this.getPullRequestComments() + const allComments = await this.getPullRequestComments() const dangerIDMessage = dangerIDToString(dangerID) this.d(`User ID: ${userID}`) this.d(`Looking at ${allComments.length} comments for ${dangerIDMessage}`) @@ -274,23 +273,23 @@ export class GitHubAPI { return response.json() } - getPullRequestComments = async (): Promise => { + getPullRequestComments = async (): Promise => { const repo = this.repoMetadata.repoSlug const prID = this.repoMetadata.pullRequestID return await this.getAllOfResource(`repos/${repo}/issues/${prID}/comments`) } - getPullRequestInlineComments = async (dangerID: string): Promise => { + getPullRequestInlineComments = async (dangerID: string): Promise<(GitHubIssueComment & { ownedByDanger: boolean })[]> => { const userID = await this.getUserID() const repo = this.repoMetadata.repoSlug const prID = this.repoMetadata.pullRequestID - return await this.getAllOfResource(`repos/${repo}/pulls/${prID}/comments`).then(v => { + return await this.getAllOfResource(`repos/${repo}/pulls/${prID}/comments`).then((v: GitHubIssueComment[]) => { return v .filter(Boolean) - .map((i: any) => { - return { id: i.id, ownedByDanger: i.user.id == userID && i.body.includes(dangerID), body: i.body } + .map(i => { + return { ...i, ownedByDanger: i.user.id == userID && i.body.includes(dangerID) } }) - .filter((i: any) => i.ownedByDanger) + .filter(i => i.ownedByDanger) }) }