diff --git a/app/components/admin/poll/results/question_component.rb b/app/components/admin/poll/results/question_component.rb index 8836a6b91..a539e8d88 100644 --- a/app/components/admin/poll/results/question_component.rb +++ b/app/components/admin/poll/results/question_component.rb @@ -7,13 +7,6 @@ class Admin::Poll::Results::QuestionComponent < ApplicationComponent end def votes_for(option) - grouped = by_answer[option.title] || [] - grouped.sum(&:amount) + partial_results.where(option: option).sum(:amount) end - - private - - def by_answer - @by_answer ||= partial_results.where(question: question).group_by(&:answer) - end end diff --git a/app/controllers/officing/results_controller.rb b/app/controllers/officing/results_controller.rb index 21bb8e5f3..8aa69ae7d 100644 --- a/app/controllers/officing/results_controller.rb +++ b/app/controllers/officing/results_controller.rb @@ -39,18 +39,19 @@ class Officing::ResultsController < Officing::BaseController params[:questions].each_pair do |question_id, results| question = @poll.questions.find(question_id) - results.each_pair do |answer_index, count| + results.each_pair do |option_index, count| next if count.blank? - answer = question.question_options.find_by(given_order: answer_index.to_i + 1).title + option = question.question_options.find_by(given_order: option_index.to_i + 1) partial_result = ::Poll::PartialResult.find_or_initialize_by( booth_assignment_id: @officer_assignment.booth_assignment_id, date: Date.current, question_id: question_id, - answer: answer + option_id: option.id ) partial_result.officer_assignment_id = @officer_assignment.id + partial_result.answer = option.title partial_result.amount = count.to_i partial_result.author = current_user partial_result.origin = "booth" diff --git a/app/models/poll/answer.rb b/app/models/poll/answer.rb index d4b0150e2..7663aa42b 100644 --- a/app/models/poll/answer.rb +++ b/app/models/poll/answer.rb @@ -11,8 +11,8 @@ class Poll::Answer < ApplicationRecord validates :option, uniqueness: { scope: :author_id }, allow_nil: true validate :max_votes - validates :answer, inclusion: { in: ->(a) { a.question.possible_answers }}, - unless: ->(a) { a.question.blank? } + validates :answer, inclusion: { in: ->(poll_answer) { poll_answer.option.possible_answers }}, + if: ->(poll_answer) { poll_answer.option.present? } scope :by_author, ->(author_id) { where(author_id: author_id) } scope :by_question, ->(question_id) { where(question_id: question_id) } diff --git a/app/models/poll/partial_result.rb b/app/models/poll/partial_result.rb index eb11dd4e3..d671d6184 100644 --- a/app/models/poll/partial_result.rb +++ b/app/models/poll/partial_result.rb @@ -5,13 +5,15 @@ class Poll::PartialResult < ApplicationRecord belongs_to :author, -> { with_hidden }, class_name: "User", inverse_of: :poll_partial_results belongs_to :booth_assignment belongs_to :officer_assignment + belongs_to :option, class_name: "Poll::Question::Option" validates :question, presence: true validates :author, presence: true validates :answer, presence: true - validates :answer, inclusion: { in: ->(a) { a.question.possible_answers }}, - unless: ->(a) { a.question.blank? } + validates :answer, inclusion: { in: ->(partial_result) { partial_result.option.possible_answers }}, + if: ->(partial_result) { partial_result.option.present? } validates :origin, inclusion: { in: ->(*) { VALID_ORIGINS }} + validates :option, uniqueness: { scope: [:booth_assignment_id, :date] }, allow_nil: true scope :by_author, ->(author_id) { where(author_id: author_id) } scope :by_question, ->(question_id) { where(question_id: question_id) } diff --git a/app/models/poll/question.rb b/app/models/poll/question.rb index 93e9c4453..97d1d2c60 100644 --- a/app/models/poll/question.rb +++ b/app/models/poll/question.rb @@ -49,10 +49,6 @@ class Poll::Question < ApplicationRecord question_options.max_by(&:total_votes)&.id end - def possible_answers - question_options.joins(:translations).pluck(:title) - end - def options_with_read_more? options_with_read_more.any? end diff --git a/app/models/poll/question/option.rb b/app/models/poll/question/option.rb index 9de143c06..4b59ec04b 100644 --- a/app/models/poll/question/option.rb +++ b/app/models/poll/question/option.rb @@ -12,6 +12,7 @@ class Poll::Question::Option < ApplicationRecord belongs_to :question, class_name: "Poll::Question" has_many :answers, class_name: "Poll::Answer", dependent: :nullify + has_many :partial_results, class_name: "Poll::PartialResult", dependent: :nullify has_many :videos, class_name: "Poll::Question::Option::Video", dependent: :destroy, foreign_key: "answer_id", @@ -50,4 +51,8 @@ class Poll::Question::Option < ApplicationRecord def with_read_more? description.present? || images.any? || documents.any? || videos.any? end + + def possible_answers + translations.pluck(:title) + end end diff --git a/db/migrate/20250909145207_add_option_id_to_poll_partial_results.rb b/db/migrate/20250909145207_add_option_id_to_poll_partial_results.rb new file mode 100644 index 000000000..f07f3c2a4 --- /dev/null +++ b/db/migrate/20250909145207_add_option_id_to_poll_partial_results.rb @@ -0,0 +1,9 @@ +class AddOptionIdToPollPartialResults < ActiveRecord::Migration[7.1] + def change + change_table :poll_partial_results do |t| + t.references :option, index: true, foreign_key: { to_table: :poll_question_answers } + + t.index [:booth_assignment_id, :date, :option_id], unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 42c7e3ecd..10cc0d023 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.1].define(version: 2025_08_14_122241) do +ActiveRecord::Schema[7.1].define(version: 2025_09_09_145207) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" enable_extension "plpgsql" @@ -1084,9 +1084,12 @@ ActiveRecord::Schema[7.1].define(version: 2025_08_14_122241) do t.text "amount_log", default: "" t.text "officer_assignment_id_log", default: "" t.text "author_id_log", default: "" + t.bigint "option_id" t.index ["answer"], name: "index_poll_partial_results_on_answer" t.index ["author_id"], name: "index_poll_partial_results_on_author_id" + t.index ["booth_assignment_id", "date", "option_id"], name: "idx_on_booth_assignment_id_date_option_id_2ffcf6ea3b", unique: true t.index ["booth_assignment_id", "date"], name: "index_poll_partial_results_on_booth_assignment_id_and_date" + t.index ["option_id"], name: "index_poll_partial_results_on_option_id" t.index ["origin"], name: "index_poll_partial_results_on_origin" t.index ["question_id"], name: "index_poll_partial_results_on_question_id" end @@ -1799,6 +1802,7 @@ ActiveRecord::Schema[7.1].define(version: 2025_08_14_122241) do add_foreign_key "poll_officer_assignments", "poll_booth_assignments", column: "booth_assignment_id" add_foreign_key "poll_partial_results", "poll_booth_assignments", column: "booth_assignment_id" add_foreign_key "poll_partial_results", "poll_officer_assignments", column: "officer_assignment_id" + add_foreign_key "poll_partial_results", "poll_question_answers", column: "option_id" add_foreign_key "poll_partial_results", "poll_questions", column: "question_id" add_foreign_key "poll_partial_results", "users", column: "author_id" add_foreign_key "poll_question_answer_videos", "poll_question_answers", column: "answer_id" diff --git a/spec/components/admin/poll/results/question_component_spec.rb b/spec/components/admin/poll/results/question_component_spec.rb index aaca189f5..dd366e759 100644 --- a/spec/components/admin/poll/results/question_component_spec.rb +++ b/spec/components/admin/poll/results/question_component_spec.rb @@ -3,11 +3,8 @@ require "rails_helper" describe Admin::Poll::Results::QuestionComponent do let(:poll) { create(:poll) } let(:question) { create(:poll_question, poll: poll, title: "What do you want?") } - - before do - create(:poll_question_option, question: question, title: "Yes") - create(:poll_question_option, question: question, title: "No") - end + let!(:option_yes) { create(:poll_question_option, question: question, title: "Yes") } + let!(:option_no) { create(:poll_question_option, question: question, title: "No") } it "renders question title and headers" do render_inline Admin::Poll::Results::QuestionComponent.new(question, poll.partial_results) @@ -25,10 +22,10 @@ describe Admin::Poll::Results::QuestionComponent do expect(page).to have_css "td", text: "No" end - it "sums votes by answer title" do - create(:poll_partial_result, question: question, answer: "Yes", amount: 2) - create(:poll_partial_result, question: question, answer: "Yes", amount: 1) - create(:poll_partial_result, question: question, answer: "No", amount: 5) + it "sums votes by option" do + create(:poll_partial_result, question: question, option: option_yes, amount: 2, date: Date.current) + create(:poll_partial_result, question: question, option: option_yes, amount: 1, date: Date.yesterday) + create(:poll_partial_result, question: question, option: option_no, amount: 5) render_inline Admin::Poll::Results::QuestionComponent.new(question, question.partial_results) diff --git a/spec/factories/polls.rb b/spec/factories/polls.rb index 100029ec7..358af5e80 100644 --- a/spec/factories/polls.rb +++ b/spec/factories/polls.rb @@ -226,7 +226,14 @@ FactoryBot.define do question factory: [:poll_question, :yes_no] author factory: :user origin { "web" } - answer { question.question_options.sample.title } + option do + if answer + question.question_options.find_by(title: answer) + else + question.question_options.sample + end + end + after(:build) { |poll_partial_result| poll_partial_result.answer ||= poll_partial_result.option&.title } end factory :poll_recount, class: "Poll::Recount" do diff --git a/spec/lib/tasks/polls_spec.rb b/spec/lib/tasks/polls_spec.rb index abe5f6130..06f40b979 100644 --- a/spec/lib/tasks/polls_spec.rb +++ b/spec/lib/tasks/polls_spec.rb @@ -42,7 +42,7 @@ describe "polls tasks" do answer_attributes = { question: question, author: user, answer: "Answer A" } create(:poll_answer, answer_attributes.merge(option: option_a)) - create(:poll_answer, answer_attributes.merge(option: option_b)) + insert(:poll_answer, answer_attributes.merge(option_id: option_b.id)) expect(Poll::Answer.count).to eq 2 @@ -126,10 +126,11 @@ describe "polls tasks" do answer = create(:poll_answer, question: yes_no_question, author: user, answer: "Yes", option: nil) abc_answer = create(:poll_answer, question: abc_question, author: user, answer: "Answer A", option: nil) - answer_with_inconsistent_option = create(:poll_answer, question: abc_question, - author: user, - answer: "Answer A", - option: option_b) + insert(:poll_answer, question: abc_question, + author: user, + answer: "Answer A", + option_id: option_b.id) + answer_with_inconsistent_option = Poll::Answer.find_by!(option: option_b) answer_with_invalid_option = build(:poll_answer, question: abc_question, author: user, answer: "Non existing", diff --git a/spec/models/poll/answer_spec.rb b/spec/models/poll/answer_spec.rb index aec882fc5..a70ed3a4d 100644 --- a/spec/models/poll/answer_spec.rb +++ b/spec/models/poll/answer_spec.rb @@ -89,17 +89,18 @@ describe Poll::Answer do expect { answer.save }.not_to raise_error end - it "is valid for answers included in the Poll::Question's question_options list" do + it "is valid for answers included in the list of titles for the option" do question = create(:poll_question) - create(:poll_question_option, title: "One", question: question) + option = create(:poll_question_option, title_en: "One", title_es: "Uno", question: question) + create(:poll_question_option, title: "Two", question: question) - create(:poll_question_option, title: "Three", question: question) + create(:poll_question_option, title: "Three", question: create(:poll_question, poll: create(:poll))) - expect(build(:poll_answer, question: question, answer: "One")).to be_valid - expect(build(:poll_answer, question: question, answer: "Two")).to be_valid - expect(build(:poll_answer, question: question, answer: "Three")).to be_valid - - expect(build(:poll_answer, question: question, answer: "Four")).not_to be_valid + expect(build(:poll_answer, option: option, answer: "One")).to be_valid + expect(build(:poll_answer, option: option, answer: "Uno")).to be_valid + expect(build(:poll_answer, option: option, answer: "Two")).not_to be_valid + expect(build(:poll_answer, option: option, answer: "Three")).not_to be_valid + expect(build(:poll_answer, option: option, answer: "Any")).not_to be_valid end end end diff --git a/spec/models/poll/partial_result_spec.rb b/spec/models/poll/partial_result_spec.rb index 2c05b69d2..429fb9393 100644 --- a/spec/models/poll/partial_result_spec.rb +++ b/spec/models/poll/partial_result_spec.rb @@ -2,17 +2,18 @@ require "rails_helper" describe Poll::PartialResult do describe "validations" do - it "validates that the answers are included in the Poll::Question's list" do + it "validates that the answers are included in the list of titles for the option" do question = create(:poll_question) - create(:poll_question_option, title: "One", question: question) + option = create(:poll_question_option, title_en: "One", title_es: "Uno", question: question) + create(:poll_question_option, title: "Two", question: question) - create(:poll_question_option, title: "Three", question: question) + create(:poll_question_option, title: "Three", question: create(:poll_question, poll: create(:poll))) - expect(build(:poll_partial_result, question: question, answer: "One")).to be_valid - expect(build(:poll_partial_result, question: question, answer: "Two")).to be_valid - expect(build(:poll_partial_result, question: question, answer: "Three")).to be_valid - - expect(build(:poll_partial_result, question: question, answer: "Four")).not_to be_valid + expect(build(:poll_partial_result, option: option, answer: "One")).to be_valid + expect(build(:poll_partial_result, option: option, answer: "Uno")).to be_valid + expect(build(:poll_partial_result, option: option, answer: "Two")).not_to be_valid + expect(build(:poll_partial_result, option: option, answer: "Three")).not_to be_valid + expect(build(:poll_partial_result, option: option, answer: "Any")).not_to be_valid end it "dynamically validates the valid origins" do @@ -21,6 +22,75 @@ describe Poll::PartialResult do expect(build(:poll_partial_result, origin: "custom")).to be_valid expect(build(:poll_partial_result, origin: "web")).not_to be_valid end + + describe "option_id uniqueness" do + let(:booth_assignment) { create(:poll_booth_assignment) } + + it "is not valid when there are two identical partial results" do + question = create(:poll_question_multiple, :abc) + option = question.question_options.first + + create(:poll_partial_result, + question: question, + booth_assignment: booth_assignment, + date: Date.current, + option: option, + answer: "Answer A") + + partial_result = build(:poll_partial_result, + question: question, + booth_assignment: booth_assignment, + date: Date.current, + option: option, + answer: "Answer A") + + expect(partial_result).not_to be_valid + expect { partial_result.save(validate: false) }.to raise_error ActiveRecord::RecordNotUnique + end + + it "is not valid when there are two results with the same option and different answer" do + question = create(:poll_question_multiple, :abc) + option = question.question_options.first + + create(:poll_partial_result, + question: question, + booth_assignment: booth_assignment, + date: Date.current, + option: option, + answer: "Answer A") + + partial_result = build(:poll_partial_result, + question: question, + booth_assignment: booth_assignment, + date: Date.current, + option: option, + answer: "Answer B") + + expect(partial_result).not_to be_valid + expect { partial_result.save(validate: false) }.to raise_error ActiveRecord::RecordNotUnique + end + + it "is valid when there are two identical results and the option is nil" do + question = create(:poll_question_multiple, :abc) + + create(:poll_partial_result, + question: question, + booth_assignment: booth_assignment, + date: Date.current, + option: nil, + answer: "Answer A") + + partial_result = build(:poll_partial_result, + question: question, + booth_assignment: booth_assignment, + date: Date.current, + option: nil, + answer: "Answer A") + + expect(partial_result).to be_valid + expect { partial_result.save }.not_to raise_error + end + end end describe "logging changes" do