Allow staff to unrestrict comments hidden by HideImpostors module (#821)

* Allow staff to unrestrict comments hidden by HideImpostors module

* Add new check HideImpostors documentation

* Fix failing CI due to flaky ReopenAwaitingModule test

There was a race condition in the ReopenAwaitingModule test due to it using the
helper message service, which is also used by other tests. This commit completely
removes the dependency of the ReopenAwaitingModule on this service by instead
hardcoding the message on initialization of the module in the registry.

docs/Modules.md

@@ -165,6 +165,7 @@ Hides the comments created by users who look like an impostor.
   and some texts after the brackets. e.g. `[dev] foo`.
 - The comment is not restricted to `staff`.
 - The comment's author is not a `helper`, `global-moderators`, nor `staff`.
+- The user who last updated the comment is not a `helper`, `global-moderators`, nor `staff`.
 
 ## IncompleteModule
 

src/main/kotlin/io/github/mojira/arisa/HelperMessageUpdateService.kt

@@ -13,6 +13,10 @@ class HelperMessageUpdateService {
     private val helperMessagesFile = File("helper-messages.json")
     private var helperMessagesLastFetch = Instant.now().minusSeconds(UPDATE_INTERVAL_IN_SECONDS + 1)
 
+    init {
+        this.checkForUpdate()
+    }
+
     fun checkForUpdate() {
         val currentTime = Instant.now()
 

src/main/kotlin/io/github/mojira/arisa/domain/Comment.kt

@@ -6,7 +6,9 @@ data class Comment(
     val id: String,
     val body: String?,
     val author: User,
+    val updateAuthor: User?,
     val getAuthorGroups: () -> List<String>?,
+    val getUpdateAuthorGroups: () -> List<String>?,
     val created: Instant,
     val updated: Instant,
     val visibilityType: String?,

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

@@ -221,7 +221,9 @@ fun JiraComment.toDomain(
         id,
         body,
         author.toDomain(jiraClient, config),
+        updateAuthor?.toDomain(jiraClient, config),
         { getGroups(jiraClient, author.name).fold({ null }, { it }) },
+        { if (updateAuthor == null) emptyList() else getGroups(jiraClient, updateAuthor.name).fold({ null }, { it }) },
         createdDate.toInstant(),
         updatedDate.toInstant(),
         visibility?.type,

src/main/kotlin/io/github/mojira/arisa/modules/HideImpostorsModule.kt

@@ -16,7 +16,8 @@ class HideImpostorsModule : Module {
                 .filter(::commentIsRecent)
                 .filter(::userContainsBrackets)
                 .filter(::isNotStaffRestricted)
-                .filter(::userIsNotVolunteer)
+                .filter(::authorIsNotVolunteer)
+                .filter(::updateAuthorIsNotVolunteer)
                 .map { it.restrict.partially1(it.body ?: "") }
                 .toList()
 
@@ -34,8 +35,13 @@ class HideImpostorsModule : Module {
         this != null && matches("""\[(?:\p{L}|\p{N}|\s)+]\s.+""".toRegex())
     }
 
-    private fun userIsNotVolunteer(comment: Comment) =
-        !(comment.getAuthorGroups()?.any { it == "helper" || it == "global-moderators" || it == "staff" } ?: false)
+    private val staffGroups = setOf("helper", "global-moderators", "staff")
+
+    private fun authorIsNotVolunteer(comment: Comment) =
+        comment.getAuthorGroups()?.none { staffGroups.contains(it) } ?: true
+
+    private fun updateAuthorIsNotVolunteer(comment: Comment) =
+        comment.getUpdateAuthorGroups()?.none { staffGroups.contains(it) } ?: true
 
     private fun isNotStaffRestricted(comment: Comment) =
         comment.visibilityType != "group" || comment.visibilityValue != "staff"

src/main/kotlin/io/github/mojira/arisa/modules/ReopenAwaitingModule.kt

@@ -6,11 +6,8 @@ import arrow.core.left
 import arrow.core.right
 import io.github.mojira.arisa.domain.ChangeLogItem
 import io.github.mojira.arisa.domain.Comment
-import io.github.mojira.arisa.domain.CommentOptions
 import io.github.mojira.arisa.domain.Issue
-import io.github.mojira.arisa.domain.Project
 import io.github.mojira.arisa.domain.User
-import io.github.mojira.arisa.infrastructure.HelperMessageService
 import java.time.Instant
 import java.time.temporal.ChronoUnit
 
@@ -43,8 +40,8 @@ class ReopenAwaitingModule(
                 reopen()
             } else {
                 assertNotEquals(changeLog.maxByOrNull { it.created }?.author?.name, "arisabot")
-                if (comments.none { isKeepARMessage(it, project) }) {
-                    addComment(CommentOptions(message))
+                if (comments.none { isKeepARMessage(it) }) {
+                    addRawBotComment(message)
                 }
             }
         }
@@ -80,17 +77,8 @@ class ReopenAwaitingModule(
         comment.visibilityValue == "staff" &&
         (comment.body?.contains(keepARTag) ?: false)
 
-    private fun isKeepARMessage(comment: Comment, project: Project) = comment.author.name == "arisabot" &&
-        (
-            comment.body?.contains(
-                HelperMessageService.getMessageWithBotSignature(
-                    project.key,
-                    message,
-                    null,
-                    "en"
-                )
-            ) ?: false
-            )
+    private fun isKeepARMessage(comment: Comment) =
+        comment.author.name == "arisabot" && comment.body?.contains(message) ?: false
 
     private fun getValidComments(
         comments: List<Comment>,

src/main/kotlin/io/github/mojira/arisa/registry/InstantModuleRegistry.kt

@@ -4,6 +4,7 @@ import com.uchuhimo.konf.Config
 import com.urielsalis.mccrashlib.CrashReader
 import io.github.mojira.arisa.ExecutionTimeframe
 import io.github.mojira.arisa.infrastructure.AttachmentUtils
+import io.github.mojira.arisa.infrastructure.HelperMessageService
 import io.github.mojira.arisa.infrastructure.LanguageDetectionApi
 import io.github.mojira.arisa.infrastructure.config.Arisa
 import io.github.mojira.arisa.modules.AffectedVersionMessageModule
@@ -209,7 +210,7 @@ class InstantModuleRegistry(config: Config) : ModuleRegistry(config) {
                 config[Arisa.Modules.ReopenAwaiting.softARDays],
                 config[Arisa.Modules.ReopenAwaiting.keepARTag],
                 config[Arisa.Modules.ReopenAwaiting.onlyOPTag],
-                config[Arisa.Modules.ReopenAwaiting.message]
+                HelperMessageService.getMessage("MC", keys = listOf(config[Arisa.Modules.ReopenAwaiting.message]))
             )
         )
 

src/test/kotlin/io/github/mojira/arisa/modules/HideImpostorsModuleTest.kt

@@ -7,6 +7,7 @@ import io.github.mojira.arisa.utils.mockUser
 import io.kotest.assertions.arrow.either.shouldBeLeft
 import io.kotest.assertions.arrow.either.shouldBeRight
 import io.kotest.core.spec.style.StringSpec
+import io.kotest.matchers.booleans.shouldBeFalse
 import io.kotest.matchers.booleans.shouldBeTrue
 import java.time.Instant
 import java.time.temporal.ChronoUnit
@@ -168,6 +169,26 @@ class HideImpostorsModuleTest : StringSpec({
         result.shouldBeLeft(OperationNotNeededModuleResponse)
     }
 
+    "should return OperationNotNeededModuleResponse when comment was edited by a staff+ user" {
+        var isRestricted = false
+        val module = HideImpostorsModule()
+        val comment = getComment(
+            author = "[test] test",
+            updateAuthor = "[Mod] Moderator",
+            getAuthorGroups = { listOf("user") },
+            getUpdateAuthorGroups = { listOf("staff") },
+            restrict = { isRestricted = true }
+        )
+        val issue = mockIssue(
+            comments = listOf(comment)
+        )
+
+        val result = module(issue, RIGHT_NOW)
+
+        result.shouldBeLeft(OperationNotNeededModuleResponse)
+        isRestricted.shouldBeFalse()
+    }
+
     "should hide comment when user starts with a valid tag but is not of a permission group" {
         var isRestricted = false
         val module = HideImpostorsModule()
@@ -279,14 +300,18 @@ private fun getUser(displayName: String) = mockUser(name = "", displayName = dis
 
 private fun getComment(
     author: String = "User",
+    updateAuthor: String? = null,
     getAuthorGroups: () -> List<String> = { emptyList() },
+    getUpdateAuthorGroups: () -> List<String> = { emptyList() },
     created: Instant = RIGHT_NOW,
     visibilityType: String? = null,
     visibilityValue: String? = null,
     restrict: (String) -> Unit = { }
 ) = mockComment(
     author = getUser(displayName = author),
+    updateAuthor = if (updateAuthor == null) null else getUser(displayName = updateAuthor),
     getAuthorGroups = getAuthorGroups,
+    getUpdateAuthorGroups = getUpdateAuthorGroups,
     created = created,
     updated = created,
     visibilityType = visibilityType,

src/test/kotlin/io/github/mojira/arisa/modules/ReopenAwaitingModuleTest.kt

@@ -2,7 +2,6 @@ package io.github.mojira.arisa.modules
 
 import arrow.core.right
 import io.github.mojira.arisa.domain.User
-import io.github.mojira.arisa.infrastructure.HelperMessageService
 import io.github.mojira.arisa.utils.RIGHT_NOW
 import io.github.mojira.arisa.utils.mockChangeLogItem
 import io.github.mojira.arisa.utils.mockComment
@@ -20,23 +19,19 @@ private val ARISA = getUser(name = "arisabot")
 private val RANDOM_USER = getUser(name = "randomUser")
 private val NEWBIE = getUser(name = "newbieUser", newUser = true)
 
-private val NOT_REOPEN_AR_MESSAGE = HelperMessageService.getMessageWithBotSignature(
-    "MC",
-    "not-reopen-ar",
-    null,
-    "en"
-)
-
 private val TEN_SECONDS_AGO = RIGHT_NOW.minusSeconds(10)
 private val TWO_YEARS_AGO = RIGHT_NOW.minus(730, ChronoUnit.DAYS)
 
+private const val NOT_REOPEN_AR_MESSAGE = "This report is currently missing crucial information. " +
+    "Please take a look at the other comments to find out what we are looking for."
+
 private val MODULE = ReopenAwaitingModule(
     setOf("staff", "global-moderators"),
     setOf("helper", "staff", "global-moderators"),
     365,
     "MEQS_KEEP_AR",
     "ARISA_REOPEN_OP",
-    "not-reopen-ar"
+    NOT_REOPEN_AR_MESSAGE
 )
 private val AWAITING_RESOLVE = mockChangeLogItem(
     created = TEN_SECONDS_AGO,
@@ -800,7 +795,8 @@ class ReopenAwaitingModuleTest : StringSpec({
             comments = listOf(tagComment, normalComment),
             changeLog = listOf(AWAITING_RESOLVE),
             reopen = { hasReopened = true; Unit.right() },
-            addComment = { hasCommented = true; Unit.right() }
+            addComment = { hasCommented = true; Unit.right() },
+            addRawBotComment = { hasCommented = true; Unit.right() }
         )
 
         val result = MODULE(issue, TEN_SECONDS_AGO)
@@ -823,7 +819,8 @@ class ReopenAwaitingModuleTest : StringSpec({
             comments = listOf(comment),
             changeLog = listOf(OLD_AWAITING_RESOLVE),
             reopen = { hasReopened = true; Unit.right() },
-            addComment = { hasCommented = true; Unit.right() }
+            addComment = { hasCommented = true; Unit.right() },
+            addRawBotComment = { hasCommented = true; Unit.right() }
         )
 
         val result = MODULE(issue, TEN_SECONDS_AGO)
@@ -886,7 +883,8 @@ class ReopenAwaitingModuleTest : StringSpec({
             comments = listOf(tagComment, fakeComment),
             changeLog = listOf(AWAITING_RESOLVE),
             reopen = { hasReopened = true; Unit.right() },
-            addComment = { hasCommented = true; Unit.right() }
+            addComment = { hasCommented = true; Unit.right() },
+            addRawBotComment = { hasCommented = true; Unit.right() }
         )
 
         val result = MODULE(issue, TEN_SECONDS_AGO)

src/test/kotlin/io/github/mojira/arisa/utils/MockDomain.kt

@@ -67,7 +67,9 @@ fun mockComment(
     id: String = "0",
     body: String = "",
     author: User = mockUser(),
+    updateAuthor: User? = null,
     getAuthorGroups: () -> List<String> = { emptyList() },
+    getUpdateAuthorGroups: () -> List<String> = { emptyList() },
     created: Instant = RIGHT_NOW,
     updated: Instant = created,
     visibilityType: String? = null,
@@ -79,7 +81,9 @@ fun mockComment(
     id,
     body,
     author,
+    updateAuthor,
     getAuthorGroups,
+    getUpdateAuthorGroups,
     created,
     updated,
     visibilityType,