Revert "Merge branch 'hide-empty-merge-request-diffs' into 'master'"revert-7a5e653f

This reverts merge request !7568

9 files changed, 37 insertions, 57 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index dbbd2ad849e..b343ba0b744 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -82,12 +82,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     @merge_request_diff =
       if params[:diff_id]
-        @merge_request.merge_request_diffs.viewable.find(params[:diff_id])
+        @merge_request.merge_request_diffs.find(params[:diff_id])
       else
         @merge_request.merge_request_diff
       end
 
-    @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
+    @merge_request_diffs = @merge_request.merge_request_diffs.select_without_diff
     @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
 
     if params[:start_sha].present?
@@ -417,7 +417,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     response = {
       title: merge_request.title,
-      sha: (merge_request.diff_head_commit.short_id if merge_request.diff_head_sha),
+      sha: merge_request.diff_head_commit.short_id,
       status: status,
       coverage: coverage
     }
@@ -564,7 +564,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   def define_pipelines_vars
     @pipelines = @merge_request.all_pipelines
 
-    if @pipelines.present? && @merge_request.commits.present?
+    if @pipelines.present?
       @pipeline = @pipelines.first
       @statuses = @pipeline.statuses.relevant
     end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 58a24eb84cb..dd65a9a8b86 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -11,9 +11,6 @@ class MergeRequestDiff < ActiveRecord::Base
 
   belongs_to :merge_request
 
-  serialize :st_commits
-  serialize :st_diffs
-
   state_machine :state, initial: :empty do
     state :collected
     state :overflow
@@ -25,7 +22,8 @@ class MergeRequestDiff < ActiveRecord::Base
     state :overflow_diff_lines_limit
   end
 
-  scope :viewable, -> { without_state(:empty) }
+  serialize :st_commits
+  serialize :st_diffs
 
   # All diff information is collected from repository after object is created.
   # It allows you to override variables like head_commit_sha before getting diff.
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 22596b4014a..4a7e6930842 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -60,15 +60,7 @@ module MergeRequests
       merge_requests = filter_merge_requests(merge_requests)
 
       merge_requests.each do |merge_request|
-        if merge_request.source_branch == @branch_name || force_push?
-          merge_request.reload_diff
-        else
-          mr_commit_ids = merge_request.commits.map(&:id)
-          push_commit_ids = @commits.map(&:id)
-          matches = mr_commit_ids & push_commit_ids
-          merge_request.reload_diff if matches.any?
-        end
-
+        reload_diff(merge_request) unless branch_removed?
         merge_request.mark_as_unchecked
       end
     end
@@ -173,5 +165,16 @@ module MergeRequests
     def branch_removed?
       Gitlab::Git.blank_ref?(@newrev)
     end
+
+    def reload_diff(merge_request)
+      if merge_request.source_branch == @branch_name || force_push?
+        merge_request.reload_diff
+      else
+        mr_commit_ids = merge_request.commits.map(&:id)
+        push_commit_ids = @commits.map(&:id)
+        matches = mr_commit_ids & push_commit_ids
+        merge_request.reload_diff if matches.any?
+      end
+    end
   end
 end
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index 20c93930abc..ac26aa569ba 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/app/views/projects/merge_requests/widget/_open.html.haml
@@ -9,10 +9,10 @@
 
     - if @project.archived?
       = render 'projects/merge_requests/widget/open/archived'
-    - elsif @merge_request.branch_missing?
-      = render 'projects/merge_requests/widget/open/missing_branch'
     - elsif @merge_request.commits.blank?
       = render 'projects/merge_requests/widget/open/nothing'
+    - elsif @merge_request.branch_missing?
+      = render 'projects/merge_requests/widget/open/missing_branch'
     - elsif @merge_request.unchecked?
       = render 'projects/merge_requests/widget/open/check'
     - elsif @merge_request.cannot_be_merged? && !resolved_conflicts
diff --git a/changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml b/changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml
new file mode 100644
index 00000000000..a6bee989f6d
--- /dev/null
+++ b/changelogs/unreleased/fix-merge-request-screen-deleted-source-branch.yml
@@ -0,0 +1,4 @@
+---
+title: Do not create a MergeRequestDiff record when source branch is deleted
+merge_request: 7481
+author:
diff --git a/changelogs/unreleased/hide-empty-merge-request-diffs.yml b/changelogs/unreleased/hide-empty-merge-request-diffs.yml
deleted file mode 100644
index e32a51b555a..00000000000
--- a/changelogs/unreleased/hide-empty-merge-request-diffs.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Fix errors happening when source branch of merge request is removed and then restored
-merge_request: 7568
-author:
diff --git a/spec/features/merge_requests/deleted_source_branch_spec.rb b/spec/features/merge_requests/deleted_source_branch_spec.rb
deleted file mode 100644
index 778b3a90cf3..00000000000
--- a/spec/features/merge_requests/deleted_source_branch_spec.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-require 'spec_helper'
-
-describe 'Deleted source branch', feature: true, js: true do
-  let(:user) { create(:user) }
-  let(:merge_request) { create(:merge_request) }
-
-  before do
-    login_as user
-    merge_request.project.team << [user, :master]
-    merge_request.update!(source_branch: 'this-branch-does-not-exist')
-    visit namespace_project_merge_request_path(
-      merge_request.project.namespace,
-      merge_request.project, merge_request
-    )
-  end
-
-  it 'shows a message about missing source branch' do
-    expect(page).to have_content(
-      'Source branch this-branch-does-not-exist does not exist'
-    )
-  end
-
-  it 'hides Discussion, Commits and Changes tabs' do
-    within '.merge-request-details' do
-      expect(page).to have_no_content('Discussion')
-      expect(page).to have_no_content('Commits')
-      expect(page).to have_no_content('Changes')
-    end
-  end
-end
diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb
index 09451f41de4..23cee891bac 100644
--- a/spec/features/merge_requests/merge_request_versions_spec.rb
+++ b/spec/features/merge_requests/merge_request_versions_spec.rb
@@ -3,12 +3,11 @@ require 'spec_helper'
 feature 'Merge Request versions', js: true, feature: true do
   let(:merge_request) { create(:merge_request, importing: true) }
   let(:project) { merge_request.source_project }
-  let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
-  let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) }
-  let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
 
   before do
     login_as :admin
+    merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+    merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e')
     visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
   end
 
@@ -54,7 +53,7 @@ feature 'Merge Request versions', js: true, feature: true do
         project.namespace,
         project,
         merge_request.iid,
-        diff_id: merge_request_diff3.id,
+        diff_id: 2,
         start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9'
       )
     end
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index e515bc9f89c..0220f7e1db2 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -227,6 +227,16 @@ describe MergeRequests::RefreshService, services: true do
       end
     end
 
+    context 'when the source branch is deleted' do
+      it 'does not create a MergeRequestDiff record' do
+        refresh_service = service.new(@project, @user)
+
+        expect do
+          refresh_service.execute(@oldrev, Gitlab::Git::BLANK_SHA, 'refs/heads/master')
+        end.not_to change { MergeRequestDiff.count }
+      end
+    end
+
     def reload_mrs
       @merge_request.reload
       @fork_merge_request.reload