Add option_id to partial results and unique index

Similar to what we did in PR "Avoid duplicate records in poll answers" 5539,
specifically in commit 503369166, we want to stop relying on the plain text
"answer" and start using "option_id" to avoid issues with counts across
translations and to add consistency to the poll_partial_results table.

Note that we also moved the `possible_answers` method from Poll::Question to
Poll::Question::Option, since the list of valid answers really comes from the
options of a question and not from the question itself. Tests were updated
to validate answers against the translations of the assigned option.

Additionally, we renamed lambda parameters in validations to improve clarity.
This commit is contained in:
taitus
2025-09-10 12:22:08 +02:00
parent f2153f2b4d
commit a29eeaf2e2
13 changed files with 137 additions and 51 deletions

View File

@@ -7,13 +7,6 @@ class Admin::Poll::Results::QuestionComponent < ApplicationComponent
end end
def votes_for(option) def votes_for(option)
grouped = by_answer[option.title] || [] partial_results.where(option: option).sum(:amount)
grouped.sum(&:amount)
end end
private
def by_answer
@by_answer ||= partial_results.where(question: question).group_by(&:answer)
end
end end

View File

@@ -39,18 +39,19 @@ class Officing::ResultsController < Officing::BaseController
params[:questions].each_pair do |question_id, results| params[:questions].each_pair do |question_id, results|
question = @poll.questions.find(question_id) question = @poll.questions.find(question_id)
results.each_pair do |answer_index, count| results.each_pair do |option_index, count|
next if count.blank? 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( partial_result = ::Poll::PartialResult.find_or_initialize_by(
booth_assignment_id: @officer_assignment.booth_assignment_id, booth_assignment_id: @officer_assignment.booth_assignment_id,
date: Date.current, date: Date.current,
question_id: question_id, question_id: question_id,
answer: answer option_id: option.id
) )
partial_result.officer_assignment_id = @officer_assignment.id partial_result.officer_assignment_id = @officer_assignment.id
partial_result.answer = option.title
partial_result.amount = count.to_i partial_result.amount = count.to_i
partial_result.author = current_user partial_result.author = current_user
partial_result.origin = "booth" partial_result.origin = "booth"

View File

@@ -11,8 +11,8 @@ class Poll::Answer < ApplicationRecord
validates :option, uniqueness: { scope: :author_id }, allow_nil: true validates :option, uniqueness: { scope: :author_id }, allow_nil: true
validate :max_votes validate :max_votes
validates :answer, inclusion: { in: ->(a) { a.question.possible_answers }}, validates :answer, inclusion: { in: ->(poll_answer) { poll_answer.option.possible_answers }},
unless: ->(a) { a.question.blank? } if: ->(poll_answer) { poll_answer.option.present? }
scope :by_author, ->(author_id) { where(author_id: author_id) } scope :by_author, ->(author_id) { where(author_id: author_id) }
scope :by_question, ->(question_id) { where(question_id: question_id) } scope :by_question, ->(question_id) { where(question_id: question_id) }

View File

@@ -5,13 +5,15 @@ class Poll::PartialResult < ApplicationRecord
belongs_to :author, -> { with_hidden }, class_name: "User", inverse_of: :poll_partial_results belongs_to :author, -> { with_hidden }, class_name: "User", inverse_of: :poll_partial_results
belongs_to :booth_assignment belongs_to :booth_assignment
belongs_to :officer_assignment belongs_to :officer_assignment
belongs_to :option, class_name: "Poll::Question::Option"
validates :question, presence: true validates :question, presence: true
validates :author, presence: true validates :author, presence: true
validates :answer, presence: true validates :answer, presence: true
validates :answer, inclusion: { in: ->(a) { a.question.possible_answers }}, validates :answer, inclusion: { in: ->(partial_result) { partial_result.option.possible_answers }},
unless: ->(a) { a.question.blank? } if: ->(partial_result) { partial_result.option.present? }
validates :origin, inclusion: { in: ->(*) { VALID_ORIGINS }} 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_author, ->(author_id) { where(author_id: author_id) }
scope :by_question, ->(question_id) { where(question_id: question_id) } scope :by_question, ->(question_id) { where(question_id: question_id) }

View File

@@ -49,10 +49,6 @@ class Poll::Question < ApplicationRecord
question_options.max_by(&:total_votes)&.id question_options.max_by(&:total_votes)&.id
end end
def possible_answers
question_options.joins(:translations).pluck(:title)
end
def options_with_read_more? def options_with_read_more?
options_with_read_more.any? options_with_read_more.any?
end end

View File

@@ -12,6 +12,7 @@ class Poll::Question::Option < ApplicationRecord
belongs_to :question, class_name: "Poll::Question" belongs_to :question, class_name: "Poll::Question"
has_many :answers, class_name: "Poll::Answer", dependent: :nullify 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", has_many :videos, class_name: "Poll::Question::Option::Video",
dependent: :destroy, dependent: :destroy,
foreign_key: "answer_id", foreign_key: "answer_id",
@@ -50,4 +51,8 @@ class Poll::Question::Option < ApplicationRecord
def with_read_more? def with_read_more?
description.present? || images.any? || documents.any? || videos.any? description.present? || images.any? || documents.any? || videos.any?
end end
def possible_answers
translations.pluck(:title)
end
end end

View File

@@ -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

View File

@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm" enable_extension "pg_trgm"
enable_extension "plpgsql" 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 "amount_log", default: ""
t.text "officer_assignment_id_log", default: "" t.text "officer_assignment_id_log", default: ""
t.text "author_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 ["answer"], name: "index_poll_partial_results_on_answer"
t.index ["author_id"], name: "index_poll_partial_results_on_author_id" 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 ["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 ["origin"], name: "index_poll_partial_results_on_origin"
t.index ["question_id"], name: "index_poll_partial_results_on_question_id" t.index ["question_id"], name: "index_poll_partial_results_on_question_id"
end 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_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_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_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", "poll_questions", column: "question_id"
add_foreign_key "poll_partial_results", "users", column: "author_id" add_foreign_key "poll_partial_results", "users", column: "author_id"
add_foreign_key "poll_question_answer_videos", "poll_question_answers", column: "answer_id" add_foreign_key "poll_question_answer_videos", "poll_question_answers", column: "answer_id"

View File

@@ -3,11 +3,8 @@ require "rails_helper"
describe Admin::Poll::Results::QuestionComponent do describe Admin::Poll::Results::QuestionComponent do
let(:poll) { create(:poll) } let(:poll) { create(:poll) }
let(:question) { create(:poll_question, poll: poll, title: "What do you want?") } let(:question) { create(:poll_question, poll: poll, title: "What do you want?") }
let!(:option_yes) { create(:poll_question_option, question: question, title: "Yes") }
before do let!(:option_no) { create(:poll_question_option, question: question, title: "No") }
create(:poll_question_option, question: question, title: "Yes")
create(:poll_question_option, question: question, title: "No")
end
it "renders question title and headers" do it "renders question title and headers" do
render_inline Admin::Poll::Results::QuestionComponent.new(question, poll.partial_results) 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" expect(page).to have_css "td", text: "No"
end end
it "sums votes by answer title" do it "sums votes by option" do
create(:poll_partial_result, question: question, answer: "Yes", amount: 2) create(:poll_partial_result, question: question, option: option_yes, amount: 2, date: Date.current)
create(:poll_partial_result, question: question, answer: "Yes", amount: 1) create(:poll_partial_result, question: question, option: option_yes, amount: 1, date: Date.yesterday)
create(:poll_partial_result, question: question, answer: "No", amount: 5) create(:poll_partial_result, question: question, option: option_no, amount: 5)
render_inline Admin::Poll::Results::QuestionComponent.new(question, question.partial_results) render_inline Admin::Poll::Results::QuestionComponent.new(question, question.partial_results)

View File

@@ -226,7 +226,14 @@ FactoryBot.define do
question factory: [:poll_question, :yes_no] question factory: [:poll_question, :yes_no]
author factory: :user author factory: :user
origin { "web" } 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 end
factory :poll_recount, class: "Poll::Recount" do factory :poll_recount, class: "Poll::Recount" do

View File

@@ -42,7 +42,7 @@ describe "polls tasks" do
answer_attributes = { question: question, author: user, answer: "Answer A" } 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_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 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) 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) 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, insert(:poll_answer, question: abc_question,
author: user, author: user,
answer: "Answer A", answer: "Answer A",
option: option_b) 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, answer_with_invalid_option = build(:poll_answer, question: abc_question,
author: user, author: user,
answer: "Non existing", answer: "Non existing",

View File

@@ -89,17 +89,18 @@ describe Poll::Answer do
expect { answer.save }.not_to raise_error expect { answer.save }.not_to raise_error
end 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) 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: "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, option: option, answer: "One")).to be_valid
expect(build(:poll_answer, question: question, answer: "Two")).to be_valid expect(build(:poll_answer, option: option, answer: "Uno")).to be_valid
expect(build(:poll_answer, question: question, answer: "Three")).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, question: question, answer: "Four")).not_to be_valid expect(build(:poll_answer, option: option, answer: "Any")).not_to be_valid
end end
end end
end end

View File

@@ -2,17 +2,18 @@ require "rails_helper"
describe Poll::PartialResult do describe Poll::PartialResult do
describe "validations" 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) 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: "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, option: option, answer: "One")).to be_valid
expect(build(:poll_partial_result, question: question, answer: "Two")).to be_valid expect(build(:poll_partial_result, option: option, answer: "Uno")).to be_valid
expect(build(:poll_partial_result, question: question, answer: "Three")).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, question: question, answer: "Four")).not_to be_valid expect(build(:poll_partial_result, option: option, answer: "Any")).not_to be_valid
end end
it "dynamically validates the valid origins" do 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: "custom")).to be_valid
expect(build(:poll_partial_result, origin: "web")).not_to be_valid expect(build(:poll_partial_result, origin: "web")).not_to be_valid
end 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 end
describe "logging changes" do describe "logging changes" do