Resolve "Link to file in Changes tab of MR no longer works for all files after incremental rendering improvement"

1 files changed, 139 insertions, 17 deletions

diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index fd5c5e104b4..4c662fac231 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -5,7 +5,23 @@ import {
   INLINE_DIFF_VIEW_TYPE,
   PARALLEL_DIFF_VIEW_TYPE,
 } from '~/diffs/constants';
-import * as actions from '~/diffs/store/actions';
+import actions, {
+  setBaseConfig,
+  fetchDiffFiles,
+  assignDiscussionsToDiff,
+  removeDiscussionsFromDiff,
+  startRenderDiffsQueue,
+  setInlineDiffViewType,
+  setParallelDiffViewType,
+  showCommentForm,
+  cancelCommentForm,
+  loadMoreLines,
+  scrollToLineIfNeededInline,
+  scrollToLineIfNeededParallel,
+  loadCollapsedDiff,
+  expandAllFiles,
+  toggleFileDiscussions,
+} from '~/diffs/store/actions';
 import * as types from '~/diffs/store/mutation_types';
 import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils';
 import axios from '~/lib/utils/axios_utils';
@@ -37,7 +53,7 @@ describe('DiffsStoreActions', () => {
       const projectPath = '/root/project';
 
       testAction(
-        actions.setBaseConfig,
+        setBaseConfig,
         { endpoint, projectPath },
         { endpoint: '', projectPath: '' },
         [{ type: types.SET_BASE_CONFIG, payload: { endpoint, projectPath } }],
@@ -55,7 +71,7 @@ describe('DiffsStoreActions', () => {
       mock.onGet(endpoint).reply(200, res);
 
       testAction(
-        actions.fetchDiffFiles,
+        fetchDiffFiles,
         {},
         { endpoint },
         [
@@ -139,7 +155,7 @@ describe('DiffsStoreActions', () => {
       const discussions = reduceDiscussionsToLineCodes([singleDiscussion]);
 
       testAction(
-        actions.assignDiscussionsToDiff,
+        assignDiscussionsToDiff,
         discussions,
         state,
         [
@@ -208,7 +224,7 @@ describe('DiffsStoreActions', () => {
       };
 
       testAction(
-        actions.removeDiscussionsFromDiff,
+        removeDiscussionsFromDiff,
         singleDiscussion,
         state,
         [
@@ -252,8 +268,7 @@ describe('DiffsStoreActions', () => {
         });
       };
 
-      actions
-        .startRenderDiffsQueue({ state, commit: pseudoCommit })
+      startRenderDiffsQueue({ state, commit: pseudoCommit })
         .then(() => {
           expect(state.diffFiles[0].renderIt).toBeTruthy();
           expect(state.diffFiles[1].renderIt).toBeTruthy();
@@ -269,7 +284,7 @@ describe('DiffsStoreActions', () => {
   describe('setInlineDiffViewType', () => {
     it('should set diff view type to inline and also set the cookie properly', done => {
       testAction(
-        actions.setInlineDiffViewType,
+        setInlineDiffViewType,
         null,
         {},
         [{ type: types.SET_DIFF_VIEW_TYPE, payload: INLINE_DIFF_VIEW_TYPE }],
@@ -287,7 +302,7 @@ describe('DiffsStoreActions', () => {
   describe('setParallelDiffViewType', () => {
     it('should set diff view type to parallel and also set the cookie properly', done => {
       testAction(
-        actions.setParallelDiffViewType,
+        setParallelDiffViewType,
         null,
         {},
         [{ type: types.SET_DIFF_VIEW_TYPE, payload: PARALLEL_DIFF_VIEW_TYPE }],
@@ -307,7 +322,7 @@ describe('DiffsStoreActions', () => {
       const payload = { lineCode: 'lineCode' };
 
       testAction(
-        actions.showCommentForm,
+        showCommentForm,
         payload,
         {},
         [{ type: types.ADD_COMMENT_FORM_LINE, payload }],
@@ -322,7 +337,7 @@ describe('DiffsStoreActions', () => {
       const payload = { lineCode: 'lineCode' };
 
       testAction(
-        actions.cancelCommentForm,
+        cancelCommentForm,
         payload,
         {},
         [{ type: types.REMOVE_COMMENT_FORM_LINE, payload }],
@@ -344,7 +359,7 @@ describe('DiffsStoreActions', () => {
       mock.onGet(endpoint).reply(200, contextLines);
 
       testAction(
-        actions.loadMoreLines,
+        loadMoreLines,
         options,
         {},
         [
@@ -370,7 +385,7 @@ describe('DiffsStoreActions', () => {
       mock.onGet(file.loadCollapsedDiffUrl).reply(200, data);
 
       testAction(
-        actions.loadCollapsedDiff,
+        loadCollapsedDiff,
         file,
         {},
         [
@@ -391,7 +406,7 @@ describe('DiffsStoreActions', () => {
   describe('expandAllFiles', () => {
     it('should change the collapsed prop from the diffFiles', done => {
       testAction(
-        actions.expandAllFiles,
+        expandAllFiles,
         null,
         {},
         [
@@ -415,7 +430,7 @@ describe('DiffsStoreActions', () => {
 
       const dispatch = jasmine.createSpy('dispatch');
 
-      actions.toggleFileDiscussions({ getters, dispatch });
+      toggleFileDiscussions({ getters, dispatch });
 
       expect(dispatch).toHaveBeenCalledWith(
         'collapseDiscussion',
@@ -433,7 +448,7 @@ describe('DiffsStoreActions', () => {
 
       const dispatch = jasmine.createSpy();
 
-      actions.toggleFileDiscussions({ getters, dispatch });
+      toggleFileDiscussions({ getters, dispatch });
 
       expect(dispatch).toHaveBeenCalledWith(
         'expandDiscussion',
@@ -451,7 +466,7 @@ describe('DiffsStoreActions', () => {
 
       const dispatch = jasmine.createSpy();
 
-      actions.toggleFileDiscussions({ getters, dispatch });
+      toggleFileDiscussions({ getters, dispatch });
 
       expect(dispatch).toHaveBeenCalledWith(
         'expandDiscussion',
@@ -460,4 +475,111 @@ describe('DiffsStoreActions', () => {
       );
     });
   });
+
+  describe('scrollToLineIfNeededInline', () => {
+    const lineMock = {
+      lineCode: 'ABC_123',
+    };
+
+    it('should not call handleLocationHash when there is not hash', () => {
+      window.location.hash = '';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededInline({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should not call handleLocationHash when the hash does not match any line', () => {
+      window.location.hash = 'XYZ_456';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededInline({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should call handleLocationHash only when the hash matches a line', () => {
+      window.location.hash = 'ABC_123';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededInline(
+        {},
+        {
+          lineCode: 'ABC_456',
+        },
+      );
+      scrollToLineIfNeededInline({}, lineMock);
+      scrollToLineIfNeededInline(
+        {},
+        {
+          lineCode: 'XYZ_456',
+        },
+      );
+
+      expect(handleLocationHashSpy).toHaveBeenCalled();
+      expect(handleLocationHashSpy).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('scrollToLineIfNeededParallel', () => {
+    const lineMock = {
+      left: null,
+      right: {
+        lineCode: 'ABC_123',
+      },
+    };
+
+    it('should not call handleLocationHash when there is not hash', () => {
+      window.location.hash = '';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededParallel({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should not call handleLocationHash when the hash does not match any line', () => {
+      window.location.hash = 'XYZ_456';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededParallel({}, lineMock);
+
+      expect(handleLocationHashSpy).not.toHaveBeenCalled();
+    });
+
+    it('should call handleLocationHash only when the hash matches a line', () => {
+      window.location.hash = 'ABC_123';
+
+      const handleLocationHashSpy = spyOnDependency(actions, 'handleLocationHash').and.stub();
+
+      scrollToLineIfNeededParallel(
+        {},
+        {
+          left: null,
+          right: {
+            lineCode: 'ABC_456',
+          },
+        },
+      );
+      scrollToLineIfNeededParallel({}, lineMock);
+      scrollToLineIfNeededParallel(
+        {},
+        {
+          left: null,
+          right: {
+            lineCode: 'XYZ_456',
+          },
+        },
+      );
+
+      expect(handleLocationHashSpy).toHaveBeenCalled();
+      expect(handleLocationHashSpy).toHaveBeenCalledTimes(1);
+    });
+  });
 });