consul/grecia
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use checkboxes and radio buttons on poll forms

Browse Source 
Our original interface to vote in a poll had a few issues:

* Since there was no button to send the form, it wasn't clear that
  selecting an option would automatically store it in the database.
* The interface was almost identical for single-choice questions and
  multiple-choice questions, which made it hard to know which type of
  question we were answering.
* Adding other type of questions, like open answers, was hard since we
  would have to add a different submit button for each answer.

So we're now using radio buttons for single-choice questions and
checkboxes for multiple-choice questions, which are the native controls
designed for these purposes, and a button to send the whole form.

Since we don't have a database table for poll ballots like we have for
budget ballots, we're adding a new `Poll::WebVote` model to manage poll
ballots. We're using WebVote instead of Ballot or Vote because they
could be mistaken with other vote classes.

Note that browsers don't allow removing answers with radio buttons, so
once somebody has voted in a single-choice question, they can't remove
the vote unless they manually edit their HTML. This is the same behavior
we had before commit 7df0e9a96.

As mentioned in c2010f975, we're now adding the `ChangeByZero` rubocop
rule, since we've removed the test that used `and change`.
This commit is contained in:
Javi Martín
2024-05-16 02:43:55 +02:00
parent fd14c55615
commit a7e1b42b6c
35 changed files with 612 additions and 687 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
spec/models/abilities/common_spec.rb
View File

@@ -51,18 +51,6 @@ describe Abilities::Common do
let(:poll_from_own_geozone) { create(:poll, geozone_restricted_to: [geozone]) }
let(:poll_from_other_geozone) { create(:poll, geozone_restricted_to: [create(:geozone)]) }
let(:poll_question_from_own_geozone) { create(:poll_question, poll: poll_from_own_geozone) }
let(:poll_question_from_other_geozone) { create(:poll_question, poll: poll_from_other_geozone) }
let(:poll_question_from_all_geozones) { create(:poll_question, poll: poll) }
let(:expired_poll_question_from_own_geozone) do
create(:poll_question, poll: expired_poll_from_own_geozone)
end
let(:expired_poll_question_from_other_geozone) do
create(:poll_question, poll: expired_poll_from_other_geozone)
end
let(:expired_poll_question_from_all_geozones) { create(:poll_question, poll: expired_poll) }
let(:own_proposal_document) { build(:document, documentable: own_proposal) }
let(:proposal_document) { build(:document, documentable: proposal) }
let(:own_budget_investment_document) { build(:document, documentable: own_investment_in_accepting_budget) }
@@ -252,39 +240,25 @@ describe Abilities::Common do
end
describe "Poll" do
it { should be_able_to(:answer, current_poll) }
it { should_not be_able_to(:answer, expired_poll) }
it { should be_able_to(:answer, current_poll) }
it { should_not be_able_to(:answer, expired_poll) }
it { should be_able_to(:answer, poll_question_from_own_geozone) }
it { should be_able_to(:answer, poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, poll_question_from_other_geozone) }
it { should be_able_to(:answer, poll_from_own_geozone) }
it { should be_able_to(:answer, poll) }
it { should_not be_able_to(:answer, poll_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, expired_poll_question_from_other_geozone) }
context "Poll::Answer" do
let(:own_answer) { create(:poll_answer, author: user) }
let(:other_user_answer) { create(:poll_answer) }
let(:expired_poll) { create(:poll, :expired) }
let(:question) { create(:poll_question, :yes_no, poll: expired_poll) }
let(:expired_poll_answer) { create(:poll_answer, author: user, question: question, answer: "Yes") }
it { should be_able_to(:destroy, own_answer) }
it { should_not be_able_to(:destroy, other_user_answer) }
it { should_not be_able_to(:destroy, expired_poll_answer) }
end
it { should_not be_able_to(:answer, expired_poll_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_from_other_geozone) }
context "without geozone" do
before { user.geozone = nil }
it { should_not be_able_to(:answer, poll_question_from_own_geozone) }
it { should be_able_to(:answer, poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, poll_question_from_other_geozone) }
it { should_not be_able_to(:answer, poll_from_own_geozone) }
it { should be_able_to(:answer, poll) }
it { should_not be_able_to(:answer, poll_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, expired_poll_question_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_from_other_geozone) }
end
end
@@ -339,26 +313,25 @@ describe Abilities::Common do
it { should be_able_to(:show, own_direct_message) }
it { should_not be_able_to(:show, create(:direct_message)) }
it { should be_able_to(:answer, current_poll) }
it { should_not be_able_to(:answer, expired_poll) }
it { should be_able_to(:answer, current_poll) }
it { should_not be_able_to(:answer, expired_poll) }
it { should be_able_to(:answer, poll_question_from_own_geozone) }
it { should be_able_to(:answer, poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, poll_question_from_other_geozone) }
it { should be_able_to(:answer, poll_from_own_geozone) }
it { should be_able_to(:answer, poll) }
it { should_not be_able_to(:answer, poll_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, expired_poll_question_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_from_other_geozone) }
context "without geozone" do
before { user.geozone = nil }
it { should_not be_able_to(:answer, poll_question_from_own_geozone) }
it { should be_able_to(:answer, poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, poll_question_from_other_geozone) }
it { should_not be_able_to(:answer, poll_from_own_geozone) }
it { should be_able_to(:answer, poll) }
it { should_not be_able_to(:answer, poll_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll_question_from_all_geozones) }
it { should_not be_able_to(:answer, expired_poll_question_from_other_geozone) }
it { should_not be_able_to(:answer, expired_poll_from_own_geozone) }
it { should_not be_able_to(:answer, expired_poll) }
it { should_not be_able_to(:answer, expired_poll_from_other_geozone) }
end
end

131
spec/models/poll/answer_spec.rb
View File

@@ -101,136 +101,5 @@ describe Poll::Answer do
expect(build(:poll_answer, question: question, answer: "Four")).not_to be_valid
end
context "creating answers at the same time", :race_condition do
it "validates max votes on single-answer questions" do
author = create(:user)
question = create(:poll_question, :yes_no)
answer = build(:poll_answer, author: author, question: question, answer: "Yes")
other_answer = build(:poll_answer, author: author, question: question, answer: "No")
[answer, other_answer].map do |poll_answer|
Thread.new { poll_answer.save }
end.each(&:join)
expect(Poll::Answer.count).to be 1
end
it "validates max votes on multiple-answer questions" do
author = create(:user, :level_two)
question = create(:poll_question_multiple, :abc, max_votes: 2)
create(:poll_answer, question: question, answer: "Answer A", author: author)
answer = build(:poll_answer, question: question, answer: "Answer B", author: author)
other_answer = build(:poll_answer, question: question, answer: "Answer C", author: author)
[answer, other_answer].map do |poll_answer|
Thread.new { poll_answer.save }
end.each(&:join)
expect(Poll::Answer.count).to be 2
end
end
end
describe "#save_and_record_voter_participation" do
let(:author) { create(:user, :level_two) }
let(:poll) { create(:poll) }
let(:question) { create(:poll_question, :yes_no, poll: poll) }
it "creates a poll_voter with user and poll data" do
answer = create(:poll_answer, question: question, author: author, answer: "Yes")
expect(answer.poll.voters).to be_blank
answer.save_and_record_voter_participation
expect(poll.reload.voters.size).to eq(1)
voter = poll.voters.first
expect(voter.document_number).to eq(answer.author.document_number)
expect(voter.poll_id).to eq(answer.poll.id)
expect(voter.officer_id).to be nil
end
it "updates a poll_voter with user and poll data" do
answer = create(:poll_answer, question: question, author: author, answer: "Yes")
answer.save_and_record_voter_participation
expect(poll.reload.voters.size).to eq(1)
updated_answer = answer.question.find_or_initialize_user_answer(
answer.author,
answer.question.question_options.excluding(answer.option).sample.id
)
updated_answer.save_and_record_voter_participation
expect(poll.reload.voters.size).to eq(1)
voter = poll.voters.first
expect(voter.document_number).to eq(updated_answer.author.document_number)
expect(voter.poll_id).to eq(updated_answer.poll.id)
end
it "does not save the answer if the voter is invalid" do
allow_any_instance_of(Poll::Voter).to receive(:valid?).and_return(false)
answer = build(:poll_answer)
expect do
answer.save_and_record_voter_participation
end.to raise_error(ActiveRecord::RecordInvalid)
expect(answer).not_to be_persisted
end
it "does not create two voters when creating two answers at the same time", :race_condition do
answer = build(:poll_answer, question: question, author: author, answer: "Yes")
other_answer = build(:poll_answer, question: question, author: author, answer: "No")
[answer, other_answer].map do |poll_answer|
Thread.new do
poll_answer.save_and_record_voter_participation
rescue ActiveRecord::RecordInvalid
end
end.each(&:join)
expect(Poll::Voter.count).to be 1
end
it "does not create two voters when calling the method twice at the same time", :race_condition do
answer = create(:poll_answer, question: question, author: author, answer: "Yes")
2.times.map do
Thread.new { answer.save_and_record_voter_participation }
end.each(&:join)
expect(Poll::Voter.count).to be 1
end
end
describe "#destroy_and_remove_voter_participation" do
let(:poll) { create(:poll) }
let(:question) { create(:poll_question, :yes_no, poll: poll) }
it "destroys voter record and answer when it was the only user's answer" do
answer = build(:poll_answer, question: question)
answer.save_and_record_voter_participation
expect { answer.destroy_and_remove_voter_participation }
.to change { Poll::Answer.count }.by(-1)
.and change { Poll::Voter.count }.by(-1)
end
it "destroys the answer but does not destroy the voter record when the user
has answered other poll questions" do
answer = build(:poll_answer, question: question)
answer.save_and_record_voter_participation
other_question = create(:poll_question, :yes_no, poll: poll)
other_answer = build(:poll_answer, question: other_question, author: answer.author)
other_answer.save_and_record_voter_participation
expect(other_answer).to be_persisted
expect { answer.destroy_and_remove_voter_participation }
.to change { Poll::Answer.count }.by(-1)
.and change { Poll::Voter.count }.by(0)
end
end
end

110
spec/models/poll/web_vote_spec.rb Normal file
View File

@@ -0,0 +1,110 @@
require "rails_helper"
describe Poll::WebVote do
describe "#update" do
let(:user) { create(:user, :level_two) }
let(:poll) { create(:poll) }
let!(:question) { create(:poll_question, :yes_no, poll: poll) }
let(:option_yes) { question.question_options.find_by(title: "Yes") }
let(:option_no) { question.question_options.find_by(title: "No") }
let(:web_vote) { Poll::WebVote.new(poll, user) }
it "creates a poll_voter with user and poll data" do
expect(poll.voters).to be_blank
expect(question.answers).to be_blank
web_vote.update(question.id.to_s => { option_id: option_yes.id.to_s })
expect(poll.reload.voters.size).to eq 1
expect(question.reload.answers.size).to eq 1
voter = poll.voters.first
answer = question.answers.first
expect(answer.author).to eq user
expect(voter.document_number).to eq user.document_number
expect(voter.poll_id).to eq answer.poll.id
expect(voter.officer_id).to be nil
end
it "updates a poll_voter with user and poll data" do
create(:poll_answer, question: question, author: user, option: option_yes)
web_vote.update(question.id.to_s => { option_id: option_no.id.to_s })
expect(poll.reload.voters.size).to eq 1
expect(question.reload.answers.size).to eq 1
voter = poll.voters.first
answer = question.answers.first
expect(answer.author).to eq user
expect(answer.option).to eq option_no
expect(voter.document_number).to eq answer.author.document_number
expect(voter.poll_id).to eq answer.poll.id
end
it "does not save the answer if the voter is invalid" do
allow_any_instance_of(Poll::Voter).to receive(:valid?).and_return(false)
expect do
web_vote.update(question.id.to_s => { option_id: option_yes.id.to_s })
end.to raise_error(ActiveRecord::RecordInvalid)
expect(poll.voters).to be_blank
expect(question.answers).to be_blank
end
it "does not create voters or answers when leaving everything blank" do
web_vote.update({})
expect(poll.reload.voters.size).to eq 0
expect(question.reload.answers.size).to eq 0
end
it "deletes existing answers and voter when no answers are given" do
create(:poll_answer, question: question, author: user, option: option_yes)
create(:poll_voter, poll: poll, user: user)
web_vote.update({})
expect(poll.reload.voters.size).to eq 0
expect(question.reload.answers.size).to eq 0
end
context "creating answers at the same time", :race_condition do
it "does not create two voters or two answers for two different answers" do
[option_yes, option_no].map do |option|
Thread.new { web_vote.update(question.id.to_s => { option_id: option.id.to_s }) }
end.each(&:join)
expect(Poll::Voter.count).to be 1
expect(Poll::Answer.count).to be 1
end
it "does not create two voters for duplicate answers" do
2.times.map do
Thread.new { web_vote.update(question.id.to_s => { option_id: option_yes.id.to_s }) }
end.each(&:join)
expect(Poll::Voter.count).to be 1
end
it "validates max votes on multiple-answer questions" do
question = create(:poll_question_multiple, :abc, poll: poll, max_votes: 2)
option_a = question.question_options.find_by(title: "Answer A")
option_b = question.question_options.find_by(title: "Answer B")
option_c = question.question_options.find_by(title: "Answer C")
create(:poll_answer, question: question, author: user, option: option_a)
[option_b, option_c].map do |option|
Thread.new do
web_vote.update(question.id.to_s => { option_id: [option_a.id.to_s, option.id.to_s] })
end
end.each(&:join)
expect(Poll::Answer.count).to be 2
end
end
end
end