Skip to content

Commit d2c7bd7

Browse files
author
Ender Tunc
committed
finalize the implementation and add tests
1 parent ba61a1d commit d2c7bd7

File tree

7 files changed

+328
-51
lines changed

7 files changed

+328
-51
lines changed

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

+10-10
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import com.monovore.decline.Opts.{flag, option, options}
2323
import com.monovore.decline._
2424
import org.http4s.Uri
2525
import org.http4s.syntax.literals._
26-
import org.scalasteward.core.application.Cli.gitlabMergeRequestApprovalsConfig
2726
import org.scalasteward.core.application.Config._
2827
import org.scalasteward.core.data.Resolver
2928
import org.scalasteward.core.forge.ForgeType
@@ -47,15 +46,15 @@ object Cli {
4746
val processTimeout = "process-timeout"
4847
}
4948

50-
implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalsConfig] =
49+
implicit val mergeRequestApprovalsConfigArgument: Argument[MergeRequestApprovalRulesCfg] =
5150
Argument.from("approvals_rule_name=required_approvals") { s =>
5251
s.split(":").toList match {
5352
case approvalRuleName :: requiredApprovalsAsString :: Nil =>
5453
Try(requiredApprovalsAsString.trim.toInt) match {
5554
case Failure(_) =>
5655
s"[$requiredApprovalsAsString] is not a valid Integer".invalidNel
5756
case Success(requiredApprovals) =>
58-
new MergeRequestApprovalsConfig(approvalRuleName.trim, requiredApprovals).validNel
57+
MergeRequestApprovalRulesCfg(approvalRuleName.trim, requiredApprovals).validNel
5958
}
6059
case _ =>
6160
s"The value is expected in the following format: APPROVALS_RULE_NAME:REQUIRED_APPROVALS.".invalidNel
@@ -294,7 +293,7 @@ object Cli {
294293

295294
private val gitlabRequiredReviewers: Opts[Option[Int]] =
296295
option[Int](
297-
"gitlabRequiredReviewers",
296+
"gitlab-required-reviewers",
298297
"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."
299298
).validate("Required reviewers must be non-negative")(_ >= 0).orNone
300299

@@ -304,24 +303,25 @@ object Cli {
304303
"Flag indicating if a merge request should remove the source branch when merging."
305304
).orFalse
306305

307-
private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalsConfig]]] =
308-
options[MergeRequestApprovalsConfig](
306+
private val gitlabMergeRequestApprovalsConfig: Opts[Option[Nel[MergeRequestApprovalRulesCfg]]] =
307+
options[MergeRequestApprovalRulesCfg](
309308
"merge-request-level-approval-rule",
310309
s"Additional repo config file $multiple"
311310
)
312-
// ToDo better message
313-
.validate("")(_.forall(_.requiredApproves >= 0) == true)
311+
.validate("Merge request level required approvals must be non-negative")(
312+
_.forall(_.requiredApprovals >= 0) == true
313+
)
314314
.orNone
315315

316316
private val gitlabReviewersAndApprovalsConfig
317-
: Opts[Option[Either[Int, Nel[MergeRequestApprovalsConfig]]]] =
317+
: Opts[Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]]] =
318318
((gitlabRequiredReviewers, gitlabMergeRequestApprovalsConfig).tupled.mapValidated {
319319
case (None, None) => None.validNel
320320
case (None, Some(gitlabMergeRequestApprovalsConfig)) =>
321321
Some(gitlabMergeRequestApprovalsConfig.asRight[Int]).validNel
322322
case (Some(requiredReviewers), None) => Some(Left(requiredReviewers)).validNel
323323
case (Some(_), Some(_)) =>
324-
s"You can't use both --gitlabRequiredReviewers and --merge-request-level-approval-rule at the same time".invalidNel
324+
s"You can't use both --gitlab-required-reviewers and --merge-request-level-approval-rule at the same time".invalidNel
325325
})
326326

327327
private val gitLabCfg: Opts[GitLabCfg] =

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,11 @@ object Config {
156156
final case class GitHubCfg(
157157
) extends ForgeSpecificCfg
158158

159-
final case class MergeRequestApprovalsConfig(approvalRuleName: String, requiredApproves: Int)
159+
final case class MergeRequestApprovalRulesCfg(approvalRuleName: String, requiredApprovals: Int)
160160

161161
final case class GitLabCfg(
162162
mergeWhenPipelineSucceeds: Boolean,
163-
requiredReviewers: Option[Either[Int, Nel[MergeRequestApprovalsConfig]]],
163+
requiredApprovals: Option[Either[Int, Nel[MergeRequestApprovalRulesCfg]]],
164164
removeSourceBranch: Boolean
165165
) extends ForgeSpecificCfg
166166

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

+39-26
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import io.circe._
2222
import io.circe.generic.semiauto._
2323
import io.circe.syntax._
2424
import org.http4s.{Request, Status, Uri}
25-
import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalsConfig}
25+
import org.scalasteward.core.application.Config.{ForgeCfg, GitLabCfg, MergeRequestApprovalRulesCfg}
2626
import org.scalasteward.core.data.Repo
2727
import org.scalasteward.core.forge.ForgeApiAlg
2828
import org.scalasteward.core.forge.data._
@@ -155,7 +155,7 @@ private[gitlab] object GitLabJsonCodec {
155155
Decoder.instance { c =>
156156
for {
157157
id <- c.downField("id").as[Int]
158-
name <- c.downField("string").as[String]
158+
name <- c.downField("name").as[String]
159159
} yield MergeRequestLevelApprovalRuleOut(id, name)
160160
}
161161

@@ -244,7 +244,7 @@ final class GitLabApiAlg[F[_]: Parallel](
244244
for {
245245
mr <- mergeRequest
246246
mrWithStatus <- waitForMergeRequestStatus(mr.iid)
247-
_ <- gitLabCfg.requiredReviewers match {
247+
_ <- gitLabCfg.requiredApprovals match {
248248
case Some(Right(approvalRules)) =>
249249
setApprovalRules(repo, mrWithStatus, approvalRules)
250250
case Some(Left(requiredReviewers)) =>
@@ -308,11 +308,11 @@ final class GitLabApiAlg[F[_]: Parallel](
308308
private def setApprovalRules(
309309
repo: Repo,
310310
mrOut: MergeRequestOut,
311-
approvalsConfig: Nel[MergeRequestApprovalsConfig]
311+
approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg]
312312
): F[MergeRequestOut] =
313313
for {
314314
_ <- logger.info(
315-
s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalsConfig"
315+
s"Adjusting merge request approvals rules on ${mrOut.webUrl} with following config: $approvalRulesCfg"
316316
)
317317
activeApprovalRules <-
318318
client
@@ -321,34 +321,47 @@ final class GitLabApiAlg[F[_]: Parallel](
321321
modify(repo)
322322
)
323323
.recoverWith { case UnexpectedResponse(_, _, _, status, body) =>
324-
// ToDo better log
325324
logger
326-
.warn(s"Unexpected response setting required reviewers: $status: $body")
325+
.warn(s"Unexpected response getting merge request approval rules: $status: $body")
327326
.as(List.empty)
328327
}
329-
approvalRuleNamesFromConfig = approvalsConfig.map(_.approvalRuleName)
330-
approvalRulesToUpdate = activeApprovalRules.intersect(approvalRuleNamesFromConfig.toList)
328+
approvalRulesToUpdate = calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg)
331329
_ <-
332-
approvalRulesToUpdate.map { mergeRequestApprovalConfig =>
333-
client
334-
.putWithBody[Unit, UpdateMergeRequestLevelApprovalRulePayload](
335-
url.updateMergeRequestLevelApprovalRule(
336-
repo,
337-
mrOut.iid,
338-
mergeRequestApprovalConfig.id
339-
),
340-
UpdateMergeRequestLevelApprovalRulePayload(mergeRequestApprovalConfig.id),
341-
modify(repo)
342-
)
343-
.recoverWith { case UnexpectedResponse(_, _, _, status, body) =>
344-
// ToDo better log
345-
logger
346-
.warn(s"Unexpected response setting required reviewers: $status: $body")
347-
.as(List.empty)
348-
}
330+
approvalRulesToUpdate.map { case (approvalRuleCfg, activeRule) =>
331+
logger.info(
332+
s"Setting required approval count to ${approvalRuleCfg.requiredApprovals} for merge request approval rule '${approvalRuleCfg.approvalRuleName}' on ${mrOut.webUrl}"
333+
) >>
334+
client
335+
.putWithBody[
336+
MergeRequestLevelApprovalRuleOut,
337+
UpdateMergeRequestLevelApprovalRulePayload
338+
](
339+
url.updateMergeRequestLevelApprovalRule(
340+
repo,
341+
mrOut.iid,
342+
activeRule.id
343+
),
344+
UpdateMergeRequestLevelApprovalRulePayload(approvalRuleCfg.requiredApprovals),
345+
modify(repo)
346+
)
347+
.as(())
348+
.recoverWith { case UnexpectedResponse(_, _, _, status, body) =>
349+
logger
350+
.warn(s"Unexpected response setting required approvals: $status: $body")
351+
}
349352
}.sequence
350353
} yield mrOut
351354

355+
private[gitlab] def calculateRulesToUpdate(
356+
activeApprovalRules: List[MergeRequestLevelApprovalRuleOut],
357+
approvalRulesCfg: Nel[MergeRequestApprovalRulesCfg]
358+
): List[(MergeRequestApprovalRulesCfg, MergeRequestLevelApprovalRuleOut)] =
359+
activeApprovalRules.flatMap { activeRule =>
360+
approvalRulesCfg
361+
.find(_.approvalRuleName == activeRule.name)
362+
.map(_ -> activeRule)
363+
}
364+
352365
private def getUsernameToUserIdsMapping(repo: Repo, usernames: Set[String]): F[Map[String, Int]] =
353366
usernames.toList
354367
.parTraverse { username =>

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

-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.scalasteward.core.forge.gitlab
1818

1919
import org.http4s.Uri
20-
import org.scalasteward.core.application.Config.MergeRequestApprovalsConfig
2120
import org.scalasteward.core.data.Repo
2221
import org.scalasteward.core.forge.data.PullRequestNumber
2322
import org.scalasteward.core.git.Branch

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

+65-5
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@ import munit.FunSuite
66
import org.http4s.syntax.literals._
77
import org.scalasteward.core.application.Cli.EnvVar
88
import org.scalasteward.core.application.Cli.ParseResult._
9-
import org.scalasteward.core.application.Config.StewardUsage
9+
import org.scalasteward.core.application.Config.{MergeRequestApprovalRulesCfg, StewardUsage}
1010
import org.scalasteward.core.forge.ForgeType
1111
import org.scalasteward.core.forge.github.GitHubApp
12+
import org.scalasteward.core.util.Nel
13+
import cats.syntax.either._
14+
1215
import scala.concurrent.duration._
1316

1417
class CliTest extends FunSuite {
@@ -63,7 +66,7 @@ class CliTest extends FunSuite {
6366
assertEquals(obtained.githubApp, Some(GitHubApp(12345678L, File("example_app_key"))))
6467
assertEquals(obtained.refreshBackoffPeriod, 1.day)
6568
assert(!obtained.gitLabCfg.mergeWhenPipelineSucceeds)
66-
assertEquals(obtained.gitLabCfg.requiredReviewers, None)
69+
assertEquals(obtained.gitLabCfg.requiredApprovals, None)
6770
assert(obtained.bitbucketCfg.useDefaultReviewers)
6871
assert(!obtained.bitbucketServerCfg.useDefaultReviewers)
6972
}
@@ -151,27 +154,84 @@ class CliTest extends FunSuite {
151154
assert(clue(obtained).startsWith("Usage"))
152155
}
153156

154-
test("parseArgs: non-default GitLab arguments") {
157+
test("parseArgs: non-default GitLab arguments and required reviewers") {
155158
val params = minimumRequiredParams ++ List(
156159
List("--gitlab-merge-when-pipeline-succeeds"),
157160
List("--gitlab-required-reviewers", "5")
158161
)
159162
val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten)
160163

161164
assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds)
162-
assertEquals(obtained.gitLabCfg.requiredReviewers, Some(5))
165+
assertEquals(obtained.gitLabCfg.requiredApprovals, Some(5.asLeft))
163166
}
164167

165-
test("parseArgs: invalid GitLab required reviewers") {
168+
test("parseArgs: non-default GitLab arguments and merge request level approval rule") {
166169
val params = minimumRequiredParams ++ List(
167170
List("--gitlab-merge-when-pipeline-succeeds"),
171+
List("--merge-request-level-approval-rule", "All eligible users:0")
172+
)
173+
val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten)
174+
175+
assert(obtained.gitLabCfg.mergeWhenPipelineSucceeds)
176+
assertEquals(
177+
obtained.gitLabCfg.requiredApprovals,
178+
Some(Nel.one(MergeRequestApprovalRulesCfg("All eligible users", 0)).asRight)
179+
)
180+
}
181+
182+
test("parseArgs: multiple Gitlab merge request level approval rule") {
183+
val params = minimumRequiredParams ++ List(
184+
List("--merge-request-level-approval-rule", "All eligible users:1"),
185+
List("--merge-request-level-approval-rule", "Only Main:2")
186+
)
187+
val Success(StewardUsage.Regular(obtained)) = Cli.parseArgs(params.flatten)
188+
189+
assertEquals(
190+
obtained.gitLabCfg.requiredApprovals,
191+
Some(
192+
Nel
193+
.of(
194+
MergeRequestApprovalRulesCfg("All eligible users", 1),
195+
MergeRequestApprovalRulesCfg("Only Main", 2)
196+
)
197+
.asRight
198+
)
199+
)
200+
}
201+
202+
test("parseArgs: only allow one way to define Gitlab required approvals arguments") {
203+
val params = minimumRequiredParams ++ List(
204+
List("--merge-request-level-approval-rule", "All eligible users:0"),
205+
List("--gitlab-required-reviewers", "5")
206+
)
207+
val Error(errorMsg) = Cli.parseArgs(params.flatten)
208+
209+
assert(
210+
clue(errorMsg).startsWith(
211+
"You can't use both --gitlab-required-reviewers and --merge-request-level-approval-rule at the same time"
212+
)
213+
)
214+
215+
}
216+
217+
test("parseArgs: invalid GitLab required reviewers") {
218+
val params = minimumRequiredParams ++ List(
168219
List("--gitlab-required-reviewers", "-3")
169220
)
170221
val Error(errorMsg) = Cli.parseArgs(params.flatten)
171222

172223
assert(clue(errorMsg).startsWith("Required reviewers must be non-negative"))
173224
}
174225

226+
test("parseArgs: invalid GitLab merge request level approval rule") {
227+
val params = minimumRequiredParams ++ List(
228+
List("--merge-request-level-approval-rule", "All eligible users:-3")
229+
)
230+
val Error(errorMsg) = Cli.parseArgs(params.flatten)
231+
232+
assert(clue(errorMsg).startsWith("Merge request level required approvals must be non-negative"))
233+
}
234+
175235
test("parseArgs: validate-repo-config") {
176236
val Success(StewardUsage.ValidateRepoConfig(file)) = Cli.parseArgs(
177237
List(

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

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package org.scalasteward.core.forge.gitlab
2+
3+
import munit.CatsEffectSuite
4+
import org.http4s.Request
5+
import org.scalasteward.core.TestInstances.ioLogger
6+
import org.scalasteward.core.application.Config.{GitLabCfg, MergeRequestApprovalRulesCfg}
7+
import org.scalasteward.core.data.Repo
8+
import org.scalasteward.core.forge.ForgeType
9+
import org.scalasteward.core.mock.MockConfig.config
10+
import org.scalasteward.core.mock.MockContext.context.httpJsonClient
11+
import org.scalasteward.core.mock.MockEff
12+
import org.scalasteward.core.util.Nel
13+
14+
class GitLabAlgTest extends CatsEffectSuite {
15+
16+
private val gitlabApiAlg = new GitLabApiAlg[MockEff](
17+
forgeCfg = config.forgeCfg.copy(tpe = ForgeType.GitLab),
18+
gitLabCfg = GitLabCfg(
19+
mergeWhenPipelineSucceeds = false,
20+
requiredApprovals = None,
21+
removeSourceBranch = true
22+
),
23+
modify = (_: Repo) => (request: Request[MockEff]) => MockEff.pure(request)
24+
)
25+
26+
test(
27+
"calculateRulesToUpdate -- ignore active approval rule that doesn't have approval rule configuration"
28+
) {
29+
val activeApprovalRules =
30+
List(
31+
MergeRequestLevelApprovalRuleOut(name = "A", id = 101),
32+
MergeRequestLevelApprovalRuleOut(name = "B", id = 201)
33+
)
34+
val approvalRulesCfg =
35+
Nel.one(MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 1))
36+
37+
val result =
38+
gitlabApiAlg.calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg)
39+
val expectedResult =
40+
List(
41+
(
42+
MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 1),
43+
MergeRequestLevelApprovalRuleOut(id = 201, name = "B")
44+
)
45+
)
46+
47+
assertEquals(result, expectedResult)
48+
}
49+
50+
test(
51+
"calculateRulesToUpdate -- ignore approval rule configuration that doesn't have active approval rule"
52+
) {
53+
val activeApprovalRules =
54+
List(MergeRequestLevelApprovalRuleOut(name = "A", id = 101))
55+
val approvalRulesCfg =
56+
Nel.of(
57+
MergeRequestApprovalRulesCfg(approvalRuleName = "A", requiredApprovals = 1),
58+
MergeRequestApprovalRulesCfg(approvalRuleName = "B", requiredApprovals = 2)
59+
)
60+
61+
val result =
62+
gitlabApiAlg.calculateRulesToUpdate(activeApprovalRules, approvalRulesCfg)
63+
val expectedResult =
64+
List(
65+
(
66+
MergeRequestApprovalRulesCfg(approvalRuleName = "A", requiredApprovals = 1),
67+
MergeRequestLevelApprovalRuleOut(name = "A", id = 101)
68+
)
69+
)
70+
71+
assertEquals(result, expectedResult)
72+
}
73+
}

0 commit comments

Comments
 (0)