Merge branch 'osw-short-circuit-mergeable-disccusions-state' into 'master'

Check MR state before submitting queries for discussion state

Closes #42236

See merge request gitlab-org/gitlab-ce!16788

app/models/merge_request.rb

@@ -618,12 +618,12 @@ class MergeRequest < ActiveRecord::Base
     can_be_merged? && !should_be_rebased?
   end
 
-  def mergeable_state?(skip_ci_check: false)
+  def mergeable_state?(skip_ci_check: false, skip_discussions_check: false)
     return false unless open?
     return false if work_in_progress?
     return false if broken?
     return false unless skip_ci_check || mergeable_ci_state?
-    return false unless mergeable_discussions_state?
+    return false unless skip_discussions_check || mergeable_discussions_state?
 
     true
   end

app/serializers/merge_request_widget_entity.rb

@@ -48,7 +48,18 @@ class MergeRequestWidgetEntity < IssuableEntity
   expose :merge_ongoing?, as: :merge_ongoing
   expose :work_in_progress?, as: :work_in_progress
   expose :source_branch_exists?, as: :source_branch_exists
-  expose :mergeable_discussions_state?, as: :mergeable_discussions_state
+
+  expose :mergeable_discussions_state?, as: :mergeable_discussions_state do |merge_request|
+    # This avoids calling MergeRequest#mergeable_discussions_state without
+    # considering the state of the MR first. If a MR isn't mergeable, we can
+    # safely short-circuit it.
+    if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
+      merge_request.mergeable_discussions_state?
+    else
+      false
+    end
+  end
+
   expose :branch_missing?, as: :branch_missing
   expose :commits_count
   expose :cannot_be_merged?, as: :has_conflicts

changelogs/unreleased/osw-short-circuit-mergeable-disccusions-state.yml

+---
+title: Stop checking if discussions are in a mergeable state if the MR isn't
+merge_request:
+author:
+type: performance

spec/models/merge_request_spec.rb

@@ -1339,6 +1339,10 @@ describe MergeRequest do
         it 'returns false' do
           expect(subject.mergeable_state?).to be_falsey
         end
+
+        it 'returns true when skipping discussions check' do
+          expect(subject.mergeable_state?(skip_discussions_check: true)).to be(true)
+        end
       end
     end
   end