From ce5fcdab852600342fe69211b038426ce2821107 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 4 Apr 2020 14:51:13 +0300 Subject: (Banning): Added logging and truncating to correct length for Discord Audit Log when ban reason length is more than 512 characters. --- bot/cogs/moderation/infractions.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index efa19f59e..f41484711 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -244,7 +244,14 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - action = ctx.guild.ban(user, reason=reason, delete_message_days=0) + if len(reason) > 512: + log.info("Ban reason is longer than 512 characters. Reason will be truncated for Audit Log.") + + action = ctx.guild.ban( + user, + reason=f"{reason[:509]}..." if len(reason) > 512 else reason, + delete_message_days=0 + ) await self.apply_infraction(ctx, infraction, user, action) if infraction.get('expires_at') is not None: -- cgit v1.2.3 From 6ad86ace9f34245180aefeed9553c407602602e8 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 4 Apr 2020 14:53:11 +0300 Subject: (Kick Command): Added logging and truncating to correct length for Discord Audit Log when kick reason length is more than 512 characters. --- bot/cogs/moderation/infractions.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index f41484711..f8c3e8da3 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -225,7 +225,10 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - action = user.kick(reason=reason) + if len(reason) > 512: + log.info("Kick reason is longer than 512 characters. Reason will be truncated for Audit Log.") + + action = user.kick(reason=f"{reason[:509]}..." if len(reason) > 512 else reason) await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() -- cgit v1.2.3 From 80e483a7ebaf57e6544429343c94cf0eed8821ef Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 4 Apr 2020 16:03:58 +0300 Subject: (Ban and Kick): Replaced force reason truncating with `textwrap.shorten`. --- bot/cogs/moderation/infractions.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index f8c3e8da3..a0bdf0d97 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -1,4 +1,5 @@ import logging +import textwrap import typing as t import discord @@ -228,7 +229,7 @@ class Infractions(InfractionScheduler, commands.Cog): if len(reason) > 512: log.info("Kick reason is longer than 512 characters. Reason will be truncated for Audit Log.") - action = user.kick(reason=f"{reason[:509]}..." if len(reason) > 512 else reason) + action = user.kick(textwrap.shorten(reason, width=509, placeholder="...") if len(reason) > 512 else reason) await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() @@ -252,7 +253,7 @@ class Infractions(InfractionScheduler, commands.Cog): action = ctx.guild.ban( user, - reason=f"{reason[:509]}..." if len(reason) > 512 else reason, + reason=textwrap.shorten(reason, width=509, placeholder="...") if len(reason) > 512 else reason, delete_message_days=0 ) await self.apply_infraction(ctx, infraction, user, action) -- cgit v1.2.3 From ec8cc8b02b0823deaa4ea2c97801d16d6aef5244 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 5 Apr 2020 18:13:38 +0300 Subject: (Ban and Kick): Applied simplification to reason truncating. --- bot/cogs/moderation/infractions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index a0bdf0d97..5bdea5755 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -229,7 +229,7 @@ class Infractions(InfractionScheduler, commands.Cog): if len(reason) > 512: log.info("Kick reason is longer than 512 characters. Reason will be truncated for Audit Log.") - action = user.kick(textwrap.shorten(reason, width=509, placeholder="...") if len(reason) > 512 else reason) + action = user.kick(reason=textwrap.shorten(reason, width=509, placeholder="...")) await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() @@ -253,7 +253,7 @@ class Infractions(InfractionScheduler, commands.Cog): action = ctx.guild.ban( user, - reason=textwrap.shorten(reason, width=509, placeholder="...") if len(reason) > 512 else reason, + reason=textwrap.shorten(reason, width=509, placeholder="..."), delete_message_days=0 ) await self.apply_infraction(ctx, infraction, user, action) -- cgit v1.2.3 From aad67373d3ac6d3d3a236d6734a6c22019dce120 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 5 Apr 2020 18:17:16 +0300 Subject: (Mod Scheduler): Added reason truncations to Scheduler's `apply_infraction` --- bot/cogs/moderation/scheduler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 917697be9..45e9d58ad 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -84,7 +84,8 @@ class InfractionScheduler(Scheduler): """Apply an infraction to the user, log the infraction, and optionally notify the user.""" infr_type = infraction["type"] icon = utils.INFRACTION_ICONS[infr_type][0] - reason = infraction["reason"] + # Truncate reason when it's too long to avoid raising error on sending ModLog entry + reason = textwrap.shorten(infraction["reason"], width=1900, placeholder="...") expiry = time.format_infraction_with_duration(infraction["expires_at"]) id_ = infraction['id'] -- cgit v1.2.3 From 025857541b7a0cbb77adf2a0282873c9e116169a Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 6 Apr 2020 08:48:32 +0300 Subject: (Ban and Kick): Changed length in `textwrap.shorten` from 309 to 312 because shorten already include `placeholder` to length. --- bot/cogs/moderation/infractions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 5bdea5755..7a044fc1c 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -229,7 +229,7 @@ class Infractions(InfractionScheduler, commands.Cog): if len(reason) > 512: log.info("Kick reason is longer than 512 characters. Reason will be truncated for Audit Log.") - action = user.kick(reason=textwrap.shorten(reason, width=509, placeholder="...")) + action = user.kick(reason=textwrap.shorten(reason, width=512, placeholder="...")) await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() @@ -253,7 +253,7 @@ class Infractions(InfractionScheduler, commands.Cog): action = ctx.guild.ban( user, - reason=textwrap.shorten(reason, width=509, placeholder="..."), + reason=textwrap.shorten(reason, width=512, placeholder="..."), delete_message_days=0 ) await self.apply_infraction(ctx, infraction, user, action) -- cgit v1.2.3 From c4f7359d2e301e6ab19666a6867c9cb69892da0b Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Mon, 6 Apr 2020 08:50:46 +0300 Subject: (Ban and Kick): Added space to `textwrap.shorten` `placeholder`. --- bot/cogs/moderation/infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 7a044fc1c..2c809535b 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -253,7 +253,7 @@ class Infractions(InfractionScheduler, commands.Cog): action = ctx.guild.ban( user, - reason=textwrap.shorten(reason, width=512, placeholder="..."), + reason=textwrap.shorten(reason, width=512, placeholder=" ..."), delete_message_days=0 ) await self.apply_infraction(ctx, infraction, user, action) -- cgit v1.2.3 From 482c3f4b475cdbe16b377dd5bb85910be0387166 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 09:04:15 +0300 Subject: (Mod Utils): Added shortening reason on embed creation in `notify_infraction`. --- bot/cogs/moderation/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index 3598f3b1f..9811d059f 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -135,7 +135,7 @@ async def notify_infraction( description=textwrap.dedent(f""" **Type:** {infr_type.capitalize()} **Expires:** {expires_at or "N/A"} - **Reason:** {reason or "No reason provided."} + **Reason:** {textwrap.shorten(reason, width=1500, placeholder="...") or "No reason provided."} """), colour=Colours.soft_red ) -- cgit v1.2.3 From a59092659271832a46c8ab0166031bffdc68c0a6 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 09:06:17 +0300 Subject: (Infractions): Removed unnecessary logging that notify when reason will be truncated for Audit Log. --- bot/cogs/moderation/infractions.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 2c809535b..d1e77311c 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -226,9 +226,6 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - if len(reason) > 512: - log.info("Kick reason is longer than 512 characters. Reason will be truncated for Audit Log.") - action = user.kick(reason=textwrap.shorten(reason, width=512, placeholder="...")) await self.apply_infraction(ctx, infraction, user, action) @@ -248,9 +245,6 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - if len(reason) > 512: - log.info("Ban reason is longer than 512 characters. Reason will be truncated for Audit Log.") - action = ctx.guild.ban( user, reason=textwrap.shorten(reason, width=512, placeholder=" ..."), -- cgit v1.2.3 From 42e18061a21e0ec1b8a4a692bc8d96f8ef1fd45b Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 09:09:10 +0300 Subject: (Infractions): Moved truncated reason to variable instead on ban coroutine creating. --- bot/cogs/moderation/infractions.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index d1e77311c..3340744b0 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -245,11 +245,9 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - action = ctx.guild.ban( - user, - reason=textwrap.shorten(reason, width=512, placeholder=" ..."), - delete_message_days=0 - ) + truncated_reason = textwrap.shorten(reason, width=512, placeholder=" ...") + + action = ctx.guild.ban(user, reason=truncated_reason, delete_message_days=0) await self.apply_infraction(ctx, infraction, user, action) if infraction.get('expires_at') is not None: -- cgit v1.2.3 From 10ea74bc5ed390c36d64a0f7413b8422f158708a Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 09:16:36 +0300 Subject: (Superstarify, Scheduler): Added reason shortening for ModLog. --- bot/cogs/moderation/scheduler.py | 2 +- bot/cogs/moderation/superstarify.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 45e9d58ad..7404ec8ac 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -326,7 +326,7 @@ class InfractionScheduler(Scheduler): log_text = { "Member": f"<@{user_id}>", "Actor": str(self.bot.get_user(actor) or actor), - "Reason": infraction["reason"], + "Reason": textwrap.shorten(infraction["reason"], width=1500, placeholder="..."), "Created": created, } diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index ca3dc4202..d77e61e6b 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -183,7 +183,7 @@ class Superstarify(InfractionScheduler, Cog): text=textwrap.dedent(f""" Member: {member.mention} (`{member.id}`) Actor: {ctx.message.author} - Reason: {reason} + Reason: {textwrap.shorten(reason, width=1500, placeholder="...")} Expires: {expiry_str} Old nickname: `{old_nick}` New nickname: `{forced_nick}` -- cgit v1.2.3 From 27e15c42e71f3d2df828d13a5e92c53c664b4431 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 09:20:04 +0300 Subject: (Scheduler): Changed reason truncating in `apply_infraction` from 1900 chars to 1500, added shortening to end message too. --- bot/cogs/moderation/scheduler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 7404ec8ac..345f08f19 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -85,7 +85,7 @@ class InfractionScheduler(Scheduler): infr_type = infraction["type"] icon = utils.INFRACTION_ICONS[infr_type][0] # Truncate reason when it's too long to avoid raising error on sending ModLog entry - reason = textwrap.shorten(infraction["reason"], width=1900, placeholder="...") + reason = textwrap.shorten(infraction["reason"], width=1500, placeholder="...") expiry = time.format_infraction_with_duration(infraction["expires_at"]) id_ = infraction['id'] @@ -128,7 +128,7 @@ class InfractionScheduler(Scheduler): f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) - end_msg = f" (reason: {infraction['reason']})" + end_msg = f" (reason: {textwrap.shorten(infraction['reason'], width=1500, placeholder='...')})" elif ctx.channel.id not in STAFF_CHANNELS: log.trace( f"Infraction #{id_} context is not in a staff channel; omitting infraction count." -- cgit v1.2.3 From 1100dba71b789bdc35c6c42d1b4003c7d28dcbb0 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 15:10:40 +0300 Subject: (ModLog): Added mod log item embed description truncating when it's too long. --- bot/cogs/moderation/modlog.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index c63b4bab9..e15a80c6d 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -2,6 +2,7 @@ import asyncio import difflib import itertools import logging +import textwrap import typing as t from datetime import datetime from itertools import zip_longest @@ -98,7 +99,7 @@ class ModLog(Cog, name="ModLog"): footer: t.Optional[str] = None, ) -> Context: """Generate log embed and send to logging channel.""" - embed = discord.Embed(description=text) + embed = discord.Embed(description=textwrap.shorten(text, width=2048, placeholder="...")) if title and icon_url: embed.set_author(name=title, icon_url=icon_url) -- cgit v1.2.3 From b765ecb280a3ba0ca017350a4c69dc9c07f97a67 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 15:12:56 +0300 Subject: (Scheduler): Removed reason truncation from `apply_infraction`, changed order of ModLog embed description item in same function. --- bot/cogs/moderation/scheduler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 345f08f19..fbb2d457b 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -85,7 +85,7 @@ class InfractionScheduler(Scheduler): infr_type = infraction["type"] icon = utils.INFRACTION_ICONS[infr_type][0] # Truncate reason when it's too long to avoid raising error on sending ModLog entry - reason = textwrap.shorten(infraction["reason"], width=1500, placeholder="...") + reason = infraction["reason"] expiry = time.format_infraction_with_duration(infraction["expires_at"]) id_ = infraction['id'] @@ -182,8 +182,8 @@ class InfractionScheduler(Scheduler): text=textwrap.dedent(f""" Member: {user.mention} (`{user.id}`) Actor: {ctx.message.author}{dm_log_text} - Reason: {reason} {expiry_log_text} + Reason: {reason} """), content=log_content, footer=f"ID {infraction['id']}" -- cgit v1.2.3 From 03028eab84a09c047c0ef879fc06fccacbe30420 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 15:18:38 +0300 Subject: (Mod Utils): Removed truncation of reason itself and added truncation to whole embed in `notify_infraction`. --- bot/cogs/moderation/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index 9811d059f..0423e5373 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -132,11 +132,11 @@ async def notify_infraction( log.trace(f"Sending {user} a DM about their {infr_type} infraction.") embed = discord.Embed( - description=textwrap.dedent(f""" + description=textwrap.shorten(textwrap.dedent(f""" **Type:** {infr_type.capitalize()} **Expires:** {expires_at or "N/A"} - **Reason:** {textwrap.shorten(reason, width=1500, placeholder="...") or "No reason provided."} - """), + **Reason:** {reason or "No reason provided."} + """), width=2048, placeholder="..."), colour=Colours.soft_red ) -- cgit v1.2.3 From 36f947c1cb52faedc1d2e00869e109f2cde12c11 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 8 Apr 2020 15:19:44 +0300 Subject: (Superstarify): Removed unnecessary truncation on `superstarify` command, reordered ModLog text. --- bot/cogs/moderation/superstarify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index d77e61e6b..e221ad909 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -183,10 +183,10 @@ class Superstarify(InfractionScheduler, Cog): text=textwrap.dedent(f""" Member: {member.mention} (`{member.id}`) Actor: {ctx.message.author} - Reason: {textwrap.shorten(reason, width=1500, placeholder="...")} Expires: {expiry_str} Old nickname: `{old_nick}` New nickname: `{forced_nick}` + Reason: {reason} """), footer=f"ID {id_}" ) -- cgit v1.2.3 From 8d283fb8eefd7be288702f88653aaebcdcda37c3 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 08:12:09 +0300 Subject: (Mod Utils): Moved embed description to variable. --- bot/cogs/moderation/utils.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index 0423e5373..fc8c26031 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -131,12 +131,14 @@ async def notify_infraction( """DM a user about their new infraction and return True if the DM is successful.""" log.trace(f"Sending {user} a DM about their {infr_type} infraction.") + text = textwrap.dedent(f""" + **Type:** {infr_type.capitalize()} + **Expires:** {expires_at or "N/A"} + **Reason:** {reason or "No reason provided."} + """) + embed = discord.Embed( - description=textwrap.shorten(textwrap.dedent(f""" - **Type:** {infr_type.capitalize()} - **Expires:** {expires_at or "N/A"} - **Reason:** {reason or "No reason provided."} - """), width=2048, placeholder="..."), + description=textwrap.shorten(text, width=2048, placeholder="..."), colour=Colours.soft_red ) -- cgit v1.2.3 From b148beeec8c897d91fa100d0bbd1cb4965f58e6e Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 08:27:10 +0300 Subject: (Scheduler): Move reason to end of log text to avoid truncating keys. --- bot/cogs/moderation/scheduler.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index fbb2d457b..3352806e7 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -283,6 +283,9 @@ class InfractionScheduler(Scheduler): f"{log_text.get('Failure', '')}" ) + # Move reason to end of entry to avoid cutting out some keys + log_text["Reason"] = log_text.pop("Reason") + # Send a log message to the mod log. await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS[infr_type][1], @@ -326,7 +329,7 @@ class InfractionScheduler(Scheduler): log_text = { "Member": f"<@{user_id}>", "Actor": str(self.bot.get_user(actor) or actor), - "Reason": textwrap.shorten(infraction["reason"], width=1500, placeholder="..."), + "Reason": infraction["reason"], "Created": created, } @@ -396,6 +399,9 @@ class InfractionScheduler(Scheduler): user = self.bot.get_user(user_id) avatar = user.avatar_url_as(static_format="png") if user else None + # Move reason to end so when reason is too long, this is not gonna cut out required items. + log_text["Reason"] = log_text.pop("Reason") + log.trace(f"Sending deactivation mod log for infraction #{id_}.") await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS[type_][1], @@ -405,7 +411,6 @@ class InfractionScheduler(Scheduler): text="\n".join(f"{k}: {v}" for k, v in log_text.items()), footer=f"ID: {id_}", content=log_content, - ) return log_text -- cgit v1.2.3 From 816e76fe8c5e5231cdc85ab974294c1d2fb4a87c Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 08:28:44 +0300 Subject: (Scheduler): Replaced `infraction['reason']` with `reason` variable using in `end_msg`. --- bot/cogs/moderation/scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 3352806e7..b238cf4e2 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -128,7 +128,7 @@ class InfractionScheduler(Scheduler): f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) - end_msg = f" (reason: {textwrap.shorten(infraction['reason'], width=1500, placeholder='...')})" + end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" elif ctx.channel.id not in STAFF_CHANNELS: log.trace( f"Infraction #{id_} context is not in a staff channel; omitting infraction count." -- cgit v1.2.3 From 3bbd10ac91e9677e24588734d256a0558c0b46a2 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 09:52:19 +0300 Subject: (Talent Pool): Applied reason shortening. --- bot/cogs/watchchannels/talentpool.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/cogs/watchchannels/talentpool.py b/bot/cogs/watchchannels/talentpool.py index ad0c51fa6..15af7e34d 100644 --- a/bot/cogs/watchchannels/talentpool.py +++ b/bot/cogs/watchchannels/talentpool.py @@ -106,8 +106,8 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): if history: total = f"({len(history)} previous nominations in total)" - start_reason = f"Watched: {history[0]['reason']}" - end_reason = f"Unwatched: {history[0]['end_reason']}" + start_reason = f"Watched: {textwrap.shorten(history[0]['reason'], width=500, placeholder='...')}" + end_reason = f"Unwatched: {textwrap.shorten(history[0]['end_reason'], width=500, placeholder='...')}" msg += f"\n\nUser's previous watch reasons {total}:```{start_reason}\n\n{end_reason}```" await ctx.send(msg) @@ -224,7 +224,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): Status: **Active** Date: {start_date} Actor: {actor.mention if actor else actor_id} - Reason: {nomination_object["reason"]} + Reason: {textwrap.shorten(nomination_object["reason"], width=200, placeholder="...")} Nomination ID: `{nomination_object["id"]}` =============== """ @@ -237,10 +237,10 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): Status: Inactive Date: {start_date} Actor: {actor.mention if actor else actor_id} - Reason: {nomination_object["reason"]} + Reason: {textwrap.shorten(nomination_object["reason"], width=200, placeholder="...")} End date: {end_date} - Unwatch reason: {nomination_object["end_reason"]} + Unwatch reason: {textwrap.shorten(nomination_object["end_reason"], width=200, placeholder="...")} Nomination ID: `{nomination_object["id"]}` =============== """ -- cgit v1.2.3 From 2c9bc9f6fe5174096a6177560acd91f869c296ef Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 11:30:51 +0300 Subject: (Watchchannel): Added footer shortening. --- bot/cogs/watchchannels/watchchannel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/cogs/watchchannels/watchchannel.py b/bot/cogs/watchchannels/watchchannel.py index 479820444..ac1aa38ee 100644 --- a/bot/cogs/watchchannels/watchchannel.py +++ b/bot/cogs/watchchannels/watchchannel.py @@ -280,8 +280,9 @@ class WatchChannel(metaclass=CogABCMeta): else: message_jump = f"in [#{msg.channel.name}]({msg.jump_url})" + footer = f"Added {time_delta} by {actor} | Reason: {reason}" embed = Embed(description=f"{msg.author.mention} {message_jump}") - embed.set_footer(text=f"Added {time_delta} by {actor} | Reason: {reason}") + embed.set_footer(text=textwrap.shorten(footer, width=128, placeholder="...")) await self.webhook_send(embed=embed, username=msg.author.display_name, avatar_url=msg.author.avatar_url) -- cgit v1.2.3 From 472b58b3fb1b3d8695d9de1a13ce92f129e6bcc4 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 11:34:54 +0300 Subject: (Big Brother): Added truncating reason. --- bot/cogs/watchchannels/bigbrother.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/watchchannels/bigbrother.py b/bot/cogs/watchchannels/bigbrother.py index 903c87f85..69df849f0 100644 --- a/bot/cogs/watchchannels/bigbrother.py +++ b/bot/cogs/watchchannels/bigbrother.py @@ -1,4 +1,5 @@ import logging +import textwrap from collections import ChainMap from discord.ext.commands import Cog, Context, group @@ -97,8 +98,8 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): if len(history) > 1: total = f"({len(history) // 2} previous infractions in total)" - end_reason = history[0]["reason"] - start_reason = f"Watched: {history[1]['reason']}" + end_reason = textwrap.shorten(history[0]["reason"], width=500, placeholder="...") + start_reason = f"Watched: {textwrap.shorten(history[1]['reason'], width=500, placeholder='...')}" msg += f"\n\nUser's previous watch reasons {total}:```{start_reason}\n\n{end_reason}```" else: msg = ":x: Failed to post the infraction: response was empty." -- cgit v1.2.3 From 39bac8873120801eb51a2c1a996d2760d9af64a4 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Tue, 14 Apr 2020 16:32:43 +0300 Subject: (ModLog): Applied force embed description truncating in `send_log_message` to avoid removing newlines. --- bot/cogs/moderation/modlog.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index e15a80c6d..fcc9d4e0a 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -99,7 +99,10 @@ class ModLog(Cog, name="ModLog"): footer: t.Optional[str] = None, ) -> Context: """Generate log embed and send to logging channel.""" - embed = discord.Embed(description=textwrap.shorten(text, width=2048, placeholder="...")) + # Truncate string directly here to avoid removing newlines + embed = discord.Embed( + description=text[:2046] + "..." if len(text) > 2048 else text + ) if title and icon_url: embed.set_author(name=title, icon_url=icon_url) -- cgit v1.2.3 From 6928c492c1d989567c47dfd49a396730c6c8bb27 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Tue, 14 Apr 2020 18:29:55 +0300 Subject: (Scheduler): Removed empty line when expiration not specified in `apply_infraction`. --- bot/cogs/moderation/scheduler.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index b238cf4e2..5b59b4d4b 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -102,7 +102,7 @@ class InfractionScheduler(Scheduler): dm_result = "" dm_log_text = "" - expiry_log_text = f"Expires: {expiry}" if expiry else "" + expiry_log_text = f"\nExpires: {expiry}" if expiry else "" log_title = "applied" log_content = None @@ -181,8 +181,7 @@ class InfractionScheduler(Scheduler): thumbnail=user.avatar_url_as(static_format="png"), text=textwrap.dedent(f""" Member: {user.mention} (`{user.id}`) - Actor: {ctx.message.author}{dm_log_text} - {expiry_log_text} + Actor: {ctx.message.author}{dm_log_text} {expiry_log_text} Reason: {reason} """), content=log_content, -- cgit v1.2.3 From 32a5bf97e31addb32d17bd0479bc4cf2f4dd9eb7 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Tue, 14 Apr 2020 18:50:24 +0300 Subject: (Scheduler): Added removal of infraction in DB, when applying infraction fail. Also don't send DM in this case. --- bot/cogs/moderation/scheduler.py | 46 ++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 5b59b4d4b..58e363da6 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -105,23 +105,7 @@ class InfractionScheduler(Scheduler): expiry_log_text = f"\nExpires: {expiry}" if expiry else "" log_title = "applied" log_content = None - - # DM the user about the infraction if it's not a shadow/hidden infraction. - if not infraction["hidden"]: - dm_result = f"{constants.Emojis.failmail} " - dm_log_text = "\nDM: **Failed**" - - # Sometimes user is a discord.Object; make it a proper user. - try: - if not isinstance(user, (discord.Member, discord.User)): - user = await self.bot.fetch_user(user.id) - except discord.HTTPException as e: - log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})") - else: - # Accordingly display whether the user was successfully notified via DM. - if await utils.notify_infraction(user, infr_type, expiry, reason, icon): - dm_result = ":incoming_envelope: " - dm_log_text = "\nDM: Sent" + failed = False if infraction["actor"] == self.bot.user.id: log.trace( @@ -165,11 +149,37 @@ class InfractionScheduler(Scheduler): log.warning(f"{log_msg}: bot lacks permissions.") else: log.exception(log_msg) + failed = True + + # DM the user about the infraction if it's not a shadow/hidden infraction. + # Don't send DM when applying failed. + if not infraction["hidden"] and not failed: + dm_result = f"{constants.Emojis.failmail} " + dm_log_text = "\nDM: **Failed**" + + # Sometimes user is a discord.Object; make it a proper user. + try: + if not isinstance(user, (discord.Member, discord.User)): + user = await self.bot.fetch_user(user.id) + except discord.HTTPException as e: + log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})") + else: + # Accordingly display whether the user was successfully notified via DM. + if await utils.notify_infraction(user, infr_type, expiry, reason, icon): + dm_result = ":incoming_envelope: " + dm_log_text = "\nDM: Sent" + + if failed: + dm_log_text = "\nDM: **Canceled**" + dm_result = f"{constants.Emojis.failmail} " + log.trace(f"Deleted infraction {infraction['id']} from database because applying infraction failed.") + await self.bot.api_client.delete(f"bot/infractions/{infraction['id']}") # Send a confirmation message to the invoking context. log.trace(f"Sending infraction #{id_} confirmation message.") await ctx.send( - f"{dm_result}{confirm_msg} **{infr_type}** to {user.mention}{expiry_msg}{end_msg}." + f"{dm_result}{confirm_msg} " + f"{f'**{infr_type}** to {user.mention}{expiry_msg}{end_msg}' if not failed else ''}." ) # Send a log message to the mod log. -- cgit v1.2.3 From 085decd12867f89a0803806928741fe6dd3c76bb Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 15 Apr 2020 08:18:19 +0300 Subject: (Test Helpers): Added `__ge__` function to `MockRole` for comparing. --- tests/helpers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 8e13f0f28..227bac95f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -205,6 +205,10 @@ class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """Simplified position-based comparisons similar to those of `discord.Role`.""" return self.position < other.position + def __ge__(self, other): + """Simplified position-based comparisons similar to those of `discord.Role`.""" + return self.position >= other.position + # Create a Member instance to get a realistic Mock of `discord.Member` member_data = {'user': 'lemon', 'roles': [1]} -- cgit v1.2.3 From 81f6efc2f4e9e157e2f7fb9f191ea410af066632 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 16 Apr 2020 11:15:16 +0300 Subject: (Infraction Tests): Created reason shortening tests for ban and kick. --- tests/bot/cogs/moderation/test_infractions.py | 54 +++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 tests/bot/cogs/moderation/test_infractions.py diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py new file mode 100644 index 000000000..39ea93952 --- /dev/null +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -0,0 +1,54 @@ +import textwrap +import unittest +from unittest.mock import AsyncMock, Mock, patch + +from bot.cogs.moderation.infractions import Infractions +from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockRole + + +class ShorteningTests(unittest.IsolatedAsyncioTestCase): + """Tests for ban and kick command reason shortening.""" + + def setUp(self): + self.bot = MockBot() + self.cog = Infractions(self.bot) + self.user = MockMember(id=1234, top_role=MockRole(id=3577, position=10)) + self.target = MockMember(id=1265, top_role=MockRole(id=9876, position=0)) + self.guild = MockGuild(id=4567) + self.ctx = MockContext(bot=self.bot, author=self.user, guild=self.guild) + + @patch("bot.cogs.moderation.utils.has_active_infraction") + @patch("bot.cogs.moderation.utils.post_infraction") + async def test_apply_ban_reason_shortening(self, post_infraction_mock, has_active_mock): + """Should truncate reason for `ctx.guild.ban`.""" + has_active_mock.return_value = False + post_infraction_mock.return_value = {"foo": "bar"} + + self.cog.apply_infraction = AsyncMock() + self.bot.get_cog.return_value = AsyncMock() + self.cog.mod_log.ignore = Mock() + + await self.cog.apply_ban(self.ctx, self.target, "foo bar" * 3000) + ban = self.cog.apply_infraction.call_args[0][3] + self.assertEqual( + ban.cr_frame.f_locals["kwargs"]["reason"], + textwrap.shorten("foo bar" * 3000, 512, placeholder=" ...") + ) + # Await ban to avoid warning + await ban + + @patch("bot.cogs.moderation.utils.post_infraction") + async def test_apply_kick_reason_shortening(self, post_infraction_mock) -> None: + """Should truncate reason for `Member.kick`.""" + post_infraction_mock.return_value = {"foo": "bar"} + + self.cog.apply_infraction = AsyncMock() + self.cog.mod_log.ignore = Mock() + + await self.cog.apply_kick(self.ctx, self.target, "foo bar" * 3000) + kick = self.cog.apply_infraction.call_args[0][3] + self.assertEqual( + kick.cr_frame.f_locals["kwargs"]["reason"], + textwrap.shorten("foo bar" * 3000, 512, placeholder="...") + ) + await kick -- cgit v1.2.3 From 216953044a870f2440fe44fcd2f9ca3ee7cf37e9 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 16 Apr 2020 11:30:09 +0300 Subject: (ModLog Tests): Created reason shortening tests for `send_log_message`. --- tests/bot/cogs/moderation/test_modlog.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/bot/cogs/moderation/test_modlog.py diff --git a/tests/bot/cogs/moderation/test_modlog.py b/tests/bot/cogs/moderation/test_modlog.py new file mode 100644 index 000000000..46e01d2ea --- /dev/null +++ b/tests/bot/cogs/moderation/test_modlog.py @@ -0,0 +1,29 @@ +import unittest + +import discord + +from bot.cogs.moderation.modlog import ModLog +from tests.helpers import MockBot, MockTextChannel + + +class ModLogTests(unittest.IsolatedAsyncioTestCase): + """Tests for moderation logs.""" + + def setUp(self): + self.bot = MockBot() + self.cog = ModLog(self.bot) + self.channel = MockTextChannel() + + async def test_log_entry_description_shortening(self): + """Should truncate embed description for ModLog entry.""" + self.bot.get_channel.return_value = self.channel + await self.cog.send_log_message( + icon_url="foo", + colour=discord.Colour.blue(), + title="bar", + text="foo bar" * 3000 + ) + embed = self.channel.send.call_args[1]["embed"] + self.assertEqual( + embed.description, ("foo bar" * 3000)[:2046] + "..." + ) -- cgit v1.2.3 From b463cb2d21683b7184698f788419d325e2f5f5cc Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 16 Apr 2020 11:33:21 +0300 Subject: (ModLog): Removed unused `textwrap` import. --- bot/cogs/moderation/modlog.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index fcc9d4e0a..eb8bd65cf 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -2,7 +2,6 @@ import asyncio import difflib import itertools import logging -import textwrap import typing as t from datetime import datetime from itertools import zip_longest -- cgit v1.2.3 From 1a3fa6a395141c4fcdd1d388d6ce3e7bd89bcbf0 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 16 Apr 2020 13:40:47 +0300 Subject: (Infractions and ModLog Tests): Replaced `shortening` with `truncation`, removed unnecessary type hint and added comment to kick truncation test about awaiting `kick`. --- tests/bot/cogs/moderation/test_infractions.py | 9 +++++---- tests/bot/cogs/moderation/test_modlog.py | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index 39ea93952..51a8cc645 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -6,8 +6,8 @@ from bot.cogs.moderation.infractions import Infractions from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockRole -class ShorteningTests(unittest.IsolatedAsyncioTestCase): - """Tests for ban and kick command reason shortening.""" +class TruncationTests(unittest.IsolatedAsyncioTestCase): + """Tests for ban and kick command reason truncation.""" def setUp(self): self.bot = MockBot() @@ -19,7 +19,7 @@ class ShorteningTests(unittest.IsolatedAsyncioTestCase): @patch("bot.cogs.moderation.utils.has_active_infraction") @patch("bot.cogs.moderation.utils.post_infraction") - async def test_apply_ban_reason_shortening(self, post_infraction_mock, has_active_mock): + async def test_apply_ban_reason_truncation(self, post_infraction_mock, has_active_mock): """Should truncate reason for `ctx.guild.ban`.""" has_active_mock.return_value = False post_infraction_mock.return_value = {"foo": "bar"} @@ -38,7 +38,7 @@ class ShorteningTests(unittest.IsolatedAsyncioTestCase): await ban @patch("bot.cogs.moderation.utils.post_infraction") - async def test_apply_kick_reason_shortening(self, post_infraction_mock) -> None: + async def test_apply_kick_reason_truncation(self, post_infraction_mock): """Should truncate reason for `Member.kick`.""" post_infraction_mock.return_value = {"foo": "bar"} @@ -51,4 +51,5 @@ class ShorteningTests(unittest.IsolatedAsyncioTestCase): kick.cr_frame.f_locals["kwargs"]["reason"], textwrap.shorten("foo bar" * 3000, 512, placeholder="...") ) + # Await kick to avoid warning await kick diff --git a/tests/bot/cogs/moderation/test_modlog.py b/tests/bot/cogs/moderation/test_modlog.py index 46e01d2ea..d60836474 100644 --- a/tests/bot/cogs/moderation/test_modlog.py +++ b/tests/bot/cogs/moderation/test_modlog.py @@ -14,7 +14,7 @@ class ModLogTests(unittest.IsolatedAsyncioTestCase): self.cog = ModLog(self.bot) self.channel = MockTextChannel() - async def test_log_entry_description_shortening(self): + async def test_log_entry_description_truncation(self): """Should truncate embed description for ModLog entry.""" self.bot.get_channel.return_value = self.channel await self.cog.send_log_message( -- cgit v1.2.3 From 601ff03823deb842d74f4689fecb68f7ce1693e6 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 18:11:44 +0200 Subject: AntiMalware Tests - Added unittest for message without attachment --- tests/bot/cogs/test_antimalware.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/bot/cogs/test_antimalware.py diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py new file mode 100644 index 000000000..41ca19e17 --- /dev/null +++ b/tests/bot/cogs/test_antimalware.py @@ -0,0 +1,20 @@ +import asyncio +import unittest + +from bot.cogs import antimalware +from tests.helpers import MockBot, MockMessage + + +class AntiMalwareCogTests(unittest.TestCase): + """Test the AntiMalware cog.""" + + def setUp(self): + """Sets up fresh objects for each test.""" + self.bot = MockBot() + self.cog = antimalware.AntiMalware(self.bot) + self.message = MockMessage() + + def test_message_without_attachment(self): + """Messages without attachments should result in no action.""" + coroutine = self.cog.on_message(self.message) + self.assertIsNone(asyncio.run(coroutine)) -- cgit v1.2.3 From 9889f0fdd1ba403ae50ba20be38feca0932d1dda Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 19:06:46 +0200 Subject: AntiMalware Tests - Added unittests for deletion of message and ignoring of dms --- tests/bot/cogs/test_antimalware.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 41ca19e17..ebf3a1277 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,8 +1,9 @@ import asyncio import unittest +from unittest.mock import AsyncMock from bot.cogs import antimalware -from tests.helpers import MockBot, MockMessage +from tests.helpers import MockAttachment, MockBot, MockMessage class AntiMalwareCogTests(unittest.TestCase): @@ -13,8 +14,27 @@ class AntiMalwareCogTests(unittest.TestCase): self.bot = MockBot() self.cog = antimalware.AntiMalware(self.bot) self.message = MockMessage() + self.message.delete = AsyncMock() def test_message_without_attachment(self): """Messages without attachments should result in no action.""" coroutine = self.cog.on_message(self.message) self.assertIsNone(asyncio.run(coroutine)) + self.message.delete.assert_not_called() + + def test_direct_message_with_attachment(self): + """Direct messages should have no action taken.""" + attachment = MockAttachment(filename="python.asdfsff") + self.message.attachments = [attachment] + self.message.guild = None + coroutine = self.cog.on_message(self.message) + asyncio.run(coroutine) + self.message.delete.assert_not_called() + + def test_message_with_illegal_extension_gets_deleted(self): + """A message containing an illegal extension should send an embed.""" + attachment = MockAttachment(filename="python.asdfsff") + self.message.attachments = [attachment] + coroutine = self.cog.on_message(self.message) + asyncio.run(coroutine) + self.message.delete.assert_called_once() -- cgit v1.2.3 From 90d2ce0e3717d4ddf79eb986e22f7542ca1770e1 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 19:13:45 +0200 Subject: AntiMalware Tests - Added unittest for messages send by staff --- tests/bot/cogs/test_antimalware.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index ebf3a1277..e3fd477fa 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -3,7 +3,8 @@ import unittest from unittest.mock import AsyncMock from bot.cogs import antimalware -from tests.helpers import MockAttachment, MockBot, MockMessage +from bot.constants import Roles +from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole class AntiMalwareCogTests(unittest.TestCase): @@ -38,3 +39,13 @@ class AntiMalwareCogTests(unittest.TestCase): coroutine = self.cog.on_message(self.message) asyncio.run(coroutine) self.message.delete.assert_called_once() + + def test_message_send_by_staff(self): + """A message send by a member of staff should be ignored.""" + moderator_role = MockRole(name="Moderator", id=Roles.moderators) + self.message.author.roles.append(moderator_role) + attachment = MockAttachment(filename="python.asdfsff") + self.message.attachments = [attachment] + coroutine = self.cog.on_message(self.message) + asyncio.run(coroutine) + self.message.delete.assert_not_called() -- cgit v1.2.3 From 3913a8eba46bf98bd09e13145da33f7a09f77960 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 19:45:57 +0200 Subject: AntiMalware Tests - Added unittest for the embed for a python file. --- tests/bot/cogs/test_antimalware.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index e3fd477fa..0bb5af943 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -3,7 +3,7 @@ import unittest from unittest.mock import AsyncMock from bot.cogs import antimalware -from bot.constants import Roles +from bot.constants import Roles, URLs from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole @@ -28,16 +28,20 @@ class AntiMalwareCogTests(unittest.TestCase): attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] self.message.guild = None + coroutine = self.cog.on_message(self.message) asyncio.run(coroutine) + self.message.delete.assert_not_called() def test_message_with_illegal_extension_gets_deleted(self): """A message containing an illegal extension should send an embed.""" attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] + coroutine = self.cog.on_message(self.message) asyncio.run(coroutine) + self.message.delete.assert_called_once() def test_message_send_by_staff(self): @@ -46,6 +50,25 @@ class AntiMalwareCogTests(unittest.TestCase): self.message.author.roles.append(moderator_role) attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] + coroutine = self.cog.on_message(self.message) asyncio.run(coroutine) + self.message.delete.assert_not_called() + + def test_python_file_redirect_embed(self): + """A message containing a .python file should result in an embed redirecting the user to our paste site""" + attachment = MockAttachment(filename="python.py") + self.message.attachments = [attachment] + self.message.channel.send = AsyncMock() + + coroutine = self.cog.on_message(self.message) + asyncio.run(coroutine) + args, kwargs = self.message.channel.send.call_args + embed = kwargs.pop("embed") + + self.assertEqual(args[0], f"Hey {self.message.author.mention}!") + self.assertEqual(embed.description, ( + "It looks like you tried to attach a Python file - " + f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" + )) -- cgit v1.2.3 From 19c15d957040b6857a4141e15c32fd0526f9920d Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 20:15:17 +0200 Subject: AntiMalware Tests - Added unittest for messages that were deleted in the meantime. --- tests/bot/cogs/test_antimalware.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 0bb5af943..da5cd9d11 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,6 +1,9 @@ import asyncio +import logging import unittest -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, Mock + +from discord import NotFound from bot.cogs import antimalware from bot.constants import Roles, URLs @@ -72,3 +75,18 @@ class AntiMalwareCogTests(unittest.TestCase): "It looks like you tried to attach a Python file - " f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" )) + + def test_removing_deleted_message_logs(self): + """Removing an already deleted message logs the correct message""" + attachment = MockAttachment(filename="python.py") + self.message.attachments = [attachment] + self.message.delete = AsyncMock(side_effect=NotFound(response=Mock(status=""), message="")) + + coroutine = self.cog.on_message(self.message) + logger = logging.getLogger("bot.cogs.antimalware") + + with self.assertLogs(logger=logger, level="INFO") as logs: + asyncio.run(coroutine) + self.assertIn( + f"INFO:bot.cogs.antimalware:Tried to delete message `{self.message.id}`, but message could not be found.", + logs.output) -- cgit v1.2.3 From 4a0b3ea1ef182ddbbb1f9d731b28768a049a531d Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 20:23:00 +0200 Subject: AntiMalware Tests - Added unittest for cog setup --- tests/bot/cogs/test_antimalware.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index da5cd9d11..67c640d23 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -90,3 +90,13 @@ class AntiMalwareCogTests(unittest.TestCase): self.assertIn( f"INFO:bot.cogs.antimalware:Tried to delete message `{self.message.id}`, but message could not be found.", logs.output) + + +class AntiMalwareSetupTests(unittest.TestCase): + """Tests setup of the `AntiMalware` cog.""" + + def test_setup(self): + """Setup of the extension should call add_cog.""" + bot = MockBot() + antimalware.setup(bot) + bot.add_cog.assert_called_once() -- cgit v1.2.3 From 3090141f673279f2836cb3aca95397eb9950ad0f Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 20:41:31 +0200 Subject: AntiMalware Tests - Added unittest message deletion log --- tests/bot/cogs/test_antimalware.py | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 67c640d23..b4e31b5ce 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,14 +1,17 @@ import asyncio import logging import unittest +from os.path import splitext from unittest.mock import AsyncMock, Mock from discord import NotFound from bot.cogs import antimalware -from bot.constants import Roles, URLs +from bot.constants import AntiMalware as AntiMalwareConfig, Roles, URLs from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole +MODULE = "bot.cogs.antimalware" + class AntiMalwareCogTests(unittest.TestCase): """Test the AntiMalware cog.""" @@ -78,17 +81,38 @@ class AntiMalwareCogTests(unittest.TestCase): def test_removing_deleted_message_logs(self): """Removing an already deleted message logs the correct message""" - attachment = MockAttachment(filename="python.py") + attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] self.message.delete = AsyncMock(side_effect=NotFound(response=Mock(status=""), message="")) coroutine = self.cog.on_message(self.message) - logger = logging.getLogger("bot.cogs.antimalware") + logger = logging.getLogger(MODULE) with self.assertLogs(logger=logger, level="INFO") as logs: asyncio.run(coroutine) self.assertIn( - f"INFO:bot.cogs.antimalware:Tried to delete message `{self.message.id}`, but message could not be found.", + f"INFO:{MODULE}:Tried to delete message `{self.message.id}`, but message could not be found.", + logs.output) + + def test_message_with_illegal_attachment_logs(self): + """Deleting a message with an illegal attachment should result in a log.""" + attachment = MockAttachment(filename="python.asdfsff") + self.message.attachments = [attachment] + + coroutine = self.cog.on_message(self.message) + file_extensions = {splitext(attachment.filename.lower())[1] for attachment in self.message.attachments} + extensions_blocked = file_extensions - set(AntiMalwareConfig.whitelist) + blocked_extensions_str = ', '.join(extensions_blocked) + logger = logging.getLogger(MODULE) + + with self.assertLogs(logger=logger, level="INFO") as logs: + asyncio.run(coroutine) + self.assertEqual( + [ + f"INFO:{MODULE}:" + f"User '{self.message.author}' ({self.message.author.id}) " + f"uploaded blacklisted file(s): {blocked_extensions_str}" + ], logs.output) -- cgit v1.2.3 From f0bc9d800dd141b9126c48251a80618e138d61f1 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 20:46:15 +0200 Subject: AntiMalware Tests - Added unittest for valid attachment --- tests/bot/cogs/test_antimalware.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index b4e31b5ce..407fa05c1 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -23,6 +23,15 @@ class AntiMalwareCogTests(unittest.TestCase): self.message = MockMessage() self.message.delete = AsyncMock() + def test_message_with_allowed_attachment(self): + """Messages with allowed extensions should not be deleted""" + attachment = MockAttachment(filename=f"python.{AntiMalwareConfig.whitelist[0]}") + self.message.attachments = [attachment] + + coroutine = self.cog.on_message(self.message) + asyncio.run(coroutine) + self.message.delete.assert_not_called() + def test_message_without_attachment(self): """Messages without attachments should result in no action.""" coroutine = self.cog.on_message(self.message) -- cgit v1.2.3 From 75f6ca6bd9b695a5deb4a4d78311bc63eb2a74d0 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 21:04:47 +0200 Subject: AntiMalware Tests - Added unittest for txt file attachment --- tests/bot/cogs/test_antimalware.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 407fa05c1..eba439afb 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, Mock from discord import NotFound from bot.cogs import antimalware -from bot.constants import AntiMalware as AntiMalwareConfig, Roles, URLs +from bot.constants import AntiMalware as AntiMalwareConfig, Channels, Roles, URLs from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole MODULE = "bot.cogs.antimalware" @@ -21,7 +21,6 @@ class AntiMalwareCogTests(unittest.TestCase): self.bot = MockBot() self.cog = antimalware.AntiMalware(self.bot) self.message = MockMessage() - self.message.delete = AsyncMock() def test_message_with_allowed_attachment(self): """Messages with allowed extensions should not be deleted""" @@ -88,6 +87,28 @@ class AntiMalwareCogTests(unittest.TestCase): f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" )) + def test_txt_file_redirect_embed(self): + attachment = MockAttachment(filename="python.txt") + self.message.attachments = [attachment] + self.message.channel.send = AsyncMock() + + coroutine = self.cog.on_message(self.message) + asyncio.run(coroutine) + args, kwargs = self.message.channel.send.call_args + embed = kwargs.pop("embed") + cmd_channel = self.bot.get_channel(Channels.bot_commands) + + self.assertEqual(args[0], f"Hey {self.message.author.mention}!") + self.assertEqual(embed.description, ( + "**Uh-oh!** It looks like your message got zapped by our spam filter. " + "We currently don't allow `.txt` attachments, so here are some tips to help you travel safely: \n\n" + "• If you attempted to send a message longer than 2000 characters, try shortening your message " + "to fit within the character limit or use a pasting service (see below) \n\n" + "• If you tried to show someone your code, you can use codeblocks \n(run `!code-blocks` in " + f"{cmd_channel.mention} for more information) or use a pasting service like: " + f"\n\n{URLs.site_schema}{URLs.site_paste}" + )) + def test_removing_deleted_message_logs(self): """Removing an already deleted message logs the correct message""" attachment = MockAttachment(filename="python.asdfsff") -- cgit v1.2.3 From c8bf44e30c286b27768601d5a04cd2459f170d4c Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Thu, 7 May 2020 21:29:15 +0200 Subject: AntiMalware Tests - Switched to unittest.IsolatedAsyncioTestCase --- tests/bot/cogs/test_antimalware.py | 48 +++++++++++++++----------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index eba439afb..6fb7b399e 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,4 +1,3 @@ -import asyncio import logging import unittest from os.path import splitext @@ -13,7 +12,7 @@ from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole MODULE = "bot.cogs.antimalware" -class AntiMalwareCogTests(unittest.TestCase): +class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): """Test the AntiMalware cog.""" def setUp(self): @@ -22,62 +21,56 @@ class AntiMalwareCogTests(unittest.TestCase): self.cog = antimalware.AntiMalware(self.bot) self.message = MockMessage() - def test_message_with_allowed_attachment(self): + async def test_message_with_allowed_attachment(self): """Messages with allowed extensions should not be deleted""" attachment = MockAttachment(filename=f"python.{AntiMalwareConfig.whitelist[0]}") self.message.attachments = [attachment] - coroutine = self.cog.on_message(self.message) - asyncio.run(coroutine) + await self.cog.on_message(self.message) self.message.delete.assert_not_called() - def test_message_without_attachment(self): + async def test_message_without_attachment(self): """Messages without attachments should result in no action.""" - coroutine = self.cog.on_message(self.message) - self.assertIsNone(asyncio.run(coroutine)) + self.assertIsNone(await self.cog.on_message(self.message)) self.message.delete.assert_not_called() - def test_direct_message_with_attachment(self): + async def test_direct_message_with_attachment(self): """Direct messages should have no action taken.""" attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] self.message.guild = None - coroutine = self.cog.on_message(self.message) - asyncio.run(coroutine) + await self.cog.on_message(self.message) self.message.delete.assert_not_called() - def test_message_with_illegal_extension_gets_deleted(self): + async def test_message_with_illegal_extension_gets_deleted(self): """A message containing an illegal extension should send an embed.""" attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] - coroutine = self.cog.on_message(self.message) - asyncio.run(coroutine) + await self.cog.on_message(self.message) self.message.delete.assert_called_once() - def test_message_send_by_staff(self): + async def test_message_send_by_staff(self): """A message send by a member of staff should be ignored.""" moderator_role = MockRole(name="Moderator", id=Roles.moderators) self.message.author.roles.append(moderator_role) attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] - coroutine = self.cog.on_message(self.message) - asyncio.run(coroutine) + await self.cog.on_message(self.message) self.message.delete.assert_not_called() - def test_python_file_redirect_embed(self): + async def test_python_file_redirect_embed(self): """A message containing a .python file should result in an embed redirecting the user to our paste site""" attachment = MockAttachment(filename="python.py") self.message.attachments = [attachment] self.message.channel.send = AsyncMock() - coroutine = self.cog.on_message(self.message) - asyncio.run(coroutine) + await self.cog.on_message(self.message) args, kwargs = self.message.channel.send.call_args embed = kwargs.pop("embed") @@ -87,13 +80,12 @@ class AntiMalwareCogTests(unittest.TestCase): f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" )) - def test_txt_file_redirect_embed(self): + async def test_txt_file_redirect_embed(self): attachment = MockAttachment(filename="python.txt") self.message.attachments = [attachment] self.message.channel.send = AsyncMock() - coroutine = self.cog.on_message(self.message) - asyncio.run(coroutine) + await self.cog.on_message(self.message) args, kwargs = self.message.channel.send.call_args embed = kwargs.pop("embed") cmd_channel = self.bot.get_channel(Channels.bot_commands) @@ -109,34 +101,32 @@ class AntiMalwareCogTests(unittest.TestCase): f"\n\n{URLs.site_schema}{URLs.site_paste}" )) - def test_removing_deleted_message_logs(self): + async def test_removing_deleted_message_logs(self): """Removing an already deleted message logs the correct message""" attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] self.message.delete = AsyncMock(side_effect=NotFound(response=Mock(status=""), message="")) - coroutine = self.cog.on_message(self.message) logger = logging.getLogger(MODULE) with self.assertLogs(logger=logger, level="INFO") as logs: - asyncio.run(coroutine) + await self.cog.on_message(self.message) self.assertIn( f"INFO:{MODULE}:Tried to delete message `{self.message.id}`, but message could not be found.", logs.output) - def test_message_with_illegal_attachment_logs(self): + async def test_message_with_illegal_attachment_logs(self): """Deleting a message with an illegal attachment should result in a log.""" attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] - coroutine = self.cog.on_message(self.message) file_extensions = {splitext(attachment.filename.lower())[1] for attachment in self.message.attachments} extensions_blocked = file_extensions - set(AntiMalwareConfig.whitelist) blocked_extensions_str = ', '.join(extensions_blocked) logger = logging.getLogger(MODULE) with self.assertLogs(logger=logger, level="INFO") as logs: - asyncio.run(coroutine) + await self.cog.on_message(self.message) self.assertEqual( [ f"INFO:{MODULE}:" -- cgit v1.2.3 From bd9537ba85154ece1dca39ec03d36dd7d39a8388 Mon Sep 17 00:00:00 2001 From: MrGrote Date: Fri, 8 May 2020 22:11:54 +0200 Subject: Update tests/bot/cogs/test_antimalware.py Co-authored-by: Mark --- tests/bot/cogs/test_antimalware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 6fb7b399e..e0aa9d6d2 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -65,7 +65,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.message.delete.assert_not_called() async def test_python_file_redirect_embed(self): - """A message containing a .python file should result in an embed redirecting the user to our paste site""" + """A message containing a .py file should result in an embed redirecting the user to our paste site""" attachment = MockAttachment(filename="python.py") self.message.attachments = [attachment] self.message.channel.send = AsyncMock() -- cgit v1.2.3 From 847a78a76c08a670e85d926e3afa43e1cc3180f4 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 11 May 2020 19:41:46 +0200 Subject: AntiMalware Tests - implemented minor feedback --- tests/bot/cogs/test_antimalware.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index e0aa9d6d2..6e06df0a8 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,4 +1,3 @@ -import logging import unittest from os.path import splitext from unittest.mock import AsyncMock, Mock @@ -6,7 +5,7 @@ from unittest.mock import AsyncMock, Mock from discord import NotFound from bot.cogs import antimalware -from bot.constants import AntiMalware as AntiMalwareConfig, Channels, Roles, URLs +from bot.constants import AntiMalware as AntiMalwareConfig, Channels, STAFF_ROLES, URLs from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole MODULE = "bot.cogs.antimalware" @@ -31,7 +30,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_message_without_attachment(self): """Messages without attachments should result in no action.""" - self.assertIsNone(await self.cog.on_message(self.message)) + await self.cog.on_message(self.message) self.message.delete.assert_not_called() async def test_direct_message_with_attachment(self): @@ -55,8 +54,8 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_message_send_by_staff(self): """A message send by a member of staff should be ignored.""" - moderator_role = MockRole(name="Moderator", id=Roles.moderators) - self.message.author.roles.append(moderator_role) + staff_role = MockRole(id=STAFF_ROLES[0]) + self.message.author.roles.append(staff_role) attachment = MockAttachment(filename="python.asdfsff") self.message.attachments = [attachment] @@ -71,6 +70,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.message.channel.send = AsyncMock() await self.cog.on_message(self.message) + self.message.channel.send.assert_called_once() args, kwargs = self.message.channel.send.call_args embed = kwargs.pop("embed") @@ -107,13 +107,13 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.message.attachments = [attachment] self.message.delete = AsyncMock(side_effect=NotFound(response=Mock(status=""), message="")) - logger = logging.getLogger(MODULE) - - with self.assertLogs(logger=logger, level="INFO") as logs: + with self.assertLogs(logger=antimalware.log, level="INFO") as logs: await self.cog.on_message(self.message) + self.message.delete.assert_called_once() self.assertIn( f"INFO:{MODULE}:Tried to delete message `{self.message.id}`, but message could not be found.", - logs.output) + logs.output + ) async def test_message_with_illegal_attachment_logs(self): """Deleting a message with an illegal attachment should result in a log.""" @@ -123,9 +123,8 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): file_extensions = {splitext(attachment.filename.lower())[1] for attachment in self.message.attachments} extensions_blocked = file_extensions - set(AntiMalwareConfig.whitelist) blocked_extensions_str = ', '.join(extensions_blocked) - logger = logging.getLogger(MODULE) - with self.assertLogs(logger=logger, level="INFO") as logs: + with self.assertLogs(logger=antimalware.log, level="INFO") as logs: await self.cog.on_message(self.message) self.assertEqual( [ @@ -133,7 +132,8 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): f"User '{self.message.author}' ({self.message.author.id}) " f"uploaded blacklisted file(s): {blocked_extensions_str}" ], - logs.output) + logs.output + ) class AntiMalwareSetupTests(unittest.TestCase): -- cgit v1.2.3 From ba71ac5b002dd3e1ee6a916ba2705a7cff697a66 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 11 May 2020 20:24:20 +0200 Subject: AntiMalware Tests - extracted the method for determining disallowed extensions and added a test for it. --- tests/bot/cogs/test_antimalware.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 6e06df0a8..78ad996f2 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -19,10 +19,11 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.bot = MockBot() self.cog = antimalware.AntiMalware(self.bot) self.message = MockMessage() + AntiMalwareConfig.whitelist = [".first", ".second", ".third"] async def test_message_with_allowed_attachment(self): """Messages with allowed extensions should not be deleted""" - attachment = MockAttachment(filename=f"python.{AntiMalwareConfig.whitelist[0]}") + attachment = MockAttachment(filename=f"python{AntiMalwareConfig.whitelist[0]}") self.message.attachments = [attachment] await self.cog.on_message(self.message) @@ -35,7 +36,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_direct_message_with_attachment(self): """Direct messages should have no action taken.""" - attachment = MockAttachment(filename="python.asdfsff") + attachment = MockAttachment(filename="python.disallowed") self.message.attachments = [attachment] self.message.guild = None @@ -45,7 +46,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_message_with_illegal_extension_gets_deleted(self): """A message containing an illegal extension should send an embed.""" - attachment = MockAttachment(filename="python.asdfsff") + attachment = MockAttachment(filename="python.disallowed") self.message.attachments = [attachment] await self.cog.on_message(self.message) @@ -56,7 +57,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): """A message send by a member of staff should be ignored.""" staff_role = MockRole(id=STAFF_ROLES[0]) self.message.author.roles.append(staff_role) - attachment = MockAttachment(filename="python.asdfsff") + attachment = MockAttachment(filename="python.disallowed") self.message.attachments = [attachment] await self.cog.on_message(self.message) @@ -103,7 +104,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_removing_deleted_message_logs(self): """Removing an already deleted message logs the correct message""" - attachment = MockAttachment(filename="python.asdfsff") + attachment = MockAttachment(filename="python.disallowed") self.message.attachments = [attachment] self.message.delete = AsyncMock(side_effect=NotFound(response=Mock(status=""), message="")) @@ -117,7 +118,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_message_with_illegal_attachment_logs(self): """Deleting a message with an illegal attachment should result in a log.""" - attachment = MockAttachment(filename="python.asdfsff") + attachment = MockAttachment(filename="python.disallowed") self.message.attachments = [attachment] file_extensions = {splitext(attachment.filename.lower())[1] for attachment in self.message.attachments} @@ -135,6 +136,22 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): logs.output ) + async def test_get_disallowed_extensions(self): + """The return value should include all non-whitelisted extensions.""" + test_values = ( + (AntiMalwareConfig.whitelist, []), + ([".first"], []), + ([".first", ".disallowed"], [".disallowed"]), + ([".disallowed"], [".disallowed"]), + ([".disallowed", ".illegal"], [".disallowed", ".illegal"]), + ) + + for extensions, expected_disallowed_extensions in test_values: + with self.subTest(extensions=extensions, expected_disallowed_extensions=expected_disallowed_extensions): + self.message.attachments = [MockAttachment(filename=f"filename{extension}") for extension in extensions] + disallowed_extensions = self.cog.get_disallowed_extensions(self.message) + self.assertCountEqual(disallowed_extensions, expected_disallowed_extensions) + class AntiMalwareSetupTests(unittest.TestCase): """Tests setup of the `AntiMalware` cog.""" -- cgit v1.2.3 From 148b12603f4ad8799d135ec9956d1841cf1c7bf7 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 11 May 2020 20:24:39 +0200 Subject: AntiMalware Tests - extracted the method for determining disallowed extensions and added a test for it. --- bot/cogs/antimalware.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/bot/cogs/antimalware.py b/bot/cogs/antimalware.py index 66b5073e8..f5fd5e2d9 100644 --- a/bot/cogs/antimalware.py +++ b/bot/cogs/antimalware.py @@ -1,4 +1,5 @@ import logging +import typing as t from os.path import splitext from discord import Embed, Message, NotFound @@ -29,8 +30,7 @@ class AntiMalware(Cog): return embed = Embed() - file_extensions = {splitext(attachment.filename.lower())[1] for attachment in message.attachments} - extensions_blocked = file_extensions - set(AntiMalwareConfig.whitelist) + extensions_blocked = self.get_disallowed_extensions(message) blocked_extensions_str = ', '.join(extensions_blocked) if ".py" in extensions_blocked: # Short-circuit on *.py files to provide a pastebin link @@ -73,6 +73,13 @@ class AntiMalware(Cog): except NotFound: log.info(f"Tried to delete message `{message.id}`, but message could not be found.") + @classmethod + def get_disallowed_extensions(cls, message: Message) -> t.Iterable[str]: + """Get an iterable containing all the disallowed extensions of attachments.""" + file_extensions = {splitext(attachment.filename.lower())[1] for attachment in message.attachments} + extensions_blocked = file_extensions - set(AntiMalwareConfig.whitelist) + return extensions_blocked + def setup(bot: Bot) -> None: """Load the AntiMalware cog.""" -- cgit v1.2.3 From ecaddcedab6946ac4650b699a790471ef2a898c9 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 11 May 2020 20:39:25 +0200 Subject: AntiMalware Tests - added a missing case for no extensions in test_get_disallowed_extensions --- tests/bot/cogs/test_antimalware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 78ad996f2..7caee6f3c 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -139,6 +139,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): async def test_get_disallowed_extensions(self): """The return value should include all non-whitelisted extensions.""" test_values = ( + ([], []), (AntiMalwareConfig.whitelist, []), ([".first"], []), ([".first", ".disallowed"], [".disallowed"]), -- cgit v1.2.3 From fa467e4ef133186ff462b0178bcab08e8a3d6b2d Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 11 May 2020 20:58:51 +0200 Subject: AntiMalware Tests - Removed exact log content checks --- tests/bot/cogs/test_antimalware.py | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index 7caee6f3c..a2ce9a740 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,5 +1,4 @@ import unittest -from os.path import splitext from unittest.mock import AsyncMock, Mock from discord import NotFound @@ -108,33 +107,17 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.message.attachments = [attachment] self.message.delete = AsyncMock(side_effect=NotFound(response=Mock(status=""), message="")) - with self.assertLogs(logger=antimalware.log, level="INFO") as logs: + with self.assertLogs(logger=antimalware.log, level="INFO"): await self.cog.on_message(self.message) self.message.delete.assert_called_once() - self.assertIn( - f"INFO:{MODULE}:Tried to delete message `{self.message.id}`, but message could not be found.", - logs.output - ) async def test_message_with_illegal_attachment_logs(self): """Deleting a message with an illegal attachment should result in a log.""" attachment = MockAttachment(filename="python.disallowed") self.message.attachments = [attachment] - file_extensions = {splitext(attachment.filename.lower())[1] for attachment in self.message.attachments} - extensions_blocked = file_extensions - set(AntiMalwareConfig.whitelist) - blocked_extensions_str = ', '.join(extensions_blocked) - - with self.assertLogs(logger=antimalware.log, level="INFO") as logs: + with self.assertLogs(logger=antimalware.log, level="INFO"): await self.cog.on_message(self.message) - self.assertEqual( - [ - f"INFO:{MODULE}:" - f"User '{self.message.author}' ({self.message.author.id}) " - f"uploaded blacklisted file(s): {blocked_extensions_str}" - ], - logs.output - ) async def test_get_disallowed_extensions(self): """The return value should include all non-whitelisted extensions.""" -- cgit v1.2.3 From ddfe583d0b1e72f98855f628ff01b72c82fa491d Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 11 May 2020 21:56:39 +0200 Subject: AntiMalware Refactor - Moved embed descriptions into constants, added tests for embed descriptions --- bot/cogs/antimalware.py | 44 ++++++++++++++++++++-------------- tests/bot/cogs/test_antimalware.py | 48 ++++++++++++++++++++++++-------------- 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/bot/cogs/antimalware.py b/bot/cogs/antimalware.py index f5fd5e2d9..ea257442e 100644 --- a/bot/cogs/antimalware.py +++ b/bot/cogs/antimalware.py @@ -10,6 +10,27 @@ from bot.constants import AntiMalware as AntiMalwareConfig, Channels, STAFF_ROLE log = logging.getLogger(__name__) +PY_EMBED_DESCRIPTION = ( + "It looks like you tried to attach a Python file - " + f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" +) + +TXT_EMBED_DESCRIPTION = ( + "**Uh-oh!** It looks like your message got zapped by our spam filter. " + "We currently don't allow `.txt` attachments, so here are some tips to help you travel safely: \n\n" + "• If you attempted to send a message longer than 2000 characters, try shortening your message " + "to fit within the character limit or use a pasting service (see below) \n\n" + "• If you tried to show someone your code, you can use codeblocks \n(run `!code-blocks` in " + "{cmd_channel_mention} for more information) or use a pasting service like: " + f"\n\n{URLs.site_schema}{URLs.site_paste}" +) + +DISALLOWED_EMBED_DESCRIPTION = ( + "It looks like you tried to attach file type(s) that we do not allow ({blocked_extensions_str}). " + f"We currently allow the following file types: **{', '.join(AntiMalwareConfig.whitelist)}**.\n\n" + "Feel free to ask in {meta_channel_mention} if you think this is a mistake." +) + class AntiMalware(Cog): """Delete messages which contain attachments with non-whitelisted file extensions.""" @@ -34,29 +55,16 @@ class AntiMalware(Cog): blocked_extensions_str = ', '.join(extensions_blocked) if ".py" in extensions_blocked: # Short-circuit on *.py files to provide a pastebin link - embed.description = ( - "It looks like you tried to attach a Python file - " - f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" - ) + embed.description = PY_EMBED_DESCRIPTION elif ".txt" in extensions_blocked: # Work around Discord AutoConversion of messages longer than 2000 chars to .txt cmd_channel = self.bot.get_channel(Channels.bot_commands) - embed.description = ( - "**Uh-oh!** It looks like your message got zapped by our spam filter. " - "We currently don't allow `.txt` attachments, so here are some tips to help you travel safely: \n\n" - "• If you attempted to send a message longer than 2000 characters, try shortening your message " - "to fit within the character limit or use a pasting service (see below) \n\n" - "• If you tried to show someone your code, you can use codeblocks \n(run `!code-blocks` in " - f"{cmd_channel.mention} for more information) or use a pasting service like: " - f"\n\n{URLs.site_schema}{URLs.site_paste}" - ) + embed.description = TXT_EMBED_DESCRIPTION.format(cmd_channel_mention=cmd_channel.mention) elif extensions_blocked: - whitelisted_types = ', '.join(AntiMalwareConfig.whitelist) meta_channel = self.bot.get_channel(Channels.meta) - embed.description = ( - f"It looks like you tried to attach file type(s) that we do not allow ({blocked_extensions_str}). " - f"We currently allow the following file types: **{whitelisted_types}**.\n\n" - f"Feel free to ask in {meta_channel.mention} if you think this is a mistake." + embed.description = DISALLOWED_EMBED_DESCRIPTION.format( + blocked_extensions_str=blocked_extensions_str, + meta_channel_mention=meta_channel.mention, ) if embed.description: diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index a2ce9a740..fab063201 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -4,7 +4,7 @@ from unittest.mock import AsyncMock, Mock from discord import NotFound from bot.cogs import antimalware -from bot.constants import AntiMalware as AntiMalwareConfig, Channels, STAFF_ROLES, URLs +from bot.constants import AntiMalware as AntiMalwareConfig, Channels, STAFF_ROLES from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole MODULE = "bot.cogs.antimalware" @@ -63,7 +63,7 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.message.delete.assert_not_called() - async def test_python_file_redirect_embed(self): + async def test_python_file_redirect_embed_description(self): """A message containing a .py file should result in an embed redirecting the user to our paste site""" attachment = MockAttachment(filename="python.py") self.message.attachments = [attachment] @@ -74,32 +74,44 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): args, kwargs = self.message.channel.send.call_args embed = kwargs.pop("embed") - self.assertEqual(args[0], f"Hey {self.message.author.mention}!") - self.assertEqual(embed.description, ( - "It looks like you tried to attach a Python file - " - f"please use a code-pasting service such as {URLs.site_schema}{URLs.site_paste}" - )) + self.assertEqual(embed.description, antimalware.PY_EMBED_DESCRIPTION) - async def test_txt_file_redirect_embed(self): + async def test_txt_file_redirect_embed_description(self): + """A message containing a .txt file should result in the correct embed.""" attachment = MockAttachment(filename="python.txt") self.message.attachments = [attachment] self.message.channel.send = AsyncMock() + antimalware.TXT_EMBED_DESCRIPTION = Mock() + antimalware.TXT_EMBED_DESCRIPTION.format.return_value = "test" await self.cog.on_message(self.message) + self.message.channel.send.assert_called_once() args, kwargs = self.message.channel.send.call_args embed = kwargs.pop("embed") cmd_channel = self.bot.get_channel(Channels.bot_commands) - self.assertEqual(args[0], f"Hey {self.message.author.mention}!") - self.assertEqual(embed.description, ( - "**Uh-oh!** It looks like your message got zapped by our spam filter. " - "We currently don't allow `.txt` attachments, so here are some tips to help you travel safely: \n\n" - "• If you attempted to send a message longer than 2000 characters, try shortening your message " - "to fit within the character limit or use a pasting service (see below) \n\n" - "• If you tried to show someone your code, you can use codeblocks \n(run `!code-blocks` in " - f"{cmd_channel.mention} for more information) or use a pasting service like: " - f"\n\n{URLs.site_schema}{URLs.site_paste}" - )) + self.assertEqual(embed.description, antimalware.TXT_EMBED_DESCRIPTION.format.return_value) + antimalware.TXT_EMBED_DESCRIPTION.format.assert_called_with(cmd_channel_mention=cmd_channel.mention) + + async def test_other_disallowed_extention_embed_description(self): + """Test the description for a non .py/.txt disallowed extension.""" + attachment = MockAttachment(filename="python.disallowed") + self.message.attachments = [attachment] + self.message.channel.send = AsyncMock() + antimalware.DISALLOWED_EMBED_DESCRIPTION = Mock() + antimalware.DISALLOWED_EMBED_DESCRIPTION.format.return_value = "test" + + await self.cog.on_message(self.message) + self.message.channel.send.assert_called_once() + args, kwargs = self.message.channel.send.call_args + embed = kwargs.pop("embed") + meta_channel = self.bot.get_channel(Channels.meta) + + self.assertEqual(embed.description, antimalware.DISALLOWED_EMBED_DESCRIPTION.format.return_value) + antimalware.DISALLOWED_EMBED_DESCRIPTION.format.assert_called_with( + blocked_extensions_str=".disallowed", + meta_channel_mention=meta_channel.mention + ) async def test_removing_deleted_message_logs(self): """Removing an already deleted message logs the correct message""" -- cgit v1.2.3 From 21916ad9c19a326eb8406ea751e5fd9f80e9d912 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:42:11 +0300 Subject: ModLog Tests: Fix truncation tests docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Leon Sandøy --- tests/bot/cogs/moderation/test_modlog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_modlog.py b/tests/bot/cogs/moderation/test_modlog.py index d60836474..b5ad21a09 100644 --- a/tests/bot/cogs/moderation/test_modlog.py +++ b/tests/bot/cogs/moderation/test_modlog.py @@ -15,7 +15,7 @@ class ModLogTests(unittest.IsolatedAsyncioTestCase): self.channel = MockTextChannel() async def test_log_entry_description_truncation(self): - """Should truncate embed description for ModLog entry.""" + """Test that embed description for ModLog entry is truncated.""" self.bot.get_channel.return_value = self.channel await self.cog.send_log_message( icon_url="foo", -- cgit v1.2.3 From 1432e5ba36fc09c7233e5be4745f540c2c4af792 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:47:59 +0300 Subject: Infraction Tests: Small fixes - Remove unnecessary space from placeholder - Rename `has_active_infraction` to `get_active_infraction` --- tests/bot/cogs/moderation/test_infractions.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index 51a8cc645..2b1ff5728 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -17,11 +17,11 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): self.guild = MockGuild(id=4567) self.ctx = MockContext(bot=self.bot, author=self.user, guild=self.guild) - @patch("bot.cogs.moderation.utils.has_active_infraction") + @patch("bot.cogs.moderation.utils.get_active_infraction") @patch("bot.cogs.moderation.utils.post_infraction") - async def test_apply_ban_reason_truncation(self, post_infraction_mock, has_active_mock): + async def test_apply_ban_reason_truncation(self, post_infraction_mock, get_active_mock): """Should truncate reason for `ctx.guild.ban`.""" - has_active_mock.return_value = False + get_active_mock.return_value = 'foo' post_infraction_mock.return_value = {"foo": "bar"} self.cog.apply_infraction = AsyncMock() @@ -32,7 +32,7 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): ban = self.cog.apply_infraction.call_args[0][3] self.assertEqual( ban.cr_frame.f_locals["kwargs"]["reason"], - textwrap.shorten("foo bar" * 3000, 512, placeholder=" ...") + textwrap.shorten("foo bar" * 3000, 512, placeholder="...") ) # Await ban to avoid warning await ban -- cgit v1.2.3 From 787c106fb4a55eacc7af04afb9bcd8f206099e81 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:50:46 +0300 Subject: Infractions: Remove space from placeholder --- bot/cogs/moderation/infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 01266d346..5bfaad796 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -259,7 +259,7 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - truncated_reason = textwrap.shorten(reason, width=512, placeholder=" ...") + truncated_reason = textwrap.shorten(reason, width=512, placeholder="...") action = ctx.guild.ban(user, reason=truncated_reason, delete_message_days=0) await self.apply_infraction(ctx, infraction, user, action) -- cgit v1.2.3 From caac9b92d7c3f73ca5428597606105730e56cefc Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:53:58 +0300 Subject: ModLog: Fix embed description truncation --- bot/cogs/moderation/modlog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index c6497b38d..9d28030d9 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -100,7 +100,7 @@ class ModLog(Cog, name="ModLog"): """Generate log embed and send to logging channel.""" # Truncate string directly here to avoid removing newlines embed = discord.Embed( - description=text[:2046] + "..." if len(text) > 2048 else text + description=text[:2045] + "..." if len(text) > 2048 else text ) if title and icon_url: -- cgit v1.2.3 From 5989bcfefa244eb05f37b76d1e1df2f45e5782fa Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:54:49 +0300 Subject: ModLog Tests: Fix embed description truncate test --- tests/bot/cogs/moderation/test_modlog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_modlog.py b/tests/bot/cogs/moderation/test_modlog.py index b5ad21a09..f2809f40a 100644 --- a/tests/bot/cogs/moderation/test_modlog.py +++ b/tests/bot/cogs/moderation/test_modlog.py @@ -25,5 +25,5 @@ class ModLogTests(unittest.IsolatedAsyncioTestCase): ) embed = self.channel.send.call_args[1]["embed"] self.assertEqual( - embed.description, ("foo bar" * 3000)[:2046] + "..." + embed.description, ("foo bar" * 3000)[:2045] + "..." ) -- cgit v1.2.3 From 874cb001df91ea8223385dd2b32ab4e3c280e183 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:57:07 +0300 Subject: Infr. Tests: Add more content to await comment --- tests/bot/cogs/moderation/test_infractions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index 2b1ff5728..f8f340c2e 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -34,7 +34,7 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): ban.cr_frame.f_locals["kwargs"]["reason"], textwrap.shorten("foo bar" * 3000, 512, placeholder="...") ) - # Await ban to avoid warning + # Await ban to avoid not awaited coroutine warning await ban @patch("bot.cogs.moderation.utils.post_infraction") @@ -51,5 +51,5 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): kick.cr_frame.f_locals["kwargs"]["reason"], textwrap.shorten("foo bar" * 3000, 512, placeholder="...") ) - # Await kick to avoid warning + # Await kick to avoid not awaited coroutine warning await kick -- cgit v1.2.3 From e9bd09d90c5acf61caa955533f406851e1a65aec Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 10:59:11 +0300 Subject: Infr. Tests: Replace `str` with `dict` To allow `.get`, I had to replace `str` return value with `dict` --- tests/bot/cogs/moderation/test_infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index f8f340c2e..139209749 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -21,7 +21,7 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): @patch("bot.cogs.moderation.utils.post_infraction") async def test_apply_ban_reason_truncation(self, post_infraction_mock, get_active_mock): """Should truncate reason for `ctx.guild.ban`.""" - get_active_mock.return_value = 'foo' + get_active_mock.return_value = {"foo": "bar"} post_infraction_mock.return_value = {"foo": "bar"} self.cog.apply_infraction = AsyncMock() -- cgit v1.2.3 From d9730e41b3144862fdd9c221d160a40144a7c881 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 11:02:37 +0300 Subject: Infr. Test: Replace `get_active_mock` return value Replace `{"foo": "bar"}` with `{"id": 1}` --- tests/bot/cogs/moderation/test_infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index 139209749..925439bf3 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -21,7 +21,7 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): @patch("bot.cogs.moderation.utils.post_infraction") async def test_apply_ban_reason_truncation(self, post_infraction_mock, get_active_mock): """Should truncate reason for `ctx.guild.ban`.""" - get_active_mock.return_value = {"foo": "bar"} + get_active_mock.return_value = {"id": 1} post_infraction_mock.return_value = {"foo": "bar"} self.cog.apply_infraction = AsyncMock() -- cgit v1.2.3 From a1b6d147befd4043acdddc00667d3bda94cc76ad Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Wed, 20 May 2020 11:15:09 +0300 Subject: Infr Tests: Make `get_active_infraction` return `None` --- tests/bot/cogs/moderation/test_infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index 925439bf3..5548d9f68 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -21,7 +21,7 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): @patch("bot.cogs.moderation.utils.post_infraction") async def test_apply_ban_reason_truncation(self, post_infraction_mock, get_active_mock): """Should truncate reason for `ctx.guild.ban`.""" - get_active_mock.return_value = {"id": 1} + get_active_mock.return_value = None post_infraction_mock.return_value = {"foo": "bar"} self.cog.apply_infraction = AsyncMock() -- cgit v1.2.3 From 0ede719d7beb36f476ac26f948ab940882978476 Mon Sep 17 00:00:00 2001 From: Jannes Jonkers Date: Mon, 25 May 2020 20:44:35 +0200 Subject: AntiMalware tests - Switched from monkeypatch to unittest.patch --- tests/bot/cogs/test_antimalware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/test_antimalware.py b/tests/bot/cogs/test_antimalware.py index fab063201..f219fc1ba 100644 --- a/tests/bot/cogs/test_antimalware.py +++ b/tests/bot/cogs/test_antimalware.py @@ -1,5 +1,5 @@ import unittest -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock, Mock, patch from discord import NotFound @@ -10,6 +10,7 @@ from tests.helpers import MockAttachment, MockBot, MockMessage, MockRole MODULE = "bot.cogs.antimalware" +@patch(f"{MODULE}.AntiMalwareConfig.whitelist", new=[".first", ".second", ".third"]) class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): """Test the AntiMalware cog.""" @@ -18,7 +19,6 @@ class AntiMalwareCogTests(unittest.IsolatedAsyncioTestCase): self.bot = MockBot() self.cog = antimalware.AntiMalware(self.bot) self.message = MockMessage() - AntiMalwareConfig.whitelist = [".first", ".second", ".third"] async def test_message_with_allowed_attachment(self): """Messages with allowed extensions should not be deleted""" -- cgit v1.2.3 From f7fe7df271b0e0930fa8b997c087bb3be141d016 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 11 Apr 2020 07:43:56 +0200 Subject: Tags: explicitly use UTF-8 to read files Not all operating systems use UTF-8 as the default encoding. For systems that don't, reading tag files with Unicode would cause an unhandled exception. (cherry picked from commit adc75ff9bbcf8b905bd78c78f253522ae5e42fc3) --- bot/cogs/tags.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index bc7f53f68..6f03a3475 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -44,7 +44,7 @@ class Tags(Cog): tag = { "title": tag_title, "embed": { - "description": file.read_text(), + "description": file.read_text(encoding="utf8"), }, "restricted_to": "developers", } -- cgit v1.2.3 From 96b026198a4ca2074f4fd7ea68e8a09acd5b38e4 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 09:34:39 +0300 Subject: Simplify infraction reason truncation tests --- tests/bot/cogs/moderation/test_infractions.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index 5548d9f68..ad3c95958 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -27,15 +27,14 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): self.cog.apply_infraction = AsyncMock() self.bot.get_cog.return_value = AsyncMock() self.cog.mod_log.ignore = Mock() + self.ctx.guild.ban = Mock() await self.cog.apply_ban(self.ctx, self.target, "foo bar" * 3000) - ban = self.cog.apply_infraction.call_args[0][3] - self.assertEqual( - ban.cr_frame.f_locals["kwargs"]["reason"], - textwrap.shorten("foo bar" * 3000, 512, placeholder="...") + self.ctx.guild.ban.assert_called_once_with( + self.target, + reason=textwrap.shorten("foo bar" * 3000, 512, placeholder="..."), + delete_message_days=0 ) - # Await ban to avoid not awaited coroutine warning - await ban @patch("bot.cogs.moderation.utils.post_infraction") async def test_apply_kick_reason_truncation(self, post_infraction_mock): @@ -44,12 +43,7 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): self.cog.apply_infraction = AsyncMock() self.cog.mod_log.ignore = Mock() + self.target.kick = Mock() await self.cog.apply_kick(self.ctx, self.target, "foo bar" * 3000) - kick = self.cog.apply_infraction.call_args[0][3] - self.assertEqual( - kick.cr_frame.f_locals["kwargs"]["reason"], - textwrap.shorten("foo bar" * 3000, 512, placeholder="...") - ) - # Await kick to avoid not awaited coroutine warning - await kick + self.target.kick.assert_called_once_with(reason=textwrap.shorten("foo bar" * 3000, 512, placeholder="...")) -- cgit v1.2.3 From 7f827abfa1922a4ec81d2f49fa2811471588269d Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 09:44:01 +0300 Subject: Scheduler: Move inline f-string if-else statement to normal if statement --- bot/cogs/moderation/scheduler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index b65048f4c..80a58484c 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -174,12 +174,15 @@ class InfractionScheduler(Scheduler): dm_result = f"{constants.Emojis.failmail} " log.trace(f"Deleted infraction {infraction['id']} from database because applying infraction failed.") await self.bot.api_client.delete(f"bot/infractions/{infraction['id']}") + infr_message = "" + else: + infr_message = f"**{infr_type}** to {user.mention}{expiry_msg}{end_msg}" # Send a confirmation message to the invoking context. log.trace(f"Sending infraction #{id_} confirmation message.") await ctx.send( f"{dm_result}{confirm_msg} " - f"{f'**{infr_type}** to {user.mention}{expiry_msg}{end_msg}' if not failed else ''}." + f"{infr_message}." ) # Send a log message to the mod log. -- cgit v1.2.3 From 136ef112999b9387f04c3e0b800a3008ac07934f Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 09:44:36 +0300 Subject: Scheduler: Remove invalid comment --- bot/cogs/moderation/scheduler.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 80a58484c..7e8455740 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -84,7 +84,6 @@ class InfractionScheduler(Scheduler): """Apply an infraction to the user, log the infraction, and optionally notify the user.""" infr_type = infraction["type"] icon = utils.INFRACTION_ICONS[infr_type][0] - # Truncate reason when it's too long to avoid raising error on sending ModLog entry reason = infraction["reason"] expiry = time.format_infraction_with_duration(infraction["expires_at"]) id_ = infraction['id'] -- cgit v1.2.3 From f42ddf64abfd487fd69eec275d1011112eb76166 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 09:53:38 +0300 Subject: Scheduler: Add try-except to infraction deletion --- bot/cogs/moderation/scheduler.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 7e8455740..dcc0001f8 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -172,7 +172,12 @@ class InfractionScheduler(Scheduler): dm_log_text = "\nDM: **Canceled**" dm_result = f"{constants.Emojis.failmail} " log.trace(f"Deleted infraction {infraction['id']} from database because applying infraction failed.") - await self.bot.api_client.delete(f"bot/infractions/{infraction['id']}") + try: + await self.bot.api_client.delete(f"bot/infractions/{id_}") + except ResponseCodeError as e: + confirm_msg += f" and failed to delete" + log_title += " and failed to delete" + log.error(f"Deletion of {infr_type} infraction #{id_} failed with error code {e.status}.") infr_message = "" else: infr_message = f"**{infr_type}** to {user.mention}{expiry_msg}{end_msg}" -- cgit v1.2.3 From e71beb79f6f78b348ff17974844806c981da3c2d Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 09:56:37 +0300 Subject: Scheduler: Remove unnecessary `f` before string --- bot/cogs/moderation/scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index dcc0001f8..0d4f0ffba 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -175,7 +175,7 @@ class InfractionScheduler(Scheduler): try: await self.bot.api_client.delete(f"bot/infractions/{id_}") except ResponseCodeError as e: - confirm_msg += f" and failed to delete" + confirm_msg += " and failed to delete" log_title += " and failed to delete" log.error(f"Deletion of {infr_type} infraction #{id_} failed with error code {e.status}.") infr_message = "" -- cgit v1.2.3 From 854b27593e13f1e35810c37f4be91e5c8c4516b2 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 09:58:14 +0300 Subject: Scheduler: Fix spaces for modlog text Co-authored-by: Mark --- bot/cogs/moderation/scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 0d4f0ffba..8b28afa69 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -198,7 +198,7 @@ class InfractionScheduler(Scheduler): thumbnail=user.avatar_url_as(static_format="png"), text=textwrap.dedent(f""" Member: {user.mention} (`{user.id}`) - Actor: {ctx.message.author}{dm_log_text} {expiry_log_text} + Actor: {ctx.message.author}{dm_log_text}{expiry_log_text} Reason: {reason} """), content=log_content, -- cgit v1.2.3 From 323317496310ef474a39d468e273703106e44768 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 10:07:21 +0300 Subject: Infr. Tests: Add `apply_infraction` awaiting assertion with args --- tests/bot/cogs/moderation/test_infractions.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/bot/cogs/moderation/test_infractions.py b/tests/bot/cogs/moderation/test_infractions.py index ad3c95958..da4e92ccc 100644 --- a/tests/bot/cogs/moderation/test_infractions.py +++ b/tests/bot/cogs/moderation/test_infractions.py @@ -35,6 +35,9 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): reason=textwrap.shorten("foo bar" * 3000, 512, placeholder="..."), delete_message_days=0 ) + self.cog.apply_infraction.assert_awaited_once_with( + self.ctx, {"foo": "bar"}, self.target, self.ctx.guild.ban.return_value + ) @patch("bot.cogs.moderation.utils.post_infraction") async def test_apply_kick_reason_truncation(self, post_infraction_mock): @@ -47,3 +50,6 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): await self.cog.apply_kick(self.ctx, self.target, "foo bar" * 3000) self.target.kick.assert_called_once_with(reason=textwrap.shorten("foo bar" * 3000, 512, placeholder="...")) + self.cog.apply_infraction.assert_awaited_once_with( + self.ctx, {"foo": "bar"}, self.target, self.target.kick.return_value + ) -- cgit v1.2.3 From e236113612c560326176da91f5a743c514ac988b Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 10:10:34 +0300 Subject: Scheduler: Remove line splitting from `ctx.send` after 7f827ab --- bot/cogs/moderation/scheduler.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 8b28afa69..3679561a3 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -184,10 +184,7 @@ class InfractionScheduler(Scheduler): # Send a confirmation message to the invoking context. log.trace(f"Sending infraction #{id_} confirmation message.") - await ctx.send( - f"{dm_result}{confirm_msg} " - f"{infr_message}." - ) + await ctx.send(f"{dm_result}{confirm_msg} {infr_message}.") # Send a log message to the mod log. log.trace(f"Sending apply mod log for infraction #{id_}.") -- cgit v1.2.3 From f1f2c488dc29d731b3343c949fe49cc3eaced842 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sat, 30 May 2020 10:17:05 +0300 Subject: Scheduler: Move space from f-string of `ctx.send` to `infr_message` --- bot/cogs/moderation/scheduler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 3679561a3..1c7786df4 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -180,11 +180,11 @@ class InfractionScheduler(Scheduler): log.error(f"Deletion of {infr_type} infraction #{id_} failed with error code {e.status}.") infr_message = "" else: - infr_message = f"**{infr_type}** to {user.mention}{expiry_msg}{end_msg}" + infr_message = f" **{infr_type}** to {user.mention}{expiry_msg}{end_msg}" # Send a confirmation message to the invoking context. log.trace(f"Sending infraction #{id_} confirmation message.") - await ctx.send(f"{dm_result}{confirm_msg} {infr_message}.") + await ctx.send(f"{dm_result}{confirm_msg}{infr_message}.") # Send a log message to the mod log. log.trace(f"Sending apply mod log for infraction #{id_}.") -- cgit v1.2.3