From b4826d53b5bdaaef6dc4329466f853847dac2bef Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin <viakliushin@gitlab.com> Date: Mon, 8 Feb 2021 16:39:18 +0100 Subject: [PATCH] Add groups endpoint for Projects API Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/28902 * Add `ProjectGroupFinder` to fetch ancestor and shared groups for the project * Add `projects/:id/groups` endpoint to Project API * Add documentation for the new endpoint --- app/finders/projects/groups_finder.rb | 68 +++++++++++ .../unreleased/28902_extend_group_api.yml | 5 + doc/api/projects.md | 37 ++++++ lib/api/entities/public_group_details.rb | 12 ++ lib/api/projects.rb | 30 +++++ spec/finders/projects/groups_finder_spec.rb | 103 ++++++++++++++++ .../api/entities/public_group_details_spec.rb | 24 ++++ spec/requests/api/projects_spec.rb | 114 ++++++++++++++++++ 8 files changed, 393 insertions(+) create mode 100644 app/finders/projects/groups_finder.rb create mode 100644 changelogs/unreleased/28902_extend_group_api.yml create mode 100644 lib/api/entities/public_group_details.rb create mode 100644 spec/finders/projects/groups_finder_spec.rb create mode 100644 spec/lib/api/entities/public_group_details_spec.rb diff --git a/app/finders/projects/groups_finder.rb b/app/finders/projects/groups_finder.rb new file mode 100644 index 0000000000000..d0c42ad5611df --- /dev/null +++ b/app/finders/projects/groups_finder.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +# Used to filter ancestor and shared project's Groups by a set of params +# +# Arguments: +# project +# current_user - which user is requesting groups +# params: +# with_shared: boolean (optional) +# shared_min_access_level: integer (optional) +# skip_groups: array of integers (optional) +# +module Projects + class GroupsFinder < UnionFinder + def initialize(project:, current_user: nil, params: {}) + @project = project + @current_user = current_user + @params = params + end + + def execute + return Group.none unless authorized? + + items = all_groups.map do |item| + item = exclude_group_ids(item) + item + end + + find_union(items, Group).with_route.order_id_desc + end + + private + + attr_reader :project, :current_user, :params + + def authorized? + Ability.allowed?(current_user, :read_project, project) + end + + # rubocop: disable CodeReuse/ActiveRecord + def all_groups + groups = [] + groups << project.group.self_and_ancestors if project.group + + if params[:with_shared] + shared_groups = project.invited_groups + + if params[:shared_min_access_level] + shared_groups = shared_groups.where( + 'project_group_links.group_access >= ?', params[:shared_min_access_level] + ) + end + + groups << shared_groups + end + + groups << Group.none if groups.compact.empty? + groups + end + # rubocop: enable CodeReuse/ActiveRecord + + def exclude_group_ids(groups) + return groups unless params[:skip_groups] + + groups.id_not_in(params[:skip_groups]) + end + end +end diff --git a/changelogs/unreleased/28902_extend_group_api.yml b/changelogs/unreleased/28902_extend_group_api.yml new file mode 100644 index 0000000000000..249ebb0039e85 --- /dev/null +++ b/changelogs/unreleased/28902_extend_group_api.yml @@ -0,0 +1,5 @@ +--- +title: Add groups endpoint for Projects API +merge_request: 53642 +author: +type: added diff --git a/doc/api/projects.md b/doc/api/projects.md index b868731469725..9c4ad2fd53a39 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1033,6 +1033,43 @@ GET /projects/:id/users ] ``` +## List a project's groups + +Get a list of ancestor groups for this project. + +```plaintext +GET /projects/:id/groups +``` + +| Attribute | Type | Required | Description | +|-----------------------------|-------------------|------------------------|-------------| +| `id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding). | +| `search` | string | **{dotted-circle}** No | Search for specific groups. | +| `skip_groups` | array of integers | **{dotted-circle}** No | Skip the group IDs passed. | +| `with_shared` | boolean | **{dotted-circle}** No | Include projects shared with this group. Default is `false`. | +| `shared_min_access_level` | integer | **{dotted-circle}** No | Limit to shared groups with at least this [access level](members.md#valid-access-levels). | + +```json +[ + { + "id": 1, + "name": "Foobar Group", + "avatar_url": "http://localhost:3000/uploads/group/avatar/1/foo.jpg", + "web_url": "http://localhost:3000/groups/foo-bar", + "full_name": "Foobar Group", + "full_path": "foo-bar", + }, + { + "id": 2, + "name": "Shared Group", + "avatar_url": "http://gitlab.example.com/uploads/group/avatar/1/bar.jpg", + "web_url": "http://gitlab.example.com/groups/foo/bar", + "full_name": "Shared Group", + "full_path": "foo/shared", + } +] +``` + ## Get project events Refer to the [Events API documentation](events.md#list-a-projects-visible-events). diff --git a/lib/api/entities/public_group_details.rb b/lib/api/entities/public_group_details.rb new file mode 100644 index 0000000000000..0dfe28d251a3f --- /dev/null +++ b/lib/api/entities/public_group_details.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module API + module Entities + class PublicGroupDetails < BasicGroupDetails + expose :avatar_url do |group, options| + group.avatar_url(only_path: false) + end + expose :full_name, :full_path + end + end +end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index fca68c3606bc8..19b63c28f8947 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -136,6 +136,17 @@ def present_projects(projects, options = {}) present records, options end + def present_groups(groups) + options = { + with: Entities::PublicGroupDetails, + current_user: current_user + } + + groups, options = with_custom_attributes(groups, options) + + present paginate(groups), options + end + def translate_params_for_compatibility(params) params[:builds_enabled] = params.delete(:jobs_enabled) if params.key?(:jobs_enabled) params @@ -561,6 +572,25 @@ def translate_params_for_compatibility(params) present paginate(users), with: Entities::UserBasic end + desc 'Get ancestor and shared groups for a project' do + success Entities::PublicGroupDetails + end + params do + optional :search, type: String, desc: 'Return list of groups matching the search criteria' + optional :skip_groups, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Array of group ids to exclude from list' + optional :with_shared, type: Boolean, default: false, + desc: 'Include shared groups' + optional :shared_min_access_level, type: Integer, values: Gitlab::Access.all_values, + desc: 'Limit returned shared groups by minimum access level to the project' + use :pagination + end + get ':id/groups', feature_category: :source_code_management do + groups = ::Projects::GroupsFinder.new(project: user_project, current_user: current_user, params: declared_params(include_missing: false)).execute + groups = groups.search(params[:search]) if params[:search].present? + + present_groups groups + end + desc 'Start the housekeeping task for a project' do detail 'This feature was introduced in GitLab 9.0.' end diff --git a/spec/finders/projects/groups_finder_spec.rb b/spec/finders/projects/groups_finder_spec.rb new file mode 100644 index 0000000000000..89d4edaec7c16 --- /dev/null +++ b/spec/finders/projects/groups_finder_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::GroupsFinder do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:root_group) { create(:group, :public) } + let_it_be(:project_group) { create(:group, :public, parent: root_group) } + let_it_be(:shared_group_with_dev_access) { create(:group, :private, parent: root_group) } + let_it_be(:shared_group_with_reporter_access) { create(:group, :private) } + + let_it_be(:public_project) { create(:project, :public, group: project_group) } + let_it_be(:private_project) { create(:project, :private, group: project_group) } + + before_all do + [public_project, private_project].each do |project| + create(:project_group_link, :developer, group: shared_group_with_dev_access, project: project) + create(:project_group_link, :reporter, group: shared_group_with_reporter_access, project: project) + end + end + + let(:params) { {} } + let(:current_user) { user } + let(:finder) { described_class.new(project: project, current_user: current_user, params: params) } + + subject { finder.execute } + + shared_examples 'finding related groups' do + it 'returns ancestor groups for this project' do + is_expected.to match_array([project_group, root_group]) + end + + context 'when the project does not belong to any group' do + before do + allow(project).to receive(:group) { nil } + end + + it { is_expected.to eq([]) } + end + + context 'when shared groups option is on' do + let(:params) { { with_shared: true } } + + it 'returns ancestor and all shared groups' do + is_expected.to match_array([project_group, root_group, shared_group_with_dev_access, shared_group_with_reporter_access]) + end + + context 'when shared_min_access_level is developer' do + let(:params) { super().merge(shared_min_access_level: Gitlab::Access::DEVELOPER) } + + it 'returns ancestor and shared groups with at least developer access' do + is_expected.to match_array([project_group, root_group, shared_group_with_dev_access]) + end + end + end + + context 'when skip group option is on' do + let(:params) { { skip_groups: [project_group.id] } } + + it 'excludes provided groups' do + is_expected.to match_array([root_group]) + end + end + end + + context 'Public project' do + it_behaves_like 'finding related groups' do + let(:project) { public_project } + + context 'when user is not authorized' do + let(:current_user) { nil } + + it 'returns ancestor groups for this project' do + is_expected.to match_array([project_group, root_group]) + end + end + end + end + + context 'Private project' do + it_behaves_like 'finding related groups' do + let(:project) { private_project } + + before do + project.add_developer(user) + end + + context 'when user is not authorized' do + let(:current_user) { nil } + + it { is_expected.to eq([]) } + end + end + end + + context 'Missing project' do + let(:project) { nil } + + it { is_expected.to eq([]) } + end + end +end diff --git a/spec/lib/api/entities/public_group_details_spec.rb b/spec/lib/api/entities/public_group_details_spec.rb new file mode 100644 index 0000000000000..34162ed00ca28 --- /dev/null +++ b/spec/lib/api/entities/public_group_details_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Entities::PublicGroupDetails do + subject(:entity) { described_class.new(group) } + + let(:group) { create(:group, :with_avatar) } + + describe '#as_json' do + subject { entity.as_json } + + it 'includes public group fields' do + is_expected.to eq( + id: group.id, + name: group.name, + web_url: group.web_url, + avatar_url: group.avatar_url(only_path: false), + full_name: group.full_name, + full_path: group.full_path + ) + end + end +end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 8a68db6ae4b9c..95e743e67ff6f 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1478,6 +1478,120 @@ end end + describe "GET /projects/:id/groups" do + let_it_be(:root_group) { create(:group, :public, name: 'root group') } + let_it_be(:project_group) { create(:group, :public, parent: root_group, name: 'project group') } + let_it_be(:shared_group_with_dev_access) { create(:group, :private, parent: root_group, name: 'shared group') } + let_it_be(:shared_group_with_reporter_access) { create(:group, :private) } + let_it_be(:private_project) { create(:project, :private, group: project_group) } + let_it_be(:public_project) { create(:project, :public, group: project_group) } + + before_all do + create(:project_group_link, :developer, group: shared_group_with_dev_access, project: private_project) + create(:project_group_link, :reporter, group: shared_group_with_reporter_access, project: private_project) + end + + shared_examples_for 'successful groups response' do + it 'returns an array of groups' do + request + + aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |g| g['name'] }).to match_array(expected_groups.map(&:name)) + end + end + end + + context 'when unauthenticated' do + it 'does not return groups for private projects' do + get api("/projects/#{private_project.id}/groups") + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'for public projects' do + let(:request) { get api("/projects/#{public_project.id}/groups") } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group] } + end + end + end + + context 'when authenticated as user' do + context 'when user does not have access to the project' do + it 'does not return groups' do + get api("/projects/#{private_project.id}/groups", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user has access to the project' do + let(:request) { get api("/projects/#{private_project.id}/groups", user), params: params } + let(:params) { {} } + + before do + private_project.add_developer(user) + end + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group] } + end + + context 'when search by root group name' do + let(:params) { { search: 'root' } } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group] } + end + end + + context 'with_shared option is on' do + let(:params) { { with_shared: true } } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group, shared_group_with_dev_access, shared_group_with_reporter_access] } + end + + context 'when shared_min_access_level is set' do + let(:params) { super().merge(shared_min_access_level: Gitlab::Access::DEVELOPER) } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group, shared_group_with_dev_access] } + end + end + + context 'when search by shared group name' do + let(:params) { super().merge(search: 'shared') } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [shared_group_with_dev_access] } + end + end + + context 'when skip_groups is set' do + let(:params) { super().merge(skip_groups: [shared_group_with_dev_access.id, root_group.id]) } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [shared_group_with_reporter_access, project_group] } + end + end + end + end + end + + context 'when authenticated as admin' do + let(:request) { get api("/projects/#{private_project.id}/groups", admin) } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group] } + end + end + end + describe 'GET /projects/:id' do context 'when unauthenticated' do it 'does not return private projects' do -- GitLab