Commit 8d2e1b72 authored by Nick Thomas's avatar Nick Thomas

Merge branch '54786-mr-empty-file-display' into 'master'

Display empty files properly on MR diffs

Closes #54786

See merge request gitlab-org/gitlab-ce!23671
parents c027101d 9f9eb03f
<script> <script>
import { mapActions, mapGetters, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
import EmptyFileViewer from '~/vue_shared/components/diff_viewer/viewers/empty_file.vue';
import InlineDiffView from './inline_diff_view.vue'; import InlineDiffView from './inline_diff_view.vue';
import ParallelDiffView from './parallel_diff_view.vue'; import ParallelDiffView from './parallel_diff_view.vue';
import NoteForm from '../../notes/components/note_form.vue'; import NoteForm from '../../notes/components/note_form.vue';
...@@ -17,6 +18,7 @@ export default { ...@@ -17,6 +18,7 @@ export default {
NoteForm, NoteForm,
DiffDiscussions, DiffDiscussions,
ImageDiffOverlay, ImageDiffOverlay,
EmptyFileViewer,
}, },
props: { props: {
diffFile: { diffFile: {
...@@ -75,14 +77,15 @@ export default { ...@@ -75,14 +77,15 @@ export default {
<div class="diff-content"> <div class="diff-content">
<div class="diff-viewer"> <div class="diff-viewer">
<template v-if="isTextFile"> <template v-if="isTextFile">
<empty-file-viewer v-if="diffFile.empty" />
<inline-diff-view <inline-diff-view
v-if="isInlineView" v-else-if="isInlineView"
:diff-file="diffFile" :diff-file="diffFile"
:diff-lines="diffFile.highlighted_diff_lines || []" :diff-lines="diffFile.highlighted_diff_lines || []"
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
/> />
<parallel-diff-view <parallel-diff-view
v-if="isParallelView" v-else-if="isParallelView"
:diff-file="diffFile" :diff-file="diffFile"
:diff-lines="diffFile.parallel_diff_lines || []" :diff-lines="diffFile.parallel_diff_lines || []"
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
......
<template>
<div class="nothing-here-block">{{ __('Empty file') }}</div>
</template>
...@@ -5,6 +5,7 @@ class DiffFileEntity < DiffFileBaseEntity ...@@ -5,6 +5,7 @@ class DiffFileEntity < DiffFileBaseEntity
include IconsHelper include IconsHelper
expose :too_large?, as: :too_large expose :too_large?, as: :too_large
expose :empty?, as: :empty
expose :added_lines expose :added_lines
expose :removed_lines expose :removed_lines
......
---
title: Display empty files properly on MR diffs
merge_request: 23671
author: Sean Nichols
type: fixed
...@@ -255,6 +255,10 @@ module Gitlab ...@@ -255,6 +255,10 @@ module Gitlab
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def empty?
valid_blobs.map(&:empty?).all?
end
def raw_binary? def raw_binary?
try_blobs(:raw_binary?) try_blobs(:raw_binary?)
end end
......
...@@ -2668,6 +2668,9 @@ msgstr "" ...@@ -2668,6 +2668,9 @@ msgstr ""
msgid "Embed" msgid "Embed"
msgstr "" msgstr ""
msgid "Empty file"
msgstr ""
msgid "Enable" msgid "Enable"
msgstr "" msgstr ""
......
...@@ -50,6 +50,45 @@ describe('DiffContent', () => { ...@@ -50,6 +50,45 @@ describe('DiffContent', () => {
}); });
}); });
describe('empty files', () => {
beforeEach(() => {
vm.diffFile.empty = true;
vm.diffFile.highlighted_diff_lines = [];
vm.diffFile.parallel_diff_lines = [];
});
it('should render a message', done => {
vm.$nextTick(() => {
const block = vm.$el.querySelector('.diff-viewer .nothing-here-block');
expect(block).not.toBe(null);
expect(block.textContent.trim()).toContain('Empty file');
done();
});
});
it('should not render multiple messages', done => {
vm.diffFile.mode_changed = true;
vm.diffFile.b_mode = '100755';
vm.diffFile.viewer.name = 'mode_changed';
vm.$nextTick(() => {
expect(vm.$el.querySelectorAll('.nothing-here-block').length).toBe(1);
done();
});
});
it('should not render diff table', done => {
vm.$nextTick(() => {
expect(vm.$el.querySelector('table')).toBe(null);
done();
});
});
});
describe('Non-Text diffs', () => { describe('Non-Text diffs', () => {
beforeEach(() => { beforeEach(() => {
vm.diffFile.viewer.name = 'image'; vm.diffFile.viewer.name = 'image';
......
...@@ -583,6 +583,12 @@ describe Gitlab::Diff::File do ...@@ -583,6 +583,12 @@ describe Gitlab::Diff::File do
end end
end end
describe '#empty?' do
it 'returns true' do
expect(diff_file.empty?).to be_truthy
end
end
describe '#different_type?' do describe '#different_type?' do
it 'returns false' do it 'returns false' do
expect(diff_file).not_to be_different_type expect(diff_file).not_to be_different_type
...@@ -662,4 +668,87 @@ describe Gitlab::Diff::File do ...@@ -662,4 +668,87 @@ describe Gitlab::Diff::File do
end end
end end
end end
describe '#empty?' do
let(:project) do
create(:project, :custom_repo, files: {})
end
let(:branch_name) { 'master' }
def create_file(file_name, content)
Files::CreateService.new(
project,
project.owner,
commit_message: 'Update',
start_branch: branch_name,
branch_name: branch_name,
file_path: file_name,
file_content: content
).execute
project.commit(branch_name).diffs.diff_files.first
end
def update_file(file_name, content)
Files::UpdateService.new(
project,
project.owner,
commit_message: 'Update',
start_branch: branch_name,
branch_name: branch_name,
file_path: file_name,
file_content: content
).execute
project.commit(branch_name).diffs.diff_files.first
end
def delete_file(file_name)
Files::DeleteService.new(
project,
project.owner,
commit_message: 'Update',
start_branch: branch_name,
branch_name: branch_name,
file_path: file_name
).execute
project.commit(branch_name).diffs.diff_files.first
end
context 'when empty file is created' do
it 'returns true' do
diff_file = create_file('empty.md', '')
expect(diff_file.empty?).to be_truthy
end
end
context 'when empty file is deleted' do
it 'returns true' do
create_file('empty.md', '')
diff_file = delete_file('empty.md')
expect(diff_file.empty?).to be_truthy
end
end
context 'when file with content is truncated' do
it 'returns false' do
create_file('with-content.md', 'file content')
diff_file = update_file('with-content.md', '')
expect(diff_file.empty?).to be_falsey
end
end
context 'when empty file has content added' do
it 'returns false' do
create_file('empty.md', '')
diff_file = update_file('empty.md', 'new content')
expect(diff_file.empty?).to be_falsey
end
end
end
end end
...@@ -32,7 +32,7 @@ shared_examples 'diff file entity' do ...@@ -32,7 +32,7 @@ shared_examples 'diff file entity' do
it 'exposes correct attributes' do it 'exposes correct attributes' do
expect(subject).to include(:too_large, :added_lines, :removed_lines, expect(subject).to include(:too_large, :added_lines, :removed_lines,
:context_lines_path, :highlighted_diff_lines, :context_lines_path, :highlighted_diff_lines,
:parallel_diff_lines) :parallel_diff_lines, :empty)
end end
it 'includes viewer' do it 'includes viewer' do
......
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