Commit 9852cae8 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'mr-diff-size-overflow' into 'master'

Show correct size when MR diff overflows

Closes #26560

See merge request !10827
parents a34f0e7b a0979c05
......@@ -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
......
......@@ -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)
......
---
title: Show sizes correctly in merge requests when diffs overflow
merge_request:
author:
......@@ -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.
......
require 'spec_helper'
feature 'Diffs URL', js: true, feature: true do
include ApplicationHelper
let(:author_user) { create(:user) }
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:forked_project) { Projects::ForkService.new(project, author_user).execute }
let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) }
let(:merge_request) { create(:merge_request, source_project: project) }
context 'when visit with */* as accept header' do
before(:each) do
......@@ -27,20 +22,23 @@ 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)
page.within('.alert') do
expect(page).to have_text("Too many changes to show. Plain diff Email patch To preserve
performance only 3 of 3 files are displayed.")
performance only 3 of 3+ files are displayed.")
end
end
end
describe 'when editing file' do
let(:changelog_id) { hexdigest("CHANGELOG") }
context 'when editing file' do
let(:author_user) { create(:user) }
let(:user) { create(:user) }
let(:forked_project) { Projects::ForkService.new(project, author_user).execute }
let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) }
let(:changelog_id) { Digest::SHA1.hexdigest("CHANGELOG") }
context 'as author' do
it 'shows direct edit link' do
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment