summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2018-01-29 23:53:47 -0200
committerOswaldo Ferreira <oswaldo@gitlab.com>2018-01-29 23:53:47 -0200
commitdbfedb5adb3651d3b700186b24a5b1b69d10a0dd (patch)
treec34420dc30ecfa6479595294d59fd623406bc550
parent1f309b69df3f71d988d4d31b08a1b683099a46e5 (diff)
downloadgitlab-ce-osw-short-circuit-mergeable-disccusions-state.tar.gz
Check MR state before submitting queries for discussion stateosw-short-circuit-mergeable-disccusions-state
-rw-r--r--app/models/merge_request.rb4
-rw-r--r--app/serializers/merge_request_widget_entity.rb13
-rw-r--r--changelogs/unreleased/osw-short-circuit-mergeable-disccusions-state.yml5
-rw-r--r--spec/models/merge_request_spec.rb4
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