diff --git a/app/components/polls/questions/options_component.html.erb b/app/components/polls/questions/options_component.html.erb index ae5d6f2a6..a183b2aac 100644 --- a/app/components/polls/questions/options_component.html.erb +++ b/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", diff --git a/app/controllers/polls/answers_controller.rb b/app/controllers/polls/answers_controller.rb index fc75620ec..a969c5edf 100644 --- a/app/controllers/polls/answers_controller.rb +++ b/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| diff --git a/app/models/poll/answer.rb b/app/models/poll/answer.rb index 039744fd5..b9c52fb38 100644 --- a/app/models/poll/answer.rb +++ b/app/models/poll/answer.rb @@ -1,12 +1,14 @@ class Poll::Answer < ApplicationRecord belongs_to :question, -> { with_hidden }, inverse_of: :answers - belongs_to :author, -> { with_hidden }, class_name: "User", inverse_of: :poll_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 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 }}, diff --git a/app/models/poll/question.rb b/app/models/poll/question.rb index 729cdbe46..c55320560 100644 --- a/app/models/poll/question.rb +++ b/app/models/poll/question.rb @@ -98,20 +98,23 @@ class Poll::Question < ApplicationRecord end end - def find_or_initialize_user_answer(user, title) - answer = answers.find_or_initialize_by(find_by_attributes(user, title)) - answer.answer = title + def find_or_initialize_user_answer(user, option_id) + option = question_options.find(option_id) + + 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 diff --git a/app/models/poll/question/option.rb b/app/models/poll/question/option.rb index 59e38aa33..c191c5923 100644 --- a/app/models/poll/question/option.rb +++ b/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", diff --git a/db/migrate/20240513135357_add_question_answer_id_to_poll_answers.rb b/db/migrate/20240513135357_add_question_answer_id_to_poll_answers.rb new file mode 100644 index 000000000..d96d9027e --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 0dfa54bd7..d565cb35c 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/controllers/polls/answers_controller_spec.rb b/spec/controllers/polls/answers_controller_spec.rb new file mode 100644 index 000000000..08f129c0b --- /dev/null +++ b/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 diff --git a/spec/factories/polls.rb b/spec/factories/polls.rb index 4f61232bc..0d7811784 100644 --- a/spec/factories/polls.rb +++ b/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 diff --git a/spec/models/poll/answer_spec.rb b/spec/models/poll/answer_spec.rb index 6e76ca44d..b06ccbcd7 100644 --- a/spec/models/poll/answer_spec.rb +++ b/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)