Prevent fetching diffs and discussions data unnecessarily on MR page
This commit is contained in:
parent
7c6d7accff
commit
d690cd9992
|
@ -3,6 +3,7 @@ import { mapState, mapGetters, mapActions } from 'vuex';
|
|||
import Icon from '~/vue_shared/components/icon.vue';
|
||||
import { __ } from '~/locale';
|
||||
import createFlash from '~/flash';
|
||||
import eventHub from '../../notes/event_hub';
|
||||
import LoadingIcon from '../../vue_shared/components/loading_icon.vue';
|
||||
import CompareVersions from './compare_versions.vue';
|
||||
import ChangedFiles from './changed_files.vue';
|
||||
|
@ -62,7 +63,7 @@ export default {
|
|||
plainDiffPath: state => state.diffs.plainDiffPath,
|
||||
emailPatchPath: state => state.diffs.emailPatchPath,
|
||||
}),
|
||||
...mapGetters(['isParallelView']),
|
||||
...mapGetters(['isParallelView', 'isNotesFetched']),
|
||||
targetBranch() {
|
||||
return {
|
||||
branchName: this.targetBranchName,
|
||||
|
@ -94,20 +95,36 @@ export default {
|
|||
this.adjustView();
|
||||
},
|
||||
shouldShow() {
|
||||
// When the shouldShow property changed to true, the route is rendered for the first time
|
||||
// and if we have the isLoading as true this means we didn't fetch the data
|
||||
if (this.isLoading) {
|
||||
this.fetchData();
|
||||
}
|
||||
|
||||
this.adjustView();
|
||||
},
|
||||
},
|
||||
mounted() {
|
||||
this.setBaseConfig({ endpoint: this.endpoint, projectPath: this.projectPath });
|
||||
this.fetchDiffFiles().catch(() => {
|
||||
createFlash(__('Fetching diff files failed. Please reload the page to try again!'));
|
||||
});
|
||||
|
||||
if (this.shouldShow) {
|
||||
this.fetchData();
|
||||
}
|
||||
},
|
||||
created() {
|
||||
this.adjustView();
|
||||
},
|
||||
methods: {
|
||||
...mapActions(['setBaseConfig', 'fetchDiffFiles']),
|
||||
fetchData() {
|
||||
this.fetchDiffFiles().catch(() => {
|
||||
createFlash(__('Something went wrong on our end. Please try again!'));
|
||||
});
|
||||
|
||||
if (!this.isNotesFetched) {
|
||||
eventHub.$emit('fetchNotesData');
|
||||
}
|
||||
},
|
||||
setActive(filePath) {
|
||||
this.activeFile = filePath;
|
||||
},
|
||||
|
|
|
@ -15,10 +15,6 @@ export const setBaseConfig = ({ commit }, options) => {
|
|||
commit(types.SET_BASE_CONFIG, { endpoint, projectPath });
|
||||
};
|
||||
|
||||
export const setLoadingState = ({ commit }, state) => {
|
||||
commit(types.SET_LOADING, state);
|
||||
};
|
||||
|
||||
export const fetchDiffFiles = ({ state, commit }) => {
|
||||
commit(types.SET_LOADING, true);
|
||||
|
||||
|
@ -88,7 +84,6 @@ export const expandAllFiles = ({ commit }) => {
|
|||
|
||||
export default {
|
||||
setBaseConfig,
|
||||
setLoadingState,
|
||||
fetchDiffFiles,
|
||||
setInlineDiffViewType,
|
||||
setParallelDiffViewType,
|
||||
|
|
|
@ -45,17 +45,17 @@ export default function initMrNotes() {
|
|||
this.updateDiscussionTabCounter();
|
||||
},
|
||||
},
|
||||
created() {
|
||||
this.setActiveTab(window.mrTabs.getCurrentAction());
|
||||
},
|
||||
mounted() {
|
||||
this.notesCountBadge = $('.issuable-details').find('.notes-tab .badge');
|
||||
this.setActiveTab(window.mrTabs.getCurrentAction());
|
||||
|
||||
window.mrTabs.eventHub.$on('MergeRequestTabChange', tab => {
|
||||
this.setActiveTab(tab);
|
||||
});
|
||||
$(document).on('visibilitychange', this.updateDiscussionTabCounter);
|
||||
window.mrTabs.eventHub.$on('MergeRequestTabChange', this.setActiveTab);
|
||||
},
|
||||
beforeDestroy() {
|
||||
$(document).off('visibilitychange', this.updateDiscussionTabCounter);
|
||||
window.mrTabs.eventHub.$off('MergeRequestTabChange', this.setActiveTab);
|
||||
},
|
||||
methods: {
|
||||
...mapActions(['setActiveTab']),
|
||||
|
|
|
@ -3,6 +3,7 @@ import { mapGetters, mapActions } from 'vuex';
|
|||
import { getLocationHash } from '../../lib/utils/url_utility';
|
||||
import Flash from '../../flash';
|
||||
import * as constants from '../constants';
|
||||
import eventHub from '../event_hub';
|
||||
import noteableNote from './noteable_note.vue';
|
||||
import noteableDiscussion from './noteable_discussion.vue';
|
||||
import systemNote from '../../vue_shared/components/notes/system_note.vue';
|
||||
|
@ -49,7 +50,7 @@ export default {
|
|||
};
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['discussions', 'getNotesDataByProp', 'discussionCount']),
|
||||
...mapGetters(['isNotesFetched', 'discussions', 'getNotesDataByProp', 'discussionCount']),
|
||||
noteableType() {
|
||||
return this.noteableData.noteableType;
|
||||
},
|
||||
|
@ -61,19 +62,30 @@ export default {
|
|||
isSkeletonNote: true,
|
||||
});
|
||||
}
|
||||
|
||||
return this.discussions;
|
||||
},
|
||||
},
|
||||
watch: {
|
||||
shouldShow() {
|
||||
if (!this.isNotesFetched) {
|
||||
this.fetchNotes();
|
||||
}
|
||||
},
|
||||
},
|
||||
created() {
|
||||
this.setNotesData(this.notesData);
|
||||
this.setNoteableData(this.noteableData);
|
||||
this.setUserData(this.userData);
|
||||
this.setTargetNoteHash(getLocationHash());
|
||||
eventHub.$once('fetchNotesData', this.fetchNotes);
|
||||
},
|
||||
mounted() {
|
||||
this.fetchNotes();
|
||||
const { parentElement } = this.$el;
|
||||
if (this.shouldShow) {
|
||||
this.fetchNotes();
|
||||
}
|
||||
|
||||
const { parentElement } = this.$el;
|
||||
if (parentElement && parentElement.classList.contains('js-vue-notes-event')) {
|
||||
parentElement.addEventListener('toggleAward', event => {
|
||||
const { awardName, noteId } = event.detail;
|
||||
|
@ -93,6 +105,7 @@ export default {
|
|||
setLastFetchedAt: 'setLastFetchedAt',
|
||||
setTargetNoteHash: 'setTargetNoteHash',
|
||||
toggleDiscussion: 'toggleDiscussion',
|
||||
setNotesFetchedState: 'setNotesFetchedState',
|
||||
}),
|
||||
getComponentName(discussion) {
|
||||
if (discussion.isSkeletonNote) {
|
||||
|
@ -119,11 +132,13 @@ export default {
|
|||
})
|
||||
.then(() => {
|
||||
this.isLoading = false;
|
||||
this.setNotesFetchedState(true);
|
||||
})
|
||||
.then(() => this.$nextTick())
|
||||
.then(() => this.checkLocationHash())
|
||||
.catch(() => {
|
||||
this.isLoading = false;
|
||||
this.setNotesFetchedState(true);
|
||||
Flash('Something went wrong while fetching comments. Please try again.');
|
||||
});
|
||||
},
|
||||
|
@ -161,11 +176,12 @@ export default {
|
|||
<template>
|
||||
<div
|
||||
v-if="shouldShow"
|
||||
id="notes">
|
||||
id="notes"
|
||||
>
|
||||
<ul
|
||||
id="notes-list"
|
||||
class="notes main-notes-list timeline">
|
||||
|
||||
class="notes main-notes-list timeline"
|
||||
>
|
||||
<component
|
||||
v-for="discussion in allDiscussions"
|
||||
:is="getComponentName(discussion)"
|
||||
|
|
|
@ -28,6 +28,9 @@ export const setInitialNotes = ({ commit }, discussions) =>
|
|||
|
||||
export const setTargetNoteHash = ({ commit }, data) => commit(types.SET_TARGET_NOTE_HASH, data);
|
||||
|
||||
export const setNotesFetchedState = ({ commit }, state) =>
|
||||
commit(types.SET_NOTES_FETCHED_STATE, state);
|
||||
|
||||
export const toggleDiscussion = ({ commit }, data) => commit(types.TOGGLE_DISCUSSION, data);
|
||||
|
||||
export const fetchDiscussions = ({ commit }, path) =>
|
||||
|
|
|
@ -8,6 +8,8 @@ export const targetNoteHash = state => state.targetNoteHash;
|
|||
|
||||
export const getNotesData = state => state.notesData;
|
||||
|
||||
export const isNotesFetched = state => state.isNotesFetched;
|
||||
|
||||
export const getNotesDataByProp = state => prop => state.notesData[prop];
|
||||
|
||||
export const getNoteableData = state => state.noteableData;
|
||||
|
|
|
@ -10,6 +10,7 @@ export default {
|
|||
|
||||
// View layer
|
||||
isToggleStateButtonLoading: false,
|
||||
isNotesFetched: false,
|
||||
|
||||
// holds endpoints and permissions provided through haml
|
||||
notesData: {
|
||||
|
|
|
@ -15,6 +15,7 @@ export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
|
|||
export const UPDATE_NOTE = 'UPDATE_NOTE';
|
||||
export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION';
|
||||
export const SET_DISCUSSION_DIFF_LINES = 'SET_DISCUSSION_DIFF_LINES';
|
||||
export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE';
|
||||
|
||||
// Issue
|
||||
export const CLOSE_ISSUE = 'CLOSE_ISSUE';
|
||||
|
|
|
@ -205,6 +205,10 @@ export default {
|
|||
Object.assign(state, { isToggleStateButtonLoading: value });
|
||||
},
|
||||
|
||||
[types.SET_NOTES_FETCHED_STATE](state, value) {
|
||||
Object.assign(state, { isNotesFetched: value });
|
||||
},
|
||||
|
||||
[types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) {
|
||||
const discussion = utils.findNoteObjectById(state.discussions, discussionId);
|
||||
const index = state.discussions.indexOf(discussion);
|
||||
|
|
|
@ -5,7 +5,6 @@ import {
|
|||
INLINE_DIFF_VIEW_TYPE,
|
||||
PARALLEL_DIFF_VIEW_TYPE,
|
||||
} from '~/diffs/constants';
|
||||
import store from '~/diffs/store';
|
||||
import * as actions from '~/diffs/store/actions';
|
||||
import * as types from '~/diffs/store/mutation_types';
|
||||
import axios from '~/lib/utils/axios_utils';
|
||||
|
@ -28,22 +27,6 @@ describe('DiffsStoreActions', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('setLoadingState', () => {
|
||||
it('should set loading state', done => {
|
||||
expect(store.state.diffs.isLoading).toEqual(true);
|
||||
const loadingState = false;
|
||||
|
||||
testAction(
|
||||
actions.setLoadingState,
|
||||
loadingState,
|
||||
{},
|
||||
[{ type: types.SET_LOADING, payload: loadingState }],
|
||||
[],
|
||||
done,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('fetchDiffFiles', () => {
|
||||
it('should fetch diff files', done => {
|
||||
const endpoint = '/fetch/diff/files';
|
||||
|
|
|
@ -291,4 +291,17 @@ describe('Actions Notes Store', () => {
|
|||
.catch(done.fail);
|
||||
});
|
||||
});
|
||||
|
||||
describe('setNotesFetchedState', () => {
|
||||
it('should set notes fetched state', done => {
|
||||
testAction(
|
||||
actions.setNotesFetchedState,
|
||||
true,
|
||||
{},
|
||||
[{ type: 'SET_NOTES_FETCHED_STATE', payload: true }],
|
||||
[],
|
||||
done,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -15,6 +15,7 @@ describe('Getters Notes Store', () => {
|
|||
discussions: [individualNote],
|
||||
targetNoteHash: 'hash',
|
||||
lastFetchedAt: 'timestamp',
|
||||
isNotesFetched: false,
|
||||
|
||||
notesData: notesDataMock,
|
||||
userData: userDataMock,
|
||||
|
@ -84,4 +85,10 @@ describe('Getters Notes Store', () => {
|
|||
expect(getters.openState(state)).toEqual(noteableDataMock.state);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isNotesFetched', () => {
|
||||
it('should return the state for the fetching notes', () => {
|
||||
expect(getters.isNotesFetched(state)).toBeFalsy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -318,4 +318,15 @@ describe('Notes Store mutations', () => {
|
|||
expect(state.isToggleStateButtonLoading).toEqual(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('SET_NOTES_FETCHING_STATE', () => {
|
||||
it('should set the given state', () => {
|
||||
const state = {
|
||||
isNotesFetched: false,
|
||||
};
|
||||
|
||||
mutations.SET_NOTES_FETCHED_STATE(state, true);
|
||||
expect(state.isNotesFetched).toEqual(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue