Merge pull request #3000 from danielleontiev/optimize-lables-adding

fthomas · web-flow · commit 5357f2879b8e · 2023-03-06T15:06:36.000+01:00
refactoring: remove ForgeApiAlg.labelPullRequest
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/ForgeApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/ForgeApiAlg.scala
@@ -40,8 +40,6 @@ trait ForgeApiAlg[F[_]] {
 
   def commentPullRequest(repo: Repo, number: PullRequestNumber, comment: String): F[Comment]
 
-  def labelPullRequest(repo: Repo, number: PullRequestNumber, labels: List[String]): F[Unit]
-
   final def createForkOrGetRepo(repo: Repo, doNotFork: Boolean): F[RepoOut] =
     if (doNotFork) getRepo(repo) else createFork(repo)
 
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/azurerepos/AzureReposApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/azurerepos/AzureReposApiAlg.scala
@@ -18,8 +18,6 @@ package org.scalasteward.core.forge.azurerepos
 
 import cats.MonadThrow
 import cats.syntax.all._
-import io.circe.Json
-import io.circe.syntax.KeyOps
 import org.http4s.{Request, Uri}
 import org.scalasteward.core.application.Config.AzureReposCfg
 import org.scalasteward.core.data.Repo
@@ -90,20 +88,6 @@ final class AzureReposApiAlg[F[_]](
       modify(repo)
     )
 
-  // https://docs.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-labels/create?view=azure-devops-rest-7.1
-  override def labelPullRequest(
-      repo: Repo,
-      number: PullRequestNumber,
-      labels: List[String]
-  ): F[Unit] =
-    client
-      .postWithBody[Json, Json](
-        url.labelPullRequest(repo, number),
-        Json.obj("name" := labels.mkString("-")),
-        modify(repo)
-      )
-      .void
-
   private def warnIfAssigneesAreUsed =
     logger.warn("assignees are not supported by AzureRepos")
 
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/azurerepos/JsonCodec.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/azurerepos/JsonCodec.scala
@@ -28,6 +28,7 @@ final private[azurerepos] case class PullRequestPayload(
     sourceRefName: String,
     targetRefName: String,
     title: String,
+    labels: Option[List[String]],
     description: String
 )
 
@@ -39,6 +40,7 @@ private[azurerepos] object PullRequestPayload {
       sourceRefName = withPrefix(data.head),
       targetRefName = data.base.name,
       title = data.title,
+      labels = Option.when(data.labels.nonEmpty)(data.labels),
       description = data.body
     )
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/azurerepos/Url.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/azurerepos/Url.scala
@@ -49,7 +49,4 @@ class Url(apiHost: Uri, organization: String) {
 
   def commentPullRequest(repo: Repo, number: PullRequestNumber): Uri =
     pullRequests(repo) / number.value / "threads"
-
-  def labelPullRequest(repo: Repo, number: PullRequestNumber): Uri =
-    pullRequests(repo) / number.value / "labels"
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/bitbucket/BitbucketApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/bitbucket/BitbucketApiAlg.scala
@@ -92,6 +92,7 @@ class BitbucketApiAlg[F[_]](
     for {
       _ <- F.whenA(data.assignees.nonEmpty)(warnIfAssigneesAreUsed)
       _ <- F.whenA(data.reviewers.nonEmpty)(warnIfReviewersAreUsed)
+      _ <- F.whenA(data.labels.nonEmpty)(warnIfLabelsAreUsed)
       pullRequestOut <- create
     } yield pullRequestOut
   }
@@ -132,11 +133,7 @@ class BitbucketApiAlg[F[_]](
       )
       .map((cc: CreateComment) => Comment(cc.content.raw))
 
-  override def labelPullRequest(
-      repo: Repo,
-      number: PullRequestNumber,
-      labels: List[String]
-  ): F[Unit] =
+  private def warnIfLabelsAreUsed =
     logger.warn(
       "Bitbucket does not support PR labels, remove --add-labels to make this warning disappear"
     )
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/bitbucketserver/BitbucketServerApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/bitbucketserver/BitbucketServerApiAlg.scala
@@ -70,6 +70,7 @@ final class BitbucketServerApiAlg[F[_]](
       reviewers <- useDefaultReviewers(repo)
       _ <- F.whenA(data.assignees.nonEmpty)(warnIfAssigneesAreUsed)
       _ <- F.whenA(data.reviewers.nonEmpty)(warnIfReviewersAreUsed)
+      _ <- F.whenA(data.labels.nonEmpty)(warnIfLabelsAreUsed)
       req = Json.NewPR(
         title = data.title,
         description = data.body,
@@ -121,11 +122,7 @@ final class BitbucketServerApiAlg[F[_]](
       .get[Json.Page[Json.PR]](url.listPullRequests(repo, s"refs/heads/$head"), modify(repo))
       .map(_.values.map(_.toPullRequestOut))
 
-  override def labelPullRequest(
-      repo: Repo,
-      number: PullRequestNumber,
-      labels: List[String]
-  ): F[Unit] =
+  private def warnIfLabelsAreUsed =
     logger.warn(
       "Bitbucket does not support PR labels, remove --add-labels to make this warning disappear"
     )
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala
@@ -223,6 +223,7 @@ object NewPullRequestData {
       artifactIdToUrl: Map[String, Uri] = Map.empty,
       artifactIdToUpdateInfoUrls: Map[String, List[UpdateInfoUrl]] = Map.empty,
       filesWithOldVersion: List[String] = List.empty,
+      addLabels: Boolean = false,
       labels: List[String] = List.empty
   ): NewPullRequestData =
     NewPullRequestData(
@@ -243,7 +244,7 @@ object NewPullRequestData {
       ),
       head = branchName,
       base = data.baseBranch,
-      labels = labels,
+      labels = if (addLabels) labels else List.empty,
       assignees = data.repoConfig.assignees,
       reviewers = data.repoConfig.reviewers
     )
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitea/GiteaApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitea/GiteaApiAlg.scala
@@ -255,25 +255,6 @@ final class GiteaApiAlg[F[_]: HttpJsonClient](
       }
   }
 
-  override def labelPullRequest(
-      repo: Repo,
-      number: PullRequestNumber,
-      labels: List[String]
-  ): F[Unit] = {
-    def attachLabels(labels: Vector[Int]): F[Unit] =
-      if (labels.nonEmpty)
-        client
-          .postWithBody[Vector[Label], AttachLabelReq](
-            url.pullRequestLabels(repo, number),
-            AttachLabelReq(labels),
-            modify(repo)
-          )
-          .void
-      else ().pure[F]
-
-    getOrCreateLabel(repo, labels.toVector) >>= attachLabels
-  }
-
   def getOrCreateLabel(repo: Repo, labels: Vector[String]): F[Vector[Int]] =
     listLabels(repo).flatMap { repoLabels =>
       val existing = repoLabels.filter(label => labels.contains(label.name))
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/github/GitHubApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/github/GitHubApiAlg.scala
@@ -53,6 +53,7 @@ final class GitHubApiAlg[F[_]](
 
     for {
       pullRequestOut <- create
+      _ <- F.whenA(data.labels.nonEmpty)(labelPullRequest(repo, pullRequestOut.number, data.labels))
       _ <-
         F.whenA(data.assignees.nonEmpty)(addAssignees(repo, pullRequestOut.number, data.assignees))
       _ <-
@@ -92,7 +93,7 @@ final class GitHubApiAlg[F[_]](
       .postWithBody(url.comments(repo, number), Comment(comment), modify(repo))
 
   /** https://docs.github.com/en/rest/reference/issues#add-labels-to-an-issue */
-  override def labelPullRequest(
+  private def labelPullRequest(
       repo: Repo,
       number: PullRequestNumber,
       labels: List[String]
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala
@@ -36,6 +36,7 @@ final private[gitlab] case class MergeRequestPayload(
     id: String,
     title: String,
     description: String,
+    labels: Option[List[String]],
     assignee_ids: Option[List[Int]],
     reviewer_ids: Option[List[Int]],
     target_project_id: Long,
@@ -58,6 +59,7 @@ private[gitlab] object MergeRequestPayload {
       description = data.body,
       assignee_ids = Option.when(assignees.nonEmpty)(assignees),
       reviewer_ids = Option.when(reviewers.nonEmpty)(reviewers),
+      labels = Option.when(data.labels.nonEmpty)(data.labels),
       target_project_id = projectId,
       source_branch = data.head,
       target_branch = data.base
@@ -329,17 +331,4 @@ final class GitLabApiAlg[F[_]: Parallel](
   ): F[Comment] =
     client.postWithBody(url.comments(repo, number), Comment(comment), modify(repo))
 
-  // https://docs.gitlab.com/ee/api/merge_requests.html#update-mr
-  override def labelPullRequest(
-      repo: Repo,
-      number: PullRequestNumber,
-      labels: List[String]
-  ): F[Unit] =
-    client
-      .putWithBody[Json, Json](
-        url.existingMergeRequest(repo, number),
-        Json.obj("labels" := labels.mkString(",")),
-        modify(repo)
-      )
-      .void
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala
@@ -234,13 +234,14 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
       allLabels = labelsFor(data.update, edits, filesWithOldVersion, artifactIdToVersionScheme)
       labels = filterLabels(allLabels, data.repoData.config.pullRequests.includeMatchedLabels)
       requestData = NewPullRequestData.from(
-        data,
-        branchName,
-        edits,
-        artifactIdToUrl,
-        artifactIdToUpdateInfoUrls.toMap,
-        filesWithOldVersion,
-        labels
+        data = data,
+        branchName = branchName,
+        edits = edits,
+        artifactIdToUrl = artifactIdToUrl,
+        artifactIdToUpdateInfoUrls = artifactIdToUpdateInfoUrls.toMap,
+        filesWithOldVersion = filesWithOldVersion,
+        addLabels = config.addLabels,
+        labels = labels
       )
     } yield requestData
 
@@ -249,9 +250,6 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
       _ <- logger.info(s"Create PR ${data.updateBranch.name}")
       requestData <- preparePullRequest(data, edits)
       pr <- forgeApiAlg.createPullRequest(data.repo, requestData)
-      _ <- forgeApiAlg
-        .labelPullRequest(data.repo, pr.number, requestData.labels)
-        .whenA(config.addLabels && requestData.labels.nonEmpty)
       prData = PullRequestData[Id](
         pr.html_url,
         data.baseSha1,
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/azurerepos/AzureReposApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/azurerepos/AzureReposApiAlgTest.scala
@@ -266,10 +266,4 @@ class AzureReposApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] {
     assertIO(obtained, expected)
   }
 
-  test("labelPullRequest") {
-    azureReposApiAlg
-      .labelPullRequest(repo, PullRequestNumber(26), List("dependency-updates"))
-      .runA(state)
-      .assert
-  }
 }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/azurerepos/JsonCodecTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/azurerepos/JsonCodecTest.scala
@@ -0,0 +1,49 @@
+package org.scalasteward.core.forge.azurerepos
+
+import io.circe.literal._
+import io.circe.syntax._
+import munit.FunSuite
+import org.scalasteward.core.forge.data.NewPullRequestData
+import org.scalasteward.core.git.Branch
+
+class JsonCodecTest extends FunSuite {
+  import JsonCodec._
+
+  private val data = NewPullRequestData(
+    title = "Test MR title",
+    body = "Test MR body",
+    head = "source",
+    base = Branch("main"),
+    labels = Nil,
+    assignees = Nil,
+    reviewers = Nil
+  )
+
+  test("PullRequestPayload") {
+    val obtained = PullRequestPayload.from(data).asJson
+    val expected =
+      json"""{
+               "sourceRefName" : "refs/heads/source",
+               "targetRefName" : "main",
+               "title" : "Test MR title",
+               "labels" : null,
+               "description" : "Test MR body"
+             }"""
+
+    assertEquals(obtained, expected)
+  }
+
+  test("PullRequestPayload with labels") {
+    val obtained = PullRequestPayload.from(data.copy(labels = List("foo", "bar"))).asJson
+    val expected =
+      json"""{
+               "sourceRefName" : "refs/heads/source",
+               "targetRefName" : "main",
+               "title" : "Test MR title",
+               "labels" : [ "foo", "bar" ],
+               "description" : "Test MR body"
+             }"""
+
+    assertEquals(obtained, expected)
+  }
+}
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/azurerepos/UrlTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/azurerepos/UrlTest.scala
@@ -60,11 +60,4 @@ class UrlTest extends FunSuite {
     )
   }
 
-  test("labelPullRequest") {
-    assertEquals(
-      url.labelPullRequest(repo, PullRequestNumber(42)).toString,
-      "https://dev.azure.com/my-azure-org/scala-steward-org/" +
-        "_apis%2Fgit%2Frepositories/scala-steward/pullrequests/42/labels?api-version=7.1-preview.1"
-    )
-  }
 }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/data/NewPullRequestDataTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/data/NewPullRequestDataTest.scala
@@ -572,6 +572,7 @@ class NewPullRequestDataTest extends FunSuite {
     val obtained = from(
       data,
       "scala-steward:update/logback-classic-1.2.3",
+      addLabels = true,
       labels = labelsFor(data.update)
     )
 
@@ -643,6 +644,7 @@ class NewPullRequestDataTest extends FunSuite {
     val obtained = from(
       data,
       "scala-steward:update/logback-classic-1.2.3",
+      addLabels = true,
       labels = labelsFor(data.update)
     )
 
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitea/GiteaApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitea/GiteaApiAlgTest.scala
@@ -191,12 +191,6 @@ class GiteaApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] {
       }
   }
 
-  test("label pr") {
-    giteaAlg
-      .labelPullRequest(repo, PullRequestNumber(2), List("label1"))
-      .runA(state)
-  }
-
   test("create fork") {
     giteaAlg
       .createFork(repo)
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/github/GitHubApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/github/GitHubApiAlgTest.scala
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlgTest.scala
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/MergeRequestPayloadTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/MergeRequestPayloadTest.scala
diff --git a/modules/core/src/test/scala/org/scalasteward/core/mock/MockConfig.scala b/modules/core/src/test/scala/org/scalasteward/core/mock/MockConfig.scala
diff --git a/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala b/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
diff --git a/modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ final private[azurerepos] case class PullRequestPayload(`
`28`	`28`	`sourceRefName: String,`
`29`	`29`	`targetRefName: String,`
`30`	`30`	`title: String,`
	`31`	`+ labels: Option[List[String]],`
`31`	`32`	`description: String`
`32`	`33`	`)`
`33`	`34`
`@@ -39,6 +40,7 @@ private[azurerepos] object PullRequestPayload {`
`39`	`40`	`sourceRefName = withPrefix(data.head),`
`40`	`41`	`targetRefName = data.base.name,`
`41`	`42`	`title = data.title,`
	`43`	`+ labels = Option.when(data.labels.nonEmpty)(data.labels),`
`42`	`44`	`description = data.body`
`43`	`45`	`)`
`44`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,4 @@ class Url(apiHost: Uri, organization: String) {`
`49`	`49`
`50`	`50`	`def commentPullRequest(repo: Repo, number: PullRequestNumber): Uri =`
`51`	`51`	`pullRequests(repo) / number.value / "threads"`
`52`		`-`
`53`		`- def labelPullRequest(repo: Repo, number: PullRequestNumber): Uri =`
`54`		`- pullRequests(repo) / number.value / "labels"`
`55`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -92,6 +92,7 @@ class BitbucketApiAlg[F[_]](`
`92`	`92`	`for {`
`93`	`93`	`_ <- F.whenA(data.assignees.nonEmpty)(warnIfAssigneesAreUsed)`
`94`	`94`	`_ <- F.whenA(data.reviewers.nonEmpty)(warnIfReviewersAreUsed)`
	`95`	`+ _ <- F.whenA(data.labels.nonEmpty)(warnIfLabelsAreUsed)`
`95`	`96`	`pullRequestOut <- create`
`96`	`97`	`} yield pullRequestOut`
`97`	`98`	`}`
`@@ -132,11 +133,7 @@ class BitbucketApiAlg[F[_]](`
`132`	`133`	`)`
`133`	`134`	`.map((cc: CreateComment) => Comment(cc.content.raw))`
`134`	`135`
`135`		`- override def labelPullRequest(`
`136`		`- repo: Repo,`
`137`		`- number: PullRequestNumber,`
`138`		`- labels: List[String]`
`139`		`- ): F[Unit] =`
	`136`	`+ private def warnIfLabelsAreUsed =`
`140`	`137`	`logger.warn(`
`141`	`138`	`"Bitbucket does not support PR labels, remove --add-labels to make this warning disappear"`
`142`	`139`	`)`