Merge branch '460-approval-project-rules-protected-branches-api' into 'master'

Scope approval rules by protected branches via API

See merge request gitlab-org/gitlab!22673

app/controllers/projects_controller.rb

@@ -51,7 +51,7 @@ class ProjectsController < Projects::ApplicationController
 
   def edit
     @badge_api_endpoint = expose_url(api_v4_projects_badges_path(id: @project.id))
-    render 'edit'
+    render_edit
   end
 
   def create
@@ -85,7 +85,7 @@ class ProjectsController < Projects::ApplicationController
       else
         flash.now[:alert] = result[:message]
 
-        format.html { render 'edit' }
+        format.html { render_edit }
       end
 
       format.js
@@ -387,7 +387,6 @@ class ProjectsController < Projects::ApplicationController
       :merge_method,
       :initialize_with_readme,
       :autoclose_referenced_issues,
-      :suggestion_commit_message,
 
       project_feature_attributes: %i[
         builds_access_level
@@ -488,6 +487,10 @@ class ProjectsController < Projects::ApplicationController
   def rate_limiter
     ::Gitlab::ApplicationRateLimiter
   end
+
+  def render_edit
+    render 'edit'
+  end
 end
 
 ProjectsController.prepend_if_ee('EE::ProjectsController')

app/models/concerns/protected_ref.rb

@@ -10,6 +10,8 @@ module ProtectedRef
     validates :project, presence: true
 
     delegate :matching, :matches?, :wildcard?, to: :ref_matcher
+
+    scope :for_project, ->(project) { where(project: project) }
   end
 
   def commit

db/migrate/20200109085206_create_approval_project_rules_protected_branches.rb

+# frozen_string_literal: true
+
+class CreateApprovalProjectRulesProtectedBranches < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :approval_project_rules_protected_branches, id: false do |t|
+      t.references :approval_project_rule,
+                   null: false,
+                   index: false,
+                   foreign_key: { on_delete: :cascade }
+      t.references :protected_branch,
+                   null: false,
+                   index: { name: 'index_approval_project_rules_protected_branches_pb_id' },
+                   foreign_key: { on_delete: :cascade }
+      t.index [:approval_project_rule_id, :protected_branch_id], name: 'index_approval_project_rules_protected_branches_unique', unique: true, using: :btree
+    end
+  end
+end

db/schema.rb

@@ -437,6 +437,13 @@ ActiveRecord::Schema.define(version: 2020_01_13_133352) do
     t.index ["group_id"], name: "index_approval_project_rules_groups_2"
   end
 
+  create_table "approval_project_rules_protected_branches", id: false, force: :cascade do |t|
+    t.bigint "approval_project_rule_id", null: false
+    t.bigint "protected_branch_id", null: false
+    t.index ["approval_project_rule_id", "protected_branch_id"], name: "index_approval_project_rules_protected_branches_unique", unique: true
+    t.index ["protected_branch_id"], name: "index_approval_project_rules_protected_branches_pb_id"
+  end
+
   create_table "approval_project_rules_users", force: :cascade do |t|
     t.bigint "approval_project_rule_id", null: false
     t.integer "user_id", null: false
@@ -4448,6 +4455,8 @@ ActiveRecord::Schema.define(version: 2020_01_13_133352) do
   add_foreign_key "approval_project_rules", "projects", on_delete: :cascade
   add_foreign_key "approval_project_rules_groups", "approval_project_rules", on_delete: :cascade
   add_foreign_key "approval_project_rules_groups", "namespaces", column: "group_id", on_delete: :cascade
+  add_foreign_key "approval_project_rules_protected_branches", "approval_project_rules", on_delete: :cascade
+  add_foreign_key "approval_project_rules_protected_branches", "protected_branches", on_delete: :cascade
   add_foreign_key "approval_project_rules_users", "approval_project_rules", on_delete: :cascade
   add_foreign_key "approval_project_rules_users", "users", on_delete: :cascade
   add_foreign_key "approvals", "merge_requests", name: "fk_310d714958", on_delete: :cascade

ee/app/controllers/ee/projects_controller.rb

@@ -23,7 +23,7 @@ module EE
       else
         flash.now[:alert] = result[:message]
 
-        render 'edit'
+        render_edit
       end
     end
 
@@ -41,7 +41,7 @@ module EE
       else
         flash.now[:alert] = result[:message]
 
-        render 'edit'
+        render_edit
       end
     end
 
@@ -137,5 +137,11 @@ module EE
     def log_unarchive_audit_event
       log_audit_event(message: 'Project unarchived')
     end
+
+    override :render_edit
+    def render_edit
+      push_frontend_feature_flag(:scoped_approval_rules, project, default_enabled: true)
+      super
+    end
   end
 end

ee/app/models/approval_merge_request_rule.rb

@@ -70,6 +70,14 @@ class ApprovalMergeRequestRule < ApplicationRecord
     retry
   end
 
+  def self.applicable_to_branch(branch)
+    includes(approval_project_rule: :protected_branches).select do |rule|
+      next true unless rule.approval_project_rule.present?
+
+      rule.approval_project_rule.applies_to_branch?(branch)
+    end
+  end
+
   def project
     merge_request.target_project
   end

ee/app/models/approval_project_rule.rb

@@ -4,6 +4,7 @@ class ApprovalProjectRule < ApplicationRecord
   include ApprovalRuleLike
 
   belongs_to :project
+  has_and_belongs_to_many :protected_branches
 
   enum rule_type: {
     regular: 0,
@@ -17,6 +18,16 @@ class ApprovalProjectRule < ApplicationRecord
 
   validates :name, uniqueness: { scope: :project_id }
 
+  def self.applicable_to_branch(branch)
+    includes(:protected_branches).select { |rule| rule.applies_to_branch?(branch) }
+  end
+
+  def applies_to_branch?(branch)
+    return true if protected_branches.empty?
+
+    protected_branches.matching(branch).any?
+  end
+
   def source_rule
     nil
   end

ee/app/models/approval_state.rb

@@ -157,7 +157,9 @@ class ApprovalState
       if approval_rules_overwritten?
         user_defined_merge_request_rules
       else
-        project.visible_user_defined_rules.map do |rule|
+        branch = project.scoped_approval_rules_enabled? ? target_branch : nil
+
+        project.visible_user_defined_rules(branch: branch).map do |rule|
           ApprovalWrappedRule.wrap(merge_request, rule)
         end
       end
@@ -201,9 +203,18 @@ class ApprovalState
 
   def wrapped_rules
     strong_memoize(:wrapped_rules) do
-      merge_request.approval_rules.map do |rule|
+      merge_request_rules = merge_request.approval_rules
+      merge_request_rules = merge_request_rules.applicable_to_branch(target_branch) if project.scoped_approval_rules_enabled?
+
+      merge_request_rules.map do |rule|
         ApprovalWrappedRule.wrap(merge_request, rule)
       end
     end
   end
+
+  def target_branch
+    strong_memoize(:target_branch) do
+      merge_request.target_branch
+    end
+  end
 end

ee/app/models/ee/project.rb

@@ -323,6 +323,10 @@ module EE
       feature_available?(:code_owner_approval_required)
     end
 
+    def scoped_approval_rules_enabled?
+      ::Feature.enabled?(:scoped_approval_rules, self, default_enabled: true)
+    end
+
     def service_desk_enabled
       ::EE::Gitlab::ServiceDesk.enabled?(project: self) && super
     end
@@ -408,14 +412,11 @@ module EE
       end
     end
 
-    def visible_user_defined_rules
-      strong_memoize(:visible_user_defined_rules) do
-        rules = approval_rules.regular_or_any_approver.order(rule_type: :desc, id: :asc)
+    def visible_user_defined_rules(branch: nil)
+      return user_defined_rules.take(1) unless multiple_approval_rules_available?
+      return user_defined_rules unless branch
 
-        next rules.take(1) unless multiple_approval_rules_available?
-
-        rules
-      end
+      user_defined_rules.applicable_to_branch(branch)
     end
 
     # TODO: Clean up this method in the https://gitlab.com/gitlab-org/gitlab/issues/33329
@@ -771,5 +772,11 @@ module EE
         errors.add(:pull_mirror_branch_prefix, _('Invalid Git ref'))
       end
     end
+
+    def user_defined_rules
+      strong_memoize(:user_defined_rules) do
+        approval_rules.regular_or_any_approver.order(rule_type: :desc, id: :asc)
+      end
+    end
   end
 end

ee/app/services/concerns/approval_rules/updater.rb

@@ -5,6 +5,7 @@ module ApprovalRules
     def action
       filter_eligible_users!
       filter_eligible_groups!
+      filter_eligible_protected_branches!
 
       if rule.update(params)
         rule.reset
@@ -27,5 +28,18 @@ module ApprovalRules
 
       params[:groups] = Group.id_in(params.delete(:group_ids)).public_or_visible_to_user(current_user)
     end
+
+    def filter_eligible_protected_branches!
+      return unless params.key?(:protected_branch_ids)
+
+      protected_branch_ids = params.delete(:protected_branch_ids)
+
+      return unless project.multiple_approval_rules_available? && can?(current_user, :admin_project, project)
+
+      params[:protected_branches] =
+        ProtectedBranch
+          .id_in(protected_branch_ids)
+          .for_project(project)
+    end
   end
 end

ee/changelogs/unreleased/460-approval-project-rules-protected-branches-api.yml

+---
+title: Scope approval rules by protected branches via API
+merge_request: 22673
+author:
+type: added

ee/lib/api/helpers/project_approval_rules_helpers.rb

@@ -13,6 +13,7 @@ module API
         optional :rule_type, type: String, desc: 'The type of approval 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'
+        optional :protected_branch_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The protected branch ids for this rule'
       end
 
       params :update_project_approval_rule do
@@ -21,6 +22,7 @@ module API
         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'
+        optional :protected_branch_ids, type: Array, coerce_with: ARRAY_COERCION_LAMBDA, desc: 'The protected branch ids for this rule'
         optional :remove_hidden_groups, type: Boolean, desc: 'Whether hidden groups should be removed'
       end
 

ee/lib/api/project_approval_rules.rb

@@ -12,33 +12,33 @@ module API
     resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       segment ':id/approval_rules' do
         desc 'Get all project approval rules' do
-          success EE::API::Entities::ApprovalRule
+          success EE::API::Entities::ProjectApprovalRule
         end
         get do
           authorize_create_merge_request_in_project
 
-          present user_project.visible_approval_rules, with: EE::API::Entities::ApprovalRule, current_user: current_user
+          present user_project.visible_approval_rules, with: EE::API::Entities::ProjectApprovalRule, current_user: current_user
         end
 
         desc 'Create new project approval rule' do
-          success EE::API::Entities::ApprovalRule
+          success EE::API::Entities::ProjectApprovalRule
         end
         params do
           use :create_project_approval_rule
         end
         post do
-          create_project_approval_rule(present_with: EE::API::Entities::ApprovalRule)
+          create_project_approval_rule(present_with: EE::API::Entities::ProjectApprovalRule)
         end
 
         segment ':approval_rule_id' do
           desc 'Update project approval rule' do
-            success EE::API::Entities::ApprovalRule
+            success EE::API::Entities::ProjectApprovalRule
           end
           params do
             use :update_project_approval_rule
           end
           put do
-            update_project_approval_rule(present_with: EE::API::Entities::ApprovalRule)
+            update_project_approval_rule(present_with: EE::API::Entities::ProjectApprovalRule)
           end
 
           desc 'Destroy project approval rule'

ee/lib/api/project_approval_settings.rb

@@ -24,25 +24,25 @@ module API
         segment 'rules' do
           desc 'Create new approval rule' do
             detail 'Private API subject to change'
-            success EE::API::Entities::ApprovalSettingRule
+            success EE::API::Entities::ProjectApprovalSettingRule
           end
           params do
             use :create_project_approval_rule
           end
           post do
-            create_project_approval_rule(present_with: EE::API::Entities::ApprovalSettingRule)
+            create_project_approval_rule(present_with: EE::API::Entities::ProjectApprovalSettingRule)
           end
 
           segment ':approval_rule_id' do
             desc 'Update approval rule' do
               detail 'Private API subject to change'
-              success EE::API::Entities::ApprovalSettingRule
+              success EE::API::Entities::ProjectApprovalSettingRule
             end
             params do
               use :update_project_approval_rule
             end
             put do
-              update_project_approval_rule(present_with: EE::API::Entities::ApprovalSettingRule)
+              update_project_approval_rule(present_with: EE::API::Entities::ProjectApprovalSettingRule)
             end
 
             desc 'Delete an approval rule' do

ee/lib/ee/api/entities.rb

@@ -420,6 +420,10 @@ module EE
         expose :contains_hidden_groups?, as: :contains_hidden_groups
       end
 
+      class ProjectApprovalRule < ApprovalRule
+        expose :protected_branches, using: ::API::Entities::ProtectedBranch, if: -> (rule, _) { rule.project.multiple_approval_rules_available? }
+      end
+
       class MergeRequestApprovalRule < ApprovalRule
         class SourceRule < Grape::Entity
           expose :approvals_required
@@ -446,7 +450,7 @@ module EE
       # This overrides the `eligible_approvers` to be exposed as `approvers`.
       #
       # To be removed in https://gitlab.com/gitlab-org/gitlab/issues/13574.
-      class ApprovalSettingRule < ApprovalRule
+      class ProjectApprovalSettingRule < ProjectApprovalRule
         expose :approvers, using: ::API::Entities::UserBasic, override: true
       end
 
@@ -454,7 +458,7 @@ module EE
       #
       # To be removed in https://gitlab.com/gitlab-org/gitlab/issues/13574.
       class ProjectApprovalSettings < Grape::Entity
-        expose :visible_approval_rules, as: :rules, using: ApprovalSettingRule
+        expose :visible_approval_rules, as: :rules, using: ProjectApprovalSettingRule
         expose :min_fallback_approvals, as: :fallback_approvals_required
       end
 

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

@@ -26,6 +26,13 @@
         "type": "object",
         "properties": {}
       }
+    },
+    "protected_branches": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {}
+      }
     }
   },
   "additionalProperties": false

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

@@ -26,6 +26,13 @@
         "type": "object",
         "properties": {}
       }
+    },
+    "protected_branches": {
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {}
+      }
     }
   },
   "additionalProperties": false

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -117,11 +117,11 @@ describe ApprovalMergeRequestRule do
   end
 
   context 'scopes' do
-    set(:rb_rule) { create(:code_owner_rule, name: '*.rb') }
-    set(:js_rule) { create(:code_owner_rule, name: '*.js') }
-    set(:css_rule) { create(:code_owner_rule, name: '*.css') }
-    set(:approval_rule) { create(:approval_merge_request_rule) }
-    set(:report_approver_rule) { create(:report_approver_rule) }
+    let!(:rb_rule) { create(:code_owner_rule, name: '*.rb') }
+    let!(:js_rule) { create(:code_owner_rule, name: '*.js') }
+    let!(:css_rule) { create(:code_owner_rule, name: '*.css') }
+    let!(:approval_rule) { create(:approval_merge_request_rule) }
+    let!(:report_approver_rule) { create(:report_approver_rule) }
 
     describe '.not_matching_pattern' do
       it 'returns the correct rules' do
@@ -153,8 +153,7 @@ describe ApprovalMergeRequestRule do
   end
 
   describe '.find_or_create_code_owner_rule' do
-    set(:merge_request) { create(:merge_request) }
-    set(:existing_code_owner_rule) { create(:code_owner_rule, name: '*.rb', merge_request: merge_request) }
+    let!(:existing_code_owner_rule) { create(:code_owner_rule, name: '*.rb', merge_request: merge_request) }
 
     it 'finds an existing rule' do
       expect(described_class.find_or_create_code_owner_rule(merge_request, '*.rb'))
@@ -182,6 +181,47 @@ describe ApprovalMergeRequestRule do
     end
   end
 
+  describe '.applicable_to_branch' do
+    let!(:rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
+    let(:branch) { 'stable' }
+
+    subject { described_class.applicable_to_branch(branch) }
+
+    context 'when there are no associated source rules' do
+      it { is_expected.to eq([rule]) }
+    end
+
+    context 'when there are associated source rules' do
+      let(:source_rule) { create(:approval_project_rule, project: merge_request.target_project) }
+
+      before do
+        rule.update!(approval_project_rule: source_rule)
+      end
+
+      context 'and there are no associated protected branches to source rule' do
+        it { is_expected.to eq([rule]) }
+      end
+
+      context 'and there are associated protected branches to source rule' do
+        before do
+          source_rule.update!(protected_branches: protected_branches)
+        end
+
+        context 'and branch matches' do
+          let(:protected_branches) { [create(:protected_branch, name: branch)] }
+
+          it { is_expected.to eq([rule]) }
+        end
+
+        context 'but branch does not match anything' do
+          let(:protected_branches) { [create(:protected_branch, name: branch.reverse)] }
+
+          it { is_expected.to be_empty }
+        end
+      end
+    end
+  end
+
   describe '#project' do
     it 'returns project of MergeRequest' do
       expect(subject.project).to be_present

ee/spec/models/approval_project_rule_spec.rb

@@ -147,4 +147,33 @@ describe ApprovalProjectRule do
       expect { rule.save }.to raise_error(ActiveRecord::RecordNotUnique)
     end
   end
+
+  describe '.applicable_to_branch' do
+    let!(:rule) { create(:approval_project_rule) }
+    let(:branch) { 'stable' }
+
+    subject { described_class.applicable_to_branch(branch) }
+
+    context 'when there are no associated protected branches' do
+      it { is_expected.to eq([rule]) }
+    end
+
+    context 'when there are associated protected branches' do
+      before do
+        rule.update!(protected_branches: protected_branches)
+      end
+
+      context 'and branch matches' do
+        let(:protected_branches) { [create(:protected_branch, name: branch)] }
+
+        it { is_expected.to eq([rule]) }
+      end
+
+      context 'but branch does not match anything' do
+        let(:protected_branches) { [create(:protected_branch, name: branch.reverse)] }
+
+        it { is_expected.to be_empty }
+      end
+    end
+  end
 end

ee/spec/models/approval_state_spec.rb

@@ -1380,4 +1380,130 @@ describe ApprovalState do
       end
     end
   end
+
+  describe '#user_defined_rules' do
+    context 'when approval rules are not overwritten' do
+      let!(:project_rule) { create(:approval_project_rule, project: project) }
+      let!(:another_project_rule) { create(:approval_project_rule, project: project) }
+
+      context 'and multiple approval rules is disabled' do
+        it 'returns the first rule' do
+          expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
+            project_rule
+          ])
+        end
+      end
+
+      context 'and multiple approval rules is enabled' do
+        before do
+          stub_licensed_features(multiple_approval_rules: true)
+        end
+
+        it 'returns the rules as is' do
+          expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
+            project_rule,
+            another_project_rule
+          ])
+        end
+
+        context 'and rules are scoped by protected branches' do
+          let(:protected_branch) { create(:protected_branch, project: project, name: 'stable-*') }
+          let(:another_protected_branch) { create(:protected_branch, project: project, name: '*-stable') }
+
+          before do
+            merge_request.update!(target_branch: 'stable-1')
+            another_project_rule.update!(protected_branches: [protected_branch])
+            project_rule.update!(protected_branches: [another_protected_branch])
+          end
+
+          context 'and scoped_approval_rules feature is enabled' do
+            it 'returns the rules that are applicable to the merge request target branch' do
+              expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+                another_project_rule
+              ])
+            end
+          end
+
+          context 'but scoped_approval_rules feature is disabled' do
+            before do
+              stub_feature_flags(scoped_approval_rules: false)
+            end
+
+            it 'returns unscoped rules' do
+              expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
+                project_rule,
+                another_project_rule
+              ])
+            end
+          end
+        end
+      end
+    end
+
+    context 'when approval rules are overwritten' do
+      let!(:mr_rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
+      let!(:another_mr_rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
+
+      before do
+        project.update!(disable_overriding_approvers_per_merge_request: false)
+      end
+
+      context 'when multiple approval rules is disabled' do
+        it 'returns the first rule' do
+          expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
+            mr_rule
+          ])
+        end
+      end
+
+      context 'when multiple approval rules is enabled' do
+        before do
+          stub_licensed_features(multiple_approval_rules: true)
+        end
+
+        it 'returns the rules as is' do
+          expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
+            mr_rule,
+            another_mr_rule
+          ])
+        end
+
+        context 'and rules have source rules that are scoped by protected branches' do
+          let(:source_rule) { create(:approval_project_rule, project: project) }
+          let(:another_source_rule) { create(:approval_project_rule, project: project) }
+          let(:protected_branch) { create(:protected_branch, project: project, name: 'stable-*') }
+          let(:another_protected_branch) { create(:protected_branch, project: project, name: '*-stable') }
+
+          before do
+            merge_request.update!(target_branch: 'stable-1')
+            source_rule.update!(protected_branches: [protected_branch])
+            another_source_rule.update!(protected_branches: [another_protected_branch])
+            mr_rule.update!(approval_project_rule: another_source_rule)
+            another_mr_rule.update!(approval_project_rule: source_rule)
+          end
+
+          context 'and scoped_approval_rules feature is enabled' do
+            it 'returns the rules that are applicable to the merge request target branch' do
+              expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+                another_mr_rule
+              ])
+            end
+          end
+
+          context 'but scoped_approval_rules feature is disabled' do
+            before do
+              stub_feature_flags(scoped_approval_rules: false)
+            end
+
+            it 'returns unscoped rules' do
+              expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
+                mr_rule,
+                another_mr_rule
+              ])
+            end
+          end
+        end
+      end
+    end
+  end
 end

ee/spec/services/approval_rules/create_service_spec.rb

@@ -83,6 +83,60 @@ describe ApprovalRules::CreateService do
 
     it_behaves_like "creatable"
 
+    context 'when protected_branch_ids param is present' do
+      let(:protected_branch) { create(:protected_branch, project: target) }
+
+      subject do
+        described_class.new(
+          target,
+          user,
+          name: 'developers',
+          approvals_required: 1,
+          protected_branch_ids: [protected_branch.id]
+        ).execute
+      end
+
+      context 'and multiple approval rules is enabled' do
+        before do
+          stub_licensed_features(multiple_approval_rules: true)
+        end
+
+        it 'associates the approval rule to the protected branch' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:rule].protected_branches).to eq([protected_branch])
+        end
+
+        context 'but user cannot administer project' do
+          before do
+            allow(Ability).to receive(:allowed?).and_call_original
+            allow(Ability).to receive(:allowed?).with(user, :admin_project, target).and_return(false)
+          end
+
+          it 'does not associate the approval rule to the protected branch' do
+            expect(subject[:status]).to eq(:success)
+            expect(subject[:rule].protected_branches).to be_empty
+          end
+        end
+
+        context 'but protected branch is for another project' do
+          let(:another_project) { create(:project) }
+          let(:protected_branch) { create(:protected_branch, project: another_project) }
+
+          it 'does not associate the approval rule to the protected branch' do
+            expect(subject[:status]).to eq(:success)
+            expect(subject[:rule].protected_branches).to be_empty
+          end
+        end
+      end
+
+      context 'and multiple approval rules is disabled' do
+        it 'does not associate the approval rule to the protected branch' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:rule].protected_branches).to be_empty
+        end
+      end
+    end
+
     ApprovalProjectRule::REPORT_TYPES_BY_DEFAULT_NAME.keys.each do |rule_name|
       context "when the rule name is `#{rule_name}`" do
         subject { described_class.new(target, user, { name: rule_name, approvals_required: 1 }) }

ee/spec/services/approval_rules/update_service_spec.rb

@@ -5,9 +5,9 @@ require 'spec_helper'
 describe ApprovalRules::UpdateService do
   let(:project) { create(:project) }
   let(:user) { project.creator }
+  let(:approval_rule) { target.approval_rules.create(name: 'foo') }
 
   shared_examples 'editable' do
-    let(:approval_rule) { target.approval_rules.create(name: 'foo') }
     let(:new_approvers) { create_list(:user, 2) }
     let(:new_groups) { create_list(:group, 2, :private) }
 
@@ -138,6 +138,58 @@ describe ApprovalRules::UpdateService do
     let(:target) { project }
 
     it_behaves_like "editable"
+
+    context 'when protected_branch_ids param is present' do
+      let(:protected_branch) { create(:protected_branch, project: target) }
+
+      subject do
+        described_class.new(
+          approval_rule,
+          user,
+          protected_branch_ids: [protected_branch.id]
+        ).execute
+      end
+
+      context 'and multiple approval rules is enabled' do
+        before do
+          stub_licensed_features(multiple_approval_rules: true)
+        end
+
+        it 'associates the approval rule to the protected branch' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:rule].protected_branches).to eq([protected_branch])
+        end
+
+        context 'but user cannot administer project' do
+          before do
+            allow(Ability).to receive(:allowed?).and_call_original
+            allow(Ability).to receive(:allowed?).with(user, :admin_project, target).and_return(false)
+          end
+
+          it 'does not associate the approval rule to the protected branch' do
+            expect(subject[:status]).to eq(:success)
+            expect(subject[:rule].protected_branches).to be_empty
+          end
+        end
+
+        context 'but protected branch is for another project' do
+          let(:another_project) { create(:project) }
+          let(:protected_branch) { create(:protected_branch, project: another_project) }
+
+          it 'does not associate the approval rule to the protected branch' do
+            expect(subject[:status]).to eq(:success)
+            expect(subject[:rule].protected_branches).to be_empty
+          end
+        end
+      end
+
+      context 'and multiple approval rules is disabled' do
+        it 'does not associate the approval rule to the protected branch' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:rule].protected_branches).to be_empty
+        end
+      end
+    end
   end
 
   context 'when target is merge request' do

ee/spec/support/shared_examples/project_approval_rules_api_shared_examples.rb

@@ -45,6 +45,22 @@ shared_examples_for 'an API endpoint for creating project approval rule' do
       expect(json_response.symbolize_keys).to include(params)
     end
 
+    context 'when protected_branch_ids param is present' do
+      let(:protected_branches) { Array.new(2).map { create(:protected_branch, project: project) } }
+
+      before do
+        stub_licensed_features(multiple_approval_rules: true)
+        post api(url, current_user), params: params.merge(protected_branch_ids: protected_branches.map(&:id))
+      end
+
+      it 'creates approval rule associated to specified protected branches' do
+        expect(json_response['protected_branches']).to contain_exactly(
+          a_hash_including('id' => protected_branches.first.id),
+          a_hash_including('id' => protected_branches.last.id)
+        )
+      end
+    end
+
     ApprovalProjectRule::REPORT_TYPES_BY_DEFAULT_NAME.keys.each do |rule_name|
       context "when creating a '#{rule_name}' approval rule" do
         it 'specifies a `rule_type` of `report_approver`' do
@@ -65,6 +81,22 @@ shared_examples_for 'an API endpoint for updating project approval rule' do
       project.add_developer(approver)
     end
 
+    context 'when protected_branch_ids param is present' do
+      let(:protected_branches) { Array.new(2).map { create(:protected_branch, project: project) } }
+
+      before do
+        stub_licensed_features(multiple_approval_rules: true)
+        put api(url, current_user), params: { protected_branch_ids: protected_branches.map(&:id) }
+      end
+
+      it 'associates approval rule to specified protected branches' do
+        expect(json_response['protected_branches']).to contain_exactly(
+          a_hash_including('id' => protected_branches.first.id),
+          a_hash_including('id' => protected_branches.last.id)
+        )
+      end
+    end
+
     context 'when approver already exists' do
       before do
         approval_rule.users << approver

lib/api/entities.rb

@@ -613,6 +613,7 @@ module API
     end
 
     class ProtectedBranch < Grape::Entity
+      expose :id
       expose :name
       expose :push_access_levels, using: Entities::ProtectedRefAccess
       expose :merge_access_levels, using: Entities::ProtectedRefAccess