From 43c0111ca156b99881cf5e489c1364a4c3ef5b90 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 14 Aug 2017 15:33:47 -0400 Subject: [PATCH] FIX: multichoice poll with more than 25 votes In the past we would double up on avatars due to incorrect logic for handling offset --- plugins/poll/lib/polls_updater.rb | 2 +- plugins/poll/plugin.rb | 17 ++++++++-- .../spec/controllers/polls_controller_spec.rb | 31 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/plugins/poll/lib/polls_updater.rb b/plugins/poll/lib/polls_updater.rb index 488331041b1..ec5b250579b 100644 --- a/plugins/poll/lib/polls_updater.rb +++ b/plugins/poll/lib/polls_updater.rb @@ -130,7 +130,7 @@ module DiscoursePoll private def self.private_to_public_poll?(post, previous_polls, current_polls, poll_name) - previous_poll = previous_polls[poll_name] + _previous_poll = previous_polls[poll_name] current_poll = current_polls[poll_name] if previous_polls["public"].nil? && current_poll["public"] == "true" diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index c8d0cf8d051..cd426235365 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -220,17 +220,26 @@ after_initialize do poll = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD][poll_name] raise Discourse::InvalidParameters.new("poll_name is invalid") if !poll + voter_limit = (params[:voter_limit] || 25).to_i + voter_limit = 0 if voter_limit < 0 + voter_limit = 50 if voter_limit > 50 + user_ids = [] options = poll["options"] if poll["type"] != "number" + + per_option_voters = {} + options.each do |option| if (params[:option_id]) next unless option["id"] == params[:option_id].to_s end next unless option["voter_ids"] - user_ids << option["voter_ids"].slice((params[:offset].to_i || 0) * 25, 25) + voters = option["voter_ids"].slice((params[:offset].to_i || 0) * voter_limit, voter_limit) + per_option_voters[option["id"]] = Set.new(voters) + user_ids << voters end user_ids.flatten! @@ -248,6 +257,10 @@ after_initialize do next unless option_id == params[:option_id].to_s end + voters = per_option_voters[option_id] + # we may have a user from a different vote + next unless voters.include?(user.id) + result[option_id] ||= [] result[option_id] << user_hash end @@ -256,7 +269,7 @@ after_initialize do user_ids = options.map { |option| option["voter_ids"] }.sort! user_ids.flatten! user_ids.uniq! - user_ids = user_ids.slice((params[:offset].to_i || 0) * 25, 25) + user_ids = user_ids.slice((params[:offset].to_i || 0) * voter_limit, voter_limit) result = [] diff --git a/plugins/poll/spec/controllers/polls_controller_spec.rb b/plugins/poll/spec/controllers/polls_controller_spec.rb index 02db8bdf930..d2d671b0e28 100644 --- a/plugins/poll/spec/controllers/polls_controller_spec.rb +++ b/plugins/poll/spec/controllers/polls_controller_spec.rb @@ -7,6 +7,7 @@ describe ::DiscoursePoll::PollsController do let!(:user) { log_in } let(:topic) { Fabricate(:topic) } let(:poll) { Fabricate(:post, topic_id: topic.id, user_id: user.id, raw: "[poll]\n- A\n- B\n[/poll]") } + let(:multi_poll) { Fabricate(:post, topic_id: topic.id, user_id: user.id, raw: "[poll min=1 max=2 type=multiple public=true]\n- A\n- B\n[/poll]") } describe "#vote" do @@ -173,4 +174,34 @@ describe ::DiscoursePoll::PollsController do end end + + describe "votes" do + + it "correctly handles offset" do + + first = "5c24fc1df56d764b550ceae1b9319125" + second = "e89dec30bbd9bf50fabf6a05b4324edf" + + user1 = log_in + xhr :put, :vote, post_id: multi_poll.id, poll_name: "poll", options: [first] + + user2 = log_in + xhr :put, :vote, post_id: multi_poll.id, poll_name: "poll", options: [first] + + user3 = log_in + xhr :put, :vote, + post_id: multi_poll.id, + poll_name: "poll", + options: [first, second] + + xhr :get, :voters, poll_name: 'poll', post_id: multi_poll.id, voter_limit: 2 + json = JSON.parse(response.body) + + # no user3 cause voter_limit is 2 + expect(json["poll"][first].map { |h| h["id"] }.sort).to eq([user1.id, user2.id]) + expect(json["poll"][second].map { |h| h["id"] }).to eq([user3.id]) + end + + end + end