From 9f46488805e86b1bc341ea1620b866016c2ce5ed Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 20 May 2020 14:34:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-0-stable-ee --- spec/frontend/diffs/components/app_spec.js | 212 ++++++++++++++++----- spec/frontend/diffs/components/commit_item_spec.js | 144 +++++++++++++- .../frontend/diffs/components/diff_content_spec.js | 2 +- .../diffs/components/diff_discussions_spec.js | 2 +- .../diffs/components/diff_expansion_cell_spec.js | 2 +- .../diffs/components/diff_gutter_avatars_spec.js | 2 +- .../diffs/components/diff_line_note_form_spec.js | 2 +- spec/frontend/diffs/components/edit_button_spec.js | 19 +- .../components/inline_diff_expansion_row_spec.js | 2 +- .../diffs/components/inline_diff_view_spec.js | 4 +- .../components/parallel_diff_expansion_row_spec.js | 2 +- .../diffs/components/parallel_diff_view_spec.js | 2 +- spec/frontend/diffs/store/actions_spec.js | 184 +++++++++++++++++- spec/frontend/diffs/store/getters_spec.js | 4 +- .../diffs/store/getters_versions_dropdowns_spec.js | 9 - spec/frontend/diffs/store/mutations_spec.js | 30 +++ spec/frontend/diffs/store/utils_spec.js | 84 +++++++- 17 files changed, 623 insertions(+), 83 deletions(-) (limited to 'spec/frontend/diffs') diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 3a0354205f8..57e3a93c6f4 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -14,10 +14,13 @@ import TreeList from '~/diffs/components/tree_list.vue'; import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants'; import createDiffsStore from '../create_diffs_store'; import axios from '~/lib/utils/axios_utils'; +import * as urlUtils from '~/lib/utils/url_utility'; import diffsMockData from '../mock_data/merge_request_diffs'; const mergeRequestDiff = { version_index: 1 }; const TEST_ENDPOINT = `${TEST_HOST}/diff/endpoint`; +const COMMIT_URL = '[BASE URL]/OLD'; +const UPDATED_COMMIT_URL = '[BASE URL]/NEW'; describe('diffs/components/app', () => { const oldMrTabs = window.mrTabs; @@ -25,8 +28,14 @@ describe('diffs/components/app', () => { let wrapper; let mock; - function createComponent(props = {}, extendStore = () => {}) { + function createComponent(props = {}, extendStore = () => {}, provisions = {}) { const localVue = createLocalVue(); + const provide = { + ...provisions, + glFeatures: { + ...(provisions.glFeatures || {}), + }, + }; localVue.use(Vuex); @@ -49,6 +58,7 @@ describe('diffs/components/app', () => { showSuggestPopover: true, ...props, }, + provide, store, methods: { isLatestVersion() { @@ -79,7 +89,10 @@ describe('diffs/components/app', () => { window.mrTabs = oldMrTabs; // reset component - wrapper.destroy(); + if (wrapper) { + wrapper.destroy(); + wrapper = null; + } mock.restore(); }); @@ -452,76 +465,109 @@ describe('diffs/components/app', () => { }); describe('keyboard shortcut navigation', () => { - const mappings = { - '[': -1, - k: -1, - ']': +1, - j: +1, - }; - let spy; + let spies = []; + let jumpSpy; + let moveSpy; + + function setup(componentProps, featureFlags) { + createComponent( + componentProps, + ({ state }) => { + state.diffs.commit = { id: 'SHA123' }; + }, + { glFeatures: { mrCommitNeighborNav: true, ...featureFlags } }, + ); + + moveSpy = jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + jumpSpy = jest.fn(); + spies = [jumpSpy, moveSpy]; + wrapper.setMethods({ + jumpToFile: jumpSpy, + }); + } describe('visible app', () => { - beforeEach(() => { - spy = jest.fn(); + it.each` + key | name | spy | args | featureFlags + ${'['} | ${'jumpToFile'} | ${0} | ${[-1]} | ${{}} + ${'k'} | ${'jumpToFile'} | ${0} | ${[-1]} | ${{}} + ${']'} | ${'jumpToFile'} | ${0} | ${[+1]} | ${{}} + ${'j'} | ${'jumpToFile'} | ${0} | ${[+1]} | ${{}} + ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'previous' }]} | ${{ mrCommitNeighborNav: true }} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'next' }]} | ${{ mrCommitNeighborNav: true }} + `( + 'calls `$name()` with correct parameters whenever the "$key" key is pressed', + ({ key, spy, args, featureFlags }) => { + setup({ shouldShow: true }, featureFlags); - createComponent({ - shouldShow: true, - }); - wrapper.setMethods({ - jumpToFile: spy, - }); - }); + return wrapper.vm.$nextTick().then(() => { + expect(spies[spy]).not.toHaveBeenCalled(); + + Mousetrap.trigger(key); + + expect(spies[spy]).toHaveBeenCalledWith(...args); + }); + }, + ); + + it.each` + key | name | spy | featureFlags + ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${{ mrCommitNeighborNav: false }} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${{ mrCommitNeighborNav: false }} + `( + 'does not call `$name()` even when the correct key is pressed if the feature flag is disabled', + ({ key, spy, featureFlags }) => { + setup({ shouldShow: true }, featureFlags); - it.each(Object.keys(mappings))( - 'calls `jumpToFile()` with correct parameter whenever pre-defined %s is pressed', - key => { return wrapper.vm.$nextTick().then(() => { - expect(spy).not.toHaveBeenCalled(); + expect(spies[spy]).not.toHaveBeenCalled(); Mousetrap.trigger(key); - expect(spy).toHaveBeenCalledWith(mappings[key]); + expect(spies[spy]).not.toHaveBeenCalled(); }); }, ); - it('does not call `jumpToFile()` when unknown key is pressed', done => { - wrapper.vm - .$nextTick() - .then(() => { - Mousetrap.trigger('d'); + it.each` + key | name | spy | allowed + ${'d'} | ${'jumpToFile'} | ${0} | ${['[', ']', 'j', 'k']} + ${'r'} | ${'moveToNeighboringCommit'} | ${1} | ${['x', 'c']} + `( + `does not call \`$name()\` when a key that is not one of \`$allowed\` is pressed`, + ({ key, spy }) => { + setup({ shouldShow: true }, { mrCommitNeighborNav: true }); - expect(spy).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); + return wrapper.vm.$nextTick().then(() => { + Mousetrap.trigger(key); + + expect(spies[spy]).not.toHaveBeenCalled(); + }); + }, + ); }); - describe('hideen app', () => { + describe('hidden app', () => { beforeEach(() => { - spy = jest.fn(); + setup({ shouldShow: false }, { mrCommitNeighborNav: true }); - createComponent({ - shouldShow: false, - }); - wrapper.setMethods({ - jumpToFile: spy, + return wrapper.vm.$nextTick().then(() => { + Mousetrap.reset(); }); }); - it('stops calling `jumpToFile()` when application is hidden', done => { - wrapper.vm - .$nextTick() - .then(() => { - Object.keys(mappings).forEach(key => { - Mousetrap.trigger(key); + it.each` + key | name | spy + ${'['} | ${'jumpToFile'} | ${0} + ${'k'} | ${'jumpToFile'} | ${0} + ${']'} | ${'jumpToFile'} | ${0} + ${'j'} | ${'jumpToFile'} | ${0} + ${'x'} | ${'moveToNeighboringCommit'} | ${1} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} + `('stops calling `$name()` when the app is hidden', ({ key, spy }) => { + Mousetrap.trigger(key); - expect(spy).not.toHaveBeenCalled(); - }); - }) - .then(done) - .catch(done.fail); + expect(spies[spy]).not.toHaveBeenCalled(); }); }); }); @@ -602,6 +648,70 @@ describe('diffs/components/app', () => { }); }); + describe('commit watcher', () => { + const spy = () => { + jest.spyOn(wrapper.vm, 'refetchDiffData').mockImplementation(() => {}); + jest.spyOn(wrapper.vm, 'adjustView').mockImplementation(() => {}); + }; + let location; + + beforeAll(() => { + location = window.location; + delete window.location; + window.location = COMMIT_URL; + document.title = 'My Title'; + }); + + beforeEach(() => { + jest.spyOn(urlUtils, 'updateHistory'); + }); + + afterAll(() => { + window.location = location; + }); + + it('when the commit changes and the app is not loading it should update the history, refetch the diff data, and update the view', () => { + createComponent({}, ({ state }) => { + state.diffs.commit = { ...state.diffs.commit, id: 'OLD' }; + }); + spy(); + + store.state.diffs.commit = { id: 'NEW' }; + + return wrapper.vm.$nextTick().then(() => { + expect(urlUtils.updateHistory).toHaveBeenCalledWith({ + title: document.title, + url: UPDATED_COMMIT_URL, + }); + expect(wrapper.vm.refetchDiffData).toHaveBeenCalled(); + expect(wrapper.vm.adjustView).toHaveBeenCalled(); + }); + }); + + it.each` + isLoading | oldSha | newSha + ${true} | ${'OLD'} | ${'NEW'} + ${false} | ${'NEW'} | ${'NEW'} + `( + 'given `{ "isLoading": $isLoading, "oldSha": "$oldSha", "newSha": "$newSha" }`, nothing should happen', + ({ isLoading, oldSha, newSha }) => { + createComponent({}, ({ state }) => { + state.diffs.isLoading = isLoading; + state.diffs.commit = { ...state.diffs.commit, id: oldSha }; + }); + spy(); + + store.state.diffs.commit = { id: newSha }; + + return wrapper.vm.$nextTick().then(() => { + expect(urlUtils.updateHistory).not.toHaveBeenCalled(); + expect(wrapper.vm.refetchDiffData).not.toHaveBeenCalled(); + expect(wrapper.vm.adjustView).not.toHaveBeenCalled(); + }); + }, + ); + }); + describe('diffs', () => { it('should render compare versions component', () => { createComponent({}, ({ state }) => { diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index 6bb3a0dcf21..0df951d43a7 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -13,6 +13,8 @@ const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; const TEST_SIGNATURE_HTML = 'Legit commit'; const TEST_PIPELINE_STATUS_PATH = `${TEST_HOST}/pipeline/status`; +const NEXT_COMMIT_URL = `${TEST_HOST}/?commit_id=next`; +const PREV_COMMIT_URL = `${TEST_HOST}/?commit_id=prev`; describe('diffs/components/commit_item', () => { let wrapper; @@ -30,12 +32,24 @@ describe('diffs/components/commit_item', () => { const getCommitActionsElement = () => wrapper.find('.commit-actions'); const getCommitPipelineStatus = () => wrapper.find(CommitPipelineStatus); - const mountComponent = propsData => { + const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons'); + const getNextCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:last-child'); + const getPrevCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:first-child'); + + const mountComponent = (propsData, featureFlags = {}) => { wrapper = mount(Component, { propsData: { commit, ...propsData, }, + provide: { + glFeatures: { + mrCommitNeighborNav: true, + ...featureFlags, + }, + }, stubs: { CommitPipelineStatus: true, }, @@ -173,4 +187,132 @@ describe('diffs/components/commit_item', () => { expect(getCommitPipelineStatus().exists()).toBe(true); }); }); + + describe('without neighbor commits', () => { + beforeEach(() => { + mountComponent({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); + }); + + it('does not render any navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(false); + }); + }); + + describe('with neighbor commits', () => { + let mrCommit; + + beforeEach(() => { + mrCommit = { + ...commit, + next_commit_id: 'next', + prev_commit_id: 'prev', + }; + + mountComponent({ commit: mrCommit }); + }); + + it('renders the commit navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + mountComponent({ + commit: { ...mrCommit, next_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + mountComponent({ + commit: { ...mrCommit, prev_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + }); + + it('does not render the commit navigation buttons if the `mrCommitNeighborNav` feature flag is disabled', () => { + mountComponent({ commit: mrCommit }, { mrCommitNeighborNav: false }); + + expect(getCommitNavButtonsElement().exists()).toEqual(false); + }); + + describe('prev commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getPrevCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(PREV_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getPrevCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ + direction: 'previous', + }); + }); + }); + + it('renders a disabled button when there is no prev commit', () => { + mountComponent({ commit: { ...mrCommit, prev_commit_id: null } }); + + const button = getPrevCommitNavElement(); + + expect(button.element.tagName).toEqual('BUTTON'); + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + + describe('next commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getNextCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(NEXT_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getNextCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ direction: 'next' }); + }); + }); + + it('renders a disabled button when there is no next commit', () => { + mountComponent({ commit: { ...mrCommit, next_commit_id: null } }); + + const button = getNextCommitNavElement(); + + expect(button.element.tagName).toEqual('BUTTON'); + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + }); }); diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js index 979c67787f7..b78895f9e55 100644 --- a/spec/frontend/diffs/components/diff_content_spec.js +++ b/spec/frontend/diffs/components/diff_content_spec.js @@ -10,7 +10,7 @@ import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; import NoteForm from '~/notes/components/note_form.vue'; import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants'; -import diffFileMockData from '../../../javascripts/diffs/mock_data/diff_file'; +import diffFileMockData from '../mock_data/diff_file'; import { diffViewerModes } from '~/ide/constants'; const localVue = createLocalVue(); diff --git a/spec/frontend/diffs/components/diff_discussions_spec.js b/spec/frontend/diffs/components/diff_discussions_spec.js index ba5a4f96204..83becc7a20a 100644 --- a/spec/frontend/diffs/components/diff_discussions_spec.js +++ b/spec/frontend/diffs/components/diff_discussions_spec.js @@ -13,7 +13,7 @@ const localVue = createLocalVue(); describe('DiffDiscussions', () => { let store; let wrapper; - const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; + const getDiscussionsMockData = () => [{ ...discussionsMockData }]; const createComponent = props => { store = createStore(); diff --git a/spec/frontend/diffs/components/diff_expansion_cell_spec.js b/spec/frontend/diffs/components/diff_expansion_cell_spec.js index 31c6a4d5b60..0504f3933e0 100644 --- a/spec/frontend/diffs/components/diff_expansion_cell_spec.js +++ b/spec/frontend/diffs/components/diff_expansion_cell_spec.js @@ -81,7 +81,7 @@ describe('DiffExpansionCell', () => { isTop: false, isBottom: false, }; - const props = Object.assign({}, defaults, options); + const props = { ...defaults, ...options }; vm = createComponentWithStore(cmp, store, props).$mount(); }; diff --git a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js index 4d8345d494d..da18d8e7894 100644 --- a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js +++ b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js @@ -2,7 +2,7 @@ import { shallowMount } from '@vue/test-utils'; import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; import discussionsMockData from '../mock_data/diff_discussions'; -const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; +const getDiscussionsMockData = () => [{ ...discussionsMockData }]; describe('DiffGutterAvatars', () => { let wrapper; diff --git a/spec/frontend/diffs/components/diff_line_note_form_spec.js b/spec/frontend/diffs/components/diff_line_note_form_spec.js index 9b032d10fdc..3e0acd0dace 100644 --- a/spec/frontend/diffs/components/diff_line_note_form_spec.js +++ b/spec/frontend/diffs/components/diff_line_note_form_spec.js @@ -9,7 +9,7 @@ describe('DiffLineNoteForm', () => { let wrapper; let diffFile; let diffLines; - const getDiffFileMock = () => Object.assign({}, diffFileMockData); + const getDiffFileMock = () => ({ ...diffFileMockData }); beforeEach(() => { diffFile = getDiffFileMock(); diff --git a/spec/frontend/diffs/components/edit_button_spec.js b/spec/frontend/diffs/components/edit_button_spec.js index f9a1d4a84a8..71512c1c4af 100644 --- a/spec/frontend/diffs/components/edit_button_spec.js +++ b/spec/frontend/diffs/components/edit_button_spec.js @@ -1,4 +1,5 @@ import { shallowMount } from '@vue/test-utils'; +import { GlDeprecatedButton } from '@gitlab/ui'; import EditButton from '~/diffs/components/edit_button.vue'; const editPath = 'test-path'; @@ -22,7 +23,7 @@ describe('EditButton', () => { canCurrentUserFork: false, }); - expect(wrapper.attributes('href')).toBe(editPath); + expect(wrapper.find(GlDeprecatedButton).attributes('href')).toBe(editPath); }); it('emits a show fork message event if current user can fork', () => { @@ -30,7 +31,7 @@ describe('EditButton', () => { editPath, canCurrentUserFork: true, }); - wrapper.trigger('click'); + wrapper.find(GlDeprecatedButton).trigger('click'); return wrapper.vm.$nextTick().then(() => { expect(wrapper.emitted('showForkMessage')).toBeTruthy(); @@ -42,7 +43,7 @@ describe('EditButton', () => { editPath, canCurrentUserFork: false, }); - wrapper.trigger('click'); + wrapper.find(GlDeprecatedButton).trigger('click'); return wrapper.vm.$nextTick().then(() => { expect(wrapper.emitted('showForkMessage')).toBeFalsy(); @@ -55,10 +56,20 @@ describe('EditButton', () => { canCurrentUserFork: true, canModifyBlob: true, }); - wrapper.trigger('click'); + wrapper.find(GlDeprecatedButton).trigger('click'); return wrapper.vm.$nextTick().then(() => { expect(wrapper.emitted('showForkMessage')).toBeFalsy(); }); }); + + it('disables button if editPath is empty', () => { + createComponent({ + editPath: '', + canCurrentUserFork: true, + canModifyBlob: true, + }); + + expect(wrapper.find(GlDeprecatedButton).attributes('disabled')).toBe('true'); + }); }); diff --git a/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js b/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js index f423c3b111e..90f012fbafe 100644 --- a/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js +++ b/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js @@ -16,7 +16,7 @@ describe('InlineDiffExpansionRow', () => { isTop: false, isBottom: false, }; - const props = Object.assign({}, defaults, options); + const props = { ...defaults, ...options }; return createComponentWithStore(cmp, createStore(), props).$mount(); }; diff --git a/spec/frontend/diffs/components/inline_diff_view_spec.js b/spec/frontend/diffs/components/inline_diff_view_spec.js index a63c13fb271..9b0cf6a84d9 100644 --- a/spec/frontend/diffs/components/inline_diff_view_spec.js +++ b/spec/frontend/diffs/components/inline_diff_view_spec.js @@ -8,8 +8,8 @@ import discussionsMockData from '../mock_data/diff_discussions'; describe('InlineDiffView', () => { let component; - const getDiffFileMock = () => Object.assign({}, diffFileMockData); - const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; + const getDiffFileMock = () => ({ ...diffFileMockData }); + const getDiscussionsMockData = () => [{ ...discussionsMockData }]; const notesLength = getDiscussionsMockData()[0].notes.length; beforeEach(done => { diff --git a/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js b/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js index 15b2a824697..38112445e8d 100644 --- a/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js +++ b/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js @@ -16,7 +16,7 @@ describe('ParallelDiffExpansionRow', () => { isTop: false, isBottom: false, }; - const props = Object.assign({}, defaults, options); + const props = { ...defaults, ...options }; return createComponentWithStore(cmp, createStore(), props).$mount(); }; diff --git a/spec/frontend/diffs/components/parallel_diff_view_spec.js b/spec/frontend/diffs/components/parallel_diff_view_spec.js index 0eefbc7ec08..03cf1b72b62 100644 --- a/spec/frontend/diffs/components/parallel_diff_view_spec.js +++ b/spec/frontend/diffs/components/parallel_diff_view_spec.js @@ -7,7 +7,7 @@ import diffFileMockData from '../mock_data/diff_file'; describe('ParallelDiffView', () => { let component; - const getDiffFileMock = () => Object.assign({}, diffFileMockData); + const getDiffFileMock = () => ({ ...diffFileMockData }); beforeEach(() => { const diffFile = getDiffFileMock(); diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index ceccce6312f..3fba661da44 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -40,9 +40,12 @@ import { receiveFullDiffError, fetchFullDiff, toggleFullDiff, + switchToFullDiffFromRenamedFile, setFileCollapsed, setExpandedDiffLines, setSuggestPopoverDismissed, + changeCurrentCommit, + moveToNeighboringCommit, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; @@ -312,7 +315,7 @@ describe('DiffsStoreActions', () => { describe('fetchDiffFilesMeta', () => { it('should fetch diff meta information', done => { - const endpointMetadata = '/fetch/diffs_meta?'; + const endpointMetadata = '/fetch/diffs_meta'; const mock = new MockAdapter(axios); const data = { diff_files: [] }; const res = { data }; @@ -1250,6 +1253,87 @@ describe('DiffsStoreActions', () => { }); }); + describe('switchToFullDiffFromRenamedFile', () => { + const SUCCESS_URL = 'fakehost/context.success'; + const ERROR_URL = 'fakehost/context.error'; + const testFilePath = 'testpath'; + const updatedViewerName = 'testviewer'; + const preparedLine = { prepared: 'in-a-test' }; + const testFile = { + file_path: testFilePath, + file_hash: 'testhash', + alternate_viewer: { name: updatedViewerName }, + }; + const updatedViewer = { name: updatedViewerName, collapsed: false }; + const testData = [{ rich_text: 'test' }, { rich_text: 'file2' }]; + let renamedFile; + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + jest.spyOn(utils, 'prepareLineForRenamedFile').mockImplementation(() => preparedLine); + }); + + afterEach(() => { + renamedFile = null; + mock.restore(); + }); + + describe('success', () => { + beforeEach(() => { + renamedFile = { ...testFile, context_lines_path: SUCCESS_URL }; + mock.onGet(SUCCESS_URL).replyOnce(200, testData); + }); + + it.each` + diffViewType + ${INLINE_DIFF_VIEW_TYPE} + ${PARALLEL_DIFF_VIEW_TYPE} + `( + 'performs the correct mutations and starts a render queue for view type $diffViewType', + ({ diffViewType }) => { + return testAction( + switchToFullDiffFromRenamedFile, + { diffFile: renamedFile }, + { diffViewType }, + [ + { + type: types.SET_DIFF_FILE_VIEWER, + payload: { filePath: testFilePath, viewer: updatedViewer }, + }, + { + type: types.SET_CURRENT_VIEW_DIFF_FILE_LINES, + payload: { filePath: testFilePath, lines: [preparedLine, preparedLine] }, + }, + ], + [{ type: 'startRenderDiffsQueue' }], + ); + }, + ); + }); + + describe('error', () => { + beforeEach(() => { + renamedFile = { ...testFile, context_lines_path: ERROR_URL }; + mock.onGet(ERROR_URL).reply(500); + }); + + it('dispatches the error handling action', () => { + const rejected = testAction( + switchToFullDiffFromRenamedFile, + { diffFile: renamedFile }, + null, + [], + [{ type: 'receiveFullDiffError', payload: testFilePath }], + ); + + return rejected.catch(error => + expect(error).toEqual(new Error('Request failed with status code 500')), + ); + }); + }); + }); + describe('setFileCollapsed', () => { it('commits SET_FILE_COLLAPSED', done => { testAction( @@ -1347,4 +1431,102 @@ describe('DiffsStoreActions', () => { ); }); }); + + describe('changeCurrentCommit', () => { + it('commits the new commit information and re-requests the diff metadata for the commit', () => { + return testAction( + changeCurrentCommit, + { commitId: 'NEW' }, + { + commit: { + id: 'OLD', + }, + endpoint: 'URL/OLD', + endpointBatch: 'URL/OLD', + endpointMetadata: 'URL/OLD', + }, + [ + { type: types.SET_DIFF_FILES, payload: [] }, + { + type: types.SET_BASE_CONFIG, + payload: { + commit: { + id: 'OLD', // Not a typo: the action fired next will overwrite all of the `commit` in state + }, + endpoint: 'URL/NEW', + endpointBatch: 'URL/NEW', + endpointMetadata: 'URL/NEW', + }, + }, + ], + [{ type: 'fetchDiffFilesMeta' }], + ); + }); + + it.each` + commitId | commit | msg + ${undefined} | ${{ id: 'OLD' }} | ${'`commitId` is a required argument'} + ${'NEW'} | ${null} | ${'`state` must already contain a valid `commit`'} + ${undefined} | ${null} | ${'`commitId` is a required argument'} + `( + 'returns a rejected promise with the error message $msg given `{ "commitId": $commitId, "state.commit": $commit }`', + ({ commitId, commit, msg }) => { + const err = new Error(msg); + const actionReturn = testAction( + changeCurrentCommit, + { commitId }, + { + endpoint: 'URL/OLD', + endpointBatch: 'URL/OLD', + endpointMetadata: 'URL/OLD', + commit, + }, + [], + [], + ); + + return expect(actionReturn).rejects.toStrictEqual(err); + }, + ); + }); + + describe('moveToNeighboringCommit', () => { + it.each` + direction | expected | currentCommit + ${'next'} | ${'NEXTSHA'} | ${{ next_commit_id: 'NEXTSHA' }} + ${'previous'} | ${'PREVIOUSSHA'} | ${{ prev_commit_id: 'PREVIOUSSHA' }} + `( + 'for the direction "$direction", dispatches the action to move to the SHA "$expected"', + ({ direction, expected, currentCommit }) => { + return testAction( + moveToNeighboringCommit, + { direction }, + { commit: currentCommit }, + [], + [{ type: 'changeCurrentCommit', payload: { commitId: expected } }], + ); + }, + ); + + it.each` + direction | diffsAreLoading | currentCommit + ${'next'} | ${false} | ${{ prev_commit_id: 'PREVIOUSSHA' }} + ${'next'} | ${true} | ${{ prev_commit_id: 'PREVIOUSSHA' }} + ${'next'} | ${false} | ${undefined} + ${'previous'} | ${false} | ${{ next_commit_id: 'NEXTSHA' }} + ${'previous'} | ${true} | ${{ next_commit_id: 'NEXTSHA' }} + ${'previous'} | ${false} | ${undefined} + `( + 'given `{ "isloading": $diffsAreLoading, "commit": $currentCommit }` in state, no actions are dispatched', + ({ direction, diffsAreLoading, currentCommit }) => { + return testAction( + moveToNeighboringCommit, + { direction }, + { commit: currentCommit, isLoading: diffsAreLoading }, + [], + [], + ); + }, + ); + }); }); diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js index ca47f51cb15..dac5be2d656 100644 --- a/spec/frontend/diffs/store/getters_spec.js +++ b/spec/frontend/diffs/store/getters_spec.js @@ -14,10 +14,10 @@ describe('Diffs Module Getters', () => { beforeEach(() => { localState = state(); - discussionMock = Object.assign({}, discussion); + discussionMock = { ...discussion }; discussionMock.diff_file.file_hash = diffFileMock.fileHash; - discussionMock1 = Object.assign({}, discussion); + discussionMock1 = { ...discussion }; discussionMock1.diff_file.file_hash = diffFileMock.fileHash; }); diff --git a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js index eb0f2364a50..0343ef75732 100644 --- a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js +++ b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js @@ -18,7 +18,6 @@ describe('Compare diff version dropdowns', () => { }; localState.targetBranchName = 'baseVersion'; localState.mergeRequestDiffs = diffsMockData; - gon.features = { diffCompareWithHead: true }; }); describe('selectedTargetIndex', () => { @@ -129,14 +128,6 @@ describe('Compare diff version dropdowns', () => { }); assertVersions(targetVersions); }); - - it('does not list head version if feature flag is not enabled', () => { - gon.features = { diffCompareWithHead: false }; - setupTest(); - const targetVersions = getters.diffCompareDropdownTargetVersions(localState, getters); - - expect(targetVersions.find(version => version.isHead)).toBeUndefined(); - }); }); it('diffCompareDropdownSourceVersions', () => { diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index 858ab5be167..c24d406fef3 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -1041,6 +1041,36 @@ describe('DiffsStoreMutations', () => { }); }); + describe('SET_DIFF_FILE_VIEWER', () => { + it("should update the correct diffFile's viewer property", () => { + const state = { + diffFiles: [ + { file_path: 'SearchString', viewer: 'OLD VIEWER' }, + { file_path: 'OtherSearchString' }, + { file_path: 'SomeOtherString' }, + ], + }; + + mutations[types.SET_DIFF_FILE_VIEWER](state, { + filePath: 'SearchString', + viewer: 'NEW VIEWER', + }); + + expect(state.diffFiles[0].viewer).toEqual('NEW VIEWER'); + expect(state.diffFiles[1].viewer).not.toBeDefined(); + expect(state.diffFiles[2].viewer).not.toBeDefined(); + + mutations[types.SET_DIFF_FILE_VIEWER](state, { + filePath: 'OtherSearchString', + viewer: 'NEW VIEWER', + }); + + expect(state.diffFiles[0].viewer).toEqual('NEW VIEWER'); + expect(state.diffFiles[1].viewer).toEqual('NEW VIEWER'); + expect(state.diffFiles[2].viewer).not.toBeDefined(); + }); + }); + describe('SET_SHOW_SUGGEST_POPOVER', () => { it('sets showSuggestPopover to false', () => { const state = { showSuggestPopover: true }; diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js index 1adcdab272a..641373e666f 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -361,6 +361,72 @@ describe('DiffsStoreUtils', () => { }); }); + describe('prepareLineForRenamedFile', () => { + const diffFile = { + file_hash: 'file-hash', + }; + const lineIndex = 4; + const sourceLine = { + foo: 'test', + rich_text: '

rich

', // Note the leading space + }; + const correctLine = { + foo: 'test', + line_code: 'file-hash_5_5', + old_line: 5, + new_line: 5, + rich_text: '

rich

', // Note no leading space + discussionsExpanded: true, + discussions: [], + hasForm: false, + text: undefined, + alreadyPrepared: true, + }; + let preppedLine; + + beforeEach(() => { + preppedLine = utils.prepareLineForRenamedFile({ + diffViewType: INLINE_DIFF_VIEW_TYPE, + line: sourceLine, + index: lineIndex, + diffFile, + }); + }); + + it('copies over the original line object to the new prepared line', () => { + expect(preppedLine).toEqual( + expect.objectContaining({ + foo: correctLine.foo, + rich_text: correctLine.rich_text, + }), + ); + }); + + it('correctly sets the old and new lines, plus a line code', () => { + expect(preppedLine.old_line).toEqual(correctLine.old_line); + expect(preppedLine.new_line).toEqual(correctLine.new_line); + expect(preppedLine.line_code).toEqual(correctLine.line_code); + }); + + it('returns a single object with the correct structure for `inline` lines', () => { + expect(preppedLine).toEqual(correctLine); + }); + + it('returns a nested object with "left" and "right" lines + the line code for `parallel` lines', () => { + preppedLine = utils.prepareLineForRenamedFile({ + diffViewType: PARALLEL_DIFF_VIEW_TYPE, + line: sourceLine, + index: lineIndex, + diffFile, + }); + + expect(Object.keys(preppedLine)).toEqual(['left', 'right', 'line_code']); + expect(preppedLine.left).toEqual(correctLine); + expect(preppedLine.right).toEqual(correctLine); + expect(preppedLine.line_code).toEqual(correctLine.line_code); + }); + }); + describe('prepareDiffData', () => { let mock; let preparedDiff; @@ -372,13 +438,13 @@ describe('DiffsStoreUtils', () => { mock = getDiffFileMock(); preparedDiff = { diff_files: [mock] }; splitInlineDiff = { - diff_files: [Object.assign({}, mock, { parallel_diff_lines: undefined })], + diff_files: [{ ...mock, parallel_diff_lines: undefined }], }; splitParallelDiff = { - diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })], + diff_files: [{ ...mock, highlighted_diff_lines: undefined }], }; completedDiff = { - diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })], + diff_files: [{ ...mock, highlighted_diff_lines: undefined }], }; preparedDiff.diff_files = utils.prepareDiffData(preparedDiff); @@ -503,11 +569,16 @@ describe('DiffsStoreUtils', () => { }, }; + // When multi line comments are fully implemented `line_code` will be + // included in all requests. Until then we need to ensure the logic does + // not change when it is included only in the "comparison" argument. + const lineRange = { start_line_code: 'abc_1_1', end_line_code: 'abc_1_2' }; + it('returns true when the discussion is up to date', () => { expect( utils.isDiscussionApplicableToLine({ discussion: discussions.upToDateDiscussion1, - diffPosition, + diffPosition: { ...diffPosition, line_range: lineRange }, latestDiff: true, }), ).toBe(true); @@ -517,7 +588,7 @@ describe('DiffsStoreUtils', () => { expect( utils.isDiscussionApplicableToLine({ discussion: discussions.outDatedDiscussion1, - diffPosition, + diffPosition: { ...diffPosition, line_range: lineRange }, latestDiff: true, }), ).toBe(false); @@ -534,6 +605,7 @@ describe('DiffsStoreUtils', () => { diffPosition: { ...diffPosition, lineCode: 'ABC_1', + line_range: lineRange, }, latestDiff: true, }), @@ -551,6 +623,7 @@ describe('DiffsStoreUtils', () => { diffPosition: { ...diffPosition, line_code: 'ABC_1', + line_range: lineRange, }, latestDiff: true, }), @@ -568,6 +641,7 @@ describe('DiffsStoreUtils', () => { diffPosition: { ...diffPosition, lineCode: 'ABC_1', + line_range: lineRange, }, latestDiff: false, }), -- cgit v1.2.3