Commit b98072f4 authored by Martin Wortschack's avatar Martin Wortschack

Merge branch 'jdb-batch-diffs-fe' into 'master'

FE Support batch diff loading

See merge request gitlab-org/gitlab!18544
parents cd4e1d52 a1b6a29f
<script> <script>
import { mapState, mapGetters, mapActions } from 'vuex'; import { mapState, mapGetters, mapActions } from 'vuex';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import { __ } from '~/locale'; import { __ } from '~/locale';
import createFlash from '~/flash'; import createFlash from '~/flash';
...@@ -36,11 +37,20 @@ export default { ...@@ -36,11 +37,20 @@ export default {
GlLoadingIcon, GlLoadingIcon,
PanelResizer, PanelResizer,
}, },
mixins: [glFeatureFlagsMixin()],
props: { props: {
endpoint: { endpoint: {
type: String, type: String,
required: true, required: true,
}, },
endpointMetadata: {
type: String,
required: true,
},
endpointBatch: {
type: String,
required: true,
},
projectPath: { projectPath: {
type: String, type: String,
required: true, required: true,
...@@ -92,6 +102,7 @@ export default { ...@@ -92,6 +102,7 @@ export default {
computed: { computed: {
...mapState({ ...mapState({
isLoading: state => state.diffs.isLoading, isLoading: state => state.diffs.isLoading,
isBatchLoading: state => state.diffs.isBatchLoading,
diffFiles: state => state.diffs.diffFiles, diffFiles: state => state.diffs.diffFiles,
diffViewType: state => state.diffs.diffViewType, diffViewType: state => state.diffs.diffViewType,
mergeRequestDiffs: state => state.diffs.mergeRequestDiffs, mergeRequestDiffs: state => state.diffs.mergeRequestDiffs,
...@@ -153,6 +164,8 @@ export default { ...@@ -153,6 +164,8 @@ export default {
mounted() { mounted() {
this.setBaseConfig({ this.setBaseConfig({
endpoint: this.endpoint, endpoint: this.endpoint,
endpointMetadata: this.endpointMetadata,
endpointBatch: this.endpointBatch,
projectPath: this.projectPath, projectPath: this.projectPath,
dismissEndpoint: this.dismissEndpoint, dismissEndpoint: this.dismissEndpoint,
showSuggestPopover: this.showSuggestPopover, showSuggestPopover: this.showSuggestPopover,
...@@ -185,6 +198,8 @@ export default { ...@@ -185,6 +198,8 @@ export default {
...mapActions('diffs', [ ...mapActions('diffs', [
'setBaseConfig', 'setBaseConfig',
'fetchDiffFiles', 'fetchDiffFiles',
'fetchDiffFilesMeta',
'fetchDiffFilesBatch',
'startRenderDiffsQueue', 'startRenderDiffsQueue',
'assignDiscussionsToDiff', 'assignDiscussionsToDiff',
'setHighlightedRow', 'setHighlightedRow',
...@@ -196,24 +211,51 @@ export default { ...@@ -196,24 +211,51 @@ export default {
this.assignedDiscussions = false; this.assignedDiscussions = false;
this.fetchData(false); this.fetchData(false);
}, },
isLatestVersion() {
return window.location.search.indexOf('diff_id') === -1;
},
fetchData(toggleTree = true) { fetchData(toggleTree = true) {
this.fetchDiffFiles() if (this.isLatestVersion() && this.glFeatures.diffsBatchLoad) {
.then(() => { this.fetchDiffFilesMeta()
if (toggleTree) { .then(() => {
this.hideTreeListIfJustOneFile(); if (toggleTree) this.hideTreeListIfJustOneFile();
} })
.catch(() => {
createFlash(__('Something went wrong on our end. Please try again!'));
});
requestIdleCallback( this.fetchDiffFilesBatch()
() => { .then(() => {
this.setDiscussions(); requestIdleCallback(
this.startRenderDiffsQueue(); () => {
}, this.setDiscussions();
{ timeout: 1000 }, this.startRenderDiffsQueue();
); },
}) { timeout: 1000 },
.catch(() => { );
createFlash(__('Something went wrong on our end. Please try again!')); })
}); .catch(() => {
createFlash(__('Something went wrong on our end. Please try again!'));
});
} else {
this.fetchDiffFiles()
.then(() => {
if (toggleTree) {
this.hideTreeListIfJustOneFile();
}
requestIdleCallback(
() => {
this.setDiscussions();
this.startRenderDiffsQueue();
},
{ timeout: 1000 },
);
})
.catch(() => {
createFlash(__('Something went wrong on our end. Please try again!'));
});
}
if (!this.isNotesFetched) { if (!this.isNotesFetched) {
eventHub.$emit('fetchNotesData'); eventHub.$emit('fetchNotesData');
...@@ -324,7 +366,8 @@ export default { ...@@ -324,7 +366,8 @@ export default {
}" }"
> >
<commit-widget v-if="commit" :commit="commit" /> <commit-widget v-if="commit" :commit="commit" />
<template v-if="renderDiffFiles"> <div v-if="isBatchLoading" class="loading"><gl-loading-icon /></div>
<template v-else-if="renderDiffFiles">
<diff-file <diff-file
v-for="file in diffFiles" v-for="file in diffFiles"
:key="file.newPath" :key="file.newPath"
......
...@@ -57,3 +57,4 @@ export const MIN_RENDERING_MS = 2; ...@@ -57,3 +57,4 @@ export const MIN_RENDERING_MS = 2;
export const START_RENDERING_INDEX = 200; export const START_RENDERING_INDEX = 200;
export const INLINE_DIFF_LINES_KEY = 'highlighted_diff_lines'; export const INLINE_DIFF_LINES_KEY = 'highlighted_diff_lines';
export const PARALLEL_DIFF_LINES_KEY = 'parallel_diff_lines'; export const PARALLEL_DIFF_LINES_KEY = 'parallel_diff_lines';
export const DIFFS_PER_PAGE = 10;
...@@ -67,6 +67,8 @@ export default function initDiffsApp(store) { ...@@ -67,6 +67,8 @@ export default function initDiffsApp(store) {
return { return {
endpoint: dataset.endpoint, endpoint: dataset.endpoint,
endpointMetadata: dataset.endpointMetadata || '',
endpointBatch: dataset.endpointBatch || '',
projectPath: dataset.projectPath, projectPath: dataset.projectPath,
helpPagePath: dataset.helpPagePath, helpPagePath: dataset.helpPagePath,
currentUser: JSON.parse(dataset.currentUserData) || {}, currentUser: JSON.parse(dataset.currentUserData) || {},
...@@ -100,6 +102,8 @@ export default function initDiffsApp(store) { ...@@ -100,6 +102,8 @@ export default function initDiffsApp(store) {
return createElement('diffs-app', { return createElement('diffs-app', {
props: { props: {
endpoint: this.endpoint, endpoint: this.endpoint,
endpointMetadata: this.endpointMetadata,
endpointBatch: this.endpointBatch,
currentUser: this.currentUser, currentUser: this.currentUser,
projectPath: this.projectPath, projectPath: this.projectPath,
helpPagePath: this.helpPagePath, helpPagePath: this.helpPagePath,
......
...@@ -13,6 +13,7 @@ import { ...@@ -13,6 +13,7 @@ import {
convertExpandLines, convertExpandLines,
idleCallback, idleCallback,
allDiscussionWrappersExpanded, allDiscussionWrappersExpanded,
prepareDiffData,
} from './utils'; } from './utils';
import * as types from './mutation_types'; import * as types from './mutation_types';
import { import {
...@@ -33,12 +34,27 @@ import { ...@@ -33,12 +34,27 @@ import {
START_RENDERING_INDEX, START_RENDERING_INDEX,
INLINE_DIFF_LINES_KEY, INLINE_DIFF_LINES_KEY,
PARALLEL_DIFF_LINES_KEY, PARALLEL_DIFF_LINES_KEY,
DIFFS_PER_PAGE,
} from '../constants'; } from '../constants';
import { diffViewerModes } from '~/ide/constants'; import { diffViewerModes } from '~/ide/constants';
export const setBaseConfig = ({ commit }, options) => { export const setBaseConfig = ({ commit }, options) => {
const { endpoint, projectPath, dismissEndpoint, showSuggestPopover } = options; const {
commit(types.SET_BASE_CONFIG, { endpoint, projectPath, dismissEndpoint, showSuggestPopover }); endpoint,
endpointMetadata,
endpointBatch,
projectPath,
dismissEndpoint,
showSuggestPopover,
} = options;
commit(types.SET_BASE_CONFIG, {
endpoint,
endpointMetadata,
endpointBatch,
projectPath,
dismissEndpoint,
showSuggestPopover,
});
}; };
export const fetchDiffFiles = ({ state, commit }) => { export const fetchDiffFiles = ({ state, commit }) => {
...@@ -67,6 +83,53 @@ export const fetchDiffFiles = ({ state, commit }) => { ...@@ -67,6 +83,53 @@ export const fetchDiffFiles = ({ state, commit }) => {
.catch(() => worker.terminate()); .catch(() => worker.terminate());
}; };
export const fetchDiffFilesBatch = ({ commit, state }) => {
const baseUrl = `${state.endpointBatch}?per_page=${DIFFS_PER_PAGE}`;
const url = page => (page ? `${baseUrl}&page=${page}` : baseUrl);
commit(types.SET_BATCH_LOADING, true);
const getBatch = page =>
axios
.get(url(page))
.then(({ data: { pagination, diff_files } }) => {
commit(types.SET_DIFF_DATA_BATCH, { diff_files });
commit(types.SET_BATCH_LOADING, false);
return pagination.next_page;
})
.then(nextPage => nextPage && getBatch(nextPage));
return getBatch()
.then(handleLocationHash)
.catch(() => null);
};
export const fetchDiffFilesMeta = ({ commit, state }) => {
const worker = new TreeWorker();
commit(types.SET_LOADING, true);
worker.addEventListener('message', ({ data }) => {
commit(types.SET_TREE_DATA, data);
worker.terminate();
});
return axios
.get(state.endpointMetadata)
.then(({ data }) => {
const strippedData = { ...data };
strippedData.diff_files = [];
commit(types.SET_LOADING, false);
commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []);
commit(types.SET_DIFF_DATA, strippedData);
prepareDiffData(data);
worker.postMessage(data.diff_files);
})
.catch(() => worker.terminate());
};
export const setHighlightedRow = ({ commit }, lineCode) => { export const setHighlightedRow = ({ commit }, lineCode) => {
const fileHash = lineCode.split('_')[0]; const fileHash = lineCode.split('_')[0];
commit(types.SET_HIGHLIGHTED_ROW, lineCode); commit(types.SET_HIGHLIGHTED_ROW, lineCode);
......
...@@ -8,6 +8,7 @@ const defaultViewType = INLINE_DIFF_VIEW_TYPE; ...@@ -8,6 +8,7 @@ const defaultViewType = INLINE_DIFF_VIEW_TYPE;
export default () => ({ export default () => ({
isLoading: true, isLoading: true,
isBatchLoading: false,
addedLines: null, addedLines: null,
removedLines: null, removedLines: null,
endpoint: '', endpoint: '',
......
export const SET_BASE_CONFIG = 'SET_BASE_CONFIG'; export const SET_BASE_CONFIG = 'SET_BASE_CONFIG';
export const SET_LOADING = 'SET_LOADING'; export const SET_LOADING = 'SET_LOADING';
export const SET_BATCH_LOADING = 'SET_BATCH_LOADING';
export const SET_DIFF_DATA = 'SET_DIFF_DATA'; export const SET_DIFF_DATA = 'SET_DIFF_DATA';
export const SET_DIFF_DATA_BATCH = 'SET_DIFF_DATA_BATCH';
export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE'; export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE';
export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS'; export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS';
export const TOGGLE_LINE_HAS_FORM = 'TOGGLE_LINE_HAS_FORM'; export const TOGGLE_LINE_HAS_FORM = 'TOGGLE_LINE_HAS_FORM';
......
...@@ -12,14 +12,32 @@ import * as types from './mutation_types'; ...@@ -12,14 +12,32 @@ import * as types from './mutation_types';
export default { export default {
[types.SET_BASE_CONFIG](state, options) { [types.SET_BASE_CONFIG](state, options) {
const { endpoint, projectPath, dismissEndpoint, showSuggestPopover } = options; const {
Object.assign(state, { endpoint, projectPath, dismissEndpoint, showSuggestPopover }); endpoint,
endpointMetadata,
endpointBatch,
projectPath,
dismissEndpoint,
showSuggestPopover,
} = options;
Object.assign(state, {
endpoint,
endpointMetadata,
endpointBatch,
projectPath,
dismissEndpoint,
showSuggestPopover,
});
}, },
[types.SET_LOADING](state, isLoading) { [types.SET_LOADING](state, isLoading) {
Object.assign(state, { isLoading }); Object.assign(state, { isLoading });
}, },
[types.SET_BATCH_LOADING](state, isBatchLoading) {
Object.assign(state, { isBatchLoading });
},
[types.SET_DIFF_DATA](state, data) { [types.SET_DIFF_DATA](state, data) {
prepareDiffData(data); prepareDiffData(data);
...@@ -28,6 +46,12 @@ export default { ...@@ -28,6 +46,12 @@ export default {
}); });
}, },
[types.SET_DIFF_DATA_BATCH](state, data) {
prepareDiffData(data);
state.diffFiles.push(...data.diff_files);
},
[types.RENDER_FILE](state, file) { [types.RENDER_FILE](state, file) {
Object.assign(file, { Object.assign(file, {
renderIt: true, renderIt: true,
......
...@@ -252,10 +252,11 @@ export function prepareDiffData(diffData) { ...@@ -252,10 +252,11 @@ export function prepareDiffData(diffData) {
showingLines += file.parallel_diff_lines.length; showingLines += file.parallel_diff_lines.length;
} }
const name = (file.viewer && file.viewer.name) || diffViewerModes.text;
Object.assign(file, { Object.assign(file, {
renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
collapsed: collapsed: name === diffViewerModes.text && showingLines > MAX_LINES_TO_BE_RENDERED,
file.viewer.name === diffViewerModes.text && showingLines > MAX_LINES_TO_BE_RENDERED,
isShowingFullFile: false, isShowingFullFile: false,
isLoadingFullFile: false, isLoadingFullFile: false,
discussions: [], discussions: [],
......
...@@ -7,4 +7,7 @@ class DiffFileMetadataEntity < Grape::Entity ...@@ -7,4 +7,7 @@ class DiffFileMetadataEntity < Grape::Entity
expose :old_path expose :old_path
expose :new_file?, as: :new_file expose :new_file?, as: :new_file
expose :deleted_file?, as: :deleted_file expose :deleted_file?, as: :deleted_file
expose :file_hash do |diff_file|
Digest::SHA1.hexdigest(diff_file.file_path)
end
end end
...@@ -78,6 +78,8 @@ ...@@ -78,6 +78,8 @@
= render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request) = render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request)
#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),
endpoint_metadata: diffs_metadata_project_json_merge_request_path(@project, @merge_request, 'json', request.query_parameters),
endpoint_batch: diffs_batch_project_json_merge_request_path(@project, @merge_request, 'json', request.query_parameters),
help_page_path: suggest_changes_help_path, help_page_path: suggest_changes_help_path,
current_user_data: @current_user_data, current_user_data: @current_user_data,
project_path: project_path(@merge_request.project), project_path: project_path(@merge_request.project),
......
...@@ -21,6 +21,7 @@ describe 'Merge request > Batch comments', :js do ...@@ -21,6 +21,7 @@ describe 'Merge request > Batch comments', :js do
context 'Feature is disabled' do context 'Feature is disabled' do
before do before do
stub_feature_flags(batch_comments: false) stub_feature_flags(batch_comments: false)
stub_feature_flags(diffs_batch_load: false)
visit_diffs visit_diffs
end end
...@@ -35,6 +36,7 @@ describe 'Merge request > Batch comments', :js do ...@@ -35,6 +36,7 @@ describe 'Merge request > Batch comments', :js do
context 'Feature is enabled' do context 'Feature is enabled' do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
stub_licensed_features(batch_comments: true) stub_licensed_features(batch_comments: true)
visit_diffs visit_diffs
......
...@@ -8,6 +8,7 @@ describe 'User expands diff', :js do ...@@ -8,6 +8,7 @@ describe 'User expands diff', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes) allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes)
allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(10.kilobytes) allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(10.kilobytes)
...@@ -20,7 +21,7 @@ describe 'User expands diff', :js do ...@@ -20,7 +21,7 @@ describe 'User expands diff', :js do
it_behaves_like 'rendering a single diff version' it_behaves_like 'rendering a single diff version'
it 'allows user to expand diff' do it 'allows user to expand diff' do
page.within find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do page.within find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9"]') do
click_link 'Click to expand it.' click_link 'Click to expand it.'
wait_for_requests wait_for_requests
......
...@@ -21,6 +21,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do ...@@ -21,6 +21,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
end end
it_behaves_like 'rendering a single diff version' it_behaves_like 'rendering a single diff version'
......
...@@ -22,6 +22,7 @@ describe 'Merge request > User sees avatars on diff notes', :js do ...@@ -22,6 +22,7 @@ describe 'Merge request > User sees avatars on diff notes', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
project.add_maintainer(user) project.add_maintainer(user)
sign_in user sign_in user
......
...@@ -11,6 +11,7 @@ describe 'Merge request > User sees diff', :js do ...@@ -11,6 +11,7 @@ describe 'Merge request > User sees diff', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
end end
it_behaves_like 'rendering a single diff version' it_behaves_like 'rendering a single diff version'
......
...@@ -17,6 +17,7 @@ describe 'Merge request > User sees versions', :js do ...@@ -17,6 +17,7 @@ describe 'Merge request > User sees versions', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
......
...@@ -10,6 +10,7 @@ describe 'User views diffs', :js do ...@@ -10,6 +10,7 @@ describe 'User views diffs', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
visit(diffs_project_merge_request_path(project, merge_request)) visit(diffs_project_merge_request_path(project, merge_request))
wait_for_requests wait_for_requests
......
...@@ -10,6 +10,7 @@ describe 'View on environment', :js do ...@@ -10,6 +10,7 @@ describe 'View on environment', :js do
before do before do
stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(single_mr_diff_view: false)
stub_feature_flags(diffs_batch_load: false)
project.add_maintainer(user) project.add_maintainer(user)
end end
......
...@@ -34,6 +34,8 @@ describe('diffs/components/app', () => { ...@@ -34,6 +34,8 @@ describe('diffs/components/app', () => {
localVue, localVue,
propsData: { propsData: {
endpoint: `${TEST_HOST}/diff/endpoint`, endpoint: `${TEST_HOST}/diff/endpoint`,
endpointMetadata: `${TEST_HOST}/diff/endpointMetadata`,
endpointBatch: `${TEST_HOST}/diff/endpointBatch`,
projectPath: 'namespace/project', projectPath: 'namespace/project',
currentUser: {}, currentUser: {},
changesEmptyStateIllustration: '', changesEmptyStateIllustration: '',
...@@ -42,6 +44,11 @@ describe('diffs/components/app', () => { ...@@ -42,6 +44,11 @@ describe('diffs/components/app', () => {
...props, ...props,
}, },
store, store,
methods: {
isLatestVersion() {
return true;
},
},
}); });
} }
...@@ -59,6 +66,58 @@ describe('diffs/components/app', () => { ...@@ -59,6 +66,58 @@ describe('diffs/components/app', () => {
wrapper.destroy(); wrapper.destroy();
}); });
describe('fetch diff methods', () => {
beforeEach(() => {
spyOn(window, 'requestIdleCallback').and.callFake(fn => fn());
createComponent();
spyOn(wrapper.vm, 'fetchDiffFiles').and.callFake(() => Promise.resolve());
spyOn(wrapper.vm, 'fetchDiffFilesMeta').and.callFake(() => Promise.resolve());
spyOn(wrapper.vm, 'fetchDiffFilesBatch').and.callFake(() => Promise.resolve());
spyOn(wrapper.vm, 'setDiscussions');
spyOn(wrapper.vm, 'startRenderDiffsQueue');
});
it('calls fetchDiffFiles if diffsBatchLoad is not enabled', () => {
wrapper.vm.glFeatures.diffsBatchLoad = false;
wrapper.vm.fetchData(false);
expect(wrapper.vm.fetchDiffFiles).toHaveBeenCalled();
wrapper.vm.$nextTick(() => {
expect(wrapper.vm.setDiscussions).toHaveBeenCalled();
expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
});
});
it('calls fetchDiffFiles if diffsBatchLoad is enabled, and not latest version', () => {
wrapper.vm.glFeatures.diffsBatchLoad = true;
wrapper.vm.isLatestVersion = () => false;
wrapper.vm.fetchData(false);
expect(wrapper.vm.fetchDiffFiles).toHaveBeenCalled();
wrapper.vm.$nextTick(() => {
expect(wrapper.vm.setDiscussions).toHaveBeenCalled();
expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
});
});
it('calls batch methods if diffsBatchLoad is enabled, and latest version', () => {
wrapper.vm.glFeatures.diffsBatchLoad = true;
wrapper.vm.fetchData(false);
expect(wrapper.vm.fetchDiffFiles).not.toHaveBeenCalled();
wrapper.vm.$nextTick(() => {
expect(wrapper.vm.setDiscussions).toHaveBeenCalled();
expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled();
expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled();
});
});
});
it('adds container-limiting classes when showFileTree is false with inline diffs', () => { it('adds container-limiting classes when showFileTree is false with inline diffs', () => {
createComponent({}, ({ state }) => { createComponent({}, ({ state }) => {
state.diffs.showTreeList = false; state.diffs.showTreeList = false;
...@@ -93,6 +152,14 @@ describe('diffs/components/app', () => { ...@@ -93,6 +152,14 @@ describe('diffs/components/app', () => {
expect(wrapper.contains(GlLoadingIcon)).toBe(true); expect(wrapper.contains(GlLoadingIcon)).toBe(true);
}); });
it('displays loading icon on batch loading', () => {
createComponent({}, ({ state }) => {
state.diffs.isBatchLoading = true;
});
expect(wrapper.contains(GlLoadingIcon)).toBe(true);
});
it('displays diffs container when not loading', () => { it('displays diffs container when not loading', () => {
createComponent(); createComponent();
......
...@@ -8,6 +8,8 @@ import { ...@@ -8,6 +8,8 @@ import {
import actions, { import actions, {
setBaseConfig, setBaseConfig,
fetchDiffFiles, fetchDiffFiles,
fetchDiffFilesBatch,
fetchDiffFilesMeta,
assignDiscussionsToDiff, assignDiscussionsToDiff,
removeDiscussionsFromDiff, removeDiscussionsFromDiff,
startRenderDiffsQueue, startRenderDiffsQueue,
...@@ -68,18 +70,41 @@ describe('DiffsStoreActions', () => { ...@@ -68,18 +70,41 @@ describe('DiffsStoreActions', () => {
describe('setBaseConfig', () => { describe('setBaseConfig', () => {
it('should set given endpoint and project path', done => { it('should set given endpoint and project path', done => {
const endpoint = '/diffs/set/endpoint'; const endpoint = '/diffs/set/endpoint';
const endpointMetadata = '/diffs/set/endpoint/metadata';
const endpointBatch = '/diffs/set/endpoint/batch';
const projectPath = '/root/project'; const projectPath = '/root/project';
const dismissEndpoint = '/-/user_callouts'; const dismissEndpoint = '/-/user_callouts';
const showSuggestPopover = false; const showSuggestPopover = false;
testAction( testAction(
setBaseConfig, setBaseConfig,
{ endpoint, projectPath, dismissEndpoint, showSuggestPopover }, {
{ endpoint: '', projectPath: '', dismissEndpoint: '', showSuggestPopover: true }, endpoint,
endpointBatch,
endpointMetadata,
projectPath,
dismissEndpoint,
showSuggestPopover,
},
{
endpoint: '',
endpointBatch: '',
endpointMetadata: '',
projectPath: '',
dismissEndpoint: '',
showSuggestPopover: true,
},
[ [
{ {
type: types.SET_BASE_CONFIG, type: types.SET_BASE_CONFIG,
payload: { endpoint, projectPath, dismissEndpoint, showSuggestPopover }, payload: {
endpoint,
endpointMetadata,
endpointBatch,
projectPath,
dismissEndpoint,
showSuggestPopover,
},
}, },
], ],
[], [],
...@@ -114,6 +139,64 @@ describe('DiffsStoreActions', () => { ...@@ -114,6 +139,64 @@ describe('DiffsStoreActions', () => {
}); });
}); });
describe('fetchDiffFilesBatch', () => {
it('should fetch batch diff files', done => {
const endpointBatch = '/fetch/diffs_batch';
const batch1 = `${endpointBatch}?per_page=10`;
const batch2 = `${endpointBatch}?per_page=10&page=2`;
const mock = new MockAdapter(axios);
const res1 = { diff_files: [], pagination: { next_page: 2 } };
const res2 = { diff_files: [], pagination: {} };
mock.onGet(batch1).reply(200, res1);
mock.onGet(batch2).reply(200, res2);
testAction(
fetchDiffFilesBatch,
{},
{ endpointBatch },
[
{ type: types.SET_BATCH_LOADING, payload: true },
{ type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } },
{ type: types.SET_BATCH_LOADING, payload: false },
{ type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: [] } },
{ type: types.SET_BATCH_LOADING, payload: false },
],
[],
() => {
mock.restore();
done();
},
);
});
});
describe('fetchDiffFilesMeta', () => {
it('should fetch diff meta information', done => {
const endpointMetadata = '/fetch/diffs_meta';
const mock = new MockAdapter(axios);
const data = { diff_files: [] };
const res = { data };
mock.onGet(endpointMetadata).reply(200, res);
testAction(
fetchDiffFilesMeta,
{},
{ endpointMetadata },
[
{ type: types.SET_LOADING, payload: true },
{ type: types.SET_LOADING, payload: false },
{ type: types.SET_MERGE_REQUEST_DIFFS, payload: [] },
{ type: types.SET_DIFF_DATA, payload: { data, diff_files: [] } },
],
[],
() => {
mock.restore();
done();
},
);
});
});
describe('setHighlightedRow', () => { describe('setHighlightedRow', () => {
it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => { it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => {
testAction(setHighlightedRow, 'ABC_123', {}, [ testAction(setHighlightedRow, 'ABC_123', {}, [
......
...@@ -28,6 +28,16 @@ describe('DiffsStoreMutations', () => { ...@@ -28,6 +28,16 @@ describe('DiffsStoreMutations', () => {
}); });
}); });
describe('SET_BATCH_LOADING', () => {
it('should set loading state', () => {
const state = {};
mutations[types.SET_BATCH_LOADING](state, false);
expect(state.isBatchLoading).toEqual(false);
});
});
describe('SET_DIFF_DATA', () => { describe('SET_DIFF_DATA', () => {
it('should set diff data type properly', () => { it('should set diff data type properly', () => {
const state = {}; const state = {};
...@@ -45,6 +55,23 @@ describe('DiffsStoreMutations', () => { ...@@ -45,6 +55,23 @@ describe('DiffsStoreMutations', () => {
}); });
}); });
describe('SET_DIFFSET_DIFF_DATA_BATCH_DATA', () => {
it('should set diff data batch type properly', () => {
const state = { diffFiles: [] };
const diffMock = {
diff_files: [diffFileMockData],
};
mutations[types.SET_DIFF_DATA_BATCH](state, diffMock);
const firstLine = state.diffFiles[0].parallel_diff_lines[0];
expect(firstLine.right.text).toBeUndefined();
expect(state.diffFiles[0].renderIt).toEqual(true);
expect(state.diffFiles[0].collapsed).toEqual(false);
});
});
describe('SET_DIFF_VIEW_TYPE', () => { describe('SET_DIFF_VIEW_TYPE', () => {
it('should set diff view type properly', () => { it('should set diff view type properly', () => {
const state = {}; const state = {};
......
...@@ -3,6 +3,10 @@ ...@@ -3,6 +3,10 @@
# This pending test can be removed when `single_mr_diff_view` is enabled by default # This pending test can be removed when `single_mr_diff_view` is enabled by default
# disabling the feature flag above is then not needed anymore. # disabling the feature flag above is then not needed anymore.
RSpec.shared_examples 'rendering a single diff version' do |attribute| RSpec.shared_examples 'rendering a single diff version' do |attribute|
before do
stub_feature_flags(diffs_batch_load: false)
end
pending 'allows editing diff settings single_mr_diff_view is enabled' do pending 'allows editing diff settings single_mr_diff_view is enabled' do
project = create(:project, :repository) project = create(:project, :repository)
user = project.creator user = project.creator
......
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