diff --git a/app/views/admin/poll/results/_result.html.erb b/app/components/admin/poll/results/question_component.html.erb similarity index 58% rename from app/views/admin/poll/results/_result.html.erb rename to app/components/admin/poll/results/question_component.html.erb index 0f2600f4f..8a3d6633a 100644 --- a/app/views/admin/poll/results/_result.html.erb +++ b/app/components/admin/poll/results/question_component.html.erb @@ -1,5 +1,4 @@ -<% by_question = @partial_results.group_by(&:question_id) %> -<% @poll.questions.each do |question| %> +

<%= question.title %>

@@ -10,12 +9,11 @@ <% question.question_options.each_with_index do |option, i| %> - <% by_answer = by_question[question.id].present? ? by_question[question.id].group_by(&:answer) : {} %> - + <% end %>
<%= option.title %><%= by_answer[option.title].present? ? by_answer[option.title].sum(&:amount) : 0 %><%= votes_for(option) %>
-<% end %> +
diff --git a/app/components/admin/poll/results/question_component.rb b/app/components/admin/poll/results/question_component.rb new file mode 100644 index 000000000..a539e8d88 --- /dev/null +++ b/app/components/admin/poll/results/question_component.rb @@ -0,0 +1,12 @@ +class Admin::Poll::Results::QuestionComponent < ApplicationComponent + attr_reader :question, :partial_results + + def initialize(question, partial_results) + @question = question + @partial_results = partial_results + end + + def votes_for(option) + partial_results.where(option: option).sum(:amount) + end +end diff --git a/app/components/officing/results/index_component.html.erb b/app/components/officing/results/index_component.html.erb new file mode 100644 index 000000000..046f2319d --- /dev/null +++ b/app/components/officing/results/index_component.html.erb @@ -0,0 +1,41 @@ +<% provide :main_class, "officing-results-index" %> + +<%= back_link_to new_officing_poll_result_path(poll) %> +

<%= poll.name %> - <%= t("officing.results.index.results") %>

+ +<% if partial_results.present? %> +
+

+ <%= booth_assignment.booth.name %> - <%= l partial_results.first.date, format: :long %> +

+
+ +
+
+ + + + + + + + + + + + + + + +
<%= t("officing.results.index.table_whites") %><%= t("officing.results.index.table_nulls") %><%= t("officing.results.index.table_total") %>
<%= recounts.sum(:white_amount) %><%= recounts.sum(:null_amount) %><%= recounts.sum(:total_amount) %>
+ + <% @poll.questions.each do |question| %> + <%= render Admin::Poll::Results::QuestionComponent.new(question, @partial_results) %> + <% end %> +
+
+<% else %> +
+ <%= t("officing.results.index.no_results") %> +
+<% end %> diff --git a/app/components/officing/results/index_component.rb b/app/components/officing/results/index_component.rb new file mode 100644 index 000000000..e4e86a9fe --- /dev/null +++ b/app/components/officing/results/index_component.rb @@ -0,0 +1,10 @@ +class Officing::Results::IndexComponent < ApplicationComponent + attr_reader :poll, :partial_results, :booth_assignment, :recounts + + def initialize(poll, partial_results, booth_assignment, recounts) + @poll = poll + @partial_results = partial_results + @booth_assignment = booth_assignment + @recounts = recounts + end +end diff --git a/app/controllers/officing/base_controller.rb b/app/controllers/officing/base_controller.rb index 706ef166c..0ac95364b 100644 --- a/app/controllers/officing/base_controller.rb +++ b/app/controllers/officing/base_controller.rb @@ -13,12 +13,6 @@ class Officing::BaseController < ApplicationController raise CanCan::AccessDenied unless current_user&.poll_officer? end - def check_officer_assignment - if @officer_assignment.blank? - go_back_to_new(t("officing.results.flash.error_wrong_booth")) - end - end - def load_officer_assignment @officer_assignments ||= current_user.poll_officer.officer_assignments.where(date: Time.current.to_date) end diff --git a/app/controllers/officing/results_controller.rb b/app/controllers/officing/results_controller.rb index 4ebd7fba2..8aa69ae7d 100644 --- a/app/controllers/officing/results_controller.rb +++ b/app/controllers/officing/results_controller.rb @@ -38,21 +38,20 @@ class Officing::ResultsController < Officing::BaseController params[:questions].each_pair do |question_id, results| question = @poll.questions.find(question_id) - go_back_to_new if question.blank? - 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 - go_back_to_new if question.blank? + 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" @@ -79,10 +78,10 @@ class Officing::ResultsController < Officing::BaseController @results << recount end - def go_back_to_new(alert = nil) + def go_back_to_new(alert) params[:d] = Date.current params[:oa] = results_params[:officer_assignment_id] - flash.now[:alert] = (alert || t("officing.results.flash.error_create")) + flash.now[:alert] = alert load_officer_assignments load_partial_results render :new @@ -123,4 +122,10 @@ class Officing::ResultsController < Officing::BaseController def index_params params.permit(:booth_assignment_id, :date) end + + def check_officer_assignment + if @officer_assignment.blank? + go_back_to_new(t("officing.results.flash.error_wrong_booth")) + end + end end diff --git a/app/lib/poll_partial_result_option_finder.rb b/app/lib/poll_partial_result_option_finder.rb new file mode 100644 index 000000000..1bf64da83 --- /dev/null +++ b/app/lib/poll_partial_result_option_finder.rb @@ -0,0 +1,7 @@ +class PollPartialResultOptionFinder < PollOptionFinder + private + + def existing_choices + question.partial_results.where(option_id: nil).distinct.pluck(:answer) + end +end 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/app/views/admin/poll/booth_assignments/_results.html.erb b/app/views/admin/poll/booth_assignments/_results.html.erb index 535485096..d5a4b1fe2 100644 --- a/app/views/admin/poll/booth_assignments/_results.html.erb +++ b/app/views/admin/poll/booth_assignments/_results.html.erb @@ -7,6 +7,9 @@ <% else %> <%= render "admin/poll/results/recount", resource: @booth_assignment %> - <%= render "admin/poll/results/result" %> + + <% @poll.questions.each do |question| %> + <%= render Admin::Poll::Results::QuestionComponent.new(question, @partial_results) %> + <% end %> <% end %> diff --git a/app/views/admin/poll/results/index.html.erb b/app/views/admin/poll/results/index.html.erb index eaf6ad745..36d862246 100644 --- a/app/views/admin/poll/results/index.html.erb +++ b/app/views/admin/poll/results/index.html.erb @@ -13,7 +13,11 @@ <% if @partial_results.present? %> <%= render "recount", resource: @poll %> - <%= render "result" %> + + <% @poll.questions.each do |question| %> + <%= render Admin::Poll::Results::QuestionComponent.new(question, @partial_results) %> + <% end %> + <%= render "results_by_booth" %> <% end %> diff --git a/app/views/officing/results/index.html.erb b/app/views/officing/results/index.html.erb index 2b2d8ef15..946c5611d 100644 --- a/app/views/officing/results/index.html.erb +++ b/app/views/officing/results/index.html.erb @@ -1,58 +1 @@ -<%= back_link_to new_officing_poll_result_path(@poll) %> -

<%= @poll.name %> - <%= t("officing.results.index.results") %>

- -<% if @partial_results.present? %> -
-

- <%= @booth_assignment.booth.name %> - <%= l @partial_results.first.date, format: :long %> -

-
- -
-
- - - - - - - - - - - - - - - -
<%= t("officing.results.index.table_whites") %><%= t("officing.results.index.table_nulls") %><%= t("officing.results.index.table_total") %>
<%= @recounts.sum(:white_amount) %><%= @recounts.sum(:null_amount) %><%= @recounts.sum(:total_amount) %>
- - <% by_question = @partial_results.group_by(&:question_id) %> - <% @poll.questions.each do |question| %> -

<%= question.title %>

- - - - - - - - - - <% question.question_options.each_with_index do |option, i| %> - <% by_answer = by_question[question.id].present? ? by_question[question.id].group_by(&:answer) : {} %> - - - - - <% end %> - -
<%= t("officing.results.index.table_answer") %><%= t("officing.results.index.table_votes") %>
<%= option.title %><%= by_answer[option.title].present? ? by_answer[option.title].first.amount : 0 %>
- <% end %> -
-
-<% else %> -
- <%= t("officing.results.index.no_results") %> -
-<% end %> +<%= render Officing::Results::IndexComponent.new(@poll, @partial_results, @booth_assignment, @recounts) %> diff --git a/config/locales/en/officing.yml b/config/locales/en/officing.yml index 066c137ae..b31d11c0f 100644 --- a/config/locales/en/officing.yml +++ b/config/locales/en/officing.yml @@ -41,7 +41,6 @@ en: results: flash: create: "Results saved" - error_create: "Results NOT saved. Error in data." error_wrong_booth: "Wrong booth. Results NOT saved." new: title: "%{poll} - Add results" @@ -58,8 +57,6 @@ en: index: no_results: "No results" results: Results - table_answer: Answer - table_votes: Votes table_whites: "Totally blank ballots" table_nulls: "Invalid ballots" table_total: "Valid ballots" diff --git a/config/locales/es/officing.yml b/config/locales/es/officing.yml index f668ce8a2..f3cfbc89e 100644 --- a/config/locales/es/officing.yml +++ b/config/locales/es/officing.yml @@ -41,7 +41,6 @@ es: results: flash: create: "Datos guardados" - error_create: "Resultados NO añadidos. Error en los datos" error_wrong_booth: "Urna incorrecta. Resultados NO guardados." new: title: "%{poll} - Añadir resultados" @@ -58,8 +57,6 @@ es: index: no_results: "No hay resultados" results: Resultados - table_answer: Respuesta - table_votes: Votos table_whites: "Papeletas totalmente en blanco" table_nulls: "Papeletas nulas" table_total: "Papeletas válidas" 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/lib/tasks/consul.rake b/lib/tasks/consul.rake index a43bdaaf6..a6eb30dbf 100644 --- a/lib/tasks/consul.rake +++ b/lib/tasks/consul.rake @@ -3,8 +3,10 @@ namespace :consul do task execute_release_tasks: ["settings:rename_setting_keys", "settings:add_new_settings", "cache:clear", - "execute_release_2.3.0_tasks"] + "execute_release_2.4.0_tasks"] - desc "Runs tasks needed to upgrade from 2.2.2 to 2.3.0" - task "execute_release_2.3.0_tasks": [] + desc "Runs tasks needed to upgrade from 2.3.1 to 2.4.0" + task "execute_release_2.4.0_tasks": [ + "polls:populate_partial_results_option_id" + ] end diff --git a/lib/tasks/polls.rake b/lib/tasks/polls.rake index a3ce11e4b..2fb3a5adf 100644 --- a/lib/tasks/polls.rake +++ b/lib/tasks/polls.rake @@ -107,4 +107,95 @@ namespace :polls do end end end + + desc "Removes duplicate poll partial results" + task remove_duplicate_partial_results: :environment do + logger = ApplicationLogger.new + duplicate_records_logger = DuplicateRecordsLogger.new + + logger.info "Removing duplicate partial results in polls" + + Tenant.run_on_each do + Poll::Question.find_each do |question| + manageable_titles = PollPartialResultOptionFinder.new(question).manageable_choices.keys + + question.question_options.each do |option| + titles = option.translations.where(title: manageable_titles).select(:title).distinct + + groups = question.partial_results.where(option_id: nil, answer: titles) + .select(:booth_assignment_id, :date) + .group(:booth_assignment_id, :date) + .having("count(*) > 1") + .pluck(:booth_assignment_id, :date) + + groups.each do |booth_assignment_id, date| + partial_results = question.partial_results.where( + option_id: nil, + booth_assignment_id: booth_assignment_id, + date: date, + answer: titles + ) + + tenant_info = " on tenant #{Tenant.current_schema}" unless Tenant.default? + + amounts_by_id = partial_results.pluck(:id, :amount).to_h + if amounts_by_id.values.uniq.size > 1 + log_message = "Found duplicate partial results with different amounts " \ + "for question_id #{question.id}, " \ + "booth_assignment_id #{booth_assignment_id} " \ + "and date #{date}. " \ + "Keeping ID #{partial_results.first.id} " \ + "with amount #{partial_results.first.amount}. " \ + "Deleting partial results with these IDs and amounts: " \ + "#{amounts_by_id.except(partial_results.first.id)}" + tenant_info.to_s + logger.info(log_message) + duplicate_records_logger.info(log_message) + end + + partial_results.excluding(partial_results.first).each do |partial_result| + partial_result.delete + + log_message = "Deleted duplicate record with ID #{partial_result.id} " \ + "from the #{Poll::PartialResult.table_name} table " \ + "with question_id #{question.id}, " \ + "booth_assignment_id #{booth_assignment_id} " \ + "and date #{date}" + tenant_info.to_s + logger.info(log_message) + duplicate_records_logger.info(log_message) + end + end + end + end + end + end + + desc "populates the poll_partial_results option_id column" + task populate_partial_results_option_id: :remove_duplicate_partial_results do + logger = ApplicationLogger.new + logger.info "Updating option_id column in poll_partial_results" + + Tenant.run_on_each do + Poll::Question.find_each do |question| + option_finder = PollPartialResultOptionFinder.new(question) + + option_finder.manageable_choices.each do |choice, ids| + question.partial_results.where(option_id: nil, answer: choice).update_all(option_id: ids.first) + end + + option_finder.unmanageable_choices.each do |choice, ids| + tenant_info = " on tenant #{Tenant.current_schema}" unless Tenant.default? + + if ids.count == 0 + logger.warn "Skipping poll_partial_results with the text \"#{choice}\" for the poll_question " \ + "with ID #{question.id}. This question has no poll_question_answers " \ + "containing the text \"#{choice}\"" + tenant_info.to_s + else + logger.warn "Skipping poll_partial_results with the text \"#{choice}\" for the poll_question " \ + "with ID #{question.id}. The text \"#{choice}\" could refer to any of these " \ + "IDs in the poll_question_answers table: #{ids.join(", ")}" + tenant_info.to_s + end + end + end + end + end end diff --git a/spec/components/admin/poll/results/question_component_spec.rb b/spec/components/admin/poll/results/question_component_spec.rb new file mode 100644 index 000000000..dd366e759 --- /dev/null +++ b/spec/components/admin/poll/results/question_component_spec.rb @@ -0,0 +1,68 @@ +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?") } + 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) + + expect(page).to have_css "h3", text: "What do you want?" + expect(page).to have_css "th", text: "Answer" + expect(page).to have_css "th", text: "Votes" + end + + it "renders one row per option" do + render_inline Admin::Poll::Results::QuestionComponent.new(question, poll.partial_results) + + expect(page).to have_css "tbody tr", count: 2 + expect(page).to have_css "td", text: "Yes" + expect(page).to have_css "td", text: "No" + end + + 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) + + page.find("tr#question_#{question.id}_0_result") do |yes_result| + expect(yes_result).to have_css "td", text: "Yes" + expect(yes_result).to have_css "td", text: "3" + end + + page.find("tr#question_#{question.id}_1_result") do |no_result| + expect(no_result).to have_css "td", text: "No" + expect(no_result).to have_css "td", text: "5" + end + end + + it "shows 0 when an option has no partial results" do + render_inline Admin::Poll::Results::QuestionComponent.new(question, poll.partial_results) + + page.find("tr#question_#{question.id}_0_result") do |yes_result| + expect(yes_result).to have_css "td", text: "Yes" + expect(yes_result).to have_css "td", text: "0" + end + + page.find("tr#question_#{question.id}_1_result") do |no_result| + expect(no_result).to have_css "td", text: "No" + expect(no_result).to have_css "td", text: "0" + end + end + + it "ignores partial results from other questions" do + other_question = create(:poll_question, poll: poll) + create(:poll_question_option, question: other_question, title: "Yes") + create(:poll_question_option, question: other_question, title: "Irrelevant") + create(:poll_partial_result, question: other_question, answer: "Yes", amount: 9) + create(:poll_partial_result, question: other_question, answer: "Irrelevant", amount: 9) + + render_inline Admin::Poll::Results::QuestionComponent.new(question, poll.partial_results) + + expect(page).to have_css "td", text: "0", count: 2 + end +end 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..10ad6a93b 100644 --- a/spec/lib/tasks/polls_spec.rb +++ b/spec/lib/tasks/polls_spec.rb @@ -3,6 +3,8 @@ require "rails_helper" describe "polls tasks" do let(:poll) { create(:poll) } let(:user) { create(:user, :level_two) } + let(:booth_assignment) { create(:poll_booth_assignment) } + let(:other_booth_assignment) { create(:poll_booth_assignment) } describe "polls:remove_duplicate_answers" do before { Rake::Task["polls:remove_duplicate_answers"].reenable } @@ -42,7 +44,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 +128,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", @@ -222,4 +225,309 @@ describe "polls tasks" do end end end + + describe "polls:remove_duplicate_partial_results" do + before { Rake::Task["polls:remove_duplicate_partial_results"].reenable } + + it "removes duplicate partial results" do + question = create(:poll_question_multiple, :abcde, poll: poll, max_votes: 4) + + result_attributes = { + question_id: question.id, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A", + option_id: nil + } + other_result_attributes = result_attributes.merge(answer: "Answer B") + + result = create(:poll_partial_result, result_attributes) + other_result = create(:poll_partial_result, other_result_attributes) + other_booth_result = create(:poll_partial_result, + result_attributes.merge(booth_assignment_id: other_booth_assignment.id)) + + 2.times { insert(:poll_partial_result, result_attributes) } + insert(:poll_partial_result, other_result_attributes) + insert(:poll_partial_result, result_attributes.merge(booth_assignment_id: other_booth_assignment.id)) + + expect(Poll::PartialResult.count).to eq 7 + + Rake.application.invoke_task("polls:remove_duplicate_partial_results") + + expect(Poll::PartialResult.count).to eq 3 + expect(Poll::PartialResult.all).to match_array [result, other_result, other_booth_result] + end + + it "removes duplicate partial results in different languages" do + question = create(:poll_question_multiple, max_votes: 2) + + create(:poll_question_option, question: question, title_en: "Yes", title_de: "Ja") + create(:poll_question_option, question: question, title_en: "No", title_de: "Nein") + create(:poll_question_option, question: question, title_en: "Maybe", title_de: "Vielleicht") + + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Yes", + option: nil) + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Ja", + option: nil) + + expect(Poll::PartialResult.count).to eq 2 + + Rake.application.invoke_task("polls:remove_duplicate_partial_results") + + expect(Poll::PartialResult.count).to eq 1 + end + + it "does not remove duplicate partial results when many options are possible" do + question = create(:poll_question, title: "How do you pronounce it?") + + create(:poll_question_option, question: question, title_en: "A", title_es: "EI") + create(:poll_question_option, question: question, title_en: "E", title_es: "I") + create(:poll_question_option, question: question, title_en: "I", title_es: "AI") + + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "I", + option: nil) + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "AI", + option: nil) + + expect(Poll::PartialResult.count).to eq 2 + + Rake.application.invoke_task("polls:remove_duplicate_partial_results") + + expect(Poll::PartialResult.count).to eq 2 + end + + it "does not remove partial results with the same text and different options" do + question = create(:poll_question_multiple, :abcde, max_votes: 4) + option_a = question.question_options.find_by(title: "Answer A") + option_b = question.question_options.find_by(title: "Answer B") + + result_attributes = { + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A" + } + create(:poll_partial_result, result_attributes.merge(option: option_a)) + insert(:poll_partial_result, result_attributes.merge(option_id: option_b.id)) + + expect(Poll::PartialResult.count).to eq 2 + + Rake.application.invoke_task("polls:remove_duplicate_partial_results") + + expect(Poll::PartialResult.count).to eq 2 + end + + it "removes duplicate partial results on tenants" do + create(:tenant, schema: "partial_results") + + Tenant.switch("partial_results") do + question = create(:poll_question_multiple, :abc) + booth_assignment = create(:poll_booth_assignment) + + result_attributes = { + question_id: question.id, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A", + option_id: nil + } + create(:poll_partial_result, result_attributes) + insert(:poll_partial_result, result_attributes) + + expect(Poll::PartialResult.count).to eq 2 + end + + Rake.application.invoke_task("polls:remove_duplicate_partial_results") + + Tenant.switch("partial_results") do + expect(Poll::PartialResult.count).to eq 1 + end + end + + it "removes duplicates in different languages even when amounts differ" do + question = create(:poll_question_multiple, max_votes: 2) + + create(:poll_question_option, question: question, title_en: "Yes", title_de: "Ja") + + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Yes", + option: nil, + amount: 3) + + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Ja", + option: nil, + amount: 5) + + expect(Poll::PartialResult.count).to eq 2 + + Rake.application.invoke_task("polls:remove_duplicate_partial_results") + + expect(Poll::PartialResult.count).to eq 1 + end + end + + describe "polls:populate_partial_results_option_id" do + before do + Rake::Task["polls:remove_duplicate_partial_results"].reenable + Rake::Task["polls:populate_partial_results_option_id"].reenable + end + + it "populates the option_id column of existing partial results when there's one valid option" do + yes_no_question = create(:poll_question, :yes_no, poll: poll) + abc_question = create(:poll_question_multiple, :abc, poll: poll) + option_a = abc_question.question_options.find_by(title: "Answer A") + option_b = abc_question.question_options.find_by(title: "Answer B") + + result = create(:poll_partial_result, + question: yes_no_question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Yes", + option: nil) + abc_result = create(:poll_partial_result, + question: abc_question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A", + option: nil) + insert(:poll_partial_result, + question: abc_question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A", + option_id: option_b.id) + inconsistent_result = Poll::PartialResult.find_by!(option: option_b) + invalid_result = build(:poll_partial_result, + question: abc_question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Non existing", + option: nil) + invalid_result.save!(validate: false) + + Rake.application.invoke_task("polls:populate_partial_results_option_id") + result.reload + abc_result.reload + inconsistent_result.reload + invalid_result.reload + + expect(result.option_id).to eq yes_no_question.question_options.find_by(title: "Yes").id + expect(abc_result.option_id).to eq option_a.id + expect(inconsistent_result.option_id).to eq option_b.id + expect(invalid_result.option_id).to be nil + end + + it "does not populate the option_id column when there are several valid options" do + question = create(:poll_question, title: "How do you pronounce it?") + + create(:poll_question_option, question: question, title_en: "A", title_es: "EI") + create(:poll_question_option, question: question, title_en: "E", title_es: "I") + create(:poll_question_option, question: question, title_en: "I", title_es: "AI") + + result = create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "I", + option: nil) + + Rake.application.invoke_task("polls:populate_partial_results_option_id") + result.reload + + expect(result.option_id).to be nil + end + + it "removes duplicate partial results before populating the option_id column" do + question = create(:poll_question_multiple, :abc) + + localized_question = create(:poll_question_multiple) + create(:poll_question_option, question: localized_question, title_en: "Yes", title_de: "Ja") + create(:poll_question_option, question: localized_question, title_en: "No", title_de: "Nein") + create(:poll_question_option, question: localized_question, title_en: "Maybe", title_de: "Vielleicht") + + result_attributes = { + question_id: question.id, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A", + option_id: nil + } + result = create(:poll_partial_result, result_attributes) + insert(:poll_partial_result, result_attributes) + + localized_result_attributes = { + question: localized_question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + option: nil + } + localized_result = create(:poll_partial_result, localized_result_attributes.merge(answer: "Yes")) + create(:poll_partial_result, localized_result_attributes.merge(answer: "Ja")) + + result.reload + localized_result.reload + + expect(result.option_id).to be nil + expect(localized_result.option_id).to be nil + + Rake.application.invoke_task("polls:populate_partial_results_option_id") + result.reload + localized_result.reload + + expect(Poll::PartialResult.count).to eq 2 + expect(result.option_id).to eq question.question_options.find_by(title: "Answer A").id + expect(localized_result.option_id).to eq localized_question.question_options.find_by(title: "Yes").id + end + + it "populates the option_id column on tenants" do + create(:tenant, schema: "partial_results") + + Tenant.switch("partial_results") do + question = create(:poll_question_multiple, :abc) + booth_assignment = create(:poll_booth_assignment) + + create(:poll_partial_result, + question: question, + booth_assignment_id: booth_assignment.id, + date: Date.current, + answer: "Answer A", + option: nil) + end + + Rake.application.invoke_task("polls:populate_partial_results_option_id") + + Tenant.switch("partial_results") do + expect(Poll::Question.count).to eq 1 + expect(Poll::PartialResult.count).to eq 1 + + question = Poll::Question.first + option_a = question.question_options.find_by(title: "Answer A") + + expect(Poll::PartialResult.first.option_id).to eq option_a.id + end + end + end end 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 diff --git a/spec/system/admin/poll/booth_assigments_spec.rb b/spec/system/admin/poll/booth_assigments_spec.rb index 0402b0d39..3548c57f6 100644 --- a/spec/system/admin/poll/booth_assigments_spec.rb +++ b/spec/system/admin/poll/booth_assigments_spec.rb @@ -228,7 +228,7 @@ describe "Admin booths assignments", :admin do expect(page).not_to have_css "#recounts_list" end - scenario "Results for a booth assignment" do + scenario "Recounts for a booth assignment" do poll = create(:poll) booth_assignment = create(:poll_booth_assignment, poll: poll) other_booth_assignment = create(:poll_booth_assignment, poll: poll) @@ -285,30 +285,6 @@ describe "Admin booths assignments", :admin do click_link "Results" - expect(page).to have_content(question_1.title) - - within("#question_#{question_1.id}_0_result") do - expect(page).to have_content("Yes") - expect(page).to have_content(11) - end - - within("#question_#{question_1.id}_1_result") do - expect(page).to have_content("No") - expect(page).to have_content(4) - end - - expect(page).to have_content(question_2.title) - - within("#question_#{question_2.id}_0_result") do - expect(page).to have_content("Today") - expect(page).to have_content(5) - end - - within("#question_#{question_2.id}_1_result") do - expect(page).to have_content("Tomorrow") - expect(page).to have_content(6) - end - within("#white_results") { expect(page).to have_content("21") } within("#null_results") { expect(page).to have_content("44") } within("#total_results") { expect(page).to have_content("66") } diff --git a/spec/system/admin/poll/polls_spec.rb b/spec/system/admin/poll/polls_spec.rb index 5559679f8..52c44137d 100644 --- a/spec/system/admin/poll/polls_spec.rb +++ b/spec/system/admin/poll/polls_spec.rb @@ -343,46 +343,6 @@ describe "Admin polls", :admin do expect(page).to have_content "There are no results" end - scenario "Show partial results" do - poll = create(:poll) - - booth_assignment_1 = create(:poll_booth_assignment, poll: poll) - booth_assignment_2 = create(:poll_booth_assignment, poll: poll) - booth_assignment_3 = create(:poll_booth_assignment, poll: poll) - - question_1 = create(:poll_question, poll: poll) - create(:poll_question_option, title: "Oui", question: question_1) - create(:poll_question_option, title: "Non", question: question_1) - - question_2 = create(:poll_question, poll: poll) - create(:poll_question_option, title: "Aujourd'hui", question: question_2) - create(:poll_question_option, title: "Demain", question: question_2) - - [booth_assignment_1, booth_assignment_2, booth_assignment_3].each do |ba| - create(:poll_partial_result, - booth_assignment: ba, - question: question_1, - answer: "Oui", - amount: 11) - - create(:poll_partial_result, - booth_assignment: ba, - question: question_2, - answer: "Demain", - amount: 5) - end - - create(:poll_recount, - booth_assignment: booth_assignment_1, - white_amount: 21, - null_amount: 44, - total_amount: 66) - - visit admin_poll_results_path(poll) - - expect(page).to have_content "Results by booth" - end - scenario "Enable stats and results for booth polls" do unvoted_poll = create(:poll) @@ -418,8 +378,9 @@ describe "Admin polls", :admin do expect(page).not_to have_content "Results by booth" end - scenario "Results by answer" do + scenario "Show results, recount and details by booth" do poll = create(:poll) + booth_assignment_1 = create(:poll_booth_assignment, poll: poll) booth_assignment_2 = create(:poll_booth_assignment, poll: poll) booth_assignment_3 = create(:poll_booth_assignment, poll: poll) @@ -427,7 +388,7 @@ describe "Admin polls", :admin do question_1 = create(:poll_question, :yes_no, poll: poll) question_2 = create(:poll_question, poll: poll) - create(:poll_question_option, title: "Today", question: question_2) + create(:poll_question_option, title: "Today", question: question_2) create(:poll_question_option, title: "Tomorrow", question: question_2) [booth_assignment_1, booth_assignment_2, booth_assignment_3].each do |ba| @@ -436,12 +397,14 @@ describe "Admin polls", :admin do question: question_1, answer: "Yes", amount: 11) + create(:poll_partial_result, booth_assignment: ba, question: question_2, answer: "Tomorrow", amount: 5) end + create(:poll_recount, booth_assignment: booth_assignment_1, white_amount: 21, @@ -449,60 +412,21 @@ describe "Admin polls", :admin do total_amount: 66) visit admin_poll_path(poll) - click_link "Results" - expect(page).to have_content(question_1.title) - question_1.question_options.each_with_index do |option, i| - within("#question_#{question_1.id}_#{i}_result") do - expect(page).to have_content(option.title) - expect(page).to have_content([33, 0][i]) - end - end - - expect(page).to have_content(question_2.title) - question_2.question_options.each_with_index do |option, i| - within("#question_#{question_2.id}_#{i}_result") do - expect(page).to have_content(option.title) - expect(page).to have_content([0, 15][i]) - end - end + expect(page).to have_content "Results by booth" within("#white_results") { expect(page).to have_content("21") } within("#null_results") { expect(page).to have_content("44") } within("#total_results") { expect(page).to have_content("66") } - end - scenario "Link to results by booth" do - poll = create(:poll) - booth_assignment1 = create(:poll_booth_assignment, poll: poll) - booth_assignment2 = create(:poll_booth_assignment, poll: poll) + expect(page).to have_link("See results", count: 3) - question = create(:poll_question, :yes_no, poll: poll) - - create(:poll_partial_result, - booth_assignment: booth_assignment1, - question: question, - answer: "Yes", - amount: 5) - - create(:poll_partial_result, - booth_assignment: booth_assignment2, - question: question, - answer: "Yes", - amount: 6) - - visit admin_poll_path(poll) - - click_link "Results" - - expect(page).to have_link("See results", count: 2) - - within("#booth_assignment_#{booth_assignment1.id}_result") do + within("#booth_assignment_#{booth_assignment_1.id}_result") do click_link "See results" end - expect(page).to have_content booth_assignment1.booth.name + expect(page).to have_content booth_assignment_1.booth.name expect(page).to have_content "Results" expect(page).to have_content "Yes" expect(page).to have_content "5" diff --git a/spec/system/officing/results_spec.rb b/spec/system/officing/results_spec.rb index 9eb49ad59..e17da5857 100644 --- a/spec/system/officing/results_spec.rb +++ b/spec/system/officing/results_spec.rb @@ -123,6 +123,9 @@ describe "Officing Results", :with_frozen_time do click_link "See results" end + expect(page).to have_content(I18n.l(Date.current.to_date, format: :long)) + expect(page).to have_content(booth_name) + within("#question_#{question_1.id}_0_result") do expect(page).to have_content("5555") expect(page).not_to have_content("7777") @@ -134,50 +137,4 @@ describe "Officing Results", :with_frozen_time do within("#question_#{question_1.id}_0_result") { expect(page).to have_content("5555") } within("#question_#{question_1.id}_1_result") { expect(page).to have_content("200") } end - - scenario "Index lists all questions and answers" do - officer_assignment = poll_officer.officer_assignments.first - booth_assignment = officer_assignment.booth_assignment - booth = booth_assignment.booth - - create( - :poll_partial_result, - officer_assignment: officer_assignment, - booth_assignment: booth_assignment, - date: poll.ends_at, - question: question_1, - amount: 33 - ) - - create( - :poll_recount, - officer_assignment: officer_assignment, - booth_assignment: booth_assignment, - date: poll.ends_at, - white_amount: 21, - null_amount: 44, - total_amount: 66 - ) - - visit officing_poll_results_path(poll, - date: I18n.l(poll.ends_at.to_date), - booth_assignment_id: officer_assignment.booth_assignment_id) - - expect(page).to have_content(I18n.l(poll.ends_at.to_date, format: :long)) - expect(page).to have_content(booth.name) - - expect(page).to have_content(question_1.title) - question_1.question_options.each_with_index do |answer, i| - within("#question_#{question_1.id}_#{i}_result") { expect(page).to have_content(answer.title) } - end - - expect(page).to have_content(question_2.title) - question_2.question_options.each_with_index do |answer, i| - within("#question_#{question_2.id}_#{i}_result") { expect(page).to have_content(answer.title) } - end - - within("#white_results") { expect(page).to have_content("21") } - within("#null_results") { expect(page).to have_content("44") } - within("#total_results") { expect(page).to have_content("66") } - end end