Prevent creation of duplicate poll voters

Note that, when taking votes from an erased user, since poll answers
don't belong to poll voters, we were not migrating them in the
`take_votes_from` method (and we aren't migrating them now either).
This commit is contained in:
Javi Martín
2024-05-11 18:13:35 +02:00
parent 175e990bb4
commit 9a8bfac5bd
6 changed files with 68 additions and 3 deletions

View File

@@ -21,7 +21,8 @@ class Officing::VotersController < Officing::BaseController
officer: current_user.poll_officer, officer: current_user.poll_officer,
booth_assignment: current_booth.booth_assignments.find_by(poll: @poll), booth_assignment: current_booth.booth_assignments.find_by(poll: @poll),
officer_assignment: officer_assignment(@poll)) officer_assignment: officer_assignment(@poll))
@voter.save!
@user.with_lock { @voter.save! }
end end
private private

View File

@@ -16,7 +16,7 @@ class Poll::Answer < ApplicationRecord
scope :by_question, ->(question_id) { where(question_id: question_id) } scope :by_question, ->(question_id) { where(question_id: question_id) }
def save_and_record_voter_participation def save_and_record_voter_participation
transaction do author.with_lock do
save! save!
Poll::Voter.find_or_create_by!(user: author, poll: poll, origin: "web") Poll::Voter.find_or_create_by!(user: author, poll: poll, origin: "web")
end end

View File

@@ -289,7 +289,16 @@ class User < ApplicationRecord
def take_votes_from(other_user) def take_votes_from(other_user)
return if other_user.blank? return if other_user.blank?
Poll::Voter.where(user_id: other_user.id).update_all(user_id: id) with_lock do
Poll::Voter.where(user_id: other_user.id).find_each do |poll_voter|
if Poll::Voter.where(poll: poll_voter.poll, user_id: id).any?
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) Budget::Ballot.where(user_id: other_user.id).update_all(user_id: id)
Vote.where("voter_id = ? AND voter_type = ?", other_user.id, "User").update_all(voter_id: id) Vote.where("voter_id = ? AND voter_type = ?", other_user.id, "User").update_all(voter_id: id)
data_log = "id: #{other_user.id} - #{Time.current.strftime("%Y-%m-%d %H:%M:%S")}" data_log = "id: #{other_user.id} - #{Time.current.strftime("%Y-%m-%d %H:%M:%S")}"

View File

@@ -0,0 +1,27 @@
require "rails_helper"
describe Officing::VotersController do
describe "POST create" do
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
begin
post :create, params: {
voter: { poll_id: poll.id, user_id: user.id },
format: :js
}
rescue ActionDispatch::IllegalStateError
end
end
end.each(&:join)
expect(Poll::Voter.count).to eq 1
end
end
end

View File

@@ -155,6 +155,16 @@ describe Poll::Answer do
expect(Poll::Voter.count).to be 1 expect(Poll::Voter.count).to be 1
end 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 end
describe "#destroy_and_remove_voter_participation" do describe "#destroy_and_remove_voter_participation" do

View File

@@ -744,6 +744,24 @@ describe User do
expect(Poll::Voter.where(user: other_user).count).to eq(0) expect(Poll::Voter.where(user: other_user).count).to eq(0)
expect(Poll::Voter.where(user: user)).to match_array [v1, v2] expect(Poll::Voter.where(user: user)).to match_array [v1, v2]
end end
it "does not reassign votes if the user has already voted" do
poll = create(:poll)
user = create(:user, :level_three)
other_user = create(:user, :level_three)
voter = create(:poll_voter, poll: poll, user: user)
other_voter = create(:poll_voter, poll: poll, user: other_user)
other_poll_voter = create(:poll_voter, poll: create(:poll), user: other_user)
expect(Poll::Voter.where(user: user)).to eq [voter]
expect(Poll::Voter.where(user: other_user)).to match_array [other_voter, other_poll_voter]
user.take_votes_from(other_user)
expect(Poll::Voter.where(user: user)).to match_array [voter, other_poll_voter]
expect(Poll::Voter.where(user: other_user)).to eq []
end
end end
describe "#take_votes_if_erased_document" do describe "#take_votes_if_erased_document" do