Fix notes race condition when linking to specific note

Before, notes (discussions.json) would be fetched twice in the following
case:
* User has a "Show comments only" or "Show history only" filter
* User then goes to an issue with a note hash, i.e. #note_*
* Frontend then makes two notes requests:
  1. Notes for the user's original filter
  2. Notes for "Show all activity" so that the note is definitely shown

This commit fixes this race condition so that there is always only
one note request made whether it is for a specific note or not.

app/assets/javascripts/lib/utils/url_utility.js

@@ -87,6 +87,14 @@ export function getLocationHash(url = window.location.href) {
   return hashIndex === -1 ? null : url.substring(hashIndex + 1);
 }
 
+/**
+ * Returns a boolean indicating whether the URL hash contains the given string value
+ */
+export function doesHashExistInUrl(hashName) {
+  const hash = getLocationHash();
+  return hash && hash.includes(hashName);
+}
+
 /**
  * Apply the fragment to the given url by returning a new url string that includes
  * the fragment. If the given url already contains a fragment, the original fragment

app/assets/javascripts/notes/components/discussion_filter.vue

 <script>
 import $ from 'jquery';
 import { mapGetters, mapActions } from 'vuex';
-import { getLocationHash } from '../../lib/utils/url_utility';
+import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
 import Icon from '~/vue_shared/components/icon.vue';
 import {
   DISCUSSION_FILTERS_DEFAULT_VALUE,
   HISTORY_ONLY_FILTER_VALUE,
   DISCUSSION_TAB_LABEL,
   DISCUSSION_FILTER_TYPES,
+  NOTE_UNDERSCORE,
 } from '../constants';
 import notesEventHub from '../event_hub';
 
@@ -28,7 +29,9 @@ export default {
   },
   data() {
     return {
-      currentValue: this.selectedValue,
+      currentValue: doesHashExistInUrl(NOTE_UNDERSCORE)
+        ? DISCUSSION_FILTERS_DEFAULT_VALUE
+        : this.selectedValue,
       defaultValue: DISCUSSION_FILTERS_DEFAULT_VALUE,
       displayFilters: true,
     };
@@ -50,7 +53,6 @@ export default {
 
     notesEventHub.$on('dropdownSelect', this.selectFilter);
     window.addEventListener('hashchange', this.handleLocationHash);
-    this.handleLocationHash();
   },
   mounted() {
     this.toggleCommentsForm();

app/assets/javascripts/notes/components/notes_app.vue

 <script>
 import { __ } from '~/locale';
 import { mapGetters, mapActions } from 'vuex';
-import { getLocationHash } from '../../lib/utils/url_utility';
+import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
 import Flash from '../../flash';
 import * as constants from '../constants';
 import eventHub from '../event_hub';
@@ -156,19 +156,17 @@ export default {
 
       this.isFetching = true;
 
-      return this.fetchDiscussions({ path: this.getNotesDataByProp('discussionsPath') })
-        .then(() => {
-          this.initPolling();
-        })
+      return this.fetchDiscussions(this.getFetchDiscussionsConfig())
+        .then(this.initPolling)
         .then(() => {
           this.setLoadingState(false);
           this.setNotesFetchedState(true);
           eventHub.$emit('fetchedNotesData');
           this.isFetching = false;
         })
-        .then(() => this.$nextTick())
-        .then(() => this.startTaskList())
-        .then(() => this.checkLocationHash())
+        .then(this.$nextTick)
+        .then(this.startTaskList)
+        .then(this.checkLocationHash)
         .catch(() => {
           this.setLoadingState(false);
           this.setNotesFetchedState(true);
@@ -199,9 +197,20 @@ export default {
     },
     startReplying(discussionId) {
       return this.convertToDiscussion(discussionId)
-        .then(() => this.$nextTick())
+        .then(this.$nextTick)
         .then(() => eventHub.$emit('startReplying', discussionId));
     },
+    getFetchDiscussionsConfig() {
+      const defaultConfig = { path: this.getNotesDataByProp('discussionsPath') };
+
+      if (doesHashExistInUrl(constants.NOTE_UNDERSCORE)) {
+        return Object.assign({}, defaultConfig, {
+          filter: constants.DISCUSSION_FILTERS_DEFAULT_VALUE,
+          persistFilter: false,
+        });
+      }
+      return defaultConfig;
+    },
   },
   systemNote: constants.SYSTEM_NOTE,
 };

app/assets/javascripts/notes/constants.js

@@ -8,8 +8,6 @@ export const OPENED = 'opened';
 export const REOPENED = 'reopened';
 export const CLOSED = 'closed';
 export const MERGED = 'merged';
-export const EMOJI_THUMBSUP = 'thumbsup';
-export const EMOJI_THUMBSDOWN = 'thumbsdown';
 export const ISSUE_NOTEABLE_TYPE = 'issue';
 export const EPIC_NOTEABLE_TYPE = 'epic';
 export const MERGE_REQUEST_NOTEABLE_TYPE = 'MergeRequest';
@@ -19,6 +17,7 @@ export const DESCRIPTION_TYPE = 'changed the description';
 export const HISTORY_ONLY_FILTER_VALUE = 2;
 export const DISCUSSION_FILTERS_DEFAULT_VALUE = 0;
 export const DISCUSSION_TAB_LABEL = 'show';
+export const NOTE_UNDERSCORE = 'note_';
 
 export const NOTEABLE_TYPE_MAPPING = {
   Issue: ISSUE_NOTEABLE_TYPE,

changelogs/unreleased/30497-race-condition-in-discussions-json-request-could-lead-to-notes-show.yml

+---
+title: Fix notes race condition when linking to specific note
+merge_request: 17777
+author:
+type: fixed

spec/frontend/lib/utils/url_utility_spec.js

@@ -136,6 +136,24 @@ describe('URL utility', () => {
     });
   });
 
+  describe('doesHashExistInUrl', () => {
+    it('should return true when the given string exists in the URL hash', () => {
+      setWindowLocation({
+        href: 'https://gitlab.com/gitlab-org/gitlab-test/issues/1#note_1',
+      });
+
+      expect(urlUtils.doesHashExistInUrl('note_')).toBe(true);
+    });
+
+    it('should return false when the given string does not exist in the URL hash', () => {
+      setWindowLocation({
+        href: 'https://gitlab.com/gitlab-org/gitlab-test/issues/1#note_1',
+      });
+
+      expect(urlUtils.doesHashExistInUrl('doesnotexist')).toBe(false);
+    });
+  });
+
   describe('setUrlFragment', () => {
     it('should set fragment when url has no fragment', () => {
       const url = urlUtils.setUrlFragment('/home/feature', 'usage');

spec/javascripts/notes/components/discussion_filter_spec.js

@@ -160,5 +160,28 @@ describe('DiscussionFilter component', () => {
         done();
       });
     });
+
+    it('fetches discussions when there is a hash', done => {
+      window.location.hash = `note_${discussionMock.notes[0].id}`;
+      vm.currentValue = discussionFiltersMock[2].value;
+      spyOn(vm, 'selectFilter');
+      vm.handleLocationHash();
+
+      vm.$nextTick(() => {
+        expect(vm.selectFilter).toHaveBeenCalled();
+        done();
+      });
+    });
+
+    it('does not fetch discussions when there is no hash', done => {
+      window.location.hash = '';
+      spyOn(vm, 'selectFilter');
+      vm.handleLocationHash();
+
+      vm.$nextTick(() => {
+        expect(vm.selectFilter).not.toHaveBeenCalled();
+        done();
+      });
+    });
   });
 });