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 72304495f43e91eb62bb47657bc3ce4858639939 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Wed, 27 May 2020 09:29:14 +0200 Subject: Remove all sending of avatar_hash. This is a companion commit to this PR: https://github.com/python-discord/site/pull/356 This PR must be merged before this commit. --- bot/cogs/moderation/utils.py | 1 - bot/cogs/sync/cog.py | 4 +--- bot/cogs/sync/syncers.py | 3 +-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index e4e0f1ec2..c99847329 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -41,7 +41,6 @@ async def post_user(ctx: Context, user: UserSnowflake) -> t.Optional[dict]: log.debug("The user being added to the DB is not a Member or User object.") payload = { - 'avatar_hash': getattr(user, 'avatar', 0), 'discriminator': int(getattr(user, 'discriminator', 0)), 'id': user.id, 'in_guild': False, diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 5708be3f4..7cc3726b2 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -94,7 +94,6 @@ class Sync(Cog): the database, the user is added. """ packed = { - 'avatar_hash': member.avatar, 'discriminator': int(member.discriminator), 'id': member.id, 'in_guild': True, @@ -135,12 +134,11 @@ class Sync(Cog): @Cog.listener() async def on_user_update(self, before: User, after: User) -> None: """Update the user information in the database if a relevant change is detected.""" - attrs = ("name", "discriminator", "avatar") + attrs = ("name", "discriminator") if any(getattr(before, attr) != getattr(after, attr) for attr in attrs): updated_information = { "name": after.name, "discriminator": int(after.discriminator), - "avatar_hash": after.avatar, } await self.patch_user(after.id, updated_information=updated_information) diff --git a/bot/cogs/sync/syncers.py b/bot/cogs/sync/syncers.py index e55bf27fd..536455668 100644 --- a/bot/cogs/sync/syncers.py +++ b/bot/cogs/sync/syncers.py @@ -17,7 +17,7 @@ log = logging.getLogger(__name__) # These objects are declared as namedtuples because tuples are hashable, # something that we make use of when diffing site roles against guild roles. _Role = namedtuple('Role', ('id', 'name', 'colour', 'permissions', 'position')) -_User = namedtuple('User', ('id', 'name', 'discriminator', 'avatar_hash', 'roles', 'in_guild')) +_User = namedtuple('User', ('id', 'name', 'discriminator', 'roles', 'in_guild')) _Diff = namedtuple('Diff', ('created', 'updated', 'deleted')) @@ -298,7 +298,6 @@ class UserSyncer(Syncer): id=member.id, name=member.name, discriminator=int(member.discriminator), - avatar_hash=member.avatar, roles=tuple(sorted(role.id for role in member.roles)), in_guild=True ) -- cgit v1.2.3 From 8e0cdb258ea6e0f25977d18336a2e07b20b5d1ee Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Wed, 27 May 2020 09:42:57 +0200 Subject: Fix failing tests related to avatar_hash --- tests/bot/cogs/sync/test_cog.py | 3 --- tests/bot/cogs/sync/test_users.py | 2 -- 2 files changed, 5 deletions(-) diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index 81398c61f..14fd909c4 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -247,14 +247,12 @@ class SyncCogListenerTests(SyncCogTestCase): before_data = { "name": "old name", "discriminator": "1234", - "avatar": "old avatar", "bot": False, } subtests = ( (True, "name", "name", "new name", "new name"), (True, "discriminator", "discriminator", "8765", 8765), - (True, "avatar", "avatar_hash", "9j2e9", "9j2e9"), (False, "bot", "bot", True, True), ) @@ -295,7 +293,6 @@ class SyncCogListenerTests(SyncCogTestCase): ) data = { - "avatar_hash": member.avatar, "discriminator": int(member.discriminator), "id": member.id, "in_guild": True, diff --git a/tests/bot/cogs/sync/test_users.py b/tests/bot/cogs/sync/test_users.py index 818883012..002a947ad 100644 --- a/tests/bot/cogs/sync/test_users.py +++ b/tests/bot/cogs/sync/test_users.py @@ -10,7 +10,6 @@ def fake_user(**kwargs): kwargs.setdefault("id", 43) kwargs.setdefault("name", "bob the test man") kwargs.setdefault("discriminator", 1337) - kwargs.setdefault("avatar_hash", None) kwargs.setdefault("roles", (666,)) kwargs.setdefault("in_guild", True) @@ -32,7 +31,6 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase): for member in members: member = member.copy() - member["avatar"] = member.pop("avatar_hash") del member["in_guild"] mock_member = helpers.MockMember(**member) -- cgit v1.2.3 From d12e84fe6834a0bc574e365a3283bc358c2ae4d9 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Fri, 29 May 2020 17:57:09 +0200 Subject: Ignore response when posting python news Sometimes a mailing list user doesn't press respond correctly to the email, and so a response is sent as a separate thread. To keep only new threads in the channel, we need to ignore those. --- bot/cogs/python_news.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/python_news.py b/bot/cogs/python_news.py index d28af4a0b..d15d0371e 100644 --- a/bot/cogs/python_news.py +++ b/bot/cogs/python_news.py @@ -153,6 +153,7 @@ class PythonNews(Cog): if ( thread_information["thread_id"] in existing_news["data"][maillist] + or 'Re: ' in thread_information["subject"] or new_date.date() < date.today() ): continue -- cgit v1.2.3 From 63f5028641e9b78c61d3bcfe3bbaa6f80c8a288a Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Fri, 29 May 2020 19:56:40 +0200 Subject: Fix `check_for_answer` breaking on missing cache The `check_for_answer` method of the HelpChannels cog relies on the channel->claimant cache being available. However, as this cache is (currently) lost during bot restarts, this method may fail with a KeyError exception. I've used `dict.get` with an `if not claimant: return` to circumvent this issue. --- bot/cogs/help_channels.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index d2a55fba6..2221132d4 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -660,10 +660,13 @@ class HelpChannels(Scheduler, commands.Cog): # Check if there is an entry in unanswered (does not persist across restarts) if channel.id in self.unanswered: - claimant_id = self.help_channel_claimants[channel].id + claimant = self.help_channel_claimants.get(channel) + if not claimant: + # The mapping for this channel was lost, we can't do anything. + return # Check the message did not come from the claimant - if claimant_id != message.author.id: + if claimant.id != message.author.id: # Mark the channel as answered self.unanswered[channel.id] = False -- cgit v1.2.3 From 28f20b969556e0ce1363ac44a5b9ff2bff2a6575 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Fri, 29 May 2020 19:49:00 +0200 Subject: Reduce the number of help channel name changes Discord has introduced a new, strict rate limit for individual channel edits that reduces the number of allow channel name/channel topic changes to 2 per 10 minutes per channel. Unfortunately, our help channel system frequently goes over that rate limit as it edits the name and topic of a channel on all three "move" actions we have: to available, to occupied, and to dormant. In addition, our "unanswered" feature adds another channel name change on top of the move-related edits. That's why I've removed the topic/emoji changing features from the help channel system. This means we now have a generic topic that fits all three categories and no status emojis in the channel names. --- bot/cogs/help_channels.py | 32 +++----------------------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 2221132d4..70cef339a 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -24,18 +24,8 @@ ASKING_GUIDE_URL = "https://pythondiscord.com/pages/asking-good-questions/" MAX_CHANNELS_PER_CATEGORY = 50 EXCLUDED_CHANNELS = (constants.Channels.how_to_get_help,) -AVAILABLE_TOPIC = """ -This channel is available. Feel free to ask a question in order to claim this channel! -""" - -IN_USE_TOPIC = """ -This channel is currently in use. If you'd like to discuss a different problem, please claim a new \ -channel from the Help: Available category. -""" - -DORMANT_TOPIC = """ -This channel is temporarily archived. If you'd like to ask a question, please use one of the \ -channels in the Help: Available category. +HELP_CHANNEL_TOPIC = """ +This is a Python help channel. You can claim your own help channel in the Python Help: Available category. """ AVAILABLE_MSG = f""" @@ -64,11 +54,6 @@ question to maximize your chance of getting a good answer. If you're not sure ho through our guide for [asking a good question]({ASKING_GUIDE_URL}). """ -AVAILABLE_EMOJI = "✅" -IN_USE_ANSWERED_EMOJI = "⌛" -IN_USE_UNANSWERED_EMOJI = "⏳" -NAME_SEPARATOR = "｜" - CoroutineFunc = t.Callable[..., t.Coroutine] @@ -196,7 +181,7 @@ class HelpChannels(Scheduler, commands.Cog): return None log.debug(f"Creating a new dormant channel named {name}.") - return await self.dormant_category.create_text_channel(name) + return await self.dormant_category.create_text_channel(name, topic=HELP_CHANNEL_TOPIC) def create_name_queue(self) -> deque: """Return a queue of element names to use for creating new channels.""" @@ -542,8 +527,6 @@ class HelpChannels(Scheduler, commands.Cog): await self.move_to_bottom_position( channel=channel, category_id=constants.Categories.help_available, - name=f"{AVAILABLE_EMOJI}{NAME_SEPARATOR}{self.get_clean_channel_name(channel)}", - topic=AVAILABLE_TOPIC, ) self.report_stats() @@ -559,8 +542,6 @@ class HelpChannels(Scheduler, commands.Cog): await self.move_to_bottom_position( channel=channel, category_id=constants.Categories.help_dormant, - name=self.get_clean_channel_name(channel), - topic=DORMANT_TOPIC, ) self.bot.stats.incr(f"help.dormant_calls.{caller}") @@ -593,8 +574,6 @@ class HelpChannels(Scheduler, commands.Cog): await self.move_to_bottom_position( channel=channel, category_id=constants.Categories.help_in_use, - name=f"{IN_USE_UNANSWERED_EMOJI}{NAME_SEPARATOR}{self.get_clean_channel_name(channel)}", - topic=IN_USE_TOPIC, ) timeout = constants.HelpChannels.idle_minutes * 60 @@ -670,11 +649,6 @@ class HelpChannels(Scheduler, commands.Cog): # Mark the channel as answered self.unanswered[channel.id] = False - # Change the emoji in the channel name to signify activity - log.trace(f"#{channel} ({channel.id}) has been answered; changing its emoji") - name = self.get_clean_channel_name(channel) - await channel.edit(name=f"{IN_USE_ANSWERED_EMOJI}{NAME_SEPARATOR}{name}") - @commands.Cog.listener() async def on_message(self, message: discord.Message) -> None: """Move an available channel to the In Use category and replace it with a dormant one.""" -- cgit v1.2.3 From aa46d01fa6c6fd14a9613412783ce377fe7e967d Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Fri, 29 May 2020 23:52:56 +0200 Subject: Clean up channel counts and add staff channels. Cleaning up a particularly dirty line by turning it into like 10 lines, and also adding the number of channels that are hidden to the `@everyone` role - which we're classifying as "Staff channels". --- bot/cogs/information.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index f0eb3a1ea..8309cff4b 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -104,8 +104,27 @@ class Information(Cog): member_count = ctx.guild.member_count # How many of each type of channel? - channels = Counter(c.type for c in ctx.guild.channels) - channel_counts = "".join(sorted(f"{str(ch).title()} channels: {channels[ch]}\n" for ch in channels)).strip() + channel_counter = Counter(c.type for c in ctx.guild.channels) + channel_type_list = [] + for channel in channel_counter: + channel_type = str(channel).title() + channel_type_list.append(f"{channel_type} channels: {channel_counter[channel]}") + + channel_type_list = sorted(channel_type_list) + channel_counts = "\n".join(channel_type_list).strip() + + # How many channels are for staff only? + everyone_role = ctx.guild.roles[0] + hidden_channels = 0 + + for channel in ctx.guild.channels: + overwrites = channel.overwrites_for(everyone_role) + if overwrites.is_empty(): + continue + + for perm, value in overwrites: + if perm == 'read_messages' and value is False: + hidden_channels += 1 # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) @@ -126,6 +145,7 @@ class Information(Cog): Members: {member_count:,} Roles: {roles} $channel_counts + Staff channels: {hidden_channels} **Members** {constants.Emojis.status_online} {statuses[Status.online]:,} -- cgit v1.2.3 From 171c1e2713355570baf50c687e7466daea834b89 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 00:02:47 +0200 Subject: Adding staff member count to !server. --- bot/cogs/information.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 8309cff4b..d830806c1 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -130,6 +130,9 @@ class Information(Cog): statuses = Counter(member.status for member in ctx.guild.members) embed = Embed(colour=Colour.blurple()) + # How many staff members? + staff_members = len(ctx.guild.get_role(constants.Roles.helpers).members) + # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting # without joining a tuple of strings we can use a Template string to insert the already-formatted channel_counts @@ -141,13 +144,16 @@ class Information(Cog): Voice region: {region} Features: {features} - **Counts** - Members: {member_count:,} - Roles: {roles} + **Channel counts** $channel_counts Staff channels: {hidden_channels} - **Members** + **Member counts** + Members: {member_count:,} + Staff members: {staff_members} + Roles: {roles} + + **Member statuses** {constants.Emojis.status_online} {statuses[Status.online]:,} {constants.Emojis.status_idle} {statuses[Status.idle]:,} {constants.Emojis.status_dnd} {statuses[Status.dnd]:,} -- cgit v1.2.3 From b258f77d1c9a1b62fef26e7fecdb89e57719dfac Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 00:12:44 +0200 Subject: Removing the periodic ping from verification. It's no longer needed, and causes problems with anti-raid and anti-spam. --- bot/cogs/verification.py | 44 +------------------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 99be3cdaa..0a087cee9 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,9 +1,7 @@ import logging from contextlib import suppress -from datetime import datetime from discord import Colour, Forbidden, Message, NotFound, Object -from discord.ext import tasks from discord.ext.commands import Cog, Context, command from bot import constants @@ -34,14 +32,6 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! <#{constants.Channels.bot_commands}>. """ -if constants.DEBUG_MODE: - PERIODIC_PING = "Periodic checkpoint message successfully sent." -else: - PERIODIC_PING = ( - f"@everyone To verify that you have read our rules, please type `{constants.Bot.prefix}accept`." - " If you encounter any problems during the verification process, " - f"send a direct message to a staff member." - ) BOT_MESSAGE_DELETE_DELAY = 10 @@ -50,7 +40,6 @@ class Verification(Cog): def __init__(self, bot: Bot): self.bot = bot - self.periodic_ping.start() @property def mod_log(self) -> ModLog: @@ -65,10 +54,7 @@ class Verification(Cog): if message.author.bot: # They're a bot, delete their message after the delay. - # But not the periodic ping; we like that one. - if message.content != PERIODIC_PING: - await message.delete(delay=BOT_MESSAGE_DELETE_DELAY) - return + await message.delete(delay=BOT_MESSAGE_DELETE_DELAY) # if a user mentions a role or guild member # alert the mods in mod-alerts channel @@ -198,34 +184,6 @@ class Verification(Cog): else: return True - @tasks.loop(hours=12) - async def periodic_ping(self) -> None: - """Every week, mention @everyone to remind them to verify.""" - messages = self.bot.get_channel(constants.Channels.verification).history(limit=10) - need_to_post = True # True if a new message needs to be sent. - - async for message in messages: - if message.author == self.bot.user and message.content == PERIODIC_PING: - delta = datetime.utcnow() - message.created_at # Time since last message. - if delta.days >= 7: # Message is older than a week. - await message.delete() - else: - need_to_post = False - - break - - if need_to_post: - await self.bot.get_channel(constants.Channels.verification).send(PERIODIC_PING) - - @periodic_ping.before_loop - async def before_ping(self) -> None: - """Only start the loop when the bot is ready.""" - await self.bot.wait_until_guild_available() - - def cog_unload(self) -> None: - """Cancel the periodic ping task when the cog is unloaded.""" - self.periodic_ping.cancel() - def setup(bot: Bot) -> None: """Load the Verification cog.""" -- 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 89752c5ff15bc55bb1986d131f562bedfdf9e63a Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 01:35:19 +0200 Subject: More precise staff-channel check. We now check: - Does the @everyone role have explicit read deny permissions? - Do staff roles have explicit read allow permissions? If the answer to both of these are yes, it's a staff channel. By 'staff roles', I mean Helpers, Moderators or Admins. --- bot/cogs/information.py | 66 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 13 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index d830806c1..715623620 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -1,11 +1,13 @@ import colorsys +import functools import logging import pprint import textwrap from collections import Counter, defaultdict from string import Template -from typing import Any, Mapping, Optional, Union +from typing import Any, List, Mapping, Optional, Union +import more_itertools from discord import Colour, Embed, Member, Message, Role, Status, utils from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group from discord.utils import escape_markdown @@ -26,6 +28,34 @@ class Information(Cog): def __init__(self, bot: Bot): self.bot = bot + @staticmethod + def _get_channels_with_role_permission(ctx: Context, role: Role, perm: str, value: Optional[bool]) -> List[int]: + """Get a list of channel IDs where a role has a specific permission set to a specific value.""" + channel_ids = [] + + for channel in ctx.guild.channels: + overwrites = channel.overwrites_for(role) + if overwrites.is_empty(): + continue + + for _perm, _value in overwrites: + if _perm == perm and _value is value: + channel_ids.append(channel.id) + + return channel_ids + + _get_channels_where_role_can_read = functools.partialmethod( + _get_channels_with_role_permission, + perm='read_messages', + value=True + ) + + _get_channels_where_role_cannot_read = functools.partialmethod( + _get_channels_with_role_permission, + perm='read_messages', + value=False + ) + @with_role(*constants.MODERATION_ROLES) @command(name="roles") async def roles_info(self, ctx: Context) -> None: @@ -114,17 +144,27 @@ class Information(Cog): channel_counts = "\n".join(channel_type_list).strip() # How many channels are for staff only? - everyone_role = ctx.guild.roles[0] - hidden_channels = 0 - - for channel in ctx.guild.channels: - overwrites = channel.overwrites_for(everyone_role) - if overwrites.is_empty(): - continue - - for perm, value in overwrites: - if perm == 'read_messages' and value is False: - hidden_channels += 1 + # We need to know two things about a channel: + # - Does the @everyone role have explicit read deny permissions? + # - Do staff roles have explicit read allow permissions? + # + # If the answer to both of these questions is yes, it's a staff channel. + helpers = ctx.guild.get_role(constants.Roles.helpers) + moderators = ctx.guild.get_role(constants.Roles.moderators) + admins = ctx.guild.get_role(constants.Roles.admins) + everyone = ctx.guild.roles[0] + + # Let's build some lists of channels. + everyone_denied = self._get_channels_where_role_cannot_read(ctx, everyone) + staff_allowed = more_itertools.flatten([ + self._get_channels_where_role_can_read(ctx, admins), # Admins has explicit read message allow + self._get_channels_where_role_can_read(ctx, moderators), # Moderators has explicit read message allow + self._get_channels_where_role_can_read(ctx, helpers), # Helpers has explicit read message allow + ]) + + # Now we need to check which channels are both denied for everyone and permitted for staff + staff_channels = [cid for cid in staff_allowed if cid in everyone_denied] + staff_channel_count = len(staff_channels) # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) @@ -146,7 +186,7 @@ class Information(Cog): **Channel counts** $channel_counts - Staff channels: {hidden_channels} + Staff channels: {staff_channel_count} **Member counts** Members: {member_count:,} -- cgit v1.2.3 From f59e63454ffa582765847e8a26d9d97dcd9ff7b2 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 01:42:02 +0200 Subject: Fix busted test_information test. I wish this test didn't exist. --- tests/bot/cogs/test_information.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index aca6b594f..79c0e0ad3 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -148,14 +148,18 @@ class InformationCogTests(unittest.TestCase): Voice region: {self.ctx.guild.region} Features: {', '.join(self.ctx.guild.features)} - **Counts** - Members: {self.ctx.guild.member_count:,} - Roles: {len(self.ctx.guild.roles)} + **Channel counts** Category channels: 1 Text channels: 1 Voice channels: 1 + Staff channels: 0 + + **Member counts** + Members: {self.ctx.guild.member_count:,} + Staff members: 0 + Roles: {len(self.ctx.guild.roles)} - **Members** + **Member statuses** {constants.Emojis.status_online} 2 {constants.Emojis.status_idle} 1 {constants.Emojis.status_dnd} 4 -- 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 From fa3cd5fef8e2f80a85ebc460cffb4e59c8e6387a Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 10:37:41 +0200 Subject: Prevent duplicates, and break into function. - We're using a set comprehension and flipping the order for counting the number of channels that are both staff allow and @everyone deny. - We're breaking the staff channel count stuff into a separate helper function so it doesn't crowd the server_info() scope. These fixes are both to address the code review from @MarkKoz, thanks Mark. --- bot/cogs/information.py | 59 +++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 715623620..9ebb89300 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -30,7 +30,7 @@ class Information(Cog): @staticmethod def _get_channels_with_role_permission(ctx: Context, role: Role, perm: str, value: Optional[bool]) -> List[int]: - """Get a list of channel IDs where a role has a specific permission set to a specific value.""" + """Get a list of channel IDs where one of the specified roles can read.""" channel_ids = [] for channel in ctx.guild.channels: @@ -56,6 +56,33 @@ class Information(Cog): value=False ) + def _get_staff_channel_count(self, ctx: Context) -> int: + """ + Get number of channels that are staff-only. + + We need to know two things about a channel: + - Does the @everyone role have explicit read deny permissions? + - Do staff roles have explicit read allow permissions? + + If the answer to both of these questions is yes, it's a staff channel. + """ + helpers = ctx.guild.get_role(constants.Roles.helpers) + moderators = ctx.guild.get_role(constants.Roles.moderators) + admins = ctx.guild.get_role(constants.Roles.admins) + everyone = ctx.guild.default_role + + # Let's build some lists of channels. + everyone_denied = self._get_channels_where_role_cannot_read(ctx, everyone) + staff_allowed = more_itertools.flatten([ + self._get_channels_where_role_can_read(ctx, admins), # Admins has explicit read message allow + self._get_channels_where_role_can_read(ctx, moderators), # Moderators has explicit read message allow + self._get_channels_where_role_can_read(ctx, helpers), # Helpers has explicit read message allow + ]) + + # Now we need to check which channels are both denied for @everyone and permitted for staff + staff_channels = set(cid for cid in everyone_denied if cid in staff_allowed) + return len(staff_channels) + @with_role(*constants.MODERATION_ROLES) @command(name="roles") async def roles_info(self, ctx: Context) -> None: @@ -143,35 +170,13 @@ class Information(Cog): channel_type_list = sorted(channel_type_list) channel_counts = "\n".join(channel_type_list).strip() - # How many channels are for staff only? - # We need to know two things about a channel: - # - Does the @everyone role have explicit read deny permissions? - # - Do staff roles have explicit read allow permissions? - # - # If the answer to both of these questions is yes, it's a staff channel. - helpers = ctx.guild.get_role(constants.Roles.helpers) - moderators = ctx.guild.get_role(constants.Roles.moderators) - admins = ctx.guild.get_role(constants.Roles.admins) - everyone = ctx.guild.roles[0] - - # Let's build some lists of channels. - everyone_denied = self._get_channels_where_role_cannot_read(ctx, everyone) - staff_allowed = more_itertools.flatten([ - self._get_channels_where_role_can_read(ctx, admins), # Admins has explicit read message allow - self._get_channels_where_role_can_read(ctx, moderators), # Moderators has explicit read message allow - self._get_channels_where_role_can_read(ctx, helpers), # Helpers has explicit read message allow - ]) - - # Now we need to check which channels are both denied for everyone and permitted for staff - staff_channels = [cid for cid in staff_allowed if cid in everyone_denied] - staff_channel_count = len(staff_channels) - # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) embed = Embed(colour=Colour.blurple()) - # How many staff members? - staff_members = len(ctx.guild.get_role(constants.Roles.helpers).members) + # How many staff members and staff channels do we have? + staff_member_count = len(ctx.guild.get_role(constants.Roles.helpers).members) + staff_channel_count = self._get_staff_channel_count() # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting @@ -190,7 +195,7 @@ class Information(Cog): **Member counts** Members: {member_count:,} - Staff members: {staff_members} + Staff members: {staff_member_count} Roles: {roles} **Member statuses** -- cgit v1.2.3 From 8562ed2feb6ef465b8b502c725566de4f0a06cbb Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 10:49:38 +0200 Subject: Don't membership check in an itertools.chain. We're using the set comprehension to prevent duplicates anyway, so flipping these back makes more sense. Also added a missing ctx and tested ok. --- bot/cogs/information.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 9ebb89300..d3a2768d4 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -80,7 +80,7 @@ class Information(Cog): ]) # Now we need to check which channels are both denied for @everyone and permitted for staff - staff_channels = set(cid for cid in everyone_denied if cid in staff_allowed) + staff_channels = set(cid for cid in staff_allowed if cid in everyone_denied) return len(staff_channels) @with_role(*constants.MODERATION_ROLES) @@ -176,7 +176,7 @@ class Information(Cog): # How many staff members and staff channels do we have? staff_member_count = len(ctx.guild.get_role(constants.Roles.helpers).members) - staff_channel_count = self._get_staff_channel_count() + staff_channel_count = self._get_staff_channel_count(ctx) # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting -- cgit v1.2.3 From 4bbb5b127315727b1534c5a0e77bfdd48173847b Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 30 May 2020 21:05:13 +0200 Subject: Free tag: link #how-to-get-help This creates a clickable link in the response embed. Referencing the category is no longer necessary. --- bot/resources/tags/free.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/resources/tags/free.md b/bot/resources/tags/free.md index 582cca9da..1493076c7 100644 --- a/bot/resources/tags/free.md +++ b/bot/resources/tags/free.md @@ -1,5 +1,5 @@ **We have a new help channel system!** -We recently moved to a new help channel system. You can now use any channel in the **<#691405807388196926>** category to ask your question. +Please see <#704250143020417084> for further information. -For more information, check out [our website](https://pythondiscord.com/pages/resources/guides/help-channels/). +A more detailed guide can be found on [our website](https://pythondiscord.com/pages/resources/guides/help-channels/). -- cgit v1.2.3 From 4549fa3defb7b9aba22505b438493bf03e74378d Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 30 May 2020 12:43:11 -0700 Subject: Simplify counting of staff channels and improve efficiency Simplification comes from being able to access permissions as attributes on the overwrite object. This removes the need to iterate all permissions. Efficiency comes from checking all roles within a single iteration of all channels. This also removes the need to flatten and filter the channels afterwards, which required additional iterations. --- bot/cogs/information.py | 75 +++++++++++++++++-------------------------------- 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index d3a2768d4..887c7c127 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -1,14 +1,13 @@ import colorsys -import functools import logging import pprint import textwrap from collections import Counter, defaultdict from string import Template -from typing import Any, List, Mapping, Optional, Union +from typing import Any, Mapping, Optional, Union -import more_itertools -from discord import Colour, Embed, Member, Message, Role, Status, utils +from discord import ChannelType, Colour, Embed, Guild, Member, Message, Role, Status, utils +from discord.abc import GuildChannel from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group from discord.utils import escape_markdown @@ -29,36 +28,14 @@ class Information(Cog): self.bot = bot @staticmethod - def _get_channels_with_role_permission(ctx: Context, role: Role, perm: str, value: Optional[bool]) -> List[int]: - """Get a list of channel IDs where one of the specified roles can read.""" - channel_ids = [] + def role_can_read(channel: GuildChannel, role: Role) -> bool: + """Return True if `role` can read messages in `channel`.""" + overwrites = channel.overwrites_for(role) + return overwrites.read_messages is True - for channel in ctx.guild.channels: - overwrites = channel.overwrites_for(role) - if overwrites.is_empty(): - continue - - for _perm, _value in overwrites: - if _perm == perm and _value is value: - channel_ids.append(channel.id) - - return channel_ids - - _get_channels_where_role_can_read = functools.partialmethod( - _get_channels_with_role_permission, - perm='read_messages', - value=True - ) - - _get_channels_where_role_cannot_read = functools.partialmethod( - _get_channels_with_role_permission, - perm='read_messages', - value=False - ) - - def _get_staff_channel_count(self, ctx: Context) -> int: + def get_staff_channel_count(self, guild: Guild) -> int: """ - Get number of channels that are staff-only. + Get the number of channels that are staff-only. We need to know two things about a channel: - Does the @everyone role have explicit read deny permissions? @@ -66,22 +43,22 @@ class Information(Cog): If the answer to both of these questions is yes, it's a staff channel. """ - helpers = ctx.guild.get_role(constants.Roles.helpers) - moderators = ctx.guild.get_role(constants.Roles.moderators) - admins = ctx.guild.get_role(constants.Roles.admins) - everyone = ctx.guild.default_role - - # Let's build some lists of channels. - everyone_denied = self._get_channels_where_role_cannot_read(ctx, everyone) - staff_allowed = more_itertools.flatten([ - self._get_channels_where_role_can_read(ctx, admins), # Admins has explicit read message allow - self._get_channels_where_role_can_read(ctx, moderators), # Moderators has explicit read message allow - self._get_channels_where_role_can_read(ctx, helpers), # Helpers has explicit read message allow - ]) - - # Now we need to check which channels are both denied for @everyone and permitted for staff - staff_channels = set(cid for cid in staff_allowed if cid in everyone_denied) - return len(staff_channels) + channel_ids = set() + for channel in guild.channels: + if channel.type is ChannelType.category: + continue + + if channel in channel_ids: + continue # Only one of the roles has to have read permissions, not all + + everyone_can_read = self.role_can_read(channel, guild.default_role) + + for role in constants.STAFF_ROLES: + role_can_read = self.role_can_read(channel, guild.get_role(role)) + if role_can_read and everyone_can_read is False: + channel_ids.add(channel.id) + + return len(channel_ids) @with_role(*constants.MODERATION_ROLES) @command(name="roles") @@ -176,7 +153,7 @@ class Information(Cog): # How many staff members and staff channels do we have? staff_member_count = len(ctx.guild.get_role(constants.Roles.helpers).members) - staff_channel_count = self._get_staff_channel_count(ctx) + staff_channel_count = self.get_staff_channel_count(ctx.guild) # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting -- cgit v1.2.3 From 8c6219cd668c814f945418000c6df896de581dc1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 30 May 2020 12:54:22 -0700 Subject: Move counting of channels to a separate method This de-clutters the main `server_info` function and improves its readability. --- bot/cogs/information.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 887c7c127..7c39dce5f 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -60,6 +60,18 @@ class Information(Cog): return len(channel_ids) + @staticmethod + def get_channel_type_counts(guild: Guild) -> str: + """Return the total amounts of the various types of channels in `guild`.""" + channel_counter = Counter(c.type for c in guild.channels) + channel_type_list = [] + for channel in channel_counter: + channel_type = str(channel).title() + channel_type_list.append(f"{channel_type} channels: {channel_counter[channel]}") + + channel_type_list = sorted(channel_type_list) + return "\n".join(channel_type_list).strip() + @with_role(*constants.MODERATION_ROLES) @command(name="roles") async def roles_info(self, ctx: Context) -> None: @@ -136,16 +148,7 @@ class Information(Cog): roles = len(ctx.guild.roles) member_count = ctx.guild.member_count - - # How many of each type of channel? - channel_counter = Counter(c.type for c in ctx.guild.channels) - channel_type_list = [] - for channel in channel_counter: - channel_type = str(channel).title() - channel_type_list.append(f"{channel_type} channels: {channel_counter[channel]}") - - channel_type_list = sorted(channel_type_list) - channel_counts = "\n".join(channel_type_list).strip() + channel_counts = self.get_channel_type_counts(ctx.guild) # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) -- cgit v1.2.3 From 795dea3c8030955736984cdab372595c4799f5e9 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 22:36:34 +0200 Subject: Add multichannel !purge via commands.Greedy We can now pass in as many channel mentions as we want after any !purge command - for example `!purge all 5 #python-general #python-language` --- bot/cogs/clean.py | 70 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index b5d9132cb..91e69ee89 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -1,9 +1,10 @@ import logging import random import re -from typing import Optional +from typing import Iterable, Optional from discord import Colour, Embed, Message, TextChannel, User +from discord.ext import commands from discord.ext.commands import Cog, Context, group from bot.bot import Bot @@ -41,10 +42,11 @@ class Clean(Cog): self, amount: int, ctx: Context, + channels: Iterable[TextChannel], bots_only: bool = False, user: User = None, regex: Optional[str] = None, - channel: Optional[TextChannel] = None + ) -> None: """A helper function that does the actual message cleaning.""" def predicate_bots_only(message: Message) -> bool: @@ -110,8 +112,8 @@ class Clean(Cog): predicate = None # Delete all messages # Default to using the invoking context's channel - if not channel: - channel = ctx.channel + if not channels: + channels = [ctx.channel] # Look through the history and retrieve message data messages = [] @@ -120,23 +122,24 @@ class Clean(Cog): invocation_deleted = False # To account for the invocation message, we index `amount + 1` messages. - async for message in channel.history(limit=amount + 1): + for channel in channels: + async for message in channel.history(limit=amount + 1): - # If at any point the cancel command is invoked, we should stop. - if not self.cleaning: - return + # If at any point the cancel command is invoked, we should stop. + if not self.cleaning: + return - # Always start by deleting the invocation - if not invocation_deleted: - self.mod_log.ignore(Event.message_delete, message.id) - await message.delete() - invocation_deleted = True - continue + # Always start by deleting the invocation + if not invocation_deleted: + self.mod_log.ignore(Event.message_delete, message.id) + await message.delete() + invocation_deleted = True + continue - # If the message passes predicate, let's save it. - if predicate is None or predicate(message): - message_ids.append(message.id) - messages.append(message) + # If the message passes predicate, let's save it. + if predicate is None or predicate(message): + message_ids.append(message.id) + messages.append(message) self.cleaning = False @@ -144,10 +147,11 @@ class Clean(Cog): self.mod_log.ignore(Event.message_delete, *message_ids) # Use bulk delete to actually do the cleaning. It's far faster. - await channel.purge( - limit=amount, - check=predicate - ) + for channel in channels: + await channel.purge( + limit=amount, + check=predicate + ) # Reverse the list to restore chronological order if messages: @@ -163,8 +167,12 @@ class Clean(Cog): return # Build the embed and send it + if len(channels) > 1: + target_channels = ", ".join([f"<#{channel.id}>" for channel in channels]) + else: + target_channels = f"<#{channels[0].id}>" message = ( - f"**{len(message_ids)}** messages deleted in <#{channel.id}> by **{ctx.author.name}**\n\n" + f"**{len(message_ids)}** messages deleted in {target_channels} by **{ctx.author.name}**\n\n" f"A log of the deleted messages can be found [here]({log_url})." ) @@ -189,10 +197,10 @@ class Clean(Cog): ctx: Context, user: User, amount: Optional[int] = 10, - channel: TextChannel = None + channels: commands.Greedy[TextChannel] = None ) -> None: """Delete messages posted by the provided user, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, user=user, channel=channel) + await self._clean_messages(amount, ctx, user=user, channels=channels) @clean_group.command(name="all", aliases=["everything"]) @with_role(*MODERATION_ROLES) @@ -200,10 +208,10 @@ class Clean(Cog): self, ctx: Context, amount: Optional[int] = 10, - channel: TextChannel = None + channels: commands.Greedy[TextChannel] = None ) -> None: """Delete all messages, regardless of poster, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, channel=channel) + await self._clean_messages(amount, ctx, channels=channels) @clean_group.command(name="bots", aliases=["bot"]) @with_role(*MODERATION_ROLES) @@ -211,10 +219,10 @@ class Clean(Cog): self, ctx: Context, amount: Optional[int] = 10, - channel: TextChannel = None + channels: commands.Greedy[TextChannel] = None ) -> None: """Delete all messages posted by a bot, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, bots_only=True, channel=channel) + await self._clean_messages(amount, ctx, bots_only=True, channels=channels) @clean_group.command(name="regex", aliases=["word", "expression"]) @with_role(*MODERATION_ROLES) @@ -223,10 +231,10 @@ class Clean(Cog): ctx: Context, regex: str, amount: Optional[int] = 10, - channel: TextChannel = None + channels: commands.Greedy[TextChannel] = None ) -> None: """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, regex=regex, channel=channel) + await self._clean_messages(amount, ctx, regex=regex, channels=channels) @clean_group.command(name="stop", aliases=["cancel", "abort"]) @with_role(*MODERATION_ROLES) -- cgit v1.2.3 From 5926f75c8d2d4b683139606bd0a39b07d28529e1 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 22:43:22 +0200 Subject: Remove a completely unacceptable newline. --- bot/cogs/clean.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 91e69ee89..8b0b8ed05 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -46,7 +46,6 @@ class Clean(Cog): bots_only: bool = False, user: User = None, regex: Optional[str] = None, - ) -> None: """A helper function that does the actual message cleaning.""" def predicate_bots_only(message: Message) -> bool: -- cgit v1.2.3 From 1cc1b3851871dfff5690432960ade09fe8ab5794 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 30 May 2020 23:14:21 +0200 Subject: Oops, add the return back. We do not wanna process bot messages. --- bot/cogs/verification.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 0a087cee9..ae156cf70 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -55,6 +55,7 @@ class Verification(Cog): if message.author.bot: # They're a bot, delete their message after the delay. await message.delete(delay=BOT_MESSAGE_DELETE_DELAY) + return # if a user mentions a role or guild member # alert the mods in mod-alerts channel -- cgit v1.2.3 From d7123487230d70b855c84fb5d99ec45f6bee6859 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 18:18:18 +0200 Subject: Better channel mentions Co-authored-by: Mark --- bot/cogs/clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 8b0b8ed05..571a5ced8 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -167,7 +167,7 @@ class Clean(Cog): # Build the embed and send it if len(channels) > 1: - target_channels = ", ".join([f"<#{channel.id}>" for channel in channels]) + target_channels = ", ".join(channel.mention for channel in channels) else: target_channels = f"<#{channels[0].id}>" message = ( -- cgit v1.2.3 From d637053eb19a6bf33e765b25b3dff9963d7b7735 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 18:19:36 +0200 Subject: Remove unnecessary conditional. Thanks @MarkKoz! --- bot/cogs/clean.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 571a5ced8..02216a4af 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -166,10 +166,8 @@ class Clean(Cog): return # Build the embed and send it - if len(channels) > 1: - target_channels = ", ".join(channel.mention for channel in channels) - else: - target_channels = f"<#{channels[0].id}>" + target_channels = ", ".join(channel.mention for channel in channels) + message = ( f"**{len(message_ids)}** messages deleted in {target_channels} by **{ctx.author.name}**\n\n" f"A log of the deleted messages can be found [here]({log_url})." -- cgit v1.2.3 From 0737b1a63ca359e88ef580143e8e4e6a879c482e Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 18:34:36 +0200 Subject: Add a mod_log.ignore_all context manager. This new context manager makes it easier to make the mod_log ignore actions like message deletions. The only existing method is the `ignore()` method, which requires that you pass all the messages you want to ignore into it. This one just ignores everything inside its scope. This isn't the DRYest approach, but it's low-cost and improves the readability of clean.py quite a bit. Ideally we should go through and give modlog a proper cleanup, because it's kinda ugly right now. --- bot/cogs/moderation/modlog.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 9d28030d9..b3ae8e215 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -3,6 +3,7 @@ import difflib import itertools import logging import typing as t +from contextlib import contextmanager from datetime import datetime from itertools import zip_longest @@ -40,6 +41,7 @@ class ModLog(Cog, name="ModLog"): def __init__(self, bot: Bot): self.bot = bot self._ignored = {event: [] for event in Event} + self._ignore_all = False self._cached_deletes = [] self._cached_edits = [] @@ -81,6 +83,15 @@ class ModLog(Cog, name="ModLog"): if item not in self._ignored[event]: self._ignored[event].append(item) + @contextmanager + def ignore_all(self) -> None: + """Ignore all events while inside this context scope.""" + self._ignore_all = True + try: + yield + finally: + self._ignore_all = False + async def send_log_message( self, icon_url: t.Optional[str], @@ -191,6 +202,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.guild_channel_update].remove(before.id) return + if self._ignore_all: + return + # Two channel updates are sent for a single edit: 1 for topic and 1 for category change. # TODO: remove once support is added for ignoring multiple occurrences for the same channel. help_categories = (Categories.help_available, Categories.help_dormant, Categories.help_in_use) @@ -386,6 +400,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_ban].remove(member.id) return + if self._ignore_all: + return + await self.send_log_message( Icons.user_ban, Colours.soft_red, "User banned", f"{member} (`{member.id}`)", @@ -426,6 +443,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_remove].remove(member.id) return + if self._ignore_all: + return + member_str = escape_markdown(str(member)) await self.send_log_message( Icons.sign_out, Colours.soft_red, @@ -444,6 +464,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_unban].remove(member.id) return + if self._ignore_all: + return + member_str = escape_markdown(str(member)) await self.send_log_message( Icons.user_unban, Colour.blurple(), @@ -462,6 +485,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_update].remove(before.id) return + if self._ignore_all: + return + diff = DeepDiff(before, after) changes = [] done = [] @@ -564,6 +590,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.message_delete].remove(message.id) return + if self._ignore_all: + return + if author.bot: return @@ -623,6 +652,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.message_delete].remove(event.message_id) return + if self._ignore_all: + return + channel = self.bot.get_channel(event.channel_id) if channel.category: @@ -797,6 +829,9 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.voice_state_update].remove(member.id) return + if self._ignore_all: + return + # Exclude all channel attributes except the name. diff = DeepDiff( before, -- cgit v1.2.3 From f344dd8a72024e05577a5aeba25e2f98501417af Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 18:37:56 +0200 Subject: Fix a bug with invocation deletion. This command was written to support only a single channel, and with the move to multi-channel purges, we need to rethink the way the invocation deletion happens. We may be invoking this command from a completely different channel, so we can't necessarily look inside the channels we're targeting for the invocation. So, we're solving this by just deleting the invocation by using ctx.message. We do this before we start iterating message history, and then we only need to iterate the number of messages that was passed into the command. A much cleaner approach, which solves the bug reported and identified by @MarkKoz. --- bot/cogs/clean.py | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 02216a4af..892c638b8 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -10,8 +10,7 @@ from discord.ext.commands import Cog, Context, group from bot.bot import Bot from bot.cogs.moderation import ModLog from bot.constants import ( - Channels, CleanMessages, Colours, Event, - Icons, MODERATION_ROLES, NEGATIVE_REPLIES + Channels, CleanMessages, Colours, Icons, MODERATION_ROLES, NEGATIVE_REPLIES ) from bot.decorators import with_role @@ -114,27 +113,23 @@ class Clean(Cog): if not channels: channels = [ctx.channel] + # Delete the invocation first + with self.mod_log.ignore_all(): + await ctx.message.delete() + # Look through the history and retrieve message data + # This is only done so we can create a log to upload. messages = [] message_ids = [] self.cleaning = True - invocation_deleted = False - # To account for the invocation message, we index `amount + 1` messages. for channel in channels: - async for message in channel.history(limit=amount + 1): + async for message in channel.history(limit=amount): # If at any point the cancel command is invoked, we should stop. if not self.cleaning: return - # Always start by deleting the invocation - if not invocation_deleted: - self.mod_log.ignore(Event.message_delete, message.id) - await message.delete() - invocation_deleted = True - continue - # If the message passes predicate, let's save it. if predicate is None or predicate(message): message_ids.append(message.id) @@ -142,15 +137,13 @@ class Clean(Cog): self.cleaning = False - # We should ignore the ID's we stored, so we don't get mod-log spam. - self.mod_log.ignore(Event.message_delete, *message_ids) - - # Use bulk delete to actually do the cleaning. It's far faster. - for channel in channels: - await channel.purge( - limit=amount, - check=predicate - ) + # Now let's delete the actual messages with purge. + with self.mod_log.ignore_all(): + for channel in channels: + await channel.purge( + limit=amount, + check=predicate + ) # Reverse the list to restore chronological order if messages: -- cgit v1.2.3 From b225791c917b582fcfbed0aee30b2cf7d3fd9ac4 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 18:50:16 +0200 Subject: Fix a bad check in get_staff_channel_count. This also changes a few aesthetic problems pointed out in review by @MarkKoz and @kwzrd. --- bot/cogs/information.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 7c39dce5f..f0bd1afdb 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -48,15 +48,13 @@ class Information(Cog): if channel.type is ChannelType.category: continue - if channel in channel_ids: - continue # Only one of the roles has to have read permissions, not all - everyone_can_read = self.role_can_read(channel, guild.default_role) for role in constants.STAFF_ROLES: role_can_read = self.role_can_read(channel, guild.get_role(role)) - if role_can_read and everyone_can_read is False: + if role_can_read and not everyone_can_read: channel_ids.add(channel.id) + break return len(channel_ids) @@ -65,12 +63,12 @@ class Information(Cog): """Return the total amounts of the various types of channels in `guild`.""" channel_counter = Counter(c.type for c in guild.channels) channel_type_list = [] - for channel in channel_counter: + for channel, count in channel_counter.items(): channel_type = str(channel).title() - channel_type_list.append(f"{channel_type} channels: {channel_counter[channel]}") + channel_type_list.append(f"{channel_type} channels: {count}") channel_type_list = sorted(channel_type_list) - return "\n".join(channel_type_list).strip() + return "\n".join(channel_type_list) @with_role(*constants.MODERATION_ROLES) @command(name="roles") -- cgit v1.2.3 From 3139991c3dcf4ec981a49aefa3d3cd75eed93fd8 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 23:33:05 +0200 Subject: Revert "Add a mod_log.ignore_all context manager." This reverts commit 0737b1a6 This isn't gonna work, because async is a thing. --- bot/cogs/moderation/modlog.py | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index b3ae8e215..9d28030d9 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -3,7 +3,6 @@ import difflib import itertools import logging import typing as t -from contextlib import contextmanager from datetime import datetime from itertools import zip_longest @@ -41,7 +40,6 @@ class ModLog(Cog, name="ModLog"): def __init__(self, bot: Bot): self.bot = bot self._ignored = {event: [] for event in Event} - self._ignore_all = False self._cached_deletes = [] self._cached_edits = [] @@ -83,15 +81,6 @@ class ModLog(Cog, name="ModLog"): if item not in self._ignored[event]: self._ignored[event].append(item) - @contextmanager - def ignore_all(self) -> None: - """Ignore all events while inside this context scope.""" - self._ignore_all = True - try: - yield - finally: - self._ignore_all = False - async def send_log_message( self, icon_url: t.Optional[str], @@ -202,9 +191,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.guild_channel_update].remove(before.id) return - if self._ignore_all: - return - # Two channel updates are sent for a single edit: 1 for topic and 1 for category change. # TODO: remove once support is added for ignoring multiple occurrences for the same channel. help_categories = (Categories.help_available, Categories.help_dormant, Categories.help_in_use) @@ -400,9 +386,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_ban].remove(member.id) return - if self._ignore_all: - return - await self.send_log_message( Icons.user_ban, Colours.soft_red, "User banned", f"{member} (`{member.id}`)", @@ -443,9 +426,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_remove].remove(member.id) return - if self._ignore_all: - return - member_str = escape_markdown(str(member)) await self.send_log_message( Icons.sign_out, Colours.soft_red, @@ -464,9 +444,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_unban].remove(member.id) return - if self._ignore_all: - return - member_str = escape_markdown(str(member)) await self.send_log_message( Icons.user_unban, Colour.blurple(), @@ -485,9 +462,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_update].remove(before.id) return - if self._ignore_all: - return - diff = DeepDiff(before, after) changes = [] done = [] @@ -590,9 +564,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.message_delete].remove(message.id) return - if self._ignore_all: - return - if author.bot: return @@ -652,9 +623,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.message_delete].remove(event.message_id) return - if self._ignore_all: - return - channel = self.bot.get_channel(event.channel_id) if channel.category: @@ -829,9 +797,6 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.voice_state_update].remove(member.id) return - if self._ignore_all: - return - # Exclude all channel attributes except the name. diff = DeepDiff( before, -- cgit v1.2.3 From 345fda6b88fef50e9bc47298085a10d8acb4fdff Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 23:36:28 +0200 Subject: Revert message ignore approach. We're removing the context manager due to async concerns, so we'll go back to the old approach again of ignoring specific messages and iterating history. --- bot/cogs/clean.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 892c638b8..b164cf232 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -10,7 +10,7 @@ from discord.ext.commands import Cog, Context, group from bot.bot import Bot from bot.cogs.moderation import ModLog from bot.constants import ( - Channels, CleanMessages, Colours, Icons, MODERATION_ROLES, NEGATIVE_REPLIES + Channels, CleanMessages, Colours, Event, Icons, MODERATION_ROLES, NEGATIVE_REPLIES ) from bot.decorators import with_role @@ -114,11 +114,10 @@ class Clean(Cog): channels = [ctx.channel] # Delete the invocation first - with self.mod_log.ignore_all(): - await ctx.message.delete() + self.mod_log.ignore(Event.message_delete, ctx.message.id) + await ctx.message.delete() # Look through the history and retrieve message data - # This is only done so we can create a log to upload. messages = [] message_ids = [] self.cleaning = True @@ -138,12 +137,12 @@ class Clean(Cog): self.cleaning = False # Now let's delete the actual messages with purge. - with self.mod_log.ignore_all(): - for channel in channels: - await channel.purge( - limit=amount, - check=predicate - ) + self.mod_log.ignore(Event.message_delete, *message_ids) + for channel in channels: + await channel.purge( + limit=amount, + check=predicate + ) # Reverse the list to restore chronological order if messages: -- cgit v1.2.3 From 196ce8a828a0fed7450cad1ee0bba25ef608214a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 31 May 2020 15:27:53 -0700 Subject: Use the messages returned by `purge` to upload message logs This ensures that only what was actually deleted will be uploaded. I managed to get a 400 response from our API when purging twice in quick succession. Searching the history manually for these messages is unreliable cause of some sort of race condition. --- bot/cogs/clean.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index b164cf232..368d91c85 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -117,11 +117,11 @@ class Clean(Cog): self.mod_log.ignore(Event.message_delete, ctx.message.id) await ctx.message.delete() - # Look through the history and retrieve message data messages = [] message_ids = [] self.cleaning = True + # Find the IDs of the messages to delete. IDs are needed in order to ignore mod log events. for channel in channels: async for message in channel.history(limit=amount): @@ -132,21 +132,17 @@ class Clean(Cog): # If the message passes predicate, let's save it. if predicate is None or predicate(message): message_ids.append(message.id) - messages.append(message) self.cleaning = False # Now let's delete the actual messages with purge. self.mod_log.ignore(Event.message_delete, *message_ids) for channel in channels: - await channel.purge( - limit=amount, - check=predicate - ) + messages += await channel.purge(limit=amount, check=predicate) # Reverse the list to restore chronological order if messages: - messages = list(reversed(messages)) + messages = reversed(messages) log_url = await self.mod_log.upload_log(messages, ctx.author.id) else: # Can't build an embed, nothing to clean! -- cgit v1.2.3 From 629817eaa87d869cc7857d5bde48d53cce6bcdc0 Mon Sep 17 00:00:00 2001 From: Rasmus Moorats Date: Tue, 2 Jun 2020 17:41:13 +0300 Subject: add modmail tag --- bot/resources/tags/modmail.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 bot/resources/tags/modmail.md diff --git a/bot/resources/tags/modmail.md b/bot/resources/tags/modmail.md new file mode 100644 index 000000000..7545419ee --- /dev/null +++ b/bot/resources/tags/modmail.md @@ -0,0 +1,9 @@ +**Contacting the moderation team via ModMail** + +<@!683001325440860340> is a bot that will relay your messages to our moderation team, so that you can start a conversation with the moderation team. Your messages will be relayed to the entire moderator team, who will be able to respond to you via the bot. + +It supports attachments, codeblocks, and reactions. As communication happens over direct messages, the conversation will stay between you and the mod team. + +**To use it, simply send a direct message to the bot.** + +Should there be an urgent and immediate need for a moderator or admin to look at a channel, feel free to ping the <@&267629731250176001> or <@&267628507062992896> role instead. -- cgit v1.2.3 From 3c305dadf7ae745fcf2ba9375d577ce750408fd3 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Fri, 5 Jun 2020 14:55:41 +0200 Subject: Send infraction DM before applying infraction I've "reverted" the change that reversed the order of DM'ing a user about their infraction and applying the actual infraction. A recent PR reversed the order to stop us from sending DMs when applying the infraction failed. However, in order to DM a user, the bot has to share a guild with the recipient and kicking them off of our server first does not help with that. That's why I reverted the change and reverted some other minor changes made in relation to this change. Note: I did not change the code sending the DM itself; I merely moved it back to where it belongs and added a comment about the necessity of doing the DM'ing first. I couldn't cleanly revert a commit to do this, as changes were spread out over and included in multiple commits that also contained changes not related to the `DM->apply infraction` order. --- bot/cogs/moderation/scheduler.py | 41 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index f0a3ad1b1..b03d89537 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -106,6 +106,27 @@ class InfractionScheduler(Scheduler): log_content = None failed = False + # DM the user about the infraction if it's not a shadow/hidden infraction. + # This needs to happen before we apply the infraction, as the bot cannot + # send DMs to user that it doesn't share a guild with. If we were to + # apply kick/ban infractions first, this would mean that we'd make it + # impossible for us to deliver a DM. See python-discord/bot#982. + 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" + if infraction["actor"] == self.bot.user.id: log.trace( f"Infraction #{id_} actor is bot; including the reason in the confirmation message." @@ -150,27 +171,7 @@ class InfractionScheduler(Scheduler): 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.") try: await self.bot.api_client.delete(f"bot/infractions/{id_}") -- cgit v1.2.3