diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 9236e14beb195b372208cb0bb87ea60ba3fcc839..9c5f2043933d77cfe487def5a9c9311f813b645c 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -62,6 +62,8 @@ import { idleCallback, allDiscussionWrappersExpanded, prepareLineForRenamedFile, + parseUrlHashAsFileHash, + isUrlHashNoteLink, } from './utils'; export const setBaseConfig = ({ commit }, options) => { @@ -101,6 +103,47 @@ export const setBaseConfig = ({ commit }, options) => { }); }; +export const fetchFileByFile = async ({ state, getters, commit }) => { + const isNoteLink = isUrlHashNoteLink(window?.location?.hash); + const id = parseUrlHashAsFileHash(window?.location?.hash, state.currentDiffFileId); + const treeEntry = id + ? getters.flatBlobsList.find(({ fileHash }) => fileHash === id) + : getters.flatBlobsList[0]; + + if (treeEntry && !treeEntry.diffLoaded && !getters.getDiffFileByHash(id)) { + // Overloading "batch" loading indicators so the UI stays mostly the same + commit(types.SET_BATCH_LOADING_STATE, 'loading'); + commit(types.SET_RETRIEVING_BATCHES, true); + + const urlParams = { + old_path: treeEntry.filePaths.old, + new_path: treeEntry.filePaths.new, + w: state.showWhitespace ? '0' : '1', + view: 'inline', + }; + + axios + .get(mergeUrlParams({ ...urlParams }, state.endpointDiffForPath)) + .then(({ data: diffData }) => { + commit(types.SET_DIFF_DATA_BATCH, { diff_files: diffData.diff_files }); + + if (!isNoteLink && !state.currentDiffFileId) { + commit(types.SET_CURRENT_DIFF_FILE, state.diffFiles[0]?.file_hash || ''); + } + + commit(types.SET_BATCH_LOADING_STATE, 'loaded'); + + eventHub.$emit('diffFilesModified'); + }) + .catch(() => { + commit(types.SET_BATCH_LOADING_STATE, 'error'); + }) + .finally(() => { + commit(types.SET_RETRIEVING_BATCHES, false); + }); + } +}; + export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { let perPage = state.viewDiffsFileByFile ? 1 : 5; let increaseAmount = 1.4; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 3739ef0cd55c8a6e9111569d10bf10ef4f68b5d5..4ca353333b7ade4c73b2b66d35a269840d23786e 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -559,19 +559,19 @@ export const allDiscussionWrappersExpanded = (diff) => { return discussionsExpanded; }; -export function isUrlHashNoteLink(urlHash) { +export function isUrlHashNoteLink(urlHash = '') { const id = urlHash.replace(/^#/, ''); return id.startsWith('note'); } -export function isUrlHashFileHeader(urlHash) { +export function isUrlHashFileHeader(urlHash = '') { const id = urlHash.replace(/^#/, ''); return id.startsWith('diff-content'); } -export function parseUrlHashAsFileHash(urlHash, currentDiffFileId = '') { +export function parseUrlHashAsFileHash(urlHash = '', currentDiffFileId = '') { const isNoteLink = isUrlHashNoteLink(urlHash); let id = urlHash.replace(/^#/, ''); diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index b00076504e3b34bfe5c156c3f52ce0d4213c6168..3067f3791c530dea8aa7cdfb959cc70054c46a56 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -1,5 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; import Cookies from '~/lib/utils/cookies'; +import waitForPromises from 'helpers/wait_for_promises'; import { useLocalStorageSpy } from 'helpers/local_storage_helper'; import { TEST_HOST } from 'helpers/test_constants'; import testAction from 'helpers/vuex_action_helper'; @@ -24,6 +25,7 @@ import { } from '~/lib/utils/http_status'; import { mergeUrlParams } from '~/lib/utils/url_utility'; import eventHub from '~/notes/event_hub'; +import diffsEventHub from '~/diffs/event_hub'; import { diffMetadata } from '../mock_data/diff_metadata'; jest.mock('~/alert'); @@ -135,6 +137,112 @@ describe('DiffsStoreActions', () => { }); }); + describe('fetchFileByFile', () => { + beforeEach(() => { + window.location.hash = 'e334a2a10f036c00151a04cea7938a5d4213a818'; + }); + + it('should do nothing if there is no tree entry for the file ID', () => { + return testAction(diffActions.fetchFileByFile, {}, { flatBlobsList: [] }, [], []); + }); + + it('should do nothing if the tree entry for the file ID has already been marked as loaded', () => { + return testAction( + diffActions.fetchFileByFile, + {}, + { + flatBlobsList: [ + { fileHash: 'e334a2a10f036c00151a04cea7938a5d4213a818', diffLoaded: true }, + ], + }, + [], + [], + ); + }); + + describe('when a tree entry exists for the file, but it has not been marked as loaded', () => { + let state; + let commit; + let hubSpy; + const endpointDiffForPath = '/diffs/set/endpoint/path'; + const diffForPath = mergeUrlParams( + { + old_path: 'old/123', + new_path: 'new/123', + w: '1', + view: 'inline', + }, + endpointDiffForPath, + ); + const treeEntry = { + fileHash: 'e334a2a10f036c00151a04cea7938a5d4213a818', + filePaths: { old: 'old/123', new: 'new/123' }, + }; + const fileResult = { + diff_files: [{ file_hash: 'e334a2a10f036c00151a04cea7938a5d4213a818' }], + }; + const getters = { + flatBlobsList: [treeEntry], + getDiffFileByHash(hash) { + return state.diffFiles?.find((entry) => entry.file_hash === hash); + }, + }; + + beforeEach(() => { + commit = jest.fn(); + state = { + endpointDiffForPath, + diffFiles: [], + }; + getters.flatBlobsList = [treeEntry]; + hubSpy = jest.spyOn(diffsEventHub, '$emit'); + }); + + it('does nothing if the file already exists in the loaded diff files', () => { + state.diffFiles = fileResult.diff_files; + + return testAction(diffActions.fetchFileByFile, state, getters, [], []); + }); + + it('does some standard work every time', async () => { + mock.onGet(diffForPath).reply(HTTP_STATUS_OK, fileResult); + + await diffActions.fetchFileByFile({ state, getters, commit }); + + expect(commit).toHaveBeenCalledWith(types.SET_BATCH_LOADING_STATE, 'loading'); + expect(commit).toHaveBeenCalledWith(types.SET_RETRIEVING_BATCHES, true); + + // wait for the mocked network request to return and start processing the .then + await waitForPromises(); + + expect(commit).toHaveBeenCalledWith(types.SET_DIFF_DATA_BATCH, fileResult); + expect(commit).toHaveBeenCalledWith(types.SET_BATCH_LOADING_STATE, 'loaded'); + + expect(hubSpy).toHaveBeenCalledWith('diffFilesModified'); + }); + + it.each` + urlHash | diffFiles | expected + ${treeEntry.fileHash} | ${[]} | ${''} + ${'abcdef1234567890'} | ${fileResult.diff_files} | ${'e334a2a10f036c00151a04cea7938a5d4213a818'} + `( + "sets the current file to the first diff file ('$id') if it's not a note hash and there isn't a current ID set", + async ({ urlHash, diffFiles, expected }) => { + window.location.hash = urlHash; + mock.onGet(diffForPath).reply(HTTP_STATUS_OK, fileResult); + state.diffFiles = diffFiles; + + await diffActions.fetchFileByFile({ state, getters, commit }); + + // wait for the mocked network request to return and start processing the .then + await waitForPromises(); + + expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, expected); + }, + ); + }); + }); + describe('fetchDiffFilesBatch', () => { it('should fetch batch diff files', () => { const endpointBatch = '/fetch/diffs_batch';