diff --git a/app/models/group.rb b/app/models/group.rb index 610e570194c..e70eab613c7 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -121,37 +121,44 @@ class Group < ActiveRecord::Base end if !user&.admin - is_staff = !!user&.staff? + sql = <<~SQL + groups.id IN ( + SELECT id + FROM groups + WHERE visibility_level = :public - if user.blank? - sql = "groups.visibility_level = :public" - elsif is_staff - sql = "groups.visibility_level IN (:public, :logged_on_users, :members, :staff)" - else - sql = <<~SQL - groups.id IN ( - SELECT id - FROM groups - WHERE visibility_level IN (:public, :logged_on_users) + UNION ALL - UNION ALL + SELECT id + FROM groups + WHERE visibility_level = :logged_on_users + AND :user_id IS NOT NULL - SELECT g.id - FROM groups g - JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id - WHERE g.visibility_level = :members + UNION ALL - UNION ALL + SELECT g.id + FROM groups g + JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id + WHERE g.visibility_level = :members - SELECT g.id - FROM groups g - JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id AND gu.owner - WHERE g.visibility_level IN (:staff, :owners) - ) - SQL - end + UNION ALL - params = Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: is_staff) + SELECT g.id + FROM groups g + LEFT JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id AND gu.owner + WHERE g.visibility_level = :staff + AND (gu.id IS NOT NULL OR :is_staff) + + UNION ALL + + SELECT g.id + FROM groups g + JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id AND gu.owner + WHERE g.visibility_level = :owners + ) + SQL + + params = Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: !!user&.staff?) groups = groups.where(sql, params) end @@ -166,37 +173,44 @@ class Group < ActiveRecord::Base end if !user&.admin - is_staff = !!user&.staff? + sql = <<~SQL + groups.id IN ( + SELECT id + FROM groups + WHERE members_visibility_level = :public - if user.blank? - sql = "groups.members_visibility_level = :public" - elsif is_staff - sql = "groups.members_visibility_level IN (:public, :logged_on_users, :members, :staff)" - else - sql = <<~SQL - groups.id IN ( - SELECT id - FROM groups - WHERE members_visibility_level IN (:public, :logged_on_users) + UNION ALL - UNION ALL + SELECT id + FROM groups + WHERE members_visibility_level = :logged_on_users + AND :user_id IS NOT NULL - SELECT g.id - FROM groups g - JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id - WHERE g.members_visibility_level = :members + UNION ALL - UNION ALL + SELECT g.id + FROM groups g + JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id + WHERE g.members_visibility_level = :members - SELECT g.id - FROM groups g - JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id AND gu.owner - WHERE g.members_visibility_level IN (:staff, :owners) - ) - SQL - end + UNION ALL - params = Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: is_staff) + SELECT g.id + FROM groups g + LEFT JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id AND gu.owner + WHERE g.members_visibility_level = :staff + AND (gu.id IS NOT NULL OR :is_staff) + + UNION ALL + + SELECT g.id + FROM groups g + JOIN group_users gu ON gu.group_id = g.id AND gu.user_id = :user_id AND gu.owner + WHERE g.members_visibility_level = :owners + ) + SQL + + params = Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: !!user&.staff?) groups = groups.where(sql, params) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e0c002e456b..a51ccb95e0c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3588,8 +3588,8 @@ en: title: "Who can see this group?" public: "Everyone" logged_on_users: "Logged on users" - members: "Group owners, members and moderators" - staff: "Group owners and moderators" + members: "Group owners, members" + staff: "Group owners and staff" owners: "Group owners" description: "Admins can see all groups." members_visibility_levels: diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0e501d5360a..dad0ef6dad1 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -698,7 +698,7 @@ describe Group do expect(can_view?(admin, group)).to eq(true) expect(can_view?(owner, group)).to eq(true) - expect(can_view?(moderator, group)).to eq(true) + expect(can_view?(moderator, group)).to eq(false) expect(can_view?(member, group)).to eq(true) expect(can_view?(logged_on_user, group)).to eq(false) expect(can_view?(nil, group)).to eq(false) @@ -763,7 +763,7 @@ describe Group do expect(can_view?(admin, group)).to eq(true) expect(can_view?(owner, group)).to eq(true) - expect(can_view?(moderator, group)).to eq(true) + expect(can_view?(moderator, group)).to eq(false) expect(can_view?(member, group)).to eq(true) expect(can_view?(logged_on_user, group)).to eq(false) expect(can_view?(nil, group)).to eq(false)