diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-01-30 15:24:58 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-01-30 15:24:58 +0000 |
commit | 7f647f9fef818a55975512b17615d09826c0f354 (patch) | |
tree | 0d9548240bdd994ceef74bedf1284e38bd747ec6 | |
parent | 03f386c2b20f95272e4846f5ecab54fd8b2566e0 (diff) | |
parent | dbfedb5adb3651d3b700186b24a5b1b69d10a0dd (diff) | |
download | gitlab-ce-7f647f9fef818a55975512b17615d09826c0f354.tar.gz |
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
-rw-r--r-- | app/models/merge_request.rb | 4 | ||||
-rw-r--r-- | app/serializers/merge_request_widget_entity.rb | 13 | ||||
-rw-r--r-- | changelogs/unreleased/osw-short-circuit-mergeable-disccusions-state.yml | 5 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 4 |
4 files changed, 23 insertions, 3 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index f6d4843abc3..d025062f562 100644 --- a/app/models/merge_request.rb +++ b/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 diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 48cd2317f46..fbfe480503b 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/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 diff --git a/changelogs/unreleased/osw-short-circuit-mergeable-disccusions-state.yml b/changelogs/unreleased/osw-short-circuit-mergeable-disccusions-state.yml new file mode 100644 index 00000000000..62931218861 --- /dev/null +++ b/changelogs/unreleased/osw-short-circuit-mergeable-disccusions-state.yml @@ -0,0 +1,5 @@ +--- +title: Stop checking if discussions are in a mergeable state if the MR isn't +merge_request: +author: +type: performance diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index eb9690df313..243eeddc7a8 100644 --- a/spec/models/merge_request_spec.rb +++ b/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 |