diff options
Diffstat (limited to 'spec/frontend/diffs/store/actions_spec.js')
| -rw-r--r-- | spec/frontend/diffs/store/actions_spec.js | 499 |
1 files changed, 435 insertions, 64 deletions
diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 78765204322..f883aea764f 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'; @@ -8,12 +9,14 @@ import { DIFF_VIEW_COOKIE_NAME, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE, + EVT_MR_PREPARED, } 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'; import * as treeWorkerUtils from '~/diffs/utils/tree_worker_utils'; -import { createAlert } from '~/flash'; +import { createAlert } from '~/alert'; import axios from '~/lib/utils/axios_utils'; import * as commonUtils from '~/lib/utils/common_utils'; import { @@ -24,9 +27,15 @@ 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('~/flash'); +jest.mock('~/alert'); + +jest.mock('~/lib/utils/secret_detection', () => ({ + confirmSensitiveAction: jest.fn(() => Promise.resolve(false)), + containsSensitiveToken: jest.requireActual('~/lib/utils/secret_detection').containsSensitiveToken, +})); describe('DiffsStoreActions', () => { let mock; @@ -69,6 +78,7 @@ describe('DiffsStoreActions', () => { const endpoint = '/diffs/set/endpoint'; const endpointMetadata = '/diffs/set/endpoint/metadata'; const endpointBatch = '/diffs/set/endpoint/batch'; + const endpointDiffForPath = '/diffs/set/endpoint/path'; const endpointCoverage = '/diffs/set/coverage_reports'; const projectPath = '/root/project'; const dismissEndpoint = '/-/user_callouts'; @@ -83,6 +93,7 @@ describe('DiffsStoreActions', () => { { endpoint, endpointBatch, + endpointDiffForPath, endpointMetadata, endpointCoverage, projectPath, @@ -93,6 +104,7 @@ describe('DiffsStoreActions', () => { { endpoint: '', endpointBatch: '', + endpointDiffForPath: '', endpointMetadata: '', endpointCoverage: '', projectPath: '', @@ -106,6 +118,7 @@ describe('DiffsStoreActions', () => { endpoint, endpointMetadata, endpointBatch, + endpointDiffForPath, endpointCoverage, projectPath, dismissEndpoint, @@ -131,6 +144,177 @@ 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 getters; + let commit; + let hubSpy; + const defaultParams = { + old_path: 'old/123', + new_path: 'new/123', + w: '1', + view: 'inline', + }; + const endpointDiffForPath = '/diffs/set/endpoint/path'; + const diffForPath = mergeUrlParams(defaultParams, endpointDiffForPath); + const treeEntry = { + fileHash: 'e334a2a10f036c00151a04cea7938a5d4213a818', + filePaths: { old: 'old/123', new: 'new/123' }, + }; + const fileResult = { + diff_files: [{ file_hash: 'e334a2a10f036c00151a04cea7938a5d4213a818' }], + }; + + beforeEach(() => { + commit = jest.fn(); + state = { + endpointDiffForPath, + diffFiles: [], + }; + getters = { + flatBlobsList: [treeEntry], + getDiffFileByHash(hash) { + return state.diffFiles?.find((entry) => entry.file_hash === hash); + }, + }; + 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); + }, + ); + + it('should fetch data without commit ID', async () => { + getters.commitId = null; + mock.onGet(diffForPath).reply(HTTP_STATUS_OK, fileResult); + + await diffActions.fetchFileByFile({ state, getters, commit }); + + // wait for the mocked network request to return and start processing the .then + await waitForPromises(); + + // This tests that commit_id is NOT added, if there isn't one in the store + expect(mock.history.get[0].url).toEqual(diffForPath); + }); + + it('should fetch data with commit ID', async () => { + const finalPath = mergeUrlParams( + { ...defaultParams, commit_id: '123' }, + endpointDiffForPath, + ); + + getters.commitId = '123'; + mock.onGet(finalPath).reply(HTTP_STATUS_OK, fileResult); + + await diffActions.fetchFileByFile({ state, getters, commit }); + + // wait for the mocked network request to return and start processing the .then + await waitForPromises(); + + expect(mock.history.get[0].url).toEqual(finalPath); + }); + + describe('version parameters', () => { + const diffId = '4'; + const startSha = 'abc'; + const pathRoot = 'a/a/-/merge_requests/1'; + + it('fetches the data when there is no mergeRequestDiff', async () => { + diffActions.fetchFileByFile({ state, getters, commit }); + + // wait for the mocked network request to return and start processing the .then + await waitForPromises(); + + expect(mock.history.get[0].url).toEqual(diffForPath); + }); + + it.each` + desc | versionPath | start_sha | diff_id + ${'no additional version information'} | ${`${pathRoot}?search=terms`} | ${undefined} | ${undefined} + ${'the diff_id'} | ${`${pathRoot}?diff_id=${diffId}`} | ${undefined} | ${diffId} + ${'the start_sha'} | ${`${pathRoot}?start_sha=${startSha}`} | ${startSha} | ${undefined} + ${'all available version information'} | ${`${pathRoot}?diff_id=${diffId}&start_sha=${startSha}`} | ${startSha} | ${diffId} + `('fetches the data and includes $desc', async ({ versionPath, start_sha, diff_id }) => { + const finalPath = mergeUrlParams( + { ...defaultParams, diff_id, start_sha }, + endpointDiffForPath, + ); + state.mergeRequestDiff = { version_path: versionPath }; + mock.onGet(finalPath).reply(HTTP_STATUS_OK, fileResult); + + diffActions.fetchFileByFile({ state, getters, commit }); + + // wait for the mocked network request to return and start processing the .then + await waitForPromises(); + + expect(mock.history.get[0].url).toEqual(finalPath); + }); + }); + }); + }); + describe('fetchDiffFilesBatch', () => { it('should fetch batch diff files', () => { const endpointBatch = '/fetch/diffs_batch'; @@ -213,36 +397,63 @@ describe('DiffsStoreActions', () => { ); }); - it('should show a warning on 404 reponse', async () => { - mock.onGet(endpointMetadata).reply(HTTP_STATUS_NOT_FOUND); + describe('on a 404 response', () => { + let dismissAlert; - await testAction( - diffActions.fetchDiffFilesMeta, - {}, - { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, - [{ type: types.SET_LOADING, payload: true }], - [], - ); + beforeAll(() => { + dismissAlert = jest.fn(); - expect(createAlert).toHaveBeenCalledTimes(1); - expect(createAlert).toHaveBeenCalledWith({ - message: expect.stringMatching( - 'Building your merge request. Wait a few moments, then refresh this page.', - ), - variant: 'warning', + mock.onGet(endpointMetadata).reply(HTTP_STATUS_NOT_FOUND); + createAlert.mockImplementation(() => ({ dismiss: dismissAlert })); + }); + + it('should show a warning', async () => { + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + + expect(createAlert).toHaveBeenCalledTimes(1); + expect(createAlert).toHaveBeenCalledWith({ + message: expect.stringMatching( + 'Building your merge request… This page will update when the build is complete.', + ), + variant: 'warning', + }); + }); + + it("should attempt to close the alert if the MR reports that it's been prepared", async () => { + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + + diffsEventHub.$emit(EVT_MR_PREPARED); + + expect(dismissAlert).toHaveBeenCalled(); }); }); it('should show no warning on any other status code', async () => { mock.onGet(endpointMetadata).reply(HTTP_STATUS_INTERNAL_SERVER_ERROR); - await testAction( - diffActions.fetchDiffFilesMeta, - {}, - { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, - [{ type: types.SET_LOADING, payload: true }], - [], - ); + try { + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + } catch (error) { + expect(error.response.status).toBe(HTTP_STATUS_INTERNAL_SERVER_ERROR); + } expect(createAlert).not.toHaveBeenCalled(); }); @@ -265,7 +476,7 @@ describe('DiffsStoreActions', () => { ); }); - it('should show flash on API error', async () => { + it('should show alert on API error', async () => { mock.onGet(endpointCoverage).reply(HTTP_STATUS_BAD_REQUEST); await testAction(diffActions.fetchCoverageFiles, {}, { endpointCoverage }, [], []); @@ -389,7 +600,7 @@ describe('DiffsStoreActions', () => { return testAction( diffActions.assignDiscussionsToDiff, [], - { diffFiles: [] }, + { diffFiles: [], flatBlobsList: [] }, [], [{ type: 'setCurrentDiffFileIdFromNote', payload: '123' }], ); @@ -810,31 +1021,32 @@ describe('DiffsStoreActions', () => { }); describe('saveDiffDiscussion', () => { - it('dispatches actions', () => { - const commitId = 'something'; - const formData = { - diffFile: getDiffFileMock(), - noteableData: {}, - }; - const note = {}; - const state = { - commit: { - id: commitId, - }, - }; - const dispatch = jest.fn((name) => { - switch (name) { - case 'saveNote': - return Promise.resolve({ - discussion: 'test', - }); - case 'updateDiscussion': - return Promise.resolve('discussion'); - default: - return Promise.resolve({}); - } - }); + const dispatch = jest.fn((name) => { + switch (name) { + case 'saveNote': + return Promise.resolve({ + discussion: 'test', + }); + case 'updateDiscussion': + return Promise.resolve('discussion'); + default: + return Promise.resolve({}); + } + }); + + const commitId = 'something'; + const formData = { + diffFile: getDiffFileMock(), + noteableData: {}, + }; + const note = {}; + const state = { + commit: { + id: commitId, + }, + }; + it('dispatches actions', () => { return diffActions.saveDiffDiscussion({ state, dispatch }, { note, formData }).then(() => { expect(dispatch).toHaveBeenCalledTimes(5); expect(dispatch).toHaveBeenNthCalledWith(1, 'saveNote', expect.any(Object), { @@ -848,6 +1060,16 @@ describe('DiffsStoreActions', () => { expect(dispatch).toHaveBeenNthCalledWith(3, 'assignDiscussionsToDiff', ['discussion']); }); }); + + it('should not add note with sensitive token', async () => { + const sensitiveMessage = 'token: glpat-1234567890abcdefghij'; + + await diffActions.saveDiffDiscussion( + { state, dispatch }, + { note: sensitiveMessage, formData }, + ); + expect(dispatch).not.toHaveBeenCalled(); + }); }); describe('toggleTreeOpen', () => { @@ -862,6 +1084,104 @@ 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', () => { + describe('when the singleFileFileByFile feature flag is enabled', () => { + it('commits SET_CURRENT_DIFF_FILE', () => { + diffActions.goToFile( + { state, commit, dispatch, getters }, + { path: file.path, singleFile: true }, + ); + + 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 }, + { path: file.path, singleFile: true }, + ); + + 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 }, + { path: file.path, singleFile: true }, + ); + + expect(document.location.hash).toBe('#test'); + }); + + it('loads the file and then scrolls to it', async () => { + diffActions.goToFile( + { state, commit, getters, dispatch }, + { path: file.path, singleFile: true }, + ); + + // 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 }, + { path: file.path, singleFile: true }, + ); + + // 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 }; @@ -1007,20 +1327,14 @@ describe('DiffsStoreActions', () => { describe('setShowWhitespace', () => { const endpointUpdateUser = 'user/prefs'; let putSpy; - let gon; beforeEach(() => { putSpy = jest.spyOn(axios, 'put'); - gon = window.gon; mock.onPut(endpointUpdateUser).reply(HTTP_STATUS_OK, {}); jest.spyOn(eventHub, '$emit').mockImplementation(); }); - afterEach(() => { - window.gon = gon; - }); - it('commits SET_SHOW_WHITESPACE', () => { return testAction( diffActions.setShowWhitespace, @@ -1390,42 +1704,89 @@ describe('DiffsStoreActions', () => { ); }); + describe('rereadNoteHash', () => { + beforeEach(() => { + window.location.hash = 'note_123'; + }); + + it('dispatches setCurrentDiffFileIdFromNote if the hash is a note URL', () => { + window.location.hash = 'note_123'; + + return testAction( + diffActions.rereadNoteHash, + {}, + {}, + [], + [{ type: 'setCurrentDiffFileIdFromNote', payload: '123' }], + ); + }); + + it('dispatches fetchFileByFile if the app is in fileByFile mode', () => { + window.location.hash = 'note_123'; + + return testAction( + diffActions.rereadNoteHash, + {}, + { viewDiffsFileByFile: true }, + [], + [{ type: 'setCurrentDiffFileIdFromNote', payload: '123' }, { type: 'fetchFileByFile' }], + ); + }); + + it('does not try to fetch the diff file if the app is not in fileByFile mode', () => { + window.location.hash = 'note_123'; + + return testAction( + diffActions.rereadNoteHash, + {}, + { viewDiffsFileByFile: false }, + [], + [{ type: 'setCurrentDiffFileIdFromNote', payload: '123' }], + ); + }); + + it('does nothing if the hash is not a note URL', () => { + window.location.hash = 'abcdef1234567890'; + + return testAction(diffActions.rereadNoteHash, {}, {}, [], []); + }); + }); + describe('setCurrentDiffFileIdFromNote', () => { it('commits SET_CURRENT_DIFF_FILE', () => { const commit = jest.fn(); - const state = { diffFiles: [{ file_hash: '123' }] }; + const getters = { flatBlobsList: [{ fileHash: '123' }] }; const rootGetters = { getDiscussion: () => ({ diff_file: { file_hash: '123' } }), notesById: { 1: { discussion_id: '2' } }, }; - diffActions.setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); + diffActions.setCurrentDiffFileIdFromNote({ commit, getters, rootGetters }, '1'); expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, '123'); }); it('does not commit SET_CURRENT_DIFF_FILE when discussion has no diff_file', () => { const commit = jest.fn(); - const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { getDiscussion: () => ({ id: '1' }), notesById: { 1: { discussion_id: '2' } }, }; - diffActions.setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); + diffActions.setCurrentDiffFileIdFromNote({ commit, rootGetters }, '1'); expect(commit).not.toHaveBeenCalled(); }); it('does not commit SET_CURRENT_DIFF_FILE when diff file does not exist', () => { const commit = jest.fn(); - const state = { diffFiles: [{ file_hash: '123' }] }; + const getters = { flatBlobsList: [{ fileHash: '123' }] }; const rootGetters = { getDiscussion: () => ({ diff_file: { file_hash: '124' } }), notesById: { 1: { discussion_id: '2' } }, }; - diffActions.setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); + diffActions.setCurrentDiffFileIdFromNote({ commit, getters, rootGetters }, '1'); expect(commit).not.toHaveBeenCalled(); }); @@ -1435,12 +1796,22 @@ describe('DiffsStoreActions', () => { it('commits SET_CURRENT_DIFF_FILE', () => { return testAction( diffActions.navigateToDiffFileIndex, - 0, - { diffFiles: [{ file_hash: '123' }] }, + { index: 0, singleFile: false }, + { flatBlobsList: [{ fileHash: '123' }] }, [{ type: types.SET_CURRENT_DIFF_FILE, payload: '123' }], [], ); }); + + it('dispatches the fetchFileByFile action when the state value viewDiffsFileByFile is true and the single-file file-by-file feature flag is enabled', () => { + return testAction( + diffActions.navigateToDiffFileIndex, + { index: 0, singleFile: true }, + { viewDiffsFileByFile: true, flatBlobsList: [{ fileHash: '123' }] }, + [{ type: types.SET_CURRENT_DIFF_FILE, payload: '123' }], + [{ type: 'fetchFileByFile' }], + ); + }); }); describe('setFileByFile', () => { |