Merge branch 'issue_25017' into 'master'
Show 'too many changes' message for merge request Closes #25017 See merge request !8444
This commit is contained in:
commit
e324ccc20c
7 changed files with 63 additions and 21 deletions
|
@ -165,4 +165,10 @@ module DiffHelper
|
||||||
|
|
||||||
link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class]
|
link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def render_overflow_warning?(diff_files)
|
||||||
|
diffs = @merge_request_diff.presence || diff_files
|
||||||
|
|
||||||
|
diffs.overflow?
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -234,28 +234,28 @@ class MergeRequestDiff < ActiveRecord::Base
|
||||||
# and save it as array of hashes in st_diffs db field
|
# and save it as array of hashes in st_diffs db field
|
||||||
def save_diffs
|
def save_diffs
|
||||||
new_attributes = {}
|
new_attributes = {}
|
||||||
new_diffs = []
|
|
||||||
|
|
||||||
if commits.size.zero?
|
if commits.size.zero?
|
||||||
new_attributes[:state] = :empty
|
new_attributes[:state] = :empty
|
||||||
else
|
else
|
||||||
diff_collection = compare.diffs(Commit.max_diff_options)
|
diff_collection = compare.diffs(Commit.max_diff_options)
|
||||||
|
new_attributes[:real_size] = compare.diffs.real_size
|
||||||
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?
|
if diff_collection.any?
|
||||||
new_diffs = dump_diffs(diff_collection)
|
new_diffs = dump_diffs(diff_collection)
|
||||||
new_attributes[:state] = :collected
|
new_attributes[:state] = :collected
|
||||||
end
|
end
|
||||||
|
|
||||||
|
new_attributes[:st_diffs] = new_diffs || []
|
||||||
|
|
||||||
|
# Set our state to 'overflow' to make the #empty? and #collected?
|
||||||
|
# methods (generated by StateMachine) return false.
|
||||||
|
#
|
||||||
|
# This attribution has to come at the end of the method so 'overflow'
|
||||||
|
# state does not get overridden by 'collected'.
|
||||||
|
new_attributes[:state] = :overflow if diff_collection.overflow?
|
||||||
end
|
end
|
||||||
|
|
||||||
new_attributes[:st_diffs] = new_diffs
|
|
||||||
update_columns_serialized(new_attributes)
|
update_columns_serialized(new_attributes)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -18,8 +18,8 @@
|
||||||
= parallel_diff_btn
|
= parallel_diff_btn
|
||||||
= render 'projects/diffs/stats', diff_files: diff_files
|
= 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
|
= render 'projects/diffs/warning', diff_files: diffs
|
||||||
|
|
||||||
.files{ data: { can_create_note: can_create_note } }
|
.files{ data: { can_create_note: can_create_note } }
|
||||||
- diff_files.each_with_index do |diff_file|
|
- diff_files.each_with_index do |diff_file|
|
||||||
|
|
|
@ -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/merge_requests/show/versions'
|
||||||
= render "projects/diffs/diffs", diffs: @diffs
|
= render "projects/diffs/diffs", diffs: @diffs
|
||||||
- elsif @merge_request_diff.empty?
|
- elsif @merge_request_diff.empty?
|
||||||
.nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
|
.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.
|
|
||||||
|
|
4
changelogs/unreleased/issue_25017.yml
Normal file
4
changelogs/unreleased/issue_25017.yml
Normal file
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Show 'too many changes' message for created merge requests when they are too large
|
||||||
|
merge_request:
|
||||||
|
author:
|
|
@ -22,4 +22,18 @@ feature 'Diffs URL', js: true, feature: true do
|
||||||
expect(page).to have_css('.diffs.tab-pane.active')
|
expect(page).to have_css('.diffs.tab-pane.active')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
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)
|
||||||
|
|
||||||
|
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.")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -76,6 +76,32 @@ describe MergeRequestDiff, models: true do
|
||||||
end
|
end
|
||||||
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
|
||||||
|
|
||||||
|
it 'saves empty state' do
|
||||||
|
allow_any_instance_of(MergeRequestDiff).to receive(:commits)
|
||||||
|
.and_return([])
|
||||||
|
|
||||||
|
mr_diff = create(:merge_request).merge_request_diff
|
||||||
|
|
||||||
|
expect(mr_diff.empty?).to be_truthy
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#commits_sha' do
|
describe '#commits_sha' do
|
||||||
it 'returns all commits SHA using serialized commits' do
|
it 'returns all commits SHA using serialized commits' do
|
||||||
subject.st_commits = [
|
subject.st_commits = [
|
||||||
|
|
Loading…
Reference in a new issue