Bugfix with temp actions being revoked too early

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());
+    }
 }

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();

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,14 +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
+        // 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) {
+        if (lastApplyAction.isEffective()) {
             return;
         }
 
@@ -101,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;
             }
         }