Merge pull request #4191 from gschlager/phpbb3-importer-polls

Fix import of polls from phpBB3

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
 

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

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

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

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
+        SELECT o.poll_option_id, o.poll_option_text, o.poll_option_total AS total_votes,
-        FROM #{@table_prefix}_poll_options
+          o.poll_option_total - (
-        WHERE topic_id = #{topic_id}
+            SELECT COUNT(DISTINCT v.vote_user_id)
-        ORDER BY poll_option_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)
+      query(<<-SQL).first
-        SELECT MAX(count) AS count
+        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

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_poll_to_custom_fields(mapped_poll[:custom_fields], extracted_poll)
-      add_vote_fields(mapped_poll[:custom_fields], topic_id, 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]
+        option[:text] = option_text
-        else
+        option[:total_votes] += row[:total_votes]
-          options_by_text[option_text] = {
+        option[:anonymous_votes] += row[:anonymous_votes]
-            ids: [row[:poll_option_id]],
-            text: option_text,
-            votes: row[:poll_option_total]
-          }
-        end
       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)
+    def update_poll_metadata(extracted_poll, topic_id, poll)
-      default_poll['voters'] = @database.count_voters(topic_id) # this includes anonymous voters
+      row = @database.get_voters(topic_id)
-      default_poll['status'] = poll.has_ended? ? :open : :closed
 
-      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)
+    # @param custom_fields [Hash]
-      custom_fields[@polls_field] = {@default_poll_name => default_poll}
+    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}"
+          user_votes = votes["#{user_id}"] ||= {}
-
+          user_votes = user_votes[@default_poll_name] ||= []
-          if custom_fields.key?(key)
+          user_votes << option_id
-            votes = custom_fields[key][@default_poll_name]
+        end
-          else
-            votes = []
-            custom_fields[key] = {@default_poll_name => votes}
       end
 
-          votes << option_id
+      custom_fields[@votes_field] = votes
-        elsif !warned
-          warned = true
-          Rails.logger.warn("Topic with id #{topic_id} has invalid votes.")
-        end
-      end
     end
   end