From 950836f293b246b4753201a48594ae97ce931c61 Mon Sep 17 00:00:00 2001
From: Marcel van Remmerden <mvanremmerden@gitlab.com>
Date: Wed, 9 Dec 2020 14:06:39 +0100
Subject: [PATCH 1/4] Hide file browser and display options from empty state
changes tab
- Also improves empty state text
---
.../javascripts/diffs/components/app.vue | 40 ++++----
.../diffs/components/compare_versions.vue | 27 +++---
.../diffs/components/no_changes.vue | 42 +++++++--
.../diffs/store/getters_versions_dropdowns.js | 20 ++--
...equests-should-not-show-file-browser-a.yml | 6 ++
locale/gitlab.pot | 2 +-
.../merge_request/user_sees_versions_spec.rb | 4 +-
spec/frontend/diffs/components/app_spec.js | 67 ++++++--------
.../diffs/components/compare_versions_spec.js | 92 ++++++++++++-------
.../diffs/components/no_changes_spec.js | 75 +++++++++++----
.../store/getters_versions_dropdowns_spec.js | 6 +-
11 files changed, 236 insertions(+), 145 deletions(-)
create mode 100644 changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index c6e3b4772568d229..4e0adaaf0a1711d0 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -141,19 +141,17 @@ export default {
},
computed: {
...mapState({
- isLoading: (state) => state.diffs.isLoading,
- isBatchLoading: (state) => state.diffs.isBatchLoading,
- diffFiles: (state) => state.diffs.diffFiles,
- diffViewType: (state) => state.diffs.diffViewType,
- mergeRequestDiffs: (state) => state.diffs.mergeRequestDiffs,
- mergeRequestDiff: (state) => state.diffs.mergeRequestDiff,
- commit: (state) => state.diffs.commit,
- renderOverflowWarning: (state) => state.diffs.renderOverflowWarning,
- numTotalFiles: (state) => state.diffs.realSize,
- numVisibleFiles: (state) => state.diffs.size,
- plainDiffPath: (state) => state.diffs.plainDiffPath,
- emailPatchPath: (state) => state.diffs.emailPatchPath,
- retrievingBatches: (state) => state.diffs.retrievingBatches,
+ isLoading: state => state.diffs.isLoading,
+ isBatchLoading: state => state.diffs.isBatchLoading,
+ diffFiles: state => state.diffs.diffFiles,
+ diffViewType: state => state.diffs.diffViewType,
+ commit: state => state.diffs.commit,
+ renderOverflowWarning: state => state.diffs.renderOverflowWarning,
+ numTotalFiles: state => state.diffs.realSize,
+ numVisibleFiles: state => state.diffs.size,
+ plainDiffPath: state => state.diffs.plainDiffPath,
+ emailPatchPath: state => state.diffs.emailPatchPath,
+ retrievingBatches: state => state.diffs.retrievingBatches,
}),
...mapState('diffs', [
'showTreeList',
@@ -186,17 +184,16 @@ export default {
return this.currentUser.can_fork === true && this.currentUser.can_create_merge_request;
},
renderDiffFiles() {
- return (
- this.diffFiles.length > 0 ||
- (this.startVersion &&
- this.startVersion.version_index === this.mergeRequestDiff.version_index)
- );
+ return this.diffFiles.length > 0;
+ },
+ renderFileTree() {
+ return this.renderDiffFiles && this.showTreeList;
},
hideFileStats() {
return this.treeWidth <= TREE_HIDE_STATS_WIDTH;
},
isLimitedContainer() {
- return !this.showTreeList && !this.isParallelView && !this.isFluidLayout;
+ return !this.renderFileTree && !this.isParallelView && !this.isFluidLayout;
},
isDiffHead() {
return parseBoolean(getParameterByName('diff_head'));
@@ -259,7 +256,7 @@ export default {
this.adjustView();
},
isLoading: 'adjustView',
- showTreeList: 'adjustView',
+ renderFileTree: 'adjustView',
},
mounted() {
this.setBaseConfig({
@@ -467,7 +464,6 @@ export default {
<div v-if="isLoading || !isTreeLoaded" class="loading"><gl-loading-icon size="lg" /></div>
<div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane">
<compare-versions
- :merge-request-diffs="mergeRequestDiffs"
:is-limited-container="isLimitedContainer"
:diff-files-count-text="numTotalFiles"
/>
@@ -495,7 +491,7 @@ export default {
class="files d-flex gl-mt-2"
>
<div
- v-if="showTreeList"
+ v-if="renderFileTree"
:style="{ width: `${treeWidth}px` }"
class="diff-tree-list js-diff-tree-list px-3 pr-md-0"
>
diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue
index f3cc359a67971669..489278fd6ef891ca 100644
--- a/app/assets/javascripts/diffs/components/compare_versions.vue
+++ b/app/assets/javascripts/diffs/components/compare_versions.vue
@@ -22,10 +22,6 @@ export default {
GlTooltip: GlTooltipDirective,
},
props: {
- mergeRequestDiffs: {
- type: Array,
- required: true,
- },
isLimitedContainer: {
type: Boolean,
required: false,
@@ -44,6 +40,7 @@ export default {
'diffCompareDropdownSourceVersions',
]),
...mapState('diffs', [
+ 'diffFiles',
'commit',
'showTreeList',
'startVersion',
@@ -51,12 +48,15 @@ export default {
'addedLines',
'removedLines',
]),
- showDropdowns() {
- return !this.commit && this.mergeRequestDiffs.length;
- },
toggleFileBrowserTitle() {
return this.showTreeList ? __('Hide file browser') : __('Show file browser');
},
+ hasChanges() {
+ return this.diffFiles.length > 0;
+ },
+ hasSourceVersions() {
+ return this.diffCompareDropdownSourceVersions.length > 0;
+ },
},
created() {
this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
@@ -82,6 +82,7 @@ export default {
}"
>
<gl-button
+ v-if="hasChanges"
v-gl-tooltip.hover
variant="default"
icon="file-tree"
@@ -90,8 +91,12 @@ export default {
:selected="showTreeList"
@click="setShowTreeList({ showTreeList: !showTreeList })"
/>
+ <div v-if="commit">
+ {{ __('Viewing commit') }}
+ <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link>
+ </div>
<gl-sprintf
- v-if="showDropdowns"
+ v-else-if="hasSourceVersions"
class="d-flex align-items-center compare-versions-container"
:message="s__('MergeRequest|Compare %{target} and %{source}')"
>
@@ -109,11 +114,7 @@ export default {
/>
</template>
</gl-sprintf>
- <div v-else-if="commit">
- {{ __('Viewing commit') }}
- <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link>
- </div>
- <div class="inline-parallel-buttons d-none d-md-flex ml-auto">
+ <div v-if="hasChanges" class="inline-parallel-buttons d-none d-md-flex ml-auto">
<diff-stats
:diff-files-count-text="diffFilesCountText"
:added-lines="addedLines"
diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue
index a640dcb0a90df501..9da13548809dfbf3 100644
--- a/app/assets/javascripts/diffs/components/no_changes.vue
+++ b/app/assets/javascripts/diffs/components/no_changes.vue
@@ -14,7 +14,31 @@ export default {
},
},
computed: {
+ ...mapGetters('diffs', [
+ 'diffCompareDropdownTargetVersions',
+ 'diffCompareDropdownSourceVersions',
+ ]),
...mapGetters(['getNoteableData']),
+ selectedSourceVersion() {
+ return this.diffCompareDropdownSourceVersions.find(x => x.selected);
+ },
+ sourceName() {
+ if (!this.selectedSourceVersion || this.selectedSourceVersion.isLatestVersion) {
+ return this.getNoteableData.source_branch;
+ }
+
+ return this.selectedSourceVersion.versionName;
+ },
+ selectedTargetVersion() {
+ return this.diffCompareDropdownTargetVersions.find(x => x.selected);
+ },
+ targetName() {
+ if (!this.selectedTargetVersion || this.selectedTargetVersion.version_index < 0) {
+ return this.getNoteableData.target_branch;
+ }
+
+ return this.selectedTargetVersion.versionName || '';
+ },
},
};
</script>
@@ -26,14 +50,16 @@ export default {
</div>
<div class="col-12">
<div class="text-content text-center">
- <gl-sprintf :message="__('No changes between %{sourceBranch} and %{targetBranch}')">
- <template #sourceBranch>
- <span class="ref-name">{{ getNoteableData.source_branch }}</span>
- </template>
- <template #targetBranch>
- <span class="ref-name">{{ getNoteableData.target_branch }}</span>
- </template>
- </gl-sprintf>
+ <div data-testid="no-changes-message">
+ <gl-sprintf :message="__('No changes between %{source} and %{target}')">
+ <template #source>
+ <span class="ref-name">{{ sourceName }}</span>
+ </template>
+ <template #target>
+ <span class="ref-name">{{ targetName }}</span>
+ </template>
+ </gl-sprintf>
+ </div>
<div class="text-center">
<gl-button :href="getNoteableData.new_blob_path" variant="success" category="primary">{{
__('Create commit')
diff --git a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
index 0d1fd26777f2578d..3f33b0c900ecc8f7 100644
--- a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
+++ b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
@@ -58,14 +58,18 @@ export const diffCompareDropdownTargetVersions = (state, getters) => {
export const diffCompareDropdownSourceVersions = (state, getters) => {
// Appended properties here are to make the compare_dropdown_layout easier to reason about
- return state.mergeRequestDiffs.map((v, i) => ({
- ...v,
- href: v.version_path,
- commitsText: n__(`%d commit,`, `%d commits,`, v.commits_count),
- versionName:
- i === 0
+ return state.mergeRequestDiffs.map((v, i) => {
+ const isLatestVersion = i === 0;
+
+ return {
+ ...v,
+ href: v.version_path,
+ commitsText: n__(`%d commit,`, `%d commits,`, v.commits_count),
+ isLatestVersion,
+ versionName: isLatestVersion
? __('latest version')
: sprintf(__(`version %{versionIndex}`), { versionIndex: v.version_index }),
- selected: v.version_index === getters.selectedSourceIndex,
- }));
+ selected: v.version_index === getters.selectedSourceIndex,
+ };
+ });
};
diff --git a/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml b/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
new file mode 100644
index 0000000000000000..d7972982989fd657
--- /dev/null
+++ b/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
@@ -0,0 +1,6 @@
+---
+title: Remove diff display preferences and file tree from changes empty state
+merge_request: 43467
+
+author:
+type: fixed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 88d9f3f3cbfd0055..79ca6fc802e14006 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -18929,7 +18929,7 @@ msgstr ""
msgid "No changes"
msgstr ""
-msgid "No changes between %{sourceBranch} and %{targetBranch}"
+msgid "No changes between %{source} and %{target}"
msgstr ""
msgid "No child epics match applied filters"
diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb
index 8930c55a28cd6274..8999c4d665673489 100644
--- a/spec/features/merge_request/user_sees_versions_spec.rb
+++ b/spec/features/merge_request/user_sees_versions_spec.rb
@@ -191,7 +191,7 @@
find('.btn-default').click
click_link 'version 1'
end
- expect(page).to have_content '0 files'
+ expect(page).to have_content 'No changes between version 1 and version 1'
end
end
@@ -217,7 +217,7 @@
expect(page).to have_content 'version 1'
end
- expect(page).to have_content '0 files'
+ expect(page).to have_content 'No changes between version 1 and version 1'
end
end
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js
index 56cc0c8b8cfbbac2..d50f5133e048d9d1 100644
--- a/spec/frontend/diffs/components/app_spec.js
+++ b/spec/frontend/diffs/components/app_spec.js
@@ -147,31 +147,21 @@ describe('diffs/components/app', () => {
});
});
- it('adds container-limiting classes when showFileTree is false with inline diffs', () => {
- createComponent({}, ({ state }) => {
- state.diffs.showTreeList = false;
- state.diffs.isParallelView = false;
- });
-
- expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(true);
- });
-
- it('does not add container-limiting classes when showFileTree is false with inline diffs', () => {
- createComponent({}, ({ state }) => {
- state.diffs.showTreeList = true;
- state.diffs.isParallelView = false;
- });
-
- expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(false);
- });
-
- it('does not add container-limiting classes when isFluidLayout', () => {
- createComponent({ isFluidLayout: true }, ({ state }) => {
- state.diffs.isParallelView = false;
- });
-
- expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(false);
- });
+ it.each`
+ props | state | expected
+ ${{ isFluidLayout: true }} | ${{ isParallelView: false }} | ${false}
+ ${{}} | ${{ isParallelView: false }} | ${true}
+ ${{}} | ${{ showTreeList: true, diffFiles: [{}], isParallelView: false }} | ${false}
+ ${{}} | ${{ showTreeList: false, diffFiles: [{}], isParallelView: false }} | ${true}
+ ${{}} | ${{ showTreeList: false, diffFiles: [], isParallelView: false }} | ${true}
+ `(
+ 'uses container-limiting classes ($expected) with state ($state) and props ($props)',
+ ({ props, state, expected }) => {
+ createComponent(props, ({ state: origState }) => Object.assign(origState.diffs, state));
+
+ expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(expected);
+ },
+ );
it('displays loading icon on loading', () => {
createComponent({}, ({ state }) => {
@@ -249,7 +239,9 @@ describe('diffs/components/app', () => {
});
it('sets width of tree list', () => {
- createComponent();
+ createComponent({}, ({ state }) => {
+ state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }];
+ });
expect(wrapper.find('.js-diff-tree-list').element.style.width).toEqual('320px');
});
@@ -283,15 +275,6 @@ describe('diffs/components/app', () => {
expect(wrapper.find(NoChanges).exists()).toBe(false);
expect(wrapper.findAll(DiffFile).length).toBe(1);
});
-
- it('does not render empty state when versions match', () => {
- createComponent({}, ({ state }) => {
- state.diffs.startVersion = mergeRequestDiff;
- state.diffs.mergeRequestDiff = mergeRequestDiff;
- });
-
- expect(wrapper.find(NoChanges).exists()).toBe(false);
- });
});
describe('keyboard shortcut navigation', () => {
@@ -517,6 +500,7 @@ describe('diffs/components/app', () => {
describe('diffs', () => {
it('should render compare versions component', () => {
createComponent({}, ({ state }) => {
+ state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }];
state.diffs.mergeRequestDiffs = diffsMockData;
state.diffs.targetBranchName = 'target-branch';
state.diffs.mergeRequestDiff = mergeRequestDiff;
@@ -525,7 +509,8 @@ describe('diffs/components/app', () => {
expect(wrapper.find(CompareVersions).exists()).toBe(true);
expect(wrapper.find(CompareVersions).props()).toEqual(
expect.objectContaining({
- mergeRequestDiffs: diffsMockData,
+ isLimitedContainer: false,
+ diffFilesCountText: null,
}),
);
});
@@ -593,9 +578,17 @@ describe('diffs/components/app', () => {
expect(wrapper.find(DiffFile).exists()).toBe(true);
});
- it('should render tree list', () => {
+ it("doesn't render tree list when no changes exist", () => {
createComponent();
+ expect(wrapper.find(TreeList).exists()).toBe(false);
+ });
+
+ it('should render tree list', () => {
+ createComponent({}, ({ state }) => {
+ state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }];
+ });
+
expect(wrapper.find(TreeList).exists()).toBe(true);
});
});
diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js
index cd7697676c44f544..ae4e043e859f729f 100644
--- a/spec/frontend/diffs/components/compare_versions_spec.js
+++ b/spec/frontend/diffs/components/compare_versions_spec.js
@@ -11,32 +11,34 @@ localVue.use(Vuex);
describe('CompareVersions', () => {
let wrapper;
+ let store;
const targetBranchName = 'tmp-wine-dev';
- const createWrapper = (props) => {
- const store = createStore();
- const mergeRequestDiff = diffsMockData[0];
-
- store.state.diffs.addedLines = 10;
- store.state.diffs.removedLines = 20;
- store.state.diffs.diffFiles.push('test');
- store.state.diffs.targetBranchName = targetBranchName;
- store.state.diffs.mergeRequestDiff = mergeRequestDiff;
- store.state.diffs.mergeRequestDiffs = diffsMockData;
-
+ const createWrapper = props => {
wrapper = mount(CompareVersionsComponent, {
localVue,
store,
propsData: {
mergeRequestDiffs: diffsMockData,
- diffFilesCountText: null,
+ diffFilesCountText: '1',
...props,
},
});
};
+ const findLimitedContainer = () => wrapper.find('.container-limited.limit-container-width');
+ const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown');
+ const findCompareTargetDropdown = () => wrapper.find('.mr-version-compare-dropdown');
beforeEach(() => {
- createWrapper();
+ store = createStore();
+ const mergeRequestDiff = diffsMockData[0];
+
+ store.state.diffs.addedLines = 10;
+ store.state.diffs.removedLines = 20;
+ store.state.diffs.diffFiles.push('test');
+ store.state.diffs.targetBranchName = targetBranchName;
+ store.state.diffs.mergeRequestDiff = mergeRequestDiff;
+ store.state.diffs.mergeRequestDiffs = diffsMockData;
});
afterEach(() => {
@@ -45,6 +47,10 @@ describe('CompareVersions', () => {
});
describe('template', () => {
+ beforeEach(() => {
+ createWrapper();
+ });
+
it('should render Tree List toggle button with correct attribute values', () => {
const treeListBtn = wrapper.find('.js-toggle-tree-list');
@@ -54,8 +60,8 @@ describe('CompareVersions', () => {
});
it('should render comparison dropdowns with correct values', () => {
- const sourceDropdown = wrapper.find('.mr-version-dropdown');
- const targetDropdown = wrapper.find('.mr-version-compare-dropdown');
+ const sourceDropdown = findCompareSourceDropdown();
+ const targetDropdown = findCompareTargetDropdown();
expect(sourceDropdown.exists()).toBe(true);
expect(targetDropdown.exists()).toBe(true);
@@ -63,16 +69,6 @@ describe('CompareVersions', () => {
expect(targetDropdown.find('button').html()).toContain(targetBranchName);
});
- it('should not render comparison dropdowns if no mergeRequestDiffs are specified', () => {
- createWrapper({ mergeRequestDiffs: [] });
-
- const sourceDropdown = wrapper.find('.mr-version-dropdown');
- const targetDropdown = wrapper.find('.mr-version-compare-dropdown');
-
- expect(sourceDropdown.exists()).toBe(false);
- expect(targetDropdown.exists()).toBe(false);
- });
-
it('should render view types buttons with correct values', () => {
const inlineBtn = wrapper.find('#inline-diff-btn');
const parallelBtn = wrapper.find('#parallel-diff-btn');
@@ -88,22 +84,34 @@ describe('CompareVersions', () => {
it('adds container-limiting classes when showFileTree is false with inline diffs', () => {
createWrapper({ isLimitedContainer: true });
- const limitedContainer = wrapper.find('.container-limited.limit-container-width');
-
- expect(limitedContainer.exists()).toBe(true);
+ expect(findLimitedContainer().exists()).toBe(true);
});
it('does not add container-limiting classes when showFileTree is false with inline diffs', () => {
createWrapper({ isLimitedContainer: false });
- const limitedContainer = wrapper.find('.container-limited.limit-container-width');
+ expect(findLimitedContainer().exists()).toBe(false);
+ });
+ });
- expect(limitedContainer.exists()).toBe(false);
+ describe('noChangedFiles', () => {
+ beforeEach(() => {
+ store.state.diffs.diffFiles = [];
+ });
+
+ it('should not render Tree List toggle button when there are no changes', () => {
+ createWrapper();
+
+ const treeListBtn = wrapper.find('.js-toggle-tree-list');
+
+ expect(treeListBtn.exists()).toBe(false);
});
});
describe('setInlineDiffViewType', () => {
it('should persist the view type in the url', () => {
+ createWrapper();
+
const viewTypeBtn = wrapper.find('#inline-diff-btn');
viewTypeBtn.trigger('click');
@@ -113,6 +121,7 @@ describe('CompareVersions', () => {
describe('setParallelDiffViewType', () => {
it('should persist the view type in the url', () => {
+ createWrapper();
const viewTypeBtn = wrapper.find('#parallel-diff-btn');
viewTypeBtn.trigger('click');
@@ -121,11 +130,14 @@ describe('CompareVersions', () => {
});
describe('commit', () => {
- beforeEach((done) => {
- wrapper.vm.$store.state.diffs.commit = getDiffWithCommit().commit;
- wrapper.mergeRequestDiffs = [];
+ beforeEach(() => {
+ store.state.diffs.commit = getDiffWithCommit().commit;
+ createWrapper();
+ });
- wrapper.vm.$nextTick(done);
+ it('does not render compare dropdowns', () => {
+ expect(findCompareSourceDropdown().exists()).toBe(false);
+ expect(findCompareTargetDropdown().exists()).toBe(false);
});
it('renders latest version button', () => {
@@ -137,4 +149,16 @@ describe('CompareVersions', () => {
expect(wrapper.text()).toContain(wrapper.vm.commit.short_id);
});
});
+
+ describe('with no versions', () => {
+ beforeEach(() => {
+ store.state.diffs.mergeRequestDiffs = [];
+ createWrapper();
+ });
+
+ it('does not render compare dropdowns', () => {
+ expect(findCompareSourceDropdown().exists()).toBe(false);
+ expect(findCompareTargetDropdown().exists()).toBe(false);
+ });
+ });
});
diff --git a/spec/frontend/diffs/components/no_changes_spec.js b/spec/frontend/diffs/components/no_changes_spec.js
index 5d00a04d41512c7a..df9af51f9cf4cf73 100644
--- a/spec/frontend/diffs/components/no_changes_spec.js
+++ b/spec/frontend/diffs/components/no_changes_spec.js
@@ -1,20 +1,22 @@
-import { createLocalVue, shallowMount } from '@vue/test-utils';
+import { createLocalVue, shallowMount, mount } from '@vue/test-utils';
import Vuex from 'vuex';
import { GlButton } from '@gitlab/ui';
import { createStore } from '~/mr_notes/stores';
import NoChanges from '~/diffs/components/no_changes.vue';
+import diffsMockData from '../mock_data/merge_request_diffs';
-describe('Diff no changes empty state', () => {
- let vm;
+const localVue = createLocalVue();
+localVue.use(Vuex);
- function createComponent(extendStore = () => {}) {
- const localVue = createLocalVue();
- localVue.use(Vuex);
+const TEST_TARGET_BRANCH = 'foo';
+const TEST_SOURCE_BRANCH = 'dev/update';
- const store = createStore();
- extendStore(store);
+describe('Diff no changes empty state', () => {
+ let wrapper;
+ let store;
- vm = shallowMount(NoChanges, {
+ function createComponent(mountFn = shallowMount) {
+ wrapper = mountFn(NoChanges, {
localVue,
store,
propsData: {
@@ -23,26 +25,61 @@ describe('Diff no changes empty state', () => {
});
}
+ beforeEach(() => {
+ store = createStore();
+ store.state.diffs.mergeRequestDiff = {};
+ store.state.notes.noteableData = {
+ target_branch: TEST_TARGET_BRANCH,
+ source_branch: TEST_SOURCE_BRANCH,
+ };
+ store.state.diffs.mergeRequestDiffs = diffsMockData;
+ });
+
afterEach(() => {
- vm.destroy();
+ wrapper.destroy();
+ wrapper = null;
});
+ const findMessage = () => wrapper.find('[data-testid="no-changes-message"]');
+
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>',
- };
- });
+ store.state.notes.noteableData = {
+ source_branch: '<script>alert("test");</script>',
+ target_branch: '<script>alert("test");</script>',
+ };
- expect(vm.find('script').exists()).toBe(false);
+ createComponent();
+
+ expect(wrapper.find('script').exists()).toBe(false);
});
describe('Renders', () => {
it('Show create commit button', () => {
createComponent();
- expect(vm.find(GlButton).exists()).toBe(true);
+
+ expect(wrapper.find(GlButton).exists()).toBe(true);
});
+
+ it.each`
+ expectedText | sourceIndex | targetIndex
+ ${`No changes between ${TEST_SOURCE_BRANCH} and ${TEST_TARGET_BRANCH}`} | ${null} | ${null}
+ ${`No changes between ${TEST_SOURCE_BRANCH} and version 1`} | ${diffsMockData[0].version_index} | ${1}
+ ${`No changes between version 3 and version 2`} | ${3} | ${2}
+ ${`No changes between version 3 and ${TEST_TARGET_BRANCH}`} | ${3} | ${-1}
+ `(
+ 'renders text "$expectedText" (sourceIndex=$sourceIndex and targetIndex=$targetIndex)',
+ ({ expectedText, targetIndex, sourceIndex }) => {
+ if (targetIndex !== null) {
+ store.state.diffs.startVersion = { version_index: targetIndex };
+ }
+ if (sourceIndex !== null) {
+ store.state.diffs.mergeRequestDiff.version_index = sourceIndex;
+ }
+
+ createComponent(mount);
+
+ expect(findMessage().text()).toBe(expectedText);
+ },
+ );
});
});
diff --git a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
index 19f3d28e45ec9c3d..f795451542287ae8 100644
--- a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
+++ b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
@@ -136,6 +136,7 @@ describe('Compare diff version dropdowns', () => {
...firstDiff,
href: firstDiff.version_path,
commitsText: `${firstDiff.commits_count} commits,`,
+ isLatestVersion: true,
versionName: 'latest version',
selected: true,
};
@@ -144,6 +145,9 @@ describe('Compare diff version dropdowns', () => {
selectedSourceIndex: expectedShape.version_index,
});
expect(sourceVersions[0]).toEqual(expectedShape);
- expect(sourceVersions[1].selected).toBe(false);
+ expect(sourceVersions[1]).toMatchObject({
+ selected: false,
+ isLatestVersion: false,
+ });
});
});
--
GitLab
From 8b753baad7ea1777eefe7c57d6f99d22b9dd8fdc Mon Sep 17 00:00:00 2001
From: Marcel van Remmerden <mvanremmerden@gitlab.com>
Date: Mon, 4 Jan 2021 13:48:50 +0100
Subject: [PATCH 2/4] Prettify vue and js files
---
.../javascripts/diffs/components/app.vue | 34 ++++++++++++-------
.../diffs/components/no_changes.vue | 4 +--
.../diffs/components/compare_versions_spec.js | 2 +-
3 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 4e0adaaf0a1711d0..cc5598f5e218dbe3 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -141,17 +141,17 @@ export default {
},
computed: {
...mapState({
- isLoading: state => state.diffs.isLoading,
- isBatchLoading: state => state.diffs.isBatchLoading,
- diffFiles: state => state.diffs.diffFiles,
- diffViewType: state => state.diffs.diffViewType,
- commit: state => state.diffs.commit,
- renderOverflowWarning: state => state.diffs.renderOverflowWarning,
- numTotalFiles: state => state.diffs.realSize,
- numVisibleFiles: state => state.diffs.size,
- plainDiffPath: state => state.diffs.plainDiffPath,
- emailPatchPath: state => state.diffs.emailPatchPath,
- retrievingBatches: state => state.diffs.retrievingBatches,
+ isLoading: (state) => state.diffs.isLoading,
+ isBatchLoading: (state) => state.diffs.isBatchLoading,
+ diffFiles: (state) => state.diffs.diffFiles,
+ diffViewType: (state) => state.diffs.diffViewType,
+ commit: (state) => state.diffs.commit,
+ renderOverflowWarning: (state) => state.diffs.renderOverflowWarning,
+ numTotalFiles: (state) => state.diffs.realSize,
+ numVisibleFiles: (state) => state.diffs.size,
+ plainDiffPath: (state) => state.diffs.plainDiffPath,
+ emailPatchPath: (state) => state.diffs.emailPatchPath,
+ retrievingBatches: (state) => state.diffs.retrievingBatches,
}),
...mapState('diffs', [
'showTreeList',
@@ -278,7 +278,12 @@ export default {
const id = window?.location?.hash;
if (id && id.indexOf('#note') !== 0) {
- this.setHighlightedRow(id.split('diff-content').pop().slice(1));
+ this.setHighlightedRow(
+ id
+ .split('diff-content')
+ .pop()
+ .slice(1),
+ );
}
},
beforeCreate() {
@@ -396,7 +401,10 @@ export default {
},
setDiscussions() {
requestIdleCallback(
- () => this.assignDiscussionsToDiff().then(this.$nextTick).then(this.startTaskList),
+ () =>
+ this.assignDiscussionsToDiff()
+ .then(this.$nextTick)
+ .then(this.startTaskList),
{ timeout: 1000 },
);
},
diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue
index 9da13548809dfbf3..e0fdbf6ac3ad3110 100644
--- a/app/assets/javascripts/diffs/components/no_changes.vue
+++ b/app/assets/javascripts/diffs/components/no_changes.vue
@@ -20,7 +20,7 @@ export default {
]),
...mapGetters(['getNoteableData']),
selectedSourceVersion() {
- return this.diffCompareDropdownSourceVersions.find(x => x.selected);
+ return this.diffCompareDropdownSourceVersions.find((x) => x.selected);
},
sourceName() {
if (!this.selectedSourceVersion || this.selectedSourceVersion.isLatestVersion) {
@@ -30,7 +30,7 @@ export default {
return this.selectedSourceVersion.versionName;
},
selectedTargetVersion() {
- return this.diffCompareDropdownTargetVersions.find(x => x.selected);
+ return this.diffCompareDropdownTargetVersions.find((x) => x.selected);
},
targetName() {
if (!this.selectedTargetVersion || this.selectedTargetVersion.version_index < 0) {
diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js
index ae4e043e859f729f..949cc855200c7c3e 100644
--- a/spec/frontend/diffs/components/compare_versions_spec.js
+++ b/spec/frontend/diffs/components/compare_versions_spec.js
@@ -14,7 +14,7 @@ describe('CompareVersions', () => {
let store;
const targetBranchName = 'tmp-wine-dev';
- const createWrapper = props => {
+ const createWrapper = (props) => {
wrapper = mount(CompareVersionsComponent, {
localVue,
store,
--
GitLab
From dc9e59aa81eff717a7fc350a562bc6bb10001b0c Mon Sep 17 00:00:00 2001
From: Paul Slaughter <pslaughter@gitlab.com>
Date: Mon, 4 Jan 2021 15:53:40 -0600
Subject: [PATCH 3/4] Update with Prettier appeasement
---
app/assets/javascripts/diffs/components/app.vue | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index cc5598f5e218dbe3..fd331e11a124820e 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -278,12 +278,7 @@ export default {
const id = window?.location?.hash;
if (id && id.indexOf('#note') !== 0) {
- this.setHighlightedRow(
- id
- .split('diff-content')
- .pop()
- .slice(1),
- );
+ this.setHighlightedRow(id.split('diff-content').pop().slice(1));
}
},
beforeCreate() {
@@ -401,10 +396,7 @@ export default {
},
setDiscussions() {
requestIdleCallback(
- () =>
- this.assignDiscussionsToDiff()
- .then(this.$nextTick)
- .then(this.startTaskList),
+ () => this.assignDiscussionsToDiff().then(this.$nextTick).then(this.startTaskList),
{ timeout: 1000 },
);
},
--
GitLab
From 3467ecc3bc959eb31d14f846113db9d4b7746fd2 Mon Sep 17 00:00:00 2001
From: Paul Slaughter <pslaughter@gitlab.com>
Date: Mon, 4 Jan 2021 16:39:25 -0600
Subject: [PATCH 4/4] Remove empty line in changelog
---
...nges-tab-on-merge-requests-should-not-show-file-browser-a.yml | 1 -
1 file changed, 1 deletion(-)
diff --git a/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml b/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
index d7972982989fd657..6d721b9cbaaa887f 100644
--- a/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
+++ b/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
@@ -1,6 +1,5 @@
---
title: Remove diff display preferences and file tree from changes empty state
merge_request: 43467
-
author:
type: fixed
--
GitLab