diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2016-11-23 14:13:27 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2016-11-23 14:13:27 +0000 |
commit | 8fcdfeaee23b87a1d7d8357e693ce7d9df97ab6a (patch) | |
tree | d3a48a79d3d60ef00a67fa0e141afab438b1fc19 | |
parent | 4646d453b3211e29ec5afba2568766018caf32e0 (diff) | |
parent | f2a0c6f6bf768db8837283ad65fe6790b9105d26 (diff) | |
download | gitlab-ce-8fcdfeaee23b87a1d7d8357e693ce7d9df97ab6a.tar.gz |
Merge branch '24863-mrs-without-discussions-are-mergeable' into 'master'
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/24863
See merge request !7698
-rw-r--r-- | app/models/merge_request.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/24863-mrs-without-discussions-are-mergeable.yml | 4 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 54 |
3 files changed, 63 insertions, 1 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 6c3c093d084..fdf54cc8a7e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -494,10 +494,14 @@ class MergeRequest < ActiveRecord::Base discussions_resolvable? && diff_discussions.none?(&:to_be_resolved?) end + def discussions_to_be_resolved? + discussions_resolvable? && !discussions_resolved? + end + def mergeable_discussions_state? return true unless project.only_allow_merge_if_all_discussions_are_resolved? - discussions_resolved? + !discussions_to_be_resolved? end def hook_attrs diff --git a/changelogs/unreleased/24863-mrs-without-discussions-are-mergeable.yml b/changelogs/unreleased/24863-mrs-without-discussions-are-mergeable.yml new file mode 100644 index 00000000000..9bdb9411135 --- /dev/null +++ b/changelogs/unreleased/24863-mrs-without-discussions-are-mergeable.yml @@ -0,0 +1,4 @@ +--- +title: Correctly determine mergeability of MR with no discussions +merge_request: +author: diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0b4277b1edd..58ccd056328 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -937,6 +937,16 @@ describe MergeRequest, models: true do expect(merge_request.mergeable_discussions_state?).to be_falsey end end + + context 'with no discussions' do + before do + merge_request.notes.destroy_all + end + + it 'returns true' do + expect(merge_request.mergeable_discussions_state?).to be_truthy + end + end end context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do @@ -1198,6 +1208,50 @@ describe MergeRequest, models: true do end end end + + describe "#discussions_to_be_resolved?" do + context "when discussions are not resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when discussions are resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(true) + + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable discussions are resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when some resolvable discussions are not resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.discussions_to_be_resolved?).to be true + end + end + end + end end describe '#conflicts_can_be_resolved_in_ui?' do |