FEATURE: moderators allowed to view groups which members can see.

Currently, if a group's visibility is set to "Group owners, members" then the mods can't view those group pages. The same rule is applied for members visibility setting too.

This reverts commit 7fc7090. And fixed the spec test fails.

app/models/group.rb

@@ -121,44 +121,37 @@ class Group < ActiveRecord::Base
     end
 
     if !user&.admin
-      sql = <<~SQL
+      is_staff = !!user&.staff?
-        groups.id IN (
-          SELECT id
-            FROM groups
-           WHERE visibility_level = :public
 
-          UNION ALL
+      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)
 
-          SELECT id
+            UNION ALL
-            FROM groups
-           WHERE visibility_level = :logged_on_users
-             AND :user_id IS NOT NULL
 
-          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
+            UNION ALL
-            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
+            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
 
-          SELECT g.id
+      params = Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: is_staff)
-            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
 
@@ -173,44 +166,37 @@ class Group < ActiveRecord::Base
     end
 
     if !user&.admin
-      sql = <<~SQL
+      is_staff = !!user&.staff?
-        groups.id IN (
-          SELECT id
-            FROM groups
-           WHERE members_visibility_level = :public
 
-          UNION ALL
+      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)
 
-          SELECT id
+            UNION ALL
-            FROM groups
-           WHERE members_visibility_level = :logged_on_users
-             AND :user_id IS NOT NULL
 
-          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
+            UNION ALL
-            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
+            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
 
-          SELECT g.id
+      params = Group.visibility_levels.to_h.merge(user_id: user&.id, is_staff: is_staff)
-            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
 

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"
+              members: "Group owners, members and moderators"
-              staff: "Group owners and staff"
+              staff: "Group owners and moderators"
               owners: "Group owners"
               description: "Admins can see all groups."
             members_visibility_levels:

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(false)
+      expect(can_view?(moderator, group)).to eq(true)
       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(false)
+      expect(can_view?(moderator, group)).to eq(true)
       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)

spec/requests/groups_controller_spec.rb

@@ -248,7 +248,7 @@ describe GroupsController do
 
         expect(response.status).to eq(200)
         group_names = response.parsed_body["groups"].map { |g| g["name"] }
-        expect(group_names).to contain_exactly("0_0", "0_1", "0_3", "1_0", "1_1", "1_3", "3_0", "3_1", "3_3")
+        expect(group_names).to contain_exactly("0_0", "0_1", "0_2", "0_3", "1_0", "1_1", "1_2", "1_3", "2_0", "2_1", "2_2", "2_3", "3_0", "3_1", "3_2", "3_3")
 
         # admin
         sign_in(admin)