diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 0f44eb06cb3d7b7c46f5831b8d47015181e45f9e..d8812de12d48a3bc6ea4ba76124928598875286d 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -1,6 +1,9 @@ import { __, s__ } from '~/locale'; export const GENERIC_ERROR = __('Something went wrong on our end. Please try again!'); +export const LOAD_SINGLE_DIFF_FAILED = s__( + 'MergeRequest|Encountered an issue while trying to fetch the single file diff.', +); export const DIFF_FILE_HEADER = { optionsDropdownTitle: __('Options'), diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 29b77431f7db68fac07fa87c5136ed53c1ceca81..6c461021f865878cbdaea0ad89b29b413b7b2e1a 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -50,6 +50,7 @@ import { TRACKING_SINGLE_FILE_MODE, TRACKING_MULTIPLE_FILES_MODE, } from '../constants'; +import { LOAD_SINGLE_DIFF_FAILED } from '../i18n'; import eventHub from '../event_hub'; import { isCollapsed } from '../utils/diff_file'; import { markFileReview, setReviewsForMergeRequest } from '../utils/file_reviews'; @@ -590,6 +591,31 @@ export const setCurrentFileHash = ({ commit }, hash) => { commit(types.SET_CURRENT_DIFF_FILE, hash); }; +export const goToFile = ({ state, commit, dispatch, getters }, { path }) => { + if (!state.viewDiffsFileByFile) { + dispatch('scrollToFile', { path }); + } else { + if (!state.treeEntries[path]) return; + + const { fileHash } = state.treeEntries[path]; + + commit(types.SET_CURRENT_DIFF_FILE, fileHash); + + if (!getters.isTreePathLoaded(path)) { + document.location.hash = fileHash; + dispatch('fetchFileByFile') + .then(() => { + dispatch('scrollToFile', { path }); + }) + .catch(() => { + createAlert({ + message: LOAD_SINGLE_DIFF_FAILED, + }); + }); + } + } +}; + export const scrollToFile = ({ state, commit, getters }, { path }) => { if (!state.treeEntries[path]) return; diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 75ce826e1be683c41677e393b69687d3ef31ba38..f86c294a03ab93478b96d146bb9c6c3cdd4c788f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27265,6 +27265,9 @@ msgstr "" msgid "MergeRequest|Compare %{target} and %{source}" msgstr "" +msgid "MergeRequest|Encountered an issue while trying to fetch the single file diff." +msgstr "" + msgid "MergeRequest|Error dismissing suggestion popover. Please try again." msgstr "" diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 8ef41cc83f2e2cfa3558539b44c01f07d226ae32..351e62fb6e009a16bf03aaf135fbc4b83ce924b1 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -10,6 +10,7 @@ import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE, } from '~/diffs/constants'; +import { LOAD_SINGLE_DIFF_FAILED } from '~/diffs/i18n'; import * as diffActions from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; import * as utils from '~/diffs/store/utils'; @@ -994,6 +995,87 @@ describe('DiffsStoreActions', () => { }); }); + describe('goToFile', () => { + const getters = {}; + const file = { path: 'path' }; + const fileHash = 'test'; + let state; + let dispatch; + let commit; + + beforeEach(() => { + getters.isTreePathLoaded = () => false; + state = { + viewDiffsFileByFile: true, + treeEntries: { + path: { + fileHash, + }, + }, + }; + commit = jest.fn(); + dispatch = jest.fn().mockResolvedValue(); + }); + + it('immediately defers to scrollToFile if the app is not in file-by-file mode', () => { + state.viewDiffsFileByFile = false; + + diffActions.goToFile({ state, dispatch }, file); + + expect(dispatch).toHaveBeenCalledWith('scrollToFile', file); + }); + + describe('when the app is in fileByFile mode', () => { + it('commits SET_CURRENT_DIFF_FILE', () => { + diffActions.goToFile({ state, commit, dispatch, getters }, file); + + expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, fileHash); + }); + + it('does nothing more if the path has already been loaded', () => { + getters.isTreePathLoaded = () => true; + + diffActions.goToFile({ state, dispatch, getters, commit }, file); + + expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, fileHash); + expect(dispatch).toHaveBeenCalledTimes(0); + }); + + describe('when the tree entry has not been loaded', () => { + it('updates location hash', () => { + diffActions.goToFile({ state, commit, getters, dispatch }, file); + + expect(document.location.hash).toBe('#test'); + }); + + it('loads the file and then scrolls to it', async () => { + diffActions.goToFile({ state, commit, getters, dispatch }, file); + + // Wait for the fetchFileByFile dispatch to return, to trigger scrollToFile + await waitForPromises(); + + expect(dispatch).toHaveBeenCalledWith('fetchFileByFile'); + expect(dispatch).toHaveBeenCalledWith('scrollToFile', file); + expect(dispatch).toHaveBeenCalledTimes(2); + }); + + it('shows an alert when there was an error fetching the file', async () => { + dispatch = jest.fn().mockRejectedValue(); + + diffActions.goToFile({ state, commit, getters, dispatch }, file); + + // Wait for the fetchFileByFile dispatch to return, to trigger the catch + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledTimes(1); + expect(createAlert).toHaveBeenCalledWith({ + message: expect.stringMatching(LOAD_SINGLE_DIFF_FAILED), + }); + }); + }); + }); + }); + describe('scrollToFile', () => { let commit; const getters = { isVirtualScrollingEnabled: false };