Fix notes_app not expanding with line_code anchor48169-fix-discussion-on-diff-links

**What was the cause?**
- The `checkLocationHash` function only looked for hashes with note_*

**What was the fix?**
- Move the bulk of this method into it's own action (for testability)
- Add a function `whereDiscussionMatchesHash` which returns a
  predicate for hashes that match:
  - `note_*`
  - `discussion_*`
  - `*` (which is treated as the line_code id)

(Also adds changelog)

8 files changed, 185 insertions, 17 deletions

diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index d8e8efb982a..a0931f5560a 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -109,6 +109,7 @@ export default {
       setTargetNoteHash: 'setTargetNoteHash',
       toggleDiscussion: 'toggleDiscussion',
       setNotesFetchedState: 'setNotesFetchedState',
+      expandDiscussionFromHash: 'expandDiscussionFromHash',
     }),
     getComponentName(discussion) {
       if (discussion.isSkeletonNote) {
@@ -157,21 +158,7 @@ export default {
       this.isPollingInitialized = true;
     },
     checkLocationHash() {
-      const hash = getLocationHash();
-      const noteId = hash && hash.replace(/^note_/, '');
-
-      if (noteId) {
-        this.discussions.forEach(discussion => {
-          if (discussion.notes) {
-            discussion.notes.forEach(note => {
-              if (`${note.id}` === `${noteId}`) {
-                // FIXME: this modifies the store state without using a mutation/action
-                Object.assign(discussion, { expanded: true });
-              }
-            });
-          }
-        });
-      }
+      this.expandDiscussionFromHash(getLocationHash());
     },
   },
 };
diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js
index 320dfa47d5a..ffc80f19e48 100644
--- a/app/assets/javascripts/notes/stores/actions.js
+++ b/app/assets/javascripts/notes/stores/actions.js
@@ -332,5 +332,13 @@ export const updateMergeRequestWidget = () => {
   mrWidgetEventHub.$emit('mr.discussion.updated');
 };
 
+export const expandDiscussionFromHash = ({ getters, dispatch }, hash) => {
+  const discussion = getters.discussionFromHash(hash);
+
+  if (discussion) {
+    dispatch('expandDiscussion', { discussionId: discussion.id });
+  }
+};
+
 // 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 d4babf1fab2..adbaa3fd7cc 100644
--- a/app/assets/javascripts/notes/stores/getters.js
+++ b/app/assets/javascripts/notes/stores/getters.js
@@ -1,6 +1,9 @@
 import _ from 'underscore';
 import * as constants from '../constants';
-import { reduceDiscussionsToLineCodes } from './utils';
+import {
+  reduceDiscussionsToLineCodes,
+  whereDiscussionMatchesHash,
+} from './utils';
 import { collapseSystemNotes } from './collapse_utils';
 
 export const discussions = state => collapseSystemNotes(state.discussions);
@@ -191,5 +194,8 @@ export const firstUnresolvedDiscussionId = (state, getters) => diffOrder => {
   return getters.unresolvedDiscussionsIdsByDate[0];
 };
 
+export const discussionFromHash = (state, getters) => hash =>
+  getters.discussions.find(whereDiscussionMatchesHash(hash));
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};
diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js
index 0e41ff03d67..0eea4740f35 100644
--- a/app/assets/javascripts/notes/stores/utils.js
+++ b/app/assets/javascripts/notes/stores/utils.js
@@ -1,6 +1,8 @@
 import AjaxCache from '~/lib/utils/ajax_cache';
 
 const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm;
+const REGEX_NOTE_HASH = /^note_/;
+const REGEX_DISCUSSION_HASH = /^discussion_/;
 
 export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0];
 
@@ -40,3 +42,44 @@ export const reduceDiscussionsToLineCodes = selectedDiscussions =>
 export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(note);
 
 export const stripQuickActions = note => note.replace(REGEX_QUICK_ACTIONS, '').trim();
+
+/**
+ * Create a predicate function to check if a discussion matches the given hash
+ *
+ * Based on the format of the hash, a different predicate is returned:
+ *
+ * | Format         | Rule                                        |
+ * |----------------|---------------------------------------------|
+ * | `note_*`       | match if discussion has a note with the id  |
+ * | `discussion_*` | match if discussion has the given id        |
+ * | `*`            | match if discussion has the given line_code |
+ *
+ * Example:
+ *
+```
+const hash = getLocationHash();
+
+return discussions.filter(whereDiscussionMatchesHash(hash));
+```
+ *
+ * @param {String} hash - A string that represents a discussion selector
+ * @returns {(discussion: any) => Boolean} A predicate that matches discussions against the hash
+ */
+export const whereDiscussionMatchesHash = hash => {
+  if (!hash) {
+    return () => false;
+  } else if (hash.match(REGEX_NOTE_HASH)) {
+    const noteId = hash.replace(REGEX_NOTE_HASH, '');
+
+    return discussion => discussion.notes.some(note => String(note.id) === noteId);
+  } else if (hash.match(REGEX_DISCUSSION_HASH)) {
+    const discussionId = hash.replace(REGEX_DISCUSSION_HASH, '');
+
+    return discussion => discussion.id === discussionId;
+  }
+
+  return discussion => discussion.line_code === hash;
+};
+
+// prevent babel-plugin-rewire from generating an invalid default during karma tests
+export default () => {};
diff --git a/changelogs/unreleased/48169-fix-discussion-on-diff-links.yml b/changelogs/unreleased/48169-fix-discussion-on-diff-links.yml
new file mode 100644
index 00000000000..1e8d60f20eb
--- /dev/null
+++ b/changelogs/unreleased/48169-fix-discussion-on-diff-links.yml
@@ -0,0 +1,5 @@
+---
+title: Fix links to discussions not expanding in diffs
+merge_request: 21812
+author:
+type: fixed
diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js
index f4643fd55ed..cc6724c6c02 100644
--- a/spec/javascripts/notes/stores/actions_spec.js
+++ b/spec/javascripts/notes/stores/actions_spec.js
@@ -509,4 +509,17 @@ describe('Actions Notes Store', () => {
       expect(mrWidgetEventHub.$emit).toHaveBeenCalledWith('mr.discussion.updated');
     });
   });
+
+  describe('expandDiscussionFromHash', () => {
+    it('should expand discussion that matches hash', done => {
+      testAction(
+        actions.expandDiscussionFromHash,
+        `discussion_${discussionMock.id}`,
+        { discussionFromHash: () => discussionMock },
+        [],
+        [{ type: 'expandDiscussion', payload: { discussionId: discussionMock.id } }],
+        done,
+      );
+    });
+  });
 });
diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js
index 7f8ede51508..353c0997c44 100644
--- a/spec/javascripts/notes/stores/getters_spec.js
+++ b/spec/javascripts/notes/stores/getters_spec.js
@@ -1,4 +1,4 @@
-import * as getters from '~/notes/stores/getters';
+import gettersModule, * as getters from '~/notes/stores/getters';
 import {
   notesDataMock,
   userDataMock,
@@ -264,4 +264,35 @@ describe('Getters Notes Store', () => {
       expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(false)).toBeFalsy();
     });
   });
+
+  describe('discussionFromHash', () => {
+    let localGetters;
+
+    beforeEach(() => {
+      localGetters = {
+        discussions: state.discussions,
+      };
+    });
+
+    it('should call "whereDiscussionMatchesHash"', () => {
+      const hash = 'TEST_HASH';
+      const whereDiscussionMatchesHash = spyOnDependency(gettersModule, 'whereDiscussionMatchesHash').and.callThrough();
+
+      getters.discussionFromHash(state, localGetters)(hash);
+
+      expect(whereDiscussionMatchesHash).toHaveBeenCalledWith(hash);
+    });
+
+    it('should return discussion if matches hash', () => {
+      const result = getters.discussionFromHash(state, localGetters)(`discussion_${individualNote.id}`);
+
+      expect(result).toEqual(individualNote);
+    });
+
+    it('should return undefined if no match', () => {
+      const result = getters.discussionFromHash(state, localGetters)('does_not_exist');
+
+      expect(result).toBeUndefined();
+    });
+  });
 });
diff --git a/spec/javascripts/notes/stores/utils_spec.js b/spec/javascripts/notes/stores/utils_spec.js
new file mode 100644
index 00000000000..7e34b67e405
--- /dev/null
+++ b/spec/javascripts/notes/stores/utils_spec.js
@@ -0,0 +1,75 @@
+import {
+  whereDiscussionMatchesHash,
+} from '~/notes/stores/utils';
+
+const TEST_DISCUSSIONS = [
+  {
+    id: 'abc',
+    notes: [],
+    line_code: 'zzz123',
+  },
+  {
+    id: 'def',
+    notes: [
+      { id: 1 },
+      { id: 2 },
+    ],
+    line_code: 'zzz456',
+  },
+  {
+    id: 'ghi',
+    notes: [],
+    line_code: null,
+  },
+  {
+    id: 'jkl',
+    notes: [
+      { id: 3 },
+      { id: 4 },
+    ],
+    line_code: null,
+  },
+  {
+    id: 'mno',
+    notes: [
+      { id: 5 },
+    ],
+    line_code: 'zzz456',
+  },
+];
+
+describe('notes/stores/utils', () => {
+  describe('whereDiscussionMatchesHash', () => {
+    it('returns filter for note id if hash is "note_*"', () => {
+      const filter = whereDiscussionMatchesHash('note_4');
+
+      const result = TEST_DISCUSSIONS.filter(filter);
+
+      expect(result).toEqual([TEST_DISCUSSIONS[3]]);
+    });
+
+    it('returns filter for discussion id if hash is "discussion_*"', () => {
+      const filter = whereDiscussionMatchesHash('discussion_ghi');
+
+      const result = TEST_DISCUSSIONS.filter(filter);
+
+      expect(result).toEqual([TEST_DISCUSSIONS[2]]);
+    });
+
+    it('return filter for line_code if hash is "*"', () => {
+      const filter = whereDiscussionMatchesHash('zzz456');
+
+      const result = TEST_DISCUSSIONS.filter(filter);
+
+      expect(result).toEqual([TEST_DISCUSSIONS[1], TEST_DISCUSSIONS[4]]);
+    });
+
+    it('return false filter if hash in falsey', () => {
+      const filter = whereDiscussionMatchesHash('');
+
+      const result = TEST_DISCUSSIONS.filter(filter);
+
+      expect(result).toEqual([]);
+    });
+  });
+});