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 Polls #9759

Merged
merged 173 commits into from May 10, 2024
Merged

Add support for Polls #9759

merged 173 commits into from May 10, 2024

Conversation

DA-344
Copy link
Contributor

@DA-344 DA-344 commented Mar 19, 2024

Summary

Adds support for Polls.

Related PRs:

Additions:

  • Poll, PollAnswer and PollMedia objects.
  • PollLayoutType enum
  • on_poll_vote_add/remove events.
  • poll attribute to Message
  • poll kwarg in Messageable.send

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

discord/poll.py Outdated Show resolved Hide resolved
discord/enums.py Outdated Show resolved Hide resolved
@No767
Copy link

No767 commented Mar 19, 2024

Is there documentation or an announcement surrounding this feature?

@DA-344
Copy link
Contributor Author

DA-344 commented Mar 19, 2024

Is there documentation or an announcement surrounding this feature?

Not yet, but it is getting a pop-up saying you to try it, though you cannot send them (you get a 403: Forbidden).

Added the different layout types.

Co-authored-by: owocado <24418520+owocado@users.noreply.github.com>
@Rapptz
Copy link
Owner

Rapptz commented Mar 19, 2024

Thanks for the PR.

Discord has asked that I do not implement this yet and will give me private documentation when the time comes.

@Rapptz Rapptz closed this Mar 19, 2024
@MCausc78
Copy link

MCausc78 commented Mar 21, 2024

@Rapptz Hey, Discord has announced that and there is PR. Can we get that PR reopened?

@Rapptz Rapptz reopened this Mar 22, 2024
@DA-344
Copy link
Contributor Author

DA-344 commented May 2, 2024

What's the current status of this PR? Are we progressing, as I can see the last few comments from @bijij are unresolved?

I am applying the suggested changes, most of them are already implemented and I forgot to add a comment and resolve the conversations, lol sorry.

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.

Please fix the CI failures.

discord/abc.py Outdated Show resolved Hide resolved
discord/ext/commands/context.py Outdated Show resolved Hide resolved
discord/http.py Outdated
Comment on lines 263 to 266
if len(poll) == 0 or len(poll) > 10:
raise ValueError('Poll must contain between 1 and 10 answers')
if poll._hours_duration < 1 or poll._hours_duration > 168:
raise ValueError('Polls duration must be between 1 hour and 7 days')
Copy link
Owner

Choose a reason for hiding this comment

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

Correct, the library delegates this stuff to the server side since it's the source of truth. It only validates when the trade-off for user experience is better (e.g. discord.ui). This is not true for polls to me.

discord/message.py Outdated Show resolved Hide resolved
discord/message.py Show resolved Hide resolved
docs/api.rst Outdated
@@ -1442,6 +1442,71 @@ Voice
:param after: The voice state after the changes.
:type after: :class:`VoiceState`

Polls
Copy link
Owner

@Rapptz Rapptz May 5, 2024

Choose a reason for hiding this comment

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

These events are alphabetically sorted, P comes before V not after. The English alphabet is ABCDEFGHIJKLMNOPQRSTUVWXYZ

docs/api.rst Outdated
Comment on lines 1450 to 1451
Called when a :class:`Message`\'s attached :class:`Poll` gets a new vote. If any of ``user`` or ``message``
are not cached this event will not be called.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Called when a :class:`Message`\'s attached :class:`Poll` gets a new vote. If any of ``user`` or ``message``
are not cached this event will not be called.
Called when a :class:`Message`\'s attached :class:`Poll` gets a new vote. If the ``user`` or ``message``
are not cached then this event will not be called.

docs/api.rst Outdated

.. note::

If the poll allows multiselect and the user votes for more than one answer
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
If the poll allows multiselect and the user votes for more than one answer
If the poll allows multiple answers and the user votes for more than one answer

docs/api.rst Outdated

:param user: The user that voted for the message's poll.
:type user: Union[:class:`User`, :class:`Member`]
:param answer: The answer the user voted to.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
:param answer: The answer the user voted to.
:param answer: The answer the user voted on.

docs/api.rst Outdated
Comment on lines 1467 to 1484
.. function:: on_poll_vote_remove(user, answer)

Called when a :class:`Message`\'s attached :class:`Poll` has lost a vote. If any of ``user`` or ``message``
are not cached this event will not be called.

This requires :attr:`Intents.message_content` and :attr:`Intents.polls` to be enabled.

.. note::

If the poll allows multiselect and the user removes more than one vote
this event will be called as many times as votes removed.

.. versionadded:: 2.4

:param user: The user that removed their vote from the message's poll.
:type user: Union[:class:`User`, :class:`Member`]
:param answer: The answer the user removed the vote from.
:type answer: :class:`PollAnswer`
Copy link
Owner

Choose a reason for hiding this comment

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

You should collapse this with the one above, similar to the other events.

DA-344 added 8 commits May 6, 2024 15:55
Parse str input for emoji into partial emoji
/ PollAnswer.emoji it is just a 'return self.media.emoji'
/ PollMedia now transforms a str into a PartialEmoji, leaving the PollMedia.emoji as Union[PartialEmoji, Emoji]
/ PollAnswer now updates the self_voted attr in _handle_vote
/ (idk if i did in an early commit) Made Poll._answers a mapping of {answer_id: answer} instead of a list
/ ('') Raise ClientException instead of RuntimeError
-- Maybe I am forgetting some changes...
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.

Just one final nit

discord/poll.py Outdated Show resolved Hide resolved
discord/client.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.

Thanks!

@Rapptz Rapptz merged commit e43bd86 into Rapptz:master May 10, 2024
8 checks passed
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.

None yet

9 participants