finalize the implementation and add tests

modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala

@@ -23,7 +23,6 @@ import com.monovore.decline.Opts.{flag, option, options}
 import com.monovore.decline._
 import org.http4s.Uri
 import org.http4s.syntax.literals._
-import org.scalasteward.core.application.Cli.gitlabMergeRequestApprovalsConfig
 import org.scalasteward.core.application.Config._
 import org.scalasteward.core.data.Resolver
 import org.scalasteward.core.forge.ForgeType
@@ -47,15 +46,15 @@ object Cli {
  val processTimeout = "process-timeout"
  }
 
- implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalsConfig] =
+ implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalRulesCfg] =
  Argument.from("approvals_rule_name=required_approvals") { s =>
  s.split(":").toList match {
  case approvalRuleName :: requiredApprovalsAsString :: Nil =>
  Try(requiredApprovalsAsString.trim.toInt) match {
  case Failure(_) =>
  s"[$requiredApprovalsAsString] is not a valid Integer".invalidNel
  case Success(requiredApprovals) =>
- new MergeRequestApprovalsConfig(approvalRuleName.trim, requiredApprovals).validNel
+ MergeRequestApprovalRulesCfg(approvalRuleName.trim, requiredApprovals).validNel
  }
  case _ =>
  s"The value is expected in the following format: APPROVALS_RULE_NAME:REQUIRED_APPROVALS.".invalidNel
@@ -294,21 +293,22 @@ object Cli {
 
  private val gitlabRequiredReviewers: Opts[Option[Int]] =
  option[Int](
- "gitlabRequiredReviewers",
+ "gitlab-required-reviewers",
  "When set, the number of required reviewers for a merge request will be set to this number (non-negative integer). Is only used in the context of gitlab-merge-when-pipeline-succeeds being enabled, and requires that the configured access token have the appropriate privileges. Also requires a Gitlab Premium subscription."
  ).validate("Required reviewers must be non-negative")(_ >= 0).orNone
 
- private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalsConfig]]] =
- options[MergeRequestApprovalsConfig](
+ private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalRulesCfg]]] =
+ options[MergeRequestApprovalRulesCfg](
  "merge-request-level-approval-rule",
  s"Additional repo config file $multiple"
  )
- // ToDo better message
- .validate("")(_.forall(_.requiredApproves >= 0) == true)
+ .validate("Merge request level required approvals must be non-negative")(
+ _.forall(_.requiredApprovals >= 0) == true
+ )
  .orNone
 
  private val gitlabReviewersAndApprovalsConfig
- : Opts[Option[Either[Int, Nel[MergeRequestApprovalsConfig]]]] =
+ : Opts[Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]]] =
  ((gitlabRequiredReviewers, gitlabMergeRequestApprovalsConfig).tupled.mapValidated {
  case (None, None) => None.validNel
  case (None, Some(gitlabMergeRequestApprovalsConfig)) =>

modules/core/src/main/scala/org/scalasteward/core/application/Config.scala

@@ -156,11 +156,11 @@ object Config {
  final case class GitHubCfg(
  ) extends ForgeSpecificCfg
 
- final case class MergeRequestApprovalsConfig(approvalRuleName: String, requiredApproves: Int)
+ final case class MergeRequestApprovalRulesCfg(approvalRuleName: String, requiredApprovals: Int)
 
  final case class GitLabCfg(
  mergeWhenPipelineSucceeds: Boolean,
- requiredReviewers: Option[Either[Int, Nel[MergeRequestApprovalsConfig]]]
+ requiredApprovals: Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]]
  ) extends ForgeSpecificCfg
 
  final case class GiteaCfg(

modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/GitLabApiAlg.scala

@@ -22,7 +22,7 @@ import io.circe._
 import io.circe.generic.semiauto._
 import io.circe.syntax._
 import org.http4s.{Request, Status, Uri}
-import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalsConfig}
+import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalRulesCfg}
 import org.scalasteward.core.data.Repo
 import org.scalasteward.core.forge.ForgeApiAlg
 import org.scalasteward.core.forge.data._
@@ -152,7 +152,7 @@ private[gitlab] object GitLabJsonCodec {
  Decoder.instance { c =>
  for {
  id <- c.downField("id").as[Int]
- name <- c.downField("string").as[String]
+ name <- c.downField("name").as[String]
  } yield MergeRequestLevelApprovalRuleOut(id, name)
  }
 
@@ -238,7 +238,7 @@ final class GitLabApiAlg[F[_]: Parallel](
  for {
  mr <- mergeRequest
  mrWithStatus <- waitForMergeRequestStatus(mr.iid)
- _ <- gitLabCfg.requiredReviewers match {
+ _ <- gitLabCfg.requiredApprovals match {
  case Some(Right(approvalRules)) =>
  setApprovalRules(repo, mrWithStatus, approvalRules)
  case Some(Left(requiredReviewers)) =>
@@ -302,11 +302,11 @@ final class GitLabApiAlg[F[_]: Parallel](
  private def setApprovalRules(
  repo: Repo,
  mrOut: MergeRequestOut,
- approvalsConfig: Nel[MergeRequestApprovalsConfig]
+ approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg]
  ): F[MergeRequestOut] =
  for {
  _ <- logger.info(
- s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalsConfig"
+ s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalRulesCfg"
  )
  activeApprovalRules <-
  client
@@ -315,34 +315,47 @@ final class GitLabApiAlg[F[_]: Parallel](
  modify(repo)
  )
  .recoverWith { case UnexpectedResponse(_, _, _, status, body) =>
- // ToDo better log
  logger
- .warn(s"Unexpected response setting required reviewers: $status: $body")
+ .warn(s"Unexpected response getting merge request approval rules: $status: $body")
  .as(List.empty)
  }
- approvalRuleNamesFromConfig = approvalsConfig.map(_.approvalRuleName)
- approvalRulesToUpdate = activeApprovalRules.intersect(approvalRuleNamesFromConfig.toList)
+ approvalRulesToUpdate = calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg)
  _ <-
- approvalRulesToUpdate.map { mergeRequestApprovalConfig =>
- client
- .putWithBody[Unit, UpdateMergeRequestLevelApprovalRulePayload](
- url.updateMergeRequestLevelApprovalRule(
- repo,
- mrOut.iid,
- mergeRequestApprovalConfig.id
- ),
- UpdateMergeRequestLevelApprovalRulePayload(mergeRequestApprovalConfig.id),
- modify(repo)
- )
- .recoverWith { case UnexpectedResponse(_, _, _, status, body) =>
- // ToDo better log
- logger
- .warn(s"Unexpected response setting required reviewers: $status: $body")
- .as(List.empty)
- }
+ approvalRulesToUpdate.map { case (approvalRuleCfg, activeRule) =>
+ logger.info(
+ s"Setting required approval count to ${approvalRuleCfg.requiredApprovals} for merge request approval rule '${approvalRuleCfg.approvalRuleName}' on ${mrOut.webUrl}"
+ ) >>
+ client
+ .putWithBody[
+ MergeRequestLevelApprovalRuleOut,
+ UpdateMergeRequestLevelApprovalRulePayload
+ ](
+ url.updateMergeRequestLevelApprovalRule(
+ repo,
+ mrOut.iid,
+ activeRule.id
+ ),
+ UpdateMergeRequestLevelApprovalRulePayload(approvalRuleCfg.requiredApprovals),
+ modify(repo)
+ )
+ .as(())
+ .recoverWith { case UnexpectedResponse(_, _, _, status, body) =>
+ logger
+ .warn(s"Unexpected response setting required approvals: $status: $body")
+ }
  }.sequence
  } yield mrOut
 
+ private[gitlab] def calculateRulesToUpdate(
+ activeApprovalRules: List[MergeRequestLevelApprovalRuleOut],
+ approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg]
+ ): List[(MergeRequestApprovalRulesCfg, MergeRequestLevelApprovalRuleOut)] =
+ activeApprovalRules.flatMap { activeRule =>
+ approvalRulesCfg
+ .find(_.approvalRuleName == activeRule.name)
+ .map(_ -> activeRule)
+ }
+
  private def getUsernameToUserIdsMapping(repo: Repo, usernames: Set[String]): F[Map[String, Int]] =
  usernames.toList
  .parTraverse { username =>

modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala

@@ -17,7 +17,6 @@
 package org.scalasteward.core.forge.gitlab
 
 import org.http4s.Uri
-import org.scalasteward.core.application.Config.MergeRequestApprovalsConfig
 import org.scalasteward.core.data.Repo
 import org.scalasteward.core.forge.data.PullRequestNumber
 import org.scalasteward.core.git.Branch

modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala

@@ -6,9 +6,12 @@ import munit.FunSuite
 import org.http4s.syntax.literals._
 import org.scalasteward.core.application.Cli.EnvVar
 import org.scalasteward.core.application.Cli.ParseResult._
-import org.scalasteward.core.application.Config.StewardUsage
+import org.scalasteward.core.application.Config.{MergeRequestApprovalRulesCfg, StewardUsage}
 import org.scalasteward.core.forge.ForgeType
 import org.scalasteward.core.forge.github.GitHubApp
+import org.scalasteward.core.util.Nel
+import cats.syntax.either._
+
 import scala.concurrent.duration._
 
 class CliTest extends FunSuite {
@@ -63,7 +66,7 @@ class CliTest extends FunSuite {
  assertEquals(obtained.githubApp, Some(GitHubApp(12345678L, File("example_app_key"))))
  assertEquals(obtained.refreshBackoffPeriod, 1.day)
  assert(!obtained.gitLabCfg.mergeWhenPipelineSucceeds)
- assertEquals(obtained.gitLabCfg.requiredReviewers, None)
+ assertEquals(obtained.gitLabCfg.requiredApprovals, None)
  assert(obtained.bitbucketCfg.useDefaultReviewers)
  assert(!obtained.bitbucketServerCfg.useDefaultReviewers)
  }
@@ -151,27 +154,84 @@ class CliTest extends FunSuite {
  assert(clue(obtained).startsWith("Usage"))
  }
 
- test("parseArgs: non-default GitLab arguments") {
+ test("parseArgs: non-default GitLab arguments and required reviewers") {
  val params = minimumRequiredParams ++ List(
  List("--gitlab-merge-when-pipeline-succeeds"),
  List("--gitlab-required-reviewers", "5")
  )
  val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten)
 
  assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds)
- assertEquals(obtained.gitLabCfg.requiredReviewers, Some(5))
+ assertEquals(obtained.gitLabCfg.requiredApprovals, Some(5.asLeft))
  }
 
- test("parseArgs: invalid GitLab required reviewers") {
+ test("parseArgs: non-default GitLab arguments and merge request level approval rule") {
  val params = minimumRequiredParams ++ List(
  List("--gitlab-merge-when-pipeline-succeeds"),
+ List("--merge-request-level-approval-rule", "All eligible users:0")
+ )
+ val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten)
+
+ assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds)
+ assertEquals(
+ obtained.gitLabCfg.requiredApprovals,
+ Some(Nel.one(MergeRequestApprovalRulesCfg("All eligible users", 0)).asRight)
+ )
+ }
+
+ test("parseArgs: multiple Gitlab merge request level approval rule") {
+ val params = minimumRequiredParams ++ List(
+ List("--merge-request-level-approval-rule", "All eligible users:1"),
+ List("--merge-request-level-approval-rule", "Only Main:2")
+ )
+ val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten)
+
+ assertEquals(
+ obtained.gitLabCfg.requiredApprovals,
+ Some(
+ Nel
+ .of(
+ MergeRequestApprovalRulesCfg("All eligible users", 1),
+ MergeRequestApprovalRulesCfg("Only Main", 2)
+ )
+ .asRight
+ )
+ )
+ }
+
+ test("parseArgs: only allow one way to define Gitlab required approvals arguments") {
+ val params = minimumRequiredParams ++ List(
+ List("--merge-request-level-approval-rule", "All eligible users:0"),
+ List("--gitlab-required-reviewers", "5")
+ )
+ val Error(errorMsg) = Cli.parseArgs(params.flatten)
+
+ assert(
+ clue(errorMsg).startsWith(
+ "You can't use both --gitlabRequiredReviewers and --merge-request-level-approval-rule at the same time"
+ )
+ )
+
+ }
+
+ test("parseArgs: invalid GitLab required reviewers") {
+ val params = minimumRequiredParams ++ List(
  List("--gitlab-required-reviewers", "-3")
  )
  val Error(errorMsg) = Cli.parseArgs(params.flatten)
 
  assert(clue(errorMsg).startsWith("Required reviewers must be non-negative"))
  }
 
+ test("parseArgs: invalid GitLab merge request level approval rule") {
+ val params = minimumRequiredParams ++ List(
+ List("--merge-request-level-approval-rule", "All eligible users:-3")
+ )
+ val Error(errorMsg) = Cli.parseArgs(params.flatten)
+
+ assert(clue(errorMsg).startsWith("Merge request level required approvals must be non-negative"))
+ }
+
  test("parseArgs: validate-repo-config") {
  val Success(StewardUsage.ValidateRepoConfig(file)) = Cli.parseArgs(
  List(

modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala

@@ -0,0 +1,69 @@
+package org.scalasteward.core.forge.gitlab
+
+import munit.CatsEffectSuite
+import org.http4s.Request
+import org.scalasteward.core.TestInstances.ioLogger
+import org.scalasteward.core.application.Config.{GitLabCfg, MergeRequestApprovalRulesCfg}
+import org.scalasteward.core.data.Repo
+import org.scalasteward.core.forge.ForgeType
+import org.scalasteward.core.mock.MockConfig.config
+import org.scalasteward.core.mock.MockContext.context.httpJsonClient
+import org.scalasteward.core.mock.MockEff
+import org.scalasteward.core.util.Nel
+
+class GitLabAlgTest extends CatsEffectSuite {
+
+ private val gitlabApiAlg = new GitLabApiAlg[MockEff](
+ forgeCfg = config.forgeCfg.copy(tpe = ForgeType.GitLab),
+ gitLabCfg = GitLabCfg(mergeWhenPipelineSucceeds = false, requiredApprovals = None),
+ modify = (_: Repo) => (request: Request[MockEff]) => MockEff.pure(request)
+ )
+
+ test(
+ "calculateRulesToUpdate -- ignore active approval rule that doesn't have approval rule configuration"
+ ) {
+ val activeApprovalRules =
+ List(
+ MergeRequestLevelApprovalRuleOut(name = "A", id = 101),
+ MergeRequestLevelApprovalRuleOut(name = "B", id = 201)
+ )
+ val approvalRulesCfg =
+ Nel.one(MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 1))
+
+ val result =
+ gitlabApiAlg.calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg)
+ val expectedResult =
+ List(
+ (
+ MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 1),
+ MergeRequestLevelApprovalRuleOut(id = 201, name = "B")
+ )
+ )
+
+ assertEquals(result, expectedResult)
+ }
+
+ test(
+ "calculateRulesToUpdate -- ignore approval rule configuration that doesn't have active approval rule"
+ ) {
+ val activeApprovalRules =
+ List(MergeRequestLevelApprovalRuleOut(name = "A", id = 101))
+ val approvalRulesCfg =
+ Nel.of(
+ MergeRequestApprovalRulesCfg(approvalRuleName = "A", requiredApprovals = 1),
+ MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 2)
+ )
+
+ val result =
+ gitlabApiAlg.calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg)
+ val expectedResult =
+ List(
+ (
+ MergeRequestApprovalRulesCfg(approvalRuleName = "A", requiredApprovals = 1),
+ MergeRequestLevelApprovalRuleOut(name = "A", id = 101)
+ )
+ )
+
+ assertEquals(result, expectedResult)
+ }
+}