Merge branch 'mr_nplus1_approvals' into 'master'

Fix N+1 issue in approval rules

See merge request gitlab-org/gitlab!45009

ee/app/models/approval_wrapped_rule.rb

@@ -115,12 +115,14 @@ class ApprovalWrappedRule
   end
 
   def overall_approver_ids
-    current_approvals = merge_request.approvals
+    strong_memoize(:overall_approver_ids) do
+      current_approvals = merge_request.approvals
 
-    if current_approvals.is_a?(ActiveRecord::Relation) && !current_approvals.loaded?
-      current_approvals.distinct.pluck(:user_id)
-    else
-      current_approvals.map(&:user_id).to_set
+      if current_approvals.is_a?(ActiveRecord::Relation) && !current_approvals.loaded?
+        current_approvals.distinct.pluck(:user_id)
+      else
+        current_approvals.map(&:user_id).to_set
+      end
     end
   end
 end

ee/changelogs/unreleased/mr_nplus1_approvals.yml

+---
+title: Optimize merge request approvals checking
+merge_request: 45009
+author:
+type: performance

ee/spec/models/approval_wrapped_rule_spec.rb

@@ -136,6 +136,22 @@ RSpec.describe ApprovalWrappedRule do
         expect(subject.approved_approvers).to contain_exactly(approver1)
       end
     end
+
+    it 'avoids N+1 queries' do
+      rule = create(:approval_project_rule, project: merge_request.project, approvals_required: approvals_required)
+      rule.users = [approver1]
+
+      approved_approvers_for_rule_id(rule.id) # warm up the cache
+      control_count = ActiveRecord::QueryRecorder.new { approved_approvers_for_rule_id(rule.id) }.count
+
+      rule.users += [approver2, approver3]
+
+      expect { approved_approvers_for_rule_id(rule.id) }.not_to exceed_query_limit(control_count)
+    end
+
+    def approved_approvers_for_rule_id(rule_id)
+      described_class.new(merge_request, ApprovalProjectRule.find(rule_id)).approved_approvers
+    end
   end
 
   describe "#commented_approvers" do