Skip to content

Commit e6c0902

Browse files
Marcono1234Marcono1234
authored
Reject version for FixedCommand if same or older than any affected version (#728)
Check can be skipped by adding ` force` suffix to the command.
1 parent 7df93d8 commit e6c0902

File tree

11 files changed

+220
-38
lines changed

11 files changed

+220
-38
lines changed

‎docs/Commands.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,16 @@ This command currently only exists because of a technical limitation. It will be
5252
Lowercases sentences in which the first letter of each word is capitalized.
5353

5454
## $ARISA_FIXED
55-
| Entry | Value |
56-
| ----------- | ------------------------ |
57-
| Syntax | `$ARISA_FIXED <version>` |
58-
| Permissions | Mod+ |
55+
| Entry | Value |
56+
| ----------- | -------------------------------- |
57+
| Syntax | `$ARISA_FIXED <version> [force]` |
58+
| Permissions | Mod+ |
59+
60+
Resolves an issue as Fixed in the specified version. This command is useful when the version has already been
61+
archived, and the web interface does not allow choosing that version.
62+
63+
When one of the affected versions of the issue was erroneously added and the fixed version is the same or an earlier
64+
version, the command fails. To skip this check, run with a trailing `force` literal, e.g. `$ARISA_FIXED 12w34a force`.
5965

6066
## $ARISA_LIST_USER_ACTIVITY
6167
| Entry | Value |

‎src/main/kotlin/io/github/mojira/arisa/domain/Issue.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ data class Issue(
5050
val addRestrictedComment: (options: CommentOptions) -> Unit,
5151
val addNotEnglishComment: (language: String) -> Unit,
5252
val addRawRestrictedComment: (body: String, restriction: String) -> Unit,
53-
val markAsFixedWithSpecificVersion: (fixVersion: String) -> Unit,
53+
val markAsFixedWithSpecificVersion: (fixVersionName: String) -> Unit,
5454
val changeReporter: (reporter: String) -> Unit,
5555
val addAttachment: (file: File, cleanupCallback: () -> Unit) -> Unit
5656
)

‎src/main/kotlin/io/github/mojira/arisa/infrastructure/jira/Functions.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,8 @@ fun getGroups(jiraClient: JiraClient, username: String) = runBlocking {
400400
}
401401
}
402402

403-
fun markAsFixedWithSpecificVersion(context: Lazy<IssueUpdateContext>, fixVersion: String) {
404-
context.value.resolve.field(Field.FIX_VERSIONS, listOf(mapOf("name" to fixVersion)))
403+
fun markAsFixedWithSpecificVersion(context: Lazy<IssueUpdateContext>, fixVersionName: String) {
404+
context.value.resolve.field(Field.FIX_VERSIONS, listOf(mapOf("name" to fixVersionName)))
405405
context.value.transitionName = "Resolve Issue"
406406
}
407407

‎src/main/kotlin/io/github/mojira/arisa/modules/commands/CommandDispatcher.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import io.github.mojira.arisa.infrastructure.jira.getIssuesFromJql
1414
import io.github.mojira.arisa.jiraClient
1515
import io.github.mojira.arisa.modules.commands.arguments.LinkList
1616
import io.github.mojira.arisa.modules.commands.arguments.LinkListArgumentType
17+
import io.github.mojira.arisa.modules.commands.arguments.StringWithFlag
18+
import io.github.mojira.arisa.modules.commands.arguments.greedyStringWithFlag
1719

1820
@Suppress("LongMethod")
1921
fun getCommandDispatcher(
@@ -121,11 +123,13 @@ fun getCommandDispatcher(
121123
literal<CommandSource>("${prefix}_FIXED")
122124
.requires(::sentByModerator)
123125
.then(
124-
argument<CommandSource, String>("version", greedyString())
126+
argument<CommandSource, StringWithFlag>("version", greedyStringWithFlag("force"))
125127
.executes {
128+
val (version, force) = it.getStringWithFlag("version")
126129
fixedCommand(
127130
it.source.issue,
128-
it.getString("version")
131+
version,
132+
force
129133
)
130134
}
131135
)
@@ -232,3 +236,4 @@ private fun sentByModerator(source: CommandSource) =
232236
private fun CommandContext<*>.getInt(name: String) = getArgument(name, Int::class.java)
233237
private fun CommandContext<*>.getLinkList(name: String) = getArgument(name, LinkList::class.java)
234238
private fun CommandContext<*>.getString(name: String) = getArgument(name, String::class.java)
239+
private fun CommandContext<*>.getStringWithFlag(name: String) = getArgument(name, StringWithFlag::class.java)

‎src/main/kotlin/io/github/mojira/arisa/modules/commands/CommandExceptions.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.github.mojira.arisa.modules.commands
22

33
import com.mojang.brigadier.LiteralMessage
4+
import com.mojang.brigadier.exceptions.Dynamic2CommandExceptionType
45
import com.mojang.brigadier.exceptions.DynamicCommandExceptionType
56
import com.mojang.brigadier.exceptions.SimpleCommandExceptionType
67

@@ -21,9 +22,10 @@ object CommandExceptions {
2122
LiteralMessage("Could not query activity of user \"$it\"")
2223
}
2324

24-
val FIX_VERSION_BEFORE_FIRST_AFFECTED_VERSION = DynamicCommandExceptionType {
25-
LiteralMessage("Cannot add fix version $it because the first affected " +
26-
"version of the issue was released after it")
25+
val FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION = Dynamic2CommandExceptionType {
26+
fixVersionName, affectedVersionName -> LiteralMessage("Cannot add fix version $fixVersionName " +
27+
"because the affected version $affectedVersionName of the issue is the same or was released after " +
28+
"it; run with `<version> force` to add the fix version anyways")
2729
}
2830

2931
val INVALID_LINK_TYPE = SimpleCommandExceptionType(
@@ -34,6 +36,10 @@ object CommandExceptions {
3436
LiteralMessage("Found invalid ticket key")
3537
)
3638

39+
val GREEDY_STRING_ONLY_FLAG = DynamicCommandExceptionType {
40+
LiteralMessage("Argument consists only of flag '$it' but does not contain a string")
41+
}
42+
3743
val LEFT_EITHER = DynamicCommandExceptionType {
3844
LiteralMessage("Something went wrong, but I'm too lazy to interpret the details for you (>ω<): $it")
3945
}

‎src/main/kotlin/io/github/mojira/arisa/modules/commands/FixedCommand.kt

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,34 @@
11
package io.github.mojira.arisa.modules.commands
22

33
import io.github.mojira.arisa.domain.Issue
4+
import java.time.Instant
45

56
class FixedCommand {
67
@Suppress("ThrowsCount")
7-
operator fun invoke(issue: Issue, version: String): Int {
8-
if (issue.fixVersions.any { it.name == version }) {
9-
throw CommandExceptions.ALREADY_FIXED_IN.create(version)
10-
}
11-
if (issue.project.versions.none { it.name == version }) {
12-
throw CommandExceptions.NO_SUCH_VERSION.create(version)
8+
operator fun invoke(issue: Issue, fixVersionName: String, force: Boolean): Int {
9+
if (issue.fixVersions.any { it.name == fixVersionName }) {
10+
throw CommandExceptions.ALREADY_FIXED_IN.create(fixVersionName)
1311
}
12+
13+
val fixVersion = issue.project.versions.firstOrNull { it.name == fixVersionName }
14+
?: throw CommandExceptions.NO_SUCH_VERSION.create(fixVersionName)
15+
1416
if (issue.resolution !in listOf(null, "", "Unresolved")) {
1517
throw CommandExceptions.ALREADY_RESOLVED.create(issue.resolution)
1618
}
17-
if (issue.affectedVersions.first().releaseDate!!.isAfter(
18-
issue.project.versions.first { it.name == version }.releaseDate
19-
)) {
20-
throw CommandExceptions.FIX_VERSION_BEFORE_FIRST_AFFECTED_VERSION.create(version)
19+
20+
if (!force) {
21+
// Fail if any affected version is same or newer than fix version
22+
// Since archived fix versions cannot be removed again this prevents accidentally adding an incorrect
23+
// fix version
24+
issue.affectedVersions.firstOrNull { it.releaseDate!!.isSameOrAfter(fixVersion.releaseDate!!) }?.let {
25+
throw CommandExceptions.FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION.create(fixVersionName, it.name)
26+
}
2127
}
2228

23-
issue.markAsFixedWithSpecificVersion(version)
29+
issue.markAsFixedWithSpecificVersion(fixVersionName)
2430
return 1
2531
}
2632
}
33+
34+
private fun Instant.isSameOrAfter(other: Instant) = !this.isBefore(other)

‎src/main/kotlin/io/github/mojira/arisa/modules/commands/arguments/GreedyStringWithTrailingFlagArgumentType.kt

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package io.github.mojira.arisa.modules.commands.arguments
2+
3+
import com.mojang.brigadier.StringReader
4+
import com.mojang.brigadier.arguments.ArgumentType
5+
import io.github.mojira.arisa.modules.commands.CommandExceptions
6+
7+
/**
8+
* Similar to [com.mojang.brigadier.arguments.StringArgumentType.greedyString], except that the string
9+
* may end with an optional flag separated by a space.
10+
*/
11+
fun greedyStringWithFlag(flag: String): ArgumentType<StringWithFlag> = GreedyStringWithTrailingFlagArgumentType(flag)
12+
13+
private class GreedyStringWithTrailingFlagArgumentType(private val flag: String) : ArgumentType<StringWithFlag> {
14+
private val flagSuffix = " $flag"
15+
16+
override fun parse(reader: StringReader): StringWithFlag {
17+
var text = reader.remaining
18+
19+
// Throw exception when input consists only of flag, to prevent accidental incorrect usage
20+
if (text == flag) {
21+
throw CommandExceptions.GREEDY_STRING_ONLY_FLAG.createWithContext(reader, flag)
22+
}
23+
24+
// Consume complete input
25+
reader.cursor = reader.totalLength
26+
27+
var isFlagSet = false
28+
if (text.endsWith(flagSuffix)) {
29+
text = text.removeSuffix(flagSuffix)
30+
isFlagSet = true
31+
}
32+
33+
return StringWithFlag(text, isFlagSet)
34+
}
35+
}
36+
37+
data class StringWithFlag(
38+
val string: String,
39+
/** `true` if the flag was explicitly provided in the command */
40+
val isFlagSet: Boolean
41+
)

‎src/test/kotlin/io/github/mojira/arisa/modules/commands/FixedCommandTest.kt

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,24 @@ private val TWO_YEARS_AGO = RIGHT_NOW.minus(730, ChronoUnit.DAYS)
1717
class FixedCommandTest : StringSpec({
1818
"should add version" {
1919
val command = FixedCommand()
20+
var fixVersion: String? = null
2021

22+
val affectedVersion1 = getVersion(released = true, archived = false, "12w34a", releaseDate = TWO_YEARS_AGO)
23+
val affectedVersion2 = getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO)
2124
val issue = mockIssue(
2225
project = mockProject(
2326
versions = listOf(
24-
getVersion(released = true, archived = false),
25-
getVersion(released = true, archived = false, "12w34b")
27+
affectedVersion1,
28+
affectedVersion2
2629
)
2730
),
28-
affectedVersions = listOf(getVersion(released = true, archived = false))
31+
affectedVersions = listOf(affectedVersion1),
32+
markAsFixedInASpecificVersion = { fixVersion = it }
2933
)
3034

31-
val result = command(issue, "12w34b")
32-
35+
val result = command(issue, affectedVersion2.name, false)
3336
result shouldBe 1
37+
fixVersion shouldBe affectedVersion2.name
3438
}
3539

3640
"should throw ALREADY_FIXED_IN when ticket is already fixed in such version" {
@@ -48,7 +52,7 @@ class FixedCommandTest : StringSpec({
4852
)
4953

5054
val exception = shouldThrow<CommandSyntaxException> {
51-
command(issue, "12w34b")
55+
command(issue, "12w34b", false)
5256
}
5357
exception.message shouldBe "The ticket was already marked as fixed in 12w34b"
5458
}
@@ -66,7 +70,7 @@ class FixedCommandTest : StringSpec({
6670
)
6771

6872
val exception = shouldThrow<CommandSyntaxException> {
69-
command(issue, "12w34b")
73+
command(issue, "12w34b", false)
7074
}
7175
exception.message shouldBe "The version 12w34b doesn't exist in this project"
7276
}
@@ -86,28 +90,71 @@ class FixedCommandTest : StringSpec({
8690
)
8791

8892
val exception = shouldThrow<CommandSyntaxException> {
89-
command(issue, "12w34b")
93+
command(issue, "12w34b", false)
9094
}
9195
exception.message shouldBe "The ticket was already resolved as Cannot Reproduce"
9296
}
9397

94-
"should throw FIX_VERSION_BEFORE_FIRST_AFFECTED_VERSION when the fix version was released before the first affected version" {
98+
"should throw FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION when the fix version is same as an affected version" {
9599
val command = FixedCommand()
96100

101+
val affectedVersion = getVersion(released = true, archived = false, "12w34a")
97102
val issue = mockIssue(
98103
project = mockProject(
99104
versions = listOf(
100-
getVersion(released = true, archived = false, "12w34a", releaseDate = TWO_YEARS_AGO),
101-
getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO)
105+
affectedVersion
102106
)
103107
),
104-
affectedVersions = listOf(getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO))
108+
affectedVersions = listOf(affectedVersion)
105109
)
106110

107111
val exception = shouldThrow<CommandSyntaxException> {
108-
command(issue, "12w34a")
112+
command(issue, affectedVersion.name, false)
109113
}
110-
exception.message shouldBe "Cannot add fix version 12w34a because the first affected version of the issue was released after it"
114+
exception.message shouldBe "Cannot add fix version 12w34a because the affected version 12w34a of the issue " +
115+
"is the same or was released after it; run with `<version> force` to add the fix version anyways"
116+
}
117+
118+
"should throw FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION when the fix version is before an affected version" {
119+
val command = FixedCommand()
120+
121+
val affectedVersion1 = getVersion(released = true, archived = false, "12w34a", releaseDate = TWO_YEARS_AGO)
122+
val affectedVersion2 = getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO)
123+
val issue = mockIssue(
124+
project = mockProject(
125+
versions = listOf(
126+
affectedVersion1,
127+
affectedVersion2
128+
)
129+
),
130+
affectedVersions = listOf(affectedVersion2)
131+
)
132+
133+
val exception = shouldThrow<CommandSyntaxException> {
134+
command(issue, affectedVersion1.name, false)
135+
}
136+
exception.message shouldBe "Cannot add fix version 12w34a because the affected version 12w34b of the issue " +
137+
"is the same or was released after it; run with `<version> force` to add the fix version anyways"
138+
}
139+
140+
"should not throw when the fix version is same as an affected version, but 'force' is used" {
141+
val command = FixedCommand()
142+
var fixVersion: String? = null
143+
144+
val affectedVersion = getVersion(released = true, archived = false, "12w34a")
145+
val issue = mockIssue(
146+
project = mockProject(
147+
versions = listOf(
148+
affectedVersion
149+
)
150+
),
151+
affectedVersions = listOf(affectedVersion),
152+
markAsFixedInASpecificVersion = { fixVersion = it }
153+
)
154+
155+
val result = command(issue, affectedVersion.name, true)
156+
result shouldBe 1
157+
fixVersion shouldBe affectedVersion.name
111158
}
112159
})
113160

‎src/test/kotlin/io/github/mojira/arisa/modules/commands/arguments/GreedyStringWithTrailingFlagArgumentTypeTest.kt

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package io.github.mojira.arisa.modules.commands.arguments
2+
3+
import com.mojang.brigadier.StringReader
4+
import com.mojang.brigadier.exceptions.CommandSyntaxException
5+
import io.kotest.assertions.throwables.shouldThrow
6+
import io.kotest.core.spec.style.StringSpec
7+
import io.kotest.matchers.shouldBe
8+
9+
class GreedyStringWithTrailingFlagArgumentTypeTest : StringSpec({
10+
val flag = "my-flag"
11+
val argumentType = greedyStringWithFlag(flag)
12+
13+
"should work without flag" {
14+
val reader = StringReader("some text")
15+
val result = argumentType.parse(reader)
16+
result shouldBe StringWithFlag(
17+
"some text",
18+
false
19+
)
20+
reader.remainingLength shouldBe 0
21+
}
22+
23+
"should work with flag" {
24+
val reader = StringReader("some text $flag")
25+
val result = argumentType.parse(reader)
26+
result shouldBe StringWithFlag(
27+
"some text",
28+
true
29+
)
30+
reader.remainingLength shouldBe 0
31+
}
32+
33+
"should not detect as flag without space" {
34+
val reader = StringReader("some text$flag")
35+
val result = argumentType.parse(reader)
36+
result shouldBe StringWithFlag(
37+
"some text$flag",
38+
false
39+
)
40+
reader.remainingLength shouldBe 0
41+
}
42+
43+
"should work with empty text and flag" {
44+
val reader = StringReader(" $flag")
45+
val result = argumentType.parse(reader)
46+
result shouldBe StringWithFlag(
47+
"",
48+
true
49+
)
50+
reader.remainingLength shouldBe 0
51+
}
52+
53+
"should fail when only flag is used" {
54+
val reader = StringReader(flag)
55+
val exception = shouldThrow<CommandSyntaxException> {
56+
argumentType.parse(reader)
57+
}
58+
exception.message shouldBe "Argument consists only of flag 'my-flag' but does not contain a string at position 0: <--[HERE]"
59+
60+
// Cursor should not have been changed
61+
reader.cursor shouldBe 0
62+
}
63+
})

0 commit comments

Comments
 (0)