Show 'too many changes' message for merge requestissue_25578

7 files changed, 51 insertions, 20 deletions

diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index c35d6611ab0..a9c8f040151 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -165,4 +165,12 @@ module DiffHelper
 
     link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class]
   end
+
+  def render_overflow_warning?(diff_files)
+    if @merge_request.present?
+      @merge_request.diff_overflow?
+    else
+      diff_files.overflow?
+    end
+  end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 61845bf4036..859899175b0 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -211,6 +211,12 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
+  def diff_overflow?
+    return false unless merge_request_diff
+
+    merge_request_diff.overflow?
+  end
+
   # MRs created before 8.4 don't store a MergeRequestDiff#base_commit_sha,
   # but we need to get a commit for the "View file @ ..." link by deleted files,
   # so we find the likely one if we can't get the actual one.
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index b8f36a2c958..b0edcbfb782 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -234,28 +234,24 @@ class MergeRequestDiff < ActiveRecord::Base
   # and save it as array of hashes in st_diffs db field
   def save_diffs
     new_attributes = {}
-    new_diffs = []
 
     if commits.size.zero?
       new_attributes[:state] = :empty
     else
+      new_attributes[:real_size] = compare.diffs.real_size
       diff_collection = compare.diffs(Commit.max_diff_options)
 
-      if diff_collection.overflow?
-        # Set our state to 'overflow' to make the #empty? and #collected?
-        # methods (generated by StateMachine) return false.
-        new_attributes[:state] = :overflow
-      end
-
-      new_attributes[:real_size] = diff_collection.real_size
-
       if diff_collection.any?
         new_diffs = dump_diffs(diff_collection)
+        new_attributes[:st_diffs] = new_diffs
         new_attributes[:state] = :collected
       end
+
+      # Set our state to 'overflow' to make the #empty? and #collected?
+      # methods (generated by StateMachine) return false.
+      new_attributes[:state] = :overflow if diff_collection.overflow?
     end
 
-    new_attributes[:st_diffs] = new_diffs
     update_columns_serialized(new_attributes)
   end
 
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index ab4a2dc36e5..c25792d528f 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -18,7 +18,7 @@
       = parallel_diff_btn
   = render 'projects/diffs/stats', diff_files: diff_files
 
-- if diff_files.overflow?
+- if render_overflow_warning?(diff_files)
   = render 'projects/diffs/warning', diff_files: diff_files
 
 .files{ data: { can_create_note: can_create_note } }
diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml
index 99c71e1454a..5f048d04b27 100644
--- a/app/views/projects/merge_requests/show/_diffs.html.haml
+++ b/app/views/projects/merge_requests/show/_diffs.html.haml
@@ -1,13 +1,5 @@
-- if @merge_request_diff.collected?
+- if @merge_request_diff.collected? || @merge_request_diff.overflow?
   = render 'projects/merge_requests/show/versions'
   = render "projects/diffs/diffs", diffs: @diffs
 - elsif @merge_request_diff.empty?
   .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
-- else
-  .alert.alert-warning
-    %h4
-      Changes view for this comparison is extremely large.
-    %p
-      You can
-      = link_to "download it", merge_request_path(@merge_request, format: :diff), class: "vlink"
-      instead.
diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb
index c9a0059645d..bd216aab3e0 100644
--- a/spec/features/merge_requests/diffs_spec.rb
+++ b/spec/features/merge_requests/diffs_spec.rb
@@ -22,4 +22,16 @@ feature 'Diffs URL', js: true, feature: true do
       expect(page).to have_css('.diffs.tab-pane.active')
     end
   end
+
+  context 'when merge request has overflow' do
+    it 'displays warning' do
+      allow_any_instance_of(MergeRequest).to receive(:diff_overflow?).and_return(true)
+
+      visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
+
+      page.within('.alert') do
+        expect(page).to have_text('Too many changes to show')
+      end
+    end
+  end
 end
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index eb876d105da..e1f83c5c925 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -76,6 +76,23 @@ describe MergeRequestDiff, models: true do
     end
   end
 
+  describe '#save diffs' do
+    it 'saves collected state' do
+      mr_diff = create(:merge_request).merge_request_diff
+
+      expect(mr_diff.collected?).to be_truthy
+    end
+
+    it 'saves overflow state' do
+      allow(Commit).to receive(:max_diff_options)
+        .and_return({ max_lines: 0, max_files: 0 })
+
+      mr_diff = create(:merge_request).merge_request_diff
+
+      expect(mr_diff.overflow?).to be_truthy
+    end
+  end
+
   describe '#commits_sha' do
     it 'returns all commits SHA using serialized commits' do
       subject.st_commits = [