diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 82d271b7a81..977be892fa6 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -61,7 +61,7 @@ after_initialize do raise StandardError.new I18n.t("poll.requires_at_least_1_valid_option") if options.empty? - poll["voters"] = 0 + poll["voters"] = poll["anonymous_voters"] || 0 all_options = Hash.new(0) post.custom_fields[VOTES_CUSTOM_FIELD] ||= {} @@ -74,7 +74,10 @@ after_initialize do poll["voters"] += 1 if (available_options & votes.to_set).size > 0 end - poll["options"].each { |o| o["votes"] = all_options[o["id"]] } + poll["options"].each do |option| + anonymous_votes = option["anonymous_votes"] || 0 + option["votes"] = all_options[option["id"]] + anonymous_votes + end post.custom_fields[POLLS_CUSTOM_FIELD] = polls post.save_custom_fields(true) @@ -327,8 +330,14 @@ after_initialize do end polls[poll_name]["voters"] = previous_polls[poll_name]["voters"] + polls[poll_name]["anonymous_voters"] = previous_polls[poll_name]["anonymous_voters"] if previous_polls[poll_name].has_key?("anonymous_voters") + for o in 0...polls[poll_name]["options"].size - polls[poll_name]["options"][o]["votes"] = previous_polls[poll_name]["options"][o]["votes"] + current_option = polls[poll_name]["options"][o] + previous_option = previous_polls[poll_name]["options"][o] + + current_option["votes"] = previous_option["votes"] + current_option["anonymous_votes"] = previous_option["anonymous_votes"] if previous_option.has_key?("anonymous_votes") end end diff --git a/plugins/poll/spec/controllers/polls_controller_spec.rb b/plugins/poll/spec/controllers/polls_controller_spec.rb index b1679d7d6ee..8a8ce7190e5 100644 --- a/plugins/poll/spec/controllers/polls_controller_spec.rb +++ b/plugins/poll/spec/controllers/polls_controller_spec.rb @@ -1,4 +1,5 @@ require "rails_helper" +require_relative "../helpers" describe ::DiscoursePoll::PollsController do routes { ::DiscoursePoll::Engine.routes } @@ -85,6 +86,18 @@ describe ::DiscoursePoll::PollsController do expect(json["errors"][0]).to eq(I18n.t("poll.poll_must_be_open_to_vote")) end + it "doesn't discard anonymous votes when someone votes" do + default_poll = poll.custom_fields["polls"]["poll"] + add_anonymous_votes(poll, default_poll, 17, {"5c24fc1df56d764b550ceae1b9319125" => 11, "e89dec30bbd9bf50fabf6a05b4324edf" => 6}) + + xhr :put, :vote, { post_id: poll.id, poll_name: "poll", options: ["5c24fc1df56d764b550ceae1b9319125"] } + expect(response).to be_success + + json = ::JSON.parse(response.body) + expect(json["poll"]["voters"]).to eq(18) + expect(json["poll"]["options"][0]["votes"]).to eq(12) + expect(json["poll"]["options"][1]["votes"]).to eq(6) + end end describe "#toggle_status" do diff --git a/plugins/poll/spec/controllers/posts_controller_spec.rb b/plugins/poll/spec/controllers/posts_controller_spec.rb index bc86459637f..990ead74d8e 100644 --- a/plugins/poll/spec/controllers/posts_controller_spec.rb +++ b/plugins/poll/spec/controllers/posts_controller_spec.rb @@ -1,4 +1,5 @@ require "rails_helper" +require_relative "../helpers" describe PostsController do let!(:user) { log_in } @@ -165,12 +166,31 @@ describe PostsController do expect(json["errors"][0]).to eq(I18n.t("poll.op_cannot_edit_options_after_5_minutes")) end - it "staff can change the options" do + it "staff can change the options and votes are merged" do log_in_user(Fabricate(:moderator)) xhr :put, :update, { id: post_id, post: { raw: new_option } } expect(response).to be_success json = ::JSON.parse(response.body) expect(json["post"]["polls"]["poll"]["options"][1]["html"]).to eq("C") + expect(json["post"]["polls"]["poll"]["voters"]).to eq(1) + expect(json["post"]["polls"]["poll"]["options"][0]["votes"]).to eq(1) + expect(json["post"]["polls"]["poll"]["options"][1]["votes"]).to eq(0) + end + + it "staff can change the options and anonymous votes are merged" do + post = Post.find_by(id: post_id) + default_poll = post.custom_fields["polls"]["poll"] + add_anonymous_votes(post, default_poll, 7, {"5c24fc1df56d764b550ceae1b9319125" => 7}) + + log_in_user(Fabricate(:moderator)) + xhr :put, :update, { id: post_id, post: { raw: new_option } } + expect(response).to be_success + + json = ::JSON.parse(response.body) + expect(json["post"]["polls"]["poll"]["options"][1]["html"]).to eq("C") + expect(json["post"]["polls"]["poll"]["voters"]).to eq(8) + expect(json["post"]["polls"]["poll"]["options"][0]["votes"]).to eq(8) + expect(json["post"]["polls"]["poll"]["options"][1]["votes"]).to eq(0) end it "support changes on the post" do diff --git a/plugins/poll/spec/helpers.rb b/plugins/poll/spec/helpers.rb new file mode 100644 index 00000000000..a0584810ab8 --- /dev/null +++ b/plugins/poll/spec/helpers.rb @@ -0,0 +1,17 @@ +module Helpers + def add_anonymous_votes(post, poll, voters, options_with_votes) + poll["voters"] += voters + poll["anonymous_voters"] = voters + + poll["options"].each do |option| + anonymous_votes = options_with_votes[option["id"]] || 0 + + if anonymous_votes > 0 + option["votes"] += anonymous_votes + option["anonymous_votes"] = anonymous_votes + end + end + + post.save_custom_fields(true) + end +end diff --git a/script/import_scripts/phpbb3/database/database_3_0.rb b/script/import_scripts/phpbb3/database/database_3_0.rb index 4c750c2de72..f6ebc9223e4 100644 --- a/script/import_scripts/phpbb3/database/database_3_0.rb +++ b/script/import_scripts/phpbb3/database/database_3_0.rb @@ -95,33 +95,50 @@ module ImportScripts::PhpBB3 def fetch_poll_options(topic_id) query(<<-SQL) - SELECT poll_option_id, poll_option_text, poll_option_total - FROM #{@table_prefix}_poll_options - WHERE topic_id = #{topic_id} - ORDER BY poll_option_id + SELECT o.poll_option_id, o.poll_option_text, o.poll_option_total AS total_votes, + o.poll_option_total - ( + SELECT COUNT(DISTINCT v.vote_user_id) + FROM #{@table_prefix}_poll_votes v + JOIN #{@table_prefix}_users u ON (v.vote_user_id = u.user_id) + JOIN #{@table_prefix}_topics t ON (v.topic_id = t.topic_id) + WHERE v.poll_option_id = o.poll_option_id AND v.topic_id = o.topic_id + ) AS anonymous_votes + FROM #{@table_prefix}_poll_options o + WHERE o.topic_id = #{topic_id} + ORDER BY o.poll_option_id SQL end def fetch_poll_votes(topic_id) - # this query ignores votes from users that do not exist anymore + # this query ignores invalid votes that belong to non-existent users or topics query(<<-SQL) SELECT u.user_id, v.poll_option_id FROM #{@table_prefix}_poll_votes v + JOIN #{@table_prefix}_poll_options o ON (v.poll_option_id = o.poll_option_id AND v.topic_id = o.topic_id) JOIN #{@table_prefix}_users u ON (v.vote_user_id = u.user_id) + JOIN #{@table_prefix}_topics t ON (v.topic_id = t.topic_id) WHERE v.topic_id = #{topic_id} SQL end - def count_voters(topic_id) + def get_voters(topic_id) # anonymous voters can't be counted, but lets try to make the count look "correct" anyway - count(<<-SQL) - SELECT MAX(count) AS count + query(<<-SQL).first + SELECT MAX(x.total_voters) AS total_voters, + MAX(x.total_voters) - ( + SELECT COUNT(DISTINCT v.vote_user_id) + FROM #{@table_prefix}_poll_votes v + JOIN #{@table_prefix}_poll_options o ON (v.poll_option_id = o.poll_option_id AND v.topic_id = o.topic_id) + JOIN #{@table_prefix}_users u ON (v.vote_user_id = u.user_id) + JOIN #{@table_prefix}_topics t ON (v.topic_id = t.topic_id) + WHERE v.topic_id = #{topic_id} + ) AS anonymous_voters FROM ( - SELECT COUNT(DISTINCT vote_user_id) AS count + SELECT COUNT(DISTINCT vote_user_id) AS total_voters FROM #{@table_prefix}_poll_votes WHERE topic_id = #{topic_id} UNION - SELECT MAX(poll_option_total) AS count + SELECT MAX(poll_option_total) AS total_voters FROM #{@table_prefix}_poll_options WHERE topic_id = #{topic_id} ) x diff --git a/script/import_scripts/phpbb3/importers/poll_importer.rb b/script/import_scripts/phpbb3/importers/poll_importer.rb index e6bae5b40b7..da3e1d0b65e 100644 --- a/script/import_scripts/phpbb3/importers/poll_importer.rb +++ b/script/import_scripts/phpbb3/importers/poll_importer.rb @@ -24,15 +24,17 @@ module ImportScripts::PhpBB3 return if extracted_poll.nil? - update_poll(extracted_poll, options, topic_id, poll) + update_poll_metadata(extracted_poll, topic_id, poll) + update_poll_options(extracted_poll, options, poll) mapped_poll = { raw: poll_text, custom_fields: {} } - add_polls_field(mapped_poll[:custom_fields], extracted_poll) - add_vote_fields(mapped_poll[:custom_fields], topic_id, poll) + add_poll_to_custom_fields(mapped_poll[:custom_fields], extracted_poll) + add_votes_to_custom_fields(mapped_poll[:custom_fields], topic_id, poll) + mapped_poll end @@ -40,23 +42,17 @@ module ImportScripts::PhpBB3 def get_poll_options(topic_id) rows = @database.fetch_poll_options(topic_id) - options_by_text = {} + options_by_text = Hash.new { |h, k| h[k] = {ids: [], total_votes: 0, anonymous_votes: 0} } rows.each do |row| option_text = @text_processor.process_raw_text(row[:poll_option_text]).delete("\n") - if options_by_text.key?(option_text) - # phpBB allows duplicate options (why?!) - we need to merge them - option = options_by_text[option_text] - option[:ids] << row[:poll_option_id] - option[:votes] += row[:poll_option_total] - else - options_by_text[option_text] = { - ids: [row[:poll_option_id]], - text: option_text, - votes: row[:poll_option_total] - } - end + # phpBB allows duplicate options (why?!) - we need to merge them + option = options_by_text[option_text] + option[:ids] << row[:poll_option_id] + option[:text] = option_text + option[:total_votes] += row[:total_votes] + option[:anonymous_votes] += row[:anonymous_votes] end options_by_text.values @@ -91,47 +87,47 @@ module ImportScripts::PhpBB3 end # @param poll [ImportScripts::PhpBB3::Poll] - def update_poll(default_poll, imported_options, topic_id, poll) - default_poll['voters'] = @database.count_voters(topic_id) # this includes anonymous voters - default_poll['status'] = poll.has_ended? ? :open : :closed + def update_poll_metadata(extracted_poll, topic_id, poll) + row = @database.get_voters(topic_id) - default_poll['options'].each_with_index do |option, index| + extracted_poll['voters'] = row[:total_voters] + extracted_poll['anonymous_voters'] = row[:anonymous_voters] if row[:anonymous_voters] > 0 + extracted_poll['status'] = poll.has_ended? ? :open : :closed + end + + # @param poll [ImportScripts::PhpBB3::Poll] + def update_poll_options(extracted_poll, imported_options, poll) + extracted_poll['options'].each_with_index do |option, index| imported_option = imported_options[index] - option['votes'] = imported_option[:votes] + option['votes'] = imported_option[:total_votes] + option['anonymous_votes'] = imported_option[:anonymous_votes] if imported_option[:anonymous_votes] > 0 poll.add_option_id(imported_option[:ids], option['id']) end end - def add_polls_field(custom_fields, default_poll) - custom_fields[@polls_field] = {@default_poll_name => default_poll} + # @param custom_fields [Hash] + def add_poll_to_custom_fields(custom_fields, extracted_poll) + custom_fields[@polls_field] = {@default_poll_name => extracted_poll} end # @param custom_fields [Hash] # @param poll [ImportScripts::PhpBB3::Poll] - def add_vote_fields(custom_fields, topic_id, poll) + def add_votes_to_custom_fields(custom_fields, topic_id, poll) rows = @database.fetch_poll_votes(topic_id) - warned = false + votes = {} rows.each do |row| option_id = poll.option_id_from_imported_option_id(row[:poll_option_id]) user_id = @lookup.user_id_from_imported_user_id(row[:user_id]) if option_id.present? && user_id.present? - key = "#{@votes_field}-#{user_id}" - - if custom_fields.key?(key) - votes = custom_fields[key][@default_poll_name] - else - votes = [] - custom_fields[key] = {@default_poll_name => votes} - end - - votes << option_id - elsif !warned - warned = true - Rails.logger.warn("Topic with id #{topic_id} has invalid votes.") + user_votes = votes["#{user_id}"] ||= {} + user_votes = user_votes[@default_poll_name] ||= [] + user_votes << option_id end end + + custom_fields[@votes_field] = votes end end