diff --git a/plugins/poll/app/models/poll.rb b/plugins/poll/app/models/poll.rb index e04e6b9b0d9..0ce6e382fc8 100644 --- a/plugins/poll/app/models/poll.rb +++ b/plugins/poll/app/models/poll.rb @@ -23,6 +23,16 @@ class Poll < ActiveRecord::Base validates :max, numericality: { allow_nil: true, only_integer: true, greater_than: 0 } validates :step, numericality: { allow_nil: true, only_integer: true, greater_than: 0 } + attr_writer :voters_count + attr_accessor :has_voted + + after_initialize { @has_voted = {} } + + def reload + @has_voted = {} + super + end + def is_closed? closed? || (close_at && close_at <= Time.zone.now) end @@ -37,7 +47,17 @@ class Poll < ActiveRecord::Base end def has_voted?(user) - user&.id && poll_votes.where(user_id: user.id).exists? + if user&.id + return @has_voted[user.id] if @has_voted.key?(user.id) + + @has_voted[user.id] = poll_votes.where(user_id: user.id).exists? + end + end + + def voters_count + return @voters_count if defined?(@voters_count) + + @voters_count = poll_votes.count("DISTINCT user_id") end def can_see_voters?(user) @@ -47,6 +67,36 @@ class Poll < ActiveRecord::Base def ranked_choice? type == "ranked_choice" end + + def self.preload!(polls, user_id: nil) + poll_ids = polls.map(&:id) + + voters_count = + PollVote + .where(poll_id: poll_ids) + .group(:poll_id) + .pluck(:poll_id, "COUNT(DISTINCT user_id)") + .to_h + + option_voters_count = + PollVote + .where(poll_option_id: PollOption.where(poll_id: poll_ids).select(:id)) + .group(:poll_option_id) + .pluck(:poll_option_id, "COUNT(*)") + .to_h + + polls.each do |poll| + poll.voters_count = voters_count[poll.id] || 0 + poll.poll_options.each do |poll_option| + poll_option.voters_count = option_voters_count[poll_option.id] || 0 + end + end + + if user_id + has_voted = PollVote.where(poll_id: poll_ids, user_id: user_id).pluck(:poll_id).to_set + polls.each { |poll| poll.has_voted[user_id] = has_voted.include?(poll.id) } + end + end end # == Schema Information diff --git a/plugins/poll/app/models/poll_option.rb b/plugins/poll/app/models/poll_option.rb index 7eea2dd0c9d..3f10eb7157f 100644 --- a/plugins/poll/app/models/poll_option.rb +++ b/plugins/poll/app/models/poll_option.rb @@ -3,6 +3,14 @@ class PollOption < ActiveRecord::Base belongs_to :poll has_many :poll_votes, dependent: :delete_all + + attr_writer :voters_count + + def voters_count + return @voters_count if defined?(@voters_count) + + @voters_count = poll_votes.count + end end # == Schema Information diff --git a/plugins/poll/app/serializers/poll_option_serializer.rb b/plugins/poll/app/serializers/poll_option_serializer.rb index 16b869c8ee6..cd42ed161eb 100644 --- a/plugins/poll/app/serializers/poll_option_serializer.rb +++ b/plugins/poll/app/serializers/poll_option_serializer.rb @@ -8,8 +8,7 @@ class PollOptionSerializer < ApplicationSerializer end def votes - # `size` instead of `count` to prevent N+1 - object.poll_votes.size + object.anonymous_votes.to_i + object.voters_count + object.anonymous_votes.to_i end def include_votes? diff --git a/plugins/poll/app/serializers/poll_serializer.rb b/plugins/poll/app/serializers/poll_serializer.rb index 0e8c98071e4..a780f1eb9c9 100644 --- a/plugins/poll/app/serializers/poll_serializer.rb +++ b/plugins/poll/app/serializers/poll_serializer.rb @@ -58,7 +58,7 @@ class PollSerializer < ApplicationSerializer end def voters - object.poll_votes.count("DISTINCT user_id") + object.anonymous_voters.to_i + object.voters_count + object.anonymous_voters.to_i end def close diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 69334308bc5..554d8fc1675 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -182,12 +182,12 @@ after_initialize do end if post_with_polls.present? - Poll - .where(post_id: post_with_polls) - .each do |p| - polls[p.post_id] ||= [] - polls[p.post_id] << p - end + all_polls = Poll.includes(:poll_options).where(post_id: post_with_polls) + Poll.preload!(all_polls, user_id: @user&.id) + all_polls.each do |p| + polls[p.post_id] ||= [] + polls[p.post_id] << p + end end polls diff --git a/plugins/poll/spec/models/poll_spec.rb b/plugins/poll/spec/models/poll_spec.rb index b71599327a6..6e577f1f5d7 100644 --- a/plugins/poll/spec/models/poll_spec.rb +++ b/plugins/poll/spec/models/poll_spec.rb @@ -34,7 +34,7 @@ RSpec.describe ::DiscoursePoll::Poll do expect(poll.can_see_results?(user)).to eq(false) poll.poll_votes.create!(poll_option_id: option.id, user_id: user.id) - expect(poll.can_see_results?(user)).to eq(true) + expect(poll.reload.can_see_results?(user)).to eq(true) end it "author can see results when results setting is on_vote" do diff --git a/plugins/poll/spec/requests/topics_controller_spec.rb b/plugins/poll/spec/requests/topics_controller_spec.rb new file mode 100644 index 00000000000..e0370135bc2 --- /dev/null +++ b/plugins/poll/spec/requests/topics_controller_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +RSpec.describe PostsController do + fab!(:admin) { Fabricate(:admin) } + fab!(:topic) { Fabricate(:topic, user: admin) } + + fab!(:post1) { Fabricate(:post, topic:, raw: "[poll]\n- A\n- B\n[/poll]") } + fab!(:post2) { Fabricate(:post, topic:, raw: "[poll results=on_vote]\n- A\n- B\n[/poll]") } + fab!(:post3) { Fabricate(:post, topic:, raw: "[poll results=on_vote]\n- A\n- B\n[/poll]") } + fab!(:post4) { Fabricate(:post, topic:, raw: "[poll results=on_vote]\n- A\n- B\n[/poll]") } + fab!(:post5) { Fabricate(:post, topic:, raw: "[poll results=staff_only]\n- A\n- B\n[/poll]") } + fab!(:post6) { Fabricate(:post, topic:, raw: "[poll results=staff_only]\n- A\n- B\n[/poll]") } + fab!(:post7) { Fabricate(:post, topic:, raw: "[poll visibility=]\n- A\n- B\n[/poll]") } + + describe "#show" do + context "when not logged in" do + it "does not create N+1 queries to load polls" do + queries = track_sql_queries { get "/t/#{topic.id}.json" } + + expect(response.status).to eq(200) + + poll_queries = queries.filter { |q| q =~ /FROM "?poll/ } + # Expected queries: + # + # - load all polls + # - load all options + # - count votes for each poll + # - count votes for each option + expect(poll_queries.size).to eq(4) + end + end + + context "when logged in" do + before { sign_in(admin) } + + it "does not create N+1 queries to load polls" do + queries = track_sql_queries { get "/t/#{topic.id}.json" } + + poll_queries = queries.filter { |q| q =~ /FROM "?poll/ } + + # Expected queries: + # + # - all queries listed for "when not logged in" + # - query to find out if the user has voted in each poll + # - queries to get "serialized voters" (NOT TRACKED) + expect(poll_queries.size).to eq(5) + end + end + end +end