From 26b6dfaf6f74a8b1e9cc1b05c30879f8a60f86bb Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 9 Nov 2017 16:07:04 +0000 Subject: Add /groups/:id/subgroups endpoint to API --- changelogs/unreleased/winh-subgroups-api.yml | 5 + doc/api/groups.md | 49 +++++++++- lib/api/groups.rb | 66 ++++++++----- spec/requests/api/groups_spec.rb | 136 +++++++++++++++++++++++++++ 4 files changed, 230 insertions(+), 26 deletions(-) create mode 100644 changelogs/unreleased/winh-subgroups-api.yml diff --git a/changelogs/unreleased/winh-subgroups-api.yml b/changelogs/unreleased/winh-subgroups-api.yml new file mode 100644 index 00000000000..c49e3621e9c --- /dev/null +++ b/changelogs/unreleased/winh-subgroups-api.yml @@ -0,0 +1,5 @@ +--- +title: Add /groups/:id/subgroups endpoint to API +merge_request: 15142 +author: marbemac +type: added diff --git a/doc/api/groups.md b/doc/api/groups.md index 16db9c2f259..6a6e94195a7 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -9,13 +9,13 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `skip_groups` | array of integers | no | Skip the group IDs passes | -| `all_available` | boolean | no | Show all the groups you have access to | -| `search` | string | no | Return list of authorized groups matching the search criteria | +| `skip_groups` | array of integers | no | Skip the group IDs passed | +| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users) | +| `search` | string | no | Return the list of authorized groups matching the search criteria | | `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | -| `owned` | boolean | no | Limit by groups owned by the current user | +| `owned` | boolean | no | Limit to groups owned by the current user | ``` GET /groups @@ -80,6 +80,47 @@ You can filter by [custom attributes](custom_attributes.md) with: GET /groups?custom_attributes[key]=value&custom_attributes[other_key]=other_value ``` +## List a groups's subgroups + +Get a list of visible direct subgroups in this group. +When accessed without authentication, only public groups are returned. + +Parameters: + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) of the parent group | +| `skip_groups` | array of integers | no | Skip the group IDs passed | +| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users) | +| `search` | string | no | Return the list of authorized groups matching the search criteria | +| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | +| `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | +| `statistics` | boolean | no | Include group statistics (admins only) | +| `owned` | boolean | no | Limit to groups owned by the current user | + +``` +GET /groups/:id/subgroups +``` + +```json +[ + { + "id": 1, + "name": "Foobar Group", + "path": "foo-bar", + "description": "An interesting group", + "visibility": "public", + "lfs_enabled": true, + "avatar_url": "http://gitlab.example.com/uploads/group/avatar/1/foo.jpg", + "web_url": "http://gitlab.example.com/groups/foo-bar", + "request_access_enabled": false, + "full_name": "Foobar Group", + "full_path": "foo-bar", + "parent_id": 123 + } +] +``` + ## List a group's projects Get a list of projects in this group. When accessed without authentication, only diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 340a7cecf09..bcf2e6dae1d 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -25,24 +25,7 @@ module API optional :statistics, type: Boolean, default: false, desc: 'Include project statistics' end - def present_groups(groups, options = {}) - options = options.reverse_merge( - with: Entities::Group, - current_user: current_user - ) - - groups = groups.with_statistics if options[:statistics] - present paginate(groups), options - end - end - - resource :groups do - include CustomAttributesEndpoints - - desc 'Get a groups list' do - success Entities::Group - end - params do + params :group_list_params do use :statistics_params optional :skip_groups, type: Array[Integer], desc: 'Array of group ids to exclude from list' optional :all_available, type: Boolean, desc: 'Show all group that you have access to' @@ -52,19 +35,47 @@ module API optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' use :pagination end - get do + + def find_groups(params) find_params = { all_available: params[:all_available], - owned: params[:owned], - custom_attributes: params[:custom_attributes] + custom_attributes: params[:custom_attributes], + owned: params[:owned] } + find_params[:parent] = find_group!(params[:id]) if params[:id] groups = GroupsFinder.new(current_user, find_params).execute groups = groups.search(params[:search]) if params[:search].present? groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present? groups = groups.reorder(params[:order_by] => params[:sort]) - present_groups groups, statistics: params[:statistics] && current_user.admin? + groups + end + + def present_groups(params, groups) + options = { + with: Entities::Group, + current_user: current_user, + statistics: params[:statistics] && current_user.admin? + } + + groups = groups.with_statistics if options[:statistics] + present paginate(groups), options + end + end + + resource :groups do + include CustomAttributesEndpoints + + desc 'Get a groups list' do + success Entities::Group + end + params do + use :group_list_params + end + get do + groups = find_groups(params) + present_groups params, groups end desc 'Create a group. Available only for users who can create groups.' do @@ -166,6 +177,17 @@ module API present paginate(projects), with: entity, current_user: current_user end + desc 'Get a list of subgroups in this group.' do + success Entities::Group + end + params do + use :group_list_params + end + get ":id/subgroups" do + groups = find_groups(params) + present_groups params, groups + end + desc 'Transfer a project to the group namespace. Available only for admin.' do success Entities::GroupDetail end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 780dbce6488..04a658cd6c3 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -427,6 +427,142 @@ describe API::Groups do end end + describe 'GET /groups/:id/subgroups', :nested_groups do + let!(:subgroup1) { create(:group, parent: group1) } + let!(:subgroup2) { create(:group, :private, parent: group1) } + let!(:subgroup3) { create(:group, :private, parent: group2) } + + context 'when unauthenticated' do + it 'returns only public subgroups' do + get api("/groups/#{group1.id}/subgroups") + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(subgroup1.id) + expect(json_response.first['parent_id']).to eq(group1.id) + end + + it 'returns 404 for a private group' do + get api("/groups/#{group2.id}/subgroups") + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when authenticated as user' do + context 'when user is not member of a public group' do + it 'returns no subgroups for the public group' do + get api("/groups/#{group1.id}/subgroups", user2) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(0) + end + + context 'when using all_available in request' do + it 'returns public subgroups' do + get api("/groups/#{group1.id}/subgroups", user2), all_available: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response[0]['id']).to eq(subgroup1.id) + expect(json_response[0]['parent_id']).to eq(group1.id) + end + end + end + + context 'when user is not member of a private group' do + it 'returns 404 for the private group' do + get api("/groups/#{group2.id}/subgroups", user1) + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when user is member of public group' do + before do + group1.add_guest(user2) + end + + it 'returns private subgroups' do + get api("/groups/#{group1.id}/subgroups", user2) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + private_subgroups = json_response.select { |group| group['visibility'] == 'private' } + expect(private_subgroups.length).to eq(1) + expect(private_subgroups.first['id']).to eq(subgroup2.id) + expect(private_subgroups.first['parent_id']).to eq(group1.id) + end + + context 'when using statistics in request' do + it 'does not include statistics' do + get api("/groups/#{group1.id}/subgroups", user2), statistics: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first).not_to include 'statistics' + end + end + end + + context 'when user is member of private group' do + before do + group2.add_guest(user1) + end + + it 'returns subgroups' do + get api("/groups/#{group2.id}/subgroups", user1) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(subgroup3.id) + expect(json_response.first['parent_id']).to eq(group2.id) + end + end + end + + context 'when authenticated as admin' do + it 'returns private subgroups of a public group' do + get api("/groups/#{group1.id}/subgroups", admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + end + + it 'returns subgroups of a private group' do + get api("/groups/#{group2.id}/subgroups", admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + end + + it 'does not include statistics by default' do + get api("/groups/#{group1.id}/subgroups", admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first).not_to include('statistics') + end + + it 'includes statistics if requested' do + get api("/groups/#{group1.id}/subgroups", admin), statistics: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first).to include('statistics') + end + end + end + describe "POST /groups" do context "when authenticated as user without group permissions" do it "does not create group" do -- cgit v1.2.1