Add Modpings cog and new Moderators role#1526
Conversation
Added a cog to allow moderators to go off and on duty. The off-duty state is cached via a redis cache, and its expiry is scheduled via the Scheduler. Additionally changes which roles are pinged on mod alerts.
mbaruh
added
a: moderation
|
|
||
| # RedisCache[str, str] | ||
| # The cache's keys are mods who are off-duty. | ||
| # The cache's values are the times when the role should be re-applied to them, stored in ISO format. |
There was a problem hiding this comment.
I remember Mark said somewhere else that using Unix timestamp is better.
There was a problem hiding this comment.
Is there a specific reason? in terms of functionality it seems to work just fine.
There was a problem hiding this comment.
A previous discussion on that is here, although I feel it's fine either way.
There was a problem hiding this comment.
Yeah, I would prefer timestamps over formatted strings (now for consistency too since it's a more established pattern) but it's not a dealbreaker.
Co-authored-by: ks129 <45097959+ks129@users.noreply.github.com>
HassanAbouelela
left a comment
HassanAbouelela
left a comment
There was a problem hiding this comment.
Looks like a solid PR. I just had a couple questions, but otherwise this is good to go.
MarkKoz
left a comment
MarkKoz
left a comment
There was a problem hiding this comment.
Aren't there also everyone pings for filters and antispam? Do those also need to be changed to be like the modlog ping?
| mod = ctx.author | ||
|
|
||
| until_date = duration.replace(microsecond=0).isoformat() | ||
| await mod.remove_roles(self.moderators_role, reason=f"Entered off-duty period until {until_date}.") |
There was a problem hiding this comment.
What happens if the user doesn't have the role (i.e. they're already off-duty)? Will this raise an exception or fail silently?
There was a problem hiding this comment.
From my tests it fails silently
| if mod.id in self._role_scheduler: | ||
| self._role_scheduler.cancel(mod.id) |
There was a problem hiding this comment.
Is this done to allow a user to extend their off-duty period while they're already off-duty?
There was a problem hiding this comment.
Yes, or not necessarily extend it, but just change it without having to cancel it first.
A moderator is expected to have the mod-team role and therefore it's enough to specify the latter in the mod and staff roles.
The lack of such a task may be indicative of a bug.
|
@MarkKoz from what I see and my tests, the filters and antispam use the modlog method for the alert, so this change should handle everything. |
Kinda defeats the purpose of being off-duty.
The renaming includes the commands inside it.
…into mbaruh/offduty
mbaruh
changed the title
ChrisLovering
left a comment
ChrisLovering
left a comment
There was a problem hiding this comment.
Minor comment which might help readability, but looks good to me otherwise!
Co-authored-by: ChrisJL <ChrisLovering@users.noreply.github.com>
The modlog alert embed no longer pings everyone.
8 participants
Added a cog to allow moderators to turn pings off and back on.
The off state is cached via a redis cache, and its expiry is scheduled via the Scheduler.
Additionally changes which roles are pinged on mod alerts to the new role.