Merge branch '47196-actionview-missingtemplate-missing-template-projects-merge_requests-invalid-projects-merge_requests-application-invalid-pr' into 'master'

Resolve "ActionView::MissingTemplate: Missing template projects/merge_requests/invalid, projects/merge_requests/application/invalid, pr..."

Closes #47196

See merge request gitlab-org/gitlab-ce!19356

3 files changed, 37 insertions, 22 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index ecea6e1b2bf..b452bfd7e6f 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -28,15 +28,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def show
-    validates_merge_request
-    close_merge_request_without_source_project
-    check_if_can_be_merged
-
-    # Return if the response has already been rendered
-    return if response_body
+    close_merge_request_if_no_source_project
+    mark_merge_request_mergeable
 
     respond_to do |format|
       format.html do
+        # use next to appease Rubocop
+        next render('invalid') if target_branch_missing?
+
         # Build a note object for comment form
         @note = @project.notes.new(noteable: @merge_request)
 
@@ -234,20 +233,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   alias_method :issuable, :merge_request
   alias_method :awardable, :merge_request
 
-  def validates_merge_request
-    # Show git not found page
-    # if there is no saved commits between source & target branch
-    if @merge_request.has_no_commits?
-      # and if target branch doesn't exist
-      return invalid_mr unless @merge_request.target_branch_exists?
-    end
-  end
-
-  def invalid_mr
-    # Render special view for MR with removed target branch
-    render 'invalid'
-  end
-
   def merge_params
     params.permit(merge_params_attributes)
   end
@@ -261,7 +246,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
       @merge_request.head_pipeline && @merge_request.head_pipeline.active?
   end
 
-  def close_merge_request_without_source_project
+  def close_merge_request_if_no_source_project
     if !@merge_request.source_project && @merge_request.open?
       @merge_request.close
     end
@@ -269,7 +254,11 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
   private
 
-  def check_if_can_be_merged
+  def target_branch_missing?
+    @merge_request.has_no_commits? && !@merge_request.target_branch_exists?
+  end
+
+  def mark_merge_request_mergeable
     @merge_request.check_if_can_be_merged
   end
 
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 6e8de6db9c3..6e710c9b20b 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -80,6 +80,16 @@ describe Projects::MergeRequestsController do
                                                                 ))
         end
       end
+
+      context "that is invalid" do
+        let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) }
+
+        it "renders merge request page" do
+          go(format: :html)
+
+          expect(response).to be_success
+        end
+      end
     end
 
     describe 'as json' do
@@ -106,6 +116,16 @@ describe Projects::MergeRequestsController do
           expect(response).to match_response_schema('entities/merge_request_widget')
         end
       end
+
+      context "that is invalid" do
+        let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) }
+
+        it "renders merge request page" do
+          go(format: :json)
+
+          expect(response).to be_success
+        end
+      end
     end
 
     describe "as diff" do
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index fab0ec22450..3441ce1b8cb 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -54,6 +54,11 @@ FactoryBot.define do
       state :opened
     end
 
+    trait :invalid do
+      source_branch "feature_one"
+      target_branch "feature_two"
+    end
+
     trait :locked do
       state :locked
     end
@@ -98,6 +103,7 @@ FactoryBot.define do
     factory :merged_merge_request, traits: [:merged]
     factory :closed_merge_request, traits: [:closed]
     factory :reopened_merge_request, traits: [:opened]
+    factory :invalid_merge_request, traits: [:invalid]
     factory :merge_request_with_diffs, traits: [:with_diffs]
     factory :merge_request_with_diff_notes do
       after(:create) do |mr|