Merge pull request #4676 from tgxworld/fix_polls_too_many_request

FIX: Public polls should not make a request per option.

plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6

@@ -11,6 +11,15 @@ function optionHtml(option) {
   return new RawHtml({ html: `<span>${option.html}</span>` });
 }
 
+function fetchVoters(payload) {
+  return ajax("/polls/voters.json", {
+    type: "get",
+    data: payload
+  }).catch(() => {
+    bootbox.alert(I18n.t('poll.error_while_fetching_voters'));
+  });
+}
+
 createWidget('discourse-poll-option', {
   tagName: 'li',
 
@@ -71,8 +80,7 @@ createWidget('discourse-poll-voters', {
     return {
       loaded: 'new',
       pollVoters: [],
-      offset: 0,
-      canLoadMore: false
+      offset: 1,
     };
   },
 
@@ -80,47 +88,45 @@ createWidget('discourse-poll-voters', {
     const { attrs, state } = this;
     if (state.loaded === 'loading') { return; }
 
-    const { voterIds } = attrs;
-
-    if (!voterIds.length) { return; }
-
-    const windowSize = Math.round(($('.poll-container:eq(0)').width() / 25) * 2);
-    const index = state.offset * windowSize;
-    const ids = voterIds.slice(index, index + windowSize);
-
     state.loaded = 'loading';
-    return ajax("/polls/voters.json", {
-      type: "get",
-      data: { user_ids: ids }
+
+    return fetchVoters({
+      post_id: attrs.postId,
+      poll_name: attrs.pollName,
+      option_id: attrs.optionId,
+      offset: state.offset
     }).then(result => {
       state.loaded = 'loaded';
-      state.pollVoters = state.pollVoters.concat(result.users);
-      state.canLoadMore = state.pollVoters.length < attrs.totalVotes;
+      state.offset += 1;
+
+      const pollResult = result[attrs.pollName]
+      const newVoters = attrs.pollType === 'number' ? pollResult : pollResult[attrs.optionId];
+      state.pollVoters = state.pollVoters.concat(newVoters);
+
       this.scheduleRerender();
-    }).catch(() => {
-      bootbox.alert(I18n.t('poll.error_while_fetching_voters'));
     });
   },
 
   loadMore() {
-    this.state.offset += 1;
     return this.fetchVoters();
   },
 
   html(attrs, state) {
-    if (state.loaded === 'new') {
-      this.fetchVoters();
-      return;
+    if (attrs.pollVoters && state.loaded === 'new') {
+      state.pollVoters = attrs.pollVoters;
     }
 
+    console.log(state.pollVoters);
     const contents = state.pollVoters.map(user => {
+      if (user === undefined) debugger;
+
       return h('li', [avatarFor('tiny', {
         username: user.username,
         template: user.avatar_template
       }), ' ']);
     });
 
-    if (state.canLoadMore) {
+    if (state.pollVoters.length < attrs.totalVotes) {
       contents.push(this.attach('discourse-poll-load-more', { id: attrs.id() }));
     }
 
@@ -131,13 +137,37 @@ createWidget('discourse-poll-voters', {
 
 createWidget('discourse-poll-standard-results', {
   tagName: 'ul.results',
+  buildKey: attrs => `${attrs.id}-standard-results`,
 
-  html(attrs) {
+  defaultState() {
+    return {
+      loaded: 'new'
+    };
+  },
+
+  fetchVoters() {
+    const { attrs, state } = this;
+
+    if (state.loaded === 'new') {
+      fetchVoters({
+        post_id: attrs.post.id,
+        poll_name: attrs.poll.get('name')
+      }).then(result => {
+        state.voters = result[attrs.poll.get('name')];
+        state.loaded = 'loaded';
+        this.scheduleRerender();
+      });
+    }
+  },
+
+  html(attrs, state) {
     const { poll } = attrs;
     const options = poll.get('options');
 
     if (options) {
       const voters = poll.get('voters');
+      const isPublic = poll.get('public');
+
       const ordered = _.clone(options).sort((a, b) => {
         if (a.votes < b.votes) {
           return 1;
@@ -158,6 +188,8 @@ createWidget('discourse-poll-standard-results', {
 
       const rounded = attrs.isMultiple ? percentages.map(Math.floor) : evenRound(percentages);
 
+      if (isPublic) this.fetchVoters();
+
       return ordered.map((option, idx) => {
         const contents = [];
         const per = rounded[idx].toString();
@@ -171,11 +203,14 @@ createWidget('discourse-poll-standard-results', {
                        h('div.bar', { attributes: { style: `width:${per}%` }})
                      ));
 
-        if (poll.get('public')) {
+        if (isPublic) {
           contents.push(this.attach('discourse-poll-voters', {
             id: () => `poll-voters-${option.id}`,
+            postId: attrs.post.id,
+            optionId: option.id,
+            pollName: poll.get('name'),
             totalVotes: option.votes,
-            voterIds: option.voter_ids
+            pollVoters: (state.voters && state.voters[option.id]) || []
           }));
         }
 
@@ -186,8 +221,33 @@ createWidget('discourse-poll-standard-results', {
 });
 
 createWidget('discourse-poll-number-results', {
-  html(attrs) {
+  buildKey: attrs => `${attrs.id}-number-results`,
+
+  defaultState() {
+    return {
+      loaded: 'new'
+    };
+  },
+
+  fetchVoters() {
+    const { attrs, state } = this;
+
+    if (state.loaded === 'new') {
+
+      fetchVoters({
+        post_id: attrs.post.id,
+        poll_name: attrs.poll.get('name')
+      }).then(result => {
+        state.voters = result[attrs.poll.get('name')];
+        state.loaded = 'loaded';
+        this.scheduleRerender();
+      });
+    }
+  },
+
+  html(attrs, state) {
     const { poll } = attrs;
+    const isPublic = poll.get('public');
 
     const totalScore = poll.get('options').reduce((total, o) => {
       return total + parseInt(o.html, 10) * parseInt(o.votes, 10);
@@ -199,12 +259,16 @@ createWidget('discourse-poll-number-results', {
     const results = [h('div.poll-results-number-rating',
                        new RawHtml({ html: `<span>${averageRating}</span>` }))];
 
-    if (poll.get('public')) {
-      const options = poll.get('options');
+    if (isPublic) {
+      this.fetchVoters();
+
       results.push(this.attach('discourse-poll-voters', {
         id: () => `poll-voters-${poll.get('name')}`,
         totalVotes: poll.get('voters'),
-        voterIds: [].concat(...(options.map(option => option.voter_ids)))
+        pollVoters: state.voters || [],
+        postId: attrs.post.id,
+        pollName: poll.get('name'),
+        pollType: poll.get('type')
       }));
     }
 
@@ -427,7 +491,6 @@ export default createWidget('discourse-poll', {
   },
 
   toggleStatus() {
-
     const { state, attrs } = this;
     const { poll } = attrs;
     const isClosed = poll.get('status') === 'closed';

plugins/poll/plugin.rb

@@ -211,13 +211,61 @@ after_initialize do
     end
 
     def voters
-      user_ids = params.require(:user_ids)
+      post_id = params.require(:post_id)
+      poll_name = params.require(:poll_name)
 
-      users = User.where(id: user_ids).map do |user|
-        UserNameSerializer.new(user).serializable_hash
+      post = Post.find_by(id: post_id)
+      raise Discourse::InvalidParameters.new("post_id is invalid") if !post
+
+      poll = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD][poll_name]
+      raise Discourse::InvalidParameters.new("poll_name is invalid") if !poll
+
+      user_ids = []
+      options = poll["options"]
+
+      if poll["type"] != "number"
+        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)
+        end
+
+        user_ids.flatten!
+        user_ids.uniq!
+
+        poll_votes = post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD]
+
+        result = {}
+
+        User.where(id: user_ids).map do |user|
+          user_hash = UserNameSerializer.new(user).serializable_hash
+
+          poll_votes[user.id.to_s][poll_name].each do |option_id|
+            if (params[:option_id])
+              next unless option_id == params[:option_id].to_s
+            end
+
+            result[option_id] ||= []
+            result[option_id] << user_hash
+          end
+        end
+      else
+        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)
+
+        result = []
+
+        users = User.where(id: user_ids).map do |user|
+          result << UserNameSerializer.new(user).serializable_hash
+        end
       end
 
-      render json: { users: users }
+      render json: { poll_name => result }
     end
   end
 
@@ -294,7 +342,16 @@ after_initialize do
                          polls: post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]})
   end
 
-  add_to_serializer(:post, :polls, false) { post_custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] }
+  add_to_serializer(:post, :polls, false) do
+    polls = post_custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD].dup
+
+    polls.each do |_, poll|
+      poll["options"].each do |option|
+        option.delete("voter_ids")
+      end
+    end
+  end
+
   add_to_serializer(:post, :include_polls?) { post_custom_fields.present? && post_custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD].present? }
 
   add_to_serializer(:post, :polls_votes, false) do

plugins/poll/spec/integration/poll_endpoints_spec.rb

@@ -1,18 +1,118 @@
 require "rails_helper"
 
 describe "DiscoursePoll endpoints" do
-  describe "fetch voters from user_ids" do
+  describe "fetch voters for a poll" do
     let(:user) { Fabricate(:user) }
+    let(:post) { Fabricate(:post, raw: "[poll public=true]\n- A\n- B\n[/poll]") }
 
     it "should return the right response" do
-      get "/polls/voters.json", { user_ids: [user.id] }
+      DiscoursePoll::Poll.vote(
+        post.id,
+        DiscoursePoll::DEFAULT_POLL_NAME,
+        ["5c24fc1df56d764b550ceae1b9319125"],
+        user
+      )
+
+      get "/polls/voters.json", {
+        post_id: post.id,
+        poll_name: DiscoursePoll::DEFAULT_POLL_NAME
+      }
 
       expect(response.status).to eq(200)
 
-      json = JSON.parse(response.body)["users"].first
+      poll = JSON.parse(response.body)[DiscoursePoll::DEFAULT_POLL_NAME]
+      option = poll["5c24fc1df56d764b550ceae1b9319125"]
 
-      expect(json["name"]).to eq(user.name)
-      expect(json["title"]).to eq(user.title)
+      expect(option.length).to eq(1)
+      expect(option.first["id"]).to eq(user.id)
+      expect(option.first["username"]).to eq(user.username)
+    end
+
+    it 'should return the right response for a single option' do
+      DiscoursePoll::Poll.vote(
+        post.id,
+        DiscoursePoll::DEFAULT_POLL_NAME,
+        ["5c24fc1df56d764b550ceae1b9319125", "e89dec30bbd9bf50fabf6a05b4324edf"],
+        user
+      )
+
+      get "/polls/voters.json", {
+        post_id: post.id,
+        poll_name: DiscoursePoll::DEFAULT_POLL_NAME,
+        option_id: 'e89dec30bbd9bf50fabf6a05b4324edf'
+      }
+
+      expect(response.status).to eq(200)
+
+      poll = JSON.parse(response.body)[DiscoursePoll::DEFAULT_POLL_NAME]
+
+      expect(poll['5c24fc1df56d764b550ceae1b9319125']).to eq(nil)
+
+      option = poll['e89dec30bbd9bf50fabf6a05b4324edf']
+
+      expect(option.length).to eq(1)
+      expect(option.first["id"]).to eq(user.id)
+      expect(option.first["username"]).to eq(user.username)
+    end
+
+    describe 'when post_id is blank' do
+      it 'should raise the right error' do
+        expect { get "/polls/voters.json", { poll_name: DiscoursePoll::DEFAULT_POLL_NAME } }
+          .to raise_error(ActionController::ParameterMissing)
+      end
+    end
+
+    describe 'when post_id is not valid' do
+      it 'should raise the right error' do
+        expect do
+          get "/polls/voters.json", {
+            post_id: -1,
+            poll_name: DiscoursePoll::DEFAULT_POLL_NAME
+          }
+        end.to raise_error(Discourse::InvalidParameters, 'post_id is invalid')
+      end
+    end
+
+    describe 'when poll_name is blank' do
+      it 'should raise the right error' do
+        expect { get "/polls/voters.json", { post_id: post.id } }
+          .to raise_error(ActionController::ParameterMissing)
+      end
+    end
+
+    describe 'when poll_name is not valid' do
+      it 'should raise the right error' do
+        expect do
+          get "/polls/voters.json", post_id: post.id, poll_name: 'wrongpoll'
+        end.to raise_error(Discourse::InvalidParameters, 'poll_name is invalid')
+      end
+    end
+
+    context "number poll" do
+      let(:post) { Fabricate(:post, raw: '[poll type=number min=1 max=20 step=1 public=true][/poll]') }
+
+      it 'should return the right response' do
+        post
+
+        DiscoursePoll::Poll.vote(
+          post.id,
+          DiscoursePoll::DEFAULT_POLL_NAME,
+          ["4d8a15e3cc35750f016ce15a43937620"],
+          user
+        )
+
+        get "/polls/voters.json", {
+          post_id: post.id,
+          poll_name: DiscoursePoll::DEFAULT_POLL_NAME
+        }
+
+        expect(response.status).to eq(200)
+
+        poll = JSON.parse(response.body)[DiscoursePoll::DEFAULT_POLL_NAME]
+
+        expect(poll.first["id"]).to eq(user.id)
+        expect(poll.first["username"]).to eq(user.username)
+      end
     end
   end
 end