consul/nairobi
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #5539 from consuldemocracy/add_option_id_to_poll_answers

Browse Source 
Avoid duplicate records in poll answers
This commit is contained in:
Javi Martín
2024-06-27 15:35:24 +02:00
committed by GitHub
parent e9377c1536 5dbd2ede14
commit 20c55da48b
18 changed files with 456 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/components/polls/questions/options_component.html.erb
View File

@@ -11,7 +11,7 @@
"aria-pressed": true %>
<% else %>
<%= button_to question_option.title,
answer_question_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",

21
app/controllers/polls/answers_controller.rb
View File

@@ -2,7 +2,24 @@ class Polls::AnswersController < ApplicationController
load_and_authorize_resource :question, class: "::Poll::Question"
load_and_authorize_resource :answer, class: "::Poll::Answer",
through: :question,
through_association: :answers
through_association: :answers,
only: :destroy
def create
authorize! :answer, @question
answer = @question.find_or_initialize_user_answer(current_user, params[:option_id])
answer.save_and_record_voter_participation
respond_to do |format|
format.html do
redirect_to request.referer
end
format.js do
render :show
end
end
end
def destroy
@answer.destroy_and_remove_voter_participation
@@ -12,7 +29,7 @@ class Polls::AnswersController < ApplicationController
redirect_to request.referer
end
format.js do
render "polls/questions/options"
render :show
end
end
end

20
app/controllers/polls/questions_controller.rb
View File

@@ -1,20 +0,0 @@
class Polls::QuestionsController < ApplicationController
load_and_authorize_resource :poll
load_and_authorize_resource :question, class: "Poll::Question"
has_orders %w[most_voted newest oldest], only: :show
def answer
answer = @question.find_or_initialize_user_answer(current_user, params[:answer])
answer.save_and_record_voter_participation
respond_to do |format|
format.html do
redirect_to request.referer
end
format.js do
render :options
end
end
end
end

31
app/lib/poll_option_finder.rb Normal file
View File

@@ -0,0 +1,31 @@
class PollOptionFinder
attr_reader :question
def initialize(question)
@question = question
end
def manageable_choices
choices_map.select { |choice, ids| ids.count == 1 }
end
def unmanageable_choices
choices_map.reject { |choice, ids| ids.count == 1 }
end
private
def choices_map
@choices_map ||= existing_choices.to_h do |choice|
[choice, options.where("lower(title) = lower(?)", choice).distinct.ids]
end
end
def options
question.question_options.joins(:translations).reorder(:id)
end
def existing_choices
question.answers.where(option_id: nil).distinct.pluck(:answer)
end
end

4
app/models/poll/answer.rb
View File

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

13
app/models/poll/question.rb
View File

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

1
app/models/poll/question/option.rb
View File

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

0
app/views/polls/questions/options.js.erb → app/views/polls/answers/show.js.erb
View File

5
config/routes/poll.rb
View File

@@ -4,9 +4,8 @@ resources :polls, only: [:show, :index] do
get :results
end
resources :questions, controller: "polls/questions", shallow: true do
post :answer, on: :member
resources :answers, controller: "polls/answers", only: :destroy, shallow: false
resources :questions, controller: "polls/questions", shallow: true, only: [] do
resources :answers, controller: "polls/answers", only: [:create, :destroy], shallow: false
end
end

9
db/migrate/20240513135357_add_question_answer_id_to_poll_answers.rb Normal file
View File

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

6
db/schema.rb
View File

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

3
lib/tasks/consul.rake
View File

@@ -8,6 +8,7 @@ namespace :consul do
desc "Runs tasks needed to upgrade from 2.1.1 to 2.2.0"
task "execute_release_2.2.0_tasks": [
"db:mask_ips",
"polls:remove_duplicate_voters"
"polls:remove_duplicate_voters",
"polls:populate_option_id"
]
end

79
lib/tasks/polls.rake
View File

@@ -28,4 +28,83 @@ namespace :polls do
end
end
end
desc "Removes duplicate poll answers"
task remove_duplicate_answers: :environment do
logger = ApplicationLogger.new
duplicate_records_logger = DuplicateRecordsLogger.new
logger.info "Removing duplicate answers in polls"
Tenant.run_on_each do
Poll::Question.find_each do |question|
manageable_titles = PollOptionFinder.new(question).manageable_choices.keys
question.question_options.each do |option|
titles = option.translations.where(title: manageable_titles).select(:title).distinct
author_ids = question.answers
.where(answer: titles)
.select(:author_id)
.group(:author_id)
.having("count(*) > 1")
.pluck(:author_id)
author_ids.each do |author_id|
poll_answers = question.answers.where(option_id: nil, answer: titles, author_id: author_id)
poll_answers.excluding(poll_answers.first).each do |poll_answer|
poll_answer.delete
tenant_info = " on tenant #{Tenant.current_schema}" unless Tenant.default?
log_message = "Deleted duplicate record with ID #{poll_answer.id} " \
"from the #{Poll::Answer.table_name} table " \
"with question_id #{question.id}, " \
"author_id #{author_id} " \
"and answer #{poll_answer.answer}" + tenant_info.to_s
logger.info(log_message)
duplicate_records_logger.info(log_message)
end
end
end
end
end
end
desc "populates the poll answers option_id column"
task populate_option_id: :remove_duplicate_answers do
logger = ApplicationLogger.new
logger.info "Updating option_id column in poll_answers"
Tenant.run_on_each do
questions = Poll::Question.select do |question|
# Change this if you've added a custom votation type
# to your Consul Democracy installation that implies
# choosing among a few given options
question.unique? || question.multiple?
end
questions.each do |question|
option_finder = PollOptionFinder.new(question)
option_finder.manageable_choices.each do |choice, ids|
question.answers.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_answers 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_answers 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

25
spec/controllers/polls/answers_controller_spec.rb Normal file
View File

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

11
spec/factories/polls.rb
View File

@@ -66,13 +66,21 @@ FactoryBot.define do
end
trait :abc do
after(:create) do |question, evaluator|
after(:create) do |question|
%w[A B C].each do |letter|
create(:poll_question_option, question: question, title: "Answer #{letter}")
end
end
end
trait :abcde do
after(:create) do |question|
%w[A B C D E].each do |letter|
create(:poll_question_option, question: question, title: "Answer #{letter}")
end
end
end
factory :poll_question_unique do
after(:create) do |question|
create(:votation_type_unique, questionable: question)
@@ -205,6 +213,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

219
spec/lib/tasks/polls_spec.rb
View File

@@ -46,4 +46,223 @@ describe "polls tasks" do
end
end
end
describe "polls:remove_duplicate_answers" do
before { Rake::Task["polls:remove_duplicate_answers"].reenable }
it "removes duplicate answers" do
question = create(:poll_question_multiple, :abcde, poll: poll, max_votes: 4)
abc_question = create(:poll_question_multiple, :abc, poll: poll)
answer_attributes = {
question_id: question.id,
author_id: user.id,
answer: "Answer A",
option_id: nil
}
abc_answer_attributes = answer_attributes.merge(question_id: abc_question.id, answer: "Answer B")
answer = create(:poll_answer, answer_attributes)
other_answer = create(:poll_answer, answer_attributes.merge(answer: "Answer B"))
other_user_answer = create(:poll_answer, answer_attributes.merge(author_id: create(:user).id))
abc_answer = create(:poll_answer, abc_answer_attributes)
2.times { insert(:poll_answer, answer_attributes) }
insert(:poll_answer, abc_answer_attributes)
expect(Poll::Answer.count).to eq 7
Rake.application.invoke_task("polls:remove_duplicate_answers")
expect(Poll::Answer.count).to eq 4
expect(Poll::Answer.all).to match_array [answer, other_answer, other_user_answer, abc_answer]
end
it "does not remove answers 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")
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))
expect(Poll::Answer.count).to eq 2
Rake.application.invoke_task("polls:remove_duplicate_answers")
expect(Poll::Answer.count).to eq 2
end
it "removes duplicate answers 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_answer, author: user, question: question, answer: "Yes", option: nil)
create(:poll_answer, author: user, question: question, answer: "Ja", option: nil)
expect(Poll::Answer.count).to eq 2
Rake.application.invoke_task("polls:remove_duplicate_answers")
expect(Poll::Answer.count).to eq 1
end
it "does not remove duplicate answers when many options are possible" do
question = create(:poll_question_multiple, title: "How do you pronounce it?", max_votes: 2)
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_answer, question: question, author: user, answer: "I", option: nil)
create(:poll_answer, question: question, author: user, answer: "AI", option: nil)
expect(Poll::Answer.count).to eq 2
Rake.application.invoke_task("polls:remove_duplicate_answers")
expect(Poll::Answer.count).to eq 2
end
it "removes duplicate answers on tenants" do
create(:tenant, schema: "answers")
Tenant.switch("answers") do
user = create(:user, :level_two)
question = create(:poll_question_multiple, :abc)
answer_attributes = {
question_id: question.id,
author_id: user.id,
answer: "Answer A",
option_id: nil
}
create(:poll_answer, answer_attributes)
insert(:poll_answer, answer_attributes)
expect(Poll::Answer.count).to eq 2
end
Rake.application.invoke_task("polls:remove_duplicate_answers")
Tenant.switch("answers") do
expect(Poll::Answer.count).to eq 1
end
end
end
describe "polls:populate_option_id" do
before do
Rake::Task["polls:remove_duplicate_answers"].reenable
Rake::Task["polls:populate_option_id"].reenable
end
it "populates the option_id column of existing answers when there's one valid answer" 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")
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)
answer_with_invalid_option = build(:poll_answer, question: abc_question,
author: user,
answer: "Non existing",
option: nil)
answer_with_invalid_option.save!(validate: false)
Rake.application.invoke_task("polls:populate_option_id")
answer.reload
abc_answer.reload
answer_with_inconsistent_option.reload
answer_with_invalid_option.reload
expect(answer.option_id).to eq yes_no_question.question_options.find_by(title: "Yes").id
expect(abc_answer.option_id).to eq option_a.id
expect(answer_with_inconsistent_option.option_id).to eq option_b.id
expect(answer_with_invalid_option.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")
answer = create(:poll_answer, question: question, author: user, answer: "I", option: nil)
Rake.application.invoke_task("polls:populate_option_id")
answer.reload
expect(answer.option_id).to be nil
end
it "removes duplicate answers before populating the option_id column" do
user = create(:user, :level_two)
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")
answer_attributes = {
question_id: question.id,
author_id: user.id,
answer: "Answer A",
option_id: nil
}
answer = create(:poll_answer, answer_attributes)
insert(:poll_answer, answer_attributes)
localized_answer_attributes = { author: user, question: localized_question, option: nil }
localized_answer = create(:poll_answer, localized_answer_attributes.merge(answer: "Yes"))
create(:poll_answer, localized_answer_attributes.merge(answer: "Ja"))
answer.reload
localized_answer.reload
expect(answer.option_id).to be nil
expect(localized_answer.option_id).to be nil
Rake.application.invoke_task("polls:populate_option_id")
answer.reload
localized_answer.reload
expect(Poll::Answer.count).to eq 2
expect(answer.option_id).to eq question.question_options.find_by(title: "Answer A").id
expect(localized_answer.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: "answers")
Tenant.switch("answers") do
question = create(:poll_question_multiple, :abc)
create(:poll_answer, question: question, answer: "Answer A", option: nil)
end
Rake.application.invoke_task("polls:populate_option_id")
Tenant.switch("answers") do
expect(Poll::Question.count).to eq 1
expect(Poll::Answer.count).to eq 1
question = Poll::Question.first
option_a = question.question_options.find_by(title: "Answer A")
expect(Poll::Answer.first.option_id).to eq option_a.id
end
end
end
end

43
spec/models/poll/answer_spec.rb
View File

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

6
spec/routing/polymorphic_routes_spec.rb
View File

@@ -53,12 +53,6 @@ describe "Polymorphic routes" do
)
end
it "routes poll questions" do
question = create(:poll_question)
expect(polymorphic_path(question)).to eq question_path(question)
end
it "routes topics" do
community = create(:proposal).community
topic = create(:topic, community: community)