From 8c46657aefeea118e90ada3ec9a61ba9c4734162 Mon Sep 17 00:00:00 2001 From: Izan Date: Wed, 29 Dec 2021 15:08:58 +0000 Subject: Incident archive improvements --- bot/exts/moderation/incidents.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/incidents.py b/bot/exts/moderation/incidents.py index 77dfad255..bd9e5b88e 100644 --- a/bot/exts/moderation/incidents.py +++ b/bot/exts/moderation/incidents.py @@ -1,6 +1,5 @@ import asyncio import re -from datetime import datetime from enum import Enum from typing import Optional @@ -13,6 +12,7 @@ from bot.constants import Channels, Colours, Emojis, Guild, Roles, Webhooks from bot.log import get_logger from bot.utils import scheduling from bot.utils.messages import format_user, sub_clyde +from bot.utils.time import TimestampFormats, discord_timestamp log = get_logger(__name__) @@ -25,9 +25,9 @@ CRAWL_LIMIT = 50 CRAWL_SLEEP = 2 DISCORD_MESSAGE_LINK_RE = re.compile( - r"(https?:\/\/(?:(ptb|canary|www)\.)?discord(?:app)?\.com\/channels\/" + r"(https?://(?:(ptb|canary|www)\.)?discord(?:app)?\.com/channels/" r"[0-9]{15,20}" - r"\/[0-9]{15,20}\/[0-9]{15,20})" + r"/[0-9]{15,20}/[0-9]{15,20})" ) @@ -97,9 +97,19 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di colour = Colours.soft_red footer = f"Rejected by {actioned_by}" + day_timestamp = discord_timestamp(incident.created_at, TimestampFormats.DATE) + time_timestamp = discord_timestamp(incident.created_at, TimestampFormats.TIME) + relative_timestamp = discord_timestamp(incident.created_at, TimestampFormats.RELATIVE) + reported_on_msg = f"__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__" + + # If the description will be too long (>4096 total characters), truncate the incident content + if len(incident.content) > (allowed_content_chars := 4096-len(reported_on_msg)-2): # -2 for the newlines + description = incident.content[:allowed_content_chars-3] + f"...\n\n{reported_on_msg}" + else: + description = incident.content + f"\n\n{reported_on_msg}" + embed = discord.Embed( - description=incident.content, - timestamp=datetime.utcnow(), + description=description, colour=colour, ) embed.set_footer(text=footer, icon_url=actioned_by.display_avatar.url) @@ -380,7 +390,7 @@ class Incidents(Cog): webhook = await self.bot.fetch_webhook(Webhooks.incidents_archive) await webhook.send( embed=embed, - username=sub_clyde(incident.author.name), + username=sub_clyde(incident.author.display_name), avatar_url=incident.author.display_avatar.url, file=attachment_file, ) -- cgit v1.2.3 From 1e0c0cfe37eb9a868454508fcb813d7cf19e12cc Mon Sep 17 00:00:00 2001 From: Izan Date: Wed, 29 Dec 2021 15:09:22 +0000 Subject: Fix tests --- tests/bot/exts/moderation/test_incidents.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/bot/exts/moderation/test_incidents.py b/tests/bot/exts/moderation/test_incidents.py index cfe0c4b03..ef33aa62b 100644 --- a/tests/bot/exts/moderation/test_incidents.py +++ b/tests/bot/exts/moderation/test_incidents.py @@ -1,4 +1,5 @@ import asyncio +import datetime import enum import logging import typing as t @@ -13,6 +14,7 @@ from async_rediscache import RedisSession from bot.constants import Colours from bot.exts.moderation import incidents from bot.utils.messages import format_user +from bot.utils.time import TimestampFormats, discord_timestamp from tests.helpers import ( MockAsyncWebhook, MockAttachment, MockBot, MockMember, MockMessage, MockReaction, MockRole, MockTextChannel, MockUser @@ -114,10 +116,19 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): async def test_make_embed_content(self): """Incident content appears as embed description.""" - incident = MockMessage(content="this is an incident") + current_time = datetime.datetime(2022, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) + incident = MockMessage(content="this is an incident", created_at=current_time) + + day_timestamp = discord_timestamp(current_time, TimestampFormats.DATE) + time_timestamp = discord_timestamp(current_time, TimestampFormats.TIME) + relative_timestamp = discord_timestamp(current_time, TimestampFormats.RELATIVE) + embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) - self.assertEqual(incident.content, embed.description) + self.assertEqual( + f"{incident.content}\n\n__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__", + embed.description + ) async def test_make_embed_with_attachment_succeeds(self): """Incident's attachment is downloaded and displayed in the embed's image field.""" @@ -391,7 +402,7 @@ class TestArchive(TestIncidents): # Define our own `incident` to be archived incident = MockMessage( content="this is an incident", - author=MockUser(name="author_name", display_avatar=Mock(url="author_avatar")), + author=MockUser(display_name="author_name", display_avatar=Mock(url="author_avatar")), id=123, ) built_embed = MagicMock(discord.Embed, id=123) # We patch `make_embed` to return this @@ -422,7 +433,7 @@ class TestArchive(TestIncidents): webhook = MockAsyncWebhook() self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook) - message_from_clyde = MockMessage(author=MockUser(name="clyde the great")) + message_from_clyde = MockMessage(author=MockUser(display_name="clyde the great")) await self.cog_instance.archive(message_from_clyde, MagicMock(incidents.Signal), MockMember()) self.assertNotIn("clyde", webhook.send.call_args.kwargs["username"]) @@ -521,12 +532,13 @@ class TestProcessEvent(TestIncidents): async def test_process_event_confirmation_task_is_awaited(self): """Task given by `Incidents.make_confirmation_task` is awaited before method exits.""" mock_task = AsyncMock() + mock_member = MockMember(display_name="Bobby Johnson", roles=[MockRole(id=1)]) with patch("bot.exts.moderation.incidents.Incidents.make_confirmation_task", mock_task): await self.cog_instance.process_event( reaction=incidents.Signal.ACTIONED.value, - incident=MockMessage(id=123), - member=MockMember(roles=[MockRole(id=1)]) + incident=MockMessage(author=mock_member, id=123), + member=mock_member ) mock_task.assert_awaited() -- cgit v1.2.3 From b7e03616ac3fc0b5e8a5a77a352df593983d187a Mon Sep 17 00:00:00 2001 From: Izan Date: Thu, 14 Jul 2022 22:21:34 +0100 Subject: Address Reviews - Use the more concise DATETIME timestamp instead of both a DATE and a TIME timestamp. - Remove underline from the "Reported ..." section at the bottom of the embed. - Re-add time of action/rejection timestamp to footer of embed. --- bot/exts/moderation/incidents.py | 7 ++++--- tests/bot/exts/moderation/test_incidents.py | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/incidents.py b/bot/exts/moderation/incidents.py index bd9e5b88e..f29cfcdd6 100644 --- a/bot/exts/moderation/incidents.py +++ b/bot/exts/moderation/incidents.py @@ -1,5 +1,6 @@ import asyncio import re +from datetime import datetime, timezone from enum import Enum from typing import Optional @@ -97,10 +98,9 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di colour = Colours.soft_red footer = f"Rejected by {actioned_by}" - day_timestamp = discord_timestamp(incident.created_at, TimestampFormats.DATE) - time_timestamp = discord_timestamp(incident.created_at, TimestampFormats.TIME) + reported_timestamp = discord_timestamp(incident.created_at) relative_timestamp = discord_timestamp(incident.created_at, TimestampFormats.RELATIVE) - reported_on_msg = f"__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__" + reported_on_msg = f"*Reported {reported_timestamp} ({relative_timestamp}).*" # If the description will be too long (>4096 total characters), truncate the incident content if len(incident.content) > (allowed_content_chars := 4096-len(reported_on_msg)-2): # -2 for the newlines @@ -111,6 +111,7 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di embed = discord.Embed( description=description, colour=colour, + timestamp=datetime.now(timezone.utc) ) embed.set_footer(text=footer, icon_url=actioned_by.display_avatar.url) diff --git a/tests/bot/exts/moderation/test_incidents.py b/tests/bot/exts/moderation/test_incidents.py index ef33aa62b..da0a79ce8 100644 --- a/tests/bot/exts/moderation/test_incidents.py +++ b/tests/bot/exts/moderation/test_incidents.py @@ -119,14 +119,13 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): current_time = datetime.datetime(2022, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) incident = MockMessage(content="this is an incident", created_at=current_time) - day_timestamp = discord_timestamp(current_time, TimestampFormats.DATE) - time_timestamp = discord_timestamp(current_time, TimestampFormats.TIME) + reported_timestamp = discord_timestamp(current_time) relative_timestamp = discord_timestamp(current_time, TimestampFormats.RELATIVE) embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) self.assertEqual( - f"{incident.content}\n\n__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__", + f"{incident.content}\n\n*Reported {reported_timestamp} ({relative_timestamp}).*", embed.description ) -- cgit v1.2.3 From 40ab60fdf85f44b08f0a9316a22e1b0bef6e0646 Mon Sep 17 00:00:00 2001 From: Izan Date: Mon, 25 Jul 2022 23:14:27 +0100 Subject: Allow referencing message as argument to `!remind edit content` --- bot/exts/utils/reminders.py | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 45cddd7a2..f33a9f448 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -209,6 +209,29 @@ class Reminders(Cog): log.debug(f"Deleting reminder #{reminder['id']} (the user has been reminded).") await self.bot.api_client.delete(f"bot/reminders/{reminder['id']}") + @staticmethod + async def try_get_content_from_reply(ctx: Context) -> t.Optional[str]: + """ + Attempts to get content from the referenced message, if applicable. + + Differs from botcore.utils.commands.clean_text_or_reply as allows for embeds. + """ + content = None + if reference := ctx.message.reference: + if isinstance((resolved_message := reference.resolved), discord.Message): + content = resolved_message.content + + # If we weren't able to get the content of a replied message + if content is None: + await send_denial(ctx, "Your reminder must have a content and/or reply to a message.") + return + + # If the replied message has no content (e.g. only attachments/embeds) + if content == "": + content = "See referenced message." + + return content + @group(name="remind", aliases=("reminder", "reminders", "remindme"), invoke_without_command=True) async def remind_group( self, ctx: Context, mentions: Greedy[ReminderMention], expiration: Duration, *, content: t.Optional[str] = None @@ -282,18 +305,11 @@ class Reminders(Cog): # If `content` isn't provided then we try to get message content of a replied message if not content: - if reference := ctx.message.reference: - if isinstance((resolved_message := reference.resolved), discord.Message): - content = resolved_message.content - # If we weren't able to get the content of a replied message - if content is None: - await send_denial(ctx, "Your reminder must have a content and/or reply to a message.") + content = await self.try_get_content_from_reply(ctx) + if not content: + # Couldn't get content from reply return - # If the replied message has no content (e.g. only attachments/embeds) - if content == "": - content = "See referenced message." - # Now we can attempt to actually set the reminder. reminder = await self.bot.api_client.post( 'bot/reminders', @@ -417,8 +433,13 @@ class Reminders(Cog): await self.edit_reminder(ctx, id_, {'expiration': expiration.isoformat()}) @edit_reminder_group.command(name="content", aliases=("reason",)) - async def edit_reminder_content(self, ctx: Context, id_: int, *, content: str) -> None: + async def edit_reminder_content(self, ctx: Context, id_: int, *, content: t.Optional[str] = None) -> None: """Edit one of your reminder's content.""" + if not content: + content = await self.try_get_content_from_reply(ctx) + if not content: + # Couldn't get content from reply + return await self.edit_reminder(ctx, id_, {"content": content}) @edit_reminder_group.command(name="mentions", aliases=("pings",)) -- cgit v1.2.3 From 0be3332904690730589113b99198f2f3fc2f1091 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 03:42:31 -0400 Subject: Added `DurationOrExpiry` type union --- bot/converters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/converters.py b/bot/converters.py index 5800ea044..e97a25bdd 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -574,5 +574,6 @@ if t.TYPE_CHECKING: Infraction = t.Optional[dict] # noqa: F811 Expiry = t.Union[Duration, ISODateTime] +DurationOrExpiry = t.Union[DurationDelta, ISODateTime] MemberOrUser = t.Union[discord.Member, discord.User] UnambiguousMemberOrUser = t.Union[UnambiguousMember, UnambiguousUser] -- cgit v1.2.3 From 0921838112120355aed0936923983b545320edef Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:31:05 -0400 Subject: Refactoring for DurationOrExpiry --- bot/exts/moderation/infraction/_utils.py | 20 ++++++++++----- bot/exts/moderation/infraction/infractions.py | 34 +++++++++++++------------- bot/exts/moderation/infraction/superstarify.py | 4 +-- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 3a2485ec2..368a637c2 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -1,5 +1,5 @@ import typing as t -from datetime import datetime +from datetime import datetime, timezone import arrow import discord @@ -8,7 +8,7 @@ from discord.ext.commands import Context import bot from bot.constants import Colours, Icons -from bot.converters import MemberOrUser +from bot.converters import MemberOrUser, DurationOrExpiry from bot.errors import InvalidInfractedUserError from bot.log import get_logger from bot.utils import time @@ -80,7 +80,7 @@ async def post_infraction( user: MemberOrUser, infr_type: str, reason: str, - expires_at: datetime = None, + duration_or_expiry: t.Optional[DurationOrExpiry] = None, hidden: bool = False, active: bool = True, dm_sent: bool = False, @@ -92,6 +92,8 @@ async def post_infraction( log.trace(f"Posting {infr_type} infraction for {user} to the API.") + current_time = datetime.now(tz=timezone.utc) + payload = { "actor": ctx.author.id, # Don't use ctx.message.author; antispam only patches ctx.author. "hidden": hidden, @@ -99,10 +101,16 @@ async def post_infraction( "type": infr_type, "user": user.id, "active": active, - "dm_sent": dm_sent + "dm_sent": dm_sent, + "last_applied": current_time, } - if expires_at: - payload['expires_at'] = expires_at.isoformat() + + # Parse duration or expiry + if duration_or_expiry is not None: + if isinstance(duration_or_expiry, datetime): + payload['expires_at'] = duration_or_expiry.isoformat() + else: # is relativedelta + payload['expires_at'] = (current_time + duration_or_expiry).isoformat() # Try to apply the infraction. If it fails because the user doesn't exist, try to add it. for should_post_user in (True, False): diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 46fd3381c..1818997bb 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -9,7 +9,7 @@ from discord.ext.commands import Context, command from bot import constants from bot.bot import Bot from bot.constants import Event -from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser +from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser, DurationOrExpiry from bot.decorators import ensure_future_timestamp, respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler @@ -86,7 +86,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -95,7 +95,7 @@ class Infractions(InfractionScheduler, commands.Cog): If duration is specified, it temporarily bans that user for the given duration. """ - await self.apply_ban(ctx, user, reason, expires_at=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) @command(aliases=("cban", "purgeban", "pban")) @ensure_future_timestamp(timestamp_arg=3) @@ -103,7 +103,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -115,10 +115,10 @@ class Infractions(InfractionScheduler, commands.Cog): clean_cog: t.Optional[Clean] = self.bot.get_cog("Clean") if clean_cog is None: # If we can't get the clean cog, fall back to native purgeban. - await self.apply_ban(ctx, user, reason, purge_days=1, expires_at=duration) + await self.apply_ban(ctx, user, reason, purge_days=1, duration_or_expiry=duration) return - infraction = await self.apply_ban(ctx, user, reason, expires_at=duration) + infraction = await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) if not infraction or not infraction.get("id"): # Ban was unsuccessful, quit early. await ctx.send(":x: Failed to apply ban.") @@ -168,7 +168,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] ) -> None: @@ -177,7 +177,7 @@ class Infractions(InfractionScheduler, commands.Cog): If duration is specified, it temporarily voice mutes that user for the given duration. """ - await self.apply_voice_mute(ctx, user, reason, expires_at=duration) + await self.apply_voice_mute(ctx, user, reason, duration_or_expiry=duration) # endregion # region: Temporary infractions @@ -187,7 +187,7 @@ class Infractions(InfractionScheduler, commands.Cog): async def tempmute( self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -214,7 +214,7 @@ class Infractions(InfractionScheduler, commands.Cog): if duration is None: duration = await Duration().convert(ctx, "1h") - await self.apply_mute(ctx, user, reason, expires_at=duration) + await self.apply_mute(ctx, user, reason, duration_or_expiry=duration) @command(aliases=("tban",)) @ensure_future_timestamp(timestamp_arg=3) @@ -241,7 +241,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_ban(ctx, user, reason, expires_at=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) @command(aliases=("tempvban", "tvban")) async def tempvoiceban(self, ctx: Context) -> None: @@ -258,7 +258,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: Expiry, + duration: DurationOrExpiry, *, reason: t.Optional[str] ) -> None: @@ -277,7 +277,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_voice_mute(ctx, user, reason, expires_at=duration) + await self.apply_voice_mute(ctx, user, reason, duration_or_expiry=duration) # endregion # region: Permanent shadow infractions @@ -305,7 +305,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: Expiry, + duration: DurationOrExpiry, *, reason: t.Optional[str] = None ) -> None: @@ -324,7 +324,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_ban(ctx, user, reason, expires_at=duration, hidden=True) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration, hidden=True) # endregion # region: Remove infractions (un- commands) @@ -428,7 +428,7 @@ class Infractions(InfractionScheduler, commands.Cog): return None # In the case of a permanent ban, we don't need get_active_infractions to tell us if one is active - is_temporary = kwargs.get("expires_at") is not None + is_temporary = kwargs.get("duration_or_expiry") is not None active_infraction = await _utils.get_active_infraction(ctx, user, "ban", is_temporary) if active_infraction: @@ -436,7 +436,7 @@ class Infractions(InfractionScheduler, commands.Cog): log.trace("Tempban ignored as it cannot overwrite an active ban.") return None - if active_infraction.get('expires_at') is None: + if active_infraction.get("duration_or_expiry") is None: log.trace("Permaban already exists, notify.") await ctx.send(f":x: User is already permanently banned (#{active_infraction['id']}).") return None diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 0e6aaa1e7..f2aab7a92 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -10,7 +10,7 @@ from discord.utils import escape_markdown from bot import constants from bot.bot import Bot -from bot.converters import Duration, Expiry +from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler @@ -109,7 +109,7 @@ class Superstarify(InfractionScheduler, Cog): self, ctx: Context, member: Member, - duration: t.Optional[Expiry], + duration: t.Optional[DurationOrExpiry], *, reason: str = '', ) -> None: -- cgit v1.2.3 From 73a67d175283ca0525af4520018aa01f288bd242 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:38:18 -0400 Subject: Ran isort on imports --- bot/exts/moderation/infraction/_utils.py | 2 +- bot/exts/moderation/infraction/infractions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 368a637c2..80b65dae8 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -8,7 +8,7 @@ from discord.ext.commands import Context import bot from bot.constants import Colours, Icons -from bot.converters import MemberOrUser, DurationOrExpiry +from bot.converters import DurationOrExpiry, MemberOrUser from bot.errors import InvalidInfractedUserError from bot.log import get_logger from bot.utils import time diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 1818997bb..aff5e392f 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -9,7 +9,7 @@ from discord.ext.commands import Context, command from bot import constants from bot.bot import Bot from bot.constants import Event -from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser, DurationOrExpiry +from bot.converters import Age, Duration, DurationOrExpiry, Expiry, MemberOrUser, UnambiguousMemberOrUser from bot.decorators import ensure_future_timestamp, respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler -- cgit v1.2.3 From 7f153894d656d01a6221fa6520ec1df9a37bd478 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:54:26 -0400 Subject: Updated tests - Refactored tests for new time duration arguments --- .../exts/moderation/infraction/test_infractions.py | 11 ++++---- tests/bot/exts/moderation/infraction/test_utils.py | 29 +++++++++++++--------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index 052048053..a18a4d23b 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -79,13 +79,13 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): """Should call voice mute applying function without expiry.""" self.cog.apply_voice_mute = AsyncMock() self.assertIsNone(await self.cog.voicemute(self.cog, self.ctx, self.user, reason="foobar")) - self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", expires_at=None) + self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", duration_or_expiry=None) async def test_temporary_voice_mute(self): """Should call voice mute applying function with expiry.""" self.cog.apply_voice_mute = AsyncMock() self.assertIsNone(await self.cog.tempvoicemute(self.cog, self.ctx, self.user, "baz", reason="foobar")) - self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", expires_at="baz") + self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", duration_or_expiry="baz") async def test_voice_unmute(self): """Should call infraction pardoning function.""" @@ -189,7 +189,8 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): user = MockUser() await self.cog.voicemute(self.cog, self.ctx, user, reason=None) - post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_mute", None, active=True, expires_at=None) + post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_mute", None, active=True, + duration_or_expiry=None) apply_infraction_mock.assert_called_once_with(self.cog, self.ctx, infraction, user, ANY) # Test action @@ -273,7 +274,7 @@ class CleanBanTests(unittest.IsolatedAsyncioTestCase): self.user, "FooBar", purge_days=1, - expires_at=None, + duration_or_expiry=None, ) async def test_cleanban_doesnt_purge_messages_if_clean_cog_available(self): @@ -285,7 +286,7 @@ class CleanBanTests(unittest.IsolatedAsyncioTestCase): self.ctx, self.user, "FooBar", - expires_at=None, + duration_or_expiry=None, ) @patch("bot.exts.moderation.infraction.infractions.Age") diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 5cf02033d..4c78c0bd8 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -318,14 +318,17 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.member.id, "active": False, "expires_at": now.isoformat(), - "dm_sent": False + "dm_sent": False, + "last_applied": datetime(2020, 1, 1).isoformat(), } - self.ctx.bot.api_client.post.return_value = "foo" - actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) + # Patch the time.now(tz=timezone.utc) function to return a specific time + with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): + self.ctx.bot.api_client.post.return_value = "foo" + actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.assertEqual(actual, "foo") + self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" @@ -356,12 +359,14 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "type": "mute", "user": self.user.id, "active": True, - "dm_sent": False + "dm_sent": False, + "last_applied": datetime(2020, 1, 1), } - self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] - - actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") - self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) - post_user_mock.assert_awaited_once_with(self.ctx, self.user) + # Patch the time.now(tz=timezone.utc) function to return a specific time + with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): + self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] + actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") + self.assertEqual(actual, "foo") + self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From 2670fc4027f30776eb091682c6cdd48188793163 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:04:38 -0400 Subject: Fixed test patches --- tests/bot/exts/moderation/infraction/test_utils.py | 32 ++++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 4c78c0bd8..def06932b 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -307,7 +307,8 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.user = MockUser(id=1234) self.ctx = MockContext(bot=self.bot, author=self.member) - async def test_normal_post_infraction(self): + @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) + async def test_normal_post_infraction(self, mock_datetime): """Should return response from POST request if there are no errors.""" now = datetime.now() payload = { @@ -322,13 +323,13 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the time.now(tz=timezone.utc) function to return a specific time - with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): - self.ctx.bot.api_client.post.return_value = "foo" - actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) + # Patch the datetime.now function to return a specific time + mock_datetime.now.return_value = datetime(2020, 1, 1) + self.ctx.bot.api_client.post.return_value = "foo" + actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.assertEqual(actual, "foo") + self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" @@ -349,8 +350,9 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.assertIsNone(actual) post_user_mock.assert_awaited_once_with(self.ctx, self.user) + @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar") - async def test_first_fail_second_success_user_post_infraction(self, post_user_mock): + async def test_first_fail_second_success_user_post_infraction(self, post_user_mock, mock_datetime): """Should post the user if they don't exist, POST infraction again, and return the response if successful.""" payload = { "actor": self.ctx.author.id, @@ -363,10 +365,10 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "last_applied": datetime(2020, 1, 1), } - # Patch the time.now(tz=timezone.utc) function to return a specific time - with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): - self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] - actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") - self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) - post_user_mock.assert_awaited_once_with(self.ctx, self.user) + # Patch the datetime.now function to return a specific time + mock_datetime.now.return_value = datetime(2020, 1, 1) + self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] + actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") + self.assertEqual(actual, "foo") + self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From c85f7a5ecb46448a235588329439545b42091006 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:19:30 -0400 Subject: Added isoformat for test payload --- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index def06932b..d3a908b28 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -362,7 +362,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.user.id, "active": True, "dm_sent": False, - "last_applied": datetime(2020, 1, 1), + "last_applied": datetime(2020, 1, 1).isoformat(), } # Patch the datetime.now function to return a specific time -- cgit v1.2.3 From 9d73552235b8e80f90dec06d7c0220e84734e205 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:20:48 -0400 Subject: Updated parameter names - Changed `duration` parameter names to `duration_or_expiry` to more accurately reflect options for help --- bot/exts/moderation/infraction/infractions.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index aff5e392f..12bd084df 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -9,7 +9,7 @@ from discord.ext.commands import Context, command from bot import constants from bot.bot import Bot from bot.constants import Event -from bot.converters import Age, Duration, DurationOrExpiry, Expiry, MemberOrUser, UnambiguousMemberOrUser +from bot.converters import Age, Duration, DurationOrExpiry, MemberOrUser, UnambiguousMemberOrUser from bot.decorators import ensure_future_timestamp, respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler @@ -86,7 +86,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[DurationOrExpiry] = None, + duration_or_expiry: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -95,7 +95,7 @@ class Infractions(InfractionScheduler, commands.Cog): If duration is specified, it temporarily bans that user for the given duration. """ - await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration_or_expiry) @command(aliases=("cban", "purgeban", "pban")) @ensure_future_timestamp(timestamp_arg=3) @@ -222,7 +222,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: Expiry, + duration_or_expiry: DurationOrExpiry, *, reason: t.Optional[str] = None ) -> None: @@ -241,7 +241,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration_or_expiry) @command(aliases=("tempvban", "tvban")) async def tempvoiceban(self, ctx: Context) -> None: -- cgit v1.2.3 From 517d3aad25c912a0f5ae47388a0d5a9deefe9d50 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:36:45 -0400 Subject: Updated ban command docstring - Updated docstring to be more explicit on parameter fields --- bot/exts/moderation/infraction/infractions.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 12bd084df..83a947a19 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -91,9 +91,11 @@ class Infractions(InfractionScheduler, commands.Cog): reason: t.Optional[str] = None ) -> None: """ - Permanently ban a user for the given reason and stop watching them with Big Brother. + Permanently ban a `user` for the given `reason` and stop watching them with Big Brother. - If duration is specified, it temporarily bans that user for the given duration. + If a duration is specified, it temporarily bans the `user` for the given duration. + Alternatively, an ISO 8601 timestamp representing the expiry time can be provided + for `duration_or_expiry`. """ await self.apply_ban(ctx, user, reason, duration_or_expiry=duration_or_expiry) -- cgit v1.2.3 From 14eb99bf130ab4f8a7e14d9b4a4172a969e7a1c0 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 06:11:11 -0400 Subject: Fixed tests - Corrected datetime patching --- tests/bot/exts/moderation/infraction/test_utils.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index d3a908b28..b1f23e31c 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -307,8 +307,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.user = MockUser(id=1234) self.ctx = MockContext(bot=self.bot, author=self.member) - @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) - async def test_normal_post_infraction(self, mock_datetime): + async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" now = datetime.now() payload = { @@ -320,16 +319,18 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "active": False, "expires_at": now.isoformat(), "dm_sent": False, - "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the datetime.now function to return a specific time - mock_datetime.now.return_value = datetime(2020, 1, 1) self.ctx.bot.api_client.post.return_value = "foo" actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.ctx.bot.api_client.post.assert_awaited_once() + await_args = str(self.ctx.bot.api_client.post.await_args) + # Check existing keys present, allow for additional keys (e.g. `last_applied`) + for key, value in payload.items(): + self.assertTrue(key in await_args) + self.assertTrue(str(value) in await_args) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" -- cgit v1.2.3 From f3c15ed33185c110f486c6e933ea46272c25f6db Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 06:11:23 -0400 Subject: Correct last_applied formatting --- bot/exts/moderation/infraction/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 80b65dae8..cd82f5b2e 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -102,7 +102,7 @@ async def post_infraction( "user": user.id, "active": active, "dm_sent": dm_sent, - "last_applied": current_time, + "last_applied": current_time.isoformat(), } # Parse duration or expiry -- cgit v1.2.3 From a98c82c1ec6b244868356b1b2db1daa52e0f2555 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 18:57:01 -0400 Subject: Use `last_applied` to display duration --- bot/exts/moderation/infraction/_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index cd82f5b2e..23ae517e9 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -188,9 +188,10 @@ async def notify_infraction( expires_at = "Never" duration = "Permanent" else: + origin = arrow.get(infraction["last_applied"]) expiry = arrow.get(infraction["expires_at"]) expires_at = time.format_relative(expiry) - duration = time.humanize_delta(infraction["inserted_at"], expiry, max_units=2) + duration = time.humanize_delta(origin, expiry, max_units=2) if infraction["active"]: remaining = time.humanize_delta(expiry, arrow.utcnow(), max_units=2) -- cgit v1.2.3 From b115d42195841e69e38c601f40a6a49826e7eac6 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sun, 31 Jul 2022 02:09:35 -0400 Subject: Added new expiry usage to apply - Added new usage of `last_applied` time for duration calculation in `apply_infraction` --- bot/exts/moderation/infraction/_scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index c7f03b2e9..28aafec2a 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -137,7 +137,7 @@ class InfractionScheduler: infr_type = infraction["type"] icon = _utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] - expiry = time.format_with_duration(infraction["expires_at"]) + expiry = time.format_with_duration(infraction["expires_at"], infraction["last_applied"]) id_ = infraction['id'] if user_reason is None: -- cgit v1.2.3 From b699d114c5fb5810915dc85f5aa0649a0b091a79 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 1 Aug 2022 00:55:00 -0400 Subject: Added microsecond rounding for `humanize_delta` --- bot/utils/time.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/bot/utils/time.py b/bot/utils/time.py index a0379c3ef..104ea026d 100644 --- a/bot/utils/time.py +++ b/bot/utils/time.py @@ -195,7 +195,11 @@ def humanize_delta( end = arrow.get(args[0]) start = arrow.get(args[1]) if len(args) == 2 else arrow.utcnow() - delta = relativedelta(end.datetime, start.datetime) + # Round microseconds + end = round_datetime(end.datetime) + start = round_datetime(start.datetime) + + delta = relativedelta(end, start) if absolute: delta = abs(delta) else: @@ -326,3 +330,14 @@ def until_expiration(expiry: Optional[Timestamp]) -> str: return "Expired" return format_relative(expiry) + + +def round_datetime(dt: datetime.datetime) -> datetime.datetime: + """ + Round a datetime object to the nearest second. + + Resulting datetime objects will have microsecond values of 0, useful for delta comparisons. + """ + if dt.microsecond >= 500000: + dt += datetime.timedelta(seconds=1) + return dt.replace(microsecond=0) -- cgit v1.2.3 From 090824f125e1e6879d179b00bd3297d829c0920a Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 1 Aug 2022 01:00:00 -0400 Subject: Infraction duration fallback if no `last_applied` field --- bot/exts/moderation/infraction/_scheduler.py | 8 +++++++- bot/exts/moderation/infraction/_utils.py | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index 28aafec2a..cfb585bf5 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -137,9 +137,15 @@ class InfractionScheduler: infr_type = infraction["type"] icon = _utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] - expiry = time.format_with_duration(infraction["expires_at"], infraction["last_applied"]) id_ = infraction['id'] + if "last_applied" in infraction: + origin = infraction["last_applied"] + else: # Fallback for previous API versions without `last_applied` + log.trace(f"No last_applied for infraction {id_}, using inserted_at time.") + origin = infraction["inserted_at"] + expiry = time.format_with_duration(infraction["expires_at"], origin) + if user_reason is None: user_reason = reason diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 23ae517e9..470ce05e1 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -188,7 +188,12 @@ async def notify_infraction( expires_at = "Never" duration = "Permanent" else: - origin = arrow.get(infraction["last_applied"]) + if "last_applied" in infraction: + origin = infraction["last_applied"] + else: # Fallback for previous API versions without `last_applied` + log.trace(f"No last_applied for infraction {infraction}, using inserted_at time.") + origin = infraction["inserted_at"] + origin = arrow.get(origin) expiry = arrow.get(infraction["expires_at"]) expires_at = time.format_relative(expiry) duration = time.humanize_delta(origin, expiry, max_units=2) -- cgit v1.2.3 From 0f4bc18ceb5ef5e55e003da409a3da8e3e1d9cf8 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 4 Aug 2022 14:54:09 +0100 Subject: Disable nose plugin in pytest This fixes an issue with pytest running functions called setup in test files when they shouldn't be run --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 77d8ee3d4..255710386 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,3 +84,9 @@ case_sensitive = true combine_as_imports = true line_length = 120 atomic = true + +[tool.pytest.ini_options] +# We don't use nose style tests so disable them in pytest. +# This stops pytest from running functions named `setup` in test files. +# See https://github.com/python-discord/bot/pull/2229#issuecomment-1204436420 +addopts = "-p no:nose" -- cgit v1.2.3 From 31f6b907b77aceb12eb8ef5079d007828a66784e Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 4 Aug 2022 17:30:59 +0100 Subject: Refactor infractions to avoid passing around coroutines This avoids warnings if they are never awaited, e.g. when functions are mocked in tests --- bot/exts/moderation/infraction/_scheduler.py | 13 +++++++------ bot/exts/moderation/infraction/infractions.py | 15 ++++++++++----- bot/exts/moderation/infraction/superstarify.py | 11 ++++++----- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index c7f03b2e9..db2ae9454 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -1,6 +1,7 @@ import textwrap import typing as t from abc import abstractmethod +from collections.abc import Callable from gettext import ngettext import arrow @@ -79,7 +80,7 @@ class InfractionScheduler: async def reapply_infraction( self, infraction: _utils.Infraction, - apply_coro: t.Optional[t.Awaitable] + action: t.Optional[Callable[[], None]] ) -> None: """Reapply an infraction if it's still active or deactivate it if less than 60 sec left.""" if infraction["expires_at"] is not None: @@ -101,7 +102,7 @@ class InfractionScheduler: # Allowing mod log since this is a passive action that should be logged. try: - await apply_coro + await action() except discord.HTTPException as e: # When user joined and then right after this left again before action completed, this can't apply roles if e.code == 10007 or e.status == 404: @@ -121,14 +122,14 @@ class InfractionScheduler: ctx: Context, infraction: _utils.Infraction, user: MemberOrUser, - action_coro: t.Optional[t.Awaitable] = None, + action: t.Optional[Callable[[], None]] = None, user_reason: t.Optional[str] = None, additional_info: str = "", ) -> bool: """ Apply an infraction to the user, log the infraction, and optionally notify the user. - `action_coro`, if not provided, will result in the infraction not getting scheduled for deletion. + `action`, if not provided, will result in the infraction not getting scheduled for deletion. `user_reason`, if provided, will be sent to the user in place of the infraction reason. `additional_info` will be attached to the text field in the mod-log embed. @@ -194,10 +195,10 @@ class InfractionScheduler: purge = infraction.get("purge", "") # Execute the necessary actions to apply the infraction on Discord. - if action_coro: + if action: log.trace(f"Awaiting the infraction #{id_} application action coroutine.") try: - await action_coro + await action() if expiry: # Schedule the expiration of the infraction. self.schedule_expiration(infraction) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 46fd3381c..8d784549f 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -52,8 +52,9 @@ class Infractions(InfractionScheduler, commands.Cog): if active_mutes: reason = f"Re-applying active mute: {active_mutes[0]['id']}" - action = member.add_roles(self._muted_role, reason=reason) + async def action() -> None: + await member.add_roles(self._muted_role, reason=reason) await self.reapply_infraction(active_mutes[0], action) # region: Permanent infractions @@ -388,7 +389,7 @@ class Infractions(InfractionScheduler, commands.Cog): log.trace(f"Attempting to kick {user} from voice because they've been muted.") await user.move_to(None, reason=reason) - await self.apply_infraction(ctx, infraction, user, action()) + await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy(member_arg=2) async def apply_kick(self, ctx: Context, user: Member, reason: t.Optional[str], **kwargs) -> None: @@ -406,7 +407,9 @@ class Infractions(InfractionScheduler, commands.Cog): if reason: reason = textwrap.shorten(reason, width=512, placeholder="...") - action = user.kick(reason=reason) + async def action() -> None: + await user.kick(reason=reason) + await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy(member_arg=2) @@ -455,7 +458,9 @@ class Infractions(InfractionScheduler, commands.Cog): if reason: reason = textwrap.shorten(reason, width=512, placeholder="...") - action = ctx.guild.ban(user, reason=reason, delete_message_days=purge_days) + async def action() -> None: + await ctx.guild.ban(user, reason=reason, delete_message_days=purge_days) + await self.apply_infraction(ctx, infraction, user, action) bb_cog: t.Optional[BigBrother] = self.bot.get_cog("Big Brother") @@ -493,7 +498,7 @@ class Infractions(InfractionScheduler, commands.Cog): await user.move_to(None, reason="Disconnected from voice to apply voice mute.") await user.remove_roles(self._voice_verified_role, reason=reason) - await self.apply_infraction(ctx, infraction, user, action()) + await self.apply_infraction(ctx, infraction, user, action) # endregion # region: Base pardon functions diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 0e6aaa1e7..02fbe7957 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -96,11 +96,12 @@ class Superstarify(InfractionScheduler, Cog): if active_superstarifies: infraction = active_superstarifies[0] - action = member.edit( - nick=self.get_nick(infraction["id"], member.id), - reason=f"Superstarified member tried to escape the prison: {infraction['id']}" - ) + async def action() -> None: + await member.edit( + nick=self.get_nick(infraction["id"], member.id), + reason=f"Superstarified member tried to escape the prison: {infraction['id']}" + ) await self.reapply_infraction(infraction, action) @command(name="superstarify", aliases=("force_nick", "star", "starify", "superstar")) @@ -175,7 +176,7 @@ class Superstarify(InfractionScheduler, Cog): ).format successful = await self.apply_infraction( - ctx, infraction, member, action(), + ctx, infraction, member, action, user_reason=user_message(reason=f'**Additional details:** {reason}\n\n' if reason else ''), additional_info=nickname_info ) -- cgit v1.2.3 From fb419bc8f0525e8296abc5017a0d316adb22d4c4 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Thu, 4 Aug 2022 17:32:12 +0100 Subject: Fix tests --- .../exts/moderation/infraction/test_infractions.py | 30 +++++++++++++--------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index 052048053..9636f0146 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -35,17 +35,20 @@ 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() + self.ctx.guild.ban = AsyncMock() await self.cog.apply_ban(self.ctx, self.target, "foo bar" * 3000) - self.ctx.guild.ban.assert_called_once_with( + self.cog.apply_infraction.assert_awaited_once_with( + self.ctx, {"foo": "bar", "purge": ""}, self.target, ANY + ) + + action = self.cog.apply_infraction.call_args.args[-1] + await action() + self.ctx.guild.ban.assert_awaited_once_with( self.target, reason=textwrap.shorten("foo bar" * 3000, 512, placeholder="..."), delete_message_days=0 ) - self.cog.apply_infraction.assert_awaited_once_with( - self.ctx, {"foo": "bar", "purge": ""}, self.target, self.ctx.guild.ban.return_value - ) @patch("bot.exts.moderation.infraction._utils.post_infraction") async def test_apply_kick_reason_truncation(self, post_infraction_mock): @@ -54,14 +57,17 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase): self.cog.apply_infraction = AsyncMock() self.cog.mod_log.ignore = Mock() - self.target.kick = Mock() + self.target.kick = AsyncMock() 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 + self.ctx, {"foo": "bar"}, self.target, ANY ) + action = self.cog.apply_infraction.call_args.args[-1] + await action() + self.target.kick.assert_awaited_once_with(reason=textwrap.shorten("foo bar" * 3000, 512, placeholder="...")) + @patch("bot.exts.moderation.infraction.infractions.constants.Roles.voice_verified", new=123456) class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): @@ -141,8 +147,8 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): async def action_tester(self, action, reason: str) -> None: """Helper method to test voice mute action.""" - self.assertTrue(inspect.iscoroutine(action)) - await action + self.assertTrue(inspect.iscoroutinefunction(action)) + await action() self.user.move_to.assert_called_once_with(None, reason=ANY) self.user.remove_roles.assert_called_once_with(self.cog._voice_verified_role, reason=reason) @@ -194,8 +200,8 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): # Test action action = self.cog.apply_infraction.call_args[0][-1] - self.assertTrue(inspect.iscoroutine(action)) - await action + self.assertTrue(inspect.iscoroutinefunction(action)) + await action() async def test_voice_unmute_user_not_found(self): """Should include info to return dict when user was not found from guild.""" -- cgit v1.2.3 From 29d882065e12f9a042b14d6dfaf3161af215c953 Mon Sep 17 00:00:00 2001 From: Izan Date: Sat, 6 Aug 2022 15:49:02 +0100 Subject: Make reference message in reminders italic. --- bot/exts/utils/reminders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index f33a9f448..f0da37f3b 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -228,7 +228,7 @@ class Reminders(Cog): # If the replied message has no content (e.g. only attachments/embeds) if content == "": - content = "See referenced message." + content = "*See referenced message.*" return content -- cgit v1.2.3 From 4a9bcdf44263d4ff1d144ec22d43d3e83c3ad85c Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:10:59 -0400 Subject: Changed datetime.now to arrow.utcnow - Used arrow.utcnow to reduce complexity and import --- bot/exts/moderation/infraction/_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 470ce05e1..eda56b97c 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -1,5 +1,5 @@ import typing as t -from datetime import datetime, timezone +from datetime import datetime import arrow import discord @@ -92,7 +92,7 @@ async def post_infraction( log.trace(f"Posting {infr_type} infraction for {user} to the API.") - current_time = datetime.now(tz=timezone.utc) + current_time = arrow.utcnow() payload = { "actor": ctx.author.id, # Don't use ctx.message.author; antispam only patches ctx.author. -- cgit v1.2.3 From 97b940a24ac90fd7ec8405b84e0b4f419c95d31a Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:11:36 -0400 Subject: Removed `inserted_at` fallback Given API updates, the fallback is not needed --- bot/exts/moderation/infraction/_scheduler.py | 11 ++++------- bot/exts/moderation/infraction/_utils.py | 7 +------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index cfb585bf5..cb3f7149f 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -138,13 +138,10 @@ class InfractionScheduler: icon = _utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] id_ = infraction['id'] - - if "last_applied" in infraction: - origin = infraction["last_applied"] - else: # Fallback for previous API versions without `last_applied` - log.trace(f"No last_applied for infraction {id_}, using inserted_at time.") - origin = infraction["inserted_at"] - expiry = time.format_with_duration(infraction["expires_at"], origin) + expiry = time.format_with_duration( + infraction["expires_at"], + infraction["last_applied"] + ) if user_reason is None: user_reason = reason diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index eda56b97c..407c97251 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -188,12 +188,7 @@ async def notify_infraction( expires_at = "Never" duration = "Permanent" else: - if "last_applied" in infraction: - origin = infraction["last_applied"] - else: # Fallback for previous API versions without `last_applied` - log.trace(f"No last_applied for infraction {infraction}, using inserted_at time.") - origin = infraction["inserted_at"] - origin = arrow.get(origin) + origin = arrow.get(infraction["last_applied"]) expiry = arrow.get(infraction["expires_at"]) expires_at = time.format_relative(expiry) duration = time.humanize_delta(origin, expiry, max_units=2) -- cgit v1.2.3 From d042f29420abf3682cdae3b8a16e9b5e043ad9e5 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:47:49 -0400 Subject: Refactored test to not use datetime patch - Used new method of dict subset comparison instead of datetime patching for better compat. with argument types --- tests/bot/exts/moderation/infraction/test_utils.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index b1f23e31c..5ba0f4273 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -1,7 +1,7 @@ import unittest from collections import namedtuple from datetime import datetime -from unittest.mock import AsyncMock, MagicMock, call, patch +from unittest.mock import AsyncMock, MagicMock, patch from botcore.site_api import ResponseCodeError from discord import Embed, Forbidden, HTTPException, NotFound @@ -351,11 +351,10 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.assertIsNone(actual) post_user_mock.assert_awaited_once_with(self.ctx, self.user) - @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar") - async def test_first_fail_second_success_user_post_infraction(self, post_user_mock, mock_datetime): + async def test_first_fail_second_success_user_post_infraction(self, post_user_mock): """Should post the user if they don't exist, POST infraction again, and return the response if successful.""" - payload = { + expected = { "actor": self.ctx.author.id, "hidden": False, "reason": "Test reason", @@ -363,13 +362,17 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.user.id, "active": True, "dm_sent": False, - "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the datetime.now function to return a specific time - mock_datetime.now.return_value = datetime(2020, 1, 1) self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + await_args = self.bot.api_client.post.await_args_list + self.assertEqual(len(await_args), 2, "Expected 2 awaits") + + # Since `last_applied` is based on current time, just check if expected is a subset of payload + for args in await_args: + payload: dict = args.kwargs["json"] + self.assertEqual(payload, payload | expected) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From 1eff1d81a19883136501757738cd8444634be893 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Sat, 6 Aug 2022 18:51:51 +0100 Subject: Add note to docstring, fix type-hints, and update log messages --- bot/exts/moderation/infraction/_scheduler.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index db2ae9454..a44b66804 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -1,7 +1,7 @@ import textwrap import typing as t from abc import abstractmethod -from collections.abc import Callable +from collections.abc import Awaitable, Callable from gettext import ngettext import arrow @@ -80,9 +80,14 @@ class InfractionScheduler: async def reapply_infraction( self, infraction: _utils.Infraction, - action: t.Optional[Callable[[], None]] + action: t.Optional[Callable[[], Awaitable[None]]] ) -> None: - """Reapply an infraction if it's still active or deactivate it if less than 60 sec left.""" + """ + Reapply an infraction if it's still active or deactivate it if less than 60 sec left. + + Note: The `action` provided is an async function rather than a coroutine + to prevent getting a RuntimeWarning if it is not used (e.g. in mocked tests). + """ if infraction["expires_at"] is not None: # Calculate the time remaining, in seconds, for the mute. expiry = dateutil.parser.isoparse(infraction["expires_at"]) @@ -112,7 +117,7 @@ class InfractionScheduler: else: log.exception( f"Got unexpected HTTPException (HTTP {e.status}, Discord code {e.code})" - f"when awaiting {infraction['type']} coroutine for {infraction['user']}." + f"when running {infraction['type']} action for {infraction['user']}." ) else: log.info(f"Re-applied {infraction['type']} to user {infraction['user']} upon rejoining.") @@ -122,7 +127,7 @@ class InfractionScheduler: ctx: Context, infraction: _utils.Infraction, user: MemberOrUser, - action: t.Optional[Callable[[], None]] = None, + action: t.Optional[Callable[[], Awaitable[None]]] = None, user_reason: t.Optional[str] = None, additional_info: str = "", ) -> bool: @@ -133,6 +138,9 @@ class InfractionScheduler: `user_reason`, if provided, will be sent to the user in place of the infraction reason. `additional_info` will be attached to the text field in the mod-log embed. + Note: The `action` provided is an async function rather than just a coroutine + to prevent getting a RuntimeWarning if it is not used (e.g. in mocked tests). + Returns whether or not the infraction succeeded. """ infr_type = infraction["type"] @@ -196,7 +204,7 @@ class InfractionScheduler: # Execute the necessary actions to apply the infraction on Discord. if action: - log.trace(f"Awaiting the infraction #{id_} application action coroutine.") + log.trace(f"Running the infraction #{id_} application action.") try: await action() if expiry: -- cgit v1.2.3 From a309f4a2791831f975f98ddb4025323948fdf512 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:57:51 -0400 Subject: Updated previous tests to use subset method --- tests/bot/exts/moderation/infraction/test_utils.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 5ba0f4273..6c9af2555 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -310,7 +310,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" now = datetime.now() - payload = { + expected = { "actor": self.ctx.author.id, "hidden": True, "reason": "Test reason", @@ -323,14 +323,12 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.ctx.bot.api_client.post.return_value = "foo" actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") self.ctx.bot.api_client.post.assert_awaited_once() - await_args = str(self.ctx.bot.api_client.post.await_args) - # Check existing keys present, allow for additional keys (e.g. `last_applied`) - for key, value in payload.items(): - self.assertTrue(key in await_args) - self.assertTrue(str(value) in await_args) + + # Since `last_applied` is based on current time, just check if expected is a subset of payload + payload: dict = self.ctx.bot.api_client.post.await_args_list[0].kwargs["json"] + self.assertEqual(payload, payload | expected) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" -- cgit v1.2.3 From e4d00ffd7f747097cb07baf54aa8b2ad72bbb010 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 15:45:52 -0400 Subject: Corrected test use of utcnow Corrected test case to use `datetime.utcnow()` to be consistent with target --- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 6c9af2555..29dadf372 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -309,7 +309,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" - now = datetime.now() + now = datetime.utcnow() expected = { "actor": self.ctx.author.id, "hidden": True, -- cgit v1.2.3 From 88a981e211c688a5b0dda4eb39181c505c8d14b5 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 15:48:54 -0400 Subject: Updated infractions display for updates - Added new infraction delta calculations to updated infractions. - Updates of infraction durations now also update the `last_applied` field. - `inserted_at` is now sent by the bot client to denote the original unmodified infraction application time --- bot/exts/moderation/infraction/_utils.py | 9 +++-- bot/exts/moderation/infraction/management.py | 24 ++++++++++---- bot/utils/time.py | 49 +++++++++++++++++++++------- 3 files changed, 59 insertions(+), 23 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 407c97251..5e708d7fe 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -12,6 +12,7 @@ from bot.converters import DurationOrExpiry, MemberOrUser from bot.errors import InvalidInfractedUserError from bot.log import get_logger from bot.utils import time +from bot.utils.time import unpack_duration log = get_logger(__name__) @@ -102,15 +103,13 @@ async def post_infraction( "user": user.id, "active": active, "dm_sent": dm_sent, + "inserted_at": current_time.isoformat(), "last_applied": current_time.isoformat(), } - # Parse duration or expiry if duration_or_expiry is not None: - if isinstance(duration_or_expiry, datetime): - payload['expires_at'] = duration_or_expiry.isoformat() - else: # is relativedelta - payload['expires_at'] = (current_time + duration_or_expiry).isoformat() + _, expiry = unpack_duration(duration_or_expiry, current_time) + payload["expires_at"] = expiry.isoformat() # Try to apply the infraction. If it fails because the user doesn't exist, try to add it. for should_post_user in (True, False): diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index a7d7a844a..6ef382119 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -2,6 +2,7 @@ import re import textwrap import typing as t +import arrow import discord from discord.ext import commands from discord.ext.commands import Context @@ -9,7 +10,7 @@ from discord.utils import escape_markdown from bot import constants from bot.bot import Bot -from bot.converters import Expiry, Infraction, MemberOrUser, Snowflake, UnambiguousUser +from bot.converters import DurationOrExpiry, Infraction, MemberOrUser, Snowflake, UnambiguousUser from bot.decorators import ensure_future_timestamp from bot.errors import InvalidInfraction from bot.exts.moderation.infraction import _utils @@ -20,6 +21,7 @@ from bot.pagination import LinePaginator from bot.utils import messages, time from bot.utils.channel import is_mod_channel from bot.utils.members import get_or_fetch_member +from bot.utils.time import unpack_duration log = get_logger(__name__) @@ -89,7 +91,7 @@ class ModManagement(commands.Cog): self, ctx: Context, infraction: Infraction, - duration: t.Union[Expiry, t.Literal["p", "permanent"], None], + duration: t.Union[DurationOrExpiry, t.Literal["p", "permanent"], None], *, reason: str = None ) -> None: @@ -129,7 +131,7 @@ class ModManagement(commands.Cog): self, ctx: Context, infraction: Infraction, - duration: t.Union[Expiry, t.Literal["p", "permanent"], None], + duration: t.Union[DurationOrExpiry, t.Literal["p", "permanent"], None], *, reason: str = None ) -> None: @@ -172,8 +174,11 @@ class ModManagement(commands.Cog): request_data['expires_at'] = None confirm_messages.append("marked as permanent") elif duration is not None: - request_data['expires_at'] = duration.isoformat() - expiry = time.format_with_duration(duration) + origin, expiry = unpack_duration(duration) + # Update `last_applied` if expiry changes. + request_data['last_applied'] = origin.isoformat() + request_data['expires_at'] = expiry.isoformat() + expiry = time.format_with_duration(expiry, origin) confirm_messages.append(f"set to expire on {expiry}") else: confirm_messages.append("expiry unchanged") @@ -380,7 +385,10 @@ class ModManagement(commands.Cog): user = infraction["user"] expires_at = infraction["expires_at"] inserted_at = infraction["inserted_at"] + last_applied = infraction["last_applied"] created = time.discord_timestamp(inserted_at) + applied = time.discord_timestamp(last_applied) + duration_edited = arrow.get(last_applied) > arrow.get(inserted_at) dm_sent = infraction["dm_sent"] # Format the user string. @@ -400,7 +408,11 @@ class ModManagement(commands.Cog): if expires_at is None: duration = "*Permanent*" else: - duration = time.humanize_delta(inserted_at, expires_at) + duration = time.humanize_delta(last_applied, expires_at) + + # Notice if infraction expiry was edited. + if duration_edited: + duration += f" (edited {applied})" # Format `dm_sent` if dm_sent is None: diff --git a/bot/utils/time.py b/bot/utils/time.py index 104ea026d..820ac2929 100644 --- a/bot/utils/time.py +++ b/bot/utils/time.py @@ -1,12 +1,18 @@ +from __future__ import annotations + import datetime import re +from copy import copy from enum import Enum from time import struct_time -from typing import Literal, Optional, Union, overload +from typing import Literal, Optional, TYPE_CHECKING, Union, overload import arrow from dateutil.relativedelta import relativedelta +if TYPE_CHECKING: + from bot.converters import DurationOrExpiry + _DURATION_REGEX = re.compile( r"((?P\d+?) ?(years|year|Y|y) ?)?" r"((?P\d+?) ?(months|month|m) ?)?" @@ -194,12 +200,8 @@ def humanize_delta( elif len(args) <= 2: end = arrow.get(args[0]) start = arrow.get(args[1]) if len(args) == 2 else arrow.utcnow() + delta = round_delta(relativedelta(end.datetime, start.datetime)) - # Round microseconds - end = round_datetime(end.datetime) - start = round_datetime(start.datetime) - - delta = relativedelta(end, start) if absolute: delta = abs(delta) else: @@ -332,12 +334,35 @@ def until_expiration(expiry: Optional[Timestamp]) -> str: return format_relative(expiry) -def round_datetime(dt: datetime.datetime) -> datetime.datetime: +def unpack_duration( + duration_or_expiry: DurationOrExpiry, + origin: Optional[Union[datetime.datetime, arrow.Arrow]] = None +) -> tuple[datetime.datetime, datetime.datetime]: + """ + Unpacks a DurationOrExpiry into a tuple of (origin, expiry). + + The `origin` defaults to the current UTC time at function call. + """ + if origin is None: + origin = datetime.datetime.now(tz=datetime.timezone.utc) + + if isinstance(origin, arrow.Arrow): + origin = origin.datetime + + if isinstance(duration_or_expiry, relativedelta): + return origin, origin + duration_or_expiry + else: + return origin, duration_or_expiry + + +def round_delta(delta: relativedelta) -> relativedelta: """ - Round a datetime object to the nearest second. + Rounds `delta` to the nearest second. - Resulting datetime objects will have microsecond values of 0, useful for delta comparisons. + Returns a copy with microsecond values of 0. """ - if dt.microsecond >= 500000: - dt += datetime.timedelta(seconds=1) - return dt.replace(microsecond=0) + delta = copy(delta) + if delta.microseconds >= 500000: + delta += relativedelta(seconds=1) + delta.microseconds = 0 + return delta -- cgit v1.2.3 From fb028d19ccfa768ad7accc9fbddc8c3b1696e0a0 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 17:00:41 -0400 Subject: Removed unused datetime import --- bot/exts/moderation/infraction/_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 5e708d7fe..5f2529c6b 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -1,5 +1,4 @@ import typing as t -from datetime import datetime import arrow import discord -- cgit v1.2.3 From 1df6034a1723fd3ff1bd88047ca6a62f920767e6 Mon Sep 17 00:00:00 2001 From: Izan Date: Mon, 15 Aug 2022 12:04:33 +0100 Subject: Fix incident tests. --- tests/bot/exts/moderation/test_incidents.py | 38 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/tests/bot/exts/moderation/test_incidents.py b/tests/bot/exts/moderation/test_incidents.py index 11fe565fc..53d98360c 100644 --- a/tests/bot/exts/moderation/test_incidents.py +++ b/tests/bot/exts/moderation/test_incidents.py @@ -20,6 +20,8 @@ from tests.helpers import ( MockUser ) +CURRENT_TIME = datetime.datetime(2022, 1, 1, tzinfo=datetime.timezone.utc) + class MockAsyncIterable: """ @@ -102,25 +104,32 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): async def test_make_embed_actioned(self): """Embed is coloured green and footer contains 'Actioned' when `outcome=Signal.ACTIONED`.""" - embed, file = await incidents.make_embed(MockMessage(), incidents.Signal.ACTIONED, MockMember()) + embed, file = await incidents.make_embed( + incident=MockMessage(created_at=CURRENT_TIME), + outcome=incidents.Signal.ACTIONED, + actioned_by=MockMember() + ) self.assertEqual(embed.colour.value, Colours.soft_green) self.assertIn("Actioned", embed.footer.text) async def test_make_embed_not_actioned(self): """Embed is coloured red and footer contains 'Rejected' when `outcome=Signal.NOT_ACTIONED`.""" - embed, file = await incidents.make_embed(MockMessage(), incidents.Signal.NOT_ACTIONED, MockMember()) + embed, file = await incidents.make_embed( + incident=MockMessage(created_at=CURRENT_TIME), + outcome=incidents.Signal.NOT_ACTIONED, + actioned_by=MockMember() + ) self.assertEqual(embed.colour.value, Colours.soft_red) self.assertIn("Rejected", embed.footer.text) async def test_make_embed_content(self): """Incident content appears as embed description.""" - current_time = datetime.datetime(2022, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) - incident = MockMessage(content="this is an incident", created_at=current_time) + incident = MockMessage(content="this is an incident", created_at=CURRENT_TIME) - reported_timestamp = discord_timestamp(current_time) - relative_timestamp = discord_timestamp(current_time, TimestampFormats.RELATIVE) + reported_timestamp = discord_timestamp(CURRENT_TIME) + relative_timestamp = discord_timestamp(CURRENT_TIME, TimestampFormats.RELATIVE) embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) @@ -133,7 +142,7 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): """Incident's attachment is downloaded and displayed in the embed's image field.""" file = MagicMock(discord.File, filename="bigbadjoe.jpg") attachment = MockAttachment(filename="bigbadjoe.jpg") - incident = MockMessage(content="this is an incident", attachments=[attachment]) + incident = MockMessage(content="this is an incident", attachments=[attachment], created_at=CURRENT_TIME) # Patch `download_file` to return our `file` with patch("bot.exts.moderation.incidents.download_file", AsyncMock(return_value=file)): @@ -145,7 +154,7 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): async def test_make_embed_with_attachment_fails(self): """Incident's attachment fails to download, proxy url is linked instead.""" attachment = MockAttachment(proxy_url="discord.com/bigbadjoe.jpg") - incident = MockMessage(content="this is an incident", attachments=[attachment]) + incident = MockMessage(content="this is an incident", attachments=[attachment], created_at=CURRENT_TIME) # Patch `download_file` to return None as if the download failed with patch("bot.exts.moderation.incidents.download_file", AsyncMock(return_value=None)): @@ -359,7 +368,6 @@ class TestCrawlIncidents(TestIncidents): class TestArchive(TestIncidents): """Tests for the `Incidents.archive` coroutine.""" - async def test_archive_webhook_not_found(self): """ Method recovers and returns False when the webhook is not found. @@ -369,7 +377,11 @@ class TestArchive(TestIncidents): """ self.cog_instance.bot.fetch_webhook = AsyncMock(side_effect=mock_404) self.assertFalse( - await self.cog_instance.archive(incident=MockMessage(), outcome=MagicMock(), actioned_by=MockMember()) + await self.cog_instance.archive( + incident=MockMessage(created_at=CURRENT_TIME), + outcome=MagicMock(), + actioned_by=MockMember() + ) ) async def test_archive_relays_incident(self): @@ -416,7 +428,7 @@ class TestArchive(TestIncidents): webhook = MockAsyncWebhook() self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook) - message_from_clyde = MockMessage(author=MockUser(display_name="clyde the great")) + message_from_clyde = MockMessage(author=MockUser(display_name="clyde the great"), created_at=CURRENT_TIME) await self.cog_instance.archive(message_from_clyde, MagicMock(incidents.Signal), MockMember()) self.assertNotIn("clyde", webhook.send.call_args.kwargs["username"]) @@ -520,7 +532,7 @@ class TestProcessEvent(TestIncidents): with patch("bot.exts.moderation.incidents.Incidents.make_confirmation_task", mock_task): await self.cog_instance.process_event( reaction=incidents.Signal.ACTIONED.value, - incident=MockMessage(author=mock_member, id=123), + incident=MockMessage(author=mock_member, id=123, created_at=CURRENT_TIME), member=mock_member ) @@ -540,7 +552,7 @@ class TestProcessEvent(TestIncidents): with patch("bot.exts.moderation.incidents.Incidents.make_confirmation_task", mock_task): await self.cog_instance.process_event( reaction=incidents.Signal.ACTIONED.value, - incident=MockMessage(id=123), + incident=MockMessage(id=123, created_at=CURRENT_TIME), member=MockMember(roles=[MockRole(id=1)]) ) except asyncio.TimeoutError: -- cgit v1.2.3 From fce6ea2f845889ab8eb6801e25e56ef0b7e67f3d Mon Sep 17 00:00:00 2001 From: dawnofmidnight Date: Mon, 15 Aug 2022 12:32:46 -0400 Subject: feat: command for banning compromised accounts --- bot/exts/moderation/infraction/infractions.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 46fd3381c..679768268 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -1,6 +1,7 @@ import textwrap import typing as t +import arrow import discord from discord import Member from discord.ext import commands @@ -11,6 +12,7 @@ from bot.bot import Bot from bot.constants import Event from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser from bot.decorators import ensure_future_timestamp, respect_role_hierarchy +from bot.exts.filters.filtering import AUTO_BAN_DURATION, AUTO_BAN_REASON from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler from bot.log import get_logger @@ -151,6 +153,11 @@ class Infractions(InfractionScheduler, commands.Cog): ctx.send = send await infr_manage_cog.infraction_append(ctx, infraction, None, reason=f"[Clean log]({log_url})") + @command() + async def compban(self, ctx: Context, user: UnambiguousMemberOrUser) -> None: + """Same as cleanban, but specifically for four days and with the ban reason used for compromised accounts.""" + await self.cleanban(ctx, user, duration=arrow.utcnow() + AUTO_BAN_DURATION, reason=AUTO_BAN_REASON) + @command(aliases=("vban",)) async def voiceban(self, ctx: Context) -> None: """ -- cgit v1.2.3 From 872281ea5faa44cf3659b1678174b1a713555288 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 15 Aug 2022 14:07:22 -0400 Subject: Added mod alerted notice to auto-infractions --- bot/exts/moderation/infraction/_scheduler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index c7f03b2e9..69c7d7afc 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -189,7 +189,10 @@ class InfractionScheduler: f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) if reason: - end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" + end_msg = ( + f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" + " Moderators have been alerted for review" + ) purge = infraction.get("purge", "") -- cgit v1.2.3 From a7234a824f3d4b2d052357d73d23a6e0c5cbb1b6 Mon Sep 17 00:00:00 2001 From: dawnofmidnight Date: Mon, 15 Aug 2022 14:38:01 -0400 Subject: fix: change use of arrow to datetime and change docstring wording --- bot/exts/filters/filtering.py | 4 ++-- bot/exts/moderation/infraction/infractions.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/exts/filters/filtering.py b/bot/exts/filters/filtering.py index ca6ad0064..51877d7f0 100644 --- a/bot/exts/filters/filtering.py +++ b/bot/exts/filters/filtering.py @@ -2,7 +2,7 @@ import asyncio import re import unicodedata import urllib.parse -from datetime import timedelta +from datetime import datetime, timedelta from typing import Any, Dict, List, Mapping, NamedTuple, Optional, Tuple, Union import arrow @@ -413,7 +413,7 @@ class Filtering(Cog): await context.invoke( context.command, msg.author, - arrow.utcnow() + AUTO_BAN_DURATION, + datetime.utcnow() + AUTO_BAN_DURATION, reason=AUTO_BAN_REASON ) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 679768268..c70ff7705 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -1,7 +1,7 @@ import textwrap import typing as t +from datetime import datetime -import arrow import discord from discord import Member from discord.ext import commands @@ -155,8 +155,8 @@ class Infractions(InfractionScheduler, commands.Cog): @command() async def compban(self, ctx: Context, user: UnambiguousMemberOrUser) -> None: - """Same as cleanban, but specifically for four days and with the ban reason used for compromised accounts.""" - await self.cleanban(ctx, user, duration=arrow.utcnow() + AUTO_BAN_DURATION, reason=AUTO_BAN_REASON) + """Same as cleanban, but specifically with the ban reason and duration used for compromised accounts.""" + await self.cleanban(ctx, user, duration=datetime.utcnow() + AUTO_BAN_DURATION, reason=AUTO_BAN_REASON) @command(aliases=("vban",)) async def voiceban(self, ctx: Context) -> None: -- cgit v1.2.3 From e68868e253128b9eb2035e5ea1bae93e67bda4f1 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Tue, 16 Aug 2022 16:29:28 -0400 Subject: Added newlines and non-mentioned mod role --- bot/exts/moderation/infraction/_scheduler.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index 69c7d7afc..0f8a55838 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -12,7 +12,7 @@ from discord.ext.commands import Context from bot import constants from bot.bot import Bot -from bot.constants import Colours +from bot.constants import Colours, Roles from bot.converters import MemberOrUser from bot.exts.moderation.infraction import _utils from bot.exts.moderation.modlog import ModLog @@ -190,8 +190,8 @@ class InfractionScheduler: ) if reason: end_msg = ( - f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" - " Moderators have been alerted for review" + f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})." + f"\n\n<@&{Roles.moderators}> have been alerted for review" ) purge = infraction.get("purge", "") @@ -246,7 +246,8 @@ class InfractionScheduler: # 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}.") + mentions = discord.AllowedMentions(users=[user], roles=False) + await ctx.send(f"{dm_result}{confirm_msg}{infr_message}.", allowed_mentions=mentions) # Send a log message to the mod log. # Don't use ctx.message.author for the actor; antispam only patches ctx.author. -- cgit v1.2.3 From 16e17f2155c0cf3241e2d5604e53b1e51e8ea386 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 03:42:31 -0400 Subject: Added `DurationOrExpiry` type union --- bot/converters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/converters.py b/bot/converters.py index 5800ea044..e97a25bdd 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -574,5 +574,6 @@ if t.TYPE_CHECKING: Infraction = t.Optional[dict] # noqa: F811 Expiry = t.Union[Duration, ISODateTime] +DurationOrExpiry = t.Union[DurationDelta, ISODateTime] MemberOrUser = t.Union[discord.Member, discord.User] UnambiguousMemberOrUser = t.Union[UnambiguousMember, UnambiguousUser] -- cgit v1.2.3 From 3ca6f7adf8126971ccd05ab7bffa30ad89257b1f Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:31:05 -0400 Subject: Refactoring for DurationOrExpiry --- bot/exts/moderation/infraction/_utils.py | 20 ++++++++++----- bot/exts/moderation/infraction/infractions.py | 34 +++++++++++++------------- bot/exts/moderation/infraction/superstarify.py | 4 +-- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 3a2485ec2..368a637c2 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -1,5 +1,5 @@ import typing as t -from datetime import datetime +from datetime import datetime, timezone import arrow import discord @@ -8,7 +8,7 @@ from discord.ext.commands import Context import bot from bot.constants import Colours, Icons -from bot.converters import MemberOrUser +from bot.converters import MemberOrUser, DurationOrExpiry from bot.errors import InvalidInfractedUserError from bot.log import get_logger from bot.utils import time @@ -80,7 +80,7 @@ async def post_infraction( user: MemberOrUser, infr_type: str, reason: str, - expires_at: datetime = None, + duration_or_expiry: t.Optional[DurationOrExpiry] = None, hidden: bool = False, active: bool = True, dm_sent: bool = False, @@ -92,6 +92,8 @@ async def post_infraction( log.trace(f"Posting {infr_type} infraction for {user} to the API.") + current_time = datetime.now(tz=timezone.utc) + payload = { "actor": ctx.author.id, # Don't use ctx.message.author; antispam only patches ctx.author. "hidden": hidden, @@ -99,10 +101,16 @@ async def post_infraction( "type": infr_type, "user": user.id, "active": active, - "dm_sent": dm_sent + "dm_sent": dm_sent, + "last_applied": current_time, } - if expires_at: - payload['expires_at'] = expires_at.isoformat() + + # Parse duration or expiry + if duration_or_expiry is not None: + if isinstance(duration_or_expiry, datetime): + payload['expires_at'] = duration_or_expiry.isoformat() + else: # is relativedelta + payload['expires_at'] = (current_time + duration_or_expiry).isoformat() # Try to apply the infraction. If it fails because the user doesn't exist, try to add it. for should_post_user in (True, False): diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 46fd3381c..1818997bb 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -9,7 +9,7 @@ from discord.ext.commands import Context, command from bot import constants from bot.bot import Bot from bot.constants import Event -from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser +from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser, DurationOrExpiry from bot.decorators import ensure_future_timestamp, respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler @@ -86,7 +86,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -95,7 +95,7 @@ class Infractions(InfractionScheduler, commands.Cog): If duration is specified, it temporarily bans that user for the given duration. """ - await self.apply_ban(ctx, user, reason, expires_at=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) @command(aliases=("cban", "purgeban", "pban")) @ensure_future_timestamp(timestamp_arg=3) @@ -103,7 +103,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -115,10 +115,10 @@ class Infractions(InfractionScheduler, commands.Cog): clean_cog: t.Optional[Clean] = self.bot.get_cog("Clean") if clean_cog is None: # If we can't get the clean cog, fall back to native purgeban. - await self.apply_ban(ctx, user, reason, purge_days=1, expires_at=duration) + await self.apply_ban(ctx, user, reason, purge_days=1, duration_or_expiry=duration) return - infraction = await self.apply_ban(ctx, user, reason, expires_at=duration) + infraction = await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) if not infraction or not infraction.get("id"): # Ban was unsuccessful, quit early. await ctx.send(":x: Failed to apply ban.") @@ -168,7 +168,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] ) -> None: @@ -177,7 +177,7 @@ class Infractions(InfractionScheduler, commands.Cog): If duration is specified, it temporarily voice mutes that user for the given duration. """ - await self.apply_voice_mute(ctx, user, reason, expires_at=duration) + await self.apply_voice_mute(ctx, user, reason, duration_or_expiry=duration) # endregion # region: Temporary infractions @@ -187,7 +187,7 @@ class Infractions(InfractionScheduler, commands.Cog): async def tempmute( self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[Expiry] = None, + duration: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -214,7 +214,7 @@ class Infractions(InfractionScheduler, commands.Cog): if duration is None: duration = await Duration().convert(ctx, "1h") - await self.apply_mute(ctx, user, reason, expires_at=duration) + await self.apply_mute(ctx, user, reason, duration_or_expiry=duration) @command(aliases=("tban",)) @ensure_future_timestamp(timestamp_arg=3) @@ -241,7 +241,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_ban(ctx, user, reason, expires_at=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) @command(aliases=("tempvban", "tvban")) async def tempvoiceban(self, ctx: Context) -> None: @@ -258,7 +258,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: Expiry, + duration: DurationOrExpiry, *, reason: t.Optional[str] ) -> None: @@ -277,7 +277,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_voice_mute(ctx, user, reason, expires_at=duration) + await self.apply_voice_mute(ctx, user, reason, duration_or_expiry=duration) # endregion # region: Permanent shadow infractions @@ -305,7 +305,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: Expiry, + duration: DurationOrExpiry, *, reason: t.Optional[str] = None ) -> None: @@ -324,7 +324,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_ban(ctx, user, reason, expires_at=duration, hidden=True) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration, hidden=True) # endregion # region: Remove infractions (un- commands) @@ -428,7 +428,7 @@ class Infractions(InfractionScheduler, commands.Cog): return None # In the case of a permanent ban, we don't need get_active_infractions to tell us if one is active - is_temporary = kwargs.get("expires_at") is not None + is_temporary = kwargs.get("duration_or_expiry") is not None active_infraction = await _utils.get_active_infraction(ctx, user, "ban", is_temporary) if active_infraction: @@ -436,7 +436,7 @@ class Infractions(InfractionScheduler, commands.Cog): log.trace("Tempban ignored as it cannot overwrite an active ban.") return None - if active_infraction.get('expires_at') is None: + if active_infraction.get("duration_or_expiry") is None: log.trace("Permaban already exists, notify.") await ctx.send(f":x: User is already permanently banned (#{active_infraction['id']}).") return None diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 0e6aaa1e7..f2aab7a92 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -10,7 +10,7 @@ from discord.utils import escape_markdown from bot import constants from bot.bot import Bot -from bot.converters import Duration, Expiry +from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler @@ -109,7 +109,7 @@ class Superstarify(InfractionScheduler, Cog): self, ctx: Context, member: Member, - duration: t.Optional[Expiry], + duration: t.Optional[DurationOrExpiry], *, reason: str = '', ) -> None: -- cgit v1.2.3 From 22ec794d1b2f65da5d737f0a06fb2680834308f6 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:38:18 -0400 Subject: Ran isort on imports --- bot/exts/moderation/infraction/_utils.py | 2 +- bot/exts/moderation/infraction/infractions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 368a637c2..80b65dae8 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -8,7 +8,7 @@ from discord.ext.commands import Context import bot from bot.constants import Colours, Icons -from bot.converters import MemberOrUser, DurationOrExpiry +from bot.converters import DurationOrExpiry, MemberOrUser from bot.errors import InvalidInfractedUserError from bot.log import get_logger from bot.utils import time diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 1818997bb..aff5e392f 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -9,7 +9,7 @@ from discord.ext.commands import Context, command from bot import constants from bot.bot import Bot from bot.constants import Event -from bot.converters import Age, Duration, Expiry, MemberOrUser, UnambiguousMemberOrUser, DurationOrExpiry +from bot.converters import Age, Duration, DurationOrExpiry, Expiry, MemberOrUser, UnambiguousMemberOrUser from bot.decorators import ensure_future_timestamp, respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler -- cgit v1.2.3 From 0e6242f7f4c41329e9270724a9780511f7165240 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:54:26 -0400 Subject: Updated tests - Refactored tests for new time duration arguments --- .../exts/moderation/infraction/test_infractions.py | 11 ++++---- tests/bot/exts/moderation/infraction/test_utils.py | 29 +++++++++++++--------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index 052048053..a18a4d23b 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -79,13 +79,13 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): """Should call voice mute applying function without expiry.""" self.cog.apply_voice_mute = AsyncMock() self.assertIsNone(await self.cog.voicemute(self.cog, self.ctx, self.user, reason="foobar")) - self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", expires_at=None) + self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", duration_or_expiry=None) async def test_temporary_voice_mute(self): """Should call voice mute applying function with expiry.""" self.cog.apply_voice_mute = AsyncMock() self.assertIsNone(await self.cog.tempvoicemute(self.cog, self.ctx, self.user, "baz", reason="foobar")) - self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", expires_at="baz") + self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", duration_or_expiry="baz") async def test_voice_unmute(self): """Should call infraction pardoning function.""" @@ -189,7 +189,8 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): user = MockUser() await self.cog.voicemute(self.cog, self.ctx, user, reason=None) - post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_mute", None, active=True, expires_at=None) + post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_mute", None, active=True, + duration_or_expiry=None) apply_infraction_mock.assert_called_once_with(self.cog, self.ctx, infraction, user, ANY) # Test action @@ -273,7 +274,7 @@ class CleanBanTests(unittest.IsolatedAsyncioTestCase): self.user, "FooBar", purge_days=1, - expires_at=None, + duration_or_expiry=None, ) async def test_cleanban_doesnt_purge_messages_if_clean_cog_available(self): @@ -285,7 +286,7 @@ class CleanBanTests(unittest.IsolatedAsyncioTestCase): self.ctx, self.user, "FooBar", - expires_at=None, + duration_or_expiry=None, ) @patch("bot.exts.moderation.infraction.infractions.Age") diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 5cf02033d..4c78c0bd8 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -318,14 +318,17 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.member.id, "active": False, "expires_at": now.isoformat(), - "dm_sent": False + "dm_sent": False, + "last_applied": datetime(2020, 1, 1).isoformat(), } - self.ctx.bot.api_client.post.return_value = "foo" - actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) + # Patch the time.now(tz=timezone.utc) function to return a specific time + with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): + self.ctx.bot.api_client.post.return_value = "foo" + actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.assertEqual(actual, "foo") + self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" @@ -356,12 +359,14 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "type": "mute", "user": self.user.id, "active": True, - "dm_sent": False + "dm_sent": False, + "last_applied": datetime(2020, 1, 1), } - self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] - - actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") - self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) - post_user_mock.assert_awaited_once_with(self.ctx, self.user) + # Patch the time.now(tz=timezone.utc) function to return a specific time + with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): + self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] + actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") + self.assertEqual(actual, "foo") + self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From 035b5accf78f8623b40e0612e3c057f0ef2b93a7 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:04:38 -0400 Subject: Fixed test patches --- tests/bot/exts/moderation/infraction/test_utils.py | 32 ++++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 4c78c0bd8..def06932b 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -307,7 +307,8 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.user = MockUser(id=1234) self.ctx = MockContext(bot=self.bot, author=self.member) - async def test_normal_post_infraction(self): + @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) + async def test_normal_post_infraction(self, mock_datetime): """Should return response from POST request if there are no errors.""" now = datetime.now() payload = { @@ -322,13 +323,13 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the time.now(tz=timezone.utc) function to return a specific time - with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): - self.ctx.bot.api_client.post.return_value = "foo" - actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) + # Patch the datetime.now function to return a specific time + mock_datetime.now.return_value = datetime(2020, 1, 1) + self.ctx.bot.api_client.post.return_value = "foo" + actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.assertEqual(actual, "foo") + self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" @@ -349,8 +350,9 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.assertIsNone(actual) post_user_mock.assert_awaited_once_with(self.ctx, self.user) + @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar") - async def test_first_fail_second_success_user_post_infraction(self, post_user_mock): + async def test_first_fail_second_success_user_post_infraction(self, post_user_mock, mock_datetime): """Should post the user if they don't exist, POST infraction again, and return the response if successful.""" payload = { "actor": self.ctx.author.id, @@ -363,10 +365,10 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "last_applied": datetime(2020, 1, 1), } - # Patch the time.now(tz=timezone.utc) function to return a specific time - with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): - self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] - actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") - self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) - post_user_mock.assert_awaited_once_with(self.ctx, self.user) + # Patch the datetime.now function to return a specific time + mock_datetime.now.return_value = datetime(2020, 1, 1) + self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] + actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") + self.assertEqual(actual, "foo") + self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From 8db7ef5df087e43804d07dabe2037af18adcb0d6 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:19:30 -0400 Subject: Added isoformat for test payload --- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index def06932b..d3a908b28 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -362,7 +362,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.user.id, "active": True, "dm_sent": False, - "last_applied": datetime(2020, 1, 1), + "last_applied": datetime(2020, 1, 1).isoformat(), } # Patch the datetime.now function to return a specific time -- cgit v1.2.3 From 1ad627adbe9f35d9d8c5bedc52295d4e79c1eb6f Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:20:48 -0400 Subject: Updated parameter names - Changed `duration` parameter names to `duration_or_expiry` to more accurately reflect options for help --- bot/exts/moderation/infraction/infractions.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index aff5e392f..12bd084df 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -9,7 +9,7 @@ from discord.ext.commands import Context, command from bot import constants from bot.bot import Bot from bot.constants import Event -from bot.converters import Age, Duration, DurationOrExpiry, Expiry, MemberOrUser, UnambiguousMemberOrUser +from bot.converters import Age, Duration, DurationOrExpiry, MemberOrUser, UnambiguousMemberOrUser from bot.decorators import ensure_future_timestamp, respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler @@ -86,7 +86,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: t.Optional[DurationOrExpiry] = None, + duration_or_expiry: t.Optional[DurationOrExpiry] = None, *, reason: t.Optional[str] = None ) -> None: @@ -95,7 +95,7 @@ class Infractions(InfractionScheduler, commands.Cog): If duration is specified, it temporarily bans that user for the given duration. """ - await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration_or_expiry) @command(aliases=("cban", "purgeban", "pban")) @ensure_future_timestamp(timestamp_arg=3) @@ -222,7 +222,7 @@ class Infractions(InfractionScheduler, commands.Cog): self, ctx: Context, user: UnambiguousMemberOrUser, - duration: Expiry, + duration_or_expiry: DurationOrExpiry, *, reason: t.Optional[str] = None ) -> None: @@ -241,7 +241,7 @@ class Infractions(InfractionScheduler, commands.Cog): Alternatively, an ISO 8601 timestamp can be provided for the duration. """ - await self.apply_ban(ctx, user, reason, duration_or_expiry=duration) + await self.apply_ban(ctx, user, reason, duration_or_expiry=duration_or_expiry) @command(aliases=("tempvban", "tvban")) async def tempvoiceban(self, ctx: Context) -> None: -- cgit v1.2.3 From 3e9ffa1ea3c7cfa7c583189fcb8675db9ee5c6d5 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:36:45 -0400 Subject: Updated ban command docstring - Updated docstring to be more explicit on parameter fields --- bot/exts/moderation/infraction/infractions.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index 12bd084df..83a947a19 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -91,9 +91,11 @@ class Infractions(InfractionScheduler, commands.Cog): reason: t.Optional[str] = None ) -> None: """ - Permanently ban a user for the given reason and stop watching them with Big Brother. + Permanently ban a `user` for the given `reason` and stop watching them with Big Brother. - If duration is specified, it temporarily bans that user for the given duration. + If a duration is specified, it temporarily bans the `user` for the given duration. + Alternatively, an ISO 8601 timestamp representing the expiry time can be provided + for `duration_or_expiry`. """ await self.apply_ban(ctx, user, reason, duration_or_expiry=duration_or_expiry) -- cgit v1.2.3 From 3280ac48a9031b727bdde69909729093593bd967 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 06:11:11 -0400 Subject: Fixed tests - Corrected datetime patching --- tests/bot/exts/moderation/infraction/test_utils.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index d3a908b28..b1f23e31c 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -307,8 +307,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.user = MockUser(id=1234) self.ctx = MockContext(bot=self.bot, author=self.member) - @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) - async def test_normal_post_infraction(self, mock_datetime): + async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" now = datetime.now() payload = { @@ -320,16 +319,18 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "active": False, "expires_at": now.isoformat(), "dm_sent": False, - "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the datetime.now function to return a specific time - mock_datetime.now.return_value = datetime(2020, 1, 1) self.ctx.bot.api_client.post.return_value = "foo" actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.ctx.bot.api_client.post.assert_awaited_once() + await_args = str(self.ctx.bot.api_client.post.await_args) + # Check existing keys present, allow for additional keys (e.g. `last_applied`) + for key, value in payload.items(): + self.assertTrue(key in await_args) + self.assertTrue(str(value) in await_args) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" -- cgit v1.2.3 From 08e344c8884b612ea0775d3141b33d0f4575d085 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 06:11:23 -0400 Subject: Correct last_applied formatting --- bot/exts/moderation/infraction/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 80b65dae8..cd82f5b2e 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -102,7 +102,7 @@ async def post_infraction( "user": user.id, "active": active, "dm_sent": dm_sent, - "last_applied": current_time, + "last_applied": current_time.isoformat(), } # Parse duration or expiry -- cgit v1.2.3 From f8e3656a74cfc0cae37cab65ed114319762f01e3 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 18:57:01 -0400 Subject: Use `last_applied` to display duration --- bot/exts/moderation/infraction/_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index cd82f5b2e..23ae517e9 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -188,9 +188,10 @@ async def notify_infraction( expires_at = "Never" duration = "Permanent" else: + origin = arrow.get(infraction["last_applied"]) expiry = arrow.get(infraction["expires_at"]) expires_at = time.format_relative(expiry) - duration = time.humanize_delta(infraction["inserted_at"], expiry, max_units=2) + duration = time.humanize_delta(origin, expiry, max_units=2) if infraction["active"]: remaining = time.humanize_delta(expiry, arrow.utcnow(), max_units=2) -- cgit v1.2.3 From 8d788a9fdc53a7af0730dfc925ea649fd5b810c4 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sun, 31 Jul 2022 02:09:35 -0400 Subject: Added new expiry usage to apply - Added new usage of `last_applied` time for duration calculation in `apply_infraction` --- bot/exts/moderation/infraction/_scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index c7f03b2e9..28aafec2a 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -137,7 +137,7 @@ class InfractionScheduler: infr_type = infraction["type"] icon = _utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] - expiry = time.format_with_duration(infraction["expires_at"]) + expiry = time.format_with_duration(infraction["expires_at"], infraction["last_applied"]) id_ = infraction['id'] if user_reason is None: -- cgit v1.2.3 From 3babbf2fa0a6a8d99e796af0cd78a7516b781898 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 1 Aug 2022 00:55:00 -0400 Subject: Added microsecond rounding for `humanize_delta` --- bot/utils/time.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/bot/utils/time.py b/bot/utils/time.py index a0379c3ef..104ea026d 100644 --- a/bot/utils/time.py +++ b/bot/utils/time.py @@ -195,7 +195,11 @@ def humanize_delta( end = arrow.get(args[0]) start = arrow.get(args[1]) if len(args) == 2 else arrow.utcnow() - delta = relativedelta(end.datetime, start.datetime) + # Round microseconds + end = round_datetime(end.datetime) + start = round_datetime(start.datetime) + + delta = relativedelta(end, start) if absolute: delta = abs(delta) else: @@ -326,3 +330,14 @@ def until_expiration(expiry: Optional[Timestamp]) -> str: return "Expired" return format_relative(expiry) + + +def round_datetime(dt: datetime.datetime) -> datetime.datetime: + """ + Round a datetime object to the nearest second. + + Resulting datetime objects will have microsecond values of 0, useful for delta comparisons. + """ + if dt.microsecond >= 500000: + dt += datetime.timedelta(seconds=1) + return dt.replace(microsecond=0) -- cgit v1.2.3 From 011cfddbbb29a4555c93995ba61ecf6919a4a68e Mon Sep 17 00:00:00 2001 From: ionite34 Date: Mon, 1 Aug 2022 01:00:00 -0400 Subject: Infraction duration fallback if no `last_applied` field --- bot/exts/moderation/infraction/_scheduler.py | 8 +++++++- bot/exts/moderation/infraction/_utils.py | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index 28aafec2a..cfb585bf5 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -137,9 +137,15 @@ class InfractionScheduler: infr_type = infraction["type"] icon = _utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] - expiry = time.format_with_duration(infraction["expires_at"], infraction["last_applied"]) id_ = infraction['id'] + if "last_applied" in infraction: + origin = infraction["last_applied"] + else: # Fallback for previous API versions without `last_applied` + log.trace(f"No last_applied for infraction {id_}, using inserted_at time.") + origin = infraction["inserted_at"] + expiry = time.format_with_duration(infraction["expires_at"], origin) + if user_reason is None: user_reason = reason diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 23ae517e9..470ce05e1 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -188,7 +188,12 @@ async def notify_infraction( expires_at = "Never" duration = "Permanent" else: - origin = arrow.get(infraction["last_applied"]) + if "last_applied" in infraction: + origin = infraction["last_applied"] + else: # Fallback for previous API versions without `last_applied` + log.trace(f"No last_applied for infraction {infraction}, using inserted_at time.") + origin = infraction["inserted_at"] + origin = arrow.get(origin) expiry = arrow.get(infraction["expires_at"]) expires_at = time.format_relative(expiry) duration = time.humanize_delta(origin, expiry, max_units=2) -- cgit v1.2.3 From 9fc8b5e72b0531dc0a05035b9e1f23dacaa404e9 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:10:59 -0400 Subject: Changed datetime.now to arrow.utcnow - Used arrow.utcnow to reduce complexity and import --- bot/exts/moderation/infraction/_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 470ce05e1..eda56b97c 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -1,5 +1,5 @@ import typing as t -from datetime import datetime, timezone +from datetime import datetime import arrow import discord @@ -92,7 +92,7 @@ async def post_infraction( log.trace(f"Posting {infr_type} infraction for {user} to the API.") - current_time = datetime.now(tz=timezone.utc) + current_time = arrow.utcnow() payload = { "actor": ctx.author.id, # Don't use ctx.message.author; antispam only patches ctx.author. -- cgit v1.2.3 From 1212ac808380280c7b7625eb2a43c75c92e17d5f Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:11:36 -0400 Subject: Removed `inserted_at` fallback Given API updates, the fallback is not needed --- bot/exts/moderation/infraction/_scheduler.py | 11 ++++------- bot/exts/moderation/infraction/_utils.py | 7 +------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index cfb585bf5..cb3f7149f 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -138,13 +138,10 @@ class InfractionScheduler: icon = _utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] id_ = infraction['id'] - - if "last_applied" in infraction: - origin = infraction["last_applied"] - else: # Fallback for previous API versions without `last_applied` - log.trace(f"No last_applied for infraction {id_}, using inserted_at time.") - origin = infraction["inserted_at"] - expiry = time.format_with_duration(infraction["expires_at"], origin) + expiry = time.format_with_duration( + infraction["expires_at"], + infraction["last_applied"] + ) if user_reason is None: user_reason = reason diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index eda56b97c..407c97251 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -188,12 +188,7 @@ async def notify_infraction( expires_at = "Never" duration = "Permanent" else: - if "last_applied" in infraction: - origin = infraction["last_applied"] - else: # Fallback for previous API versions without `last_applied` - log.trace(f"No last_applied for infraction {infraction}, using inserted_at time.") - origin = infraction["inserted_at"] - origin = arrow.get(origin) + origin = arrow.get(infraction["last_applied"]) expiry = arrow.get(infraction["expires_at"]) expires_at = time.format_relative(expiry) duration = time.humanize_delta(origin, expiry, max_units=2) -- cgit v1.2.3 From e74ff62122935da349d38a8c06eef14d1e3ba9aa Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:47:49 -0400 Subject: Refactored test to not use datetime patch - Used new method of dict subset comparison instead of datetime patching for better compat. with argument types --- tests/bot/exts/moderation/infraction/test_utils.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index b1f23e31c..5ba0f4273 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -1,7 +1,7 @@ import unittest from collections import namedtuple from datetime import datetime -from unittest.mock import AsyncMock, MagicMock, call, patch +from unittest.mock import AsyncMock, MagicMock, patch from botcore.site_api import ResponseCodeError from discord import Embed, Forbidden, HTTPException, NotFound @@ -351,11 +351,10 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.assertIsNone(actual) post_user_mock.assert_awaited_once_with(self.ctx, self.user) - @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar") - async def test_first_fail_second_success_user_post_infraction(self, post_user_mock, mock_datetime): + async def test_first_fail_second_success_user_post_infraction(self, post_user_mock): """Should post the user if they don't exist, POST infraction again, and return the response if successful.""" - payload = { + expected = { "actor": self.ctx.author.id, "hidden": False, "reason": "Test reason", @@ -363,13 +362,17 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.user.id, "active": True, "dm_sent": False, - "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the datetime.now function to return a specific time - mock_datetime.now.return_value = datetime(2020, 1, 1) self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + await_args = self.bot.api_client.post.await_args_list + self.assertEqual(len(await_args), 2, "Expected 2 awaits") + + # Since `last_applied` is based on current time, just check if expected is a subset of payload + for args in await_args: + payload: dict = args.kwargs["json"] + self.assertEqual(payload, payload | expected) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From d93afa3dbe16404a60e23acaf394affcda5aff89 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:57:51 -0400 Subject: Updated previous tests to use subset method --- tests/bot/exts/moderation/infraction/test_utils.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 5ba0f4273..6c9af2555 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -310,7 +310,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" now = datetime.now() - payload = { + expected = { "actor": self.ctx.author.id, "hidden": True, "reason": "Test reason", @@ -323,14 +323,12 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.ctx.bot.api_client.post.return_value = "foo" actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") self.ctx.bot.api_client.post.assert_awaited_once() - await_args = str(self.ctx.bot.api_client.post.await_args) - # Check existing keys present, allow for additional keys (e.g. `last_applied`) - for key, value in payload.items(): - self.assertTrue(key in await_args) - self.assertTrue(str(value) in await_args) + + # Since `last_applied` is based on current time, just check if expected is a subset of payload + payload: dict = self.ctx.bot.api_client.post.await_args_list[0].kwargs["json"] + self.assertEqual(payload, payload | expected) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" -- cgit v1.2.3 From 08a6bbc306d1ec54998b690eeeb258548e8e08fa Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 15:45:52 -0400 Subject: Corrected test use of utcnow Corrected test case to use `datetime.utcnow()` to be consistent with target --- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 6c9af2555..29dadf372 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -309,7 +309,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" - now = datetime.now() + now = datetime.utcnow() expected = { "actor": self.ctx.author.id, "hidden": True, -- cgit v1.2.3 From 1946ad455408c26916d3c5f6a68d886c25ddb93b Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 15:48:54 -0400 Subject: Updated infractions display for updates - Added new infraction delta calculations to updated infractions. - Updates of infraction durations now also update the `last_applied` field. - `inserted_at` is now sent by the bot client to denote the original unmodified infraction application time --- bot/exts/moderation/infraction/_utils.py | 9 +++-- bot/exts/moderation/infraction/management.py | 24 ++++++++++---- bot/utils/time.py | 49 +++++++++++++++++++++------- 3 files changed, 59 insertions(+), 23 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 407c97251..5e708d7fe 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -12,6 +12,7 @@ from bot.converters import DurationOrExpiry, MemberOrUser from bot.errors import InvalidInfractedUserError from bot.log import get_logger from bot.utils import time +from bot.utils.time import unpack_duration log = get_logger(__name__) @@ -102,15 +103,13 @@ async def post_infraction( "user": user.id, "active": active, "dm_sent": dm_sent, + "inserted_at": current_time.isoformat(), "last_applied": current_time.isoformat(), } - # Parse duration or expiry if duration_or_expiry is not None: - if isinstance(duration_or_expiry, datetime): - payload['expires_at'] = duration_or_expiry.isoformat() - else: # is relativedelta - payload['expires_at'] = (current_time + duration_or_expiry).isoformat() + _, expiry = unpack_duration(duration_or_expiry, current_time) + payload["expires_at"] = expiry.isoformat() # Try to apply the infraction. If it fails because the user doesn't exist, try to add it. for should_post_user in (True, False): diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index a7d7a844a..6ef382119 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -2,6 +2,7 @@ import re import textwrap import typing as t +import arrow import discord from discord.ext import commands from discord.ext.commands import Context @@ -9,7 +10,7 @@ from discord.utils import escape_markdown from bot import constants from bot.bot import Bot -from bot.converters import Expiry, Infraction, MemberOrUser, Snowflake, UnambiguousUser +from bot.converters import DurationOrExpiry, Infraction, MemberOrUser, Snowflake, UnambiguousUser from bot.decorators import ensure_future_timestamp from bot.errors import InvalidInfraction from bot.exts.moderation.infraction import _utils @@ -20,6 +21,7 @@ from bot.pagination import LinePaginator from bot.utils import messages, time from bot.utils.channel import is_mod_channel from bot.utils.members import get_or_fetch_member +from bot.utils.time import unpack_duration log = get_logger(__name__) @@ -89,7 +91,7 @@ class ModManagement(commands.Cog): self, ctx: Context, infraction: Infraction, - duration: t.Union[Expiry, t.Literal["p", "permanent"], None], + duration: t.Union[DurationOrExpiry, t.Literal["p", "permanent"], None], *, reason: str = None ) -> None: @@ -129,7 +131,7 @@ class ModManagement(commands.Cog): self, ctx: Context, infraction: Infraction, - duration: t.Union[Expiry, t.Literal["p", "permanent"], None], + duration: t.Union[DurationOrExpiry, t.Literal["p", "permanent"], None], *, reason: str = None ) -> None: @@ -172,8 +174,11 @@ class ModManagement(commands.Cog): request_data['expires_at'] = None confirm_messages.append("marked as permanent") elif duration is not None: - request_data['expires_at'] = duration.isoformat() - expiry = time.format_with_duration(duration) + origin, expiry = unpack_duration(duration) + # Update `last_applied` if expiry changes. + request_data['last_applied'] = origin.isoformat() + request_data['expires_at'] = expiry.isoformat() + expiry = time.format_with_duration(expiry, origin) confirm_messages.append(f"set to expire on {expiry}") else: confirm_messages.append("expiry unchanged") @@ -380,7 +385,10 @@ class ModManagement(commands.Cog): user = infraction["user"] expires_at = infraction["expires_at"] inserted_at = infraction["inserted_at"] + last_applied = infraction["last_applied"] created = time.discord_timestamp(inserted_at) + applied = time.discord_timestamp(last_applied) + duration_edited = arrow.get(last_applied) > arrow.get(inserted_at) dm_sent = infraction["dm_sent"] # Format the user string. @@ -400,7 +408,11 @@ class ModManagement(commands.Cog): if expires_at is None: duration = "*Permanent*" else: - duration = time.humanize_delta(inserted_at, expires_at) + duration = time.humanize_delta(last_applied, expires_at) + + # Notice if infraction expiry was edited. + if duration_edited: + duration += f" (edited {applied})" # Format `dm_sent` if dm_sent is None: diff --git a/bot/utils/time.py b/bot/utils/time.py index 104ea026d..820ac2929 100644 --- a/bot/utils/time.py +++ b/bot/utils/time.py @@ -1,12 +1,18 @@ +from __future__ import annotations + import datetime import re +from copy import copy from enum import Enum from time import struct_time -from typing import Literal, Optional, Union, overload +from typing import Literal, Optional, TYPE_CHECKING, Union, overload import arrow from dateutil.relativedelta import relativedelta +if TYPE_CHECKING: + from bot.converters import DurationOrExpiry + _DURATION_REGEX = re.compile( r"((?P\d+?) ?(years|year|Y|y) ?)?" r"((?P\d+?) ?(months|month|m) ?)?" @@ -194,12 +200,8 @@ def humanize_delta( elif len(args) <= 2: end = arrow.get(args[0]) start = arrow.get(args[1]) if len(args) == 2 else arrow.utcnow() + delta = round_delta(relativedelta(end.datetime, start.datetime)) - # Round microseconds - end = round_datetime(end.datetime) - start = round_datetime(start.datetime) - - delta = relativedelta(end, start) if absolute: delta = abs(delta) else: @@ -332,12 +334,35 @@ def until_expiration(expiry: Optional[Timestamp]) -> str: return format_relative(expiry) -def round_datetime(dt: datetime.datetime) -> datetime.datetime: +def unpack_duration( + duration_or_expiry: DurationOrExpiry, + origin: Optional[Union[datetime.datetime, arrow.Arrow]] = None +) -> tuple[datetime.datetime, datetime.datetime]: + """ + Unpacks a DurationOrExpiry into a tuple of (origin, expiry). + + The `origin` defaults to the current UTC time at function call. + """ + if origin is None: + origin = datetime.datetime.now(tz=datetime.timezone.utc) + + if isinstance(origin, arrow.Arrow): + origin = origin.datetime + + if isinstance(duration_or_expiry, relativedelta): + return origin, origin + duration_or_expiry + else: + return origin, duration_or_expiry + + +def round_delta(delta: relativedelta) -> relativedelta: """ - Round a datetime object to the nearest second. + Rounds `delta` to the nearest second. - Resulting datetime objects will have microsecond values of 0, useful for delta comparisons. + Returns a copy with microsecond values of 0. """ - if dt.microsecond >= 500000: - dt += datetime.timedelta(seconds=1) - return dt.replace(microsecond=0) + delta = copy(delta) + if delta.microseconds >= 500000: + delta += relativedelta(seconds=1) + delta.microseconds = 0 + return delta -- cgit v1.2.3 From 6a87a38d4e44b1afe617cdfa0cc5027aad8a3933 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 17:00:41 -0400 Subject: Removed unused datetime import --- bot/exts/moderation/infraction/_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 5e708d7fe..5f2529c6b 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -1,5 +1,4 @@ import typing as t -from datetime import datetime import arrow import discord -- cgit v1.2.3 From c9d5aac1d71bdbee672355e75ee8e655892c8b6c Mon Sep 17 00:00:00 2001 From: ionite34 Date: Tue, 16 Aug 2022 17:35:44 -0400 Subject: Added article to automute message --- bot/exts/moderation/infraction/_scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index 0f8a55838..280b0fb0c 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -191,7 +191,7 @@ class InfractionScheduler: if reason: end_msg = ( f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})." - f"\n\n<@&{Roles.moderators}> have been alerted for review" + f"\n\nThe <@&{Roles.moderators}> have been alerted for review" ) purge = infraction.get("purge", "") -- cgit v1.2.3 From 4eaecb32830aa122cd53656de562caf20b79e33d Mon Sep 17 00:00:00 2001 From: dawnofmidnight Date: Wed, 17 Aug 2022 17:02:25 -0400 Subject: fix: replace datetime.utcnow() with arrow --- bot/exts/filters/filtering.py | 4 ++-- bot/exts/moderation/infraction/infractions.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/exts/filters/filtering.py b/bot/exts/filters/filtering.py index 51877d7f0..e4df0b1fd 100644 --- a/bot/exts/filters/filtering.py +++ b/bot/exts/filters/filtering.py @@ -2,7 +2,7 @@ import asyncio import re import unicodedata import urllib.parse -from datetime import datetime, timedelta +from datetime import timedelta from typing import Any, Dict, List, Mapping, NamedTuple, Optional, Tuple, Union import arrow @@ -413,7 +413,7 @@ class Filtering(Cog): await context.invoke( context.command, msg.author, - datetime.utcnow() + AUTO_BAN_DURATION, + (arrow.utcnow() + AUTO_BAN_DURATION).datetime, reason=AUTO_BAN_REASON ) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index c70ff7705..08a3609a7 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -1,7 +1,7 @@ import textwrap import typing as t -from datetime import datetime +import arrow import discord from discord import Member from discord.ext import commands @@ -156,7 +156,7 @@ class Infractions(InfractionScheduler, commands.Cog): @command() async def compban(self, ctx: Context, user: UnambiguousMemberOrUser) -> None: """Same as cleanban, but specifically with the ban reason and duration used for compromised accounts.""" - await self.cleanban(ctx, user, duration=datetime.utcnow() + AUTO_BAN_DURATION, reason=AUTO_BAN_REASON) + await self.cleanban(ctx, user, duration=(arrow.utcnow() + AUTO_BAN_DURATION).datetime, reason=AUTO_BAN_REASON) @command(aliases=("vban",)) async def voiceban(self, ctx: Context) -> None: -- cgit v1.2.3 From 06f9f48d2ec947f447824cda05d30f3fef9a3ef1 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 18 Aug 2022 11:24:54 -0400 Subject: Made DM duration remaining optional with resend --- bot/exts/moderation/infraction/_utils.py | 12 +++++++----- bot/exts/moderation/infraction/management.py | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 5f2529c6b..02e3ae1aa 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -170,13 +170,16 @@ async def send_active_infraction_message(ctx: Context, infraction: Infraction) - async def notify_infraction( infraction: Infraction, user: MemberOrUser, - reason: t.Optional[str] = None + reason: t.Optional[str] = None, + resend: bool = False ) -> bool: """ DM a user about their new infraction and return True if the DM is successful. `reason` can be used to override what is in `infraction`. Otherwise, this data will be retrieved from `infraction`. + + If `resend` is True, the DM will include both the duration and remaining time. """ infr_id = infraction["id"] infr_type = infraction["type"].replace("_", " ").title() @@ -191,11 +194,10 @@ async def notify_infraction( expires_at = time.format_relative(expiry) duration = time.humanize_delta(origin, expiry, max_units=2) - if infraction["active"]: + if infraction["active"] and resend: remaining = time.humanize_delta(expiry, arrow.utcnow(), max_units=2) - if duration != remaining: - duration += f" ({remaining} remaining)" - else: + duration += f" ({remaining} remaining)" + elif not infraction["active"]: expires_at += " (Inactive)" log.trace(f"Sending {user} a DM about their {infr_type} infraction.") diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index 6ef382119..090fbcdbd 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -79,7 +79,7 @@ class ModManagement(commands.Cog): reason = infraction["reason"] or "No reason provided." reason += "\n\n**This is a re-sent message for a previously applied infraction which may have been edited.**" - if await _utils.notify_infraction(infraction, member, reason): + if await _utils.notify_infraction(infraction, member, reason, resend=True): await ctx.send(f":incoming_envelope: Resent DM for infraction `{id_}`.") else: await ctx.send(f"{constants.Emojis.failmail} Failed to resend DM for infraction `{id_}`.") -- cgit v1.2.3 From 8767bf6d73bf1e61f8047a4090930fcf0c08f42c Mon Sep 17 00:00:00 2001 From: Dorukyum <53639936+Dorukyum@users.noreply.github.com> Date: Thu, 18 Aug 2022 19:02:05 +0300 Subject: Check if channel.guild is None --- bot/exts/moderation/modlog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/modlog.py b/bot/exts/moderation/modlog.py index 67991730e..efa87ce25 100644 --- a/bot/exts/moderation/modlog.py +++ b/bot/exts/moderation/modlog.py @@ -552,7 +552,7 @@ class ModLog(Cog, name="ModLog"): channel = self.bot.get_channel(channel_id) # Ignore not found channels, DMs, and messages outside of the main guild. - if not channel or not hasattr(channel, "guild") or channel.guild.id != GuildConstant.id: + if not channel or channel.guild is None or channel.guild.id != GuildConstant.id: return True # Look at the parent channel of a thread. -- cgit v1.2.3 From dada405211eac996196cdfb0496f4ff22f9a656a Mon Sep 17 00:00:00 2001 From: arl Date: Thu, 18 Aug 2022 19:01:22 -0400 Subject: fix: don't include replied mentions in mention filter (#2017) Co-authored-by: Izan Co-authored-by: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Co-authored-by: Xithrius <15021300+Xithrius@users.noreply.github.com> --- bot/rules/mentions.py | 56 +++++++++++++++++++++++++++++++++----- tests/bot/rules/test_mentions.py | 58 ++++++++++++++++++++++++++++++++++++---- tests/helpers.py | 22 +++++++++++++++ 3 files changed, 124 insertions(+), 12 deletions(-) diff --git a/bot/rules/mentions.py b/bot/rules/mentions.py index 6f5addad1..ca1d0c01c 100644 --- a/bot/rules/mentions.py +++ b/bot/rules/mentions.py @@ -1,23 +1,65 @@ from typing import Dict, Iterable, List, Optional, Tuple -from discord import Member, Message +from discord import DeletedReferencedMessage, Member, Message, MessageType, NotFound + +import bot +from bot.log import get_logger + +log = get_logger(__name__) async def apply( last_message: Message, recent_messages: List[Message], config: Dict[str, int] ) -> Optional[Tuple[str, Iterable[Member], Iterable[Message]]]: - """Detects total mentions exceeding the limit sent by a single user.""" + """ + Detects total mentions exceeding the limit sent by a single user. + + Excludes mentions that are bots, themselves, or replied users. + + In very rare cases, may not be able to determine a + mention was to a reply, in which case it is not ignored. + """ relevant_messages = tuple( msg for msg in recent_messages if msg.author == last_message.author ) + # We use `msg.mentions` here as that is supplied by the api itself, to determine who was mentioned. + # Additionally, `msg.mentions` includes the user replied to, even if the mention doesn't occur in the body. + # In order to exclude users who are mentioned as a reply, we check if the msg has a reference + # + # While we could use regex to parse the message content, and get a list of + # the mentions, that solution is very prone to breaking. + # We would need to deal with codeblocks, escaping markdown, and any discrepancies between + # our implementation and discord's markdown parser which would cause false positives or false negatives. + total_recent_mentions = 0 + for msg in relevant_messages: + # We check if the message is a reply, and if it is try to get the author + # since we ignore mentions of a user that we're replying to + reply_author = None - total_recent_mentions = sum( - not user.bot - for msg in relevant_messages - for user in msg.mentions - ) + if msg.type == MessageType.reply: + ref = msg.reference + + if not (resolved := ref.resolved): + # It is possible, in a very unusual situation, for a message to have a reference + # that is both not in the cache, and deleted while running this function. + # In such a situation, this will throw an error which we catch. + try: + resolved = await bot.instance.get_partial_messageable(resolved.channel_id).fetch_message( + resolved.message_id + ) + except NotFound: + log.info('Could not fetch the reference message as it has been deleted.') + + if resolved and not isinstance(resolved, DeletedReferencedMessage): + reply_author = resolved.author + + for user in msg.mentions: + # Don't count bot or self mentions, or the user being replied to (if applicable) + if user.bot or user in {msg.author, reply_author}: + continue + total_recent_mentions += 1 if total_recent_mentions > config['max']: return ( diff --git a/tests/bot/rules/test_mentions.py b/tests/bot/rules/test_mentions.py index f8805ac48..e1f904917 100644 --- a/tests/bot/rules/test_mentions.py +++ b/tests/bot/rules/test_mentions.py @@ -1,15 +1,32 @@ -from typing import Iterable +from typing import Iterable, Optional + +import discord from bot.rules import mentions from tests.bot.rules import DisallowedCase, RuleTest -from tests.helpers import MockMember, MockMessage +from tests.helpers import MockMember, MockMessage, MockMessageReference -def make_msg(author: str, total_user_mentions: int, total_bot_mentions: int = 0) -> MockMessage: - """Makes a message with `total_mentions` mentions.""" +def make_msg( + author: str, + total_user_mentions: int, + total_bot_mentions: int = 0, + *, + reference: Optional[MockMessageReference] = None +) -> MockMessage: + """Makes a message from `author` with `total_user_mentions` user mentions and `total_bot_mentions` bot mentions.""" user_mentions = [MockMember() for _ in range(total_user_mentions)] bot_mentions = [MockMember(bot=True) for _ in range(total_bot_mentions)] - return MockMessage(author=author, mentions=user_mentions+bot_mentions) + + mentions = user_mentions + bot_mentions + if reference is not None: + # For the sake of these tests we assume that all references are mentions. + mentions.append(reference.resolved.author) + msg_type = discord.MessageType.reply + else: + msg_type = discord.MessageType.default + + return MockMessage(author=author, mentions=mentions, reference=reference, type=msg_type) class TestMentions(RuleTest): @@ -56,6 +73,16 @@ class TestMentions(RuleTest): ("bob",), 3, ), + DisallowedCase( + [make_msg("bob", 3, reference=MockMessageReference())], + ("bob",), + 3, + ), + DisallowedCase( + [make_msg("bob", 3, reference=MockMessageReference(reference_author_is_bot=True))], + ("bob",), + 3 + ) ) await self.run_disallowed(cases) @@ -71,6 +98,27 @@ class TestMentions(RuleTest): await self.run_allowed(cases) + async def test_ignore_reply_mentions(self): + """Messages with an allowed amount of mentions in the content, also containing reply mentions.""" + cases = ( + [ + make_msg("bob", 2, reference=MockMessageReference()) + ], + [ + make_msg("bob", 2, reference=MockMessageReference(reference_author_is_bot=True)) + ], + [ + make_msg("bob", 2, reference=MockMessageReference()), + make_msg("bob", 0, reference=MockMessageReference()) + ], + [ + make_msg("bob", 2, reference=MockMessageReference(reference_author_is_bot=True)), + make_msg("bob", 0, reference=MockMessageReference(reference_author_is_bot=True)) + ] + ) + + await self.run_allowed(cases) + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: last_message = case.recent_messages[0] return tuple( diff --git a/tests/helpers.py b/tests/helpers.py index 17214553c..687e15b96 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -492,6 +492,28 @@ class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): spec_set = attachment_instance +message_reference_instance = discord.MessageReference( + message_id=unittest.mock.MagicMock(id=1), + channel_id=unittest.mock.MagicMock(id=2), + guild_id=unittest.mock.MagicMock(id=3) +) + + +class MockMessageReference(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock MessageReference objects. + + Instances of this class will follow the specification of `discord.MessageReference` instances. + For more information, see the `MockGuild` docstring. + """ + spec_set = message_reference_instance + + def __init__(self, *, reference_author_is_bot: bool = False, **kwargs): + super().__init__(**kwargs) + referenced_msg_author = MockMember(name="bob", bot=reference_author_is_bot) + self.resolved = MockMessage(author=referenced_msg_author) + + class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. -- cgit v1.2.3 From dcd946b32c199cba04b9d0399c1d26d4d3acff51 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Fri, 19 Aug 2022 14:16:37 -0400 Subject: Duration for DM changed to Edited flag --- bot/exts/moderation/infraction/_utils.py | 15 ++++++--------- bot/exts/moderation/infraction/management.py | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index 02e3ae1aa..c03081b07 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -44,8 +44,8 @@ LONGEST_EXTRAS = max(len(INFRACTION_APPEAL_SERVER_FOOTER), len(INFRACTION_APPEAL INFRACTION_DESCRIPTION_TEMPLATE = ( "**Type:** {type}\n" - "**Expires:** {expires}\n" "**Duration:** {duration}\n" + "**Expires:** {expires}\n" "**Reason:** {reason}\n" ) @@ -170,16 +170,13 @@ async def send_active_infraction_message(ctx: Context, infraction: Infraction) - async def notify_infraction( infraction: Infraction, user: MemberOrUser, - reason: t.Optional[str] = None, - resend: bool = False + reason: t.Optional[str] = None ) -> bool: """ DM a user about their new infraction and return True if the DM is successful. `reason` can be used to override what is in `infraction`. Otherwise, this data will be retrieved from `infraction`. - - If `resend` is True, the DM will include both the duration and remaining time. """ infr_id = infraction["id"] infr_type = infraction["type"].replace("_", " ").title() @@ -194,12 +191,12 @@ async def notify_infraction( expires_at = time.format_relative(expiry) duration = time.humanize_delta(origin, expiry, max_units=2) - if infraction["active"] and resend: - remaining = time.humanize_delta(expiry, arrow.utcnow(), max_units=2) - duration += f" ({remaining} remaining)" - elif not infraction["active"]: + if not infraction["active"]: expires_at += " (Inactive)" + if infraction["inserted_at"] != infraction["last_applied"]: + duration += " (Edited)" + log.trace(f"Sending {user} a DM about their {infr_type} infraction.") if reason is None: diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index 090fbcdbd..6ef382119 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -79,7 +79,7 @@ class ModManagement(commands.Cog): reason = infraction["reason"] or "No reason provided." reason += "\n\n**This is a re-sent message for a previously applied infraction which may have been edited.**" - if await _utils.notify_infraction(infraction, member, reason, resend=True): + if await _utils.notify_infraction(infraction, member, reason): await ctx.send(f":incoming_envelope: Resent DM for infraction `{id_}`.") else: await ctx.send(f"{constants.Emojis.failmail} Failed to resend DM for infraction `{id_}`.") -- cgit v1.2.3 From 3fa905b14a8ac8e23159d487b2046d7ca62c7afb Mon Sep 17 00:00:00 2001 From: wookie184 Date: Sat, 10 Sep 2022 14:43:42 +0100 Subject: Fix error in help channels cog which assumed an embed would have a description --- bot/exts/help_channels/_channel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index d9cebf215..cfe774f4c 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -183,7 +183,8 @@ async def ensure_cached_claimant(channel: discord.TextChannel) -> None: log.info("Hit the dormant message embed before finding a claimant in %s (%d).", channel, channel.id) break # Only set the claimant if the first embed matches the claimed channel embed regex - if match := CLAIMED_BY_RE.match(message.embeds[0].description): + description = message.embeds[0].description + if (description is not None) and (match := CLAIMED_BY_RE.match(description)): await _caches.claimants.set(channel.id, int(match.group("user_id"))) return -- cgit v1.2.3 From 350df44731875c4e8b28fc509d83b431cfefbc5c Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Thu, 1 Sep 2022 21:31:33 +0100 Subject: Use venvs with poetry in Dockerfile This is required due to a regression in poetry, see https://github.com/HassanAbouelela/actions/pull/7 --- Dockerfile | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5bb400658..20b099a0b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,17 +1,26 @@ FROM --platform=linux/amd64 python:3.10-slim # Set pip to have no saved cache -ENV PIP_NO_CACHE_DIR=false \ - POETRY_VIRTUALENVS_CREATE=false +ENV PIP_NO_CACHE_DIR=1 \ + PIP_DISABLE_PIP_VERSION_CHECK=on \ + POETRY_VERSION=1.1.15 \ + POETRY_HOME="/opt/poetry" \ + POETRY_VIRTUALENVS_IN_PROJECT=true \ + POETRY_NO_INTERACTION=1 \ + INSTALL_DIR="/opt/dependencies" \ + APP_DIR="/bot" +ENV PATH="$POETRY_HOME/bin:/$INSTALL_DIR/.venv/bin:$PATH" -# Install poetry -RUN pip install -U poetry +RUN apt-get update \ + && apt-get -y upgrade \ + && apt-get install --no-install-recommends -y curl \ + && apt-get clean && rm -rf /var/lib/apt/lists/* -# Create the working directory -WORKDIR /bot +RUN curl -sSL https://install.python-poetry.org | python # Install project dependencies +WORKDIR $INSTALL_DIR COPY pyproject.toml poetry.lock ./ RUN poetry install --no-dev @@ -22,6 +31,7 @@ ARG git_sha="development" ENV GIT_SHA=$git_sha # Copy the source code in last to optimize rebuilding the image +WORKDIR $APP_DIR COPY . . ENTRYPOINT ["python3"] -- cgit v1.2.3 From e12e5911f971beae0c07c12f9cf632bbaf4e6629 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Thu, 1 Sep 2022 21:31:59 +0100 Subject: Use HassanAbouelela/setup-python for CI --- .github/workflows/lint-test.yml | 62 ++++------------------------------------- 1 file changed, 5 insertions(+), 57 deletions(-) diff --git a/.github/workflows/lint-test.yml b/.github/workflows/lint-test.yml index 2b3dd5b4f..a331659e6 100644 --- a/.github/workflows/lint-test.yml +++ b/.github/workflows/lint-test.yml @@ -33,57 +33,16 @@ jobs: REDDIT_SECRET: ham REDIS_PASSWORD: '' - # Configure pip to cache dependencies and do a user install - PIP_NO_CACHE_DIR: false - PIP_USER: 1 - - # Make sure package manager does not use virtualenv - POETRY_VIRTUALENVS_CREATE: false - - # Specify explicit paths for python dependencies and the pre-commit - # environment so we know which directories to cache - POETRY_CACHE_DIR: ${{ github.workspace }}/.cache/py-user-base - PYTHONUSERBASE: ${{ github.workspace }}/.cache/py-user-base - PRE_COMMIT_HOME: ${{ github.workspace }}/.cache/pre-commit-cache - - # See https://github.com/pre-commit/pre-commit/issues/2178#issuecomment-1002163763 - # for why we set this. - SETUPTOOLS_USE_DISTUTILS: stdlib - steps: - - name: Add custom PYTHONUSERBASE to PATH - run: echo '${{ env.PYTHONUSERBASE }}/bin/' >> $GITHUB_PATH - - name: Checkout repository uses: actions/checkout@v2 - - name: Setup python - id: python - uses: actions/setup-python@v2 - with: - python-version: '3.10' - - # This step caches our Python dependencies. To make sure we - # only restore a cache when the dependencies, the python version, - # the runner operating system, and the dependency location haven't - # changed, we create a cache key that is a composite of those states. - # - # Only when the context is exactly the same, we will restore the cache. - - name: Python Dependency Caching - uses: actions/cache@v2 - id: python_cache + - name: Install Python Dependencies + uses: HassanAbouelela/actions/setup-python@setup-python_v1.3.1 with: - path: ${{ env.PYTHONUSERBASE }} - key: "python-0-${{ runner.os }}-${{ env.PYTHONUSERBASE }}-\ - ${{ steps.python.outputs.python-version }}-\ - ${{ hashFiles('./pyproject.toml', './poetry.lock') }}" - - # Install our dependencies if we did not restore a dependency cache - - name: Install dependencies using poetry - if: steps.python_cache.outputs.cache-hit != 'true' - run: | - pip install poetry - poetry install + # Set dev=true to install flake8 extensions, which are dev dependencies + dev: true + python_version: '3.10' # Check all of our non-dev dependencies are compatible with the MIT license. # If you added a new dependencies that is being rejected, @@ -94,17 +53,6 @@ jobs: pip-licenses --allow-only="$ALLOWED_LICENSE" \ --package $(poetry export -f requirements.txt --without-hashes | sed "s/==.*//g" | tr "\n" " ") - # This step caches our pre-commit environment. To make sure we - # do create a new environment when our pre-commit setup changes, - # we create a cache key based on relevant factors. - - name: Pre-commit Environment Caching - uses: actions/cache@v2 - with: - path: ${{ env.PRE_COMMIT_HOME }} - key: "precommit-0-${{ runner.os }}-${{ env.PRE_COMMIT_HOME }}-\ - ${{ steps.python.outputs.python-version }}-\ - ${{ hashFiles('./.pre-commit-config.yaml') }}" - # We will not run `flake8` here, as we will use a separate flake8 # action. As pre-commit does not support user installs, we set # PIP_USER=0 to not do a user install. -- cgit v1.2.3 From 0b495e757c909d6a5802ffa729cdc9d3e4069978 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sat, 10 Sep 2022 19:57:16 +0100 Subject: Bump poetry in Docker and lint to 1.2.0 --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 20b099a0b..d0687475e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM --platform=linux/amd64 python:3.10-slim # Set pip to have no saved cache ENV PIP_NO_CACHE_DIR=1 \ PIP_DISABLE_PIP_VERSION_CHECK=on \ - POETRY_VERSION=1.1.15 \ + POETRY_VERSION=1.2.0 \ POETRY_HOME="/opt/poetry" \ POETRY_VIRTUALENVS_IN_PROJECT=true \ POETRY_NO_INTERACTION=1 \ -- cgit v1.2.3 From 03a3a03d39099b09ed62337b075a9ada43353144 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 12 Sep 2022 21:37:04 +0100 Subject: Don't use fake in-project venvs for poetry Instead let poetry install the venv for the project in the right place, leading to a more 'traditional' poetry setup. --- Dockerfile | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/Dockerfile b/Dockerfile index d0687475e..9cf9c7b27 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,16 +1,15 @@ FROM --platform=linux/amd64 python:3.10-slim -# Set pip to have no saved cache -ENV PIP_NO_CACHE_DIR=1 \ - PIP_DISABLE_PIP_VERSION_CHECK=on \ - POETRY_VERSION=1.2.0 \ - POETRY_HOME="/opt/poetry" \ - POETRY_VIRTUALENVS_IN_PROJECT=true \ - POETRY_NO_INTERACTION=1 \ - INSTALL_DIR="/opt/dependencies" \ - APP_DIR="/bot" - -ENV PATH="$POETRY_HOME/bin:/$INSTALL_DIR/.venv/bin:$PATH" +# Define Git SHA build argument for sentry +ARG git_sha="development" + +ENV POETRY_VERSION=1.2.0 \ + POETRY_HOME="/opt/poetry" \ + POETRY_NO_INTERACTION=1 \ + APP_DIR="/bot" \ + GIT_SHA=$git_sha + +ENV PATH="$POETRY_HOME/bin:$PATH" RUN apt-get update \ && apt-get -y upgrade \ @@ -20,19 +19,12 @@ RUN apt-get update \ RUN curl -sSL https://install.python-poetry.org | python # Install project dependencies -WORKDIR $INSTALL_DIR +WORKDIR $APP_DIR COPY pyproject.toml poetry.lock ./ RUN poetry install --no-dev -# Define Git SHA build argument -ARG git_sha="development" - -# Set Git SHA environment variable for Sentry -ENV GIT_SHA=$git_sha - # Copy the source code in last to optimize rebuilding the image -WORKDIR $APP_DIR COPY . . -ENTRYPOINT ["python3"] -CMD ["-m", "bot"] +ENTRYPOINT ["poetry"] +CMD ["run", "python", "-m", "bot"] -- cgit v1.2.3 From 71b1802b0f91b855443fa5ed01d9134944fd6f38 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 13 Sep 2022 00:25:36 +0100 Subject: Ignore mounted in-project venvs on startup Poetry's virtualenvs.in-project config deafults to None, meaning it will use in-project venvs if it finds one, otherwise it will use the cache dir. In dev we mount the entire root project directory to /bot. This means if the host's venv in in the project dir, this will get mounted and prioritised by poetry run. If the host is on a non-linux OS this will cause poetry to fail to boot. --- Dockerfile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 9cf9c7b27..da65bbf3b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,9 +3,12 @@ FROM --platform=linux/amd64 python:3.10-slim # Define Git SHA build argument for sentry ARG git_sha="development" +# POETRY_VIRTUALENVS_IN_PROJECT is required to ensure in-projects venvs mounted from the host in dev +# don't get prioritised by `poetry run` ENV POETRY_VERSION=1.2.0 \ POETRY_HOME="/opt/poetry" \ POETRY_NO_INTERACTION=1 \ + POETRY_VIRTUALENVS_IN_PROJECT=false \ APP_DIR="/bot" \ GIT_SHA=$git_sha @@ -21,7 +24,7 @@ RUN curl -sSL https://install.python-poetry.org | python # Install project dependencies WORKDIR $APP_DIR COPY pyproject.toml poetry.lock ./ -RUN poetry install --no-dev +RUN poetry install --without dev # Copy the source code in last to optimize rebuilding the image COPY . . -- cgit v1.2.3 From 2db6a241bde75e6cd36f3fe0f32a3530187a827a Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 13 Sep 2022 00:27:53 +0100 Subject: Specify the path for poetry venvs Without this the venv would be created in /root/.cache and the nonn-root user that prod runs under would not have access to it. --- Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index da65bbf3b..65ca8ce51 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,7 +6,8 @@ ARG git_sha="development" # POETRY_VIRTUALENVS_IN_PROJECT is required to ensure in-projects venvs mounted from the host in dev # don't get prioritised by `poetry run` ENV POETRY_VERSION=1.2.0 \ - POETRY_HOME="/opt/poetry" \ + POETRY_HOME="/opt/poetry/home" \ + POETRY_CACHE_DIR="/opt/poetry/cache" \ POETRY_NO_INTERACTION=1 \ POETRY_VIRTUALENVS_IN_PROJECT=false \ APP_DIR="/bot" \ -- cgit v1.2.3 From 9c85342f4449f2116fb36ee1500ad06a94c1e14c Mon Sep 17 00:00:00 2001 From: Izan Date: Wed, 14 Sep 2022 20:56:30 +0100 Subject: Update docstrings & comment. --- bot/exts/utils/reminders.py | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index f0da37f3b..c65785314 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -214,7 +214,7 @@ class Reminders(Cog): """ Attempts to get content from the referenced message, if applicable. - Differs from botcore.utils.commands.clean_text_or_reply as allows for embeds. + Differs from botcore.utils.commands.clean_text_or_reply as allows for messages with no content. """ content = None if reference := ctx.message.reference: @@ -398,20 +398,7 @@ class Reminders(Cog): @remind_group.group(name="edit", aliases=("change", "modify"), invoke_without_command=True) async def edit_reminder_group(self, ctx: Context) -> None: - """ - Commands for modifying your current reminders. - - The `expiration` duration supports the following symbols for each unit of time: - - years: `Y`, `y`, `year`, `years` - - months: `m`, `month`, `months` - - weeks: `w`, `W`, `week`, `weeks` - - days: `d`, `D`, `day`, `days` - - hours: `H`, `h`, `hour`, `hours` - - minutes: `M`, `minute`, `minutes` - - seconds: `S`, `s`, `second`, `seconds` - - For example, to edit a reminder to expire in 3 days and 1 minute, you can do `!remind edit duration 1234 3d1M`. - """ + """Commands for modifying your current reminders.""" await ctx.send_help(ctx.command) @edit_reminder_group.command(name="duration", aliases=("time",)) @@ -434,11 +421,15 @@ class Reminders(Cog): @edit_reminder_group.command(name="content", aliases=("reason",)) async def edit_reminder_content(self, ctx: Context, id_: int, *, content: t.Optional[str] = None) -> None: - """Edit one of your reminder's content.""" + """ + Edit one of your reminder's content. + + You can either supply the new content yourself, or reply to a message to use its content. + """ if not content: content = await self.try_get_content_from_reply(ctx) if not content: - # Couldn't get content from reply + # Message doesn't have a reply to get content from return await self.edit_reminder(ctx, id_, {"content": content}) -- cgit v1.2.3 From 8028690d4eae27e57dfe1429b8067eabaa94eef9 Mon Sep 17 00:00:00 2001 From: Aleksey Zasorin Date: Fri, 16 Sep 2022 10:42:16 -0700 Subject: Removed "redis_ready" from additional_spec_asyncs in MockBot (#2275) The attribute was removed from Bot in fc05849 --- tests/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/helpers.py b/tests/helpers.py index 687e15b96..a4b919dcb 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -317,7 +317,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): guild_id=1, intents=discord.Intents.all(), ) - additional_spec_asyncs = ("wait_for", "redis_ready") + additional_spec_asyncs = ("wait_for",) def __init__(self, **kwargs) -> None: super().__init__(**kwargs) -- cgit v1.2.3 From 343858f55ab8cbb482c70b8ffd3c8e48ab35d3b1 Mon Sep 17 00:00:00 2001 From: Mohammad Ibrahim <74553450+Ibrahim2750mi@users.noreply.github.com> Date: Sat, 17 Sep 2022 18:28:48 +0530 Subject: Removed italics from the help command (#2272) * removed asterisk from embed description * removed italics from line 334, 375 and 415 * pagination.py, L239 added italics --- bot/exts/info/help.py | 8 ++++---- bot/pagination.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/exts/info/help.py b/bot/exts/info/help.py index 282f8c97a..48f840e51 100644 --- a/bot/exts/info/help.py +++ b/bot/exts/info/help.py @@ -307,7 +307,7 @@ class CustomHelpCommand(HelpCommand): # Remove line breaks from docstrings, if not used to separate paragraphs. # Allow overriding this behaviour via putting \u2003 at the start of a line. formatted_doc = re.sub("(? Date: Sun, 18 Sep 2022 02:07:12 +0400 Subject: Use Python Poetry Base Action (#2277) --- Dockerfile | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/Dockerfile b/Dockerfile index 65ca8ce51..205b66209 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,29 +1,11 @@ -FROM --platform=linux/amd64 python:3.10-slim +FROM --platform=linux/amd64 ghcr.io/chrislovering/python-poetry-base:3.10-slim # Define Git SHA build argument for sentry ARG git_sha="development" - -# POETRY_VIRTUALENVS_IN_PROJECT is required to ensure in-projects venvs mounted from the host in dev -# don't get prioritised by `poetry run` -ENV POETRY_VERSION=1.2.0 \ - POETRY_HOME="/opt/poetry/home" \ - POETRY_CACHE_DIR="/opt/poetry/cache" \ - POETRY_NO_INTERACTION=1 \ - POETRY_VIRTUALENVS_IN_PROJECT=false \ - APP_DIR="/bot" \ - GIT_SHA=$git_sha - -ENV PATH="$POETRY_HOME/bin:$PATH" - -RUN apt-get update \ - && apt-get -y upgrade \ - && apt-get install --no-install-recommends -y curl \ - && apt-get clean && rm -rf /var/lib/apt/lists/* - -RUN curl -sSL https://install.python-poetry.org | python +ENV GIT_SHA=$git_sha # Install project dependencies -WORKDIR $APP_DIR +WORKDIR /bot COPY pyproject.toml poetry.lock ./ RUN poetry install --without dev -- cgit v1.2.3