Add #can_reopen? and tests

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