Skip to content

Commit

Permalink
FIX: only count 'human' users in group.user_count
Browse files Browse the repository at this point in the history
  • Loading branch information
ZogStriP committed Jan 31, 2018
1 parent 826584b commit 4bcf9c5
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 20 deletions.
18 changes: 9 additions & 9 deletions app/models/group.rb
Expand Up @@ -281,13 +281,13 @@ def self.refresh_automatic_group!(name)
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
Expand All @@ -301,15 +301,15 @@ def self.refresh_automatic_group!(name)
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
Expand Down
19 changes: 8 additions & 11 deletions spec/models/group_spec.rb
Expand Up @@ -338,39 +338,36 @@ def real_staff
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
admin = Fabricate(:admin)
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!

groups = Group.includes(:users).to_a
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
Expand Down

1 comment on commit 4bcf9c5

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/admin-member-count-is-off-on-groups-page/77215/3

Please sign in to comment.