From 4bcf9c5bf33b9edf5c38a822897cc441594bbbcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 31 Jan 2018 21:55:01 +0100 Subject: [PATCH] FIX: only count 'human' users in group.user_count --- app/models/group.rb | 18 +++++++++--------- spec/models/group_spec.rb | 19 ++++++++----------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index a86d60c5812..61f7e79f137 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -281,13 +281,13 @@ class Group < ActiveRecord::Base remove_subquery = case name when :admins - "SELECT id FROM users WHERE NOT admin" + "SELECT id FROM users WHERE id <= 0 OR NOT admin" when :moderators - "SELECT id FROM users WHERE NOT moderator" + "SELECT id FROM users WHERE id <= 0 OR NOT moderator" when :staff - "SELECT id FROM users WHERE NOT admin AND NOT moderator" + "SELECT id FROM users WHERE id <= 0 OR (NOT admin AND NOT moderator)" when :trust_level_0, :trust_level_1, :trust_level_2, :trust_level_3, :trust_level_4 - "SELECT id FROM users WHERE trust_level < #{id - 10}" + "SELECT id FROM users WHERE id <= 0 OR trust_level < #{id - 10}" end exec_sql <<-SQL @@ -301,15 +301,15 @@ class Group < ActiveRecord::Base insert_subquery = case name when :admins - "SELECT id FROM users WHERE admin" + "SELECT id FROM users WHERE id > 0 AND admin" when :moderators - "SELECT id FROM users WHERE moderator" + "SELECT id FROM users WHERE id > 0 AND moderator" when :staff - "SELECT id FROM users WHERE moderator OR admin" + "SELECT id FROM users WHERE id > 0 AND (moderator OR admin)" when :trust_level_1, :trust_level_2, :trust_level_3, :trust_level_4 - "SELECT id FROM users WHERE trust_level >= #{id - 10}" + "SELECT id FROM users WHERE id > 0 AND trust_level >= #{id - 10}" when :trust_level_0 - "SELECT id FROM users" + "SELECT id FROM users WHERE id > 0" end exec_sql <<-SQL diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 4dce0a96cd3..856b6fa0f52 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -338,7 +338,7 @@ describe Group do user2 = Fabricate(:coding_horror) user2.change_trust_level!(TrustLevel[3]) - expect(Group[:trust_level_2].user_ids.sort.reject { |id| id < -1 }).to eq [-1, user.id, user2.id].sort + expect(Group[:trust_level_2].user_ids).to include(user.id, user2.id) end it "Correctly updates all automatic groups upon request" do @@ -346,7 +346,7 @@ describe Group do user = Fabricate(:user) user.change_trust_level!(TrustLevel[2]) - Group.exec_sql("update groups set user_count=0 where id = #{Group::AUTO_GROUPS[:trust_level_2]}") + Group.exec_sql("UPDATE groups SET user_count = 0 WHERE id = #{Group::AUTO_GROUPS[:trust_level_2]}") Group.refresh_automatic_groups! @@ -354,23 +354,20 @@ describe Group do expect(groups.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).sort.reject { |id| id < -1 }).to eq([-1, admin.id]) + expect(g.users.count).to eq g.user_count + expect(g.users.pluck(:id)).to contain_exactly(admin.id) g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:staff] } - expect(g.users.count).to eq (g.user_count) - expect(g.users.pluck(:id).sort.reject { |id| id < -1 }).to eq([-1, admin.id]) + expect(g.users.count).to eq g.user_count + expect(g.users.pluck(:id)).to contain_exactly(admin.id) g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:trust_level_1] } - # admin, system and user expect(g.users.count).to eq g.user_count - expect(g.users.where('users.id > -2').count).to eq 3 + expect(g.users.pluck(:id)).to contain_exactly(admin.id, user.id) g = groups.find { |grp| grp.id == Group::AUTO_GROUPS[:trust_level_2] } - # system and user expect(g.users.count).to eq g.user_count - expect(g.users.where('users.id > -2').count).to eq 2 - + expect(g.users.pluck(:id)).to contain_exactly(user.id) end it "can set members via usernames helper" do