Use Grape DSL to document methods and their paramsrc-members-services-based-on-18583

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 80 insertions, 98 deletions

diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb
index fb495255367..6c454bd1703 100644
--- a/lib/api/access_requests.rb
+++ b/lib/api/access_requests.rb
@@ -5,15 +5,14 @@ module API
     helpers ::API::Helpers::MembersHelpers
 
     %w[group project].each do |source_type|
+      params do
+        requires :id, type: String, desc: "The #{source_type} ID"
+      end
       resource source_type.pluralize do
-        # Get a list of group/project access requests viewable by the authenticated user.
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #
-        # Example Request:
-        #  GET /groups/:id/access_requests
-        #  GET /projects/:id/access_requests
+        desc "Gets a list of access requests for a #{source_type}." do
+          detail 'This feature was introduced in GitLab 8.11.'
+          success Entities::AccessRequester
+        end
         get ":id/access_requests" do
           source = find_source(source_type, params[:id])
 
@@ -23,14 +22,10 @@ module API
           present access_requesters.map(&:user), with: Entities::AccessRequester, access_requesters: access_requesters
         end
 
-        # Request access to the group/project
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #
-        # Example Request:
-        #  POST /groups/:id/access_requests
-        #  POST /projects/:id/access_requests
+        desc "Requests access for the authenticated user to a #{source_type}." do
+          detail 'This feature was introduced in GitLab 8.11.'
+          success Entities::AccessRequester
+        end
         post ":id/access_requests" do
           source = find_source(source_type, params[:id])
           access_requester = ::Members::RequestAccessService.new(source, current_user).execute
@@ -42,37 +37,30 @@ module API
           end
         end
 
-        # Approve a group/project access request
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   user_id (required) - The user ID of the access requester
-        #   access_level (optional) - Access level
-        #
-        # Example Request:
-        #   PUT /groups/:id/access_requests/:user_id/approve
-        #   PUT /projects/:id/access_requests/:user_id/approve
+        desc 'Approves an access request for the given user.' do
+          detail 'This feature was introduced in GitLab 8.11.'
+          success Entities::Member
+        end
+        params do
+          requires :user_id, type: Integer, desc: 'The user ID of the access requester'
+          optional :access_level, type: Integer, desc: 'A valid access level (defaults: `30`, developer access level)'
+        end
         put ':id/access_requests/:user_id/approve' do
-          required_attributes! [:user_id]
           source = find_source(source_type, params[:id])
 
-          member = ::Members::ApproveAccessRequestService.new(source, current_user, params).execute
+          member = ::Members::ApproveAccessRequestService.new(source, current_user, declared(params)).execute
 
           status :created
           present member.user, with: Entities::Member, member: member
         end
 
-        # Deny a group/project access request
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   user_id (required) - The user ID of the access requester
-        #
-        # Example Request:
-        #   DELETE /groups/:id/access_requests/:user_id
-        #   DELETE /projects/:id/access_requests/:user_id
+        desc 'Denies an access request for the given user.' do
+          detail 'This feature was introduced in GitLab 8.11.'
+        end
+        params do
+          requires :user_id, type: Integer, desc: 'The user ID of the access requester'
+        end
         delete ":id/access_requests/:user_id" do
-          required_attributes! [:user_id]
           source = find_source(source_type, params[:id])
 
           ::Members::DestroyService.new(source, current_user, declared(params)).execute(:requesters)
diff --git a/lib/api/members.rb b/lib/api/members.rb
index 4cc80f09a9b..9fca3e461a5 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -5,16 +5,16 @@ module API
     helpers ::API::Helpers::MembersHelpers
 
     %w[group project].each do |source_type|
+      params do
+        requires :id, type: String, desc: "The #{source_type} ID"
+      end
       resource source_type.pluralize do
-        # Get a list of group/project members viewable by the authenticated user.
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   query         - Query string
-        #
-        # Example Request:
-        #   GET /groups/:id/members
-        #   GET /projects/:id/members
+        desc 'Gets a list of group or project members viewable by the authenticated user.' do
+          success Entities::Member
+        end
+        params do
+          optional :query, type: String, desc: 'A query string to search for members'
+        end
         get ":id/members" do
           source = find_source(source_type, params[:id])
 
@@ -25,15 +25,12 @@ module API
           present members.map(&:user), with: Entities::Member, members: members
         end
 
-        # Get a group/project member
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   user_id (required) - The user ID of the member
-        #
-        # Example Request:
-        #   GET /groups/:id/members/:user_id
-        #   GET /projects/:id/members/:user_id
+        desc 'Gets a member of a group or project.' do
+          success Entities::Member
+        end
+        params do
+          requires :user_id, type: Integer, desc: 'The user ID of the member'
+        end
         get ":id/members/:user_id" do
           source = find_source(source_type, params[:id])
 
@@ -43,26 +40,25 @@ module API
           present member.user, with: Entities::Member, member: member
         end
 
-        # Add a new group/project member
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   user_id (required) - The user ID of the new member
-        #   access_level (required) - A valid access level
-        #   expires_at (optional) - Date string in the format YEAR-MONTH-DAY
-        #
-        # Example Request:
-        #   POST /groups/:id/members
-        #   POST /projects/:id/members
+        desc 'Adds a member to a group or project.' do
+          success Entities::Member
+        end
+        params do
+          requires :user_id, type: Integer, desc: 'The user ID of the new member'
+          requires :access_level, type: Integer, desc: 'A valid access level (defaults: `30`, developer access level)'
+          optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY'
+        end
         post ":id/members" do
           source = find_source(source_type, params[:id])
           authorize_admin_source!(source_type, source)
-          required_attributes! [:user_id, :access_level]
 
           member = source.members.find_by(user_id: params[:user_id])
 
-          # This is to ensure back-compatibility but 409 behavior should be used
-          # for both project and group members in 9.0!
+          # We need this explicit check because `source.add_user` doesn't
+          # currently return the member created so it would return 201 even if
+          # the member already existed...
+          # The `source_type == 'group'` check is to ensure back-compatibility
+          # but 409 behavior should be used for both project and group members in 9.0!
           conflict!('Member already exists') if source_type == 'group' && member
 
           access_requester = source.requesters.find_by(user_id: params[:user_id])
@@ -70,7 +66,7 @@ module API
             # We delete a potential access requester before creating the new member.
             # We pass current_user = access_requester so that the requester doesn't
             # receive a "access denied" email.
-            ::Members::DestroyService.new(source, access_requester.user, params).execute(:requesters)
+            ::Members::DestroyService.new(source, access_requester.user, declared(params)).execute(:requesters)
           end
 
           unless member
@@ -93,21 +89,17 @@ module API
           end
         end
 
-        # Update a group/project member
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   user_id (required) - The user ID of the member
-        #   access_level (required) - A valid access level
-        #   expires_at (optional) - Date string in the format YEAR-MONTH-DAY
-        #
-        # Example Request:
-        #   PUT /groups/:id/members/:user_id
-        #   PUT /projects/:id/members/:user_id
+        desc 'Updates a member of a group or project.' do
+          success Entities::Member
+        end
+        params do
+          requires :user_id, type: Integer, desc: 'The user ID of the new member'
+          requires :access_level, type: Integer, desc: 'A valid access level'
+          optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY'
+        end
         put ":id/members/:user_id" do
           source = find_source(source_type, params[:id])
           authorize_admin_source!(source_type, source)
-          required_attributes! [:user_id, :access_level]
 
           member = source.members.find_by!(user_id: params[:user_id])
           attrs = attributes_for_keys [:access_level, :expires_at]
@@ -122,18 +114,12 @@ module API
           end
         end
 
-        # Remove a group/project member
-        #
-        # Parameters:
-        #   id (required) - The group/project ID
-        #   user_id (required) - The user ID of the member
-        #
-        # Example Request:
-        #   DELETE /groups/:id/members/:user_id
-        #   DELETE /projects/:id/members/:user_id
+        desc 'Removes a user from a group or project.'
+        params do
+          requires :user_id, type: Integer, desc: 'The user ID of the member'
+        end
         delete ":id/members/:user_id" do
           source = find_source(source_type, params[:id])
-          required_attributes! [:user_id]
 
           # This is to ensure back-compatibility but find_by! should be used
           # in that casse in 9.0!
@@ -148,7 +134,7 @@ module API
           if member.nil?
             { message: "Access revoked", id: params[:user_id].to_i }
           else
-            ::Members::DestroyService.new(source, current_user, params).execute
+            ::Members::DestroyService.new(source, current_user, declared(params)).execute
 
             present member.user, with: Entities::Member, member: member
           end
diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb
index 1e365bf353a..8da2b0c5f18 100644
--- a/spec/requests/api/members_spec.rb
+++ b/spec/requests/api/members_spec.rb
@@ -88,7 +88,10 @@ describe API::Members, api: true  do
   shared_examples 'POST /:sources/:id/members' do |source_type|
     context "with :sources == #{source_type.pluralize}" do
       it_behaves_like 'a 404 response when source is private' do
-        let(:route) { post api("/#{source_type.pluralize}/#{source.id}/members", stranger) }
+        let(:route) do
+          post api("/#{source_type.pluralize}/#{source.id}/members", stranger),
+               user_id: access_requester.id, access_level: Member::MASTER
+        end
       end
 
       context 'when authenticated as a non-member or member with insufficient rights' do
@@ -96,7 +99,8 @@ describe API::Members, api: true  do
           context "as a #{type}" do
             it 'returns 403' do
               user = public_send(type)
-              post api("/#{source_type.pluralize}/#{source.id}/members", user)
+              post api("/#{source_type.pluralize}/#{source.id}/members", user),
+                   user_id: access_requester.id, access_level: Member::MASTER
 
               expect(response).to have_http_status(403)
             end
@@ -165,7 +169,10 @@ describe API::Members, api: true  do
   shared_examples 'PUT /:sources/:id/members/:user_id' do |source_type|
     context "with :sources == #{source_type.pluralize}" do
       it_behaves_like 'a 404 response when source is private' do
-        let(:route) { put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) }
+        let(:route) do
+          put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger),
+              access_level: Member::MASTER
+        end
       end
 
       context 'when authenticated as a non-member or member with insufficient rights' do
@@ -173,7 +180,8 @@ describe API::Members, api: true  do
           context "as a #{type}" do
             it 'returns 403' do
               user = public_send(type)
-              put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user)
+              put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user),
+                  access_level: Member::MASTER
 
               expect(response).to have_http_status(403)
             end