summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/diffs/components/app.vue25
-rw-r--r--app/assets/javascripts/diffs/store/actions.js5
-rw-r--r--app/assets/javascripts/mr_notes/index.js10
-rw-r--r--app/assets/javascripts/notes/components/notes_app.vue28
-rw-r--r--app/assets/javascripts/notes/stores/actions.js3
-rw-r--r--app/assets/javascripts/notes/stores/getters.js2
-rw-r--r--app/assets/javascripts/notes/stores/modules/index.js1
-rw-r--r--app/assets/javascripts/notes/stores/mutation_types.js1
-rw-r--r--app/assets/javascripts/notes/stores/mutations.js4
-rw-r--r--spec/javascripts/diffs/store/actions_spec.js17
-rw-r--r--spec/javascripts/notes/stores/actions_spec.js13
-rw-r--r--spec/javascripts/notes/stores/getters_spec.js7
-rw-r--r--spec/javascripts/notes/stores/mutation_spec.js11
13 files changed, 90 insertions, 37 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);
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index f0bd098f698..6829c1e956a 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -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';
diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js
index 985c2f81ef3..71ef3aa9b03 100644
--- a/spec/javascripts/notes/stores/actions_spec.js
+++ b/spec/javascripts/notes/stores/actions_spec.js
@@ -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,
+ );
+ });
+ });
});
diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js
index 5501e50e97b..815cc09621f 100644
--- a/spec/javascripts/notes/stores/getters_spec.js
+++ b/spec/javascripts/notes/stores/getters_spec.js
@@ -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();
+ });
+ });
});
diff --git a/spec/javascripts/notes/stores/mutation_spec.js b/spec/javascripts/notes/stores/mutation_spec.js
index 556a1c244c0..ccc7328447b 100644
--- a/spec/javascripts/notes/stores/mutation_spec.js
+++ b/spec/javascripts/notes/stores/mutation_spec.js
@@ -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);
+ });
+ });
});