Merge branch 'improve-group-members-api' into 'master'

Improve group members api

Related #1720

See merge request !1225

3 files changed, 81 insertions, 51 deletions

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 2c7cd9038c3..d26667ba3f7 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -27,6 +27,7 @@ module API
     helpers APIHelpers
 
     mount Groups
+    mount GroupMembers
     mount Users
     mount Projects
     mount Repositories
diff --git a/lib/api/group_members.rb b/lib/api/group_members.rb
new file mode 100644
index 00000000000..d596517c816
--- /dev/null
+++ b/lib/api/group_members.rb
@@ -0,0 +1,80 @@
+module API
+  class GroupMembers < Grape::API
+    before { authenticate! }
+
+    resource :groups do
+      helpers do
+        def find_group(id)
+          group = Group.find(id)
+
+          if can?(current_user, :read_group, group)
+            group
+          else
+            render_api_error!("403 Forbidden - #{current_user.username} lacks sufficient access to #{group.name}", 403)
+          end
+        end
+
+        def validate_access_level?(level)
+          Gitlab::Access.options_with_owner.values.include? level.to_i
+        end
+      end
+
+      # Get a list of group members viewable by the authenticated user.
+      #
+      # Example Request:
+      #  GET /groups/:id/members
+      get ":id/members" do
+        group = find_group(params[:id])
+        members = group.group_members
+        users = (paginate members).collect(&:user)
+        present users, with: Entities::GroupMember, group: group
+      end
+
+      # Add a user to the list of group members
+      #
+      # Parameters:
+      #   id (required) - group id
+      #   user_id (required) - the users id
+      #   access_level (required) - Project access level
+      # Example Request:
+      #  POST /groups/:id/members
+      post ":id/members" do
+        group = find_group(params[:id])
+        authorize! :manage_group, group
+        required_attributes! [:user_id, :access_level]
+
+        unless validate_access_level?(params[:access_level])
+          render_api_error!("Wrong access level", 422)
+        end
+
+        if group.group_members.find_by(user_id: params[:user_id])
+          render_api_error!("Already exists", 409)
+        end
+
+        group.add_users([params[:user_id]], params[:access_level])
+        member = group.group_members.find_by(user_id: params[:user_id])
+        present member.user, with: Entities::GroupMember, group: group
+      end
+
+      # Remove member.
+      #
+      # Parameters:
+      #   id (required) - group id
+      #   user_id (required) - the users id
+      #
+      # Example Request:
+      #   DELETE /groups/:id/members/:user_id
+      delete ":id/members/:user_id" do
+        group = find_group(params[:id])
+        authorize! :manage_group, group
+        member = group.group_members.find_by(user_id: params[:user_id])
+
+        if member.nil?
+          render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}",404)
+        else
+          member.destroy
+        end
+      end
+    end
+  end
+end
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 4841e04689d..f0ab6938b1c 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -97,57 +97,6 @@ module API
           not_found!
         end
       end
-
-      # Get a list of group members viewable by the authenticated user.
-      #
-      # Example Request:
-      #  GET /groups/:id/members
-      get ":id/members" do
-        group = find_group(params[:id])
-        members = group.group_members
-        users = (paginate members).collect(&:user)
-        present users, with: Entities::GroupMember, group: group
-      end
-
-      # Add a user to the list of group members
-      #
-      # Parameters:
-      #   id (required) - group id
-      #   user_id (required) - the users id
-      #   access_level (required) - Project access level
-      # Example Request:
-      #  POST /groups/:id/members
-      post ":id/members" do
-        required_attributes! [:user_id, :access_level]
-        unless validate_access_level?(params[:access_level])
-          render_api_error!("Wrong access level", 422)
-        end
-        group = find_group(params[:id])
-        if group.group_members.find_by(user_id: params[:user_id])
-          render_api_error!("Already exists", 409)
-        end
-        group.add_users([params[:user_id]], params[:access_level])
-        member = group.group_members.find_by(user_id: params[:user_id])
-        present member.user, with: Entities::GroupMember, group: group
-      end
-
-      # Remove member.
-      #
-      # Parameters:
-      #   id (required) - group id
-      #   user_id (required) - the users id
-      #
-      # Example Request:
-      #   DELETE /groups/:id/members/:user_id
-      delete ":id/members/:user_id" do
-        group = find_group(params[:id])
-        member =  group.group_members.find_by(user_id: params[:user_id])
-        if member.nil?
-          render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}",404)
-        else
-          member.destroy
-        end
-      end
     end
   end
 end