From f8ea3e884128883381d8b4d37ff94e62f0400e36 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Thu, 9 Dec 2021 22:58:15 +0530 Subject: [PATCH 01/11] Add day duration convertor This convertor is used to convert hours/minutes in 12/24 hour format to a datetime object using today's datetime as the base. --- bot/converters.py | 48 +++++++++++++++++++++++++++++++++ bot/exts/moderation/modpings.py | 8 +++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 559e759e1a..7a5312467a 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -574,6 +574,54 @@ async def convert(self, ctx: Context, arg: str) -> t.Optional[dict]: raise e +class DayDuration(Converter): + """ + Convert a string representing day time (hours and minutes) to an UTC datetime object. + + The hours and mintues would be combined with UTC day, if no 'am' or 'pm' is passed with + the string, then it is assumed that the time is in 24 hour format. + + The following formats are excepted: + - H:M + - H:M am/pm + - H am/pm + - H + + where `H` represents Hours and `M` represents Minutes. + """ + + TIME_RE = re.compile( + r"^(1[0-2]|0?[1-9]):?([0-5][0-9])? ?([AaPp][Mm])$" # Twelve hour format + "|" + r"^([0-9]|0[0-9]|1[0-9]|2[0-4]):?([0-5][0-9])?$" # Twenty four hour format + ) + + async def convert(self, _ctx: Context, argument: str) -> datetime: + """Attempts to converting `argument` to an UTC datetime object.""" + match = self.TIME_RE.fullmatch(argument).groups() + if not match: + raise BadArgument(f"`{argument}` is not a valid time duration string.") + + hour_12, minute_12, meridiem, hour_24, minute_24 = match + time = None + + if hour_12 and meridiem and minute_12: + time = datetime.strptime(f"{hour_12}:{minute_12} {meridiem}", "%I:%M %p") + elif hour_12 and meridiem: + time = datetime.strptime(f"{hour_12} {meridiem}", "%I %p") + elif hour_24 and minute_24: + time = datetime.strptime(f"{hour_24}:{minute_24}", "%H:%M") + else: + time = datetime.strptime(hour_24, "%H") + + today = datetime.utcnow().date() + return time.replace( + year=today.year, + month=today.month, + day=today.day + ) + + if t.TYPE_CHECKING: ValidDiscordServerInvite = dict # noqa: F811 ValidFilterListType = str # noqa: F811 diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index 20a8c39d70..d214601de4 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -3,13 +3,13 @@ import arrow from async_rediscache import RedisCache -from dateutil.parser import isoparse, parse as dateutil_parse +from dateutil.parser import isoparse from discord import Embed, Member from discord.ext.commands import Cog, Context, group, has_any_role from bot.bot import Bot from bot.constants import Colours, Emojis, Guild, Icons, MODERATION_ROLES, Roles -from bot.converters import Expiry +from bot.converters import DayDuration, Expiry from bot.log import get_logger from bot.utils import scheduling from bot.utils.scheduling import Scheduler @@ -199,9 +199,9 @@ async def on_command(self, ctx: Context) -> None: invoke_without_command=True ) @has_any_role(*MODERATION_ROLES) - async def schedule_modpings(self, ctx: Context, start: str, end: str) -> None: + async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDuration) -> None: """Schedule modpings role to be added at and removed at everyday at UTC time!""" - start, end = dateutil_parse(start), dateutil_parse(end) + print(start, end) if end < start: end += datetime.timedelta(days=1) From 3dda75457f5f9ecf5fd7969dcf351d747141a244 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Thu, 9 Dec 2021 23:05:42 +0530 Subject: [PATCH 02/11] Make 16hours the max off time and not on Reference: https://discord.com/channels/267624335836053506/635950537262759947/918455823921995826 --- bot/exts/moderation/modpings.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index d214601de4..c6083387d5 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -17,7 +17,7 @@ log = get_logger(__name__) -MAXIMUM_WORK_LIMIT = 16 +MAXIMUM_WORK_OFF_LIMIT = 16 class ModPings(Cog): @@ -201,15 +201,13 @@ async def on_command(self, ctx: Context) -> None: @has_any_role(*MODERATION_ROLES) async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDuration) -> None: """Schedule modpings role to be added at and removed at everyday at UTC time!""" - print(start, end) - if end < start: end += datetime.timedelta(days=1) - if (end - start) > datetime.timedelta(hours=MAXIMUM_WORK_LIMIT): + if datetime.timedelta(hours=24) - (end - start) > datetime.timedelta(hours=MAXIMUM_WORK_OFF_LIMIT): await ctx.send( - f":x: {ctx.author.mention} You can't have the modpings role for" - f" more than {MAXIMUM_WORK_LIMIT} hours!" + f":x: {ctx.author.mention} You can't have the modpings role off for" + f" more than {MAXIMUM_WORK_OFF_LIMIT} hours!" ) return From 034c6c737bee360a5da665089a43cbe1e5ebf7d6 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 10 Dec 2021 12:39:02 +0530 Subject: [PATCH 03/11] Make the help message more verbose --- bot/exts/moderation/modpings.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index c6083387d5..98ceee1356 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -200,7 +200,24 @@ async def on_command(self, ctx: Context) -> None: ) @has_any_role(*MODERATION_ROLES) async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDuration) -> None: - """Schedule modpings role to be added at and removed at everyday at UTC time!""" + """ + Schedule modpings role to be added at and removed at everyday at UTC time! + + You can have the modpings role off for a maximum of 16 hours i.e. having the modpings role + on for a minimum of 8 hours in a day. + + The command excepts two arguments `start` and `end` which represent hour and minute of a day, + you would get the modpings role at `start` and it would removed from you at `end`. `start` and + `end` can be in the following formats: + - H:Mam/pm (10:14pm) + - HMam/pm (1014pm) + - Ham/pm (10pm) + - H (22 - 24hour format as no meridiem is specified) + - HM (2214 - 24hour format as no meridiem is specified) + + If a moderator has scheduled temporarily removed modpings role and its time for their modpings + schedule start, the off would take higher priority and the modpings role won't be added for them. + """ if end < start: end += datetime.timedelta(days=1) From a0c742e70688645a31bed762fffb033ebe57ce94 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 10 Dec 2021 13:21:59 +0530 Subject: [PATCH 04/11] Don't add modpings role back (after manual off) if schedule over --- bot/exts/moderation/modpings.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index 98ceee1356..a205d91799 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -124,6 +124,15 @@ async def add_role_schedule(self, mod: Member, work_time: int, schedule_start: d async def reapply_role(self, mod: Member) -> None: """Reapply the moderator's role to the given moderator.""" + mod_schedule = self.modpings_schedule.get(mod.id) + if ( + mod_schedule + and datetime.datetime.utcnow() + < datetime.datetime.utcfromtimestamp(mod_schedule.split("|")[0]) + ): + log.trace(f"Skipping re-applying role to mod with ID {mod.id} as their modpings schedule is over.") + return + log.trace(f"Re-applying role to mod with ID {mod.id}.") await mod.add_roles(self.moderators_role, reason="Pings off period expired.") await self.pings_off_mods.delete(mod.id) From 6b8a8744cff9881f357604347be491c18c1ee3df Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 10 Dec 2021 13:55:09 +0530 Subject: [PATCH 05/11] Get member object to properly add modpings role --- bot/exts/moderation/modpings.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index a205d91799..181051acd9 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -12,6 +12,7 @@ from bot.converters import DayDuration, Expiry from bot.log import get_logger from bot.utils import scheduling +from bot.utils.members import get_or_fetch_member from bot.utils.scheduling import Scheduler from bot.utils.time import TimestampFormats, discord_timestamp @@ -85,7 +86,14 @@ async def reschedule_modpings_schedule(self) -> None: start_timestamp, work_time = schedule.split("|") start = datetime.datetime.fromtimestamp(float(start_timestamp)) - mod = await self.bot.fetch_user(mod_id) + guild = self.bot.get_guild(Guild.id) + mod = await get_or_fetch_member(guild, mod_id) + if not mod: + log.info( + f"I tried to get moderator with ID `{mod_id}`, but they don't appear to be on the server :pensive:" + ) + continue + self._modpings_scheduler.schedule_at( start, mod_id, From 8329eea53ddb7b99166fd4afbb6719b07a73d6c1 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Sat, 11 Dec 2021 06:20:45 +0530 Subject: [PATCH 06/11] Fix typos Co-authored-by: Johannes Christ --- bot/converters.py | 6 +++--- bot/exts/moderation/modpings.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 7a5312467a..d8df445341 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -576,12 +576,12 @@ async def convert(self, ctx: Context, arg: str) -> t.Optional[dict]: class DayDuration(Converter): """ - Convert a string representing day time (hours and minutes) to an UTC datetime object. + Convert a string representing day time (hours and minutes) to a UTC datetime object. - The hours and mintues would be combined with UTC day, if no 'am' or 'pm' is passed with + The hours and minutes would be combined with UTC day if no 'am' or 'pm' is passed with the string, then it is assumed that the time is in 24 hour format. - The following formats are excepted: + The following formats are accepted: - H:M - H:M am/pm - H am/pm diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index 181051acd9..c7417192c9 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -90,7 +90,7 @@ async def reschedule_modpings_schedule(self) -> None: mod = await get_or_fetch_member(guild, mod_id) if not mod: log.info( - f"I tried to get moderator with ID `{mod_id}`, but they don't appear to be on the server :pensive:" + f"I tried to get moderator with ID `{mod_id}`, but they don't appear to be on the server 😔" ) continue @@ -218,7 +218,7 @@ async def on_command(self, ctx: Context) -> None: @has_any_role(*MODERATION_ROLES) async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDuration) -> None: """ - Schedule modpings role to be added at and removed at everyday at UTC time! + Schedule modpings role to be added at and removed at every day at UTC! You can have the modpings role off for a maximum of 16 hours i.e. having the modpings role on for a minimum of 8 hours in a day. From 82eca3d65cc492d8c91ccf001c05f55a2acd0197 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Mon, 13 Dec 2021 15:31:30 +0530 Subject: [PATCH 07/11] Make max modpings off check more explicit --- bot/exts/moderation/modpings.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index c7417192c9..1aab6555be 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -223,7 +223,7 @@ async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDura You can have the modpings role off for a maximum of 16 hours i.e. having the modpings role on for a minimum of 8 hours in a day. - The command excepts two arguments `start` and `end` which represent hour and minute of a day, + The command expects two arguments `start` and `end` which represent hour and minute of a day, you would get the modpings role at `start` and it would removed from you at `end`. `start` and `end` can be in the following formats: - H:Mam/pm (10:14pm) @@ -238,7 +238,9 @@ async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDura if end < start: end += datetime.timedelta(days=1) - if datetime.timedelta(hours=24) - (end - start) > datetime.timedelta(hours=MAXIMUM_WORK_OFF_LIMIT): + modpings_on_period = end - start + # Check if the modpings off period for a day is more than the max + if datetime.timedelta(hours=24) - modpings_on_period > datetime.timedelta(hours=MAXIMUM_WORK_OFF_LIMIT): await ctx.send( f":x: {ctx.author.mention} You can't have the modpings role off for" f" more than {MAXIMUM_WORK_OFF_LIMIT} hours!" From b471b16e17a67a8627bcc4e453cc16bdce5d8c77 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Mon, 13 Dec 2021 16:34:48 +0530 Subject: [PATCH 08/11] Add test cases for convertor --- bot/converters.py | 9 +++--- tests/bot/test_converters.py | 60 +++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index d8df445341..05b7d55d97 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -583,8 +583,9 @@ class DayDuration(Converter): The following formats are accepted: - H:M - - H:M am/pm - - H am/pm + - H:Mam/pm + - HMam/pm + - Ham/pm - H where `H` represents Hours and `M` represents Minutes. @@ -598,11 +599,11 @@ class DayDuration(Converter): async def convert(self, _ctx: Context, argument: str) -> datetime: """Attempts to converting `argument` to an UTC datetime object.""" - match = self.TIME_RE.fullmatch(argument).groups() + match = self.TIME_RE.fullmatch(argument) if not match: raise BadArgument(f"`{argument}` is not a valid time duration string.") - hour_12, minute_12, meridiem, hour_24, minute_24 = match + hour_12, minute_12, meridiem, hour_24, minute_24 = match.groups() time = None if hour_12 and meridiem and minute_12: diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 1bb678db24..880f86e280 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -6,7 +6,7 @@ from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument -from bot.converters import Duration, HushDurationConverter, ISODateTime, PackageName +from bot.converters import DayDuration, Duration, HushDurationConverter, ISODateTime, PackageName class ConverterTests(unittest.IsolatedAsyncioTestCase): @@ -252,3 +252,61 @@ async def test_hush_duration_converter_for_invalid(self): with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await converter.convert(self.context, invalid_minutes_string) + + async def test_day_duration_convertor_for_valid(self): + """DayDuration converter returns correct datetime for valid datetime string.""" + test_values = ( + # H:M am/pm + ("2:14 pm", 51240), + ("2:14 am", 8040), + ("2:14Pm", 51240), + ("2:14AM", 8040), + + # HM am/pm + ("942pm", 78120), + ("854 am", 32040), + + # H am/pm + ("11pm", 82800), + ("2 am", 7200), + + # H:M + ("2:14", 8040), + ("23:05", 83100), + ("2305", 83100), + + # H + ("5", 18000), + ("18", 64800), + ) + converter = DayDuration() + for day_duration_string, expected_1970 in test_values: + with self.subTest(day_duration_string=day_duration_string, expected_1970_dt=expected_1970): + converted = await converter.convert(self.context, day_duration_string) + + expected_1970_dt = datetime.utcfromtimestamp(expected_1970) + today = datetime.utcnow().date() + expected_now = expected_1970_dt.replace( + year=today.year, + month=today.month, + day=today.day + ) + + self.assertEqual(expected_now, converted) + + async def test_day_duration_convertor_for_invalid(self): + """DayDuration converter raises the correct exception for invalid datetime strings.""" + test_values = ( + # Check if it fails when providing the date part + '2019-11-12 09:15', + + # Other non-valid strings + 'fisk the tag master', + ) + + converter = DayDuration() + for datetime_string in test_values: + with self.subTest(datetime_string=datetime_string): + exception_message = f"`{datetime_string}` is not a valid time duration string." + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, datetime_string) From 7f8c188ea222504b30b9737b43ac895dce593d6b Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 24 Dec 2021 17:14:25 +0530 Subject: [PATCH 09/11] Apply grammar changes Co-authored-by: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> --- bot/converters.py | 4 ++-- bot/exts/moderation/modpings.py | 14 ++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 05b7d55d97..1e5a87a23c 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -578,7 +578,7 @@ class DayDuration(Converter): """ Convert a string representing day time (hours and minutes) to a UTC datetime object. - The hours and minutes would be combined with UTC day if no 'am' or 'pm' is passed with + The hours and minutes would be combined with UTC day. If no 'am' or 'pm' is passed with the string, then it is assumed that the time is in 24 hour format. The following formats are accepted: @@ -598,7 +598,7 @@ class DayDuration(Converter): ) async def convert(self, _ctx: Context, argument: str) -> datetime: - """Attempts to converting `argument` to an UTC datetime object.""" + """Attempts to convert `argument` to a UTC datetime object.""" match = self.TIME_RE.fullmatch(argument) if not match: raise BadArgument(f"`{argument}` is not a valid time duration string.") diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index 1aab6555be..61b75ae79e 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -218,22 +218,20 @@ async def on_command(self, ctx: Context) -> None: @has_any_role(*MODERATION_ROLES) async def schedule_modpings(self, ctx: Context, start: DayDuration, end: DayDuration) -> None: """ - Schedule modpings role to be added at and removed at every day at UTC! + Schedule modpings role to be added at and removed at every day (UTC time)! - You can have the modpings role off for a maximum of 16 hours i.e. having the modpings role - on for a minimum of 8 hours in a day. + You must have the pingable Moderators role for a minimum of 8 hours a day, + meaning the schedule removing this role has a maximum duration of 16 hours. + The command expects two arguments, `start` and `end`, which are when the role is removed and re-added. - The command expects two arguments `start` and `end` which represent hour and minute of a day, - you would get the modpings role at `start` and it would removed from you at `end`. `start` and - `end` can be in the following formats: + The following formats are accepted for `start` and `end`: - H:Mam/pm (10:14pm) - HMam/pm (1014pm) - Ham/pm (10pm) - H (22 - 24hour format as no meridiem is specified) - HM (2214 - 24hour format as no meridiem is specified) - If a moderator has scheduled temporarily removed modpings role and its time for their modpings - schedule start, the off would take higher priority and the modpings role won't be added for them. + The pingable Moderators role won't be re-added until the scheduled time has finished. """ if end < start: end += datetime.timedelta(days=1) From d0d17242528309b2154357d94cf31bce2021497b Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 24 Dec 2021 17:15:20 +0530 Subject: [PATCH 10/11] Upgrade mod left server log to warning --- bot/exts/moderation/modpings.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index 61b75ae79e..c3f3dc0b15 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -89,9 +89,7 @@ async def reschedule_modpings_schedule(self) -> None: guild = self.bot.get_guild(Guild.id) mod = await get_or_fetch_member(guild, mod_id) if not mod: - log.info( - f"I tried to get moderator with ID `{mod_id}`, but they don't appear to be on the server 😔" - ) + log.warning(f"I tried to get moderator with ID `{mod_id}`, but they don't appear to be on the server 😔") continue self._modpings_scheduler.schedule_at( From 24c1566583fd83b07000106f627cf0abd0f7001e Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 24 Dec 2021 17:16:46 +0530 Subject: [PATCH 11/11] Add typehint for DayDuration convertor --- bot/converters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/converters.py b/bot/converters.py index 1e5a87a23c..bad72ab9c3 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -641,6 +641,7 @@ async def convert(self, _ctx: Context, argument: str) -> datetime: UnambiguousUser = discord.User # noqa: F811 UnambiguousMember = discord.Member # noqa: F811 Infraction = t.Optional[dict] # noqa: F811 + DayDuration = datetime # noqa: F811 Expiry = t.Union[Duration, ISODateTime] MemberOrUser = t.Union[discord.Member, discord.User]