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 #2408

Merged
merged 133 commits into from
Jun 15, 2024
Merged

feat: Polls #2408

merged 133 commits into from
Jun 15, 2024

Conversation

NeloBlivion
Copy link
Member

@NeloBlivion NeloBlivion commented Mar 21, 2024

Summary

Implements polls and related features. WIP while the original PR becomes more stable over the experimental phase.
References:

Notes:

  • While the API for PollMedia denotes both text and emoji as optional, text must be non-null for both questions and answers; as such, it's required here.
  • For Poll and PollAnswer, PollMedia is abstracted to its attributes (e.g. Poll.PollMedia.text is assigned to Poll.question). An undocumented attribute _media still remains. After reconsidering, this wasn't great. For __init__s you can just pass the strings, but otherwise you'd access through Poll.question.text, for example.
  • Will work on a new AsyncIterator for answer.users Complete

Information

  • 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, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

discord/abc.py Outdated Show resolved Hide resolved
discord/enums.py Outdated Show resolved Hide resolved
discord/raw_models.py Outdated Show resolved Hide resolved
discord/state.py Show resolved Hide resolved
@plun1331 plun1331 added priority: medium Medium Priority status: in progress Work in Progess feature Implements a feature upcoming discord feature Involves a feature not yet fully released by Discord changelog needed labels Mar 22, 2024
@plun1331 plun1331 added this to the v2.6 milestone Mar 22, 2024
@jab416171
Copy link
Contributor

  • doesn't work with ctx.respond(poll=poll)
  • getting this error with this code:
@commands.slash_command(name="poll")
async def poll(self, ctx, question: Option(str), option1: Option(str), option2: Option(str)):
    """Start a poll"""
    poll = Poll(question=question, answers=[option1, option2], duration=300, allow_multiselect=False)
    await ctx.send(poll=poll)
exception in command poll:
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 131, in wrapped
    ret = await coro(arg)
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 1009, in _invoke
    await self.callback(self.cog, ctx, **kwargs)
  File "/mybot/cogs/mycog.py", line 483, in poll
    await ctx.send(poll=poll)
  File "/usr/local/lib/python3.8/site-packages/discord/abc.py", line 1609, in send
    poll = poll.to_dict()
  File "/usr/local/lib/python3.8/site-packages/discord/poll.py", line 304, in to_dict
    "question": self.question.to_dict(),
AttributeError: 'str' object has no attribute 'to_dict'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/discord/bot.py", line 1130, in invoke_application_command
    await ctx.command.invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 376, in invoke
    await injected(ctx)
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 139, in wrapped
    raise ApplicationCommandInvokeError(exc) from exc
discord.errors.ApplicationCommandInvokeError: Application Command raised an exception: AttributeError: 'str' object has no attribute 'to_dict'
Ignoring exception in on_application_command_error
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 131, in wrapped
    ret = await coro(arg)
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 1009, in _invoke
    await self.callback(self.cog, ctx, **kwargs)
  File "/mybot/cogs/mycog.py", line 483, in poll
    await ctx.send(poll=poll)
  File "/usr/local/lib/python3.8/site-packages/discord/abc.py", line 1609, in send
    poll = poll.to_dict()
  File "/usr/local/lib/python3.8/site-packages/discord/poll.py", line 304, in to_dict
    "question": self.question.to_dict(),
AttributeError: 'str' object has no attribute 'to_dict'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/discord/client.py", line 400, in _run_event
    await coro(*args, **kwargs)
  File "/mybot/cogs/help.py", line 81, in on_application_command_error
    raise error
  File "/usr/local/lib/python3.8/site-packages/discord/bot.py", line 1130, in invoke_application_command
    await ctx.command.invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 376, in invoke
    await injected(ctx)
  File "/usr/local/lib/python3.8/site-packages/discord/commands/core.py", line 139, in wrapped
    raise ApplicationCommandInvokeError(exc) from exc
discord.errors.ApplicationCommandInvokeError: Application Command raised an exception: AttributeError: 'str' object has no attribute 'to_dict'```

@NeloBlivion
Copy link
Member Author

NeloBlivion commented Mar 22, 2024

  • doesn't work with ctx.respond(poll=poll)
  • getting this error with this code:

these are fixed in the latest version. Note that question should be a full PollMedia object (e.g. PollMedia("my question")), but STC

@jab416171
Copy link
Contributor

I still have both of these errors on 9423a8a, and I did change it to use PollMedia

@NeloBlivion
Copy link
Member Author

I still have both of these errors on 9423a8a, and I did change it to use PollMedia

Try now? Also if this back and forth goes longer, may be better to tag me on discord

discord/message.py Outdated Show resolved Hide resolved
@plun1331
Copy link
Member

@NeloBlivion
Copy link
Member Author

NeloBlivion commented Apr 18, 2024

As the main PR is merged, this is effectively ready

  • poll can be sent with content and embed(s), but not file(s) due to clashes with upcoming poll features that are undocumented (unless i should implement these early?)
  • poll cannot be edited after being sent, but content and embeds on the same message can.
  • New Intents.polls and Permissions.send_polls
  • While polls rely on message_content intent, they are cached separately through Client.polls and Client.get_poll. Polls will be cached through the following:
    • Message sends
    • ANY message edit
    • Any form of message fetch - e.g. channel.fetch_message for individual messages, or channel.history will get several polls
  • on_message_edit will fire when a poll ends (naturally, or Message.end_poll(), or poll.end() AND the votes have been finalized.
  • Intents.polls is a default intent that is required for tracking poll votes through on_poll_vote_add and on_poll_vote_remove, both using the args poll, user, answer

Now for the creation of polls, I wasn't entirely sure on the best approach. question only supports text, so it can be initialized as follows:

  • Poll(some_question)
  • Poll(question=PollMedia(some_question))

But answers are more complicated, due to supporting text, emoji and soon attachment. So you can either pass answers as a list of PollAnswer, or use Poll.add_answer chaining like you would with embeds:

  • Poll(question=some_question, answers=[PollAnswer(some_answer), PollAnswer(another_answer, emoji=...)])
  • Poll(some_question).add_answer(some_answer)

Due to the extensiveness of the polls API, I feel this is one of those instances where it's beneficial to add extra convenience for the user instead of sticking to it religiously.
The other args are completely optional, using the defaults present on Discord's UI when creating polls (allow_multiselect=False, duration=24, no other layout_type yet.)
Due to results being nullable for some reason at random, a few conveniences have been added:

  • poll.results.is_finalized -> poll.has_ended()
  • poll.results.total_votes() -> poll.total_votes()
  • Answer counts are typically stored in poll.results.answer_counts, but each PollAnswer has the count attribute; returning None if results is also None, 0 if the answer isn't in answer_counts, otherwise the relevant count.

If you are able to, please stress test this PR and see if the internally cached vote counts are able to keep up with realtime counts on Discord, especially when bouncing between 1-0 votes on specific answers; this part involves manual deletion and creation of PollAnswerCount objects, so it may be possible for something to go wrong that I couldn't catch in solo tests.

@Lulalaby Lulalaby self-requested a review April 18, 2024 21:50
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

Changelog

Copy link
Member

@JustaSqu1d JustaSqu1d left a comment

Choose a reason for hiding this comment

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

documentation suggestions

  • fix minor mistakes
  • more formal writing
  • formatting

docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
discord/poll.py Outdated Show resolved Hide resolved
discord/poll.py Outdated Show resolved Hide resolved
discord/poll.py Show resolved Hide resolved
discord/poll.py Outdated Show resolved Hide resolved
discord/raw_models.py Outdated Show resolved Hide resolved
Copy link
Member

@JustaSqu1d JustaSqu1d left a comment

Choose a reason for hiding this comment

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

The docs follows standard US English conventions.

AMERICA!! 🦅

discord/poll.py Outdated Show resolved Hide resolved
discord/poll.py Outdated Show resolved Hide resolved
NeloBlivion and others added 3 commits May 26, 2024 23:44
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: UK <41271523+NeloBlivion@users.noreply.github.com>
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: UK <41271523+NeloBlivion@users.noreply.github.com>
@Dorukyum
Copy link
Member

Will test personally, but your testing would still be appreciated (you haven't checked the box in the checklist).

@Icebluewolf
Copy link
Contributor

I have thoroughly tested the PR as of 1 month ago. All of the core functionality works. It does not look like any major changes to functionality were made recently.

Copy link
Member

@JustaSqu1d JustaSqu1d left a comment

Choose a reason for hiding this comment

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

minor grammar/spelling

docs/api/enums.rst Outdated Show resolved Hide resolved
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: Lala Sabathil <aiko@aitsys.dev>
@JustaSqu1d JustaSqu1d added status: awaiting review Awaiting review from a maintainer and removed status: in progress Work in Progess changelog needed upcoming discord feature Involves a feature not yet fully released by Discord labels Jun 15, 2024
discord/state.py Outdated Show resolved Hide resolved
Signed-off-by: Dorukyum <53639936+Dorukyum@users.noreply.github.com>
@Lulalaby Lulalaby merged commit cd9ca50 into Pycord-Development:master Jun 15, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Implements a feature priority: medium Medium Priority status: awaiting review Awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants