Rename members_count to members_count_with_descendants and expose only to group admins

6 files changed, 39 insertions, 20 deletions

diff --git a/doc/api/namespaces.md b/doc/api/namespaces.md
index d38a80176d9..8b5afe29040 100644
--- a/doc/api/namespaces.md
+++ b/doc/api/namespaces.md
@@ -30,9 +30,7 @@ Example response:
     "id": 1,
     "path": "user1",
     "kind": "user",
-    "full_path": "user1",
-    "parent_id": "null",
-    "members_count": "null"
+    "full_path": "user1"
   },
   {
     "id": 2,
@@ -40,7 +38,7 @@ Example response:
     "kind": "group",
     "full_path": "group1",
     "parent_id": "null",
-    "members_count": 2
+    "members_count_with_descendants": 2
 
   },
   {
@@ -49,11 +47,13 @@ Example response:
     "kind": "group",
     "full_path": "foo/bar",
     "parent_id": "9",
-    "members_count": 5
+    "members_count_with_descendants": 5
   }
 ]
 ```
 
+**Note**: `members_count_with_descendants` are presented only for group masters/owners.
+
 ## Search for namespace
 
 Get all namespaces that match a string in their name or path.
@@ -82,7 +82,7 @@ Example response:
     "kind": "group",
     "full_path": "twitter",
     "parent_id": "null",
-    "members_count": 2
+    "members_count_with_descendants": 2
   }
 ]
 ```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 2fe5280bc1c..cef5a0abe12 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -446,8 +446,12 @@ module API
     class Namespace < Grape::Entity
       expose :id, :name, :path, :kind, :full_path, :parent_id
 
-      expose :members_count do |namespace, _|
-        namespace.users_with_descendants.count if namespace.kind == 'group'
+      expose :members_count_with_descendants, if: -> (namespace, opts) { expose_members_count_with_descendants?(namespace, opts) } do |namespace, _|
+        namespace.users_with_descendants.count
+      end
+
+      def expose_members_count_with_descendants?(namespace, opts)
+        namespace.kind == 'group' && Ability.allowed?(opts[:current_user], :admin_group, namespace)
       end
     end
 
diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb
index 30761cb9b55..f1eaff6b0eb 100644
--- a/lib/api/namespaces.rb
+++ b/lib/api/namespaces.rb
@@ -17,7 +17,7 @@ module API
 
         namespaces = namespaces.search(params[:search]) if params[:search].present?
 
-        present paginate(namespaces), with: Entities::Namespace
+        present paginate(namespaces), with: Entities::Namespace, current_user: current_user
       end
     end
   end
diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb
index 03b1f549ce0..fdb7ebb6482 100644
--- a/spec/requests/api/namespaces_spec.rb
+++ b/spec/requests/api/namespaces_spec.rb
@@ -18,9 +18,15 @@ describe API::Namespaces do
       it "returns correct attributes" do
         get api("/namespaces", admin)
 
+        group_kind_json_response = json_response.find { |resource| resource['kind'] == 'group' }
+        user_kind_json_response = json_response.find { |resource| resource['kind'] == 'user' }
+
         expect(response).to have_http_status(200)
         expect(response).to include_pagination_headers
-        expect(json_response.first).to include('id', 'name', 'path', 'full_path', 'parent_id', 'members_count')
+        expect(group_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path',
+                                                                 'parent_id', 'members_count_with_descendants')
+
+        expect(user_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', 'parent_id')
       end
 
       it "admin: returns an array of all namespaces" do
@@ -45,12 +51,25 @@ describe API::Namespaces do
     end
 
     context "when authenticated as a regular user" do
-      it "returns correct attributes" do
+      it "returns members_count_with_descendants if user can admin group" do
+        group1.add_owner(user)
+
         get api("/namespaces", user)
 
-        expect(response).to have_http_status(200)
-        expect(response).to include_pagination_headers
-        expect(json_response.first).to include('id', 'name', 'path', 'full_path', 'parent_id', 'members_count')
+        owned_group_response = json_response.find { |resource| resource['id'] == group1.id }
+
+        expect(owned_group_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path',
+                                                             'parent_id', 'members_count_with_descendants')
+      end
+
+      it "does not returns members_count_with_descendants if user cannot admin group" do
+        group1.add_guest(user)
+
+        get api("/namespaces", user)
+
+        guest_group_response = json_response.find { |resource| resource['id'] == group1.id }
+
+        expect(guest_group_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', 'parent_id')
       end
 
       it "user: returns an array of namespaces" do
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index b4964ed302a..fd7ff0b9cff 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -698,9 +698,7 @@ describe API::Projects do
           'name' => user.namespace.name,
           'path' => user.namespace.path,
           'kind' => user.namespace.kind,
-          'full_path' => user.namespace.full_path,
-          'parent_id' => nil,
-          'members_count' => nil
+          'full_path' => user.namespace.full_path
         })
       end
 
diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb
index 225c28d6f13..cb74868324c 100644
--- a/spec/requests/api/v3/projects_spec.rb
+++ b/spec/requests/api/v3/projects_spec.rb
@@ -734,9 +734,7 @@ describe API::V3::Projects do
           'name' => user.namespace.name,
           'path' => user.namespace.path,
           'kind' => user.namespace.kind,
-          'full_path' => user.namespace.full_path,
-          'parent_id' => nil,
-          'members_count' => nil
+          'full_path' => user.namespace.full_path
         })
       end