Merge pull request #139 from janssensjelle/feature/ban-improvement

[IMPROVEMENT] - Deduplicate evidence/note

src/cmds/core/ban.py

@@ -12,7 +12,7 @@
 from src.core import settings
 from src.database.models import Ban, Infraction
 from src.database.session import AsyncSessionLocal
-from src.helpers.ban import add_evidence_note, add_infraction, ban_member, unban_member
+from src.helpers.ban import add_infraction, ban_member, unban_member
 from src.helpers.duration import validate_duration
 from src.helpers.schedule import schedule
 
@@ -34,7 +34,6 @@ async def ban(
         member = await self.bot.get_member_or_user(ctx.guild, user.id)
         if not member:
             return await ctx.respond(f"User {user} not found.")
-        await add_evidence_note(member.id, "ban", reason, evidence, ctx.user.id)
         response = await ban_member(
             self.bot, ctx.guild, member, "500w", reason, evidence, ctx.user, needs_approval=False
         )
@@ -54,7 +53,6 @@ async def tempban(
         member = await self.bot.get_member_or_user(ctx.guild, user.id)
         if not member:
             return await ctx.respond(f"User {user} not found.")
-        await add_evidence_note(member.id, "ban", reason, evidence, ctx.user.id)
         response = await ban_member(
             self.bot, ctx.guild, member, duration, reason, evidence, ctx.user, needs_approval=True
         )

src/cmds/core/user.py

@@ -1,7 +1,6 @@
 import logging
 import os
 import random
-from datetime import datetime
 from typing import Tuple, Union
 
 import discord
@@ -16,7 +15,7 @@
 from src.core import settings
 from src.database.models import HtbDiscordLink
 from src.database.session import AsyncSessionLocal
-from src.helpers.ban import add_evidence_note, add_infraction
+from src.helpers.ban import add_infraction
 from src.helpers.checks import member_is_staff
 
 logger = logging.getLogger(__name__)
@@ -81,8 +80,6 @@ async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidenc
         if len(reason) == 0:
             reason = "No reason given..."
 
-        await add_evidence_note(member.id, "kick", reason, evidence, ctx.user.id)
-
         try:
             await member.send(f"You have been kicked from {ctx.guild.name} for the following reason:\n>>> {reason}\n")
         except Forbidden as ex:
@@ -97,7 +94,7 @@ async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidenc
             )
 
         await ctx.guild.kick(user=member, reason=reason)
-        infraction_reason = f"{ctx.user.name} was kicked on {datetime.now().strftime('%Y-%m-%d %H:%M:%S')} for {reason}"
+        infraction_reason = f"Previously kicked for: {reason} - Evidence: {evidence}"
         await add_infraction(ctx.guild, member, 0, infraction_reason, ctx.user)
         return await ctx.respond(f"{member.name} got the boot!")
 

src/helpers/ban.py

@@ -11,7 +11,7 @@
 
 from src.bot import Bot
 from src.core import settings
-from src.database.models import Ban, Infraction, Mute, UserNote
+from src.database.models import Ban, Infraction, Mute
 from src.database.session import AsyncSessionLocal
 from src.helpers.checks import member_is_staff
 from src.helpers.duration import validate_duration
@@ -62,6 +62,9 @@ async def ban_member(
     if len(reason) == 0:
         reason = "No reason given ..."
 
+    if not evidence:
+        evidence = "none provided"
+
     # Validate duration
     dur, dur_exc = validate_duration(duration)
     # Check if duration is valid,
@@ -80,7 +83,10 @@ async def ban_member(
         approved=False if needs_approval else True
     )
     infraction = Infraction(
-        user_id=member.id, reason=f"Previously banned for: {reason}", weight=0, moderator_id=author.id,
+        user_id=member.id,
+        reason=f"Previously banned for: {reason} - Evidence: {evidence}",
+        weight=0,
+        moderator_id=author.id,
         date=datetime.now().date()
     )
     ban_id, is_existing = await _get_ban_or_create(member, ban, infraction)
@@ -287,17 +293,3 @@ async def add_infraction(
         logger.warning(f"HTTPException when trying to add infraction for user with ID {member.id}", exc_info=ex)
 
     return SimpleResponse(message=message, delete_after=None)
-
-
-async def add_evidence_note(
-        user_id: int, action: str, reason: str, evidence: str, moderator_id: int
-) -> None:
-    """Add a note with evidence to the user's history records."""
-    if not evidence:
-        evidence = "none provided"
-    note = f"Reason for {action}: {reason} (Evidence: {evidence})"
-    today = arrow.utcnow().date()
-    user_note = UserNote(user_id=user_id, note=note, date=today, moderator_id=moderator_id)
-    async with AsyncSessionLocal() as session:
-        session.add(user_note)
-        await session.commit()

tests/src/cmds/core/test_ban.py

@@ -31,8 +31,7 @@ async def test_ban_success(self, ctx, bot):
         bot.get_member_or_user.return_value = user
 
         with (
-            patch('src.cmds.core.ban.ban_member', new_callable=AsyncMock) as ban_member_mock,
-            patch('src.cmds.core.ban.add_evidence_note', new_callable=AsyncMock) as add_evidence_note_mock
+            patch('src.cmds.core.ban.ban_member', new_callable=AsyncMock) as ban_member_mock
         ):
             ban_response = SimpleResponse(
                 message=f"Member {user.display_name} has been banned permanently.", delete_after=0
@@ -43,7 +42,6 @@ async def test_ban_success(self, ctx, bot):
             await cog.ban.callback(cog, ctx, user, "Any valid reason", "Some evidence")
 
             # Assertions
-            add_evidence_note_mock.assert_called_once_with(user.id, "ban", "Any valid reason", "Some evidence", ctx.user.id)
             ban_member_mock.assert_called_once_with(
                 bot, ctx.guild, user, "500w", "Any valid reason", "Some evidence", ctx.user, needs_approval=False
             )
@@ -57,9 +55,10 @@ async def test_tempban_success(self, ctx, bot):
         user = helpers.MockMember(id=2, name="Banned User")
         bot.get_member_or_user.return_value = user
 
-        with patch('src.helpers.ban.validate_duration', new_callable=AsyncMock) as validate_duration_mock, \
-             patch('src.cmds.core.ban.ban_member', new_callable=AsyncMock) as ban_member_mock, \
-             patch('src.cmds.core.ban.add_evidence_note', new_callable=AsyncMock) as add_evidence_note_mock:
+        with (
+            patch('src.helpers.ban.validate_duration', new_callable=AsyncMock) as validate_duration_mock, \
+            patch('src.cmds.core.ban.ban_member', new_callable=AsyncMock) as ban_member_mock
+        ):
             validate_duration_mock.return_value = (calendar.timegm(time.gmtime()) + parse_duration_str("5d"), "")
             ban_response = SimpleResponse(
                 message=f"Member {user.display_name} has been banned temporarily.", delete_after=0
@@ -70,7 +69,6 @@ async def test_tempban_success(self, ctx, bot):
             await cog.tempban.callback(cog, ctx, user, "5d", "Any valid reason", "Some evidence")
 
             # Assertions
-            add_evidence_note_mock.assert_called_once_with(user.id, "ban", "Any valid reason", "Some evidence", ctx.user.id)
             ban_member_mock.assert_called_once_with(
                 bot, ctx.guild, user, "5d", "Any valid reason", "Some evidence", ctx.user, needs_approval=True
             )
@@ -87,8 +85,7 @@ async def test_tempban_failed_with_wrong_duration(self, ctx, bot, guild):
 
         with (
             patch('src.helpers.ban.validate_duration', new_callable=AsyncMock) as validate_duration_mock,
-            patch('src.cmds.core.ban.ban_member', new_callable=AsyncMock) as ban_member_mock,
-            patch('src.cmds.core.ban.add_evidence_note', new_callable=AsyncMock) as add_evidence_note_mock,
+            patch('src.cmds.core.ban.ban_member', new_callable=AsyncMock) as ban_member_mock
         ):
             validate_duration_mock.return_value = (
                 0, "Malformed duration. Please use duration units, (e.g. 12h, 14d, 5w)."
@@ -102,9 +99,6 @@ async def test_tempban_failed_with_wrong_duration(self, ctx, bot, guild):
             await cog.tempban.callback(cog, ctx, user, "5", "Any valid reason", "Some evidence")
 
             # Assertions
-            add_evidence_note_mock.assert_called_once_with(
-                user.id, "ban", "Any valid reason", "Some evidence", ctx.user.id
-            )
             ban_member_mock.assert_called_once_with(
                 bot, ctx.guild, user, "5", "Any valid reason", "Some evidence", ctx.user, needs_approval=True
             )

tests/src/cmds/core/test_user.py

@@ -1,4 +1,3 @@
-from datetime import datetime
 from unittest.mock import AsyncMock, patch
 
 import pytest
@@ -24,16 +23,14 @@ async def test_kick_success(self, ctx, guild, bot, session):
 
         with (
             patch('src.cmds.core.user.add_infraction', new_callable=AsyncMock) as add_infraction_mock,
-            patch('src.cmds.core.user.add_evidence_note', new_callable=AsyncMock) as add_evidence_mock,
             patch('src.cmds.core.user.member_is_staff', return_value=False)
         ):
             cog = user.UserCog(bot)
             await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules")
 
             reason = "Violation of rules"
-            add_evidence_mock.assert_called_once_with(user_to_kick.id, "kick", reason, None, ctx.user.id)
             add_infraction_mock.assert_called_once_with(
-                ctx.guild, user_to_kick, 0, f"{ctx.user.name} was kicked on {datetime.now().strftime('%Y-%m-%d %H:%M:%S')} for {reason}", ctx.user
+                ctx.guild, user_to_kick, 0, f"Previously kicked for: {reason} - Evidence: None", ctx.user
             )
 
             # Assertions