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

[Modlog API] Fundamental change to modlog API #2897

Closed
1 of 3 tasks
DiscordLiz opened this issue Jul 21, 2019 · 3 comments · Fixed by #2901
Closed
1 of 3 tasks

[Modlog API] Fundamental change to modlog API #2897

DiscordLiz opened this issue Jul 21, 2019 · 3 comments · Fixed by #2901
Assignees
Labels
Breaking Change Status: Accepted

Comments

@DiscordLiz
Copy link
Contributor

@DiscordLiz DiscordLiz commented Jul 21, 2019

Feature request

Select the type of feature you are requesting:

  • Cog
  • Command
  • API functionality

Describe your requested feature

The modlog API should not auto-detect events caused by the bot.

  • This would allow the simplification of interacting with the modlog API.
  • This would make interacting with the modlog API easier, but required for certain actions.
  • This would not remove auto-detecting bans made by moderators through the discord interface rather than the bot

A practical case and rationale

While working on #2866, I noticed there is no clean way to create a case type for multiple audit log entries. In wanting to allow role mutes to be supplemented with permission overwrites as needed, I would need a case type which supported multiple audit log actions.

Rather than change the modlog API to support this, removing the action type all together and just not detecting actions the bot took allows cleaner ease of use.

Downsides

This does mean that someone can't do a ban with a reason and have an audit log entry automatically generated, they would need to create a case.

Mitigating the impact of downsides

If we provide a few pre-registered case types which cover generic use already registered and support their use, the downside becomes one extra import and one extra line per mod action.

Breakage

This would remove a key from the dicts used in case registration.
This would be a change in expected behavior if cogs are not updated for this.

@mikeshardmind mikeshardmind added Breaking Change Status: Needs Discussion Status: Accepted and removed Status: Needs Discussion labels Jul 21, 2019
@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Jul 21, 2019
@mikeshardmind
Copy link
Contributor

@mikeshardmind mikeshardmind commented Jul 21, 2019

So long as this doesn't add too much work for 3rd party cog creators, these changes can be made.

Should I take the practical example and that it would help the other PR you are working on as being willing to work on this, or leave it unassigned for anyone interested to pick up?

@DiscordLiz
Copy link
Contributor Author

@DiscordLiz DiscordLiz commented Jul 21, 2019

I'm willing to make the changes.

@mikeshardmind
Copy link
Contributor

@mikeshardmind mikeshardmind commented Jul 22, 2019

Just for clarity, the parsing in mod with get_audit_log_info + the event listener there for automatic case generation without a (discord) audit log entry is why this key is currently being used in core red. any modification around this should address that.

@mikeshardmind mikeshardmind removed this from the 3.2.0 milestone Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Status: Accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants