diff options
author | Fatih Acet <acetfatih@gmail.com> | 2018-06-29 10:22:07 +0300 |
---|---|---|
committer | Tim Zallmann <tzallmann@gitlab.com> | 2018-06-29 10:22:07 +0300 |
commit | d690cd9992dbd6a0d231f6c7ea1688ef90f9fc2e (patch) | |
tree | 02df5a6d7e6711612180cb155dfad0944a7454ca /app/assets | |
parent | 7c6d7accff0a4921d3c863a38382a6114ddb825e (diff) |
Prevent fetching diffs and discussions data unnecessarily on MR page
Diffstat (limited to 'app/assets')
-rw-r--r-- | app/assets/javascripts/diffs/components/app.vue | 25 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/actions.js | 5 | ||||
-rw-r--r-- | app/assets/javascripts/mr_notes/index.js | 10 | ||||
-rw-r--r-- | app/assets/javascripts/notes/components/notes_app.vue | 28 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/actions.js | 3 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/getters.js | 2 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/modules/index.js | 1 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/mutation_types.js | 1 | ||||
-rw-r--r-- | app/assets/javascripts/notes/stores/mutations.js | 4 |
9 files changed, 59 insertions, 20 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index deddb61ca31..e0aecda54f6 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -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; }, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index bf188a44022..5e0fd5109bb 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -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, diff --git a/app/assets/javascripts/mr_notes/index.js b/app/assets/javascripts/mr_notes/index.js index 3c0c9995cc2..8aabb840847 100644 --- a/app/assets/javascripts/mr_notes/index.js +++ b/app/assets/javascripts/mr_notes/index.js @@ -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']), diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index 98f8b9af168..7853847fc37 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -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)" diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 0a40b48257f..671fa4d7d22 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -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) => diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index ab28bb48e9e..a5518383d44 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -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; diff --git a/app/assets/javascripts/notes/stores/modules/index.js b/app/assets/javascripts/notes/stores/modules/index.js index a978490c009..b4cb9267e0f 100644 --- a/app/assets/javascripts/notes/stores/modules/index.js +++ b/app/assets/javascripts/notes/stores/modules/index.js @@ -10,6 +10,7 @@ export default { // View layer isToggleStateButtonLoading: false, + isNotesFetched: false, // holds endpoints and permissions provided through haml notesData: { diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index caead4cb860..a25098fbc06 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -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'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index ea165709e61..e5e40ce07fa 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -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); |