Skip to content
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

feat: Update AutoMod implementation #1809

Merged

Conversation

TheEnigmaBlade
Copy link
Contributor

Summary

Since the original implementation of the AutoMod API (during the AutoMod beta), the API has been repeatedly changed. This pull request changes the following:

  • Adds the trigger type (rule_trigger_type) to AutoModActionExecutionEvent, which seems to have been forgotten.
  • Adds newer properties to AutoMod trigger metadata (ex. regex_patterns, allow_list).
  • Adds the mention_spam trigger type enum value.
  • Removes the harmful_link enum value from AutoModTriggerType, as Discord merged the filter with spam early in the beta.
  • Updates the documentation to include all AutoMod classes and enums, along with the updates above.

I apologize about the improper commit messages, as I read the guidelines 3 months too late.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.

@TheEnigmaBlade TheEnigmaBlade requested a review from a team as a code owner November 29, 2022 05:43
@TheEnigmaBlade TheEnigmaBlade changed the title Update AutoMod implementation feat: Update AutoMod implementation Nov 29, 2022
discord/automod.py Outdated Show resolved Hide resolved
discord/automod.py Outdated Show resolved Hide resolved
discord/enums.py Outdated Show resolved Hide resolved
discord/types/automod.py Outdated Show resolved Hide resolved
Copy link
Member

@BobDotCom BobDotCom left a comment

Choose a reason for hiding this comment

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

In addition to the other comments, please add a changelog entry for all of the changes here

discord/automod.py Show resolved Hide resolved
discord/automod.py Show resolved Hide resolved
discord/automod.py Show resolved Hide resolved
discord/raw_models.py Show resolved Hide resolved
@TheEnigmaBlade
Copy link
Contributor Author

All requested changes have been applied.

discord/automod.py Show resolved Hide resolved
docs/api/enums.rst Show resolved Hide resolved
@plun1331
Copy link
Member

  • Removes the harmful_link enum value from AutoModTriggerType, as Discord merged the filter with spam early in the beta.

This change doesn't seem to have been made, and instead of removing it we could instead deprecate it and alias it for spam

@BobDotCom
Copy link
Member

I apologize about the improper commit messages, as I read the guidelines 3 months too late.

FWIW, your commit messages are fine. We use squash merges for PRs, only the PR title matters.

…oModTriggerMetadata.mention_total_limit` to add supported trigger type.
@TheEnigmaBlade
Copy link
Contributor Author

  • Removes the harmful_link enum value from AutoModTriggerType, as Discord merged the filter with spam early in the beta.

This change doesn't seem to have been made, and instead of removing it we could instead deprecate it and alias it for spam

The removal was reverted by @Lulalaby a little earlier, which is fine. Deprecation without aliasing would probably be best to avoid breaking changes, as aliasing harmful_link to spam could cause existing usages to double-process the same spam event (ex. actioning a user twice; logging twice). That could be considered a breaking change in my eyes.

I added the property to the docs for AllowModTriggerType (since it didn't exist before) with a deprecated message. Is that fine?

@BobDotCom
Copy link
Member

BobDotCom commented Nov 30, 2022

The removal was reverted by @Lulalaby a little earlier, which is fine. Deprecation without aliasing would probably be best to avoid breaking changes, as aliasing harmful_link to spam could cause existing usages to double-process the same spam event (ex. actioning a user twice; logging twice). That could be considered a breaking change in my eyes.

I added the property to the docs for AllowModTriggerType (since it didn't exist before) with a deprecated message. Is that fine?

I actually think it should be removed, considering it's not on the api docs https://discord.com/developers/docs/resources/auto-moderation#auto-moderation-rule-object-trigger-types.

@Lulalaby @plun1331 opinions?

@Lulalaby
Copy link
Member

It's still existing, some bots might encounter it, that's why I reverted the removal again

@TheEnigmaBlade
Copy link
Contributor Author

According to the Discord staff I've talked to previously, the harmful link filter is permanently removed in favor of the spam filter. It only existed during the early beta tests and has not been public, so it would be impossible for a bot to ever receive such an event.

@plun1331 plun1331 linked an issue Dec 5, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Dec 5, 2022

Codecov Report

Merging #1809 (906cd9f) into master (b5f4b91) will decrease coverage by 0.02%.
The diff coverage is 15.78%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1809      +/-   ##
==========================================
- Coverage   33.21%   33.18%   -0.03%     
==========================================
  Files          96       96              
  Lines       18657    18674      +17     
==========================================
+ Hits         6196     6197       +1     
- Misses      12461    12477      +16     
Flag Coverage Δ
macos-latest-3.10 33.16% <15.78%> (-0.03%) ⬇️
macos-latest-3.11 33.16% <15.78%> (-0.03%) ⬇️
macos-latest-3.8 33.18% <15.78%> (-0.03%) ⬇️
macos-latest-3.9 33.18% <15.78%> (-0.03%) ⬇️
ubuntu-latest-3.10 33.16% <15.78%> (-0.03%) ⬇️
ubuntu-latest-3.11 33.16% <15.78%> (-0.03%) ⬇️
ubuntu-latest-3.8 33.18% <15.78%> (-0.03%) ⬇️
ubuntu-latest-3.9 33.18% <15.78%> (-0.03%) ⬇️
windows-latest-3.10 33.16% <15.78%> (-0.03%) ⬇️
windows-latest-3.11 33.16% <15.78%> (-0.03%) ⬇️
windows-latest-3.8 33.18% <15.78%> (-0.03%) ⬇️
windows-latest-3.9 33.18% <15.78%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
discord/automod.py 28.83% <6.25%> (-2.93%) ⬇️
discord/raw_models.py 28.10% <50.00%> (-0.19%) ⬇️
discord/enums.py 78.75% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5f4b91...906cd9f. Read the comment docs.

@Lulalaby Lulalaby enabled auto-merge (squash) December 7, 2022 01:47
@Lulalaby Lulalaby merged commit a960c19 into Pycord-Development:master Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for automod incomplete
5 participants