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(polls): implement Polls #1176

Merged
merged 100 commits into from
Aug 24, 2024
Merged

feat(polls): implement Polls #1176

merged 100 commits into from
Aug 24, 2024

Conversation

Snipy7374
Copy link
Contributor

@Snipy7374 Snipy7374 commented Mar 25, 2024

Summary

Notes:

  • Emojis in PollMedia doesn't work yet... idk if this is an issue on my code or on the API
  • I still haven't tested sending Poll(s) on interaction responses and on Webhook(s) because that still returns 403 (Forbidden)
  • I followed the discord api docs to name things, but if you have suggestions they are welcome, for example i feel like the new events names may be too long
  • In the future the question field on Poll will be able to accept a PollMedia too, and PollMedia will likely supports new things
  • Messages that contain a Poll can't be edited after sending (yet), tho in the future this shouldn't require much code changes
  • Rn sending messages that contains Poll(s) can't have any other field set, this will not require any code change in the future since i'm not doing any check

Doubts:
I feel like i'm passing way too many arguments for the new events? What's your opinion? Solved

  • Should we check for the number of answers the user is passing to construct the Poll? The maximum allowed is 10 answers per poll

I have a typing (skill) issue at poll.py/PollAnswer#245 what am i supposed to do there? Solved

Should we make answers and polls be EQ comparable in some way? (we could use the internal message object, if set, to check if two polls are equals) Doesn't make sense on second thought

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • 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, ...)

@shiftinv shiftinv added t: enhancement New feature t: api support Support of Discord API features labels Mar 25, 2024
@shiftinv shiftinv added this to the disnake v2.10 milestone Mar 25, 2024
@Victorsitou Victorsitou added the s: waiting for api/docs Issue/PR is waiting for API support/documentation label Mar 25, 2024
disnake/poll.py Outdated Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
disnake/poll.py Show resolved Hide resolved
Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

First off, thanks for implementing this, and sorry the review took so long! A bunch of other things unfortunately got in the way.

Many of these comments are simple documentation adjustments and can be committed as-is, some other parts are a bit more complex (as usual, coming up with a good developer interface is tricky).
There's still a bunch of bikeshedding to be done as you mentioned, so i've left out comments about a couple things that only make sense to talk about once everything else is taken care of, for now.

That said, a new permission and poll intents have been added to the api since, would be great to have them implemented here too.

disnake/abc.py Outdated Show resolved Hide resolved
disnake/http.py Outdated Show resolved Hide resolved
changelog/1175.feature.rst Outdated Show resolved Hide resolved
disnake/enums.py Outdated Show resolved Hide resolved
disnake/enums.py Outdated Show resolved Hide resolved
disnake/raw_models.py Show resolved Hide resolved
disnake/raw_models.py Outdated Show resolved Hide resolved
disnake/state.py Outdated Show resolved Hide resolved
disnake/types/poll.py Show resolved Hide resolved
disnake/types/poll.py Outdated Show resolved Hide resolved
@shiftinv shiftinv removed the s: waiting for api/docs Issue/PR is waiting for API support/documentation label Apr 23, 2024
Snipy7374 and others added 15 commits April 23, 2024 23:07
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
disnake/http.py Outdated Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
disnake/permissions.py Show resolved Hide resolved
disnake/flags.py Show resolved Hide resolved
disnake/flags.py Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
disnake/poll.py Outdated Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
disnake/flags.py Show resolved Hide resolved
disnake/flags.py Outdated Show resolved Hide resolved
disnake/flags.py Outdated Show resolved Hide resolved
disnake/flags.py Show resolved Hide resolved
disnake/poll.py Outdated Show resolved Hide resolved
Snipy7374 and others added 6 commits August 21, 2024 18:34
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
@Snipy7374
Copy link
Contributor Author

disnake/poll.py Outdated Show resolved Hide resolved
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
Snipy7374 and others added 3 commits August 23, 2024 18:09
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: Snipy7374 <100313469+Snipy7374@users.noreply.github.com>
disnake/state.py Outdated Show resolved Hide resolved
Signed-off-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
@shiftinv shiftinv enabled auto-merge (squash) August 24, 2024 11:45
Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

thank you for all the work on this feature 🙏

@shiftinv shiftinv merged commit d4972ab into DisnakeDev:master Aug 24, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: api support Support of Discord API features t: enhancement New feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

implement new Poll discord API feature
4 participants