Support editing of timeout durations. #2839

shtlrs · 2023-12-09T15:36:46Z

All of the infractions that we have are scheduled for expiry thanks to our own Scheduler.

However, when it comes to timeouts/mutes, the expiry is handled by Discord itself, and the duration that we set in our databse serves no purposes besides their notifying the user about re-acquiring the ability to send messages again.

So this particular case needs to be handled on its own whenver the infraction type is a timeout and a new expiry datetime has been assigned.

vivekashok1221

Few wrinkles to iron out but code works otherwise.

bot/exts/moderation/infraction/management.py

vivekashok1221 · 2023-12-25T16:32:13Z

bot/exts/moderation/infraction/management.py

+                if user and infraction["type"] == "timeout":
+                    await user.edit(reason=reason, timed_out_until=expiry)


If the user is not present in the server, the infraction duration is updated in the database and a confirmation message is sent- but the actual timeout duration is not changed Discord-side (basically the same undesirable behaviour we have for editing timeout duration right now) .

We should either:
a) notify that the user has left the server
b) apply the new timeout duration when the user rejoins the server (might have to bring back some of logic we had for on_member_join event from back when we used @muted role for muting users)

I'm assuming it's not possible to change timeout duration for users while they are not present in the server.

vivekashok1221 · 2024-03-17T19:57:56Z

bot/exts/moderation/infraction/infractions.py

+        if active_timeouts and not member.is_timed_out():
+            reason = f"Applying active timeout for returning member: {active_timeouts[0]['id']}"
+
+        async def action() -> None:
+            await member.edit(timed_out_until=arrow.get(active_timeouts[0]["expires_at"]).datetime, reason=reason)


I get the following error when my test account that I muted rejoins the server (while the timeout is still active) because member.is_timed_out() is True and hence reason is not assigned a value:

bot-1 | File "/bot/bot/exts/moderation/infraction/infractions.py", line 664, in action bot-1 | await member.edit(timed_out_until=arrow.get(active_timeouts[0]["expires_at"]).datetime, reason=reason) bot-1 | ^^^^^^ bot-1 | NameError: cannot access free variable 'reason' where it is not associated with a value in enclosing scope

vivekashok1221

Sorry for the late review, I was teaching my houseplants Morse code. Please forgive.

vivekashok1221 · 2024-03-17T20:09:53Z

bot/exts/moderation/infraction/infractions.py

+            reason = f"Applying active timeout for returning member: {active_timeouts[0]['id']}"
+
+        async def action() -> None:
+            await member.edit(timed_out_until=arrow.get(active_timeouts[0]["expires_at"]).datetime, reason=reason)


This line triggers an IndexError whenever a user without an active timeout infraction joins the server.

Good find, this and the NameError were fixed in the force push

vivekashok1221 · 2024-03-18T10:34:38Z

bot/exts/moderation/infraction/infractions.py

+        if active_timeouts and not member.is_timed_out():
+            reason = f"Applying active timeout for returning member: {active_timeouts[0]['id']}"
+
+            async def action() -> None:
+                await member.edit(timed_out_until=arrow.get(active_timeouts[0]["expires_at"]).datetime, reason=reason)
+            await self.reapply_infraction(active_timeouts[0], action)


Doesn't this overlook users whose timeout duration was edited while they were not present on the server but are still under a timeout when they rejoin the server? Once they rejoin the server, they will still be under the old timeout duration and not the new edited duration.

How does this look ?

vivekashok1221 · 2024-03-19T11:05:58Z

bot/exts/moderation/infraction/infractions.py

+            timeout_infraction = active_timeouts[0]
+            expiry = arrow.get(timeout_infraction["expires_at"], tzinfo=UTC).datetime
+            if member.is_timed_out():
+                expiry = max(expiry, member.timed_out_until)


Shouldn't we always use the expiry from the database?
With your current approach, if a moderator reduces the timeout duration, the infraction log will display the new duration. However, when the user rejoins the server, the old (original) longer timeout duration will be applied.

I'm not on a streak this period of the year...

You're right, I'll fix that.

vivekashok1221 · 2024-03-19T11:22:13Z

I've noticed something that could warrant its own issue. When attempting to unmute a user who left the server, the bot will reply that the it has failed to pardon the infraction since the user has left the server. However, it will also mark the infraction as inactive in the db. This means that once the user rejoins, someone will have to manually remove the timeout.

shtlrs · 2024-03-19T23:28:20Z

I've noticed something that could warrant its own issue. When attempting to unmute a user who left the server, the bot will reply that the it has failed to pardon the infraction since the user has left the server. However, it will also mark the infraction as inactive in the db. This means that once the user rejoins, someone will have to manually remove the timeout.

Yes, i think this should be dealt with in a separate issue.

vivekashok1221

LGTM 🌟 🥇

I do have a minor comment but I'm fine with merging this PR as it is.

bot/exts/moderation/infraction/infractions.py

vivekashok1221

LGTM 🌟 🥇

MarkKoz

Just some nits

MarkKoz · 2024-03-25T01:28:22Z

bot/exts/moderation/infraction/_utils.py

+
+
+def cap_timeout_duration(duration: datetime.datetime | relativedelta) -> tuple[bool, datetime.datetime]:
+    """Caps the duration of a duration to Discord's limit."""


Suggested change

"""Caps the duration of a duration to Discord's limit."""

"""Cap a duration to Discord's limit."""

MarkKoz · 2024-03-25T01:28:45Z

bot/exts/moderation/infraction/_utils.py

+
+
+async def notify_timeout_cap(bot: Bot, ctx: Context, user: discord.Member) -> None:
+    """Notifies moderators about a timeout duration being capped."""


Imperative mood is our prevailing style.

Suggested change

"""Notifies moderators about a timeout duration being capped."""

"""Notify moderators about a timeout duration being capped."""

MarkKoz · 2024-03-25T01:34:58Z

bot/exts/moderation/infraction/infractions.py

+    @commands.Cog.listener()
+    async def on_member_join(self, member: Member) -> None:
+        """
+


Extra newline here

Suggested change

if a user isn't a member of the server, trying to edit their timeout will result in an error

If timeout was edited to a longer duration, members could evade being timeout upon rejoining, so we reapply them if that's the case

shtlrs requested review from mbaruh, Den4200, ks129 and jb3 as code owners December 9, 2023 15:36

ichard26 added a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 2 - normal Normal Priority s: needs review Author is waiting for someone to review and approve t: bug Something isn't working labels Dec 9, 2023

ichard26 requested review from ChrisLovering, vivekashok1221 and ichard26 and removed request for jb3, ks129 and Den4200 December 25, 2023 05:06

vivekashok1221 requested changes Dec 25, 2023

View reviewed changes

wookie184 added s: waiting for author Waiting for author to address a review or respond to a comment and removed s: needs review Author is waiting for someone to review and approve labels Jan 28, 2024

shtlrs force-pushed the allow-mute-edits branch from 6e919f8 to 7d9bde1 Compare February 17, 2024 09:59

shtlrs requested a review from vivekashok1221 February 17, 2024 10:07

vivekashok1221 requested changes Mar 17, 2024

View reviewed changes

shtlrs force-pushed the allow-mute-edits branch from 67b8c8a to 36ee066 Compare March 18, 2024 09:23

shtlrs requested review from vivekashok1221 and removed request for ChrisLovering, ichard26 and vivekashok1221 March 18, 2024 09:25

vivekashok1221 requested changes Mar 18, 2024

View reviewed changes

shtlrs force-pushed the allow-mute-edits branch from 36ee066 to 8e1ff85 Compare March 18, 2024 12:35

shtlrs requested a review from vivekashok1221 March 18, 2024 12:40

vivekashok1221 requested changes Mar 19, 2024

View reviewed changes

shtlrs force-pushed the allow-mute-edits branch 2 times, most recently from 0c480ec to eba7740 Compare March 19, 2024 23:26

shtlrs requested a review from vivekashok1221 March 19, 2024 23:27

shtlrs closed this Mar 19, 2024

shtlrs reopened this Mar 19, 2024

shtlrs mentioned this pull request Mar 19, 2024

Automatically apply pardoned timeouts upon rejoining the server #2963

Open

vivekashok1221 approved these changes Mar 20, 2024

View reviewed changes

bot/exts/moderation/infraction/infractions.py Show resolved Hide resolved

shtlrs removed the s: waiting for author Waiting for author to address a review or respond to a comment label Mar 20, 2024

shtlrs force-pushed the allow-mute-edits branch from eba7740 to 129705a Compare March 21, 2024 13:58

vivekashok1221 approved these changes Mar 22, 2024

View reviewed changes

MarkKoz approved these changes Mar 25, 2024

View reviewed changes

shtlrs added 4 commits March 29, 2024 23:38

allow timeout edits

0254200

do not edit discord timeout for users who left the server

a57cbef

if a user isn't a member of the server, trying to edit their timeout will result in an error

cap timeout duration upon edit

f494b23

apply timeouts on members who left the server

4072c88

If timeout was edited to a longer duration, members could evade being timeout upon rejoining, so we reapply them if that's the case

shtlrs force-pushed the allow-mute-edits branch from 129705a to 3fd41e1 Compare March 29, 2024 22:45

notify mods about timeout cap upon edit

f319bf8

shtlrs force-pushed the allow-mute-edits branch from 3fd41e1 to f319bf8 Compare March 29, 2024 22:48

shtlrs merged commit 972fd18 into main Mar 29, 2024
5 checks passed

shtlrs deleted the allow-mute-edits branch March 29, 2024 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support editing of timeout durations. #2839

Support editing of timeout durations. #2839

shtlrs commented Dec 9, 2023

vivekashok1221 left a comment

vivekashok1221 Dec 25, 2023

vivekashok1221 Dec 25, 2023

shtlrs Feb 17, 2024

vivekashok1221 Mar 17, 2024

vivekashok1221 left a comment

vivekashok1221 Mar 17, 2024

shtlrs Mar 18, 2024 •

edited

Loading

vivekashok1221 Mar 18, 2024

shtlrs Mar 18, 2024 •

edited

Loading

vivekashok1221 Mar 19, 2024

shtlrs Mar 19, 2024

vivekashok1221 commented Mar 19, 2024 •

edited

Loading

shtlrs commented Mar 19, 2024

vivekashok1221 left a comment

vivekashok1221 left a comment

MarkKoz left a comment

MarkKoz Mar 25, 2024

MarkKoz Mar 25, 2024

MarkKoz Mar 25, 2024

		if user and infraction["type"] == "timeout":
		await user.edit(reason=reason, timed_out_until=expiry)



		def cap_timeout_duration(duration: datetime.datetime \| relativedelta) -> tuple[bool, datetime.datetime]:
		"""Caps the duration of a duration to Discord's limit."""

	"""Caps the duration of a duration to Discord's limit."""
	"""Cap a duration to Discord's limit."""



		async def notify_timeout_cap(bot: Bot, ctx: Context, user: discord.Member) -> None:
		"""Notifies moderators about a timeout duration being capped."""

	"""Notifies moderators about a timeout duration being capped."""
	"""Notify moderators about a timeout duration being capped."""

Support editing of timeout durations. #2839

Support editing of timeout durations. #2839

Conversation

shtlrs commented Dec 9, 2023

vivekashok1221 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vivekashok1221 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shtlrs Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shtlrs Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vivekashok1221 commented Mar 19, 2024 • edited Loading

shtlrs commented Mar 19, 2024

vivekashok1221 left a comment

Choose a reason for hiding this comment

vivekashok1221 left a comment

Choose a reason for hiding this comment

LGTM 🌟 🥇

MarkKoz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shtlrs Mar 18, 2024 •

edited

Loading

shtlrs Mar 18, 2024 •

edited

Loading

vivekashok1221 commented Mar 19, 2024 •

edited

Loading