From a1d072846d036753f3d51f6a7f9a4ce06490f63d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Mon, 30 Apr 2018 18:22:03 -0700 Subject: [PATCH] Add PR reviews and comments to GitHub api (#179) Also, a slight re-organization of the GitHub api test code. --- .../github/__snapshots__/api.test.js.snap | 12 +- src/plugins/github/api.js | 56 ++++++- src/plugins/github/api.test.js | 140 ++++++++++-------- 3 files changed, 142 insertions(+), 66 deletions(-) diff --git a/src/plugins/github/__snapshots__/api.test.js.snap b/src/plugins/github/__snapshots__/api.test.js.snap index 927ea0a..f03e248 100644 --- a/src/plugins/github/__snapshots__/api.test.js.snap +++ b/src/plugins/github/__snapshots__/api.test.js.snap @@ -1,8 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`GitHub porcelain API Author 1`] = `2`; +exports[`GitHub porcelain API has wrappers for Authors 1`] = `2`; -exports[`GitHub porcelain API Author 2`] = ` +exports[`GitHub porcelain API has wrappers for Authors 2`] = ` Object { "address": Object { "id": "https://github.com/decentralion", @@ -18,9 +18,9 @@ Object { } `; -exports[`GitHub porcelain API Comment 1`] = `3`; +exports[`GitHub porcelain API has wrappers for Comments 1`] = `3`; -exports[`GitHub porcelain API Comment 2`] = ` +exports[`GitHub porcelain API has wrappers for Comments 2`] = ` Object { "address": Object { "id": "https://github.com/sourcecred/example-repo/issues/6#issuecomment-373768442", @@ -35,7 +35,7 @@ Object { } `; -exports[`GitHub porcelain API Issue 1`] = ` +exports[`GitHub porcelain API has wrappers for Issues 1`] = ` Object { "address": Object { "id": "https://github.com/sourcecred/example-repo/issues/1", @@ -52,7 +52,7 @@ Object { } `; -exports[`GitHub porcelain API PullRequest 1`] = ` +exports[`GitHub porcelain API has wrappers for PullRequests 1`] = ` Object { "address": Object { "id": "https://github.com/sourcecred/example-repo/pull/3", diff --git a/src/plugins/github/api.js b/src/plugins/github/api.js index 4d7d6bd..b8f3b88 100644 --- a/src/plugins/github/api.js +++ b/src/plugins/github/api.js @@ -11,6 +11,9 @@ import type { NodeType, IssueNodePayload, PullRequestNodePayload, + PullRequestReviewNodePayload, + PullRequestReviewCommentNodePayload, + PullRequestReviewState, CommentNodePayload, AuthorNodePayload, AuthorSubtype, @@ -23,9 +26,17 @@ import { AUTHOR_NODE_TYPE, ISSUE_NODE_TYPE, PULL_REQUEST_NODE_TYPE, + PULL_REQUEST_REVIEW_NODE_TYPE, + PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE, } from "./types"; -export type Entity = Issue | PullRequest | Comment | Author; +export type Entity = + | Issue + | PullRequest + | Comment + | Author + | PullRequestReview + | PullRequestReviewComment; function assertEntityType(e: Entity, t: NodeType) { if (e.type() !== t) { @@ -105,7 +116,12 @@ class GithubEntity { } class Post< - T: IssueNodePayload | PullRequestNodePayload | CommentNodePayload + T: + | IssueNodePayload + | PullRequestNodePayload + | CommentNodePayload + | PullRequestReviewNodePayload + | PullRequestReviewCommentNodePayload > extends GithubEntity { authors(): Author[] { return this.graph @@ -159,6 +175,14 @@ export class PullRequest extends Commentable { title(): string { return this.node().payload.title; } + reviews(): PullRequestReview[] { + return this.graph + .neighborhood(this.nodeAddress, { + edgeType: CONTAINS_EDGE_TYPE, + nodeType: PULL_REQUEST_REVIEW_NODE_TYPE, + }) + .map(({neighbor}) => new PullRequestReview(this.graph, neighbor)); + } } export class Issue extends Commentable { @@ -180,3 +204,31 @@ export class Comment extends Post { return (e: any); } } + +export class PullRequestReview extends Post { + static from(e: Entity): PullRequestReview { + assertEntityType(e, PULL_REQUEST_REVIEW_NODE_TYPE); + return (e: any); + } + state(): PullRequestReviewState { + return this.node().payload.state; + } + + comments(): PullRequestReviewComment[] { + return this.graph + .neighborhood(this.nodeAddress, { + edgeType: CONTAINS_EDGE_TYPE, + nodeType: PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE, + }) + .map(({neighbor}) => new PullRequestReviewComment(this.graph, neighbor)); + } +} + +export class PullRequestReviewComment extends Post< + PullRequestReviewCommentNodePayload +> { + static from(e: Entity): PullRequestReviewComment { + assertEntityType(e, PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE); + return (e: any); + } +} diff --git a/src/plugins/github/api.test.js b/src/plugins/github/api.test.js index 198537b..6207642 100644 --- a/src/plugins/github/api.test.js +++ b/src/plugins/github/api.test.js @@ -19,66 +19,90 @@ describe("GitHub porcelain API", () => { } return result; } + describe("has wrappers for", () => { + it("Issues", () => { + const issue = issueOrPRByNumber(1); + expect(issue.title()).toBe("An example issue."); + expect(issue.body()).toBe("This is just an example issue."); + expect(issue.number()).toBe(1); + expect(issue.type()).toBe(ISSUE_NODE_TYPE); + expect(issue.url()).toBe( + "https://github.com/sourcecred/example-repo/issues/1" + ); + expect(issue.node()).toMatchSnapshot(); + expect(issue.address()).toEqual(issue.node().address); + expect(issue.authors().map((x) => x.login())).toEqual(["decentralion"]); + }); - it("Issue", () => { - const issue = issueOrPRByNumber(1); - expect(issue.title()).toBe("An example issue."); - expect(issue.body()).toBe("This is just an example issue."); - expect(issue.number()).toBe(1); - expect(issue.type()).toBe(ISSUE_NODE_TYPE); - expect(issue.url()).toBe( - "https://github.com/sourcecred/example-repo/issues/1" - ); - expect(issue.node()).toMatchSnapshot(); - expect(issue.address()).toEqual(issue.node().address); - expect(issue.authors().map((x) => x.login())).toEqual(["decentralion"]); + it("PullRequests", () => { + const pullRequest = issueOrPRByNumber(3); + expect(pullRequest.body()).toBe("Oh look, it's a pull request."); + expect(pullRequest.title()).toBe("Add README, merge via PR."); + expect(pullRequest.url()).toBe( + "https://github.com/sourcecred/example-repo/pull/3" + ); + expect(pullRequest.number()).toBe(3); + expect(pullRequest.type()).toBe(PULL_REQUEST_NODE_TYPE); + expect(pullRequest.node()).toMatchSnapshot(); + expect(pullRequest.address()).toEqual(pullRequest.node().address); + }); + + it("Pull Request Reviews", () => { + const pr = PullRequest.from(issueOrPRByNumber(5)); + const reviews = pr.reviews(); + expect(reviews).toHaveLength(2); + expect(reviews[0].state()).toBe("CHANGES_REQUESTED"); + expect(reviews[1].state()).toBe("APPROVED"); + }); + + it("Pull Request Review Comments", () => { + const pr = PullRequest.from(issueOrPRByNumber(5)); + const reviews = pr.reviews(); + expect(reviews).toHaveLength(2); + const comments = reviews[0].comments(); + expect(comments).toHaveLength(1); + const comment = comments[0]; + expect(comment.url()).toBe( + "https://github.com/sourcecred/example-repo/pull/5#discussion_r171460198" + ); + expect(comment.body()).toBe("seems a bit capricious"); + expect(comment.authors().map((a) => a.login())).toEqual(["wchargin"]); + }); + + it("Comments", () => { + const issue = issueOrPRByNumber(6); + const comments = issue.comments(); + expect(comments.length).toMatchSnapshot(); + const comment = comments[0]; + expect(comment.type()).toBe(COMMENT_NODE_TYPE); + expect(comment.body()).toBe("A wild COMMENT appeared!"); + expect(comment.url()).toBe( + "https://github.com/sourcecred/example-repo/issues/6#issuecomment-373768442" + ); + expect(comment.node()).toMatchSnapshot(); + expect(comment.address()).toEqual(comment.node().address); + expect(comment.authors().map((x) => x.login())).toEqual(["decentralion"]); + }); + + it("Authors", () => { + const authors = repo.authors(); + // So we don't need to manually update the test if a new person posts + expect(authors.length).toMatchSnapshot(); + + const decentralion = authors.find((x) => x.login() === "decentralion"); + if (decentralion == null) { + throw new Error("Who let the lions out?"); + } + expect(decentralion.url()).toBe("https://github.com/decentralion"); + expect(decentralion.type()).toBe(AUTHOR_NODE_TYPE); + expect(decentralion.subtype()).toBe("USER"); + expect(decentralion.node()).toMatchSnapshot(); + expect(decentralion.address()).toEqual(decentralion.node().address); + }); }); - it("PullRequest", () => { - const pullRequest = issueOrPRByNumber(3); - expect(pullRequest.body()).toBe("Oh look, it's a pull request."); - expect(pullRequest.url()).toBe( - "https://github.com/sourcecred/example-repo/pull/3" - ); - expect(pullRequest.number()).toBe(3); - expect(pullRequest.type()).toBe(PULL_REQUEST_NODE_TYPE); - expect(pullRequest.node()).toMatchSnapshot(); - expect(pullRequest.address()).toEqual(pullRequest.node().address); - }); - - it("Comment", () => { - const issue = issueOrPRByNumber(6); - const comments = issue.comments(); - expect(comments.length).toMatchSnapshot(); - const comment = comments[0]; - expect(comment.type()).toBe(COMMENT_NODE_TYPE); - expect(comment.body()).toBe("A wild COMMENT appeared!"); - expect(comment.url()).toBe( - "https://github.com/sourcecred/example-repo/issues/6#issuecomment-373768442" - ); - expect(comment.node()).toMatchSnapshot(); - expect(comment.address()).toEqual(comment.node().address); - expect(comment.authors().map((x) => x.login())).toEqual(["decentralion"]); - }); - - it("Author", () => { - const authors = repo.authors(); - // So we don't need to manually update the test if a new person posts - expect(authors.length).toMatchSnapshot(); - - const decentralion = authors.find((x) => x.login() === "decentralion"); - if (decentralion == null) { - throw new Error("Who let the lions out?"); - } - expect(decentralion.url()).toBe("https://github.com/decentralion"); - expect(decentralion.type()).toBe(AUTHOR_NODE_TYPE); - expect(decentralion.subtype()).toBe("USER"); - expect(decentralion.node()).toMatchSnapshot(); - expect(decentralion.address()).toEqual(decentralion.node().address); - }); - - describe("type coercion", () => { - it("type coercion works when typed correctly", () => { + describe("has type coercion that", () => { + it("allows refining types when correct", () => { const _unused_issue: Issue = Issue.from(issueOrPRByNumber(1)); const _unused_pr: PullRequest = PullRequest.from(issueOrPRByNumber(3)); const _unused_author: Author = Author.from( @@ -88,7 +112,7 @@ describe("GitHub porcelain API", () => { issueOrPRByNumber(2).comments()[0] ); }); - it("type coercion throws error when typed incorrectly", () => { + it("throws an error on bad type refinement", () => { expect(() => PullRequest.from(issueOrPRByNumber(1))).toThrowError( "to have type PULL_REQUEST" );