From 3db65129b90abe03e485dfe7601ad43512f2cfae Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Fri, 19 May 2017 15:19:45 +1000 Subject: [PATCH 01/16] Initial pass to add the read-only PR review API. --- .../src/main/scala/github4s/Decoders.scala | 7 +++++++ .../src/main/scala/github4s/Encoders.scala | 2 ++ .../src/main/scala/github4s/GithubAPIs.scala | 7 +++++++ .../scala/github4s/api/PullRequests.scala | 9 +++++++++ .../free/algebra/PullRequestOps.scala | 13 +++++++++++++ .../github4s/free/domain/PullRequest.scala | 19 +++++++++++++++++++ .../free/interpreters/Interpreters.scala | 2 ++ 7 files changed, 59 insertions(+) diff --git a/github4s/shared/src/main/scala/github4s/Decoders.scala b/github4s/shared/src/main/scala/github4s/Decoders.scala index f2495a36c..3fa15b013 100644 --- a/github4s/shared/src/main/scala/github4s/Decoders.scala +++ b/github4s/shared/src/main/scala/github4s/Decoders.scala @@ -160,6 +160,13 @@ object Decoders { } } + implicit val decodePrrStatus: Decoder[PullRequestReviewState] = + Decoder.decodeString.map { + case "APPROVE" => PRRStateApprove() + case "REQUEST_CHANGES" => PRRStateRequestChanges() + case "COMMENTED" => PRRStateCommented() + } + implicit val decodeGist: Decoder[Gist] = Decoder.instance { c ⇒ for { url ← c.downField("url").as[String] diff --git a/github4s/shared/src/main/scala/github4s/Encoders.scala b/github4s/shared/src/main/scala/github4s/Encoders.scala index d89085411..5c4c62851 100644 --- a/github4s/shared/src/main/scala/github4s/Encoders.scala +++ b/github4s/shared/src/main/scala/github4s/Encoders.scala @@ -32,4 +32,6 @@ object Encoders { case d: CreatePullRequestData => d.asJson case d: CreatePullRequestIssue => d.asJson } + + implicit val encodePrrStatus: Encoder[PullRequestReviewState] = Encoder.encodeString.contramap(_.value) } diff --git a/github4s/shared/src/main/scala/github4s/GithubAPIs.scala b/github4s/shared/src/main/scala/github4s/GithubAPIs.scala index c2c34a303..adc5a22a3 100644 --- a/github4s/shared/src/main/scala/github4s/GithubAPIs.scala +++ b/github4s/shared/src/main/scala/github4s/GithubAPIs.scala @@ -315,4 +315,11 @@ class GHPullRequests(accessToken: Option[String] = None)(implicit O: PullRequest maintainerCanModify: Option[Boolean] = Some(true) ): GHIO[GHResponse[PullRequest]] = O.createPullRequest(owner, repo, newPullRequest, head, base, maintainerCanModify, accessToken) + + def listReviews( + owner: String, + repo: String, + pullRequest: Int + ): GHIO[GHResponse[List[PullRequestReview]]] = + O.listPullRequestReviews(owner, repo, pullRequest, accessToken) } diff --git a/github4s/shared/src/main/scala/github4s/api/PullRequests.scala b/github4s/shared/src/main/scala/github4s/api/PullRequests.scala index f8bad328b..8a7a75afc 100644 --- a/github4s/shared/src/main/scala/github4s/api/PullRequests.scala +++ b/github4s/shared/src/main/scala/github4s/api/PullRequests.scala @@ -118,4 +118,13 @@ class PullRequests[C, M[_]]( httpClient .post[PullRequest](accessToken, s"repos/$owner/$repo/pulls", headers, data.asJson.noSpaces) } + + def listReviews( + accessToken: Option[String] = None, + headers: Map[String, String] = Map(), + owner: String, + repo: String, + pullRequest: Int): M[GHResponse[List[PullRequestReview]]] = { + httpClient.get[List[PullRequestReview]](accessToken, s"repos/$owner/$repo/pulls/$pullRequest/reviews", headers) + } } diff --git a/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala b/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala index 65281794d..82bcc34cf 100644 --- a/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala +++ b/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala @@ -49,6 +49,13 @@ final case class CreatePullRequest( accessToken: Option[String] = None ) extends PullRequestOp[GHResponse[PullRequest]] +final case class ListPullRequestReviews( + owner: String, + repo: String, + pullRequest: Int, + accessToken: Option[String] = None +) extends PullRequestOp[GHResponse[List[PullRequestReview]]] + /** * Exposes Pull Request operations as a Free monadic algebra that may be combined with other * Algebras via Coproduct @@ -83,6 +90,12 @@ class PullRequestOps[F[_]](implicit I: Inject[PullRequestOp, F]) { Free.inject[PullRequestOp, F]( CreatePullRequest(owner, repo, newPullRequest, head, base, maintainerCanModify, accessToken)) + def listPullRequestReviews( + owner: String, + repo: String, + pullRequest: Int, + accessToken: Option[String] = None): Free[F, GHResponse[List[PullRequestReview]]] = + Free.inject[PullRequestOp, F](ListPullRequestReviews(owner, repo, pullRequest, accessToken)) } /** diff --git a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala index 65b2c96e6..ffc1dc2c7 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala @@ -101,3 +101,22 @@ case object PRFilterOrderDesc extends PRFilterDirection("desc") sealed trait NewPullRequest case class NewPullRequestData(title: String, body: String) extends NewPullRequest case class NewPullRequestIssue(issue: Int) extends NewPullRequest + +case class PullRequestReview( + id: Int, + user: User, + body: String, + commit_id: String, + state: PullRequestReviewState, + html_url: String, + pull_request_url: String) + +sealed abstract class PullRequestReviewState(val value: String) +object PullRequestReviewState { + +} +case class PRRStateApprove() extends PullRequestReviewState("APPROVE") +case class PRRStateRequestChanges() extends PullRequestReviewState("REQUEST_CHANGES") +case class PRRStateCommented() extends PullRequestReviewState("COMMENTED") +// TODO: This requires additional, conditional support for just this state. +//case class PRRStatePending() extends PullRequestReviewState("PENDING") diff --git a/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala b/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala index 94b496632..b412c0ab3 100644 --- a/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala +++ b/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala @@ -309,6 +309,8 @@ class Interpreters[M[_], C]( head, base, maintainerCanModify) + case ListPullRequestReviews(owner, repo, pullRequest, accessToken) ⇒ + pullRequests.listReviews(accessToken, headers, owner, repo, pullRequest) } } } From 80839bbfb5d3302ef6258c93c02b8cd5bf98942e Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Fri, 19 May 2017 15:25:41 +1000 Subject: [PATCH 02/16] Add inline docs --- .../src/main/scala/github4s/api/PullRequests.scala | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/github4s/shared/src/main/scala/github4s/api/PullRequests.scala b/github4s/shared/src/main/scala/github4s/api/PullRequests.scala index 8a7a75afc..6f01192fb 100644 --- a/github4s/shared/src/main/scala/github4s/api/PullRequests.scala +++ b/github4s/shared/src/main/scala/github4s/api/PullRequests.scala @@ -119,6 +119,15 @@ class PullRequests[C, M[_]]( .post[PullRequest](accessToken, s"repos/$owner/$repo/pulls", headers, data.asJson.noSpaces) } + /** + * List pull request reviews. + * + * @param accessToken Token to identify the authenticated user + * @param headers Optional user header to include in the request + * @param owner Owner of the repo + * @param repo Name of the repo + * @param pullRequest ID number of the PR to get reviews for. + */ def listReviews( accessToken: Option[String] = None, headers: Map[String, String] = Map(), From a6bf4a368f7aa6a0de7de8373d7c1abce7ad2c4e Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Fri, 19 May 2017 16:08:15 +1000 Subject: [PATCH 03/16] Remove defunct object --- .../src/main/scala/github4s/free/domain/PullRequest.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala index ffc1dc2c7..eff801284 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala @@ -112,9 +112,6 @@ case class PullRequestReview( pull_request_url: String) sealed abstract class PullRequestReviewState(val value: String) -object PullRequestReviewState { - -} case class PRRStateApprove() extends PullRequestReviewState("APPROVE") case class PRRStateRequestChanges() extends PullRequestReviewState("REQUEST_CHANGES") case class PRRStateCommented() extends PullRequestReviewState("COMMENTED") From 71e9b8932541c0a50fa58531d8678a14e3ccf94a Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Fri, 19 May 2017 16:46:48 +1000 Subject: [PATCH 04/16] Add a getReview endpoint --- .../src/main/scala/github4s/Decoders.scala | 1 + .../src/main/scala/github4s/GithubAPIs.scala | 8 ++++++++ .../scala/github4s/api/PullRequests.scala | 20 +++++++++++++++++++ .../free/algebra/PullRequestOps.scala | 16 +++++++++++++++ .../github4s/free/domain/PullRequest.scala | 3 +-- .../free/interpreters/Interpreters.scala | 2 ++ 6 files changed, 48 insertions(+), 2 deletions(-) diff --git a/github4s/shared/src/main/scala/github4s/Decoders.scala b/github4s/shared/src/main/scala/github4s/Decoders.scala index 3fa15b013..24c477140 100644 --- a/github4s/shared/src/main/scala/github4s/Decoders.scala +++ b/github4s/shared/src/main/scala/github4s/Decoders.scala @@ -165,6 +165,7 @@ object Decoders { case "APPROVE" => PRRStateApprove() case "REQUEST_CHANGES" => PRRStateRequestChanges() case "COMMENTED" => PRRStateCommented() + case "PENDING" => PRRStatePending() } implicit val decodeGist: Decoder[Gist] = Decoder.instance { c ⇒ diff --git a/github4s/shared/src/main/scala/github4s/GithubAPIs.scala b/github4s/shared/src/main/scala/github4s/GithubAPIs.scala index adc5a22a3..f41e80b4d 100644 --- a/github4s/shared/src/main/scala/github4s/GithubAPIs.scala +++ b/github4s/shared/src/main/scala/github4s/GithubAPIs.scala @@ -322,4 +322,12 @@ class GHPullRequests(accessToken: Option[String] = None)(implicit O: PullRequest pullRequest: Int ): GHIO[GHResponse[List[PullRequestReview]]] = O.listPullRequestReviews(owner, repo, pullRequest, accessToken) + + def getReview( + owner: String, + repo: String, + pullRequest: Int, + review: Int + ): GHIO[GHResponse[PullRequestReview]] = + O.getPullRequestReview(owner, repo, pullRequest, review, accessToken) } diff --git a/github4s/shared/src/main/scala/github4s/api/PullRequests.scala b/github4s/shared/src/main/scala/github4s/api/PullRequests.scala index 6f01192fb..0d346acb0 100644 --- a/github4s/shared/src/main/scala/github4s/api/PullRequests.scala +++ b/github4s/shared/src/main/scala/github4s/api/PullRequests.scala @@ -136,4 +136,24 @@ class PullRequests[C, M[_]]( pullRequest: Int): M[GHResponse[List[PullRequestReview]]] = { httpClient.get[List[PullRequestReview]](accessToken, s"repos/$owner/$repo/pulls/$pullRequest/reviews", headers) } + + /** + * Get a specific pull request review. + * + * @param accessToken Token to identify the authenticated user + * @param headers Optional user header to include in the request + * @param owner Owner of the repo + * @param repo Name of the repo + * @param pullRequest ID number of the PR to get reviews for + * @param review ID number of the review to retrieve. + */ + def getReview( + accessToken: Option[String] = None, + headers: Map[String, String] = Map(), + owner: String, + repo: String, + pullRequest: Int, + review: Int): M[GHResponse[PullRequestReview]] = { + httpClient.get[PullRequestReview](accessToken, s"repos/$owner/$repo/pulls/$pullRequest/reviews/$review", headers) + } } diff --git a/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala b/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala index 82bcc34cf..0058987ee 100644 --- a/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala +++ b/github4s/shared/src/main/scala/github4s/free/algebra/PullRequestOps.scala @@ -56,6 +56,14 @@ final case class ListPullRequestReviews( accessToken: Option[String] = None ) extends PullRequestOp[GHResponse[List[PullRequestReview]]] +final case class GetPullRequestReview( + owner: String, + repo: String, + pullRequest: Int, + review: Int, + accessToken: Option[String] = None +) extends PullRequestOp[GHResponse[PullRequestReview]] + /** * Exposes Pull Request operations as a Free monadic algebra that may be combined with other * Algebras via Coproduct @@ -96,6 +104,14 @@ class PullRequestOps[F[_]](implicit I: Inject[PullRequestOp, F]) { pullRequest: Int, accessToken: Option[String] = None): Free[F, GHResponse[List[PullRequestReview]]] = Free.inject[PullRequestOp, F](ListPullRequestReviews(owner, repo, pullRequest, accessToken)) + + def getPullRequestReview( + owner: String, + repo: String, + pullRequest: Int, + review: Int, + accessToken: Option[String] = None): Free[F, GHResponse[PullRequestReview]] = + Free.inject[PullRequestOp, F](GetPullRequestReview(owner, repo, pullRequest, review, accessToken)) } /** diff --git a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala index eff801284..f90a68e73 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala @@ -115,5 +115,4 @@ sealed abstract class PullRequestReviewState(val value: String) case class PRRStateApprove() extends PullRequestReviewState("APPROVE") case class PRRStateRequestChanges() extends PullRequestReviewState("REQUEST_CHANGES") case class PRRStateCommented() extends PullRequestReviewState("COMMENTED") -// TODO: This requires additional, conditional support for just this state. -//case class PRRStatePending() extends PullRequestReviewState("PENDING") +case class PRRStatePending() extends PullRequestReviewState("PENDING") diff --git a/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala b/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala index b412c0ab3..cc001f4af 100644 --- a/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala +++ b/github4s/shared/src/main/scala/github4s/free/interpreters/Interpreters.scala @@ -311,6 +311,8 @@ class Interpreters[M[_], C]( maintainerCanModify) case ListPullRequestReviews(owner, repo, pullRequest, accessToken) ⇒ pullRequests.listReviews(accessToken, headers, owner, repo, pullRequest) + case GetPullRequestReview(owner, repo, pullRequest, review, accessToken) ⇒ + pullRequests.getReview(accessToken, headers, owner, repo, pullRequest, review) } } } From da817c395993b935f2d97c6d670700f934719485 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Mon, 22 May 2017 09:50:45 +1000 Subject: [PATCH 05/16] Add the rest of the tests for this issue --- .../test/scala/github4s/unit/ApiSpec.scala | 42 +++++++ .../github4s/utils/MockGithubApiServer.scala | 53 ++++++++ .../integration/GHPullRequestsSpec.scala | 42 +++++++ .../github4s/unit/GHPullRequestsSpec.scala | 44 +++++++ .../github4s/unit/PullRequestsSpec.scala | 46 +++++++ .../scala/github4s/utils/FakeResponses.scala | 116 ++++++++++++++++++ .../test/scala/github4s/utils/TestData.scala | 15 ++- 7 files changed, 356 insertions(+), 2 deletions(-) diff --git a/github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala b/github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala index 9fcb558c7..4355f1bff 100644 --- a/github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala +++ b/github4s/jvm/src/test/scala/github4s/unit/ApiSpec.scala @@ -758,6 +758,48 @@ class ApiSpec response should be('left) } + "PullRequests >> List PullRequestReviews" should "return a list of reviews when valid data is provided" in { + val response = pullRequests.listReviews( + accessToken, + headerUserAgent, + validRepoOwner, + validRepoName, + validPullRequestNumber) + response should be('right) + } + + it should "return an error when invalid data is passed" in { + val response = pullRequests.listReviews( + accessToken, + headerUserAgent, + validRepoOwner, + invalidRepoName, + validPullRequestNumber) + response should be('left) + } + + "PullRequests >> Get PullRequestReview" should "return a single review when valid data is provided" in { + val response = pullRequests.getReview( + accessToken, + headerUserAgent, + validRepoOwner, + validRepoName, + validPullRequestNumber, + validPullRequestReviewNumber) + response should be('right) + } + + it should "return an error when invalid data is passed" in { + val response = pullRequests.getReview( + accessToken, + headerUserAgent, + validRepoOwner, + invalidRepoName, + validPullRequestNumber, + validPullRequestReviewNumber) + response should be('left) + } + "Issues >> List" should "return the expected issues when a valid owner/repo is provided" in { val response = issues.list(accessToken, headerUserAgent, validRepoOwner, validRepoName) diff --git a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala index 85a4ab26a..5095d930e 100644 --- a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala +++ b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala @@ -598,6 +598,59 @@ trait MockGithubApiServer extends MockServerService with FakeResponses with Test """.stripMargin))) .respond(response.withStatusCode(createdStatusCode).withBody(validCreatePullRequest)) + //PullRequests >> listReviews + mockServer + .when( + request + .withMethod("GET") + .withPath(s"/repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews") + .withHeader("Authorization", tokenHeader)) + .respond(response.withStatusCode(okStatusCode).withBody(listReviewsValidResponse)) + + mockServer + .when( + request + .withMethod("GET") + .withPath(s"/repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews") + .withHeader(not("Authorization"))) + .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) + + mockServer + .when( + request + .withMethod("GET") + .withPath(s"/repos/$validRepoOwner/$invalidRepoName/pulls/$validPullRequestNumber/reviews") + .withHeader("Authorization", tokenHeader)) + .respond(response.withStatusCode(notFoundStatusCode).withBody(notFoundResponse)) + + //PullRequests >> getReview + mockServer + .when( + request + .withMethod("GET") + .withPath( + s"/repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews/$validPullRequestReviewNumber") + .withHeader("Authorization", tokenHeader)) + .respond(response.withStatusCode(okStatusCode).withBody(getReviewValidResponse)) + + mockServer + .when( + request + .withMethod("GET") + .withPath( + s"/repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews/$validPullRequestReviewNumber") + .withHeader(not("Authorization"))) + .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) + + mockServer + .when( + request + .withMethod("GET") + .withPath( + s"/repos/$validRepoOwner/$invalidRepoName/pulls/$validPullRequestNumber/reviews/$validPullRequestReviewNumber") + .withHeader("Authorization", tokenHeader)) + .respond(response.withStatusCode(notFoundStatusCode).withBody(notFoundResponse)) + //Issues >> list mockServer .when( diff --git a/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala b/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala index 4d81d1374..a1dcde3d4 100644 --- a/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala +++ b/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala @@ -80,4 +80,46 @@ trait GHPullRequestsSpec[T] extends BaseIntegrationSpec[T] { testFutureIsLeft(response) } + "PullRequests >> ListReviews" should "return a right response when a valid pr is provided" in { + val response = + Github(accessToken).pullRequests + .listReviews(validRepoOwner, validRepoName, validPullRequestNumber) + .execFuture[T](headerUserAgent) + + testFutureIsRight[List[PullRequestReview]](response, { r => + r.result.nonEmpty shouldBe true + r.statusCode shouldBe okStatusCode + }) + } + + it should "return error when an invalid repo name is passed" in { + val response = + Github(accessToken).pullRequests + .listReviews(validRepoOwner, invalidRepoName, validPullRequestNumber) + .execFuture[T](headerUserAgent) + + testFutureIsLeft(response) + } + + "PullRequests >> GetReview" should "return a right response when a valid pr review is provided" in { + val response = + Github(accessToken).pullRequests + .getReview(validRepoOwner, validRepoName, validPullRequestNumber, validPullRequestReviewNumber) + .execFuture[T](headerUserAgent) + + testFutureIsRight[PullRequestReview](response, { r => + r.result.id shouldBe validPullRequestReviewNumber + r.statusCode shouldBe okStatusCode + }) + } + + it should "return error when an invalid repo name is passed" in { + val response = + Github(accessToken).pullRequests + .getReview(validRepoOwner, validRepoName, validPullRequestNumber, validPullRequestReviewNumber) + .execFuture[T](headerUserAgent) + + testFutureIsLeft(response) + } + } diff --git a/github4s/shared/src/test/scala/github4s/unit/GHPullRequestsSpec.scala b/github4s/shared/src/test/scala/github4s/unit/GHPullRequestsSpec.scala index 6dde838c3..07810420b 100644 --- a/github4s/shared/src/test/scala/github4s/unit/GHPullRequestsSpec.scala +++ b/github4s/shared/src/test/scala/github4s/unit/GHPullRequestsSpec.scala @@ -106,4 +106,48 @@ class GHPullRequestsSpec extends BaseSpec { validBase, Some(true)) } + + "GHPullRequests.listReviews" should "call to PullRequestOps with the right parameters" in { + + val response: Free[GitHub4s, GHResponse[List[PullRequestReview]]] = + Free.pure(Right(GHResult(List(pullRequestReview), okStatusCode, Map.empty))) + + val pullRequestOps = mock[PullRequestOpsTest] + (pullRequestOps.listPullRequestReviews _) + .expects( + validRepoOwner, + validRepoName, + validPullRequestNumber, + sampleToken) + .returns(response) + + val ghPullRequests = new GHPullRequests(sampleToken)(pullRequestOps) + ghPullRequests.listReviews( + validRepoOwner, + validRepoName, + validPullRequestNumber) + } + + "GHPullRequests.getReview" should "call to PullRequestOps with the right parameters" in { + + val response: Free[GitHub4s, GHResponse[PullRequestReview]] = + Free.pure(Right(GHResult(pullRequestReview, okStatusCode, Map.empty))) + + val pullRequestOps = mock[PullRequestOpsTest] + (pullRequestOps.getPullRequestReview _) + .expects( + validRepoOwner, + validRepoName, + validPullRequestNumber, + validPullRequestReviewNumber, + sampleToken) + .returns(response) + + val ghPullRequests = new GHPullRequests(sampleToken)(pullRequestOps) + ghPullRequests.getReview( + validRepoOwner, + validRepoName, + validPullRequestNumber, + validPullRequestReviewNumber) + } } diff --git a/github4s/shared/src/test/scala/github4s/unit/PullRequestsSpec.scala b/github4s/shared/src/test/scala/github4s/unit/PullRequestsSpec.scala index 09b8039cc..aa8471469 100644 --- a/github4s/shared/src/test/scala/github4s/unit/PullRequestsSpec.scala +++ b/github4s/shared/src/test/scala/github4s/unit/PullRequestsSpec.scala @@ -131,4 +131,50 @@ class PullRequestsSpec extends BaseSpec { validBase, Some(true)) } + + "GHPullRequests.listPullRequestReviews" should "call to httpClient.post with the right parameters" in { + + val response: GHResponse[List[PullRequestReview]] = + Right(GHResult(List(pullRequestReview), okStatusCode, Map.empty)) + + val httpClientMock = httpClientMockGet[List[PullRequestReview]]( + url = s"repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews", + response = response + ) + + val pullRequests = new PullRequests[String, Id] { + override val httpClient: HttpClient[String, Id] = httpClientMock + } + + pullRequests.listReviews( + sampleToken, + headerUserAgent, + validRepoOwner, + validRepoName, + validPullRequestNumber) + } + + "GHPullRequests.getPullRequestReview" should "call to httpClient.post with the right parameters" in { + + val response: GHResponse[PullRequestReview] = + Right(GHResult(pullRequestReview, okStatusCode, Map.empty)) + + val httpClientMock = httpClientMockGet[PullRequestReview]( + url = s"repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews/$validPullRequestReviewNumber", + response = response + ) + + val pullRequests = new PullRequests[String, Id] { + override val httpClient: HttpClient[String, Id] = httpClientMock + } + + pullRequests.getReview( + sampleToken, + headerUserAgent, + validRepoOwner, + validRepoName, + validPullRequestNumber, + validPullRequestReviewNumber) + } + } diff --git a/github4s/shared/src/test/scala/github4s/utils/FakeResponses.scala b/github4s/shared/src/test/scala/github4s/utils/FakeResponses.scala index 19840fa40..30311af9e 100644 --- a/github4s/shared/src/test/scala/github4s/utils/FakeResponses.scala +++ b/github4s/shared/src/test/scala/github4s/utils/FakeResponses.scala @@ -1789,6 +1789,122 @@ trait FakeResponses { | } |]""".stripMargin + val listReviewsValidResponse = + """ + |[ + | { + | "id": 39170343, + | "user": { + | "login": "BenFradet", + | "id": 1737211, + | "avatar_url": "https://avatars1.githubusercontent.com/u/1737211?v=3", + | "gravatar_id": "", + | "url": "https://api.github.com/users/BenFradet", + | "html_url": "https://github.com/BenFradet", + | "followers_url": "https://api.github.com/users/BenFradet/followers", + | "following_url": "https://api.github.com/users/BenFradet/following{/other_user}", + | "gists_url": "https://api.github.com/users/BenFradet/gists{/gist_id}", + | "starred_url": "https://api.github.com/users/BenFradet/starred{/owner}{/repo}", + | "subscriptions_url": "https://api.github.com/users/BenFradet/subscriptions", + | "organizations_url": "https://api.github.com/users/BenFradet/orgs", + | "repos_url": "https://api.github.com/users/BenFradet/repos", + | "events_url": "https://api.github.com/users/BenFradet/events{/privacy}", + | "received_events_url": "https://api.github.com/users/BenFradet/received_events", + | "type": "User", + | "site_admin": false + | }, + | "body": "Looks like a good start :+1: ", + | "state": "COMMENTED", + | "html_url": "https://github.com/47deg/github4s/pull/139#pullrequestreview-39170343", + | "pull_request_url": "https://api.github.com/repos/47deg/github4s/pulls/139", + | "_links": { + | "html": { + | "href": "https://github.com/47deg/github4s/pull/139#pullrequestreview-39170343" + | }, + | "pull_request": { + | "href": "https://api.github.com/repos/47deg/github4s/pulls/139" + | } + | }, + | "submitted_at": "2017-05-19T11:23:47Z", + | "commit_id": "71e9b8932541c0a50fa58531d8678a14e3ccf94a" + | }, + | { + | "id": 39355613, + | "user": { + | "login": "juanpedromoreno", + | "id": 4879373, + | "avatar_url": "https://avatars1.githubusercontent.com/u/4879373?v=3", + | "gravatar_id": "", + | "url": "https://api.github.com/users/juanpedromoreno", + | "html_url": "https://github.com/juanpedromoreno", + | "followers_url": "https://api.github.com/users/juanpedromoreno/followers", + | "following_url": "https://api.github.com/users/juanpedromoreno/following{/other_user}", + | "gists_url": "https://api.github.com/users/juanpedromoreno/gists{/gist_id}", + | "starred_url": "https://api.github.com/users/juanpedromoreno/starred{/owner}{/repo}", + | "subscriptions_url": "https://api.github.com/users/juanpedromoreno/subscriptions", + | "organizations_url": "https://api.github.com/users/juanpedromoreno/orgs", + | "repos_url": "https://api.github.com/users/juanpedromoreno/repos", + | "events_url": "https://api.github.com/users/juanpedromoreno/events{/privacy}", + | "received_events_url": "https://api.github.com/users/juanpedromoreno/received_events", + | "type": "User", + | "site_admin": false + | }, + | "body": "", + | "state": "COMMENTED", + | "html_url": "https://github.com/47deg/github4s/pull/139#pullrequestreview-39355613", + | "pull_request_url": "https://api.github.com/repos/47deg/github4s/pulls/139", + | "_links": { + | "html": { + | "href": "https://github.com/47deg/github4s/pull/139#pullrequestreview-39355613" + | }, + | "pull_request": { + | "href": "https://api.github.com/repos/47deg/github4s/pulls/139" + | } + | }, + | "submitted_at": "2017-05-21T12:48:05Z", + | "commit_id": "71e9b8932541c0a50fa58531d8678a14e3ccf94a" + | } + |]""".stripMargin + + val getReviewValidResponse = + """ + |{ + | "id": 39318789, + | "user": { + | "login": "mscharley", + | "id": 336509, + | "avatar_url": "https://avatars3.githubusercontent.com/u/336509?v=3", + | "gravatar_id": "", + | "url": "https://api.github.com/users/mscharley", + | "html_url": "https://github.com/mscharley", + | "followers_url": "https://api.github.com/users/mscharley/followers", + | "following_url": "https://api.github.com/users/mscharley/following{/other_user}", + | "gists_url": "https://api.github.com/users/mscharley/gists{/gist_id}", + | "starred_url": "https://api.github.com/users/mscharley/starred{/owner}{/repo}", + | "subscriptions_url": "https://api.github.com/users/mscharley/subscriptions", + | "organizations_url": "https://api.github.com/users/mscharley/orgs", + | "repos_url": "https://api.github.com/users/mscharley/repos", + | "events_url": "https://api.github.com/users/mscharley/events{/privacy}", + | "received_events_url": "https://api.github.com/users/mscharley/received_events", + | "type": "User", + | "site_admin": false + | }, + | "body": "Adding a review to help support integration testing for #139 ", + | "state": "COMMENTED", + | "html_url": "https://github.com/47deg/github4s/pull/1#pullrequestreview-39318789", + | "pull_request_url": "https://api.github.com/repos/47deg/github4s/pulls/1", + | "_links": { + | "html": { + | "href": "https://github.com/47deg/github4s/pull/1#pullrequestreview-39318789" + | }, + | "pull_request": { + | "href": "https://api.github.com/repos/47deg/github4s/pulls/1" + | } + | }, + | "submitted_at": "2017-05-19T23:31:49Z", + | "commit_id": "765089ac90b2cb935a45d9ce214d3340e503663c" + |}""".stripMargin + val createIssueValidResponse = """ |{ diff --git a/github4s/shared/src/test/scala/github4s/utils/TestData.scala b/github4s/shared/src/test/scala/github4s/utils/TestData.scala index 690409009..69efc247f 100644 --- a/github4s/shared/src/test/scala/github4s/utils/TestData.scala +++ b/github4s/shared/src/test/scala/github4s/utils/TestData.scala @@ -103,8 +103,9 @@ trait TestData extends DummyGithubUrls { val validTagTitle = "v0.1.1" val validTagSha = "c3d0be41ecbe669545ee3e94d31ed9a4bc91ee3c" - val validPullRequestFileSha = "f80f79cafbe3f2ba71311b82e1171e73bd37a470" - val validPullRequestNumber = 1 + val validPullRequestFileSha = "f80f79cafbe3f2ba71311b82e1171e73bd37a470" + val validPullRequestNumber = 1 + val validPullRequestReviewNumber = 39318789 val validHead = "test-pr-issue" val invalidHead = "" @@ -357,4 +358,14 @@ trait TestData extends DummyGithubUrls { val validGistUrl = "https://api.github.com/gists/aa5a315d61ae9438b18d" val validGistId = "aa5a315d61ae9438b18d" val gist = Gist(validGistUrl, validGistId, validGistDescription, validGistPublic) + + val pullRequestReview = PullRequestReview( + id = validPullRequestReviewNumber, + user = user, + body = validCommentBody, + commit_id = validCommitSha, + state = PRRStateCommented(), + html_url = "", + pull_request_url = "" + ) } From 11408bcbc89fd2f842b100b3a51b69b37c734fd6 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Mon, 22 May 2017 09:55:41 +1000 Subject: [PATCH 06/16] Review state case class => case object --- github4s/shared/src/main/scala/github4s/Decoders.scala | 8 ++++---- .../src/main/scala/github4s/free/domain/PullRequest.scala | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/github4s/shared/src/main/scala/github4s/Decoders.scala b/github4s/shared/src/main/scala/github4s/Decoders.scala index 24c477140..d854cd997 100644 --- a/github4s/shared/src/main/scala/github4s/Decoders.scala +++ b/github4s/shared/src/main/scala/github4s/Decoders.scala @@ -162,10 +162,10 @@ object Decoders { implicit val decodePrrStatus: Decoder[PullRequestReviewState] = Decoder.decodeString.map { - case "APPROVE" => PRRStateApprove() - case "REQUEST_CHANGES" => PRRStateRequestChanges() - case "COMMENTED" => PRRStateCommented() - case "PENDING" => PRRStatePending() + case "APPROVE" => PRRStateApprove + case "REQUEST_CHANGES" => PRRStateRequestChanges + case "COMMENTED" => PRRStateCommented + case "PENDING" => PRRStatePending } implicit val decodeGist: Decoder[Gist] = Decoder.instance { c ⇒ diff --git a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala index f90a68e73..9272b580c 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala @@ -112,7 +112,7 @@ case class PullRequestReview( pull_request_url: String) sealed abstract class PullRequestReviewState(val value: String) -case class PRRStateApprove() extends PullRequestReviewState("APPROVE") -case class PRRStateRequestChanges() extends PullRequestReviewState("REQUEST_CHANGES") -case class PRRStateCommented() extends PullRequestReviewState("COMMENTED") -case class PRRStatePending() extends PullRequestReviewState("PENDING") +case object PRRStateApprove extends PullRequestReviewState("APPROVE") +case object PRRStateRequestChanges extends PullRequestReviewState("REQUEST_CHANGES") +case object PRRStateCommented extends PullRequestReviewState("COMMENTED") +case object PRRStatePending extends PullRequestReviewState("PENDING") From ea2934ad15b0d8068f16a98630dfadaa7b1fced5 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Mon, 22 May 2017 17:17:51 +1000 Subject: [PATCH 07/16] Add documentation for this PR --- docs/src/main/tut/pull_request.md | 53 +++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/docs/src/main/tut/pull_request.md b/docs/src/main/tut/pull_request.md index 57335bc41..ff8964994 100644 --- a/docs/src/main/tut/pull_request.md +++ b/docs/src/main/tut/pull_request.md @@ -11,6 +11,8 @@ with Github4s, you can: - [List pull requests](#list-pull-requests) - [List the files in a pull request](#list-the-files-in-a-pull-request) - [Create a pull request](#create-a-pull-request) +- [List reviews](#list-reviews) +- [Get a review](#get-an-individual-review) The following examples assume the following imports and token: @@ -128,6 +130,57 @@ createPullRequestIssue.exec[cats.Id, HttpResponse[String]]() match { See [the API doc](https://developer.github.com/v3/pulls/#create-a-pull-request) for full reference. +## List pull requests + +You can list the reviews for a pull request using `listReviews`, it takes as arguments: + +- the repository coordinates (`owner` and `name` of the repository). +- the pull request id. + +As an example, if we wanted to see all the reviews for pull request 139 of `47deg/github4s`: + +```scala +val listReviews = Github(accessToken).pullRequests.listReviews( + "47deg", + "github4s", + 139) + +listReviews.exec[cats.Id, HttpResponse[String]]() match { + case Left(e) => println("Something went wrong: s{e.getMessage}") + case Right(r) => println(r.result) +} +``` + +The `result` on the right is the matching [List[PullRequestReview]][pr-scala]. + +## Get an individual review + +You can list the reviews for a pull request using `listReviews`, it takes as arguments: + +- the repository coordinates (`owner` and `name` of the repository). +- the pull request id. +- the review id. + +As an example, if we wanted to see review 39355613 for pull request 139 of `47deg/github4s`: + +```scala +val review = Github(accessToken).pullRequests.getReview( + "47deg", + "github4s", + 139, + 39355613) + +review.exec[cats.Id, HttpResponse[String]]() match { + case Left(e) => println("Something went wrong: s{e.getMessage}") + case Right(r) => println(r.result) +} +``` + +The `result` on the right is the matching [PullRequestReview][pr-scala]. + + +See [the API doc](https://developer.github.com/v3/pulls/reviews/#list-reviews-on-a-pull-request) for full reference. + As you can see, a few features of the pull request endpoint are missing. As a result, if you'd like to see a feature supported, feel free to create an issue and/or a pull request! From ec8fd76fa250eadada27bd69ed4bbbe36d456c68 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Mon, 22 May 2017 21:10:14 +1000 Subject: [PATCH 08/16] Fix various issues with the PR --- docs/src/main/tut/pull_request.md | 15 +++++++------ .../github4s/utils/MockGithubApiServer.scala | 21 ++----------------- .../test/scala/github4s/utils/TestData.scala | 2 +- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/docs/src/main/tut/pull_request.md b/docs/src/main/tut/pull_request.md index ff8964994..9d62535e7 100644 --- a/docs/src/main/tut/pull_request.md +++ b/docs/src/main/tut/pull_request.md @@ -130,7 +130,9 @@ createPullRequestIssue.exec[cats.Id, HttpResponse[String]]() match { See [the API doc](https://developer.github.com/v3/pulls/#create-a-pull-request) for full reference. -## List pull requests +# Review API + +## List pull request reviews You can list the reviews for a pull request using `listReviews`, it takes as arguments: @@ -139,7 +141,7 @@ You can list the reviews for a pull request using `listReviews`, it takes as arg As an example, if we wanted to see all the reviews for pull request 139 of `47deg/github4s`: -```scala +```tut:silent val listReviews = Github(accessToken).pullRequests.listReviews( "47deg", "github4s", @@ -153,9 +155,11 @@ listReviews.exec[cats.Id, HttpResponse[String]]() match { The `result` on the right is the matching [List[PullRequestReview]][pr-scala]. +See [the API doc](https://developer.github.com/v3/pulls/reviews/#list-reviews-on-a-pull-request) for full reference. + ## Get an individual review -You can list the reviews for a pull request using `listReviews`, it takes as arguments: +You can get an individual review for a pull request using `getReview`, it takes as arguments: - the repository coordinates (`owner` and `name` of the repository). - the pull request id. @@ -163,7 +167,7 @@ You can list the reviews for a pull request using `listReviews`, it takes as arg As an example, if we wanted to see review 39355613 for pull request 139 of `47deg/github4s`: -```scala +```tut:silent val review = Github(accessToken).pullRequests.getReview( "47deg", "github4s", @@ -178,8 +182,7 @@ review.exec[cats.Id, HttpResponse[String]]() match { The `result` on the right is the matching [PullRequestReview][pr-scala]. - -See [the API doc](https://developer.github.com/v3/pulls/reviews/#list-reviews-on-a-pull-request) for full reference. +See [the API doc](https://developer.github.com/v3/pulls/reviews/#get-a-single-review) for full reference. As you can see, a few features of the pull request endpoint are missing. As a result, if you'd like to see a feature supported, feel free to create an issue and/or a pull request! diff --git a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala index 5095d930e..d405db51a 100644 --- a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala +++ b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala @@ -633,15 +633,6 @@ trait MockGithubApiServer extends MockServerService with FakeResponses with Test .withHeader("Authorization", tokenHeader)) .respond(response.withStatusCode(okStatusCode).withBody(getReviewValidResponse)) - mockServer - .when( - request - .withMethod("GET") - .withPath( - s"/repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews/$validPullRequestReviewNumber") - .withHeader(not("Authorization"))) - .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) - mockServer .when( request @@ -660,20 +651,12 @@ trait MockGithubApiServer extends MockServerService with FakeResponses with Test .withHeader("Authorization", tokenHeader)) .respond(response.withStatusCode(okStatusCode).withBody(listIssuesValidResponse)) - mockServer - .when( - request - .withMethod("GET") - .withPath(s"/repos/$validRepoOwner/$validRepoName/issues") - .withHeader(not("Authorization"))) - .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) - mockServer .when( request .withMethod("GET") .withPath(s"/repos/$validRepoOwner/$invalidRepoName/issues") - .withHeader(not("Authorization"))) + .withHeader("Authorization", tokenHeader)) .respond(response.withStatusCode(notFoundStatusCode).withBody(notFoundResponse)) //Issues >> create @@ -690,7 +673,7 @@ trait MockGithubApiServer extends MockServerService with FakeResponses with Test request .withMethod("POST") .withPath(s"/repos/$validRepoOwner/$validRepoName/issues") - .withHeader(not("Authorization"))) + .withHeader("Authorization", tokenHeader)) .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) mockServer diff --git a/github4s/shared/src/test/scala/github4s/utils/TestData.scala b/github4s/shared/src/test/scala/github4s/utils/TestData.scala index 69efc247f..711c2bc9f 100644 --- a/github4s/shared/src/test/scala/github4s/utils/TestData.scala +++ b/github4s/shared/src/test/scala/github4s/utils/TestData.scala @@ -364,7 +364,7 @@ trait TestData extends DummyGithubUrls { user = user, body = validCommentBody, commit_id = validCommitSha, - state = PRRStateCommented(), + state = PRRStateCommented, html_url = "", pull_request_url = "" ) From 3fe4cf20cb2100a55b25252c78bec5921669f097 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Mon, 22 May 2017 21:28:18 +1000 Subject: [PATCH 09/16] Fix derp in the links --- docs/src/main/tut/pull_request.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/main/tut/pull_request.md b/docs/src/main/tut/pull_request.md index 9d62535e7..1e6444db3 100644 --- a/docs/src/main/tut/pull_request.md +++ b/docs/src/main/tut/pull_request.md @@ -11,7 +11,7 @@ with Github4s, you can: - [List pull requests](#list-pull-requests) - [List the files in a pull request](#list-the-files-in-a-pull-request) - [Create a pull request](#create-a-pull-request) -- [List reviews](#list-reviews) +- [List reviews](#list-pull-request-reviews) - [Get a review](#get-an-individual-review) The following examples assume the following imports and token: From 04eb6e40ec1a6c461f1453823a38a5c3e959f22a Mon Sep 17 00:00:00 2001 From: Juan Pedro Moreno Date: Mon, 22 May 2017 14:03:14 +0200 Subject: [PATCH 10/16] Fixes ghost users associated with pull requests, issues and comments --- .../shared/src/main/scala/github4s/free/domain/Issue.scala | 4 ++-- .../src/main/scala/github4s/free/domain/PullRequest.scala | 2 +- github4s/shared/src/test/scala/github4s/utils/TestData.scala | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/github4s/shared/src/main/scala/github4s/free/domain/Issue.scala b/github4s/shared/src/main/scala/github4s/free/domain/Issue.scala index 14f80ad6a..72e3a341d 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/Issue.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/Issue.scala @@ -28,7 +28,7 @@ case class Issue( html_url: String, number: Int, state: String, - user: User, + user: Option[User], assignee: Option[User], labels: List[Label] = List.empty, locked: Option[Boolean], @@ -73,7 +73,7 @@ case class Comment( url: String, html_url: String, body: String, - user: User, + user: Option[User], created_at: String, updated_at: String) diff --git a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala index 65b2c96e6..cb1c59f9a 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala @@ -37,7 +37,7 @@ case class PullRequestBase( label: Option[String], ref: String, sha: String, - user: User, + user: Option[User], repo: Option[Repository]) case class PullRequestFile( diff --git a/github4s/shared/src/test/scala/github4s/utils/TestData.scala b/github4s/shared/src/test/scala/github4s/utils/TestData.scala index 690409009..bc58f3c67 100644 --- a/github4s/shared/src/test/scala/github4s/utils/TestData.scala +++ b/github4s/shared/src/test/scala/github4s/utils/TestData.scala @@ -162,7 +162,7 @@ trait TestData extends DummyGithubUrls { html_url = githubApiUrl, number = validIssueNumber, state = validIssueState, - user = user, + user = Some(user), assignee = None, labels = List.empty, locked = None, @@ -307,7 +307,7 @@ trait TestData extends DummyGithubUrls { "https://api.github.com/repos/octocat/Hello-World/issues/comments/1", "https: //github.com/octocat/Hello-World/issues/1347#issuecomment-1", validCommentBody, - user, + Some(user), "2011-04-14T16:00:49Z", "2011-04-14T16:00:49Z" ) From 48bb78aca2043113971a105c7481c1262794e3e7 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Mon, 22 May 2017 22:19:46 +1000 Subject: [PATCH 11/16] Fix model to allow for ghostly users --- .../src/main/scala/github4s/free/domain/PullRequest.scala | 2 +- github4s/shared/src/test/scala/github4s/utils/TestData.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala index 9272b580c..c13664510 100644 --- a/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala +++ b/github4s/shared/src/main/scala/github4s/free/domain/PullRequest.scala @@ -104,7 +104,7 @@ case class NewPullRequestIssue(issue: Int) extends NewPullReques case class PullRequestReview( id: Int, - user: User, + user: Option[User], body: String, commit_id: String, state: PullRequestReviewState, diff --git a/github4s/shared/src/test/scala/github4s/utils/TestData.scala b/github4s/shared/src/test/scala/github4s/utils/TestData.scala index 711c2bc9f..3d9fdd322 100644 --- a/github4s/shared/src/test/scala/github4s/utils/TestData.scala +++ b/github4s/shared/src/test/scala/github4s/utils/TestData.scala @@ -361,7 +361,7 @@ trait TestData extends DummyGithubUrls { val pullRequestReview = PullRequestReview( id = validPullRequestReviewNumber, - user = user, + user = Some(user), body = validCommentBody, commit_id = validCommitSha, state = PRRStateCommented, From abb7a87cb96ce6e8a6f8f1502edacd157069e10a Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Tue, 23 May 2017 07:19:26 +1000 Subject: [PATCH 12/16] Fix copy paste woes --- docs/src/main/tut/pull_request.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/src/main/tut/pull_request.md b/docs/src/main/tut/pull_request.md index 1e6444db3..5127a4f7a 100644 --- a/docs/src/main/tut/pull_request.md +++ b/docs/src/main/tut/pull_request.md @@ -49,7 +49,7 @@ val prFilters = List(PRFilterOpen, PRFilterSortPopularity) val listPullRequests = Github(accessToken).pullRequests.list("scala", "scala", prFilters) listPullRequests.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: s{e.getMessage}") + case Left(e) => println("Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` @@ -71,7 +71,7 @@ To list the files for a pull request: val listPullRequestFiles = Github(accessToken).pullRequests.listFiles("47deg", "github4s", 102) listPullRequestFiles.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: s{e.getMessage}") + case Left(e) => println("Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` @@ -103,7 +103,7 @@ val createPullRequestData = Github(accessToken).pullRequests.create( Some(true)) createPullRequestData.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: s{e.getMessage}") + case Left(e) => println("Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` @@ -123,7 +123,7 @@ val createPullRequestIssue = Github(accessToken).pullRequests.create( Some(true)) createPullRequestIssue.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: s{e.getMessage}") + case Left(e) => println("Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` @@ -148,7 +148,7 @@ val listReviews = Github(accessToken).pullRequests.listReviews( 139) listReviews.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: s{e.getMessage}") + case Left(e) => println("Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` @@ -175,7 +175,7 @@ val review = Github(accessToken).pullRequests.getReview( 39355613) review.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: s{e.getMessage}") + case Left(e) => println("Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` From 9784ec69ee6f6fb7f9ae937dac809b0f55f187ad Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Tue, 23 May 2017 07:29:14 +1000 Subject: [PATCH 13/16] Another minor testing fix --- .../test/scala/github4s/integration/GHPullRequestsSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala b/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala index a1dcde3d4..169f45ddd 100644 --- a/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala +++ b/github4s/shared/src/test/scala/github4s/integration/GHPullRequestsSpec.scala @@ -116,7 +116,7 @@ trait GHPullRequestsSpec[T] extends BaseIntegrationSpec[T] { it should "return error when an invalid repo name is passed" in { val response = Github(accessToken).pullRequests - .getReview(validRepoOwner, validRepoName, validPullRequestNumber, validPullRequestReviewNumber) + .getReview(validRepoOwner, invalidRepoName, validPullRequestNumber, validPullRequestReviewNumber) .execFuture[T](headerUserAgent) testFutureIsLeft(response) From dbbb7b9980a50845d8256910d3926701cbd185e6 Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Tue, 23 May 2017 07:33:49 +1000 Subject: [PATCH 14/16] Fix a regression that somehow made it in --- docs/src/main/tut/pull_request.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/main/tut/pull_request.md b/docs/src/main/tut/pull_request.md index 3e8eba1d8..a6bc6105e 100644 --- a/docs/src/main/tut/pull_request.md +++ b/docs/src/main/tut/pull_request.md @@ -49,7 +49,7 @@ val prFilters = List(PRFilterOpen, PRFilterSortPopularity) val listPullRequests = Github(accessToken).pullRequests.list("scala", "scala", prFilters) listPullRequests.exec[cats.Id, HttpResponse[String]]() match { - case Left(e) => println("Something went wrong: ${e.getMessage}") + case Left(e) => println(s"Something went wrong: ${e.getMessage}") case Right(r) => println(r.result) } ``` From 8652b4b39aedc065dec28736e86acbf2072ebcea Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Tue, 23 May 2017 18:50:37 +1000 Subject: [PATCH 15/16] More minor fixes --- docs/src/main/tut/pull_request.md | 4 ++-- .../test/scala/github4s/utils/MockGithubApiServer.scala | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/docs/src/main/tut/pull_request.md b/docs/src/main/tut/pull_request.md index a6bc6105e..2bbb81c9d 100644 --- a/docs/src/main/tut/pull_request.md +++ b/docs/src/main/tut/pull_request.md @@ -134,7 +134,7 @@ See [the API doc](https://developer.github.com/v3/pulls/#create-a-pull-request) ## List pull request reviews -You can list the reviews for a pull request using `listReviews`, it takes as arguments: +You can list the reviews for a pull request using `listReviews`; it takes as arguments: - the repository coordinates (`owner` and `name` of the repository). - the pull request id. @@ -159,7 +159,7 @@ See [the API doc](https://developer.github.com/v3/pulls/reviews/#list-reviews-on ## Get an individual review -You can get an individual review for a pull request using `getReview`, it takes as arguments: +You can get an individual review for a pull request using `getReview`; it takes as arguments: - the repository coordinates (`owner` and `name` of the repository). - the pull request id. diff --git a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala index d405db51a..a10c4cef4 100644 --- a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala +++ b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala @@ -607,14 +607,6 @@ trait MockGithubApiServer extends MockServerService with FakeResponses with Test .withHeader("Authorization", tokenHeader)) .respond(response.withStatusCode(okStatusCode).withBody(listReviewsValidResponse)) - mockServer - .when( - request - .withMethod("GET") - .withPath(s"/repos/$validRepoOwner/$validRepoName/pulls/$validPullRequestNumber/reviews") - .withHeader(not("Authorization"))) - .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) - mockServer .when( request From 737ac9fc37eff1236a65da5f9c0d6b2b2327096f Mon Sep 17 00:00:00 2001 From: Matthew Scharley Date: Tue, 23 May 2017 18:56:17 +1000 Subject: [PATCH 16/16] Fix regression --- .../jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala index a10c4cef4..f3a77b5c0 100644 --- a/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala +++ b/github4s/jvm/src/test/scala/github4s/utils/MockGithubApiServer.scala @@ -665,7 +665,7 @@ trait MockGithubApiServer extends MockServerService with FakeResponses with Test request .withMethod("POST") .withPath(s"/repos/$validRepoOwner/$validRepoName/issues") - .withHeader("Authorization", tokenHeader)) + .withHeader(not("Authorization"))) .respond(response.withStatusCode(unauthorizedStatusCode).withBody(unauthorizedResponse)) mockServer