Avoid duplicate records in poll answers

Until now, we've stored the text of the answer somebody replied to. The
idea was to handle the scenarios where the user voters for an option but
then that option is deleted and restored, or the texts of the options
are accidentally edited and so the option "Yes" is now "Now" and vice
versa.

However, since commit 3a6e99cb8, options can no longer be edited once
the poll starts, so there's no risk of the option changing once somebody
has voted.

This means we can now store the ID of the option that has been voted.
That'll also help us deal with a bug introduced int 673ec075e, since
answers in different locales are not counted as the same answer. Note we
aren't dealing with this bug right now.

We're still keeping (and storing) the answer as well. There are two
reasons for that.

First, we might add an "open answer" type of questions in the future and
use this column for it.

Second, we've still got logic depending on the answer, and we need to be
careful when changing it because there are existing installations where
the answer is present but the option_id is not.

Note that we're using `dependent: nullify`. The reasoning is that, since
we're storing both the option_id and the answer text, we can still use
the answer text when removing the option. In practice, this won't matter
much, though, since we've got a validation rule that makes it impossible
to destroy options once the poll has started.

Also note we're still allowing duplicate records when the option is nil.
We need to do that until we've removed every duplicate record in the
database.

app/components/polls/questions/options_component.html.erb

@@ -11,7 +11,7 @@
                       "aria-pressed": true %>
       <% else %>
         <%= button_to question_option.title,
-                      question_answers_path(question, answer: question_option.title),
+                      question_answers_path(question, option_id: question_option.id),
                       remote: true,
                       title: t("poll_questions.show.vote_answer", answer: question_option.title),
                       class: "button secondary hollow",

app/controllers/polls/answers_controller.rb

@@ -8,7 +8,7 @@ class Polls::AnswersController < ApplicationController
   def create
     authorize! :answer, @question
 
-    answer = @question.find_or_initialize_user_answer(current_user, params[:answer])
+    answer = @question.find_or_initialize_user_answer(current_user, params[:option_id])
     answer.save_and_record_voter_participation
 
     respond_to do |format|

app/models/poll/answer.rb

@@ -1,5 +1,6 @@
 class Poll::Answer < ApplicationRecord
   belongs_to :question, -> { with_hidden }, inverse_of: :answers
+  belongs_to :option, class_name: "Poll::Question::Option"
   belongs_to :author, -> { with_hidden }, class_name: "User", inverse_of: :poll_answers
 
   delegate :poll, :poll_id, to: :question
@@ -7,6 +8,7 @@ class Poll::Answer < ApplicationRecord
   validates :question, presence: true
   validates :author, presence: true
   validates :answer, presence: true
+  validates :option, uniqueness: { scope: :author_id }, allow_nil: true
   validate :max_votes
 
   validates :answer, inclusion: { in: ->(a) { a.question.possible_answers }},

app/models/poll/question.rb

@@ -98,20 +98,23 @@ class Poll::Question < ApplicationRecord
     end
   end
 
-  def find_or_initialize_user_answer(user, title)
+  def find_or_initialize_user_answer(user, option_id)
-    answer = answers.find_or_initialize_by(find_by_attributes(user, title))
+    option = question_options.find(option_id)
-    answer.answer = title
+
+    answer = answers.find_or_initialize_by(find_by_attributes(user, option))
+    answer.option = option
+    answer.answer = option.title
     answer
   end
 
   private
 
-    def find_by_attributes(user, title)
+    def find_by_attributes(user, option)
       case vote_type
       when "unique", nil
         { author: user }
       when "multiple"
-        { author: user, answer: title }
+        { author: user, answer: option.title }
       end
     end
 end

app/models/poll/question/option.rb

@@ -11,6 +11,7 @@ class Poll::Question::Option < ApplicationRecord
   accepts_nested_attributes_for :documents, allow_destroy: true
 
   belongs_to :question, class_name: "Poll::Question"
+  has_many :answers, class_name: "Poll::Answer", dependent: :nullify
   has_many :videos, class_name: "Poll::Question::Option::Video",
                     dependent: :destroy,
                     foreign_key: "answer_id",

db/migrate/20240513135357_add_question_answer_id_to_poll_answers.rb

@@ -0,0 +1,9 @@
+class AddQuestionAnswerIdToPollAnswers < ActiveRecord::Migration[7.0]
+  def change
+    change_table :poll_answers do |t|
+      t.references :option, index: true, foreign_key: { to_table: :poll_question_answers }
+
+      t.index [:option_id, :author_id], unique: true
+    end
+  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.0].define(version: 2024_05_11_141119) do
+ActiveRecord::Schema[7.0].define(version: 2024_05_13_135357) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
   enable_extension "plpgsql"
@@ -1024,7 +1024,10 @@ ActiveRecord::Schema[7.0].define(version: 2024_05_11_141119) do
     t.string "answer"
     t.datetime "created_at", precision: nil
     t.datetime "updated_at", precision: nil
+    t.bigint "option_id"
     t.index ["author_id"], name: "index_poll_answers_on_author_id"
+    t.index ["option_id", "author_id"], name: "index_poll_answers_on_option_id_and_author_id", unique: true
+    t.index ["option_id"], name: "index_poll_answers_on_option_id"
     t.index ["question_id", "answer"], name: "index_poll_answers_on_question_id_and_answer"
     t.index ["question_id"], name: "index_poll_answers_on_question_id"
   end
@@ -1798,6 +1801,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_05_11_141119) do
   add_foreign_key "moderators", "users"
   add_foreign_key "notifications", "users"
   add_foreign_key "organizations", "users"
+  add_foreign_key "poll_answers", "poll_question_answers", column: "option_id"
   add_foreign_key "poll_answers", "poll_questions", column: "question_id"
   add_foreign_key "poll_booth_assignments", "polls"
   add_foreign_key "poll_officer_assignments", "poll_booth_assignments", column: "booth_assignment_id"

spec/controllers/polls/answers_controller_spec.rb

@@ -0,0 +1,25 @@
+require "rails_helper"
+
+describe Polls::AnswersController do
+  describe "POST create" do
+    it "doesn't create duplicate records on simultaneous requests", :race_condition do
+      question = create(:poll_question_multiple, :abc)
+      sign_in(create(:user, :level_two))
+
+      2.times.map do
+        Thread.new do
+          begin
+            post :create, params: {
+              question_id: question.id,
+              option_id: question.question_options.find_by(title: "Answer A").id,
+              format: :js
+            }
+          rescue ActionDispatch::IllegalStateError, ActiveRecord::RecordInvalid
+          end
+        end
+      end.each(&:join)
+
+      expect(Poll::Answer.count).to eq 1
+    end
+  end
+end

spec/factories/polls.rb

@@ -205,6 +205,7 @@ FactoryBot.define do
     question factory: [:poll_question, :yes_no]
     author factory: [:user, :level_two]
     answer { question.question_options.sample.title }
+    option { question.question_options.find_by(title: answer) }
   end
 
   factory :poll_partial_result, class: "Poll::PartialResult" do

spec/models/poll/answer_spec.rb

@@ -51,6 +51,44 @@ describe Poll::Answer do
       expect(answer).not_to be_valid
     end
 
+    it "is not valid when there are two identical answers" do
+      author = create(:user)
+      question = create(:poll_question_multiple, :abc)
+      option = question.question_options.first
+
+      create(:poll_answer, author: author, question: question, option: option, answer: "Answer A")
+
+      answer = build(:poll_answer, author: author, question: question, option: option, answer: "Answer A")
+
+      expect(answer).not_to be_valid
+      expect { answer.save(validate: false) }.to raise_error ActiveRecord::RecordNotUnique
+    end
+
+    it "is not valid when there are two answers with the same option and different answer" do
+      author = create(:user)
+      question = create(:poll_question_multiple, :abc)
+      option = question.question_options.first
+
+      create(:poll_answer, author: author, question: question, option: option, answer: "Answer A")
+
+      answer = build(:poll_answer, author: author, question: question, option: option, answer: "Answer B")
+
+      expect(answer).not_to be_valid
+      expect { answer.save(validate: false) }.to raise_error ActiveRecord::RecordNotUnique
+    end
+
+    it "is valid when there are two identical answers and the option is nil" do
+      author = create(:user)
+      question = create(:poll_question_multiple, :abc)
+
+      create(:poll_answer, author: author, question: question, option: nil, answer: "Answer A")
+
+      answer = build(:poll_answer, author: author, question: question, option: nil, answer: "Answer A")
+
+      expect(answer).to be_valid
+      expect { answer.save }.not_to raise_error
+    end
+
     it "is valid for answers included in the Poll::Question's question_options list" do
       question = create(:poll_question)
       create(:poll_question_option, title: "One", question: question)
@@ -119,7 +157,10 @@ describe Poll::Answer do
 
       expect(poll.reload.voters.size).to eq(1)
 
-      updated_answer = answer.question.find_or_initialize_user_answer(answer.author, "No")
+      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)