Commit ee2f3cac authored by Phil Hughes's avatar Phil Hughes

Fix diff changes empty state

The empty state now only gets shown when no files exist in the branch.
If the user is reviewing 2 versions with no files, we don't show the state.

Refactors the diff app spec to use Vue test utils.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/48635
parent 6b68d82f
...@@ -42,6 +42,11 @@ export default { ...@@ -42,6 +42,11 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
changesEmptyStateIllustration: {
type: String,
required: false,
default: '',
},
}, },
data() { data() {
return { return {
...@@ -63,7 +68,7 @@ export default { ...@@ -63,7 +68,7 @@ export default {
plainDiffPath: state => state.diffs.plainDiffPath, plainDiffPath: state => state.diffs.plainDiffPath,
emailPatchPath: state => state.diffs.emailPatchPath, emailPatchPath: state => state.diffs.emailPatchPath,
}), }),
...mapState('diffs', ['showTreeList', 'isLoading']), ...mapState('diffs', ['showTreeList', 'isLoading', 'startVersion']),
...mapGetters('diffs', ['isParallelView']), ...mapGetters('diffs', ['isParallelView']),
...mapGetters(['isNotesFetched', 'getNoteableData']), ...mapGetters(['isNotesFetched', 'getNoteableData']),
targetBranch() { targetBranch() {
...@@ -79,6 +84,13 @@ export default { ...@@ -79,6 +84,13 @@ export default {
showCompareVersions() { showCompareVersions() {
return this.mergeRequestDiffs && this.mergeRequestDiff; return this.mergeRequestDiffs && this.mergeRequestDiff;
}, },
renderDiffFiles() {
return (
this.diffFiles.length > 0 ||
(this.startVersion &&
this.startVersion.version_index === this.mergeRequestDiff.version_index)
);
},
}, },
watch: { watch: {
diffViewType() { diffViewType() {
...@@ -191,7 +203,7 @@ export default { ...@@ -191,7 +203,7 @@ export default {
<div v-show="showTreeList" class="diff-tree-list"><tree-list /></div> <div v-show="showTreeList" class="diff-tree-list"><tree-list /></div>
<div class="diff-files-holder"> <div class="diff-files-holder">
<commit-widget v-if="commit" :commit="commit" /> <commit-widget v-if="commit" :commit="commit" />
<template v-if="diffFiles.length > 0"> <template v-if="renderDiffFiles">
<diff-file <diff-file
v-for="file in diffFiles" v-for="file in diffFiles"
:key="file.newPath" :key="file.newPath"
...@@ -199,7 +211,7 @@ export default { ...@@ -199,7 +211,7 @@ export default {
:can-current-user-fork="canCurrentUserFork" :can-current-user-fork="canCurrentUserFork"
/> />
</template> </template>
<no-changes v-else /> <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" />
</div> </div>
</div> </div>
</div> </div>
......
<script> <script>
import { mapState } from 'vuex'; import { mapGetters } from 'vuex';
import emptyImage from '~/../../views/shared/icons/_mr_widget_empty_state.svg'; import _ from 'underscore';
import { GlButton } from '@gitlab/ui';
import { __, sprintf } from '~/locale';
export default { export default {
data() { components: {
return { GlButton,
emptyImage, },
}; props: {
changesEmptyStateIllustration: {
type: String,
required: true,
},
}, },
computed: { computed: {
...mapState({ ...mapGetters(['getNoteableData']),
sourceBranch: state => state.notes.noteableData.source_branch, emptyStateText() {
targetBranch: state => state.notes.noteableData.target_branch, return sprintf(
newBlobPath: state => state.notes.noteableData.new_blob_path, __(
}), 'No changes between %{ref_start}%{source_branch}%{ref_end} and %{ref_start}%{target_branch}%{ref_end}',
),
{
ref_start: '<span class="ref-name">',
ref_end: '</span>',
source_branch: _.escape(this.getNoteableData.source_branch),
target_branch: _.escape(this.getNoteableData.target_branch),
},
false,
);
},
}, },
}; };
</script> </script>
<template> <template>
<div class="row empty-state nothing-here-block"> <div class="row empty-state">
<div class="col-xs-12"> <div class="col-12">
<div class="svg-content"><span v-html="emptyImage"></span></div> <div class="svg-content svg-250"><img :src="changesEmptyStateIllustration" /></div>
</div> </div>
<div class="col-xs-12"> <div class="col-12">
<div class="text-content text-center"> <div class="text-content text-center">
No changes between <span class="ref-name">{{ sourceBranch }}</span> and <span v-html="emptyStateText"></span>
<span class="ref-name">{{ targetBranch }}</span>
<div class="text-center"> <div class="text-center">
<a :href="newBlobPath" class="btn btn-success"> {{ __('Create commit') }} </a> <gl-button :href="getNoteableData.new_blob_path" variant="success">{{
__('Create commit')
}}</gl-button>
</div> </div>
</div> </div>
</div> </div>
......
...@@ -17,6 +17,7 @@ export default function initDiffsApp(store) { ...@@ -17,6 +17,7 @@ export default function initDiffsApp(store) {
endpoint: dataset.endpoint, endpoint: dataset.endpoint,
projectPath: dataset.projectPath, projectPath: dataset.projectPath,
currentUser: JSON.parse(dataset.currentUserData) || {}, currentUser: JSON.parse(dataset.currentUserData) || {},
changesEmptyStateIllustration: dataset.changesEmptyStateIllustration,
}; };
}, },
computed: { computed: {
...@@ -31,6 +32,7 @@ export default function initDiffsApp(store) { ...@@ -31,6 +32,7 @@ export default function initDiffsApp(store) {
currentUser: this.currentUser, currentUser: this.currentUser,
projectPath: this.projectPath, projectPath: this.projectPath,
shouldShow: this.activeTab === 'diffs', shouldShow: this.activeTab === 'diffs',
changesEmptyStateIllustration: this.changesEmptyStateIllustration,
}, },
}); });
}, },
......
...@@ -77,7 +77,8 @@ ...@@ -77,7 +77,8 @@
#js-diffs-app.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked?, #js-diffs-app.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked?,
endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', request.query_parameters), endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', request.query_parameters),
current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json, current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json,
project_path: project_path(@merge_request.project)} } project_path: project_path(@merge_request.project),
changes_empty_state_illustration: image_path('illustrations/merge_request_changes_empty.svg') } }
.mr-loading-status .mr-loading-status
= spinner = spinner
......
---
title: Fixed merge request diffs empty states
merge_request:
author:
type: fixed
...@@ -4368,6 +4368,9 @@ msgstr "" ...@@ -4368,6 +4368,9 @@ msgstr ""
msgid "No changes" msgid "No changes"
msgstr "" msgstr ""
msgid "No changes between %{ref_start}%{source_branch}%{ref_end} and %{ref_start}%{target_branch}%{ref_end}"
msgstr ""
msgid "No connection could be made to a Gitaly Server, please check your logs!" msgid "No connection could be made to a Gitaly Server, please check your logs!"
msgstr "" msgstr ""
......
import Vue from 'vue'; import Vuex from 'vuex';
import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { shallowMount, createLocalVue } from '@vue/test-utils';
import { TEST_HOST } from 'spec/test_constants'; import { TEST_HOST } from 'spec/test_constants';
import App from '~/diffs/components/app.vue'; import App from '~/diffs/components/app.vue';
import NoChanges from '~/diffs/components/no_changes.vue';
import DiffFile from '~/diffs/components/diff_file.vue';
import createDiffsStore from '../create_diffs_store'; import createDiffsStore from '../create_diffs_store';
describe('diffs/components/app', () => { describe('diffs/components/app', () => {
const oldMrTabs = window.mrTabs; const oldMrTabs = window.mrTabs;
const Component = Vue.extend(App); let store;
let vm; let vm;
beforeEach(() => { function createComponent(props = {}, extendStore = () => {}) {
// setup globals (needed for component to mount :/) const localVue = createLocalVue();
window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']);
window.mrTabs.expandViewContainer = jasmine.createSpy(); localVue.use(Vuex);
window.location.hash = 'ABC_123';
// setup component store = createDiffsStore();
const store = createDiffsStore();
store.state.diffs.isLoading = false; store.state.diffs.isLoading = false;
vm = mountComponentWithStore(Component, { extendStore(store);
store,
props: { vm = shallowMount(localVue.extend(App), {
localVue,
propsData: {
endpoint: `${TEST_HOST}/diff/endpoint`, endpoint: `${TEST_HOST}/diff/endpoint`,
projectPath: 'namespace/project', projectPath: 'namespace/project',
currentUser: {}, currentUser: {},
changesEmptyStateIllustration: '',
...props,
}, },
store,
}); });
}
beforeEach(() => {
// setup globals (needed for component to mount :/)
window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']);
window.mrTabs.expandViewContainer = jasmine.createSpy();
window.location.hash = 'ABC_123';
}); });
afterEach(() => { afterEach(() => {
...@@ -35,21 +46,53 @@ describe('diffs/components/app', () => { ...@@ -35,21 +46,53 @@ describe('diffs/components/app', () => {
window.mrTabs = oldMrTabs; window.mrTabs = oldMrTabs;
// reset component // reset component
vm.$destroy(); vm.destroy();
}); });
it('does not show commit info', () => { it('does not show commit info', () => {
expect(vm.$el).not.toContainElement('.blob-commit-info'); createComponent();
expect(vm.contains('.blob-commit-info')).toBe(false);
}); });
it('sets highlighted row if hash exists in location object', done => { it('sets highlighted row if hash exists in location object', done => {
vm.$props.shouldShow = true; createComponent({
shouldShow: true,
vm.$nextTick() });
.then(() => {
expect(vm.$store.state.diffs.highlightedRow).toBe('ABC_123'); // Component uses $nextTick so we wait until that has finished
}) setTimeout(() => {
.then(done) expect(store.state.diffs.highlightedRow).toBe('ABC_123');
.catch(done.fail);
done();
});
});
describe('empty state', () => {
it('renders empty state when no diff files exist', () => {
createComponent();
expect(vm.contains(NoChanges)).toBe(true);
});
it('does not render empty state when diff files exist', () => {
createComponent({}, () => {
store.state.diffs.diffFiles.push({
id: 1,
});
});
expect(vm.contains(NoChanges)).toBe(false);
expect(vm.findAll(DiffFile).length).toBe(1);
});
it('does not render empty state when versions match', () => {
createComponent({}, () => {
store.state.diffs.startVersion = { version_index: 1 };
store.state.diffs.mergeRequestDiff = { version_index: 1 };
});
expect(vm.contains(NoChanges)).toBe(false);
});
}); });
}); });
// TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 import { createLocalVue, shallowMount } from '@vue/test-utils';
import Vuex from 'vuex';
import { createStore } from '~/mr_notes/stores';
import NoChanges from '~/diffs/components/no_changes.vue';
describe('Diff no changes empty state', () => {
let vm;
function createComponent(extendStore = () => {}) {
const localVue = createLocalVue();
localVue.use(Vuex);
const store = createStore();
extendStore(store);
vm = shallowMount(localVue.extend(NoChanges), {
localVue,
store,
propsData: {
changesEmptyStateIllustration: '',
},
});
}
afterEach(() => {
vm.destroy();
});
it('prevents XSS', () => {
createComponent(store => {
// eslint-disable-next-line no-param-reassign
store.state.notes.noteableData = {
source_branch: '<script>alert("test");</script>',
target_branch: '<script>alert("test");</script>',
};
});
expect(vm.contains('script')).toBe(false);
});
});
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