Prevent fetching diffs and discussions data unnecessarily on MR page

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);