Merge branch '199790-approval-settings-target-branch-api' into 'master'

Filter rules by target_branch in approval_settings

See merge request gitlab-org/gitlab!26439

changelogs/unreleased/199790-approval-settings-target-branch-api.yml

+---
+title: Filter rules by target_branch in approval_settings
+merge_request: 26439
+author:
+type: added

ee/app/models/approval_state.rb

@@ -13,9 +13,10 @@ class ApprovalState
   def_delegators :@merge_request, :merge_status, :approved_by_users, :approvals, :approval_feature_available?
   alias_method :approved_approvers, :approved_by_users
 
-  def initialize(merge_request)
+  def initialize(merge_request, target_branch: nil)
     @merge_request = merge_request
     @project = merge_request.target_project
+    @target_branch = target_branch || merge_request.target_branch
   end
 
   # Excludes the author if 'author-approval' is explicitly disabled on project settings.
@@ -168,6 +169,8 @@ class ApprovalState
 
   private
 
+  attr_reader :target_branch
+
   def filter_approvers(approvers, unactioned:)
     approvers = approvers.uniq
     approvers -= approved_approvers if unactioned
@@ -211,10 +214,4 @@ class ApprovalState
       end
     end
   end
-
-  def target_branch
-    strong_memoize(:target_branch) do
-      merge_request.target_branch
-    end
-  end
 end

ee/app/models/concerns/approvable.rb

@@ -31,9 +31,12 @@ module Approvable
     end
   end
 
-  def approval_state
-    @approval_state ||= ApprovalState.new(self)
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  def approval_state(target_branch: nil)
+    @approval_state ||= {}
+    @approval_state[target_branch] ||= ApprovalState.new(self, target_branch: target_branch)
   end
+  # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
   def approvals_given
     approvals.size

ee/app/models/ee/project.rb

@@ -407,9 +407,9 @@ module EE
       super
     end
 
-    def visible_approval_rules
-      strong_memoize(:visible_approval_rules) do
-        visible_user_defined_rules + approval_rules.report_approver
+    def visible_approval_rules(target_branch: nil)
+      strong_memoize(:"visible_approval_rules_#{target_branch}") do
+        visible_user_defined_rules(branch: target_branch) + approval_rules.report_approver
       end
     end
 

ee/lib/api/merge_request_approvals.rb

@@ -27,10 +27,14 @@ module API
         render_api_error!(errors, 400)
       end
 
-      def present_merge_request_approval_state(presenter:)
+      def present_merge_request_approval_state(presenter:, target_branch: nil)
         merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
-        present merge_request.approval_state, with: presenter, current_user: current_user
+        present(
+          merge_request.approval_state(target_branch: target_branch),
+          with: presenter,
+          current_user: current_user
+        )
       end
     end
 
@@ -62,8 +66,14 @@ module API
           success: ::EE::API::Entities::MergeRequestApprovalSettings,
           hidden: true
         }
+        params do
+          optional :target_branch, type: String, desc: 'Branch that scoped approval rules apply to'
+        end
         get 'approval_settings' do
-          present_merge_request_approval_state(presenter: ::EE::API::Entities::MergeRequestApprovalSettings)
+          present_merge_request_approval_state(
+            presenter: ::EE::API::Entities::MergeRequestApprovalSettings,
+            target_branch: declared_params[:target_branch]
+          )
         end
 
         desc 'Get approval state of merge request' do

ee/lib/api/project_approval_settings.rb

@@ -15,10 +15,18 @@ module API
           detail 'Private API subject to change'
           success EE::API::Entities::ProjectApprovalSettings
         end
+        params do
+          optional :target_branch, type: String, desc: 'Branch that scoped approval rules apply to'
+        end
         get do
           authorize_create_merge_request_in_project
 
-          present user_project, with: EE::API::Entities::ProjectApprovalSettings, current_user: current_user
+          present(
+            user_project,
+            with: EE::API::Entities::ProjectApprovalSettings,
+            current_user: current_user,
+            target_branch: declared_params[:target_branch]
+          )
         end
 
         segment 'rules' do

ee/lib/ee/api/entities.rb

@@ -300,7 +300,10 @@ 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: ProjectApprovalSettingRule
+        expose :rules, using: ProjectApprovalSettingRule do |project, options|
+          project.visible_approval_rules(target_branch: options[:target_branch])
+        end
+
         expose :min_fallback_approvals, as: :fallback_approvals_required
       end
 

ee/spec/models/approval_state_spec.rb

@@ -1519,6 +1519,16 @@ describe ApprovalState do
                 another_project_rule
               ])
             end
+
+            context 'and target_branch is specified' do
+              subject { described_class.new(merge_request, target_branch: 'v1-stable') }
+
+              it 'returns the rules that are applicable to the specified target_branch' do
+                expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+                  project_rule
+                ])
+              end
+            end
           end
 
           context 'but scoped_approval_rules feature is disabled' do
@@ -1585,6 +1595,16 @@ describe ApprovalState do
                 another_mr_rule
               ])
             end
+
+            context 'and target_branch is specified' do
+              subject { described_class.new(merge_request, target_branch: 'v1-stable') }
+
+              it 'returns the rules that are applicable to the specified target_branch' do
+                expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+                  mr_rule
+                ])
+              end
+            end
           end
 
           context 'but scoped_approval_rules feature is disabled' do

ee/spec/requests/api/merge_request_approvals_spec.rb

@@ -165,6 +165,50 @@ describe API::MergeRequestApprovals do
       expect(rule_response['approved_by'][0]['username']).to eq(approver.username)
       expect(rule_response['source_rule']).to eq(nil)
     end
+
+    context 'when target_branch is specified' do
+      let(:protected_branch) { create(:protected_branch, project: project, name: 'master') }
+      let(:another_protected_branch) { create(:protected_branch, project: project, name: 'test') }
+
+      let(:project_rule) do
+        create(
+          :approval_project_rule,
+          project: project,
+          protected_branches: [protected_branch]
+        )
+      end
+
+      let(:another_project_rule) do
+        create(
+          :approval_project_rule,
+          project: project,
+          protected_branches: [another_protected_branch]
+        )
+      end
+
+      let!(:another_rule) do
+        create(
+          :approval_merge_request_rule,
+          approval_project_rule: another_project_rule,
+          merge_request: merge_request
+        )
+      end
+
+      before do
+        rule.update!(approval_project_rule: project_rule)
+      end
+
+      it 'filters the rules returned by target branch' do
+        get api("#{url}?target_branch=master", user)
+
+        expect(json_response['rules'].size).to eq(1)
+
+        rule_response = json_response['rules'].first
+
+        expect(rule_response['id']).to eq(rule.id)
+        expect(rule_response['name']).to eq('foo')
+      end
+    end
   end
 
   describe 'GET :id/merge_requests/:merge_request_iid/approval_state' do

ee/spec/requests/api/project_approval_settings_spec.rb

@@ -42,6 +42,36 @@ describe API::ProjectApprovalSettings do
         expect(rule['name']).to eq('security')
       end
 
+      context 'when target_branch is specified' do
+        let(:protected_branch) { create(:protected_branch, project: project, name: 'master') }
+        let(:another_protected_branch) { create(:protected_branch, project: project, name: 'test') }
+
+        let!(:another_rule) do
+          create(
+            :approval_project_rule,
+            name: 'test',
+            project: project,
+            protected_branches: [another_protected_branch]
+          )
+        end
+
+        before do
+          stub_licensed_features(multiple_approval_rules: true)
+          rule.update!(protected_branches: [protected_branch])
+        end
+
+        it 'filters the rules returned by target branch' do
+          get api("#{url}?target_branch=test", developer)
+
+          expect(json_response['rules'].size).to eq(1)
+
+          rule_response = json_response['rules'].first
+
+          expect(rule_response['id']).to eq(another_rule.id)
+          expect(rule_response['name']).to eq('test')
+        end
+      end
+
       context 'private group filtering' do
         let_it_be(:private_group) { create :group, :private }