-
-
Notifications
You must be signed in to change notification settings - Fork 69
refactor(Moderation): rewrite backend and commands #2595
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2595 +/- ##
==========================================
- Coverage 87.65% 85.58% -2.07%
==========================================
Files 92 98 +6
Lines 3686 3657 -29
Branches 206 212 +6
==========================================
- Hits 3231 3130 -101
- Misses 451 523 +72
Partials 4 4 ☔ View full report in Codecov by Sentry. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Does this also fix #1833? |
As a matter of a fact, yes, it does. Linked the issue now, ty |
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.
Couple of things
Co-authored-by: Jeroen Claassens <[email protected]>
Important
This PR needs extensive testing, as the behaviour of many parts were rewritten due to the Actions layer being the highest level API the entire moderation front-end uses.This PR rewrites the Actions layer in the moderation module, changing the functional style
ModerationActions
class which had a lot of code duplication and almost 1000 lines of code, to simple classes for each action that define what each action does in a comprehensive way.Changelog
timeout
count in/case list overview:True
./case show
and moderation logs./case list
.s!unban
.s!history
for/case list
.s!moderations
for/case list
.s!mutes
for/case list type:Mute
.s!warnings
for/case list type:Warnings
.s!reason
for/case edit reason:<>
.s!time
for/case edit duration:<>
.s!unwarn
for/case archive
.Set Nickname
,Role Add
, andRole Remove
actions not undoing correctly if re-scheduled.es-419
not falling back toen-US
when a key is not available ines-ES
.For years, I received one-off issues with Skyra's moderation module, specially the mute system, because it wasn't giving the roles back. And alongside with the eventual implementation of timeouts, #2594 got way messier than it should have been.
As such, I decided to rewrite the Actions layer. This is the 3rd layer and is the heart of the moderation module, where the actions are applied at and where the database is updated.
(btw, there's a timeout action now, the aforementioned PR will be updated to add the command and the necessary events)
With the new Actions layer, I wanted to reconsider the moderation entry metadata and give them more meaning:
Appeal
becameUndo
, which better conveys the meaning of the type.Fast
was originally intended for fast moderation actions (under X time such as 15 seconds) that wouldn't be logged in the moderation logs channel. However, that's both a maintenance headache and an extremely niche feature nobody asked for. This bit is phased out and deprecated.Invalidated
becameArchived
, which finally clears what it means, and in a backwards compatible way.Because
Archived
, I finally realised that any moderation entry should be archivable, not just warnings, so I removed theunwarn
command and added a new subcommand in thecase
command. This was a bit of a scope creep, but I felt like it was a very needed change. I deprecated the messageRun handlers in the command and slashified it entirely, allowing me to merge 7 commands in a single one.And with the new command, case deletes are now a more obvious posibility, which caught the attention and got more testing. That led to the discovery of new edge cases.
So because of these edge cases, I'm also rewriting the Data layer (4th) and stripping TypeORM entirely from the layer's API using a class compatible with both TypeORM and Prisma. This new layer has stricter types and is more accurate.
A funny tale; some users know that Skyra's unmute was unreliable for years, this is an issue I couldn't figure out and mostly blamed at Discord because it happens sometimes, but not always.
When I rewrote the Actions layer, I found a few issues.
Skyra was looking for the cases as intended, and cancelling the last entry. However...
And then when I found out I had to rewrite the Data layer...
Collection
, which is insert-sorted.As a side note, this PR also backports several changes from #2593.