diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 014a6ce023b51fd0e86bf6be7302b7f2ae6e831f..f7d1253d957c569508e9ad6082c69ce5f8ac55bb 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -327,9 +327,10 @@ class MergeRequest < ActiveRecord::Base !source_project.forked_from?(target_project) end - def can_reopen? + def reopenable? return false if closed_without_fork? || closed_without_source_project? || merged? - return true if closed? + + closed? 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 b8013c4ee823684c3f5d180328fb5b69e15cdb73..3900b4f6f1736fce2c984eca093a058853425d9a 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"} - - if @merge_request.can_reopen? + - if @merge_request.reopenable? = 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 4b8daacc7e0c5cbeaf2af2bbc532ece891196e72..3b815ded2d38f54aa284709796a08bc7f1d190f7 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1043,7 +1043,7 @@ describe MergeRequest, models: true do let(:project) { create(:project) } let(:user) { create(:user) } let(:fork_project) { create(:project, forked_from_project: project, namespace: user.namespace) } - let(:destroy_project) { Projects::DestroyService.new(fork_project, user, {}) } + let(:destroy_service) { Projects::DestroyService.new(fork_project, user) } context 'when the merge request is closed' do let(:closed_merge_request) do @@ -1057,7 +1057,7 @@ describe MergeRequest, models: true do end it 'returns true if the source project does not exist' do - destroy_project.execute + destroy_service.execute closed_merge_request.reload expect(closed_merge_request.closed_without_source_project?).to be_truthy @@ -1071,12 +1071,12 @@ describe MergeRequest, models: true do end end - describe '#can_reopen?' do + describe '#reopenable?' do context 'when the merge request is closed' do it 'returns true' do subject.close - expect(subject.can_reopen?).to be_truthy + expect(subject.reopenable?).to be_truthy end context 'forked project' do @@ -1092,26 +1092,26 @@ describe MergeRequest, models: true do it 'returns false if unforked' do Projects::UnlinkForkService.new(fork_project, user).execute - expect(merge_request.reload.can_reopen?).to be_falsey + expect(merge_request.reload.reopenable?).to be_falsey end it 'returns false if the source project is deleted' do - Projects::DestroyService.new(fork_project, user, {}).execute + Projects::DestroyService.new(fork_project, user).execute - expect(merge_request.reload.can_reopen?).to be_falsey + expect(merge_request.reload.reopenable?).to be_falsey end it 'returns false if the merge request is merged' do merge_request.update_attributes(state: 'merged') - expect(merge_request.reload.can_reopen?).to be_falsey + expect(merge_request.reload.reopenable?).to be_falsey end end end context 'when the merge request is opened' do it 'returns false' do - expect(subject.can_reopen?).to be_falsey + expect(subject.reopenable?).to be_falsey end end end