diff --git a/app/jobs/regular/refresh_users_reviewable_counts.rb b/app/jobs/regular/refresh_users_reviewable_counts.rb index 10fa49467ff..27e743d14d7 100644 --- a/app/jobs/regular/refresh_users_reviewable_counts.rb +++ b/app/jobs/regular/refresh_users_reviewable_counts.rb @@ -4,8 +4,9 @@ class Jobs::RefreshUsersReviewableCounts < ::Jobs::Base def execute(args) group_ids = args[:group_ids] return if group_ids.blank? - User.where(id: GroupUser.where(group_id: group_ids).distinct.pluck(:user_id)).each( - &:publish_reviewable_counts - ) + User + .human_users + .where(id: GroupUser.where(group_id: group_ids).distinct.select(:user_id)) + .each(&:publish_reviewable_counts) end end diff --git a/app/models/group.rb b/app/models/group.rb index 0357e98695c..26dd49a28eb 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -554,13 +554,13 @@ class Group < ActiveRecord::Base remove_subquery = case name when :admins - "SELECT id FROM users WHERE id <= 0 OR NOT admin OR staged" + "SELECT id FROM users WHERE NOT admin OR staged" when :moderators - "SELECT id FROM users WHERE id <= 0 OR NOT moderator OR staged" + "SELECT id FROM users WHERE NOT moderator OR staged" when :staff - "SELECT id FROM users WHERE id <= 0 OR (NOT admin AND NOT moderator) OR staged" + "SELECT id FROM users WHERE (NOT admin AND NOT moderator) OR staged" when :trust_level_0, :trust_level_1, :trust_level_2, :trust_level_3, :trust_level_4 - "SELECT id FROM users WHERE id <= 0 OR trust_level < #{id - 10} OR staged" + "SELECT id FROM users WHERE trust_level < #{id - 10} OR staged" end removed_user_ids = DB.query_single <<-SQL @@ -584,15 +584,15 @@ class Group < ActiveRecord::Base insert_subquery = case name when :admins - "SELECT id FROM users WHERE id > 0 AND admin AND NOT staged" + "SELECT id FROM users WHERE admin AND NOT staged" when :moderators - "SELECT id FROM users WHERE id > 0 AND moderator AND NOT staged" + "SELECT id FROM users WHERE moderator AND NOT staged" when :staff - "SELECT id FROM users WHERE id > 0 AND (moderator OR admin) AND NOT staged" + "SELECT id FROM users WHERE (moderator OR admin) AND NOT staged" when :trust_level_1, :trust_level_2, :trust_level_3, :trust_level_4 - "SELECT id FROM users WHERE id > 0 AND trust_level >= #{id - 10} AND NOT staged" + "SELECT id FROM users WHERE trust_level >= #{id - 10} AND NOT staged" when :trust_level_0 - "SELECT id FROM users WHERE id > 0 AND NOT staged" + "SELECT id FROM users WHERE NOT staged" end added_user_ids = DB.query_single <<-SQL @@ -636,14 +636,15 @@ class Group < ActiveRecord::Base end def self.reset_groups_user_count!(only_group_ids: []) - where_sql = "" - - if only_group_ids.present? - where_sql = "WHERE group_id IN (#{only_group_ids.map(&:to_i).join(",")})" - end + where_sql = + if only_group_ids.present? + "WHERE group_id IN (#{only_group_ids.map(&:to_i).join(",")}) AND user_id > 0" + else + "WHERE user_id > 0" + end DB.exec <<-SQL - WITH X AS ( + WITH tally AS ( SELECT group_id, COUNT(user_id) users @@ -652,10 +653,10 @@ class Group < ActiveRecord::Base GROUP BY group_id ) UPDATE groups - SET user_count = X.users - FROM X - WHERE id = X.group_id - AND user_count <> X.users + SET user_count = tally.users + FROM tally + WHERE id = tally.group_id + AND user_count <> tally.users SQL end private_class_method :reset_groups_user_count! @@ -940,7 +941,8 @@ class Group < ActiveRecord::Base SET user_count = (SELECT COUNT(gu.user_id) FROM group_users gu - WHERE gu.group_id = g.id) + WHERE gu.group_id = g.id + AND gu.user_id > 0) WHERE g.id = #{self.id}; SQL end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 13cebcdb879..06d63d096a6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -491,24 +491,23 @@ RSpec.describe Group do Group.delete_all Group.refresh_automatic_groups! - groups = Group.includes(:users).to_a - expect(groups.count).to eq Group::AUTO_GROUPS.count + expect(Group.count).to eq Group::AUTO_GROUPS.count - g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:admins] } - expect(g.users.count).to eq g.user_count - expect(g.users.pluck(:id)).to contain_exactly(admin.id) + g = Group[:admins] + expect(g.human_users.count).to eq(g.user_count) + expect(g.human_users).to contain_exactly(admin) - g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:staff] } - expect(g.users.count).to eq g.user_count - expect(g.users.pluck(:id)).to contain_exactly(admin.id) + g = Group[:admins] + expect(g.human_users.count).to eq(g.user_count) + expect(g.human_users).to contain_exactly(admin) - g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:trust_level_1] } - expect(g.users.count).to eq g.user_count - expect(g.users.pluck(:id)).to contain_exactly(admin.id, user.id) + g = Group[:trust_level_1] + expect(g.human_users.count).to eq(g.user_count) + expect(g.human_users).to contain_exactly(admin, user) - g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:trust_level_2] } - expect(g.users.count).to eq g.user_count - expect(g.users.pluck(:id)).to contain_exactly(user.id) + g = Group[:trust_level_2] + expect(g.human_users.count).to eq(g.user_count) + expect(g.human_users).to contain_exactly(user) end it "can set members via usernames helper" do