Merge pull request #5762 from consuldemocracy/unique_index_on_poll_voters

Add unique index to poll voters table

app/controllers/officing/voters_controller.rb

@@ -14,18 +14,14 @@ class Officing::VotersController < Officing::BaseController
     @poll = Poll.find(voter_params[:poll_id])
     @user = User.find(voter_params[:user_id])
 
-    @user.with_lock do
+    @voter = Poll::Voter.create_with(
-      @voter = Poll::Voter.new(document_type: @user.document_type,
+      document_type: @user.document_type,
-                               document_number: @user.document_number,
+      document_number: @user.document_number,
-                               user: @user,
+      origin: "booth",
-                               poll: @poll,
+      officer: current_user.poll_officer,
-                               origin: "booth",
+      booth_assignment: current_booth.booth_assignments.find_by(poll: @poll),
-                               officer: current_user.poll_officer,
+      officer_assignment: officer_assignment(@poll)
-                               booth_assignment: current_booth.booth_assignments.find_by(poll: @poll),
+    ).find_or_create_by!(user: @user, poll: @poll)
-                               officer_assignment: officer_assignment(@poll))
-
-      @voter.save!
-    end
   end
 
   private

app/controllers/polls_controller.rb

@@ -23,6 +23,8 @@ class PollsController < ApplicationController
   end
 
   def answer
+    raise CanCan::AccessDenied if @poll.voted_in_booth?(current_user)
+
     @web_vote = Poll::WebVote.new(@poll, current_user)
 
     if @web_vote.update(answer_params)

app/models/poll/voter.rb

@@ -15,7 +15,6 @@ class Poll
     validates :officer_assignment_id, presence: true, if: ->(voter) { voter.origin == "booth" }
 
     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

app/models/user.rb

@@ -289,14 +289,10 @@ class User < ApplicationRecord
   def take_votes_from(other_user)
     return if other_user.blank?
 
-    with_lock do
+    Poll::Voter.where(user_id: other_user.id).find_each do |poll_voter|
-      Poll::Voter.where(user_id: other_user.id).find_each do |poll_voter|
+      transaction(requires_new: true) { poll_voter.update_column(:user_id, id) }
-        if Poll::Voter.where(poll: poll_voter.poll, user_id: id).any?
+    rescue ActiveRecord::RecordNotUnique
-          poll_voter.delete
+      poll_voter.delete
-        else
-          poll_voter.update_column(:user_id, id)
-        end
-      end
     end
 
     Budget::Ballot.where(user_id: other_user.id).update_all(user_id: id)

config/locales/en/activerecord.yml

@@ -576,8 +576,6 @@ en:
           attributes:
             document_number:
               not_in_census: "Document not in census"
-            user_id:
-              has_voted: "User has already voted"
         legislation/process:
           attributes:
             end_date:

config/locales/es/activerecord.yml

@@ -576,8 +576,6 @@ 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:
             end_date:

db/migrate/20250814122241_add_unique_index_to_poll_voters.rb

@@ -0,0 +1,5 @@
+class AddUniqueIndexToPollVoters < ActiveRecord::Migration[7.1]
+  def change
+    add_index :poll_voters, [:user_id, :poll_id], unique: true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2025_03_13_014205) do
+ActiveRecord::Schema[7.1].define(version: 2025_08_14_122241) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
   enable_extension "plpgsql"
@@ -1209,6 +1209,7 @@ ActiveRecord::Schema[7.1].define(version: 2025_03_13_014205) do
     t.index ["officer_assignment_id"], name: "index_poll_voters_on_officer_assignment_id"
     t.index ["poll_id", "document_number", "document_type"], name: "doc_by_poll"
     t.index ["poll_id"], name: "index_poll_voters_on_poll_id"
+    t.index ["user_id", "poll_id"], name: "index_poll_voters_on_user_id_and_poll_id", unique: true
     t.index ["user_id"], name: "index_poll_voters_on_user_id"
   end
 

spec/controllers/officing/voters_controller_spec.rb

@@ -2,20 +2,19 @@ require "rails_helper"
 
 describe Officing::VotersController do
   describe "POST create" do
+    let(:officer) { create(:poll_officer) }
+    before { sign_in(officer.user) }
+
     it "does not create two records with two simultaneous requests", :race_condition do
-      officer = create(:poll_officer)
       poll = create(:poll, officers: [officer])
       user = create(:user, :level_two)
 
-      sign_in(officer.user)
-
       2.times.map do
         Thread.new do
           post :create, params: {
             voter: { poll_id: poll.id, user_id: user.id },
             format: :js
           }
-        rescue ActiveRecord::RecordInvalid
         end
       end.each(&:join)
 
@@ -24,7 +23,6 @@ describe Officing::VotersController do
     end
 
     it "stores officer and booth information" do
-      officer = create(:poll_officer)
       user = create(:user, :in_census)
       poll1 = create(:poll, name: "Would you be interested in XYZ?")
       poll2 = create(:poll, name: "Testing polls")
@@ -37,7 +35,6 @@ describe Officing::VotersController do
 
       validate_officer
       set_officing_booth(booth)
-      sign_in(officer.user)
 
       post :create, params: {
         voter: { poll_id: poll1.id, user_id: user.id },
@@ -61,5 +58,20 @@ describe Officing::VotersController do
       expect(voter2.booth_assignment).to eq(assignment2)
       expect(voter2.officer_assignment).to eq(assignment2.officer_assignments.first)
     end
+
+    it "does not overwrite non key attributes when a web voter already exists" do
+      user = create(:user, :level_two, document_number: "11223344Z")
+      poll = create(:poll, officers: [officer])
+      existing = create(:poll_voter, poll: poll, user: user, origin: "web")
+
+      expect do
+        post :create, params: { voter: { poll_id: poll.id, user_id: user.id }, format: :js }
+        expect(response).to be_successful
+      end.not_to change { Poll::Voter.count }
+
+      existing.reload
+      expect(existing.origin).to eq "web"
+      expect(existing.document_number).to eq "11223344Z"
+    end
   end
 end

spec/controllers/polls_controller_spec.rb

@@ -28,5 +28,18 @@ describe PollsController do
 
       expect(Poll::Answer.count).to eq 1
     end
+
+    it "denies access when users have already voted in a booth" do
+      poll = create(:poll)
+      user = create(:user, :level_two)
+      create(:poll_voter, :from_booth, poll: poll, user: user)
+
+      sign_in(user)
+
+      post :answer, params: { id: poll.id, web_vote: {}}
+
+      expect(response).to redirect_to "/"
+      expect(flash[:alert]).to eq "You do not have permission to access this page."
+    end
   end
 end

spec/lib/tasks/polls_spec.rb

@@ -4,49 +4,6 @@ describe "polls tasks" do
   let(:poll) { create(:poll) }
   let(:user) { create(:user, :level_two) }
 
-  describe "polls:remove_duplicate_voters" do
-    before { Rake::Task["polls:remove_duplicate_voters"].reenable }
-
-    it "removes duplicate voters" do
-      second_user = create(:user, :level_two)
-
-      voter = create(:poll_voter, poll: poll, user: user)
-      second_voter = create(:poll_voter, poll: poll, user: second_user)
-      other_user_voter = create(:poll_voter, poll: poll, user: create(:user, :level_two))
-      other_poll_voter = create(:poll_voter, poll: create(:poll), user: user)
-
-      2.times { insert(:poll_voter, poll_id: poll.id, user_id: user.id) }
-      insert(:poll_voter, poll_id: poll.id, user_id: second_user.id)
-
-      expect(Poll::Voter.count).to eq 7
-
-      Rake.application.invoke_task("polls:remove_duplicate_voters")
-
-      expect(Poll::Voter.count).to eq 4
-      expect(Poll::Voter.all).to match_array [voter, second_voter, other_user_voter, other_poll_voter]
-    end
-
-    it "removes duplicate voters on tenants" do
-      create(:tenant, schema: "voters")
-
-      Tenant.switch("voters") do
-        poll = create(:poll)
-        user = create(:user, :level_two)
-
-        create(:poll_voter, poll: poll, user: user)
-        insert(:poll_voter, poll_id: poll.id, user_id: user.id)
-
-        expect(Poll::Voter.count).to eq 2
-      end
-
-      Rake.application.invoke_task("polls:remove_duplicate_voters")
-
-      Tenant.switch("voters") do
-        expect(Poll::Voter.count).to eq 1
-      end
-    end
-  end
-
   describe "polls:remove_duplicate_answers" do
     before { Rake::Task["polls:remove_duplicate_answers"].reenable }
 

spec/models/poll/voter_spec.rb

@@ -33,8 +33,7 @@ describe Poll::Voter do
 
       voter = build(:poll_voter, user: user, poll: poll)
 
-      expect(voter).not_to be_valid
+      expect { voter.save }.to raise_error ActiveRecord::RecordNotUnique
-      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
@@ -42,8 +41,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.save }.to raise_error ActiveRecord::RecordNotUnique
-      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
@@ -51,8 +49,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.save }.to raise_error ActiveRecord::RecordNotUnique
-      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
@@ -68,8 +65,8 @@ describe Poll::Voter do
       create(:poll_voter, :from_web, user: answer.author, poll: answer.poll)
 
       voter = build(:poll_voter, poll: answer.question.poll, user: answer.author)
-      expect(voter).not_to be_valid
+
-      expect(voter.errors.messages[:user_id]).to eq(["User has already voted"])
+      expect { voter.save }.to raise_error ActiveRecord::RecordNotUnique
     end
 
     context "Skip verification is enabled" do
@@ -83,7 +80,7 @@ describe Poll::Voter do
 
         voter = build(:poll_voter, user: user, poll: poll)
 
-        expect(voter).not_to be_valid
+        expect { voter.save }.to raise_error ActiveRecord::RecordNotUnique
       end
 
       it "is valid if other users have voted in the same poll" do
@@ -146,6 +143,15 @@ describe Poll::Voter do
         expect(voter).to be_valid
       end
     end
+
+    describe ".create_or_find_by!" do
+      it "finds the voter when it already exists instead of failing the validation" do
+        existing_voter = Poll::Voter.create_with(origin: "web")
+                                    .create_or_find_by!(user: voter.user, poll: voter.poll)
+
+        expect(existing_voter).to eq voter
+      end
+    end
   end
 
   describe "scopes" do

spec/models/poll/web_vote_spec.rb

@@ -73,6 +73,18 @@ describe Poll::WebVote do
       expect(question.reload.answers.size).to eq 0
     end
 
+    it "raises an exception when the user has already voted in a booth" do
+      create(:poll_voter, :from_booth, poll: poll, user: user)
+
+      expect do
+        web_vote.update(question.id.to_s => { option_id: option_yes.id.to_s })
+      end.to raise_error(ActiveRecord::RecordNotFound)
+
+      expect(poll.reload.voters.size).to eq 1
+      expect(poll.voters.first.user).to eq user
+      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|