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

Add support for guild incidents #9590

Merged
merged 15 commits into from
Jan 27, 2024

Conversation

Soheab
Copy link
Contributor

@Soheab Soheab commented Sep 30, 2023

Summary

This PR adds support for the incidents_data field on guilds.

Relevant API Docs:

This was discussed in discord.py >💬Guild incident data

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • 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, ...)

@Soheab Soheab marked this pull request as draft September 30, 2023 20:39
@Soheab Soheab marked this pull request as ready for review October 3, 2023 15:40
@Soheab Soheab changed the title [WIP] Add support for getting and editing guild incidents Add support for getting and editing guild incidents Oct 3, 2023
@Soheab Soheab changed the title Add support for getting and editing guild incidents Add support for guild incidents Oct 3, 2023
@Rapptz Rapptz added the pending PR implements an in-progress or explicitly unreleased Discord feature label Oct 5, 2023
Copy link
Owner

@Rapptz Rapptz left a comment

Choose a reason for hiding this comment

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

With this API, there's no way to edit the incident data from a clean slate. What I mean is, if e.g. Guild.incidents_data is None because it's unset then there's no way to add new incident data.

I'm honestly unsure how I feel about the attribute as a whole, it does mirror the API as-is so it's not particularly controversial but it doesn't feel very ergonomic.

It would be nice if there was e.g. Guild.invites_paused() -> bool and Guild.dms_paused() -> bool helper functions. I don't know if the are_ prefix is good for these, it feels awkward.

@Soheab
Copy link
Contributor Author

Soheab commented Oct 5, 2023

With this API, there's no way to edit the incident data from a clean slate. What I mean is, if e.g. Guild.incidents_data is None because it's unset then there's no way to add new incident data.

Looks like I forgot to change the type but it does always return an instance of IncidentData to use .edit on.

I'm honestly unsure how I feel about the attribute as a whole, it does mirror the API as-is so it's not particularly controversial but it doesn't feel very ergonomic.

Not sure either. Maybe the two fields should just be on Guild.edit and have those helper methods mentioned above? Or a method called edit_security_actions(...) -> Guild or something like that (edit_incidents, ...).

@Soheab Soheab marked this pull request as ready for review November 21, 2023 11:57
Copy link
Owner

@Rapptz Rapptz left a comment

Choose a reason for hiding this comment

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

This implementation is much better.

discord/guild.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
Copy link
Owner

@Rapptz Rapptz left a comment

Choose a reason for hiding this comment

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

This looks good, thanks.

@Rapptz Rapptz merged commit 851c857 into Rapptz:master Jan 27, 2024
8 checks passed
@Rapptz Rapptz removed the pending PR implements an in-progress or explicitly unreleased Discord feature label Jan 27, 2024
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.

2 participants