From e6c984f2bd74448d7b14a8e090c402a1a6afaaba Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Thu, 7 Jul 2022 08:48:34 +0200 Subject: [PATCH 1/2] Bugfix with temp actions being revoked too early --- .../commands/moderation/temp/TemporaryModerationRoutine.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java index 26671c196e..ab4bd88a67 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java @@ -81,13 +81,14 @@ private void checkExpiredActions() { private void processGroupedActions(@NotNull RevocationGroupIdentifier groupIdentifier) { // Do not revoke an action which was overwritten by a permanent action that was issued - // afterwards + // afterwards, or if the last action has not even expired yet // For example if a user was perm-banned after being temp-banned ActionRecord lastApplyAction = actionsStore .findLastActionAgainstTargetByType(groupIdentifier.guildId, groupIdentifier.targetId, groupIdentifier.type) .orElseThrow(); - if (lastApplyAction.actionExpiresAt() == null) { + if (lastApplyAction.actionExpiresAt() == null + || lastApplyAction.actionExpiresAt().isAfter(Instant.now())) { return; } From 86f367eeb2acd810800afa2a37f89cf287143b45 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Thu, 7 Jul 2022 12:54:44 +0200 Subject: [PATCH 2/2] Introducing "isEffective" helper, minor refactoring for readability --- .../tjbot/commands/moderation/ActionRecord.java | 12 +++++++++++- .../moderation/RejoinModerationRoleListener.java | 10 ++-------- .../moderation/temp/TemporaryModerationRoutine.java | 11 ++++------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ActionRecord.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ActionRecord.java index b568a2e0d2..910b7c8663 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ActionRecord.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ActionRecord.java @@ -26,7 +26,7 @@ public record ActionRecord(int caseId, @NotNull Instant issuedAt, long guildId, /** * Creates the action record that corresponds to the given action entry from the database table. - * + * * @param action the action to convert * @return the corresponding action record */ @@ -37,4 +37,14 @@ public record ActionRecord(int caseId, @NotNull Instant issuedAt, long guildId, ModerationAction.valueOf(action.getActionType()), action.getActionExpiresAt(), action.getReason()); } + + /** + * Whether this action is still effective. That is, it is either a permanent action or temporary + * but not expired yet. + * + * @return True when still effective, false otherwise + */ + public boolean isEffective() { + return actionExpiresAt == null || actionExpiresAt.isAfter(Instant.now()); + } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/RejoinModerationRoleListener.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/RejoinModerationRoleListener.java index d2822c65f5..2d306f0247 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/RejoinModerationRoleListener.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/RejoinModerationRoleListener.java @@ -12,7 +12,6 @@ import org.togetherjava.tjbot.commands.EventReceiver; import org.togetherjava.tjbot.config.Config; -import java.time.Instant; import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -82,23 +81,18 @@ private boolean shouldApplyModerationRole(@NotNull ModerationRole moderationRole member.getGuild().getIdLong(), member.getIdLong(), moderationRole.revokeAction); if (lastRevokeAction.isEmpty()) { // User was never e.g. unmuted - return isActionEffective(lastApplyAction.orElseThrow()); + return lastApplyAction.orElseThrow().isEffective(); } // The last issued action takes priority if (lastApplyAction.orElseThrow() .issuedAt() .isAfter(lastRevokeAction.orElseThrow().issuedAt())) { - return isActionEffective(lastApplyAction.orElseThrow()); + return lastApplyAction.orElseThrow().isEffective(); } return false; } - private static boolean isActionEffective(@NotNull ActionRecord action) { - // Effective if permanent or expires in the future - return action.actionExpiresAt() == null || action.actionExpiresAt().isAfter(Instant.now()); - } - private static void applyModerationRole(@NotNull ModerationRole moderationRole, @NotNull Member member) { Guild guild = member.getGuild(); diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java index ab4bd88a67..78e393e3b0 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/temp/TemporaryModerationRoutine.java @@ -13,7 +13,6 @@ import org.togetherjava.tjbot.commands.moderation.ModerationActionsStore; import org.togetherjava.tjbot.config.Config; -import java.time.Instant; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -80,15 +79,14 @@ private void checkExpiredActions() { } private void processGroupedActions(@NotNull RevocationGroupIdentifier groupIdentifier) { - // Do not revoke an action which was overwritten by a permanent action that was issued - // afterwards, or if the last action has not even expired yet + // Do not revoke an action which was overwritten by a still effective action that was issued + // afterwards // For example if a user was perm-banned after being temp-banned ActionRecord lastApplyAction = actionsStore .findLastActionAgainstTargetByType(groupIdentifier.guildId, groupIdentifier.targetId, groupIdentifier.type) .orElseThrow(); - if (lastApplyAction.actionExpiresAt() == null - || lastApplyAction.actionExpiresAt().isAfter(Instant.now())) { + if (lastApplyAction.isEffective()) { return; } @@ -102,8 +100,7 @@ private void processGroupedActions(@NotNull RevocationGroupIdentifier groupIdent if (lastRevokeActionOpt.isPresent()) { ActionRecord lastRevokeAction = lastRevokeActionOpt.orElseThrow(); if (lastRevokeAction.issuedAt().isAfter(lastApplyAction.issuedAt()) - && (lastRevokeAction.actionExpiresAt() == null - || lastRevokeAction.actionExpiresAt().isAfter(Instant.now()))) { + && lastRevokeAction.isEffective()) { return; } }