Add poll questions that accept multiple answers per user #5012
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Senen
force-pushed
the
multiple_answers
branch
2 times, most recently
from
6cbe3c2
to
e5dc702
Compare
taitus
approved these changes
Oct 14, 2022
javierm
reviewed
Oct 17, 2022
Senen
force-pushed
the
refactor_public_polls
branch
from
82e36ed
to
dde5dd5
Compare
Senen
force-pushed
the
multiple_answers
branch
2 times, most recently
from
0700282
to
fce3acb
Compare
|
Hi @javierm, thanks for the review. I think I applied all the suggestions 👌🏼 . The only thing I kept the same is the text mentioned in your comment: #discussion_r996903531. Not sure what to do about it 🤔 |
javierm
reviewed
Oct 17, 2022
| <p> | ||
| <strong><%= t("admin.polls.votation_type.title") %></strong> | ||
| <br> | ||
| <%= VotationType.human_attribute_name("votation_type/vote_type.#{@question.vote_type}") %> |
There was a problem hiding this comment.
I think we can simplify this line a bit:
Suggested change
| <%= VotationType.human_attribute_name("votation_type/vote_type.#{@question.vote_type}") %> | |
| <%= VotationType.human_attribute_name("vote_type.#{@question.vote_type}") %> |
| @@ -10,16 +10,18 @@ def already_answered?(answer) | |||
| user_answers.find_by(answer: answer.title).present? | |||
There was a problem hiding this comment.
Maybe we can now reuse the new method 🤔.
Suggested change
| user_answers.find_by(answer: answer.title).present? | |
| user_answer(answer).present? |
| method: :delete, | ||
| remote: true, | ||
| title: t("poll_questions.show.voted", answer: question_answer.title), | ||
| class: "button answered expand", |
There was a problem hiding this comment.
Does the expand class do anything? 🤔 Since it isn't present in the button to remove the question, I guess it's a typo.
Suggested change
| class: "button answered expand", | |
| class: "button answered", |
In this context, `this` already points to the element, so there is no need to search it again.
In case we receive consecutive requests we are locking the poll author record until the first request transaction ends, so the author answers count during subsequent requests validations is up to date.
Senen
force-pushed
the
refactor_public_polls
branch
from
dde5dd5
to
8311e7e
Compare
Senen
force-pushed
the
multiple_answers
branch
from
fce3acb
to
45eabf1
Compare
Now we can remove answers we should provide a way of removing voting.
The `reload` method added to max_votes validation is needed because the author gets here with some changes because of the around_action `switch_locale`, which adds some changes to the current user record and therefore, the lock method raises an exception when trying to lock it requiring us to save or discard those record changes.
Senen
force-pushed
the
multiple_answers
branch
from
45eabf1
to
1eb52fb
Compare
javierm
approved these changes
Oct 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
This PR includes the multiple questions part of the original PR:
This PR depends on:
Objectives
Add a new type of question where the users can select multiple answers.
Visual Changes
Check original PR screenshots related to multiple answers.
The only difference for questions with multiple answers is that now buttons are disabled when the user reaches the maximum of answers allowed.