diff --git a/app/models/group.rb b/app/models/group.rb index a02eb74599fb0a4f5510cc4cef17e34daff2663b..17590f90d497e89688f35c9c50be393b25fbc71c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -266,6 +266,12 @@ def users_with_parents User.where(id: members_with_parents.select(:user_id)) end + def users_with_descendants + members_with_descendants = GroupMember.non_request.where(source_id: descendants.pluck(:id).push(id)) + + User.where(id: members_with_descendants.select(:user_id)) + end + def max_member_access_for_user(user) return GroupMember::OWNER if user.admin? diff --git a/changelogs/unreleased/add-group-members-counting-and-plan-related-data-on-namespaces-api.yml b/changelogs/unreleased/add-group-members-counting-and-plan-related-data-on-namespaces-api.yml new file mode 100644 index 0000000000000000000000000000000000000000..f2591042e98399b04b4dc03874a32c5b5e9e3d73 --- /dev/null +++ b/changelogs/unreleased/add-group-members-counting-and-plan-related-data-on-namespaces-api.yml @@ -0,0 +1,4 @@ +--- +title: Add group members counting and plan related data on namespaces API +merge_request: +author: diff --git a/doc/api/namespaces.md b/doc/api/namespaces.md index 4ad6071a0ed2f7218d09b8b79fa2550c8d89ad8a..35250e9e2634726989eb5c5a8bdd904aecc930a1 100644 --- a/doc/api/namespaces.md +++ b/doc/api/namespaces.md @@ -29,22 +29,31 @@ Example response: { "id": 1, "path": "user1", - "kind": "user" + "kind": "user", + "full_path": "user1" }, { "id": 2, "path": "group1", "kind": "group" + "full_path": "group1", + "parent_id": "null", + "members_count_with_descendants": 2, + "plan": "bronze" }, { "id": 3, "path": "bar", "kind": "group", "full_path": "foo/bar", + "parent_id": "9", + "members_count_with_descendants": 5 } ] ``` +**Note**: `members_count_with_descendants` and `plan` are presented only for group masters/owners. + ## Search for namespace Get all namespaces that match a string in their name or path. @@ -72,6 +81,8 @@ Example response: "path": "twitter", "kind": "group", "full_path": "twitter", + "parent_id": "null", + "members_count_with_descendants": 2 } ] ``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 5e5c24156590292af091cc3142ca72d790d6f858..91d76ab3bf1ad12b3d3d9a19aabb51e70e2c8b78 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -510,11 +510,19 @@ class Todo < Grape::Entity end class Namespace < Grape::Entity - expose :id, :name, :path, :kind, :full_path + expose :id, :name, :path, :kind, :full_path, :parent_id + + 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 # EE-only expose :shared_runners_minutes_limit, if: lambda { |_, options| options[:current_user]&.admin? } - expose :plan, if: lambda { |_, options| options[:current_user]&.admin? } + expose :plan, if: -> (namespace, opts) { Ability.allowed?(opts[:current_user], :admin_namespace, namespace) } end class MemberAccess < Grape::Entity diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index 97942f6ec0604fdc162fb5a411933d7c4f12f569..6d2ec3a70809368af52b6ecc7b6fdae5d0737aed 100644 --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -17,7 +17,7 @@ class Namespaces < Grape::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 desc 'Update a namespace' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 5a441a13a0048fb9e64430c001e939510353ed5b..fd8f2ef8fc6dda58271ad3b9d098559c9a9b1350 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -385,6 +385,25 @@ def setup_group_members(group) end end + describe '#users_with_descendants', :nested_groups do + let(:user_a) { create(:user) } + let(:user_b) { create(:user) } + + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:deep_nested_group) { create(:group, parent: nested_group) } + + it 'returns member users on every nest level without duplication' do + group.add_developer(user_a) + nested_group.add_developer(user_b) + deep_nested_group.add_developer(user_a) + + expect(group.users_with_descendants).to contain_exactly(user_a, user_b) + expect(nested_group.users_with_descendants).to contain_exactly(user_a, user_b) + expect(deep_nested_group.users_with_descendants).to contain_exactly(user_a) + end + end + describe '#user_ids_for_project_authorizations' do it 'returns the user IDs for which to refresh authorizations' do master = create(:user) diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index 18f1b4ccb887fb7fde5d4178fa6cee4ddb34dac4..56b3e2a10c56afdb00e49c8cc1ed5a2ac2cba518 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -15,6 +15,22 @@ end context "when authenticated as admin" 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(group_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', + 'parent_id', 'members_count_with_descendants', + 'plan', 'shared_runners_minutes_limit') + + expect(user_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', + 'parent_id', 'plan', 'shared_runners_minutes_limit') + end + it "admin: returns an array of all namespaces" do get api("/namespaces", admin) @@ -37,6 +53,27 @@ end context "when authenticated as a regular user" do + it "returns correct attributes when user can admin group" do + group1.add_owner(user) + + get api("/namespaces", user) + + 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', + 'plan', 'parent_id', 'members_count_with_descendants') + end + + it "returns correct attributes when 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 get api("/namespaces", user) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 58cce26e354b1ecdb7ed96441c2b7b43a5149128..b7ec7a4a6f0bf02c32ece5b5eba552875bd0124c 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -748,7 +748,9 @@ 'name' => user.namespace.name, 'path' => user.namespace.path, 'kind' => user.namespace.kind, - 'full_path' => user.namespace.full_path + 'full_path' => user.namespace.full_path, + 'parent_id' => nil, + 'plan' => nil }) end diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb index 9b26929a167d754560012a3e8592c3edcf8f7872..aca85240386a52429057841dacb7f456f1c16679 100644 --- a/spec/requests/api/v3/projects_spec.rb +++ b/spec/requests/api/v3/projects_spec.rb @@ -785,7 +785,9 @@ 'name' => user.namespace.name, 'path' => user.namespace.path, 'kind' => user.namespace.kind, - 'full_path' => user.namespace.full_path + 'full_path' => user.namespace.full_path, + 'parent_id' => nil, + 'plan' => nil }) end