summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/merge_request.rb17
-rw-r--r--app/models/merge_request_diff.rb2
-rw-r--r--changelogs/unreleased/mr-diff-size-overflow.yml4
-rw-r--r--lib/gitlab/diff/file_collection/merge_request_diff.rb4
-rw-r--r--spec/features/merge_requests/diffs_spec.rb14
-rw-r--r--spec/models/merge_request_spec.rb32
6 files changed, 50 insertions, 23 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1d4827375d7..9d2288c311e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -191,22 +191,23 @@ class MergeRequest < ActiveRecord::Base
merge_request_diff ? merge_request_diff.raw_diffs(*args) : compare.raw_diffs(*args)
end
- def diffs(diff_options = nil)
+ def diffs(diff_options = {})
if compare
- compare.diffs(diff_options)
+ # When saving MR diffs, `no_collapse` is implicitly added (because we need
+ # to save the entire contents to the DB), so add that here for
+ # consistency.
+ compare.diffs(diff_options.merge(no_collapse: true))
else
merge_request_diff.diffs(diff_options)
end
end
def diff_size
- # The `#diffs` method ends up at an instance of a class inheriting from
- # `Gitlab::Diff::FileCollection::Base`, so use those options as defaults
- # here too, to get the same diff size without performing highlighting.
- #
- opts = Gitlab::Diff::FileCollection::Base.default_options.merge(diff_options || {})
+ # Calling `merge_request_diff.diffs.real_size` will also perform
+ # highlighting, which we don't need here.
+ return real_size if merge_request_diff
- raw_diffs(opts).size
+ diffs.real_size
end
def diff_base_commit
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 6604af2b47e..f0a3c30ea74 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -260,7 +260,7 @@ class MergeRequestDiff < ActiveRecord::Base
new_attributes[:state] = :empty
else
diff_collection = compare.diffs(Commit.max_diff_options)
- new_attributes[:real_size] = compare.diffs.real_size
+ new_attributes[:real_size] = diff_collection.real_size
if diff_collection.any?
new_diffs = dump_diffs(diff_collection)
diff --git a/changelogs/unreleased/mr-diff-size-overflow.yml b/changelogs/unreleased/mr-diff-size-overflow.yml
new file mode 100644
index 00000000000..87449930cf2
--- /dev/null
+++ b/changelogs/unreleased/mr-diff-size-overflow.yml
@@ -0,0 +1,4 @@
+---
+title: Show sizes correctly in merge requests when diffs overflow
+merge_request:
+author:
diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb
index 329d12f13d1..0bd226ef050 100644
--- a/lib/gitlab/diff/file_collection/merge_request_diff.rb
+++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb
@@ -15,6 +15,10 @@ module Gitlab
super.tap { |_| store_highlight_cache }
end
+ def real_size
+ @merge_request_diff.real_size
+ end
+
private
# Extracted method to highlight in the same iteration to the diff_collection.
diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb
index 4a6c76a5caf..b495bbef8d9 100644
--- a/spec/features/merge_requests/diffs_spec.rb
+++ b/spec/features/merge_requests/diffs_spec.rb
@@ -1,10 +1,11 @@
require 'spec_helper'
feature 'Diffs URL', js: true, feature: true do
+ let(:merge_request) { create(:merge_request) }
+ let(:project) { merge_request.source_project }
+
before do
login_as :admin
- @merge_request = create(:merge_request)
- @project = @merge_request.source_project
end
context 'when visit with */* as accept header' do
@@ -13,9 +14,9 @@ feature 'Diffs URL', js: true, feature: true do
end
it 'renders the notes' do
- create :note_on_merge_request, project: @project, noteable: @merge_request, note: 'Rebasing with master'
+ create :note_on_merge_request, project: project, noteable: merge_request, note: 'Rebasing with master'
- visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
+ visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
# Load notes and diff through AJAX
expect(page).to have_css('.note-text', visible: false, text: 'Rebasing with master')
@@ -25,10 +26,9 @@ feature 'Diffs URL', js: true, feature: true do
context 'when merge request has overflow' do
it 'displays warning' do
- allow_any_instance_of(MergeRequestDiff).to receive(:overflow?).and_return(true)
- allow(Commit).to receive(:max_diff_options).and_return(max_files: 20, max_lines: 20)
+ allow(Commit).to receive(:max_diff_options).and_return(max_files: 3)
- visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
+ 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. Plain diff Email patch To preserve
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 415d3e7b200..be08b96641a 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -199,10 +199,10 @@ describe MergeRequest, models: true do
end
context 'when there are no MR diffs' do
- it 'delegates to the compare object' do
+ it 'delegates to the compare object, setting no_collapse: true' do
merge_request.compare = double(:compare)
- expect(merge_request.compare).to receive(:diffs).with(options)
+ expect(merge_request.compare).to receive(:diffs).with(options.merge(no_collapse: true))
merge_request.diffs(options)
end
@@ -215,15 +215,22 @@ describe MergeRequest, models: true do
end
context 'when there are MR diffs' do
- before do
+ it 'returns the correct count' do
merge_request.save
+
+ expect(merge_request.diff_size).to eq('105')
end
- it 'returns the correct count' do
- expect(merge_request.diff_size).to eq(105)
+ it 'returns the correct overflow count' do
+ allow(Commit).to receive(:max_diff_options).and_return(max_files: 2)
+ merge_request.save
+
+ expect(merge_request.diff_size).to eq('2+')
end
it 'does not perform highlighting' do
+ merge_request.save
+
expect(Gitlab::Diff::Highlight).not_to receive(:new)
merge_request.diff_size
@@ -231,7 +238,7 @@ describe MergeRequest, models: true do
end
context 'when there are no MR diffs' do
- before do
+ def set_compare(merge_request)
merge_request.compare = CompareService.new(
merge_request.source_project,
merge_request.source_branch
@@ -242,10 +249,21 @@ describe MergeRequest, models: true do
end
it 'returns the correct count' do
- expect(merge_request.diff_size).to eq(105)
+ set_compare(merge_request)
+
+ expect(merge_request.diff_size).to eq('105')
+ end
+
+ it 'returns the correct overflow count' do
+ allow(Commit).to receive(:max_diff_options).and_return(max_files: 2)
+ set_compare(merge_request)
+
+ expect(merge_request.diff_size).to eq('2+')
end
it 'does not perform highlighting' do
+ set_compare(merge_request)
+
expect(Gitlab::Diff::Highlight).not_to receive(:new)
merge_request.diff_size
View Lorry Controller Status