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 an option for polls to allow users to vote for multiple items #69

Merged
merged 11 commits into from
Jun 6, 2023

Conversation

dsevillamartin
Copy link
Member

@dsevillamartin dsevillamartin commented Jun 4, 2023

Fixes #3

Changes proposed in this pull request:

  • Add option for allowing multiple option votes from users
  • Add option for max votes allowed per user if the poll allows multiple option votes
  • Reorganize help section display to always have icons and show multiple instances, plus max votes
  • Creates new API endpoint due to the inability to adapt the existing one without breaking changes
    • Same with Flarum and Pusher events

Reviewers should focus on:

  • The performance of the new endpoint is probably not great in larger scale, due to the constant refreshing of vote counts. This could be changed to increment & decrement (preferably moved to a queue as well, though then users would see the wrong count and polls with a few votes would look wrong). At least, only the options that were changed (added/removed) are refreshed.

Screenshot
image
image

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).

dsevillamartin and others added 3 commits June 4, 2023 17:45
This creates a new API endpoint due to backwards compatibility with handling 'unvoting' from a single-vote poll vs multiple.
Also fixes (I believe) Pusher not working (the event name was mistyped?)
@dsevillamartin dsevillamartin marked this pull request as ready for review June 4, 2023 21:53
@dsevillamartin
Copy link
Member Author

New changes make the poll look like this. I know a 'max votes' option could probably be standalone without a switch for multiple votes, but that switch might be better UX? Unsure. I don't think it's a big concern regardless.

image
image

@dsevillamartin
Copy link
Member Author

dsevillamartin commented Jun 5, 2023

I've improved the performance of the new endpoint.
These metrics are with the poll screenshoted above - two options. The # of selects goes up like 1 per additional option after this improvement, before it went up by 2 per option.

# OF QUERIES Voting Unvoting Changing Vote
Before PR (legacy endpoint) 21 19 24
Before optimization 21 (~14 ms) 19 (~12 ms) 24 (~15 ms )
After optimization 19 (~8 ms) 17 (~6 ms) 21 (~9 ms)

This performance seems to be better than the legacy endpoint.

@dsevillamartin dsevillamartin merged commit fb2ccf9 into master Jun 6, 2023
@dsevillamartin dsevillamartin deleted the ds/3-select-multiple-options branch July 6, 2023 14:08
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.

Select one or multiple options per poll
3 participants