From 096f546c24bc41d8846b0808f218f9430bd9ba8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 6 Aug 2020 22:05:39 +0200 Subject: [PATCH] Make sure users only vote once in the same poll When skipping verification, we cannot apply the validation rule saying the document number and document type must be unique, because they'll be `nil` in many cases. So we were skipping the rule, but that makes it possible for the same user to vote several times (for instance, once in a booth and once via web). So we're changing the scope of the uniqueness rule: instead of being unique per document number, voters are unique per user. The reason we made them unique per document number was that back in commit 900563e3 (when we added the rule), we hadn't added the relation between users and poll voters yet. --- app/models/poll/voter.rb | 3 ++- config/locales/en/activerecord.yml | 1 + config/locales/es/activerecord.yml | 1 + spec/models/poll/voter_spec.rb | 32 ++++++++++++++++++++++++++---- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/models/poll/voter.rb b/app/models/poll/voter.rb index 66395aa67..2ea150718 100644 --- a/app/models/poll/voter.rb +++ b/app/models/poll/voter.rb @@ -14,7 +14,8 @@ class Poll validates :booth_assignment_id, presence: true, if: ->(voter) { voter.origin == "booth" } validates :officer_assignment_id, presence: true, if: ->(voter) { voter.origin == "booth" } - validates :document_number, presence: true, uniqueness: { scope: [:poll_id, :document_type], message: :has_voted }, unless: :skip_user_verification? + validates :document_number, presence: true, unless: :skip_user_verification? + validates :user_id, uniqueness: { scope: [:poll_id], message: :has_voted } validates :origin, inclusion: { in: VALID_ORIGINS } before_validation :set_demographic_info, :set_document_info, :set_denormalized_booth_assignment_id diff --git a/config/locales/en/activerecord.yml b/config/locales/en/activerecord.yml index 9ea6e1c51..dd024d838 100644 --- a/config/locales/en/activerecord.yml +++ b/config/locales/en/activerecord.yml @@ -508,6 +508,7 @@ en: attributes: document_number: not_in_census: "Document not in census" + user_id: has_voted: "User has already voted" legislation/process: attributes: diff --git a/config/locales/es/activerecord.yml b/config/locales/es/activerecord.yml index c3bc2c3e9..3d324a27c 100644 --- a/config/locales/es/activerecord.yml +++ b/config/locales/es/activerecord.yml @@ -510,6 +510,7 @@ es: attributes: document_number: not_in_census: "Este documento no aparece en el censo" + user_id: has_voted: "Este usuario ya ha votado" legislation/process: attributes: diff --git a/spec/models/poll/voter_spec.rb b/spec/models/poll/voter_spec.rb index c919e971f..9ed82be67 100644 --- a/spec/models/poll/voter_spec.rb +++ b/spec/models/poll/voter_spec.rb @@ -34,7 +34,7 @@ describe Poll::Voter do voter = build(:poll_voter, user: user, poll: poll) expect(voter).not_to be_valid - expect(voter.errors.messages[:document_number]).to eq(["User has already voted"]) + expect(voter.errors.messages[:user_id]).to eq(["User has already voted"]) end it "is not valid if the user has already voted in the same poll/booth" do @@ -43,7 +43,7 @@ describe Poll::Voter do voter = build(:poll_voter, user: user, poll: poll, booth_assignment: booth_assignment) expect(voter).not_to be_valid - expect(voter.errors.messages[:document_number]).to eq(["User has already voted"]) + expect(voter.errors.messages[:user_id]).to eq(["User has already voted"]) end it "is not valid if the user has already voted in different booth in the same poll" do @@ -52,7 +52,7 @@ describe Poll::Voter do voter = build(:poll_voter, :from_booth, user: user, poll: poll, booth: booth) expect(voter).not_to be_valid - expect(voter.errors.messages[:document_number]).to eq(["User has already voted"]) + expect(voter.errors.messages[:user_id]).to eq(["User has already voted"]) end it "is valid if the user has already voted in the same booth in different poll" do @@ -69,7 +69,31 @@ describe Poll::Voter do voter = build(:poll_voter, poll: answer.question.poll, user: answer.author) expect(voter).not_to be_valid - expect(voter.errors.messages[:document_number]).to eq(["User has already voted"]) + expect(voter.errors.messages[:user_id]).to eq(["User has already voted"]) + end + + context "Skip verification is enabled" do + before do + Setting["feature.user.skip_verification"] = true + user.update!(document_number: nil, document_type: nil) + end + + it "is not valid if the user has already voted in the same poll" do + create(:poll_voter, user: user, poll: poll) + + voter = build(:poll_voter, user: user, poll: poll) + + expect(voter).not_to be_valid + end + + it "is valid if other users have voted in the same poll" do + another_user = create(:user, :level_two, document_number: nil, document_type: nil) + create(:poll_voter, user: another_user, poll: poll) + + voter = build(:poll_voter, user: user, poll: poll) + + expect(voter).to be_valid + end end context "origin" do