Add API endpoints for approval rules

Accepts nested attribute for rules

ee/app/controllers/ee/projects/merge_requests/application_controller.rb

@@ -14,6 +14,7 @@ module EE
 
         def merge_request_params_attributes
           attrs = super.push(
+            { approval_rules_attributes: [:id, :name, { user_ids: [] }, { group_ids: [] }, :approvals_required, :approval_project_rule_id, :_destroy] },
             :approvals_before_merge,
             :approver_group_ids,
             :approver_ids

ee/app/models/ee/merge_request.rb

@@ -28,6 +28,8 @@ module EE
       delegate :merge_requests_author_approval?, to: :target_project, allow_nil: true
 
       participant :participant_approvers
+
+      accepts_nested_attributes_for :approval_rules, allow_destroy: true
     end
 
     override :mergeable?

ee/app/services/ee/merge_requests/refresh_service.rb

@@ -40,21 +40,27 @@ module EE
       def update_approvers
         return yield unless project.feature_available?(:code_owners)
 
-        previous_diffs = fetch_latest_merge_request_diffs
+        if ::Feature.enabled?(:approval_rule)
+          results = yield
 
-        results = yield
+          merge_requests_for_source_branch.each(&:sync_code_owners_with_approvers)
+        else
+          previous_diffs = fetch_latest_merge_request_diffs
 
-        merge_requests = merge_requests_for_source_branch
-        ActiveRecord::Associations::Preloader.new.preload(merge_requests, :latest_merge_request_diff) # rubocop: disable CodeReuse/ActiveRecord)
+          results = yield
 
-        merge_requests.each do |merge_request|
-          previous_diff = previous_diffs.find { |diff| diff.merge_request == merge_request }
-          previous_code_owners = ::Gitlab::CodeOwners.for_merge_request(merge_request, merge_request_diff: previous_diff)
-          new_code_owners = merge_request.code_owners - previous_code_owners
+          merge_requests = merge_requests_for_source_branch
+          ActiveRecord::Associations::Preloader.new.preload(merge_requests, :latest_merge_request_diff) # rubocop: disable CodeReuse/ActiveRecord)
 
-          create_approvers(merge_request, new_code_owners)
+          merge_requests.each do |merge_request|
+            previous_diff = previous_diffs.find { |diff| diff.merge_request == merge_request }
+            previous_code_owners = ::Gitlab::CodeOwners.for_merge_request(merge_request, merge_request_diff: previous_diff)
+            new_code_owners = merge_request.code_owners - previous_code_owners
 
-          merge_request.sync_code_owners_with_approvers
+            create_approvers(merge_request, new_code_owners)
+
+            merge_request.sync_code_owners_with_approvers
+          end
         end
 
         results

ee/app/services/ee/projects/update_service.rb

@@ -79,6 +79,7 @@ module EE
       def sync_approval_rules
         return if ::Feature.enabled?(:approval_rules, project)
         return unless project.previous_changes.include?(:approvals_before_merge)
+        return if ::Feature.enabled?(:approval_rule)
 
         project.approval_rules.update_all(approvals_required: project.approvals_before_merge)
       end

ee/lib/api/helpers/approval_helpers.rb

+# frozen_string_literal: true
+
+module API
+  module Helpers
+    module ApprovalHelpers
+      def present_approval(merge_request)
+        if Feature.enabled?(:approval_rule)
+          present merge_request.approval_state, with: ::EE::API::Entities::ApprovalState, current_user: current_user
+        else
+          present merge_request.present(current_user: current_user), with: ::EE::API::Entities::MergeRequestApprovals, current_user: current_user
+        end
+      end
+    end
+  end
+end

ee/lib/api/merge_request_approvals.rb

@@ -6,6 +6,7 @@ module API
 
     ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) }
 
+    helpers ::API::Helpers::ApprovalHelpers
     helpers do
       def handle_merge_request_errors!(errors)
         if errors.has_key? :project_access
@@ -38,18 +39,29 @@ module API
         # Examples:
         #   GET /projects/:id/merge_requests/:merge_request_iid/approvals
         #
+        # @deprecated
         desc 'List approvals for merge request' do
-          success Entities::MergeRequestApprovals
+          success ::EE::API::Entities::ApprovalState
         end
         get 'approvals' do
           merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
-          present merge_request.present(current_user: current_user), with: Entities::MergeRequestApprovals, current_user: current_user
+          present_approval(merge_request)
+        end
+
+        desc 'List approval rules for merge request', {
+          success: ::EE::API::Entities::MergeRequestApprovalRules,
+          hidden: true
+        }
+        get 'approval_rules' do
+          merge_request = find_merge_request_with_access(params[:merge_request_iid])
+
+          present merge_request.approval_state, with: ::EE::API::Entities::MergeRequestApprovalRules, current_user: current_user
         end
 
         desc 'Change approval-related configuration' do
           detail 'This feature was introduced in 10.6'
-          success Entities::MergeRequestApprovals
+          success ::EE::API::Entities::ApprovalState
         end
         params do
           requires :approvals_required, type: Integer, desc: 'The amount of approvals required. Must be higher than the project approvals'
@@ -62,7 +74,7 @@ module API
           merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, approvals_before_merge: params[:approvals_required]).execute(merge_request)
 
           if merge_request.valid?
-            present merge_request.present(current_user: current_user), with: Entities::MergeRequestApprovals, current_user: current_user
+            present_approval(merge_request)
           else
             handle_merge_request_errors! merge_request.errors
           end
@@ -70,7 +82,7 @@ module API
 
         desc 'Update approvers and approver groups' do
           detail 'This feature was introduced in 10.6'
-          success Entities::MergeRequestApprovals
+          success ::EE::API::Entities::ApprovalState
         end
         params do
           requires :approver_ids, type: Array[String], coerce_with: ARRAY_COERCION_LAMBDA, desc: 'Array of User IDs to set as approvers.'
@@ -84,7 +96,7 @@ module API
           merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, declared(params, include_parent_namespaces: false).merge(remove_old_approvers: true)).execute(merge_request)
 
           if merge_request.valid?
-            present merge_request.present(current_user: current_user), with: Entities::MergeRequestApprovals, current_user: current_user
+            present_approval(merge_request)
           else
             handle_merge_request_errors! merge_request.errors
           end
@@ -99,7 +111,7 @@ module API
         #   POST /projects/:id/merge_requests/:merge_request_iid/approve
         #
         desc 'Approve a merge request' do
-          success Entities::MergeRequestApprovals
+          success ::EE::API::Entities::ApprovalState
         end
         params do
           optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
@@ -115,11 +127,11 @@ module API
             .new(user_project, current_user)
             .execute(merge_request)
 
-          present merge_request.present(current_user: current_user), with: Entities::MergeRequestApprovals, current_user: current_user
+          present_approval(merge_request)
         end
 
         desc 'Remove an approval from a merge request' do
-          success Entities::MergeRequestApprovals
+          success ::EE::API::Entities::ApprovalState
         end
         post 'unapprove' do
           merge_request = find_project_merge_request(params[:merge_request_iid])
@@ -130,7 +142,7 @@ module API
             .new(user_project, current_user)
             .execute(merge_request)
 
-          present merge_request.present(current_user: current_user), with: Entities::MergeRequestApprovals, current_user: current_user
+          present_approval(merge_request)
         end
       end
     end

ee/lib/api/project_approval_rules.rb

+# frozen_string_literal: true
+
+module API
+  class ProjectApprovalRules < ::Grape::API
+    before { authenticate! }
+
+    ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) }
+
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
+    resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+      segment ':id/approval_rules' do
+        desc 'Get all project approval rules' do
+          detail 'This feature was introduced in 11.6'
+          success EE::API::Entities::ProjectApprovalRules
+        end
+        get do
+          authorize! :create_merge_request_in, user_project
+
+          present user_project, with: EE::API::Entities::ProjectApprovalRules, current_user: current_user
+        end
+
+        desc 'Create new approval rule' do
+          detail 'This feature was introduced in 11.6'
+          success EE::API::Entities::ApprovalRule
+        end
+        params do
+          requires :name, type: String, desc: 'The name of the approval rule'
+          requires :approvals_required, type: Integer, desc: 'The number of required approvals for this rule'
+          optional :users, as: :user_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The user ids for this rule'
+          optional :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule'
+        end
+        post do
+          authorize! :admin_project, user_project
+
+          result = ::ApprovalRules::CreateService.new(user_project, current_user, declared_params(include_missing: false)).execute
+
+          if result[:status] == :success
+            present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user
+          else
+            render_api_error!(result[:message], 400)
+          end
+        end
+
+        segment ':approval_rule_id' do
+          desc 'Update approval rule' do
+            detail 'This feature was introduced in 11.6'
+            success EE::API::Entities::ApprovalRule
+          end
+          params do
+            requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule'
+            optional :name, type: String, desc: 'The name of the approval rule'
+            optional :approvals_required, type: Integer, desc: 'The number of required approvals for this rule'
+            optional :users, as: :user_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The user ids for this rule'
+            optional :groups, as: :group_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The group ids for this rule'
+          end
+          put do
+            authorize! :admin_project, user_project
+
+            params = declared_params(include_missing: false)
+            puts params.inspect
+            approval_rule = user_project.approval_rules.find(params.delete(:approval_rule_id))
+            result = ::ApprovalRules::UpdateService.new(approval_rule, current_user, params).execute
+
+            if result[:status] == :success
+              present result[:rule], with: EE::API::Entities::ApprovalRule, current_user: current_user
+            else
+              render_api_error!(result[:message], 400)
+            end
+          end
+
+          desc 'Delete an approval rule' do
+            detail 'This feature was introduced in 11.6'
+          end
+          params do
+            requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule'
+          end
+          delete do
+            authorize! :admin_project, user_project
+
+            approval_rule = user_project.approval_rules.find(params[:approval_rule_id])
+            destroy_conditionally!(approval_rule)
+
+            no_content!
+          end
+        end
+      end
+    end
+  end
+end

ee/lib/api/project_approvals.rb

@@ -14,15 +14,15 @@ module API
       segment ':id/approvals' do
         desc 'Get all project approvers and related configuration' do
           detail 'This feature was introduced in 10.6'
-          success ::API::Entities::ApprovalSettings
+          success EE::API::Entities::ApprovalSettings
         end
         get '/' do
-          present user_project.present(current_user: current_user), with: ::API::Entities::ApprovalSettings
+          present user_project.present(current_user: current_user), with: EE::API::Entities::ApprovalSettings
         end
 
         desc 'Change approval-related configuration' do
           detail 'This feature was introduced in 10.6'
-          success ::API::Entities::ApprovalSettings
+          success EE::API::Entities::ApprovalSettings
         end
         params do
           optional :approvals_before_merge, type: Integer, desc: 'The amount of approvals required before an MR can be merged'
@@ -36,7 +36,7 @@ module API
           result = ::Projects::UpdateService.new(user_project, current_user, project_params).execute
 
           if result[:status] == :success
-            present user_project.present(current_user: current_user), with: ::API::Entities::ApprovalSettings
+            present user_project.present(current_user: current_user), with: EE::API::Entities::ApprovalSettings
           else
             render_validation_error!(user_project)
           end
@@ -45,7 +45,7 @@ module API
 
       desc 'Update approvers and approver groups' do
         detail 'This feature was introduced in 10.6'
-        success ::API::Entities::ApprovalSettings
+        success EE::API::Entities::ApprovalSettings
       end
       params do
         requires :approver_ids, type: Array[String], coerce_with: ARRAY_COERCION_LAMBDA, desc: 'Array of User IDs to set as approvers.'
@@ -55,7 +55,7 @@ module API
         result = ::Projects::UpdateService.new(user_project, current_user, declared(params, include_parent_namespaces: false).merge(remove_old_approvers: true)).execute
 
         if result[:status] == :success
-          present user_project.present(current_user: current_user), with: ::API::Entities::ApprovalSettings
+          present user_project.present(current_user: current_user), with: EE::API::Entities::ApprovalSettings
         else
           render_validation_error!(user_project)
         end

ee/lib/ee/api/endpoints.rb

@@ -9,6 +9,7 @@ module EE
         mount ::EE::API::Boards
         mount ::EE::API::GroupBoards
 
+        mount ::API::ProjectApprovalRules
         mount ::API::Unleash
         mount ::API::EpicIssues
         mount ::API::EpicLinks

ee/lib/ee/api/entities.rb

@@ -243,10 +243,57 @@ module EE
         expose :title
       end
 
+      class ApprovalRule < Grape::Entity
+        def initialize(object, options = {})
+          presenter = ::ApprovalRulePresenter.new(object, current_user: options[:current_user])
+          super(presenter, options)
+        end
+
+        expose :id, :name
+        expose :approvers, using: ::API::Entities::UserBasic
+        expose :approvals_required
+        expose :users, using: ::API::Entities::UserBasic
+        expose :groups, using: ::API::Entities::Group
+      end
+
+      class MergeRequestApprovalRule < ApprovalRule
+        expose :approved_approvers, as: :approved_by, using: ::API::Entities::UserBasic
+        expose :code_owner
+      end
+
+      # Decorates ApprovalState
+      class MergeRequestApprovalRules < Grape::Entity
+        expose :wrapped_approval_rules, as: :rules, using: MergeRequestApprovalRule
+      end
+
+      # Decorates Project
+      class ProjectApprovalRules < Grape::Entity
+        expose :approval_rules, as: :rules, using: ApprovalRule
+      end
+
+      # @deprecated
+      class Approver < Grape::Entity
+        expose :user, using: ::API::Entities::UserBasic
+      end
+
+      # @deprecated
+      class ApproverGroup < Grape::Entity
+        expose :group, using: ::API::Entities::Group
+      end
+
+      class ApprovalSettings < Grape::Entity
+        expose :approvers, using: EE::API::Entities::Approver
+        expose :approver_groups, using: EE::API::Entities::ApproverGroup
+        expose :approvals_before_merge
+        expose :reset_approvals_on_push
+        expose :disable_overriding_approvers_per_merge_request
+      end
+
       class Approvals < Grape::Entity
         expose :user, using: ::API::Entities::UserBasic
       end
 
+      # @deprecated, replaced with ApprovalState
       class MergeRequestApprovals < ::API::Entities::ProjectEntity
         def initialize(merge_request, options = {})
           presenter = merge_request.present(current_user: options[:current_user])
@@ -259,6 +306,10 @@ module EE
         expose :approvals_left
         expose :approvals, as: :approved_by, using: EE::API::Entities::Approvals
         expose :approvers_left, as: :suggested_approvers, using: ::API::Entities::UserBasic
+        # @deprecated
+        expose :approvers, using: EE::API::Entities::Approver
+        # @deprecated
+        expose :approver_groups, using: EE::API::Entities::ApproverGroup
 
         expose :user_has_approved do |merge_request, options|
           merge_request.has_approved?(options[:current_user])
@@ -269,6 +320,63 @@ module EE
         end
       end
 
+      class ApprovalState < Grape::Entity
+        expose :merge_request, merge: true, using: ::API::Entities::ProjectEntity
+        expose(:merge_status) { |approval_state| approval_state.merge_request.merge_status }
+
+        expose :approved?, as: :approved
+
+        # @deprecated, reads from first regular rule instead
+        expose :approvals_required do |approval_state|
+          approval_state.first_regular_rule&.approvals_required
+        end
+
+        expose :approvals_left
+
+        expose :approved_by, using: EE::API::Entities::Approvals do |approval_state|
+          approval_state.merge_request.approvals
+        end
+
+        expose :suggested_approvers, using: ::API::Entities::UserBasic do |approval_state, options|
+          # TODO order by relevance
+          approval_state.unactioned_approvers
+        end
+
+        # @deprecated, reads from first regular rule instead
+        expose :approvers do |approval_state|
+          if rule = approval_state.first_regular_rule
+            rule.users.map do |user|
+              { user: ::API::Entities::UserBasic.represent(user) }
+            end
+          else
+            []
+          end
+        end
+        # @deprecated, reads from first regular rule instead
+        expose :approver_groups do |approval_state|
+          if rule = approval_state.first_regular_rule
+            presenter = ::ApprovalRulePresenter.new(rule, current_user: options[:current_user])
+            presenter.groups.map do |group|
+              { group: ::API::Entities::Group.represent(group) }
+            end
+          else
+            []
+          end
+        end
+
+        expose :user_has_approved do |approval_state, options|
+          approval_state.has_approved?(options[:current_user])
+        end
+
+        expose :user_can_approve do |approval_state, options|
+          approval_state.can_approve?(options[:current_user])
+        end
+
+        expose :approval_rules_left do |approval_state, options|
+          approval_state.approval_rules_left.map(&:name)
+        end
+      end
+
       class LdapGroup < Grape::Entity
         expose :cn
       end

ee/lib/ee/api/merge_requests.rb

@@ -9,6 +9,10 @@ module EE
         helpers do
           params :optional_params_ee do
             optional :approvals_before_merge, type: Integer, desc: 'Number of approvals required before this can be merged'
+            optional :approval_rules_attributes, type: Array, documentation: { hidden: true } do
+              optional :id, type: Integer, desc: 'The ID of a rule'
+              optional :approvals_required, type: Integer, desc: 'Total number of approvals required'
+            end
           end
         end
 

ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rule.json

+{
+  "type": "object",
+  "properties": {
+    "id": { "type": "integer" },
+    "name": { "type": "string" },
+    "approvals_required": { "type": "integer" },
+    "approvers": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {}
+      }
+    },
+    "groups": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {}
+      }
+    },
+    "users": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {}
+      }
+    }
+  },
+  "additionalProperties": false
+}

ee/spec/fixtures/api/schemas/public_api/v4/project_approval_rules.json

+{
+  "type": "object",
+  "properties": {
+    "rules": {
+      "type": "array",
+      "items": { "$ref": "project_approval_rule.json" }
+    }
+  },
+  "additionalProperties": false
+}

ee/spec/presenters/merge_request_presenter_spec.rb

@@ -42,6 +42,27 @@ describe MergeRequestPresenter do
     end
   end
 
+  describe '#approvers_left with approval_rule enabled' do
+    let!(:private_group) { create(:group_with_members, :private) }
+    let!(:public_group) { create(:group_with_members) }
+    let!(:public_approver_group) { create(:approver_group, target: resource, group: public_group) }
+    let!(:private_approver_group) { create(:approver_group, target: resource, group: private_group) }
+    let!(:approver) { create(:approver, target: resource) }
+
+    before do
+      stub_feature_flags approval_rule: true
+      resource.approvals.create!(user: approver.user)
+    end
+
+    subject { described_class.new(resource, current_user: user).approvers_left }
+
+    it 'contains all approvers' do
+      approvers = public_approver_group.users + private_approver_group.users - [user]
+
+      is_expected.to match_array(approvers)
+    end
+  end
+
   describe '#overall_approver_groups' do
     let!(:private_group) { create(:group_with_members, :private) }
     let!(:public_group) { create(:group_with_members) }
@@ -88,4 +109,24 @@ describe MergeRequestPresenter do
       end
     end
   end
+
+  describe '#all_approvers_including_groups with approval_rule enabled' do
+    let!(:private_group) { create(:group_with_members, :private) }
+    let!(:public_group) { create(:group_with_members) }
+    let!(:public_approver_group) { create(:approver_group, target: resource, group: public_group) }
+    let!(:private_approver_group) { create(:approver_group, target: resource, group: private_group) }
+    let!(:approver) { create(:approver, target: resource) }
+
+    before do
+      stub_feature_flags approval_rule: true
+    end
+
+    subject { described_class.new(resource, current_user: user).all_approvers_including_groups }
+
+    it do
+      approvers = [public_approver_group.users, private_approver_group.users, approver.user].flatten - [user]
+
+      is_expected.to match_array(approvers)
+    end
+  end
 end

ee/spec/requests/api/merge_requests_spec.rb

@@ -18,6 +18,32 @@ describe API::MergeRequests do
     project.add_reporter(user)
   end
 
+  describe 'PUT /projects/:id/merge_requests' do
+    context 'when updating existing approval rules' do
+      def update_merge_request(params)
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params
+      end
+
+      let!(:rule) { create(:approval_merge_request_rule, merge_request: merge_request, approvals_required: 1) }
+
+      it 'is successful' do
+        update_merge_request(
+          title: "New title",
+          approval_rules_attributes: [
+            { id: rule.id, approvals_required: 2 }
+          ]
+        )
+
+        expect(response).to have_gitlab_http_status(200)
+
+        merge_request.reload
+
+        expect(merge_request.approval_rules.size).to eq(1)
+        expect(merge_request.approval_rules.first.approvals_required).to eq(2)
+      end
+    end
+  end
+
   describe "POST /projects/:id/merge_requests" do
     def create_merge_request(args)
       defaults = {

ee/spec/requests/api/project_approval_rules_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::ProjectApprovalRules do
+  set(:group)    { create(:group_with_members) }
+  set(:user)     { create(:user) }
+  set(:user2)    { create(:user) }
+  set(:admin)    { create(:user, :admin) }
+  set(:project)  { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
+  set(:approver) { create(:user) }
+
+  let(:url) { "/projects/#{project.id}/approval_rules" }
+
+  describe 'GET /projects/:id/approval_rules' do
+    context 'when the request is correct' do
+      let!(:rule) do
+        rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7)
+        rule.users << approver
+        rule
+      end
+
+      let(:developer) do
+        user = create(:user)
+        project.add_guest(user)
+        user
+      end
+
+      it 'matches the response schema' do
+        get api(url, developer)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/project_approval_rules', dir: 'ee')
+
+        json = json_response
+
+        expect(json['rules'].size).to eq(1)
+
+        rule = json['rules'].first
+
+        expect(rule['approvals_required']).to eq(7)
+        expect(rule['name']).to eq('security')
+      end
+
+      context 'private group filtering' do
+        set(:private_group) { create :group, :private }
+
+        before do
+          rule.groups << private_group
+        end
+
+        it 'excludes private groups if user has no access' do
+          get api(url, developer)
+
+          json = json_response
+          rule = json['rules'].first
+
+          expect(rule['groups'].size).to eq(0)
+        end
+
+        it 'includes private groups if user has access' do
+          private_group.add_owner(developer)
+
+          get api(url, developer)
+
+          json = json_response
+          rule = json['rules'].first
+
+          expect(rule['groups'].size).to eq(1)
+        end
+      end
+    end
+  end
+
+  describe 'POST /projects/:id/approval_rules' do
+    let(:current_user) { user }
+    let(:params) do
+      {
+        name: 'security',
+        approvals_required: 10
+      }
+    end
+
+    context 'when missing parameters' do
+      it 'returns 400 status' do
+        post api(url, current_user)
+
+        expect(response).to have_gitlab_http_status(400)
+      end
+    end
+
+    context 'when user is without access' do
+      it 'returns 403' do
+        post api(url, user2), params
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    context 'when the request is correct' do
+      it 'returns 201 status' do
+        post api(url, current_user), params
+
+        expect(response).to have_gitlab_http_status(201)
+        expect(response).to match_response_schema('public_api/v4/project_approval_rule', dir: 'ee')
+      end
+
+      it 'changes settings properly' do
+        create(:approval_project_rule, project: project, approvals_required: 2)
+
+        project.reset_approvals_on_push = false
+        project.disable_overriding_approvers_per_merge_request = true
+        project.save
+
+        post api(url, current_user), params
+
+        expect(json_response.symbolize_keys).to include(params)
+      end
+    end
+  end
+
+  describe 'PUT /projects/:id/approval_rules/:approval_rule_id' do
+    let!(:approval_rule) { create(:approval_project_rule, project: project) }
+    let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule.id}" }
+
+    shared_examples_for 'a user with access' do
+      before do
+        project.add_developer(approver)
+      end
+
+      context 'when approver already exists' do
+        before do
+          approval_rule.users << approver
+          approval_rule.groups << group
+        end
+
+        context 'when sending json data' do
+          it 'removes all approvers if empty params are given' do
+            expect do
+              put api(url, current_user), params: { users: [], groups: [] }.to_json, headers: { CONTENT_TYPE: 'application/json' }
+            end.to change { approval_rule.users.count + approval_rule.groups.count }.from(2).to(0)
+
+            expect(response).to have_gitlab_http_status(200)
+          end
+        end
+      end
+
+      it 'sets approvers' do
+        expect do
+          put api(url, current_user), params: { users: [approver.id] }
+        end.to change { approval_rule.users.count }.from(0).to(1)
+
+        expect(approval_rule.users).to contain_exactly(approver)
+        expect(approval_rule.groups).to be_empty
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['approvers'][0]['id']).to eq(approver.id)
+      end
+    end
+
+    context 'as a project admin' do
+      it_behaves_like 'a user with access' do
+        let(:current_user) { user }
+        let(:visible_approver_groups_count) { 0 }
+      end
+    end
+
+    context 'as a global admin' do
+      it_behaves_like 'a user with access' do
+        let(:current_user) { admin }
+        let(:visible_approver_groups_count) { 1 }
+      end
+    end
+
+    context 'as a random user' do
+      it 'returns 403' do
+        project.approvers.create(user: approver)
+
+        expect do
+          put api(url, user2), { users: [], groups: [] }.to_json, { CONTENT_TYPE: 'application/json' }
+        end.not_to change { approval_rule.approvers.size }
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+  end
+
+  describe 'DELETE /projects/:id/approval_rules/:approval_rule_id' do
+    let!(:approval_rule) { create(:approval_project_rule, project: project) }
+    let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule.id}" }
+
+    it 'destroys' do
+      delete api(url, user)
+
+      expect(ApprovalProjectRule.exists?(id: approval_rule.id)).to eq(false)
+      expect(response).to have_gitlab_http_status(204)
+    end
+
+    context 'when approval rule not found' do
+      let!(:approval_rule_2) { create(:approval_project_rule) }
+      let(:url) { "/projects/#{project.id}/approval_rules/#{approval_rule_2.id}" }
+
+      it 'returns not found' do
+        delete api(url, user)
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'when user is not eligible to delete' do
+      it 'returns forbidden' do
+        delete api(url, user2)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+  end
+end

ee/spec/services/ee/merge_requests/refresh_service_spec.rb

@@ -137,6 +137,36 @@ describe MergeRequests::RefreshService do
           end
         end
       end
+
+      context 'when code owners enabled, with approval_rule enabled' do
+        let(:relevant_merge_requests) { [merge_request, another_merge_request] }
+        let(:new_owners) { [owner] }
+
+        before do
+          stub_feature_flags(approval_rule: true)
+
+          relevant_merge_requests.each do |merge_request|
+            expect(::Gitlab::CodeOwners).to receive(:for_merge_request).with(merge_request).and_return(new_owners)
+          end
+
+          [forked_merge_request].each do |merge_request|
+            expect(::Gitlab::CodeOwners).not_to receive(:for_merge_request)
+          end
+        end
+
+        it 'triggers syncing of code owners' do
+          relevant_merge_requests.each do |merge_request|
+            expect(merge_request.approval_rules.code_owner.exists?).to eq(false)
+          end
+
+          subject
+
+          relevant_merge_requests.each do |merge_request|
+            code_owner_rule = merge_request.approval_rules.code_owner.first
+            expect(code_owner_rule.users).to eq(new_owners)
+          end
+        end
+      end
     end
   end
 end

ee/spec/services/projects/update_service_spec.rb

@@ -251,6 +251,18 @@ describe Projects::UpdateService, '#execute' do
         expect(rule.reload.approvals_required).to eq(0)
       end
     end
+
+    context 'when approval_rule feature is enabled' do
+      it "does not update approval_rules' approvals_required" do
+        stub_feature_flags(approval_rule: true)
+
+        rule = create(:approval_project_rule, project: project)
+
+        expect do
+          update_project(project, user, approvals_before_merge: 42)
+        end.not_to change { rule.reload.approvals_required }
+      end
+    end
   end
 
   def update_project(project, user, opts)

lib/api/entities.rb

@@ -741,32 +741,6 @@ module API
       end
     end
 
-    class Approver < Grape::Entity
-      expose :user, using: Entities::UserBasic
-    end
-
-    class ApproverGroup < Grape::Entity
-      expose :group, using: Entities::Group
-    end
-
-    class MergeRequestApprovals < ProjectEntity
-      expose :merge_status
-      expose :approvals_required
-      expose :approvals_left
-      expose :approvals, as: :approved_by, using: Entities::Approver
-      expose :approvers_left, as: :suggested_approvers, using: Entities::UserBasic
-      expose :approvers, using: Entities::Approver
-      expose :approver_groups, using: Entities::ApproverGroup
-
-      expose :user_has_approved do |merge_request, options|
-        merge_request.has_approved?(options[:current_user])
-      end
-
-      expose :user_can_approve do |merge_request, options|
-        merge_request.can_approve?(options[:current_user])
-      end
-    end
-
     class MergeRequestDiff < Grape::Entity
       expose :id, :head_commit_sha, :base_commit_sha, :start_commit_sha,
         :created_at, :merge_request_id, :state, :real_size
@@ -1560,14 +1534,6 @@ module API
       klass.prepend(with)
     end
 
-    class ApprovalSettings < Grape::Entity
-      expose :approvers, using: Entities::Approver
-      expose :approver_groups, using: Entities::ApproverGroup
-      expose :approvals_before_merge
-      expose :reset_approvals_on_push
-      expose :disable_overriding_approvers_per_merge_request
-    end
-
     class ManagedLicense < Grape::Entity
       expose :id, :name, :approval_status
     end