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

refactoring: remove ForgeApiAlg.labelPullRequest

Author: fthomas

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)
 

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")
 

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
     )
 }

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"
 }

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"
     )

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"
     )

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
     )

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))

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]

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
 }

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,

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
-  }
 }

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)
+  }
+}

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"
-    )
-  }
 }

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)
     )
 

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)