diff --git a/docs/help.md b/docs/help.md index 78ea0316a9..3b90c02a56 100644 --- a/docs/help.md +++ b/docs/help.md @@ -5,7 +5,7 @@ All command line arguments for the `scala-steward` application. ``` Usage: scala-steward validate-repo-config - scala-steward --workspace --repos-file [--git-author-name ] --git-author-email [--git-author-signing-key ] --git-ask-pass [--sign-commits] [--forge-type ] [--forge-api-host ] --forge-login [--do-not-fork] [--add-labels] [--ignore-opts-files] [--env-var ]... [--process-timeout ] [--whitelist ]... [--read-only ]... [--enable-sandbox | --disable-sandbox] [--max-buffer-size ] [--repo-config ]... [--disable-default-repo-config] [--scalafix-migrations ]... [--disable-default-scalafix-migrations] [--artifact-migrations ]... [--disable-default-artifact-migrations] [--cache-ttl ] [--bitbucket-use-default-reviewers] [--bitbucket-server-use-default-reviewers] [--gitlab-merge-when-pipeline-succeeds] [--gitlab-required-reviewers ] [--gitlab-remove-source-branch] [--azure-repos-organization ] [--github-app-id --github-app-key-file ] [--url-checker-test-url ]... [--default-maven-repo ] [--refresh-backoff-period ] + scala-steward --workspace --repos-file [--git-author-name ] --git-author-email [--git-author-signing-key ] --git-ask-pass [--sign-commits] [--forge-type ] [--forge-api-host ] --forge-login [--do-not-fork] [--add-labels] [--ignore-opts-files] [--env-var ]... [--process-timeout ] [--whitelist ]... [--read-only ]... [--enable-sandbox | --disable-sandbox] [--max-buffer-size ] [--repo-config ]... [--disable-default-repo-config] [--scalafix-migrations ]... [--disable-default-scalafix-migrations] [--artifact-migrations ]... [--disable-default-artifact-migrations] [--cache-ttl ] [--bitbucket-use-default-reviewers] [--bitbucket-server-use-default-reviewers] [--gitlab-merge-when-pipeline-succeeds] [--gitlab-required-reviewers ] [--merge-request-level-approval-rule ]... [--gitlab-remove-source-branch] [--azure-repos-organization ] [--github-app-id --github-app-key-file ] [--url-checker-test-url ]... [--default-maven-repo ] [--refresh-backoff-period ] @@ -80,6 +80,8 @@ Options and flags: Whether to merge a gitlab merge request when the pipeline succeeds --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. + --merge-request-level-approval-rule + Additional repo config file (can be used multiple times) --gitlab-remove-source-branch Flag indicating if a merge request should remove the source branch when merging. --azure-repos-organization diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala index 3bbccaa37d..11b821f63f 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala @@ -31,7 +31,9 @@ import org.scalasteward.core.forge.github.GitHubApp import org.scalasteward.core.git.Author import org.scalasteward.core.util.Nel import org.scalasteward.core.util.dateTime.renderFiniteDuration + import scala.concurrent.duration._ +import scala.util.{Failure, Success, Try} object Cli { final case class EnvVar(name: String, value: String) @@ -44,6 +46,25 @@ object Cli { val processTimeout = "process-timeout" } + implicit val mergeRequestApprovalsCfgArgument: Argument[MergeRequestApprovalRulesCfg] = + Argument.from("approvals_rule_name=required_approvals") { s => + s.trim.split("=").toList match { + case approvalRuleName :: requiredApprovalsAsString :: Nil => + Try(requiredApprovalsAsString.trim.toInt) match { + case Failure(_) => + Validated.invalidNel(s"[$requiredApprovalsAsString] is not a valid Integer") + case Success(requiredApprovals) => + Validated.valid( + MergeRequestApprovalRulesCfg(approvalRuleName.trim, requiredApprovals) + ) + } + case _ => + Validated.invalidNel( + "The value is expected in the following format: APPROVALS_RULE_NAME=REQUIRED_APPROVALS" + ) + } + } + implicit val envVarArgument: Argument[EnvVar] = Argument.from("name=value") { s => s.trim.split('=').toList match { @@ -286,10 +307,32 @@ object Cli { "Flag indicating if a merge request should remove the source branch when merging." ).orFalse - private val gitLabCfg: Opts[GitLabCfg] = - (gitlabMergeWhenPipelineSucceeds, gitlabRequiredReviewers, gitlabRemoveSourceBranch).mapN( - GitLabCfg.apply + private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalRulesCfg]]] = + options[MergeRequestApprovalRulesCfg]( + "merge-request-level-approval-rule", + s"Additional repo config file $multiple" ) + .validate("Merge request level required approvals must be non-negative")( + _.forall(_.requiredApprovals >= 0) == true + ) + .orNone + + private val gitlabReviewersAndApprovalsConfig + : Opts[Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]]] = + ((gitlabRequiredReviewers, gitlabMergeRequestApprovalsConfig).tupled.mapValidated { + case (None, None) => None.validNel + case (None, Some(gitlabMergeRequestApprovalsConfig)) => + Some(gitlabMergeRequestApprovalsConfig.asRight[Int]).validNel + case (Some(requiredReviewers), None) => Some(Left(requiredReviewers)).validNel + case (Some(_), Some(_)) => + s"You can't use both --gitlab-required-reviewers and --merge-request-level-approval-rule at the same time".invalidNel + }) + + private val gitLabCfg: Opts[GitLabCfg] = + (gitlabMergeWhenPipelineSucceeds, gitlabReviewersAndApprovalsConfig, gitlabRemoveSourceBranch) + .mapN( + GitLabCfg.apply + ) private val githubAppId: Opts[Long] = option[Long]( diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala index 2ad3116a02..bf5688478b 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/Config.scala @@ -156,9 +156,11 @@ object Config { final case class GitHubCfg( ) extends ForgeSpecificCfg + final case class MergeRequestApprovalRulesCfg(approvalRuleName: String, requiredApprovals: Int) + final case class GitLabCfg( mergeWhenPipelineSucceeds: Boolean, - requiredReviewers: Option[Int], + requiredApprovals: Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]], removeSourceBranch: Boolean ) extends ForgeSpecificCfg 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 index 8ec4d09efc..b30345897e 100644 --- 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 @@ -23,13 +23,18 @@ 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} +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._ import org.scalasteward.core.git.{Branch, Sha1} import org.scalasteward.core.util.uri.uriDecoder -import org.scalasteward.core.util.{intellijThisImportIsUsed, HttpJsonClient, UnexpectedResponse} +import org.scalasteward.core.util.{ + intellijThisImportIsUsed, + HttpJsonClient, + Nel, + UnexpectedResponse +} import org.typelevel.log4cats.Logger import scala.concurrent.duration.{Duration, DurationInt} @@ -48,6 +53,8 @@ final private[gitlab] case class MergeRequestPayload( target_branch: Branch ) +final private[gitlab] case class UpdateMergeRequestLevelApprovalRulePayload(approvals_required: Int) + private[gitlab] object MergeRequestPayload { def apply( id: String, @@ -87,6 +94,11 @@ final private[gitlab] case class MergeRequestApprovalsOut( approvalsRequired: Int ) +final private[gitlab] case class MergeRequestLevelApprovalRuleOut( + id: Int, + name: String +) + final private[gitlab] case class CommitId(id: Sha1) { val commitOut: CommitOut = CommitOut(id) } @@ -102,6 +114,8 @@ private[gitlab] object GitLabJsonCodec { intellijThisImportIsUsed(uriDecoder) implicit val forkPayloadEncoder: Encoder[ForkPayload] = deriveEncoder + implicit val updateMergeRequestLevelApprovalRulePayloadEncoder + : Encoder[UpdateMergeRequestLevelApprovalRulePayload] = deriveEncoder implicit val userOutDecoder: Decoder[UserOut] = Decoder.instance { _.downField("username").as[String].map(UserOut(_)) } @@ -140,6 +154,14 @@ private[gitlab] object GitLabJsonCodec { } yield MergeRequestApprovalsOut(requiredReviewers) } + implicit val mergeRequestLevelApprovalRuleOutDecoder: Decoder[MergeRequestLevelApprovalRuleOut] = + Decoder.instance { c => + for { + id <- c.downField("id").as[Int] + name <- c.downField("name").as[String] + } yield MergeRequestLevelApprovalRuleOut(id, name) + } + implicit val projectIdDecoder: Decoder[ProjectId] = deriveDecoder implicit val mergeRequestPayloadEncoder: Encoder[MergeRequestPayload] = deriveEncoder[MergeRequestPayload].mapJson(_.dropNullValues) @@ -240,7 +262,13 @@ final class GitLabApiAlg[F[_]: Parallel]( for { mr <- mergeRequest mrWithStatus <- waitForMergeRequestStatus(mr.iid) - _ <- maybeSetReviewers(repo, mrWithStatus) + _ <- gitLabCfg.requiredApprovals match { + case Some(Right(approvalRules)) => + setApprovalRules(repo, mrWithStatus, approvalRules) + case Some(Left(requiredReviewers)) => + setReviewers(repo, mrWithStatus, requiredReviewers) + case None => F.unit + } mergedUponSuccess <- mergePipelineUponSuccess(repo, mrWithStatus) } yield mergedUponSuccess } @@ -270,28 +298,86 @@ final class GitLabApiAlg[F[_]: Parallel]( case mr => logger.info(s"Unable to automatically merge ${mr.webUrl}").map(_ => mr) } + import cats.implicits._ - private def maybeSetReviewers(repo: Repo, mrOut: MergeRequestOut): F[MergeRequestOut] = - gitLabCfg.requiredReviewers match { - case Some(requiredReviewers) => - for { - _ <- logger.info( - s"Setting number of required reviewers on ${mrOut.webUrl} to $requiredReviewers" + private def setReviewers( + repo: Repo, + mrOut: MergeRequestOut, + requiredReviewers: Int + ): F[MergeRequestOut] = + for { + _ <- logger.info( + s"Setting number of required reviewers on ${mrOut.webUrl} to $requiredReviewers" + ) + _ <- + client + .put[MergeRequestApprovalsOut]( + url.requiredApprovals(repo, mrOut.iid, requiredReviewers), + modify(repo) ) - _ <- + .map(_ => ()) + .recoverWith { case UnexpectedResponse(_, _, _, status, body) => + logger + .warn(s"Unexpected response setting required reviewers: $status: $body") + .as(()) + } + } yield mrOut + + private def setApprovalRules( + repo: Repo, + mrOut: MergeRequestOut, + approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg] + ): F[MergeRequestOut] = + for { + _ <- logger.info( + s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalRulesCfg" + ) + activeApprovalRules <- + client + .get[List[MergeRequestLevelApprovalRuleOut]]( + url.listMergeRequestLevelApprovalRules(repo, mrOut.iid), + modify(repo) + ) + .recoverWith { case UnexpectedResponse(_, _, _, status, body) => + logger + .warn(s"Unexpected response getting merge request approval rules: $status: $body") + .as(List.empty) + } + approvalRulesToUpdate = calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg) + _ <- + 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 - .put[MergeRequestApprovalsOut]( - url.requiredApprovals(repo, mrOut.iid, requiredReviewers), + .putWithBody[ + MergeRequestLevelApprovalRuleOut, + UpdateMergeRequestLevelApprovalRulePayload + ]( + url.updateMergeRequestLevelApprovalRule( + repo, + mrOut.iid, + activeRule.id + ), + UpdateMergeRequestLevelApprovalRulePayload(approvalRuleCfg.requiredApprovals), modify(repo) ) - .map(_ => ()) + .as(()) .recoverWith { case UnexpectedResponse(_, _, _, status, body) => logger - .warn(s"Unexpected response setting required reviewers: $status: $body") - .as(()) + .warn(s"Unexpected response setting required approvals: $status: $body") } - } yield mrOut - case None => F.pure(mrOut) + }.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]] = diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala index 6f12fb90ae..cefaf4078c 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/forge/gitlab/Url.scala @@ -47,6 +47,15 @@ class Url(apiHost: Uri) { (existingMergeRequest(repo, number) / "approvals") .withQueryParam("approvals_required", approvalsRequired) + def listMergeRequestLevelApprovalRules(repo: Repo, number: PullRequestNumber): Uri = + existingMergeRequest(repo, number) / "approval_rules" + + def updateMergeRequestLevelApprovalRule( + repo: Repo, + number: PullRequestNumber, + approvalRuleId: Int + ): Uri = existingMergeRequest(repo, number) / "approval_rules" / approvalRuleId + def listMergeRequests(repo: Repo, source: String, target: String): Uri = mergeRequest(repo) .withQueryParam("source_branch", source) diff --git a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala index 6093cdd3fa..aa5800325d 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala @@ -1,14 +1,18 @@ package org.scalasteward.core.application import better.files.File +import cats.data.Validated import cats.data.Validated.Valid 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 +67,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,20 +155,72 @@ 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-remove-source-branch"), List("--gitlab-required-reviewers", "5") ) val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds) - assertEquals(obtained.gitLabCfg.requiredReviewers, Some(5)) + assert(obtained.gitLabCfg.removeSourceBranch) + 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("--gitlab-remove-source-branch"), + List("--merge-request-level-approval-rule", "All eligible users=0") + ) + val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten) + + assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds) + assert(obtained.gitLabCfg.removeSourceBranch) + 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 --gitlab-required-reviewers 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) @@ -172,6 +228,15 @@ class CliTest extends FunSuite { 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( @@ -240,4 +305,21 @@ class CliTest extends FunSuite { ) assert(error.startsWith("Missing value for option: --azure-repos-organization")) } + + test("mergeRequestApprovalsConfigArgument: without equals sign") { + assertEquals( + Cli.mergeRequestApprovalsCfgArgument.read("only-main"), + Validated.invalidNel( + s"The value is expected in the following format: APPROVALS_RULE_NAME=REQUIRED_APPROVALS" + ) + ) + } + + test("mergeRequestApprovalsConfigArgument: non-integer required approvals") { + val nonIntegerRequiredApprovals = "two" + assertEquals( + Cli.mergeRequestApprovalsCfgArgument.read(s"only-main=$nonIntegerRequiredApprovals"), + Validated.invalidNel(s"[$nonIntegerRequiredApprovals] is not a valid Integer") + ) + } } diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala new file mode 100644 index 0000000000..1ff86842a9 --- /dev/null +++ b/modules/core/src/test/scala/org/scalasteward/core/forge/gitlab/GitLabAlgTest.scala @@ -0,0 +1,73 @@ +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, + removeSourceBranch = true + ), + 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) + } +} 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 index b6a224456a..906758bbeb 100644 --- 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 @@ -1,5 +1,6 @@ package org.scalasteward.core.forge.gitlab +import cats.syntax.either._ import cats.syntax.semigroupk._ import io.circe.Json import io.circe.literal._ @@ -12,7 +13,7 @@ import org.http4s.headers.Allow import org.http4s.implicits._ import org.scalasteward.core.TestInstances.{dummyRepoCache, ioLogger} import org.scalasteward.core.TestSyntax._ -import org.scalasteward.core.application.Config.GitLabCfg +import org.scalasteward.core.application.Config.{GitLabCfg, MergeRequestApprovalRulesCfg} import org.scalasteward.core.data.{Repo, RepoData, UpdateData} import org.scalasteward.core.forge.data._ import org.scalasteward.core.forge.gitlab.GitLabJsonCodec._ @@ -22,6 +23,7 @@ import org.scalasteward.core.mock.MockConfig.config import org.scalasteward.core.mock.MockContext.context.httpJsonClient import org.scalasteward.core.mock.{MockEff, MockState} import org.scalasteward.core.repoconfig.RepoConfig +import org.scalasteward.core.util.Nel import org.typelevel.ci.CIStringSyntax class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { @@ -103,6 +105,16 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { ) ) + case GET -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" => + Ok(getMrApprovalRules) + + case PUT -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" / "101" => + Ok( + updateMrApprovalRule.deepMerge( + json""" { "id": 101, "approvals_required": 0 } """ + ) + ) + case POST -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "notes" => Ok(json""" { "body": "Superseded by #1234" @@ -124,7 +136,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab), GitLabCfg( mergeWhenPipelineSucceeds = false, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = false ), user @@ -134,7 +146,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = false, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = false ), user @@ -144,7 +156,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = true, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = false ), user @@ -154,7 +166,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = false, - requiredReviewers = None, + requiredApprovals = None, removeSourceBranch = true ), user @@ -164,7 +176,7 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = true, - requiredReviewers = Some(0), + requiredApprovals = Some(0.asLeft), removeSourceBranch = false ), user @@ -174,7 +186,18 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), GitLabCfg( mergeWhenPipelineSucceeds = true, - requiredReviewers = Some(0), + requiredApprovals = Some(0.asLeft), + removeSourceBranch = false + ), + user + ) + + private val gitlabApiAlgWithApprovalRules = ForgeSelection.forgeApiAlg[MockEff]( + config.forgeCfg.copy(tpe = ForgeType.GitLab, doNotFork = true), + GitLabCfg( + mergeWhenPipelineSucceeds = true, + requiredApprovals = + Some(Nel.one(MergeRequestApprovalRulesCfg("All eligible users", 0)).asRight), removeSourceBranch = false ), user @@ -363,6 +386,78 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { assertIO(prOut, expected) } + test("createPullRequest -- with approval rules") { + val prOut = gitlabApiAlgWithApprovalRules + .createPullRequest( + Repo("foo", "bar"), + newPRData + ) + .runA(state) + + val expected = PullRequestOut( + uri"https://gitlab.com/foo/bar/merge_requests/150", + PullRequestState.Open, + PullRequestNumber(150), + "title" + ) + + assertIO(prOut, expected) + } + + test("createPullRequest -- no fail upon list approval rules error") { + val localApp = HttpApp[MockEff] { req => + (req: @unchecked) match { + case GET -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" => + BadRequest(s"Cannot get merge request approval rules") + } + } + + val localState = MockState.empty.copy(clientResponses = auth <+> localApp <+> httpApp) + + val prOut = gitlabApiAlgWithApprovalRules + .createPullRequest( + Repo("foo", "bar"), + newPRData + ) + .runA(localState) + + val expected = PullRequestOut( + uri"https://gitlab.com/foo/bar/merge_requests/150", + PullRequestState.Open, + PullRequestNumber(150), + "title" + ) + + assertIO(prOut, expected) + } + + test("createPullRequest -- no fail upon update approval rule error") { + val localApp = HttpApp[MockEff] { req => + (req: @unchecked) match { + case PUT -> Root / "projects" / "foo/bar" / "merge_requests" / "150" / "approval_rules" / "101" => + BadRequest(s"Cannot update merge request approval rule") + } + } + + val localState = MockState.empty.copy(clientResponses = auth <+> localApp <+> httpApp) + + val prOut = gitlabApiAlgWithApprovalRules + .createPullRequest( + Repo("foo", "bar"), + newPRData + ) + .runA(localState) + + val expected = PullRequestOut( + uri"https://gitlab.com/foo/bar/merge_requests/150", + PullRequestState.Open, + PullRequestNumber(150), + "title" + ) + + assertIO(prOut, expected) + } + test("createPullRequest -- with non-existent user as reviewer") { val prOut = gitlabApiAlgWithAssigneeAndReviewers .createPullRequest( @@ -605,4 +700,68 @@ class GitLabApiAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] { "multiple_approval_rules_available": true } """ + + private val updateMrApprovalRule = + json""" + { + "id": 1021, + "name": "scala-steward", + "rule_type": "regular", + "eligible_approvers": [ + { + "id": 1, + "username": "scala-steward", + "name": "Scala Steward", + "state": "active", + "avatar_url": "https://secure.gravatar.com/avatar/5286ca631fff30960bfc2b337144556f?s=800&d=identicon", + "web_url": "https://gitlab.com/scala-steward" + } + ], + "approvals_required": 0, + "users": [], + "groups": [], + "contains_hidden_groups": false, + "section": null, + "source_rule": { + "approvals_required": 3 + }, + "overridden": true + } + """ + + private val getMrApprovalRules = + json""" + [ + { + "id": 101, + "name": "All eligible users", + "rule_type": "any_approver", + "eligible_approvers": [], + "approvals_required": 2, + "users": [], + "groups": [], + "contains_hidden_groups": false, + "section": null, + "source_rule": { + "approvals_required": 0 + }, + "overridden": true + }, + { + "id": 102, + "name": "scala-steward-test", + "rule_type": "regular", + "eligible_approvers": [], + "approvals_required": 2, + "users": [], + "groups": [], + "contains_hidden_groups": false, + "section": null, + "source_rule": { + "approvals_required": 3 + }, + "overridden": true + } + ] + """ }