Show warning when files are hidden in an MR

What was happening was that, for example, if our limit files was 1,000
and our diff had 1,500 we would only save 1000 files from gitaly in
the database, and when reading them back it wouldn't overflow since it
only contains 1,000 files, and as such, no warning would be shown.
However, it is still overflown, and the warning should be shown.

MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/64515
Issue: gitlab.com/gitlab-org/gitlab/-/issues/332883

Changelog: fixed

app/helpers/diff_helper.rb

@@ -190,10 +190,8 @@ module DiffHelper
   end
 
   def render_overflow_warning?(diffs_collection)
-    diff_files = diffs_collection.raw_diff_files
-
-    diff_files.overflow?.tap do |overflown|
-      log_overflow_limits(diff_files)
+    diffs_collection.overflow?.tap do |overflown|
+      log_overflow_limits(diff_files: diffs_collection.raw_diff_files, collection_overflow: overflown)
     end
   end
 
@@ -285,12 +283,12 @@ module DiffHelper
     conflicts_service.conflicts.files.index_by(&:our_path)
   end
 
-  def log_overflow_limits(diff_files)
+  def log_overflow_limits(diff_files:, collection_overflow:)
     if diff_files.any?(&:too_large?)
       Gitlab::Metrics.add_event(:diffs_overflow_single_file_limits)
     end
 
-    Gitlab::Metrics.add_event(:diffs_overflow_collection_limits) if diff_files.overflow?
+    Gitlab::Metrics.add_event(:diffs_overflow_collection_limits) if collection_overflow
     Gitlab::Metrics.add_event(:diffs_overflow_max_bytes_limits) if diff_files.overflow_max_bytes?
     Gitlab::Metrics.add_event(:diffs_overflow_max_files_limits) if diff_files.overflow_max_files?
     Gitlab::Metrics.add_event(:diffs_overflow_max_lines_limits) if diff_files.overflow_max_lines?

app/serializers/merge_request_diff_entity.rb

@@ -16,7 +16,6 @@ class MergeRequestDiffEntity < Grape::Entity
   end
 
   expose :created_at
-  expose :state
   expose :commits_count
 
   expose :latest?, as: :latest

lib/gitlab/diff/file_collection/base.rb

@@ -85,6 +85,10 @@ module Gitlab
           # No-op
         end
 
+        def overflow?
+          raw_diff_files.overflow?
+        end
+
         private
 
         def empty_pagination_data

lib/gitlab/diff/file_collection/merge_request_diff_base.rb

@@ -48,6 +48,10 @@ module Gitlab
           @merge_request_diff.real_size
         end
 
+        def overflow?
+          @merge_request_diff.overflow?
+        end
+
         private
 
         def highlight_cache

spec/helpers/diff_helper_spec.rb

@@ -293,23 +293,22 @@ RSpec.describe DiffHelper do
   describe '#render_overflow_warning?' do
     using RSpec::Parameterized::TableSyntax
 
-    let(:diffs_collection) { instance_double(Gitlab::Diff::FileCollection::MergeRequestDiff, raw_diff_files: diff_files) }
+    let(:diffs_collection) { instance_double(Gitlab::Diff::FileCollection::MergeRequestDiff, raw_diff_files: diff_files, overflow?: false) }
     let(:diff_files) { Gitlab::Git::DiffCollection.new(files) }
     let(:safe_file) { { too_large: false, diff: '' } }
     let(:large_file) { { too_large: true, diff: '' } }
     let(:files) { [safe_file, safe_file] }
 
-    before do
-      allow(diff_files).to receive(:overflow?).and_return(false)
-      allow(diff_files).to receive(:overflow_max_bytes?).and_return(false)
-      allow(diff_files).to receive(:overflow_max_files?).and_return(false)
-      allow(diff_files).to receive(:overflow_max_lines?).and_return(false)
-      allow(diff_files).to receive(:collapsed_safe_bytes?).and_return(false)
-      allow(diff_files).to receive(:collapsed_safe_files?).and_return(false)
-      allow(diff_files).to receive(:collapsed_safe_lines?).and_return(false)
-    end
-
     context 'when no limits are hit' do
+      before do
+        allow(diff_files).to receive(:overflow_max_bytes?).and_return(false)
+        allow(diff_files).to receive(:overflow_max_files?).and_return(false)
+        allow(diff_files).to receive(:overflow_max_lines?).and_return(false)
+        allow(diff_files).to receive(:collapsed_safe_bytes?).and_return(false)
+        allow(diff_files).to receive(:collapsed_safe_files?).and_return(false)
+        allow(diff_files).to receive(:collapsed_safe_lines?).and_return(false)
+      end
+
       it 'returns false and does not log any overflow events' do
         expect(Gitlab::Metrics).not_to receive(:add_event).with(:diffs_overflow_collection_limits)
         expect(Gitlab::Metrics).not_to receive(:add_event).with(:diffs_overflow_single_file_limits)
@@ -343,7 +342,7 @@ RSpec.describe DiffHelper do
 
     context 'when the file collection has an overflow' do
       before do
-        allow(diff_files).to receive(:overflow?).and_return(true)
+        allow(diffs_collection).to receive(:overflow?).and_return(true)
       end
 
       it 'returns true and only logs all the correct collection overflow event' do

spec/lib/gitlab/diff/file_collection/base_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Diff::FileCollection::Base do
+  let(:merge_request) { create(:merge_request) }
+  let(:diffable) { merge_request.merge_request_diff }
+  let(:diff_options) { {} }
+
+  describe '#overflow?' do
+    subject(:overflown) { described_class.new(diffable, project: merge_request.project, diff_options: diff_options).overflow? }
+
+    context 'when it is not overflown' do
+      it 'returns false' do
+        expect(overflown).to eq(false)
+      end
+    end
+
+    context 'when it is overflown' do
+      let(:diff_options) { { max_files: 1 } }
+
+      it 'returns true' do
+        expect(overflown).to eq(true)
+      end
+    end
+  end
+end

spec/lib/gitlab/diff/file_collection/merge_request_diff_base_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Diff::FileCollection::MergeRequestDiffBase do
+  let(:merge_request) { create(:merge_request) }
+  let(:diffable) { merge_request.merge_request_diff }
+
+  describe '#overflow?' do
+    subject(:overflown) { described_class.new(diffable, diff_options: nil).overflow? }
+
+    context 'when it is not overflown' do
+      it 'returns false' do
+        expect(overflown).to eq(false)
+      end
+    end
+
+    context 'when it is overflown' do
+      before do
+        diffable.update!(state: :overflow)
+      end
+
+      it 'returns true' do
+        expect(overflown).to eq(true)
+      end
+    end
+  end
+end

spec/serializers/merge_request_diff_entity_spec.rb

@@ -29,7 +29,7 @@ RSpec.describe MergeRequestDiffEntity do
       expect(subject).to include(
         :version_index, :created_at, :commits_count,
         :latest, :short_commit_sha, :version_path,
-        :compare_path, :state
+        :compare_path
       )
     end
   end