feat(polls): implement Polls #1176

Snipy7374 · 2024-03-25T10:21:59Z

Summary

Closes implement new Poll discord API feature #1175

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, ...)

disnake/poll.py

Enegg · 2024-04-16T13:14:07Z

disnake/poll.py

@@ -0,0 +1,426 @@
+# SPDX-License-Identifier: MIT


I think it would be better to have two separate objects for what the user can construct vs what the api returns; there's a bunch of fields the user constructible version won't have populated until some magic event, and you made accessing them throw AttributeError

I think you'd agree that physically restricting what the user can do with their object is better than merely mentioning it in the docstring (which seem to often grow large enough a good part of it won't be seen unless scrolled down)

I agree with this. Something similar to what I did in the Onboarding PR could be implemented.

shiftinv

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

disnake/http.py

changelog/1175.feature.rst

disnake/enums.py

disnake/raw_models.py

disnake/state.py

disnake/types/poll.py

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

disnake/http.py

disnake/poll.py

disnake/permissions.py

disnake/flags.py

disnake/poll.py

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Snipy7374 · 2024-04-25T09:29:21Z

Ty for the reviews and sorry if this is taking longer than expected, it's my first API feature so i'm quite inexperienced. I still have the same typing (skill) issue at poll.py/PollAnswer#273, should i just type ignore it? It doesn't seem like our fault.

shiftinv · 2024-04-26T14:07:01Z

Ty for the reviews and sorry if this is taking longer than expected, it's my first API feature so i'm quite inexperienced.

No worries, it's a pretty substantial feature and this is already really good c:
I feel like many of the issues here boil down to the API structure being somewhat weird in the name of extensibility, and avoiding possible breaking changes in the lib while also making things developer-friendly isn't really trivial.

I still have the same typing (skill) issue at poll.py/PollAnswer#273, should i just type ignore it? It doesn't seem like our fault.

Hm, seems that HTTPClient.get_poll_answer_voters should return a {users: List[User]} typeddict?

feat(polls): add poll answer voters iterator

Victorsitou

Looks good so far 👍

Victorsitou · 2024-05-31T14:58:14Z

disnake/enums.py

@@ -1344,6 +1361,11 @@ class EntitlementType(Enum):
 application_subscription = 8


+class PollLayoutType(Enum):
+ default = 1
+ """The default poll layout type."""


This should be documented in the respective .rst file.

This is already documented in disnake.Event page and in the messages API reference section too. Is there somewhere else this should be documented?

Victorsitou · 2024-05-31T15:04:30Z

disnake/message.py

@@ -22,7 +22,7 @@
 overload,
 )

-from . import utils
+from . import poll, utils


This is so insignificant, but the Poll class should be imported instead of doing this

Victorsitou · 2024-05-31T15:10:47Z

disnake/poll.py

@@ -0,0 +1,426 @@
+# SPDX-License-Identifier: MIT


I agree with this. Something similar to what I did in the Onboarding PR could be implemented.

Victorsitou · 2024-05-31T15:21:49Z

disnake/raw_models.py

+ The message ID that got or lost a vote.
+ user_id: :class:`int`
+ The user ID who added the vote or whose vote was removed.
+ member: Optional[:class:`Member`]


Although this is not consistent across all raw models, changing this to cached_member would be more descriptive.

Victorsitou · 2024-05-31T15:33:40Z

disnake/state.py

+ member_id = raw.user_id
+ guild = self._get_guild(raw.guild_id)
+ if guild is not None:
+ member = guild.get_member(member_id)
+ if (message := self._get_message(raw.message_id)) and message.poll:
+ answer = utils.get(message.poll.answers, id=raw.answer_id)
+ else:
+ answer = None
+ if member is not None:
+ raw.member = member
+ else:
+ raw.member = None
+ else:
+ raw.member = None
+ answer = None
+ self.dispatch(f"raw_message_poll_vote_{event_type}", raw)
+
+ if raw.member and answer:
+ self.dispatch(f"message_poll_vote_{event_type}", raw.member, answer)


Suggested change

member_id = raw.user_id

guild = self._get_guild(raw.guild_id)

if guild is not None:

member = guild.get_member(member_id)

if (message := self._get_message(raw.message_id)) and message.poll:

answer = utils.get(message.poll.answers, id=raw.answer_id)

else:

answer = None

if member is not None:

raw.member = member

else:

raw.member = None

else:

raw.member = None

answer = None

self.dispatch(f"raw_message_poll_vote_{event_type}", raw)

if raw.member and answer:

self.dispatch(f"message_poll_vote_{event_type}", raw.member, answer)

guild = self._get_guild(raw.guild_id)

answer = None

if guild is not None:

member = guild.get_member(raw.user_id)

message = self._get_message(raw.message_id)

if message is not None and message.poll is not None:

answer = utils.get(message.poll.answers, id=raw.answer_id)

if member is not None:

raw.member = member

self.dispatch(f"raw_message_poll_vote_{event_type}", raw)

if raw.member is not None and answer is not None:

self.dispatch(f"message_poll_vote_{event_type}", raw.member, answer)

Snipy7374 added 3 commits March 22, 2024 12:55

initial commit

ba43ade

implement polls

bf6d98a

Merge branch 'master' into ft/polls

eb0effb

shiftinv added t: enhancement New feature t: api support Support of Discord API features labels Mar 25, 2024

shiftinv added this to the disnake v2.10 milestone Mar 25, 2024

Victorsitou added the s: waiting for api/docs Issue/PR is waiting for API support/documentation label Mar 25, 2024

Snipy7374 added 2 commits March 25, 2024 17:16

remove print uwu

2ff5d08

remove testing super-properties

0a9078d

Enegg suggested changes Apr 16, 2024

View reviewed changes

shiftinv requested changes Apr 23, 2024

View reviewed changes

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

Update changelog/1175.feature.rst

d3c40cf

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/message.py

8c624c6

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/poll.py

f689810

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/types/poll.py

690622d

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/types/poll.py

975502b

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/raw_models.py

7d9be24

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/raw_models.py

ad60403

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/poll.py

a3397fa

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/poll.py

30dac53

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/poll.py

4b080d7

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Update disnake/poll.py

bf60105

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

implement requested changes

74c1591

Merge branch 'master' into ft/polls

f77a198

add send poll permission

b8c6723

add new polls intents

1007db9

shiftinv reviewed Apr 24, 2024

View reviewed changes

disnake/poll.py Outdated Show resolved Hide resolved

Update disnake/http.py

2a978fe

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

Snipy7374 and others added 4 commits April 25, 2024 10:54

Update disnake/poll.py

dd8e6bc

Co-authored-by: shiftinv <[email protected]> Signed-off-by: Snipy7374 <[email protected]>

apply requested changes

e2db39b

handle all emoji cases

6314220

Merge branch 'master' into ft/polls

2772b67

Snipy7374 added 4 commits April 25, 2024 11:39

solve docs errors

e82e740

fix raw polls events dispatch

0620b81

fix errors in the docs

ca8b0c0

change PollAnswer signature and simplify from_dict classmethod

d9abc9f

remove undefined attributes and fix type-checking errors

99cf23c

Snipy7374 requested review from shiftinv and Enegg April 26, 2024 22:18

shiftinv and others added 3 commits May 10, 2024 14:28

feat: add poll answer voters iterator

250d9fc

Merge pull request from shiftinv/pr/ft/polls

a960d63

feat(polls): add poll answer voters iterator

Merge branch 'master' into ft/polls

0cb7a9d

Victorsitou reviewed May 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(polls): implement Polls #1176

feat(polls): implement Polls #1176

Snipy7374 commented Mar 25, 2024 •

edited

Enegg Apr 16, 2024

Victorsitou May 31, 2024

shiftinv left a comment

Snipy7374 commented Apr 25, 2024

shiftinv commented Apr 26, 2024

Victorsitou left a comment

Victorsitou May 31, 2024

Snipy7374 May 31, 2024

Victorsitou May 31, 2024

Victorsitou May 31, 2024

Victorsitou May 31, 2024

Victorsitou May 31, 2024

feat(polls): implement Polls #1176

Are you sure you want to change the base?

feat(polls): implement Polls #1176

Conversation

Snipy7374 commented Mar 25, 2024 • edited

Summary

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shiftinv left a comment

Choose a reason for hiding this comment

Snipy7374 commented Apr 25, 2024

shiftinv commented Apr 26, 2024

Victorsitou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Snipy7374 commented Mar 25, 2024 •

edited