Merge branch 'id-remove-unused-approvers-actions' into 'master'

Remove unused approvals controller actions

See merge request gitlab-org/gitlab!36855

ee/app/controllers/ee/projects/merge_requests_controller.rb

@@ -5,8 +5,6 @@ module EE
     module MergeRequestsController
       extend ActiveSupport::Concern
 
-      APPROVAL_RENDERING_ACTIONS = [:approve, :approvals, :unapprove].freeze
-
       prepended do
         include DescriptionDiffActions
 
@@ -28,32 +26,6 @@ module EE
         feature_category :metrics, only: [:metrics_reports]
       end
 
-      def approve
-        unless merge_request.can_approve?(current_user)
-          return render_404
-        end
-
-        ::MergeRequests::ApprovalService
-          .new(project, current_user)
-          .execute(merge_request)
-
-        render_approvals_json
-      end
-
-      def approvals
-        render_approvals_json
-      end
-
-      def unapprove
-        if merge_request.approved_by?(current_user)
-          ::MergeRequests::RemoveApprovalService
-            .new(project, current_user)
-            .execute(merge_request)
-        end
-
-        render_approvals_json
-      end
-
       def license_scanning_reports
         reports_response(merge_request.compare_license_scanning_reports(current_user))
       end
@@ -82,46 +54,8 @@ module EE
         reports_response(merge_request.compare_metrics_reports)
       end
 
-      protected
-
-      # rubocop:disable Gitlab/ModuleWithInstanceVariables
-      # Assigning both @merge_request and @issuable like in
-      # `Projects::MergeRequests::ApplicationController`, and calling super if
-      # we don't need the extra includes requires us to disable this cop.
-      # rubocop: disable CodeReuse/ActiveRecord
-      def merge_request
-        return super unless APPROVAL_RENDERING_ACTIONS.include?(action_name.to_sym)
-
-        @issuable = @merge_request ||= project.merge_requests
-                                         .includes(
-                                           :approved_by_users,
-                                           approvers: :user
-                                         )
-                                         .find_by!(iid: params[:id])
-        super
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
-      def render_approvals_json
-        respond_to do |format|
-          format.json do
-            render json: EE::API::Entities::ApprovalState.new(
-              merge_request.approval_state,
-              current_user: current_user
-            )
-          end
-        end
-      end
-
       private
 
-      def merge_access_check
-        super_result = super
-
-        return super_result if super_result
-        return render_404 unless @merge_request.approved?
-      end
-
       def whitelist_query_limiting_ee_merge
         ::Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab/issues/4792')
       end

ee/changelogs/unreleased/id-remove-unused-approvers-actions.yml

+---
+title: Remove unused approvals controller actions
+merge_request: 36855
+author:
+type: removed

ee/config/routes/merge_requests.rb

@@ -12,10 +12,6 @@ resources :merge_requests, only: [], constraints: { id: /\d+/ } do
     get :secret_detection_reports
     get :dast_reports
 
-    get :approvals
-    post :approvals, action: :approve
-    delete :approvals, action: :unapprove
-
     post :rebase
   end
 

ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -2,94 +2,6 @@
 
 require 'spec_helper'
 
-RSpec.shared_examples 'approvals' do
-  let!(:approver) { create(:user) }
-  let!(:approval_rule) { create(:approval_project_rule, project: project, users: [approver, user], approvals_required: 2) }
-
-  before do
-    project.add_developer(approver)
-  end
-
-  describe 'approve' do
-    before do
-      post :approve,
-           params: {
-             namespace_id: project.namespace.to_param,
-             project_id: project.to_param,
-             id: merge_request.iid
-           },
-           format: :json
-    end
-
-    it 'approves the merge request' do
-      approvals = json_response
-
-      expect(response).to be_successful
-      expect(approvals['approvals_left']).to eq 1
-      expect(approvals['approved_by'].size).to eq 1
-      expect(approvals['approved_by'][0]['user']['username']).to eq user.username
-      expect(approvals['user_has_approved']).to be true
-      expect(approvals['user_can_approve']).to be false
-      expect(approvals['suggested_approvers'].size).to eq 1
-      expect(approvals['suggested_approvers'][0]['username']).to eq approver.username
-    end
-  end
-
-  describe 'approvals' do
-    let!(:approval) { create(:approval, merge_request: merge_request, user: approver) }
-
-    def get_approvals
-      get :approvals,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project.to_param,
-            id: merge_request.iid
-          },
-          format: :json
-    end
-
-    it 'shows approval information' do
-      get_approvals
-
-      approvals = json_response
-
-      expect(response).to be_successful
-      expect(approvals['approvals_left']).to eq 1
-      expect(approvals['approved_by'].size).to eq 1
-      expect(approvals['approved_by'][0]['user']['username']).to eq approver.username
-      expect(approvals['user_has_approved']).to be false
-      expect(approvals['user_can_approve']).to be true
-      expect(approvals['suggested_approvers'].size).to eq 1
-      expect(approvals['suggested_approvers'][0]['username']).to eq user.username
-    end
-  end
-
-  describe 'unapprove' do
-    let!(:approval) { create(:approval, merge_request: merge_request, user: user) }
-
-    before do
-      delete :unapprove,
-             params: {
-               namespace_id: project.namespace.to_param,
-               project_id: project.to_param,
-               id: merge_request.iid
-             },
-             format: :json
-    end
-
-    it 'unapproves the merge request' do
-      approvals = json_response
-
-      expect(response).to be_successful
-      expect(approvals['approvals_left']).to eq 2
-      expect(approvals['approved_by']).to be_empty
-      expect(approvals['user_has_approved']).to be false
-      expect(approvals['user_can_approve']).to be true
-      expect(approvals['suggested_approvers'].size).to eq 2
-    end
-  end
-end
-
 RSpec.shared_examples 'authorize read pipeline' do
   context 'public project with private builds' do
     let(:comparison_status) { {} }
@@ -125,8 +37,6 @@ RSpec.describe Projects::MergeRequestsController do
     sign_in(viewer)
   end
 
-  it_behaves_like 'approvals'
-
   describe 'PUT update' do
     before do
       project.update(approvals_before_merge: 2)
@@ -403,19 +313,6 @@ RSpec.describe Projects::MergeRequestsController do
         expect(response).to have_gitlab_http_status(:ok)
       end
     end
-
-    context 'with a forked project' do
-      let(:forked_project) { fork_project(project, fork_owner, repository: true) }
-      let(:fork_owner) { create(:user) }
-
-      before do
-        project.add_developer(fork_owner)
-        merge_request.update!(source_project: forked_project)
-        forked_project.add_reporter(user)
-      end
-
-      it_behaves_like 'approvals'
-    end
   end
 
   describe 'GET #dependency_scanning_reports' do