Merge branch 'api-remove-owned-groups' into 'master'

Api remove owned groups

Closes #27937

See merge request !9505

8 files changed, 87 insertions, 40 deletions

diff --git a/changelogs/unreleased/api-remove-owned-groups.yml b/changelogs/unreleased/api-remove-owned-groups.yml
new file mode 100644
index 00000000000..cf0301b7fe0
--- /dev/null
+++ b/changelogs/unreleased/api-remove-owned-groups.yml
@@ -0,0 +1,4 @@
+---
+title: 'API: Remove /groups/owned endpoint'
+merge_request: 9505
+author: Robert Schilling
diff --git a/doc/api/groups.md b/doc/api/groups.md
index 4a39dbc5555..39adb5be502 100644
--- a/doc/api/groups.md
+++ b/doc/api/groups.md
@@ -14,6 +14,7 @@ Parameters:
 | `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 |
 
 ```
 GET /groups
@@ -40,20 +41,6 @@ GET /groups
 
 You can search for groups by name or path, see below.
 
-## List owned groups
-
-Get a list of groups which are owned by the authenticated user.
-
-```
-GET /groups/owned
-```
-
-Parameters:
-
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `statistics` | boolean | no | Include group statistics |
-
 ## List a group's projects
 
 Get a list of projects in this group.
diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md
index e141723b580..8af041be234 100644
--- a/doc/api/v3_to_v4.md
+++ b/doc/api/v3_to_v4.md
@@ -41,5 +41,5 @@ changes are in V4:
 - Renamed `branch_name` to `branch` on DELETE `id/repository/branches/:branch` response [!8936](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8936)
 - Remove `public` param from create and edit actions of projects [!8736](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8736)
 - Notes do not return deprecated field `upvote` and `downvote` [!9384](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9384)
-- Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449)
-
+- Remove `GET /groups/owned`. Use `GET /groups?owned=true` instead [!9505](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9505)
+- Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449)
\ No newline at end of file
diff --git a/lib/api/api.rb b/lib/api/api.rb
index 1803387bb8c..dc732012a33 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -10,6 +10,7 @@ module API
       mount ::API::V3::Commits
       mount ::API::V3::DeployKeys
       mount ::API::V3::Files
+      mount ::API::V3::Groups
       mount ::API::V3::Issues
       mount ::API::V3::Labels
       mount ::API::V3::Members
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 9f29c4466ab..9cffd6180ae 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -36,12 +36,15 @@ module API
         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'
         optional :search, type: String, desc: 'Search for a specific group'
+        optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user'
         optional :order_by, type: String, values: %w[name path], default: 'name', desc: 'Order by name or path'
         optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)'
         use :pagination
       end
       get do
-        groups = if current_user.admin
+        groups = if params[:owned]
+                   current_user.owned_groups
+                 elsif current_user.admin
                    Group.all
                  elsif params[:all_available]
                    GroupsFinder.new.execute(current_user)
@@ -56,17 +59,6 @@ module API
         present_groups groups, statistics: params[:statistics] && current_user.is_admin?
       end
 
-      desc 'Get list of owned groups for authenticated user' do
-        success Entities::Group
-      end
-      params do
-        use :pagination
-        use :statistics_params
-      end
-      get '/owned' do
-        present_groups current_user.owned_groups, statistics: params[:statistics]
-      end
-
       desc 'Create a group. Available only for users who can create groups.' do
         success Entities::Group
       end
diff --git a/lib/api/v3/groups.rb b/lib/api/v3/groups.rb
new file mode 100644
index 00000000000..c826bc4fe0b
--- /dev/null
+++ b/lib/api/v3/groups.rb
@@ -0,0 +1,38 @@
+module API
+  module V3
+    class Groups < Grape::API
+      include PaginationParams
+
+      before { authenticate! }
+
+      helpers do
+        params :statistics_params do
+          optional :statistics, type: Boolean, default: false, desc: 'Include project statistics'
+        end
+
+        def present_groups(groups, options = {})
+          options = options.reverse_merge(
+            with: ::API::Entities::Group,
+            current_user: current_user,
+          )
+
+          groups = groups.with_statistics if options[:statistics]
+          present paginate(groups), options
+        end
+      end
+
+      resource :groups do
+        desc 'Get list of owned groups for authenticated user' do
+          success ::API::Entities::Group
+        end
+        params do
+          use :pagination
+          use :statistics_params
+        end
+        get '/owned' do
+          present_groups current_user.owned_groups, statistics: params[:statistics]
+        end
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 9c3a92bedbd..f54a5e77866 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -150,20 +150,10 @@ describe API::Groups, api: true  do
         expect(response_groups).to eq([group1.name, group3.name])
       end
     end
-  end
-
-  describe 'GET /groups/owned' do
-    context 'when unauthenticated' do
-      it 'returns authentication error' do
-        get api('/groups/owned')
-
-        expect(response).to have_http_status(401)
-      end
-    end
 
-    context 'when authenticated as group owner' do
+    context 'when using owned in the request' do
       it 'returns an array of groups the user owns' do
-        get api('/groups/owned', user2)
+        get api('/groups', user2), owned: true
 
         expect(response).to have_http_status(200)
         expect(response).to include_pagination_headers
diff --git a/spec/requests/api/v3/groups_spec.rb b/spec/requests/api/v3/groups_spec.rb
new file mode 100644
index 00000000000..8b29ad03737
--- /dev/null
+++ b/spec/requests/api/v3/groups_spec.rb
@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe API::V3::Groups, api: true  do
+  include ApiHelpers
+  include UploadHelpers
+
+  let(:user2) { create(:user) }
+  let!(:group2) { create(:group, :private) }
+  let!(:project2) { create(:empty_project, namespace: group2) }
+
+  before do
+    group2.add_owner(user2)
+  end
+
+  describe 'GET /groups/owned' do
+    context 'when unauthenticated' do
+      it 'returns authentication error' do
+        get v3_api('/groups/owned')
+
+        expect(response).to have_http_status(401)
+      end
+    end
+
+    context 'when authenticated as group owner' do
+      it 'returns an array of groups the user owns' do
+        get v3_api('/groups/owned', user2)
+
+        expect(response).to have_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.first['name']).to eq(group2.name)
+      end
+    end
+  end
+end