-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Don't allow modifying already started polls #4904
Merged
Merged
+1,284
−262
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
javierm
force-pushed
the
poll_restrictions
branch
from
September 5, 2022 21:48
9c5e940
to
bfe2997
Compare
javierm
changed the title
Poll restrictions
Don't allow modifying already started polls
Sep 14, 2022
javierm
force-pushed
the
poll_restrictions
branch
from
September 20, 2022 10:54
bfe2997
to
60a86da
Compare
spec/components/admin/poll/questions/answers/documents/table_actions_component_spec.rb
Show resolved
Hide resolved
document = create(:document, documentable: current_answer) | ||
delete :destroy, params: { id: document } | ||
|
||
expect(flash[:alert]).to eq "You do not have permission to carry out the action 'destroy' on Document." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [111/110] (https://rubystyle.guide#max-line-length)
javierm
force-pushed
the
poll_restrictions
branch
from
September 20, 2022 11:25
60a86da
to
bcd4970
Compare
The extra check to see the voter count has increased was redundant; we already check the request has finished inside the `vote_for_poll_via_web` method and we check all three voters are created in the results table. Updating the poll so it's in the past after starting the browser might result in database inconsistencies while running the tests, so we're using `travel_to` instead.
Instead of having to add `beginning_of_minute` to deal with an issue with Capybara filling datetime fields as mentioned in commit 5a0fde4, we can travel to the beginning of the minute so we don't have to take the seconds into account.
javierm
force-pushed
the
poll_restrictions
branch
5 times, most recently
from
September 20, 2022 13:55
8210bcc
to
ce33d4c
Compare
We need to update a couple of tests because a poll is created in the tests with a timestamp that includes nanoseconds and in the form to edit the time of the poll the nanoseconds are not sent, meaning it was detected as a change.
javierm
force-pushed
the
poll_restrictions
branch
from
September 20, 2022 15:22
ce33d4c
to
d7af5a2
Compare
In this form, the only case where `poll` might be present without `question.poll` being present to is going to be the `new` action. We can assign the poll in the `new` action and get rid of the `poll` variable in the form.
javierm
force-pushed
the
poll_restrictions
branch
2 times, most recently
from
September 20, 2022 15:48
d26b0f9
to
834c8c7
Compare
Adding, modifiying, and/or deleting questions for an already started poll is far away from being democratic and can lead to unwanted side effects like missing votes in the results or stats. So, from now on, only modifiying questions will be possible only if the poll has not started yet.
Just like we did with questions.
Deleting answers was not even possible. But it was possible to delete questions. So we implemented the same behavior.
Same rules that will apply for the answer itself should apply for the attachments like videos, images, and/or documents.
Note that the `create` action doesn't create an image but updates an answer instead. We're removing the references to `:create` in the abilities since it isn't used. In the future we might change the form to add an image to an answer because it's been broken for ages since it shows all the attached images.
We were using the same logic in four different places, so we're creating a new class to handle that logic. Note that I didn't find a way to delegate the `content` method to a `Admin::TableActionsComponent`, so we're delegating the `action` method instead. That means we need to create a method returning an `Admin::TableActionsComponent`. We also need to cache this object; otherwise we were getting an error when calling `actions.action` from the `Admin::Poll::Questions::TableActionsComponent`.
We were rendering the `new` action, but that action doesn't exist. Before commit ec861ca, we were rendering the `edit` action of an answer, which was confusing as well. Note that, when adding an invalid document, `@answer.documents` contains that invalid document (which is not present in the database). Since we're rendering the index, this new document would appear in the list of the documents that can be deleted; to avoid that, we're kind of "reloading" the answer object in the component by finding the record in the database. We aren't using `@answer.reload` because doing so would remove the validation errors.
javierm
force-pushed
the
poll_restrictions
branch
from
September 20, 2022 15:55
834c8c7
to
24099e8
Compare
taitus
approved these changes
Sep 20, 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
Note
This pull request has been extracted from #4859. All the texts and images present here were added by @decabeza
Objectives
Currently, administrators can add, modify, and delete questions for an already started poll. This behavior doesn't seem democratic and can lead to unwanted side effects like missing votes in the results or stats.
This PR adds new poll restrictions.
All the polls will have to be created on future dates.
Also, for an already started and/or ended poll, it will not be possible to:
Also, this PR includes the possibility to delete answers if the poll has not started yet, since this functionality was not implemented yet.
Visual Changes
All polls should begin at a future date.
Start date cannot be changed if voting has already started.
End date cannot be changed to a non-future date if voting has already started.
End date cannot be changed if voting has ended.
Added edit and delete actions to the question answers table. In addition, the heading "Valid answers" has been deleted in the table as it does not provide information. All answers are valid. There are no invalid answers.
Warning messages about edit polls content when has started