diff options
author | Katarzyna Kobierska <kkobierska@gmail.com> | 2016-09-08 13:25:16 +0200 |
---|---|---|
committer | Katarzyna Kobierska <kkobierska@gmail.com> | 2016-09-13 14:40:05 +0200 |
commit | 34c146a17dbb66322bc57b0755c979c05e7d4340 (patch) | |
tree | 94c876b4047f7cdff99a2a1f176891c7c39cd52e | |
parent | 554baec9e8d2d077afff5502fdfa4ab78036499c (diff) | |
download | gitlab-ce-34c146a17dbb66322bc57b0755c979c05e7d4340.tar.gz |
Add #can_reopen? and tests
-rw-r--r-- | app/helpers/merge_requests_helper.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 4 | ||||
-rw-r--r-- | app/views/projects/merge_requests/_discussion.html.haml | 2 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 64 |
4 files changed, 48 insertions, 24 deletions
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 8abe7865fed..e9a61e2c2a6 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -98,7 +98,7 @@ module MergeRequestsHelper end def merge_request_button_visibility(merge_request, closed) - return 'hidden' if merge_request.closed? == closed || (merge_request.merged? == closed && !merge_request.closed?) || merge_request.closed_without_fork? + return 'hidden' if merge_request.can_reopen? end def merge_request_version_path(project, merge_request, merge_request_diff, start_sha = nil) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5986a464bbc..014a6ce023b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -328,10 +328,8 @@ class MergeRequest < ActiveRecord::Base end def can_reopen? - return false if closed_without_fork? || closed_without_source_project? + return false if closed_without_fork? || closed_without_source_project? || merged? return true if closed? - - # false end def ensure_merge_request_diff diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml index 0aec7b2fbbb..b8013c4ee82 100644 --- a/app/views/projects/merge_requests/_discussion.html.haml +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -2,7 +2,7 @@ - if can?(current_user, :update_merge_request, @merge_request) - if @merge_request.open? = link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"} - - unless @merge_request.open? || @merge_request.closed_without_fork? + - if @merge_request.can_reopen? = link_to 'Reopen merge request', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request", data: {original_text: "Reopen merge request", alternative_text: "Comment & reopen merge request"} %comment-and-resolve-btn{ "inline-template" => true, ":discussion-id" => "" } %button.btn.btn-nr.btn-default.append-right-10.js-comment-resolve-button{ "v-if" => "showButton", type: "submit", data: { namespace_path: "#{@merge_request.project.namespace.path}", project_path: "#{@merge_request.project.path}" } } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 4780d9fb3fe..d58156ba045 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1039,24 +1039,24 @@ describe MergeRequest, models: true do end end - describe "#closed_without_source_project?" do + describe '#closed_without_source_project?' do let(:project) { create(:project) } let(:fork_project) { create(:project, forked_from_project: project) } let(:user) { create(:user) } let(:destroy_project) { Projects::DestroyService.new(fork_project, user, {}) } - context "when the merge request is closed" do + context 'when the merge request is closed' do let(:closed_merge_request) do create(:closed_merge_request, source_project: fork_project, target_project: project) end - it "returns false if the source project exist" do + it 'returns false if the source project exists' do expect(closed_merge_request.closed_without_source_project?).to be_falsey end - it "returns true if the source project does not exist" do + it 'returns true if the source project does not exist' do destroy_project.async_execute closed_merge_request.reload @@ -1064,29 +1064,55 @@ describe MergeRequest, models: true do end end - context "when the merge request is open" do - let(:open_merge_request) do - create(:merge_request, - source_project: fork_project, - target_project: project) - end - - it "returns false" do - expect(open_merge_request.closed_without_source_project?).to be_falsey + context 'when the merge request is open' do + it 'returns false' do + expect(subject.closed_without_source_project?).to be_falsey end end end describe '#can_reopen?' do - it "returns true" do - subject.close - binding.pry + context 'when the merge request is closed' do + it 'returns true' do + subject.close - expect(subject.can_reopen?).to be_truthy + expect(subject.can_reopen?).to be_truthy + end + + context 'forked project' do + let(:project) { create(:project) } + let(:fork_project) { create(:project, forked_from_project: project) } + let(:user) { create(:user) } + let(:merge_request) do + create(:closed_merge_request, + source_project: fork_project, + target_project: project) + end + + it 'returns false if unforked' do + Projects::UnlinkForkService.new(fork_project, user).execute + + expect(merge_request.reload.can_reopen?).to be_falsey + end + + it 'returns false if the source project is deleted' do + Projects::DestroyService.new(fork_project, user, {}).async_execute + + expect(merge_request.reload.can_reopen?).to be_falsey + end + + it 'returnes false if the merge request is merged' do + merge_request.update_attributes(state: 'merged') + + expect(merge_request.reload.can_reopen?).to be_falsey + end + end end - it "returns false" do - expect(subject.can_reopen?).to be_falsey + context 'when merge request is opened' do + it 'returns false' do + expect(subject.can_reopen?).to be_falsey + end end end end |