Skip to content

Conversation

kyranet
Copy link
Member

@kyranet kyranet commented Mar 15, 2024

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

  • Added generic error handling for chat input commands.
  • Added timeout count in /case list overview:True.
  • Improved the style in /case show and moderation logs.
  • Improved the style in /case list.
  • Improved action status checking.
  • Improved ban check in s!unban.
  • Deprecated s!history for /case list.
  • Deprecated s!moderations for /case list.
  • Deprecated s!mutes for /case list type:Mute.
  • Deprecated s!warnings for /case list type:Warnings.
  • Deprecated s!reason for /case edit reason:<>.
  • Deprecated s!time for /case edit duration:<>.
  • Deprecated s!unwarn for /case archive.
  • Fixed unmutes not giving the roles back.
  • Fixed moderation actions not cancelling pending schedules correctly.
  • Fixed Set Nickname, Role Add, and Role Remove actions not undoing correctly if re-scheduled.
  • Fixed moderation logs not being localized.
  • Fixed es-419 not falling back to en-US when a key is not available in es-ES.
  • Fixed the message for upper ranges not being inclusive.

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 became Undo, 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 became Archived, 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 the unwarn command and added a new subcommand in the case 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...

  • 🚩 it was looking for non-temporary actions, so temporary mutes would be ignored.

And then when I found out I had to rewrite the Data layer...

  • 🚩 "last" doesn't mean "most recent", not with a Collection, which is insert-sorted.

As a side note, this PR also backports several changes from #2593.

@kyranet kyranet requested a review from favna as a code owner March 15, 2024 16:32
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

Attention: Patch coverage is 71.79487% with 77 lines in your changes are missing coverage. Please review.

Project coverage is 85.58%. Comparing base (eb11ea2) to head (b0321a5).
Report is 5 commits behind head on main.

Files Patch % Lines
src/lib/util/resolvers/TimeSpan.ts 0.00% 41 Missing ⚠️
src/lib/util/resolvers/Case.ts 0.00% 27 Missing ⚠️
src/lib/i18n/translate.ts 50.00% 5 Missing ⚠️
src/lib/util/resolvers/index.ts 0.00% 2 Missing ⚠️
src/lib/util/util.ts 80.00% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@kyranet kyranet marked this pull request as draft March 16, 2024 11:51
@kyranet

This comment was marked as resolved.

@kyranet kyranet changed the title refactor(Moderation): rewrite actions layer refactor(Moderation): rewrite backend Mar 17, 2024
@kyranet kyranet linked an issue Mar 17, 2024 that may be closed by this pull request
@kyranet kyranet marked this pull request as ready for review March 18, 2024 15:51
@kyranet

This comment was marked as resolved.

@kyranet kyranet marked this pull request as draft March 18, 2024 18:21
@kyranet kyranet changed the title refactor(Moderation): rewrite backend refactor(Moderation): rewrite backend and commands Mar 19, 2024
@kyranet kyranet marked this pull request as ready for review March 26, 2024 20:19
@favna
Copy link
Member

favna commented Mar 27, 2024

Does this also fix #1833?

@kyranet kyranet linked an issue Mar 27, 2024 that may be closed by this pull request
@kyranet
Copy link
Member Author

kyranet commented Mar 27, 2024

As a matter of a fact, yes, it does. Linked the issue now, ty

@favna favna assigned favna and unassigned favna Mar 27, 2024
Copy link
Member

@favna favna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of things

kyranet and others added 2 commits March 27, 2024 21:25
@kyranet kyranet requested a review from favna March 27, 2024 21:30
@kyranet kyranet merged commit a1f06d0 into main Mar 28, 2024
@kyranet kyranet deleted the refactor/moderation/rewrite-actions-layer branch March 28, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

request: streamline viewing moderation logs for a specific user rewrite: moderation module improvements
2 participants