Add latest changes from gitlab-org/gitlab@master

13 files changed, 52 insertions, 80 deletions

diff --git a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js
index 092c69a01d3..fa5f8ea4005 100644
--- a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js
+++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js
@@ -24,9 +24,9 @@ const JumpToDiscussion = Vue.extend({
   computed: {
     buttonText() {
       if (this.discussionId) {
-        return __('Jump to next unresolved discussion');
+        return __('Jump to next unresolved thread');
       } else {
-        return __('Jump to first unresolved discussion');
+        return __('Jump to first unresolved thread');
       }
     },
     allResolved() {
diff --git a/app/assets/javascripts/mr_notes/init_notes.js b/app/assets/javascripts/mr_notes/init_notes.js
index a942bcc13da..2580f8e86b1 100644
--- a/app/assets/javascripts/mr_notes/init_notes.js
+++ b/app/assets/javascripts/mr_notes/init_notes.js
@@ -69,13 +69,11 @@ export default () => {
       },
     },
     render(createElement) {
-      const isDiffView = this.activeTab === 'diffs';
-
       // NOTE: Even though `discussionKeyboardNavigator` is added to the `notes-app`,
       // it adds a global key listener so it works on the diffs tab as well.
       // If we create a single Vue app for all of the MR tabs, we should move this
       // up the tree, to the root.
-      return createElement(discussionKeyboardNavigator, { props: { isDiffView } }, [
+      return createElement(discussionKeyboardNavigator, [
         createElement('notes-app', {
           props: {
             noteableData: this.noteableData,
diff --git a/app/assets/javascripts/notes/components/discussion_actions.vue b/app/assets/javascripts/notes/components/discussion_actions.vue
index fad1bc67be7..8ab31ef3448 100644
--- a/app/assets/javascripts/notes/components/discussion_actions.vue
+++ b/app/assets/javascripts/notes/components/discussion_actions.vue
@@ -73,7 +73,7 @@ export default {
       v-if="discussion.resolvable && shouldShowJumpToNextDiscussion"
       class="btn-group discussion-actions ml-sm-2"
     >
-      <jump-to-next-discussion-button @onClick="$emit('jumpToNextDiscussion')" />
+      <jump-to-next-discussion-button />
     </div>
   </div>
 </template>
diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index 98f1f385e9b..70e22db364b 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -1,5 +1,5 @@
 <script>
-import { mapActions, mapGetters } from 'vuex';
+import { mapGetters } from 'vuex';
 import { GlTooltipDirective } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
 import discussionNavigation from '../mixins/discussion_navigation';
@@ -17,9 +17,7 @@ export default {
       'getUserData',
       'getNoteableData',
       'resolvableDiscussionsCount',
-      'firstUnresolvedDiscussionId',
       'unresolvedDiscussionsCount',
-      'getDiscussion',
     ]),
     isLoggedIn() {
       return this.getUserData.id;
@@ -37,16 +35,6 @@ export default {
       return this.resolvableDiscussionsCount - this.unresolvedDiscussionsCount;
     },
   },
-  methods: {
-    ...mapActions(['expandDiscussion']),
-    jumpToFirstUnresolvedDiscussion() {
-      const diffTab = window.mrTabs.currentAction === 'diffs';
-      const discussionId =
-        this.firstUnresolvedDiscussionId(diffTab) || this.firstUnresolvedDiscussionId();
-      const firstDiscussion = this.getDiscussion(discussionId);
-      this.jumpToDiscussion(firstDiscussion);
-    },
-  },
 };
 </script>
 
@@ -83,9 +71,9 @@ export default {
       <div v-if="isLoggedIn && !allResolved" class="btn-group btn-group-sm" role="group">
         <button
           v-gl-tooltip
-          title="Jump to first unresolved thread"
+          title="Jump to next unresolved thread"
           class="btn btn-default discussion-next-btn"
-          @click="jumpToFirstUnresolvedDiscussion"
+          @click="jumpToNextDiscussion"
         >
           <icon name="comment-next" />
         </button>
diff --git a/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue b/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue
index f87ca097b40..630d4fd89b1 100644
--- a/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue
+++ b/app/assets/javascripts/notes/components/discussion_jump_to_next_button.vue
@@ -1,6 +1,7 @@
 <script>
 import { GlTooltipDirective } from '@gitlab/ui';
 import icon from '~/vue_shared/components/icon.vue';
+import discussionNavigation from '../mixins/discussion_navigation';
 
 export default {
   name: 'JumpToNextDiscussionButton',
@@ -10,6 +11,7 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
+  mixins: [discussionNavigation],
 };
 </script>
 
@@ -19,8 +21,8 @@ export default {
       ref="button"
       v-gl-tooltip
       class="btn btn-default discussion-next-btn"
-      :title="s__('MergeRequests|Jump to next unresolved discussion')"
-      @click="$emit('onClick')"
+      :title="s__('MergeRequests|Jump to next unresolved thread')"
+      @click="jumpToNextDiscussion"
     >
       <icon name="comment-next" />
     </button>
diff --git a/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue b/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
index 7d742fbfeee..2dc222d08f9 100644
--- a/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
+++ b/app/assets/javascripts/notes/components/discussion_keyboard_navigator.vue
@@ -1,53 +1,18 @@
 <script>
 /* global Mousetrap */
 import 'mousetrap';
-import { mapGetters, mapActions } from 'vuex';
 import discussionNavigation from '~/notes/mixins/discussion_navigation';
 
 export default {
   mixins: [discussionNavigation],
-  props: {
-    isDiffView: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
-  },
-  data() {
-    return {
-      currentDiscussionId: null,
-    };
-  },
-  computed: {
-    ...mapGetters([
-      'nextUnresolvedDiscussionId',
-      'previousUnresolvedDiscussionId',
-      'getDiscussion',
-    ]),
-  },
   mounted() {
-    Mousetrap.bind('n', () => this.jumpToNextDiscussion());
-    Mousetrap.bind('p', () => this.jumpToPreviousDiscussion());
+    Mousetrap.bind('n', this.jumpToNextDiscussion);
+    Mousetrap.bind('p', this.jumpToPreviousDiscussion);
   },
   beforeDestroy() {
     Mousetrap.unbind('n');
     Mousetrap.unbind('p');
   },
-  methods: {
-    ...mapActions(['expandDiscussion']),
-    jumpToNextDiscussion() {
-      const nextId = this.nextUnresolvedDiscussionId(this.currentDiscussionId, this.isDiffView);
-      const nextDiscussion = this.getDiscussion(nextId);
-      this.jumpToDiscussion(nextDiscussion);
-      this.currentDiscussionId = nextId;
-    },
-    jumpToPreviousDiscussion() {
-      const prevId = this.previousUnresolvedDiscussionId(this.currentDiscussionId, this.isDiffView);
-      const prevDiscussion = this.getDiscussion(prevId);
-      this.jumpToDiscussion(prevDiscussion);
-      this.currentDiscussionId = prevId;
-    },
-  },
   render() {
     return this.$slots.default;
   },
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index 3462ee72dd3..189ff88feb3 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -14,7 +14,6 @@ import noteForm from './note_form.vue';
 import diffWithNote from './diff_with_note.vue';
 import noteable from '../mixins/noteable';
 import resolvable from '../mixins/resolvable';
-import discussionNavigation from '../mixins/discussion_navigation';
 import eventHub from '../event_hub';
 import DiscussionNotes from './discussion_notes.vue';
 import DiscussionActions from './discussion_actions.vue';
@@ -35,7 +34,7 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
-  mixins: [noteable, resolvable, discussionNavigation, diffLineNoteFormMixin],
+  mixins: [noteable, resolvable, diffLineNoteFormMixin],
   props: {
     discussion: {
       type: Object,
@@ -79,12 +78,8 @@ export default {
       'convertedDisscussionIds',
       'getNoteableData',
       'userCanReply',
-      'nextUnresolvedDiscussionId',
-      'unresolvedDiscussionsCount',
-      'hasUnresolvedDiscussions',
       'showJumpToNextDiscussion',
       'getUserData',
-      'getDiscussion',
     ]),
     currentUser() {
       return this.getUserData;
@@ -152,7 +147,6 @@ export default {
       'saveNote',
       'removePlaceholderNotes',
       'toggleResolveNote',
-      'expandDiscussion',
       'removeConvertedDiscussion',
     ]),
     showReplyForm() {
@@ -219,15 +213,6 @@ export default {
           callback(err);
         });
     },
-    jumpToNextDiscussion() {
-      const nextId = this.nextUnresolvedDiscussionId(
-        this.discussion.id,
-        this.discussionsByDiffOrder,
-      );
-      const nextDiscussion = this.getDiscussion(nextId);
-
-      this.jumpToDiscussion(nextDiscussion);
-    },
     deleteNoteHandler(note) {
       this.$emit('noteDeleted', this.discussion, note);
     },
@@ -294,7 +279,6 @@ export default {
                     :should-show-jump-to-next-discussion="shouldShowJumpToNextDiscussion"
                     @showReplyForm="showReplyForm"
                     @resolve="resolveHandler"
-                    @jumpToNextDiscussion="jumpToNextDiscussion"
                   />
                   <div v-if="isReplying" class="avatar-note-form-holder">
                     <user-avatar-link
diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js
index 94ca01e44cc..e5066695403 100644
--- a/app/assets/javascripts/notes/mixins/discussion_navigation.js
+++ b/app/assets/javascripts/notes/mixins/discussion_navigation.js
@@ -1,8 +1,21 @@
+import { mapGetters, mapActions, mapState } from 'vuex';
 import { scrollToElement } from '~/lib/utils/common_utils';
 import eventHub from '../../notes/event_hub';
 
 export default {
+  computed: {
+    ...mapGetters([
+      'nextUnresolvedDiscussionId',
+      'previousUnresolvedDiscussionId',
+      'getDiscussion',
+    ]),
+    ...mapState({
+      currentDiscussionId: state => state.notes.currentDiscussionId,
+    }),
+  },
   methods: {
+    ...mapActions(['expandDiscussion', 'setCurrentDiscussionId']),
+
     diffsJump(id) {
       const selector = `ul.notes[data-discussion-id="${id}"]`;
 
@@ -58,5 +71,21 @@ export default {
         }
       }
     },
+
+    jumpToNextDiscussion() {
+      this.handleDiscussionJump(this.nextUnresolvedDiscussionId);
+    },
+
+    jumpToPreviousDiscussion() {
+      this.handleDiscussionJump(this.previousUnresolvedDiscussionId);
+    },
+
+    handleDiscussionJump(fn) {
+      const isDiffView = window.mrTabs.currentAction === 'diffs';
+      const targetId = fn(this.currentDiscussionId, isDiffView);
+      const discussion = this.getDiscussion(targetId);
+      this.jumpToDiscussion(discussion);
+      this.setCurrentDiscussionId(targetId);
+    },
   },
 };
diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js
index 9bd245c094d..f3dc6187c3f 100644
--- a/app/assets/javascripts/notes/stores/actions.js
+++ b/app/assets/javascripts/notes/stores/actions.js
@@ -506,5 +506,8 @@ export const fetchDescriptionVersion = (_, { endpoint, startingVersion }) => {
     });
 };
 
+export const setCurrentDiscussionId = ({ commit }, discussionId) =>
+  commit(types.SET_CURRENT_DISCUSSION_ID, discussionId);
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};
diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js
index 3d0ec8cd3a7..35398999abc 100644
--- a/app/assets/javascripts/notes/stores/getters.js
+++ b/app/assets/javascripts/notes/stores/getters.js
@@ -59,7 +59,6 @@ export const getDiscussionLastNote = state => discussion =>
 
 export const unresolvedDiscussionsCount = state => state.unresolvedDiscussionsCount;
 export const resolvableDiscussionsCount = state => state.resolvableDiscussionsCount;
-export const hasUnresolvedDiscussions = state => state.hasUnresolvedDiscussions;
 
 export const showJumpToNextDiscussion = (state, getters) => (mode = 'discussion') => {
   const orderedDiffs =
diff --git a/app/assets/javascripts/notes/stores/modules/index.js b/app/assets/javascripts/notes/stores/modules/index.js
index 6168aeae35d..771b80108b8 100644
--- a/app/assets/javascripts/notes/stores/modules/index.js
+++ b/app/assets/javascripts/notes/stores/modules/index.js
@@ -8,6 +8,7 @@ export default () => ({
     convertedDisscussionIds: [],
     targetNoteHash: null,
     lastFetchedAt: null,
+    currentDiscussionId: null,
 
     // View layer
     isToggleStateButtonLoading: false,
@@ -26,7 +27,6 @@ export default () => ({
     commentsDisabled: false,
     resolvableDiscussionsCount: 0,
     unresolvedDiscussionsCount: 0,
-    hasUnresolvedDiscussions: false,
   },
   actions,
   getters,
diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js
index 796370920bb..8eb426d3f9b 100644
--- a/app/assets/javascripts/notes/stores/mutation_types.js
+++ b/app/assets/javascripts/notes/stores/mutation_types.js
@@ -25,6 +25,7 @@ export const COLLAPSE_DISCUSSION = 'COLLAPSE_DISCUSSION';
 export const EXPAND_DISCUSSION = 'EXPAND_DISCUSSION';
 export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
 export const UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS = 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS';
+export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID';
 
 // 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 e70f0238316..71091d26b85 100644
--- a/app/assets/javascripts/notes/stores/mutations.js
+++ b/app/assets/javascripts/notes/stores/mutations.js
@@ -267,7 +267,6 @@ export default {
         discussion.resolvable &&
         discussion.notes.some(note => note.resolvable && !note.resolved),
     ).length;
-    state.hasUnresolvedDiscussions = state.unresolvedDiscussionsCount > 1;
   },
 
   [types.CONVERT_TO_DISCUSSION](state, discussionId) {
@@ -281,4 +280,8 @@ export default {
     convertedDisscussionIds.splice(convertedDisscussionIds.indexOf(discussionId), 1);
     Object.assign(state, { convertedDisscussionIds });
   },
+
+  [types.SET_CURRENT_DISCUSSION_ID](state, discussionId) {
+    state.currentDiscussionId = discussionId;
+  },
 };