Implemented optional duration parameter in slowmode command #3331

b0nes1 · 2025-06-02T19:31:24Z

Closes #3327.

Implemented an optional duration argument for the slowmode command. If the duration argument is present, the intended expiration datetime and the original slowmode interval are saved in redis cache. After the duration is elapsed, the original slowmode interval is grabbed from redis and restored to the channel.

The expiration datetime is cached so that it can persist bot restart.

tests/bot/exts/moderation/test_slowmode.py

jb3

One minor comment, though if others disagree then let me know, it is more a nit based on me not wanting to add dependence onto intricacies of Redis and hoping multiple keys exist referring to the same slowmode delay etc.

jb3 · 2025-06-24T21:25:04Z

bot/exts/moderation/slowmode.py

+    # Stores the expiration timestamp in POSIX format for active slowmodes, keyed by channel ID.
+    slowmode_expiration_cache = RedisCache()
+
+    # Stores the original slowmode interval by channel ID, allowing its restoration after temporary slowmode expires.
+    original_slowmode_cache = RedisCache()


Given these are both structured and known values, I would maybe consider serializing them together into one store?

This is a nit though and others may disagree, it just appears that we use them more or less in the same place so whilst if we were just working in dict-land this would be preferred when working in a place with an underlying store I prefer it being the same.

mbaruh · 2025-06-24T22:55:14Z

bot/exts/moderation/slowmode.py

+        if await self.slowmode_expiration_cache.contains(channel.id):
+            expiration_time = await self.slowmode_expiration_cache.get(channel.id)


Suggested change

if await self.slowmode_expiration_cache.contains(channel.id):

expiration_time = await self.slowmode_expiration_cache.get(channel.id)

expiration_time = await self.slowmode_expiration_cache.get(channel.id, None)

if expiration_time is not None:

mbaruh · 2025-06-24T22:57:49Z

bot/exts/moderation/slowmode.py

+            expiration_time = await self.slowmode_expiration_cache.get(channel.id)
+            expiration_timestamp = discord_timestamp(expiration_time, TimestampFormats.RELATIVE)
+            await ctx.send(
+                f"The slowmode delay for {channel.mention} is {humanized_delay} and expires in {expiration_timestamp}."


Suggested change

f"The slowmode delay for {channel.mention} is {humanized_delay} and expires in {expiration_timestamp}."

f"The slowmode delay for {channel.mention} is {humanized_delay} and expires {expiration_timestamp}."

The "in" should be already included in a relative timestamp if I'm not mistaken

Also, should this specify what happens when the slowmode expires? e.g "and will revert to {humanized_original_delay} {expiration_timestamp}"

mbaruh · 2025-06-24T23:03:34Z

bot/exts/moderation/slowmode.py

-            if channel.id in COMMONLY_SLOWMODED_CHANNELS:
-                log.info(f"Recording slowmode change in stats for {channel.name}.")
-                self.bot.stats.gauge(f"slowmode.{COMMONLY_SLOWMODED_CHANNELS[channel.id]}", slowmode_delay)
+        if not slowmode_delay <= SLOWMODE_MAX_DELAY:


Suggested change

if not slowmode_delay <= SLOWMODE_MAX_DELAY:

if slowmode_delay > SLOWMODE_MAX_DELAY:

mbaruh · 2025-06-24T23:07:31Z

bot/exts/moderation/slowmode.py


    @slowmode_group.command(name="set", aliases=["s"])
    async def set_slowmode(
        self,
        ctx: Context,
        channel: MessageHolder,
        delay: DurationDelta | Literal["0s", "0seconds"],
+        duration: DurationDelta | None = None


Use Duration instead of DurationDelta, which handles the conversion to datetime for you (and change the variable name to something like expiry to make it less confusing with delay).

mbaruh · 2025-06-24T23:08:38Z

bot/exts/moderation/slowmode.py

+            slowmode_duration = time.relativedelta_to_timedelta(duration).total_seconds()
+            humanized_duration = time.humanize_delta(duration)
+
+            expiration_time = datetime.now(tz=UTC) + timedelta(seconds=slowmode_duration)


This section is then simplified with the usage of Duration.

mbaruh · 2025-06-24T23:26:32Z

bot/exts/moderation/slowmode.py

-                f"{ctx.author} tried to set the slowmode delay of #{channel} to {humanized_delay}, "
-                "which is not between 0 and 6 hours."
+                f"{ctx.author} set the slowmode delay for #{channel} to"
+                f"{humanized_delay} which expires in {humanized_duration}."


Suggested change

f"{humanized_delay} which expires in {humanized_duration}."

f"{humanized_delay} which expires {humanized_duration}."

Same here, can specify what it's going to revert to.

mbaruh · 2025-06-24T23:26:52Z

bot/exts/moderation/slowmode.py

+            await channel.edit(slowmode_delay=slowmode_delay)
+            await ctx.send(
+            f"{Emojis.check_mark} The slowmode delay for {channel.mention}"
+            f" is now {humanized_delay} and expires in {expiration_timestamp}."


mbaruh · 2025-06-24T23:30:56Z

bot/exts/moderation/slowmode.py

+        await channel.send(
+            f"{Emojis.check_mark} A previously applied slowmode has expired and has been reverted to {slowmode_delay}."
+        )


Slowmode changes are usually silent. Unless the mods agreed that this is what they want I would send this message to #mods and/or #mod-log

Looks like mod-log will already pick up when the slowmode delay is modified and sends it as an embed. Should I also send the above to that channel?

And if i send it to mod-log, should get_or_fetch_channel be used here also? Should the mod-log channel be defined as an instance attribute inside init or is it fine to define it inside of the command provided that get_or_fetch_channel is used?

No if it's automatically picked up then great. I'd just send this to #mods

mbaruh · 2025-06-24T23:36:27Z

bot/exts/moderation/slowmode.py

+    async def _revert_slowmode(self, channel_id: int) -> None:
+        original_slowmode = await self.original_slowmode_cache.get(channel_id)
+        slowmode_delay = time.humanize_delta(seconds=original_slowmode)
+        channel = self.bot.get_channel(channel_id)


I would use get_or_fetch_channel here

https://bot-core.pythondiscord.com/main/output/pydis_core.utils.channel.html#pydis_core.utils.channel.get_or_fetch_channel

mbaruh · 2025-06-24T23:39:11Z

bot/exts/moderation/slowmode.py

+        if channel is None:
+            channel = ctx.channel
+        if await self.slowmode_expiration_cache.contains(channel.id):
+            await self.slowmode_expiration_cache.delete(channel.id)
+            await self.original_slowmode_cache.delete(channel.id)
+            self.scheduler.cancel(channel.id)


Shouldn't it all be already handled in set_slowmode?

…ches into one object; update and tidy related tests.

b0nes1 requested a review from mbaruh as a code owner June 2, 2025 19:31

b0nes1 marked this pull request as draft June 2, 2025 19:33

b0nes1 commented Jun 4, 2025

View reviewed changes

tests/bot/exts/moderation/test_slowmode.py Outdated Show resolved Hide resolved

b0nes1 marked this pull request as ready for review June 15, 2025 01:31

jb3 requested changes Jun 24, 2025

View reviewed changes

mbaruh requested changes Jun 24, 2025

View reviewed changes

b0nes1 requested review from mbaruh and jb3 June 30, 2025 21:51

jb3 approved these changes Jun 30, 2025

View reviewed changes

b0nes1 added 6 commits June 30, 2025 23:16

Implemented optional duration parameter in slowmode command

13975b4

WIP: Added tests for slowmode command with duration argument.

db5ee77

Fix issue with unawaited coroutine in test

dd444fe

Fix bug in test causing channel edit to not be recognised

875c6d2

Refactor slowmode command to use Duration converter; Combine redis ca…

a15262d

…ches into one object; update and tidy related tests.

General refactoring, change message formatting, update tests

a798c38

b0nes1 force-pushed the main branch from 17a83cb to a798c38 Compare June 30, 2025 22:18

b0nes1 added 2 commits July 1, 2025 01:51

Remove stray comment

3af14eb

Use consistent time module import

0591b2c

		if await self.slowmode_expiration_cache.contains(channel.id):
		expiration_time = await self.slowmode_expiration_cache.get(channel.id)

	f"The slowmode delay for {channel.mention} is {humanized_delay} and expires in {expiration_timestamp}."
	f"The slowmode delay for {channel.mention} is {humanized_delay} and expires {expiration_timestamp}."

	if not slowmode_delay <= SLOWMODE_MAX_DELAY:
	if slowmode_delay > SLOWMODE_MAX_DELAY:

	f"{humanized_delay} which expires in {humanized_duration}."
	f"{humanized_delay} which expires {humanized_duration}."

Uh oh!

Implemented optional duration parameter in slowmode command #3331

Are you sure you want to change the base?

Implemented optional duration parameter in slowmode command #3331

Uh oh!

Conversation

b0nes1 commented Jun 2, 2025

Uh oh!

Uh oh!

jb3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbaruh Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbaruh Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mbaruh Jun 24, 2025 •

edited

Loading

mbaruh Jun 24, 2025 •

edited

Loading