-
-
Notifications
You must be signed in to change notification settings - Fork 716
Implemented optional duration parameter in slowmode command #3331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
db32927
2ca0248
1570c6e
e159c2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,14 +1,18 @@ | ||||||||||
from datetime import UTC, datetime, timedelta | ||||||||||
from typing import Literal | ||||||||||
|
||||||||||
from async_rediscache import RedisCache | ||||||||||
from dateutil.relativedelta import relativedelta | ||||||||||
from discord import TextChannel, Thread | ||||||||||
from discord.ext.commands import Cog, Context, group, has_any_role | ||||||||||
from pydis_core.utils.scheduling import Scheduler | ||||||||||
|
||||||||||
from bot.bot import Bot | ||||||||||
from bot.constants import Channels, Emojis, MODERATION_ROLES | ||||||||||
from bot.converters import DurationDelta | ||||||||||
from bot.log import get_logger | ||||||||||
from bot.utils import time | ||||||||||
from bot.utils.time import TimestampFormats, discord_timestamp | ||||||||||
|
||||||||||
log = get_logger(__name__) | ||||||||||
|
||||||||||
|
@@ -26,8 +30,15 @@ | |||||||||
class Slowmode(Cog): | ||||||||||
"""Commands for getting and setting slowmode delays of text channels.""" | ||||||||||
|
||||||||||
# 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() | ||||||||||
|
||||||||||
def __init__(self, bot: Bot) -> None: | ||||||||||
self.bot = bot | ||||||||||
self.scheduler = Scheduler(self.__class__.__name__) | ||||||||||
|
||||||||||
@group(name="slowmode", aliases=["sm"], invoke_without_command=True) | ||||||||||
async def slowmode_group(self, ctx: Context) -> None: | ||||||||||
|
@@ -42,17 +53,29 @@ async def get_slowmode(self, ctx: Context, channel: MessageHolder) -> None: | |||||||||
channel = ctx.channel | ||||||||||
|
||||||||||
humanized_delay = time.humanize_delta(seconds=channel.slowmode_delay) | ||||||||||
|
||||||||||
await ctx.send(f"The slowmode delay for {channel.mention} is {humanized_delay}.") | ||||||||||
if await self.slowmode_expiration_cache.contains(channel.id): | ||||||||||
expiration_time = await self.slowmode_expiration_cache.get(channel.id) | ||||||||||
Comment on lines
+56
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
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}." | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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}" |
||||||||||
) | ||||||||||
else: | ||||||||||
await ctx.send(f"The slowmode delay for {channel.mention} is {humanized_delay}.") | ||||||||||
|
||||||||||
@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 | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||||||||||
) -> None: | ||||||||||
"""Set the slowmode delay for a text channel.""" | ||||||||||
""" | ||||||||||
Set the slowmode delay for a text channel. | ||||||||||
|
||||||||||
Supports temporary slowmodes with the `duration` argument that automatically | ||||||||||
revert to the original delay after expiration. | ||||||||||
""" | ||||||||||
# Use the channel this command was invoked in if one was not given | ||||||||||
if channel is None: | ||||||||||
channel = ctx.channel | ||||||||||
|
@@ -66,37 +89,98 @@ async def set_slowmode( | |||||||||
humanized_delay = time.humanize_delta(delay) | ||||||||||
|
||||||||||
# Ensure the delay is within discord's limits | ||||||||||
if slowmode_delay <= SLOWMODE_MAX_DELAY: | ||||||||||
log.info(f"{ctx.author} set the slowmode delay for #{channel} to {humanized_delay}.") | ||||||||||
|
||||||||||
await channel.edit(slowmode_delay=slowmode_delay) | ||||||||||
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: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
log.info( | ||||||||||
f"{ctx.author} tried to set the slowmode delay of #{channel} to {humanized_delay}, " | ||||||||||
"which is not between 0 and 6 hours." | ||||||||||
) | ||||||||||
|
||||||||||
await ctx.send( | ||||||||||
f"{Emojis.check_mark} The slowmode delay for {channel.mention} is now {humanized_delay}." | ||||||||||
f"{Emojis.cross_mark} The slowmode delay must be between 0 and 6 hours." | ||||||||||
) | ||||||||||
return | ||||||||||
|
||||||||||
else: | ||||||||||
if duration is not None: | ||||||||||
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) | ||||||||||
Comment on lines
+104
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section is then simplified with the usage of |
||||||||||
expiration_timestamp = discord_timestamp(expiration_time, TimestampFormats.RELATIVE) | ||||||||||
|
||||||||||
# Only update original_slowmode_cache if the last slowmode was not temporary. | ||||||||||
if not await self.slowmode_expiration_cache.contains(channel.id): | ||||||||||
await self.original_slowmode_cache.set(channel.id, channel.slowmode_delay) | ||||||||||
await self.slowmode_expiration_cache.set(channel.id, expiration_time.timestamp()) | ||||||||||
|
||||||||||
self.scheduler.schedule_at(expiration_time, channel.id, self._revert_slowmode(channel.id)) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The task ID has to be unique. If you're scheduling with a channel ID that already has a schedule, this creates a clash and the new coroutine won't be registered. You're already checking whether the current slowmode is scheduled to expire just above, can cancel the current task while you're at it (although this logic will probably change if you're changing to a single cache). |
||||||||||
log.info( | ||||||||||
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}." | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Same here, can specify what it's going to revert to. |
||||||||||
) | ||||||||||
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}." | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here |
||||||||||
) | ||||||||||
else: | ||||||||||
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) | ||||||||||
|
||||||||||
log.info(f"{ctx.author} set the slowmode delay for #{channel} to {humanized_delay}.") | ||||||||||
await channel.edit(slowmode_delay=slowmode_delay) | ||||||||||
await ctx.send( | ||||||||||
f"{Emojis.cross_mark} The slowmode delay must be between 0 and 6 hours." | ||||||||||
f"{Emojis.check_mark} The slowmode delay for {channel.mention} is now {humanized_delay}." | ||||||||||
) | ||||||||||
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) | ||||||||||
|
||||||||||
async def _reschedule(self) -> None: | ||||||||||
log.trace("Rescheduling the expiration of temporary slowmodes from cache.") | ||||||||||
for channel_id, expiration in await self.slowmode_expiration_cache.items(): | ||||||||||
expiration_datetime = datetime.fromtimestamp(expiration, tz=UTC) | ||||||||||
channel = self.bot.get_channel(channel_id) | ||||||||||
log.info(f"Rescheduling slowmode expiration for #{channel} ({channel_id}).") | ||||||||||
self.scheduler.schedule_at(expiration_datetime, channel_id, self._revert_slowmode(channel_id)) | ||||||||||
|
||||||||||
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) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use |
||||||||||
log.info(f"Slowmode in #{channel} ({channel.id}) has expired and has reverted to {slowmode_delay}.") | ||||||||||
await channel.edit(slowmode_delay=original_slowmode) | ||||||||||
await channel.send( | ||||||||||
f"{Emojis.check_mark} A previously applied slowmode has expired and has been reverted to {slowmode_delay}." | ||||||||||
) | ||||||||||
Comment on lines
+154
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||||||||||
await self.slowmode_expiration_cache.delete(channel.id) | ||||||||||
await self.original_slowmode_cache.delete(channel.id) | ||||||||||
|
||||||||||
@slowmode_group.command(name="reset", aliases=["r"]) | ||||||||||
async def reset_slowmode(self, ctx: Context, channel: MessageHolder) -> None: | ||||||||||
"""Reset the slowmode delay for a text channel to 0 seconds.""" | ||||||||||
await self.set_slowmode(ctx, channel, relativedelta(seconds=0)) | ||||||||||
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) | ||||||||||
Comment on lines
+164
to
+169
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't it all be already handled in |
||||||||||
|
||||||||||
async def cog_check(self, ctx: Context) -> bool: | ||||||||||
"""Only allow moderators to invoke the commands in this cog.""" | ||||||||||
return await has_any_role(*MODERATION_ROLES).predicate(ctx) | ||||||||||
|
||||||||||
async def cog_load(self) -> None: | ||||||||||
"""Wait for guild to become available and reschedule slowmodes which should expire.""" | ||||||||||
await self.bot.wait_until_guild_available() | ||||||||||
await self._reschedule() | ||||||||||
|
||||||||||
async def cog_unload(self) -> None: | ||||||||||
"""Cancel all scheduled tasks.""" | ||||||||||
self.scheduler.cancel_all() | ||||||||||
|
||||||||||
|
||||||||||
async def setup(bot: Bot) -> None: | ||||||||||
"""Load the Slowmode cog.""" | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.