diff options
Diffstat (limited to 'spec/frontend/diffs')
-rw-r--r-- | spec/frontend/diffs/components/app_spec.js | 198 | ||||
-rw-r--r-- | spec/frontend/diffs/components/commit_item_spec.js | 2 | ||||
-rw-r--r-- | spec/frontend/diffs/components/compare_dropdown_layout_spec.js | 8 | ||||
-rw-r--r-- | spec/frontend/diffs/components/compare_versions_spec.js | 4 | ||||
-rw-r--r-- | spec/frontend/diffs/components/diff_content_spec.js | 16 | ||||
-rw-r--r-- | spec/frontend/diffs/components/diff_expansion_cell_spec.js | 5 | ||||
-rw-r--r-- | spec/frontend/diffs/components/diff_file_row_spec.js | 38 | ||||
-rw-r--r-- | spec/frontend/diffs/components/diff_row_spec.js | 6 | ||||
-rw-r--r-- | spec/frontend/diffs/components/image_diff_overlay_spec.js | 14 | ||||
-rw-r--r-- | spec/frontend/diffs/components/settings_dropdown_spec.js | 95 | ||||
-rw-r--r-- | spec/frontend/diffs/diff_file_spec.js | 60 | ||||
-rw-r--r-- | spec/frontend/diffs/store/actions_spec.js | 48 | ||||
-rw-r--r-- | spec/frontend/diffs/store/mutations_spec.js | 334 | ||||
-rw-r--r-- | spec/frontend/diffs/store/utils_spec.js | 261 | ||||
-rw-r--r-- | spec/frontend/diffs/utils/diff_file_spec.js | 126 | ||||
-rw-r--r-- | spec/frontend/diffs/utils/preferences_spec.js | 40 |
16 files changed, 548 insertions, 707 deletions
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 225710eab63..416564b72c3 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -12,16 +12,19 @@ import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue'; import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue'; import CommitWidget from '~/diffs/components/commit_widget.vue'; 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'; +import { EVT_VIEW_FILE_BY_FILE } from '~/diffs/constants'; + +import eventHub from '~/diffs/event_hub'; + 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'; +const COMMIT_URL = `${TEST_HOST}/COMMIT/OLD`; +const UPDATED_COMMIT_URL = `${TEST_HOST}/COMMIT/NEW`; function getCollapsedFilesWarning(wrapper) { return wrapper.find(CollapsedFilesWarning); @@ -62,7 +65,7 @@ describe('diffs/components/app', () => { changesEmptyStateIllustration: '', dismissEndpoint: '', showSuggestPopover: true, - viewDiffsFileByFile: false, + fileByFileUserPreference: false, ...props, }, provide, @@ -75,12 +78,6 @@ describe('diffs/components/app', () => { }); } - function getOppositeViewType(currentViewType) { - return currentViewType === INLINE_DIFF_VIEW_TYPE - ? PARALLEL_DIFF_VIEW_TYPE - : INLINE_DIFF_VIEW_TYPE; - } - beforeEach(() => { // setup globals (needed for component to mount :/) window.mrTabs = { @@ -125,104 +122,6 @@ describe('diffs/components/app', () => { wrapper.vm.$nextTick(done); }); - describe('when the diff view type changes and it should load a single diff view style', () => { - const noLinesDiff = { - highlighted_diff_lines: [], - parallel_diff_lines: [], - }; - const parallelLinesDiff = { - highlighted_diff_lines: [], - parallel_diff_lines: ['line'], - }; - const inlineLinesDiff = { - highlighted_diff_lines: ['line'], - parallel_diff_lines: [], - }; - const fullDiff = { - highlighted_diff_lines: ['line'], - parallel_diff_lines: ['line'], - }; - - function expectFetchToOccur({ vueInstance, done = () => {}, existingFiles = 1 } = {}) { - vueInstance.$nextTick(() => { - expect(vueInstance.diffFiles.length).toEqual(existingFiles); - expect(vueInstance.fetchDiffFilesBatch).toHaveBeenCalled(); - - done(); - }); - } - - it('fetches diffs if it has none', done => { - wrapper.vm.isLatestVersion = () => false; - - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, existingFiles: 0, done }); - }); - - it('fetches diffs if it has both view styles, but no lines in either', done => { - wrapper.vm.isLatestVersion = () => false; - - store.state.diffs.diffFiles.push(noLinesDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, done }); - }); - - it('fetches diffs if it only has inline view style', done => { - wrapper.vm.isLatestVersion = () => false; - - store.state.diffs.diffFiles.push(inlineLinesDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, done }); - }); - - it('fetches diffs if it only has parallel view style', done => { - wrapper.vm.isLatestVersion = () => false; - - store.state.diffs.diffFiles.push(parallelLinesDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, done }); - }); - - it('fetches batch diffs if it has none', done => { - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, existingFiles: 0, done }); - }); - - it('fetches batch diffs if it has both view styles, but no lines in either', done => { - store.state.diffs.diffFiles.push(noLinesDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, done }); - }); - - it('fetches batch diffs if it only has inline view style', done => { - store.state.diffs.diffFiles.push(inlineLinesDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, done }); - }); - - it('fetches batch diffs if it only has parallel view style', done => { - store.state.diffs.diffFiles.push(parallelLinesDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expectFetchToOccur({ vueInstance: wrapper.vm, done }); - }); - - it('does not fetch batch diffs if it has already fetched both styles of diff', () => { - store.state.diffs.diffFiles.push(fullDiff); - store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); - - expect(wrapper.vm.diffFiles.length).toEqual(1); - expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled(); - }); - }); - it('calls batch methods if diffsBatchLoad is enabled, and not latest version', done => { expect(wrapper.vm.diffFilesLength).toEqual(0); wrapper.vm.isLatestVersion = () => false; @@ -743,70 +642,76 @@ describe('diffs/components/app', () => { }); }); - describe('hideTreeListIfJustOneFile', () => { - let toggleShowTreeList; + describe('setTreeDisplay', () => { + let setShowTreeList; beforeEach(() => { - toggleShowTreeList = jest.fn(); + setShowTreeList = jest.fn(); }); afterEach(() => { localStorage.removeItem('mr_tree_show'); }); - it('calls toggleShowTreeList when only 1 file', () => { + it('calls setShowTreeList when only 1 file', () => { createComponent({}, ({ state }) => { state.diffs.diffFiles.push({ sha: '123' }); }); wrapper.setMethods({ - toggleShowTreeList, + setShowTreeList, }); - wrapper.vm.hideTreeListIfJustOneFile(); + wrapper.vm.setTreeDisplay(); - expect(toggleShowTreeList).toHaveBeenCalledWith(false); + expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: false, saving: false }); }); - it('does not call toggleShowTreeList when more than 1 file', () => { + it('calls setShowTreeList with true when more than 1 file is in diffs array', () => { createComponent({}, ({ state }) => { state.diffs.diffFiles.push({ sha: '123' }); state.diffs.diffFiles.push({ sha: '124' }); }); wrapper.setMethods({ - toggleShowTreeList, + setShowTreeList, }); - wrapper.vm.hideTreeListIfJustOneFile(); + wrapper.vm.setTreeDisplay(); - expect(toggleShowTreeList).not.toHaveBeenCalled(); + expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: true, saving: false }); }); - it('does not call toggleShowTreeList when localStorage is set', () => { - localStorage.setItem('mr_tree_show', 'true'); + it.each` + showTreeList + ${true} + ${false} + `('calls setShowTreeList with localstorage $showTreeList', ({ showTreeList }) => { + localStorage.setItem('mr_tree_show', showTreeList); createComponent({}, ({ state }) => { state.diffs.diffFiles.push({ sha: '123' }); }); wrapper.setMethods({ - toggleShowTreeList, + setShowTreeList, }); - wrapper.vm.hideTreeListIfJustOneFile(); + wrapper.vm.setTreeDisplay(); - expect(toggleShowTreeList).not.toHaveBeenCalled(); + expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList, saving: false }); }); }); describe('file-by-file', () => { - it('renders a single diff', () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it('renders a single diff', async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }); state.diffs.diffFiles.push({ file_hash: '312' }); }); + await wrapper.vm.$nextTick(); + expect(wrapper.findAll(DiffFile).length).toBe(1); }); @@ -814,31 +719,37 @@ describe('diffs/components/app', () => { const fileByFileNav = () => wrapper.find('[data-testid="file-by-file-navigation"]'); const paginator = () => fileByFileNav().find(GlPagination); - it('sets previous button as disabled', () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it('sets previous button as disabled', async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); }); + await wrapper.vm.$nextTick(); + expect(paginator().attributes('prevpage')).toBe(undefined); expect(paginator().attributes('nextpage')).toBe('2'); }); - it('sets next button as disabled', () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it('sets next button as disabled', async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); state.diffs.currentDiffFileId = '312'; }); + await wrapper.vm.$nextTick(); + expect(paginator().attributes('prevpage')).toBe('1'); expect(paginator().attributes('nextpage')).toBe(undefined); }); - it("doesn't display when there's fewer than 2 files", () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it("doesn't display when there's fewer than 2 files", async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }); state.diffs.currentDiffFileId = '123'; }); + await wrapper.vm.$nextTick(); + expect(fileByFileNav().exists()).toBe(false); }); @@ -849,11 +760,13 @@ describe('diffs/components/app', () => { `( 'it calls navigateToDiffFileIndex with $index when $link is clicked', async ({ currentDiffFileId, targetFile }) => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); state.diffs.currentDiffFileId = currentDiffFileId; }); + await wrapper.vm.$nextTick(); + jest.spyOn(wrapper.vm, 'navigateToDiffFileIndex'); paginator().vm.$emit('input', targetFile); @@ -864,5 +777,24 @@ describe('diffs/components/app', () => { }, ); }); + + describe('control via event stream', () => { + it.each` + setting + ${true} + ${false} + `( + 'triggers the action with the new fileByFile setting - $setting - when the event with that setting is received', + async ({ setting }) => { + createComponent(); + await wrapper.vm.$nextTick(); + + eventHub.$emit(EVT_VIEW_FILE_BY_FILE, { setting }); + await wrapper.vm.$nextTick(); + + expect(store.state.diffs.viewDiffsFileByFile).toBe(setting); + }, + ); + }); }); }); diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index 9e4fcddd1b4..8a7eb6aaca6 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -84,7 +84,7 @@ describe('diffs/components/commit_item', () => { it('renders commit sha', () => { const shaElement = getShaElement(); - const labelElement = shaElement.find('[data-testid="commit-sha-group"] button'); + const labelElement = shaElement.find('[data-testid="commit-sha-short-id"]'); const buttonElement = shaElement.find('button.input-group-text'); expect(labelElement.text()).toEqual(commit.short_id); diff --git a/spec/frontend/diffs/components/compare_dropdown_layout_spec.js b/spec/frontend/diffs/components/compare_dropdown_layout_spec.js index a163a43daf1..92e4a2d9c62 100644 --- a/spec/frontend/diffs/components/compare_dropdown_layout_spec.js +++ b/spec/frontend/diffs/components/compare_dropdown_layout_spec.js @@ -1,4 +1,4 @@ -import { shallowMount } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { trimText } from 'helpers/text_helper'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import CompareDropdownLayout from '~/diffs/components/compare_dropdown_layout.vue'; @@ -22,7 +22,7 @@ describe('CompareDropdownLayout', () => { }); const createComponent = (propsData = {}) => { - wrapper = shallowMount(CompareDropdownLayout, { + wrapper = mount(CompareDropdownLayout, { propsData: { ...propsData, }, @@ -35,7 +35,7 @@ describe('CompareDropdownLayout', () => { href: listItem.find('a').attributes('href'), text: trimText(listItem.text()), createdAt: listItem.findAll(TimeAgo).wrappers[0]?.props('time'), - isActive: listItem.find('a.is-active').exists(), + isActive: listItem.classes().includes('is-active'), })); afterEach(() => { @@ -69,7 +69,7 @@ describe('CompareDropdownLayout', () => { expect(findListItemsData()).toEqual([ { href: 'version/1', - text: 'version 1 (base) abcdef1 1 commit', + text: 'version 1 (base) abcdef1 1 commit 2 years ago', createdAt: TEST_CREATED_AT, isActive: true, }, diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js index b3dfc71260c..09e9669c474 100644 --- a/spec/frontend/diffs/components/compare_versions_spec.js +++ b/spec/frontend/diffs/components/compare_versions_spec.js @@ -59,8 +59,8 @@ describe('CompareVersions', () => { expect(sourceDropdown.exists()).toBe(true); expect(targetDropdown.exists()).toBe(true); - expect(sourceDropdown.find('a span').html()).toContain('latest version'); - expect(targetDropdown.find('a span').html()).toContain(targetBranchName); + expect(sourceDropdown.find('a p').html()).toContain('latest version'); + expect(targetDropdown.find('button').html()).toContain(targetBranchName); }); it('should not render comparison dropdowns if no mergeRequestDiffs are specified', () => { diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js index e3a6f7f16a9..43d295ff1b3 100644 --- a/spec/frontend/diffs/components/diff_content_spec.js +++ b/spec/frontend/diffs/components/diff_content_spec.js @@ -6,13 +6,11 @@ import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; import NotDiffableViewer from '~/vue_shared/components/diff_viewer/viewers/not_diffable.vue'; import NoPreviewViewer from '~/vue_shared/components/diff_viewer/viewers/no_preview.vue'; import ParallelDiffView from '~/diffs/components/parallel_diff_view.vue'; -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 '../mock_data/diff_file'; import { diffViewerModes } from '~/ide/constants'; -import { diffLines } from '~/diffs/store/getters'; import DiffView from '~/diffs/components/diff_view.vue'; const localVue = createLocalVue(); @@ -74,7 +72,7 @@ describe('DiffContent', () => { isInlineView: isInlineViewGetterMock, isParallelView: isParallelViewGetterMock, getCommentFormForDiffFile: getCommentFormForDiffFileGetterMock, - diffLines, + diffLines: () => () => [...diffFileMockData.parallel_diff_lines], }, actions: { saveDiffDiscussion: saveDiffDiscussionMock, @@ -122,11 +120,11 @@ describe('DiffContent', () => { expect(wrapper.find(ParallelDiffView).exists()).toBe(true); }); - it('should render diff view if `unifiedDiffLines` & `unifiedDiffComponents` are true', () => { + it('should render diff view if `unifiedDiffComponents` are true', () => { isParallelViewGetterMock.mockReturnValue(true); createComponent({ props: { diffFile: textDiffFile }, - provide: { glFeatures: { unifiedDiffLines: true, unifiedDiffComponents: true } }, + provide: { glFeatures: { unifiedDiffComponents: true } }, }); expect(wrapper.find(DiffView).exists()).toBe(true); @@ -167,14 +165,6 @@ describe('DiffContent', () => { describe('with image files', () => { const imageDiffFile = { ...defaultProps.diffFile, viewer: { name: diffViewerModes.image } }; - it('should have image diff view in place', () => { - getCommentFormForDiffFileGetterMock.mockReturnValue(() => true); - createComponent({ props: { diffFile: imageDiffFile } }); - - expect(wrapper.find(InlineDiffView).exists()).toBe(false); - expect(wrapper.find(ImageDiffOverlay).exists()).toBe(true); - }); - it('renders diff file discussions', () => { getCommentFormForDiffFileGetterMock.mockReturnValue(() => true); createComponent({ diff --git a/spec/frontend/diffs/components/diff_expansion_cell_spec.js b/spec/frontend/diffs/components/diff_expansion_cell_spec.js index 81e08f09f62..a3b4b5c3abb 100644 --- a/spec/frontend/diffs/components/diff_expansion_cell_spec.js +++ b/spec/frontend/diffs/components/diff_expansion_cell_spec.js @@ -5,18 +5,16 @@ import { getByText } from '@testing-library/dom'; import { createStore } from '~/mr_notes/stores'; import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue'; import { getPreviousLineIndex } from '~/diffs/store/utils'; -import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants'; +import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; import diffFileMockData from '../mock_data/diff_file'; const EXPAND_UP_CLASS = '.js-unfold'; const EXPAND_DOWN_CLASS = '.js-unfold-down'; const lineSources = { [INLINE_DIFF_VIEW_TYPE]: 'highlighted_diff_lines', - [PARALLEL_DIFF_VIEW_TYPE]: 'parallel_diff_lines', }; const lineHandlers = { [INLINE_DIFF_VIEW_TYPE]: line => line, - [PARALLEL_DIFF_VIEW_TYPE]: line => line.right || line.left, }; function makeLoadMoreLinesPayload({ @@ -126,7 +124,6 @@ describe('DiffExpansionCell', () => { describe('any row', () => { [ { diffViewType: INLINE_DIFF_VIEW_TYPE, lineIndex: 8, file: { parallel_diff_lines: [] } }, - { diffViewType: PARALLEL_DIFF_VIEW_TYPE, lineIndex: 7, file: { highlighted_diff_lines: [] } }, ].forEach(({ diffViewType, file, lineIndex }) => { describe(`with diffViewType (${diffViewType})`, () => { beforeEach(() => { diff --git a/spec/frontend/diffs/components/diff_file_row_spec.js b/spec/frontend/diffs/components/diff_file_row_spec.js index 23adc8f9da4..7403a7918a9 100644 --- a/spec/frontend/diffs/components/diff_file_row_spec.js +++ b/spec/frontend/diffs/components/diff_file_row_spec.js @@ -7,12 +7,9 @@ import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; describe('Diff File Row component', () => { let wrapper; - const createComponent = (props = {}, highlightCurrentDiffRow = false) => { + const createComponent = (props = {}) => { wrapper = shallowMount(DiffFileRow, { propsData: { ...props }, - provide: { - glFeatures: { highlightCurrentDiffRow }, - }, }); }; @@ -60,26 +57,23 @@ describe('Diff File Row component', () => { }); it.each` - features | fileType | isViewed | expected - ${{ highlightCurrentDiffRow: true }} | ${'blob'} | ${false} | ${'gl-font-weight-bold'} - ${{}} | ${'blob'} | ${true} | ${''} - ${{}} | ${'tree'} | ${false} | ${''} - ${{}} | ${'tree'} | ${true} | ${''} + fileType | isViewed | expected + ${'blob'} | ${false} | ${'gl-font-weight-bold'} + ${'blob'} | ${true} | ${''} + ${'tree'} | ${false} | ${''} + ${'tree'} | ${true} | ${''} `( - 'with (features="$features", fileType="$fileType", isViewed=$isViewed), sets fileClasses="$expected"', - ({ features, fileType, isViewed, expected }) => { - createComponent( - { - file: { - type: fileType, - fileHash: '#123456789', - }, - level: 0, - hideFileStats: false, - viewedFiles: isViewed ? { '#123456789': true } : {}, + 'with (fileType="$fileType", isViewed=$isViewed), sets fileClasses="$expected"', + ({ fileType, isViewed, expected }) => { + createComponent({ + file: { + type: fileType, + fileHash: '#123456789', }, - features.highlightCurrentDiffRow, - ); + level: 0, + hideFileStats: false, + viewedFiles: isViewed ? { '#123456789': true } : {}, + }); expect(wrapper.find(FileRow).props('fileClasses')).toBe(expected); }, ); diff --git a/spec/frontend/diffs/components/diff_row_spec.js b/spec/frontend/diffs/components/diff_row_spec.js index f9e76cf8107..0ec075c8ad8 100644 --- a/spec/frontend/diffs/components/diff_row_spec.js +++ b/spec/frontend/diffs/components/diff_row_spec.js @@ -97,18 +97,18 @@ describe('DiffRow', () => { ${'right'} `('$side side', ({ side }) => { it(`renders empty cells if ${side} is unavailable`, () => { - const wrapper = createWrapper({ props: { line: testLines[2] } }); + const wrapper = createWrapper({ props: { line: testLines[2], inline: false } }); expect(wrapper.find(`[data-testid="${side}LineNumber"]`).exists()).toBe(false); expect(wrapper.find(`[data-testid="${side}EmptyCell"]`).exists()).toBe(true); }); it('renders comment button', () => { - const wrapper = createWrapper({ props: { line: testLines[3] } }); + const wrapper = createWrapper({ props: { line: testLines[3], inline: false } }); expect(wrapper.find(`[data-testid="${side}CommentButton"]`).exists()).toBe(true); }); it('renders avatars', () => { - const wrapper = createWrapper({ props: { line: testLines[0] } }); + const wrapper = createWrapper({ props: { line: testLines[0], inline: false } }); expect(wrapper.find(`[data-testid="${side}Discussions"]`).exists()).toBe(true); }); }); diff --git a/spec/frontend/diffs/components/image_diff_overlay_spec.js b/spec/frontend/diffs/components/image_diff_overlay_spec.js index 5a88a3cabd1..087715111b4 100644 --- a/spec/frontend/diffs/components/image_diff_overlay_spec.js +++ b/spec/frontend/diffs/components/image_diff_overlay_spec.js @@ -24,6 +24,8 @@ describe('Diffs image diff overlay component', () => { propsData: { discussions: [...imageDiffDiscussions], fileHash: 'ABC', + renderedWidth: 200, + renderedHeight: 200, ...props, }, methods: { @@ -71,8 +73,8 @@ describe('Diffs image diff overlay component', () => { createComponent(); const imageBadges = getAllImageBadges(); - expect(imageBadges.at(0).attributes('style')).toBe('left: 10px; top: 10px;'); - expect(imageBadges.at(1).attributes('style')).toBe('left: 5px; top: 5px;'); + expect(imageBadges.at(0).attributes('style')).toBe('left: 10%; top: 5%;'); + expect(imageBadges.at(1).attributes('style')).toBe('left: 5%; top: 2.5%;'); }); it('renders single badge for discussion object', () => { @@ -95,6 +97,8 @@ describe('Diffs image diff overlay component', () => { y: 0, width: 100, height: 200, + xPercent: 0, + yPercent: 0, }); }); @@ -120,11 +124,13 @@ describe('Diffs image diff overlay component', () => { describe('comment form', () => { const getCommentIndicator = () => wrapper.find('.comment-indicator'); beforeEach(() => { - createComponent({}, store => { + createComponent({ canComment: true }, store => { store.state.diffs.commentForms.push({ fileHash: 'ABC', x: 20, y: 10, + xPercent: 10, + yPercent: 10, }); }); }); @@ -134,7 +140,7 @@ describe('Diffs image diff overlay component', () => { }); it('sets comment form badge position', () => { - expect(getCommentIndicator().attributes('style')).toBe('left: 20px; top: 10px;'); + expect(getCommentIndicator().attributes('style')).toBe('left: 10%; top: 10%;'); }); }); }); diff --git a/spec/frontend/diffs/components/settings_dropdown_spec.js b/spec/frontend/diffs/components/settings_dropdown_spec.js index 72330d8efba..eb9f9b4db73 100644 --- a/spec/frontend/diffs/components/settings_dropdown_spec.js +++ b/spec/frontend/diffs/components/settings_dropdown_spec.js @@ -2,12 +2,18 @@ import { mount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; import diffModule from '~/diffs/store/modules'; import SettingsDropdown from '~/diffs/components/settings_dropdown.vue'; -import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; +import { + EVT_VIEW_FILE_BY_FILE, + PARALLEL_DIFF_VIEW_TYPE, + INLINE_DIFF_VIEW_TYPE, +} from '~/diffs/constants'; +import eventHub from '~/diffs/event_hub'; const localVue = createLocalVue(); localVue.use(Vuex); describe('Diff settings dropdown component', () => { + let wrapper; let vm; let actions; @@ -25,10 +31,15 @@ describe('Diff settings dropdown component', () => { extendStore(store); - vm = mount(SettingsDropdown, { + wrapper = mount(SettingsDropdown, { localVue, store, }); + vm = wrapper.vm; + } + + function getFileByFileCheckbox(vueWrapper) { + return vueWrapper.find('[data-testid="file-by-file"]'); } beforeEach(() => { @@ -41,14 +52,14 @@ describe('Diff settings dropdown component', () => { }); afterEach(() => { - vm.destroy(); + wrapper.destroy(); }); describe('tree view buttons', () => { it('list view button dispatches setRenderTreeList with false', () => { createComponent(); - vm.find('.js-list-view').trigger('click'); + wrapper.find('.js-list-view').trigger('click'); expect(actions.setRenderTreeList).toHaveBeenCalledWith(expect.anything(), false); }); @@ -56,7 +67,7 @@ describe('Diff settings dropdown component', () => { it('tree view button dispatches setRenderTreeList with true', () => { createComponent(); - vm.find('.js-tree-view').trigger('click'); + wrapper.find('.js-tree-view').trigger('click'); expect(actions.setRenderTreeList).toHaveBeenCalledWith(expect.anything(), true); }); @@ -68,8 +79,8 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-list-view').classes('selected')).toBe(true); - expect(vm.find('.js-tree-view').classes('selected')).toBe(false); + expect(wrapper.find('.js-list-view').classes('selected')).toBe(true); + expect(wrapper.find('.js-tree-view').classes('selected')).toBe(false); }); it('sets tree button as selected when renderTreeList is true', () => { @@ -79,8 +90,8 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-list-view').classes('selected')).toBe(false); - expect(vm.find('.js-tree-view').classes('selected')).toBe(true); + expect(wrapper.find('.js-list-view').classes('selected')).toBe(false); + expect(wrapper.find('.js-tree-view').classes('selected')).toBe(true); }); }); @@ -92,8 +103,8 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-inline-diff-button').classes('selected')).toBe(true); - expect(vm.find('.js-parallel-diff-button').classes('selected')).toBe(false); + expect(wrapper.find('.js-inline-diff-button').classes('selected')).toBe(true); + expect(wrapper.find('.js-parallel-diff-button').classes('selected')).toBe(false); }); it('sets parallel button as selected', () => { @@ -103,14 +114,14 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-inline-diff-button').classes('selected')).toBe(false); - expect(vm.find('.js-parallel-diff-button').classes('selected')).toBe(true); + expect(wrapper.find('.js-inline-diff-button').classes('selected')).toBe(false); + expect(wrapper.find('.js-parallel-diff-button').classes('selected')).toBe(true); }); it('calls setInlineDiffViewType when clicking inline button', () => { createComponent(); - vm.find('.js-inline-diff-button').trigger('click'); + wrapper.find('.js-inline-diff-button').trigger('click'); expect(actions.setInlineDiffViewType).toHaveBeenCalled(); }); @@ -118,7 +129,7 @@ describe('Diff settings dropdown component', () => { it('calls setParallelDiffViewType when clicking parallel button', () => { createComponent(); - vm.find('.js-parallel-diff-button').trigger('click'); + wrapper.find('.js-parallel-diff-button').trigger('click'); expect(actions.setParallelDiffViewType).toHaveBeenCalled(); }); @@ -132,7 +143,7 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('#show-whitespace').element.checked).toBe(false); + expect(wrapper.find('#show-whitespace').element.checked).toBe(false); }); it('sets as checked when showWhitespace is true', () => { @@ -142,13 +153,13 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('#show-whitespace').element.checked).toBe(true); + expect(wrapper.find('#show-whitespace').element.checked).toBe(true); }); it('calls setShowWhitespace on change', () => { createComponent(); - const checkbox = vm.find('#show-whitespace'); + const checkbox = wrapper.find('#show-whitespace'); checkbox.element.checked = true; checkbox.trigger('change'); @@ -159,4 +170,52 @@ describe('Diff settings dropdown component', () => { }); }); }); + + describe('file-by-file toggle', () => { + beforeEach(() => { + jest.spyOn(eventHub, '$emit'); + }); + + it.each` + fileByFile | checked + ${true} | ${true} + ${false} | ${false} + `( + 'sets the checkbox to { checked: $checked } if the fileByFile setting is $fileByFile', + async ({ fileByFile, checked }) => { + createComponent(store => { + Object.assign(store.state.diffs, { + viewDiffsFileByFile: fileByFile, + }); + }); + + await vm.$nextTick(); + + expect(getFileByFileCheckbox(wrapper).element.checked).toBe(checked); + }, + ); + + it.each` + start | emit + ${true} | ${false} + ${false} | ${true} + `( + 'when the file by file setting starts as $start, toggling the checkbox should emit an event set to $emit', + async ({ start, emit }) => { + createComponent(store => { + Object.assign(store.state.diffs, { + viewDiffsFileByFile: start, + }); + }); + + await vm.$nextTick(); + + getFileByFileCheckbox(wrapper).trigger('click'); + + await vm.$nextTick(); + + expect(eventHub.$emit).toHaveBeenCalledWith(EVT_VIEW_FILE_BY_FILE, { setting: emit }); + }, + ); + }); }); diff --git a/spec/frontend/diffs/diff_file_spec.js b/spec/frontend/diffs/diff_file_spec.js deleted file mode 100644 index 5d74760ef66..00000000000 --- a/spec/frontend/diffs/diff_file_spec.js +++ /dev/null @@ -1,60 +0,0 @@ -import { prepareRawDiffFile } from '~/diffs/diff_file'; - -const DIFF_FILES = [ - { - file_hash: 'ABC', // This file is just a normal file - }, - { - file_hash: 'DEF', // This file replaces a symlink - a_mode: '0', - b_mode: '0755', - }, - { - file_hash: 'DEF', // This symlink is replaced by a file - a_mode: '120000', - b_mode: '0', - }, - { - file_hash: 'GHI', // This symlink replaces a file - a_mode: '0', - b_mode: '120000', - }, - { - file_hash: 'GHI', // This file is replaced by a symlink - a_mode: '0755', - b_mode: '0', - }, -]; - -function makeBrokenSymlinkObject(replaced, wasSymbolic, isSymbolic, wasReal, isReal) { - return { - replaced, - wasSymbolic, - isSymbolic, - wasReal, - isReal, - }; -} - -describe('diff_file utilities', () => { - describe('prepareRawDiffFile', () => { - it.each` - fileIndex | description | brokenSymlink - ${0} | ${'a file that is not symlink-adjacent'} | ${false} - ${1} | ${'a file that replaces a symlink'} | ${makeBrokenSymlinkObject(false, false, false, false, true)} - ${2} | ${'a symlink that is replaced by a file'} | ${makeBrokenSymlinkObject(true, true, false, false, false)} - ${3} | ${'a symlink that replaces a file'} | ${makeBrokenSymlinkObject(false, false, true, false, false)} - ${4} | ${'a file that is replaced by a symlink'} | ${makeBrokenSymlinkObject(true, false, false, true, false)} - `( - 'properly marks $description with the correct .brokenSymlink value', - ({ fileIndex, brokenSymlink }) => { - const preppedRaw = prepareRawDiffFile({ - file: DIFF_FILES[fileIndex], - allFiles: DIFF_FILES, - }); - - expect(preppedRaw.brokenSymlink).toStrictEqual(brokenSymlink); - }, - ); - }); -}); diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 0af5ddd9764..fef7676e795 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -32,7 +32,7 @@ import { setHighlightedRow, toggleTreeOpen, scrollToFile, - toggleShowTreeList, + setShowTreeList, renderFileForDiscussionId, setRenderTreeList, setShowWhitespace, @@ -48,6 +48,7 @@ import { moveToNeighboringCommit, setCurrentDiffFileIdFromNote, navigateToDiffFileIndex, + setFileByFile, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; @@ -159,10 +160,10 @@ describe('DiffsStoreActions', () => { .onGet( mergeUrlParams( { - per_page: DIFFS_PER_PAGE, w: '1', view: 'inline', page: 1, + per_page: DIFFS_PER_PAGE, }, endpointBatch, ), @@ -171,10 +172,10 @@ describe('DiffsStoreActions', () => { .onGet( mergeUrlParams( { - per_page: DIFFS_PER_PAGE, w: '1', view: 'inline', page: 2, + per_page: DIFFS_PER_PAGE, }, endpointBatch, ), @@ -248,7 +249,7 @@ describe('DiffsStoreActions', () => { { type: types.SET_LOADING, payload: true }, { type: types.SET_LOADING, payload: false }, { type: types.SET_MERGE_REQUEST_DIFFS, payload: diffMetadata.merge_request_diffs }, - { type: types.SET_DIFF_DATA, payload: noFilesData }, + { type: types.SET_DIFF_METADATA, payload: noFilesData }, ], [], () => { @@ -901,15 +902,22 @@ describe('DiffsStoreActions', () => { }); }); - describe('toggleShowTreeList', () => { + describe('setShowTreeList', () => { it('commits toggle', done => { - testAction(toggleShowTreeList, null, {}, [{ type: types.TOGGLE_SHOW_TREE_LIST }], [], done); + testAction( + setShowTreeList, + { showTreeList: true }, + {}, + [{ type: types.SET_SHOW_TREE_LIST, payload: true }], + [], + done, + ); }); it('updates localStorage', () => { jest.spyOn(localStorage, 'setItem').mockImplementation(() => {}); - toggleShowTreeList({ commit() {}, state: { showTreeList: true } }); + setShowTreeList({ commit() {} }, { showTreeList: true }); expect(localStorage.setItem).toHaveBeenCalledWith('mr_tree_show', true); }); @@ -917,7 +925,7 @@ describe('DiffsStoreActions', () => { it('does not update localStorage', () => { jest.spyOn(localStorage, 'setItem').mockImplementation(() => {}); - toggleShowTreeList({ commit() {}, state: { showTreeList: true } }, false); + setShowTreeList({ commit() {} }, { showTreeList: true, saving: false }); expect(localStorage.setItem).not.toHaveBeenCalled(); }); @@ -1236,10 +1244,6 @@ describe('DiffsStoreActions', () => { { diffViewType: 'inline' }, [ { - type: 'SET_HIDDEN_VIEW_DIFF_FILE_LINES', - payload: { filePath: 'path', lines: ['test'] }, - }, - { type: 'SET_CURRENT_VIEW_DIFF_FILE_LINES', payload: { filePath: 'path', lines: ['test'] }, }, @@ -1259,10 +1263,6 @@ describe('DiffsStoreActions', () => { { diffViewType: 'inline' }, [ { - type: 'SET_HIDDEN_VIEW_DIFF_FILE_LINES', - payload: { filePath: 'path', lines }, - }, - { type: 'SET_CURRENT_VIEW_DIFF_FILE_LINES', payload: { filePath: 'path', lines: lines.slice(0, 200) }, }, @@ -1456,4 +1456,20 @@ describe('DiffsStoreActions', () => { ); }); }); + + describe('setFileByFile', () => { + it.each` + value + ${true} + ${false} + `('commits SET_FILE_BY_FILE with the new value $value', ({ value }) => { + return testAction( + setFileByFile, + { fileByFile: value }, + { viewDiffsFileByFile: null }, + [{ type: types.SET_FILE_BY_FILE, payload: value }], + [], + ); + }); + }); }); diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index c0645faf89e..13e7cad835d 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -1,7 +1,7 @@ import createState from '~/diffs/store/modules/diff_state'; import mutations from '~/diffs/store/mutations'; import * as types from '~/diffs/store/mutation_types'; -import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; +import { INLINE_DIFF_VIEW_TYPE, INLINE_DIFF_LINES_KEY } from '~/diffs/constants'; import diffFileMockData from '../mock_data/diff_file'; import * as utils from '~/diffs/store/utils'; @@ -67,28 +67,28 @@ describe('DiffsStoreMutations', () => { }); }); - describe('SET_DIFF_DATA', () => { - it('should not modify the existing state', () => { + describe('SET_DIFF_METADATA', () => { + it('should overwrite state with the camelCased data that is passed in', () => { const state = { - diffFiles: [ - { - content_sha: diffFileMockData.content_sha, - file_hash: diffFileMockData.file_hash, - highlighted_diff_lines: [], - }, - ], + diffFiles: [], }; const diffMock = { diff_files: [diffFileMockData], }; + const metaMock = { + other_key: 'value', + }; - mutations[types.SET_DIFF_DATA](state, diffMock); + mutations[types.SET_DIFF_METADATA](state, diffMock); + expect(state.diffFiles[0]).toEqual(diffFileMockData); - expect(state.diffFiles[0].parallel_diff_lines).toBeUndefined(); + mutations[types.SET_DIFF_METADATA](state, metaMock); + expect(state.diffFiles[0]).toEqual(diffFileMockData); + expect(state.otherKey).toEqual('value'); }); }); - describe('SET_DIFFSET_DIFF_DATA_BATCH_DATA', () => { + describe('SET_DIFF_DATA_BATCH_DATA', () => { it('should set diff data batch type properly', () => { const state = { diffFiles: [] }; const diffMock = { @@ -97,9 +97,6 @@ describe('DiffsStoreMutations', () => { mutations[types.SET_DIFF_DATA_BATCH](state, diffMock); - const firstLine = state.diffFiles[0].parallel_diff_lines[0]; - - expect(firstLine.right.text).toBeUndefined(); expect(state.diffFiles[0].renderIt).toEqual(true); expect(state.diffFiles[0].collapsed).toEqual(false); }); @@ -142,8 +139,7 @@ describe('DiffsStoreMutations', () => { }; const diffFile = { file_hash: options.fileHash, - highlighted_diff_lines: [], - parallel_diff_lines: [], + [INLINE_DIFF_LINES_KEY]: [], }; const state = { diffFiles: [diffFile], diffViewType: 'viewType' }; const lines = [{ old_line: 1, new_line: 1 }]; @@ -171,9 +167,7 @@ describe('DiffsStoreMutations', () => { ); expect(utils.addContextLines).toHaveBeenCalledWith({ - inlineLines: diffFile.highlighted_diff_lines, - parallelLines: diffFile.parallel_diff_lines, - diffViewType: 'viewType', + inlineLines: diffFile[INLINE_DIFF_LINES_KEY], contextLines: options.contextLines, bottom: options.params.bottom, lineNumbers: options.lineNumbers, @@ -225,19 +219,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - parallel_diff_lines: [ - { - left: { - line_code: 'ABC_1', - discussions: [], - }, - right: { - line_code: 'ABC_2', - discussions: [], - }, - }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [], @@ -267,12 +249,8 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); - - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1); }); it('should not duplicate discussions on line', () => { @@ -291,19 +269,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - parallel_diff_lines: [ - { - left: { - line_code: 'ABC_1', - discussions: [], - }, - right: { - line_code: 'ABC_2', - discussions: [], - }, - }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [], @@ -333,24 +299,16 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); - - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1); mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); - - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1); }); it('updates existing discussion', () => { @@ -369,19 +327,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - parallel_diff_lines: [ - { - left: { - line_code: 'ABC_1', - discussions: [], - }, - right: { - line_code: 'ABC_2', - discussions: [], - }, - }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [], @@ -411,12 +357,8 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); - - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1); mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion: { @@ -427,11 +369,8 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].notes.length).toBe(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].notes.length).toBe(1); - - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].resolved).toBe(true); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].resolved).toBe(true); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].notes.length).toBe(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].resolved).toBe(true); }); it('should not duplicate inline diff discussions', () => { @@ -450,7 +389,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [ @@ -472,7 +411,6 @@ describe('DiffsStoreMutations', () => { discussions: [], }, ], - parallel_diff_lines: [], }, ], }; @@ -497,7 +435,7 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toBe(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toBe(1); }); it('should add legacy discussions to the given line', () => { @@ -517,19 +455,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - parallel_diff_lines: [ - { - left: { - line_code: 'ABC_1', - discussions: [], - }, - right: { - line_code: 'ABC_1', - discussions: [], - }, - }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [], @@ -557,11 +483,8 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); - - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toEqual(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1); }); it('should add discussions by line_codes and positions attributes', () => { @@ -580,19 +503,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - parallel_diff_lines: [ - { - left: { - line_code: 'ABC_1', - discussions: [], - }, - right: { - line_code: 'ABC_1', - discussions: [], - }, - }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [], @@ -624,11 +535,8 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions).toHaveLength(1); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toBe(1); - - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions).toHaveLength(1); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toBe(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions).toHaveLength(1); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toBe(1); }); it('should add discussion to file', () => { @@ -638,8 +546,7 @@ describe('DiffsStoreMutations', () => { { file_hash: 'ABC', discussions: [], - parallel_diff_lines: [], - highlighted_diff_lines: [], + [INLINE_DIFF_LINES_KEY]: [], }, ], }; @@ -668,30 +575,7 @@ describe('DiffsStoreMutations', () => { diffFiles: [ { file_hash: 'ABC', - parallel_diff_lines: [ - { - left: { - line_code: 'ABC_1', - discussions: [ - { - id: 1, - line_code: 'ABC_1', - notes: [], - }, - { - id: 2, - line_code: 'ABC_1', - notes: [], - }, - ], - }, - right: { - line_code: 'ABC_1', - discussions: [], - }, - }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: 'ABC_1', discussions: [ @@ -717,8 +601,7 @@ describe('DiffsStoreMutations', () => { lineCode: 'ABC_1', }); - expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(0); - expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(0); + expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions.length).toEqual(0); }); }); @@ -738,15 +621,11 @@ describe('DiffsStoreMutations', () => { }); }); - describe('TOGGLE_SHOW_TREE_LIST', () => { - it('toggles showTreeList', () => { + describe('SET_SHOW_TREE_LIST', () => { + it('sets showTreeList', () => { const state = createState(); - mutations[types.TOGGLE_SHOW_TREE_LIST](state); - - expect(state.showTreeList).toBe(false, 'Failed to toggle showTreeList to false'); - - mutations[types.TOGGLE_SHOW_TREE_LIST](state); + mutations[types.SET_SHOW_TREE_LIST](state, true); expect(state.showTreeList).toBe(true, 'Failed to toggle showTreeList to true'); }); @@ -776,11 +655,7 @@ describe('DiffsStoreMutations', () => { it('sets hasForm on lines', () => { const file = { file_hash: 'hash', - parallel_diff_lines: [ - { left: { line_code: '123', hasForm: false }, right: {} }, - { left: {}, right: { line_code: '124', hasForm: false } }, - ], - highlighted_diff_lines: [ + [INLINE_DIFF_LINES_KEY]: [ { line_code: '123', hasForm: false }, { line_code: '124', hasForm: false }, ], @@ -795,11 +670,8 @@ describe('DiffsStoreMutations', () => { fileHash: 'hash', }); - expect(file.highlighted_diff_lines[0].hasForm).toBe(true); - expect(file.highlighted_diff_lines[1].hasForm).toBe(false); - - expect(file.parallel_diff_lines[0].left.hasForm).toBe(true); - expect(file.parallel_diff_lines[1].right.hasForm).toBe(false); + expect(file[INLINE_DIFF_LINES_KEY][0].hasForm).toBe(true); + expect(file[INLINE_DIFF_LINES_KEY][1].hasForm).toBe(false); }); }); @@ -885,8 +757,7 @@ describe('DiffsStoreMutations', () => { file_path: 'test', isLoadingFullFile: true, isShowingFullFile: false, - highlighted_diff_lines: [], - parallel_diff_lines: [], + [INLINE_DIFF_LINES_KEY]: [], }, ], }; @@ -903,8 +774,7 @@ describe('DiffsStoreMutations', () => { file_path: 'test', isLoadingFullFile: true, isShowingFullFile: false, - highlighted_diff_lines: [], - parallel_diff_lines: [], + [INLINE_DIFF_LINES_KEY]: [], }, ], }; @@ -927,80 +797,42 @@ describe('DiffsStoreMutations', () => { }); }); - describe('SET_HIDDEN_VIEW_DIFF_FILE_LINES', () => { - [ - { current: 'highlighted', hidden: 'parallel', diffViewType: 'inline' }, - { current: 'parallel', hidden: 'highlighted', diffViewType: 'parallel' }, - ].forEach(({ current, hidden, diffViewType }) => { - it(`sets the ${hidden} lines when diff view is ${diffViewType}`, () => { - const file = { file_path: 'test', parallel_diff_lines: [], highlighted_diff_lines: [] }; - const state = { - diffFiles: [file], - diffViewType, - }; - - mutations[types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { - filePath: 'test', - lines: ['test'], - }); - - expect(file[`${current}_diff_lines`]).toEqual([]); - expect(file[`${hidden}_diff_lines`]).toEqual(['test']); - }); - }); - }); - describe('SET_CURRENT_VIEW_DIFF_FILE_LINES', () => { - [ - { current: 'highlighted', hidden: 'parallel', diffViewType: 'inline' }, - { current: 'parallel', hidden: 'highlighted', diffViewType: 'parallel' }, - ].forEach(({ current, hidden, diffViewType }) => { - it(`sets the ${current} lines when diff view is ${diffViewType}`, () => { - const file = { file_path: 'test', parallel_diff_lines: [], highlighted_diff_lines: [] }; - const state = { - diffFiles: [file], - diffViewType, - }; - - mutations[types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { - filePath: 'test', - lines: ['test'], - }); - - expect(file[`${current}_diff_lines`]).toEqual(['test']); - expect(file[`${hidden}_diff_lines`]).toEqual([]); + it(`sets the highlighted lines`, () => { + const file = { file_path: 'test', [INLINE_DIFF_LINES_KEY]: [] }; + const state = { + diffFiles: [file], + }; + + mutations[types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { + filePath: 'test', + lines: ['test'], }); + + expect(file[INLINE_DIFF_LINES_KEY]).toEqual(['test']); }); }); describe('ADD_CURRENT_VIEW_DIFF_FILE_LINES', () => { - [ - { current: 'highlighted', hidden: 'parallel', diffViewType: 'inline' }, - { current: 'parallel', hidden: 'highlighted', diffViewType: 'parallel' }, - ].forEach(({ current, hidden, diffViewType }) => { - it(`pushes to ${current} lines when diff view is ${diffViewType}`, () => { - const file = { file_path: 'test', parallel_diff_lines: [], highlighted_diff_lines: [] }; - const state = { - diffFiles: [file], - diffViewType, - }; - - mutations[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { - filePath: 'test', - line: 'test', - }); - - expect(file[`${current}_diff_lines`]).toEqual(['test']); - expect(file[`${hidden}_diff_lines`]).toEqual([]); - - mutations[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { - filePath: 'test', - line: 'test2', - }); - - expect(file[`${current}_diff_lines`]).toEqual(['test', 'test2']); - expect(file[`${hidden}_diff_lines`]).toEqual([]); + it('pushes to inline lines', () => { + const file = { file_path: 'test', [INLINE_DIFF_LINES_KEY]: [] }; + const state = { + diffFiles: [file], + }; + + mutations[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { + filePath: 'test', + line: 'test', + }); + + expect(file[INLINE_DIFF_LINES_KEY]).toEqual(['test']); + + mutations[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { + filePath: 'test', + line: 'test2', }); + + expect(file[INLINE_DIFF_LINES_KEY]).toEqual(['test', 'test2']); }); }); @@ -1060,4 +892,18 @@ describe('DiffsStoreMutations', () => { expect(state.showSuggestPopover).toBe(false); }); }); + + describe('SET_FILE_BY_FILE', () => { + it.each` + value | opposite + ${true} | ${false} + ${false} | ${true} + `('sets viewDiffsFileByFile to $value', ({ value, opposite }) => { + const state = { viewDiffsFileByFile: opposite }; + + mutations[types.SET_FILE_BY_FILE](state, value); + + expect(state.viewDiffsFileByFile).toBe(value); + }); + }); }); diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js index 866be0abd22..7ee97224707 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -10,7 +10,7 @@ import { OLD_LINE_TYPE, MATCH_LINE_TYPE, INLINE_DIFF_VIEW_TYPE, - PARALLEL_DIFF_VIEW_TYPE, + INLINE_DIFF_LINES_KEY, } from '~/diffs/constants'; import { MERGE_REQUEST_NOTEABLE_TYPE } from '~/notes/constants'; import diffFileMockData from '../mock_data/diff_file'; @@ -20,14 +20,6 @@ import { noteableDataMock } from '../../notes/mock_data'; const getDiffFileMock = () => JSON.parse(JSON.stringify(diffFileMockData)); const getDiffMetadataMock = () => JSON.parse(JSON.stringify(diffMetadata)); -function extractLinesFromFile(file) { - const unpackedParallel = file.parallel_diff_lines - .flatMap(({ left, right }) => [left, right]) - .filter(Boolean); - - return [...file.highlighted_diff_lines, ...unpackedParallel]; -} - describe('DiffsStoreUtils', () => { describe('findDiffFile', () => { const files = [{ file_hash: 1, name: 'one' }]; @@ -45,7 +37,7 @@ describe('DiffsStoreUtils', () => { }); }); - describe('findIndexInInlineLines and findIndexInParallelLines', () => { + describe('findIndexInInlineLines', () => { const expectSet = (method, lines, invalidLines) => { expect(method(lines, { oldLineNumber: 3, newLineNumber: 5 })).toEqual(4); expect(method(invalidLines || lines, { oldLineNumber: 32, newLineNumber: 53 })).toEqual(-1); @@ -53,44 +45,26 @@ describe('DiffsStoreUtils', () => { describe('findIndexInInlineLines', () => { it('should return correct index for given line numbers', () => { - expectSet(utils.findIndexInInlineLines, getDiffFileMock().highlighted_diff_lines); - }); - }); - - describe('findIndexInParallelLines', () => { - it('should return correct index for given line numbers', () => { - expectSet(utils.findIndexInParallelLines, getDiffFileMock().parallel_diff_lines, []); + expectSet(utils.findIndexInInlineLines, getDiffFileMock()[INLINE_DIFF_LINES_KEY]); }); }); }); describe('getPreviousLineIndex', () => { - [ - { diffViewType: INLINE_DIFF_VIEW_TYPE, file: { parallel_diff_lines: [] } }, - { diffViewType: PARALLEL_DIFF_VIEW_TYPE, file: { highlighted_diff_lines: [] } }, - ].forEach(({ diffViewType, file }) => { - describe(`with diffViewType (${diffViewType}) in split diffs`, () => { - let diffFile; - - beforeEach(() => { - diffFile = { ...clone(diffFileMockData), ...file }; - }); + describe(`with diffViewType (inline) in split diffs`, () => { + let diffFile; - it('should return the correct previous line number', () => { - const emptyLines = - diffViewType === INLINE_DIFF_VIEW_TYPE - ? diffFile.parallel_diff_lines - : diffFile.highlighted_diff_lines; - - // This expectation asserts that we cannot possibly be using the opposite view type lines in the next expectation - expect(emptyLines.length).toBe(0); - expect( - utils.getPreviousLineIndex(diffViewType, diffFile, { - oldLineNumber: 3, - newLineNumber: 5, - }), - ).toBe(4); - }); + beforeEach(() => { + diffFile = { ...clone(diffFileMockData) }; + }); + + it('should return the correct previous line number', () => { + expect( + utils.getPreviousLineIndex(INLINE_DIFF_VIEW_TYPE, diffFile, { + oldLineNumber: 3, + newLineNumber: 5, + }), + ).toBe(4); }); }); }); @@ -100,82 +74,50 @@ describe('DiffsStoreUtils', () => { const diffFile = getDiffFileMock(); const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; const inlineIndex = utils.findIndexInInlineLines( - diffFile.highlighted_diff_lines, + diffFile[INLINE_DIFF_LINES_KEY], lineNumbers, ); - const parallelIndex = utils.findIndexInParallelLines( - diffFile.parallel_diff_lines, - lineNumbers, - ); - const atInlineIndex = diffFile.highlighted_diff_lines[inlineIndex]; - const atParallelIndex = diffFile.parallel_diff_lines[parallelIndex]; + const atInlineIndex = diffFile[INLINE_DIFF_LINES_KEY][inlineIndex]; utils.removeMatchLine(diffFile, lineNumbers, false); - expect(diffFile.highlighted_diff_lines[inlineIndex]).not.toEqual(atInlineIndex); - expect(diffFile.parallel_diff_lines[parallelIndex]).not.toEqual(atParallelIndex); + expect(diffFile[INLINE_DIFF_LINES_KEY][inlineIndex]).not.toEqual(atInlineIndex); utils.removeMatchLine(diffFile, lineNumbers, true); - expect(diffFile.highlighted_diff_lines[inlineIndex + 1]).not.toEqual(atInlineIndex); - expect(diffFile.parallel_diff_lines[parallelIndex + 1]).not.toEqual(atParallelIndex); + expect(diffFile[INLINE_DIFF_LINES_KEY][inlineIndex + 1]).not.toEqual(atInlineIndex); }); }); describe('addContextLines', () => { - [INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE].forEach(diffViewType => { - it(`should add context lines for ${diffViewType}`, () => { - const diffFile = getDiffFileMock(); - const inlineLines = diffFile.highlighted_diff_lines; - const parallelLines = diffFile.parallel_diff_lines; - const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; - const contextLines = [{ lineNumber: 42, line_code: '123' }]; - const options = { inlineLines, parallelLines, contextLines, lineNumbers, diffViewType }; - const inlineIndex = utils.findIndexInInlineLines(inlineLines, lineNumbers); - const parallelIndex = utils.findIndexInParallelLines(parallelLines, lineNumbers); - const normalizedParallelLine = { - left: options.contextLines[0], - right: options.contextLines[0], - line_code: '123', - }; + it(`should add context lines`, () => { + const diffFile = getDiffFileMock(); + const inlineLines = diffFile[INLINE_DIFF_LINES_KEY]; + const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; + const contextLines = [{ lineNumber: 42, line_code: '123' }]; + const options = { inlineLines, contextLines, lineNumbers }; + const inlineIndex = utils.findIndexInInlineLines(inlineLines, lineNumbers); - utils.addContextLines(options); + utils.addContextLines(options); - if (diffViewType === INLINE_DIFF_VIEW_TYPE) { - expect(inlineLines[inlineIndex]).toEqual(contextLines[0]); - } else { - expect(parallelLines[parallelIndex]).toEqual(normalizedParallelLine); - } - }); + expect(inlineLines[inlineIndex]).toEqual(contextLines[0]); + }); - it(`should add context lines properly with bottom parameter for ${diffViewType}`, () => { - const diffFile = getDiffFileMock(); - const inlineLines = diffFile.highlighted_diff_lines; - const parallelLines = diffFile.parallel_diff_lines; - const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; - const contextLines = [{ lineNumber: 42, line_code: '123' }]; - const options = { - inlineLines, - parallelLines, - contextLines, - lineNumbers, - bottom: true, - diffViewType, - }; - const normalizedParallelLine = { - left: options.contextLines[0], - right: options.contextLines[0], - line_code: '123', - }; + it(`should add context lines properly with bottom parameter`, () => { + const diffFile = getDiffFileMock(); + const inlineLines = diffFile[INLINE_DIFF_LINES_KEY]; + const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; + const contextLines = [{ lineNumber: 42, line_code: '123' }]; + const options = { + inlineLines, + contextLines, + lineNumbers, + bottom: true, + }; - utils.addContextLines(options); + utils.addContextLines(options); - if (diffViewType === INLINE_DIFF_VIEW_TYPE) { - expect(inlineLines[inlineLines.length - 1]).toEqual(contextLines[0]); - } else { - expect(parallelLines[parallelLines.length - 1]).toEqual(normalizedParallelLine); - } - }); + expect(inlineLines[inlineLines.length - 1]).toEqual(contextLines[0]); }); }); @@ -195,7 +137,6 @@ describe('DiffsStoreUtils', () => { new_line: 3, old_line: 1, }, - diffViewType: PARALLEL_DIFF_VIEW_TYPE, linePosition: LINE_POSITION_LEFT, lineRange: { start_line_code: 'abc_1_1', end_line_code: 'abc_2_2' }, }; @@ -256,7 +197,6 @@ describe('DiffsStoreUtils', () => { new_line: 3, old_line: 1, }, - diffViewType: PARALLEL_DIFF_VIEW_TYPE, linePosition: LINE_POSITION_LEFT, }; @@ -424,20 +364,6 @@ describe('DiffsStoreUtils', () => { 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); - }); - it.each` brokenSymlink ${false} @@ -474,35 +400,26 @@ describe('DiffsStoreUtils', () => { preparedDiff = { diff_files: [mock] }; splitInlineDiff = { - diff_files: [{ ...mock, parallel_diff_lines: undefined }], + diff_files: [{ ...mock }], }; splitParallelDiff = { - diff_files: [{ ...mock, highlighted_diff_lines: undefined }], + diff_files: [{ ...mock, [INLINE_DIFF_LINES_KEY]: undefined }], }; completedDiff = { - diff_files: [{ ...mock, highlighted_diff_lines: undefined }], + diff_files: [{ ...mock, [INLINE_DIFF_LINES_KEY]: undefined }], }; - preparedDiff.diff_files = utils.prepareDiffData(preparedDiff); - splitInlineDiff.diff_files = utils.prepareDiffData(splitInlineDiff); - splitParallelDiff.diff_files = utils.prepareDiffData(splitParallelDiff); - completedDiff.diff_files = utils.prepareDiffData(completedDiff, [mock]); + preparedDiff.diff_files = utils.prepareDiffData({ diff: preparedDiff }); + splitInlineDiff.diff_files = utils.prepareDiffData({ diff: splitInlineDiff }); + splitParallelDiff.diff_files = utils.prepareDiffData({ diff: splitParallelDiff }); + completedDiff.diff_files = utils.prepareDiffData({ + diff: completedDiff, + priorFiles: [mock], + }); }); it('sets the renderIt and collapsed attribute on files', () => { - const firstParallelDiffLine = preparedDiff.diff_files[0].parallel_diff_lines[2]; - - expect(firstParallelDiffLine.left.discussions.length).toBe(0); - expect(firstParallelDiffLine.left).not.toHaveAttr('text'); - expect(firstParallelDiffLine.right.discussions.length).toBe(0); - expect(firstParallelDiffLine.right).not.toHaveAttr('text'); - const firstParallelChar = firstParallelDiffLine.right.rich_text.charAt(0); - - expect(firstParallelChar).not.toBe(' '); - expect(firstParallelChar).not.toBe('+'); - expect(firstParallelChar).not.toBe('-'); - - const checkLine = preparedDiff.diff_files[0].highlighted_diff_lines[0]; + const checkLine = preparedDiff.diff_files[0][INLINE_DIFF_LINES_KEY][0]; expect(checkLine.discussions.length).toBe(0); expect(checkLine).not.toHaveAttr('text'); @@ -516,29 +433,14 @@ describe('DiffsStoreUtils', () => { expect(preparedDiff.diff_files[0].collapsed).toBeFalsy(); }); - it('adds line_code to all lines', () => { - expect( - preparedDiff.diff_files[0].parallel_diff_lines.filter(line => !line.line_code), - ).toHaveLength(0); - }); - - it('uses right line code if left has none', () => { - const firstLine = preparedDiff.diff_files[0].parallel_diff_lines[0]; - - expect(firstLine.line_code).toEqual(firstLine.right.line_code); - }); - it('guarantees an empty array for both diff styles', () => { - expect(splitInlineDiff.diff_files[0].parallel_diff_lines.length).toEqual(0); - expect(splitInlineDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0); - expect(splitParallelDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0); - expect(splitParallelDiff.diff_files[0].highlighted_diff_lines.length).toEqual(0); + expect(splitInlineDiff.diff_files[0][INLINE_DIFF_LINES_KEY].length).toBeGreaterThan(0); + expect(splitParallelDiff.diff_files[0][INLINE_DIFF_LINES_KEY].length).toEqual(0); }); it('merges existing diff files with newly loaded diff files to ensure split diffs are eventually completed', () => { expect(completedDiff.diff_files.length).toEqual(1); - expect(completedDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0); - expect(completedDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0); + expect(completedDiff.diff_files[0][INLINE_DIFF_LINES_KEY].length).toBeGreaterThan(0); }); it('leaves files in the existing state', () => { @@ -548,20 +450,26 @@ describe('DiffsStoreUtils', () => { content_sha: 'ABC', file_hash: 'DEF', }; - const updatedFilesList = utils.prepareDiffData({ diff_files: [fakeNewFile] }, priorFiles); + const updatedFilesList = utils.prepareDiffData({ + diff: { diff_files: [fakeNewFile] }, + priorFiles, + }); expect(updatedFilesList).toEqual([mock, fakeNewFile]); }); it('completes an existing split diff without overwriting existing diffs', () => { // The current state has a file that has only loaded inline lines - const priorFiles = [{ ...mock, parallel_diff_lines: [] }]; + const priorFiles = [{ ...mock }]; // The next (batch) load loads two files: the other half of that file, and a new file const fakeBatch = [ - { ...mock, highlighted_diff_lines: undefined }, - { ...mock, highlighted_diff_lines: undefined, content_sha: 'ABC', file_hash: 'DEF' }, + { ...mock, [INLINE_DIFF_LINES_KEY]: undefined }, + { ...mock, [INLINE_DIFF_LINES_KEY]: undefined, content_sha: 'ABC', file_hash: 'DEF' }, ]; - const updatedFilesList = utils.prepareDiffData({ diff_files: fakeBatch }, priorFiles); + const updatedFilesList = utils.prepareDiffData({ + diff: { diff_files: fakeBatch }, + priorFiles, + }); expect(updatedFilesList).toEqual([ mock, @@ -584,7 +492,7 @@ describe('DiffsStoreUtils', () => { ...splitInlineDiff.diff_files, ...splitParallelDiff.diff_files, ...completedDiff.diff_files, - ].flatMap(file => extractLinesFromFile(file)); + ].flatMap(file => [...file[INLINE_DIFF_LINES_KEY]]); lines.forEach(line => { expect(line.commentsDisabled).toBe(false); @@ -599,7 +507,7 @@ describe('DiffsStoreUtils', () => { beforeEach(() => { mock = getDiffMetadataMock(); - preparedDiffFiles = utils.prepareDiffData(mock); + preparedDiffFiles = utils.prepareDiffData({ diff: mock, meta: true }); }); it('sets the renderIt and collapsed attribute on files', () => { @@ -608,15 +516,14 @@ describe('DiffsStoreUtils', () => { }); it('guarantees an empty array of lines for both diff styles', () => { - expect(preparedDiffFiles[0].parallel_diff_lines.length).toEqual(0); - expect(preparedDiffFiles[0].highlighted_diff_lines.length).toEqual(0); + expect(preparedDiffFiles[0][INLINE_DIFF_LINES_KEY].length).toEqual(0); }); it('leaves files in the existing state', () => { const fileMock = getDiffFileMock(); const metaData = getDiffMetadataMock(); const priorFiles = [fileMock]; - const updatedFilesList = utils.prepareDiffData(metaData, priorFiles); + const updatedFilesList = utils.prepareDiffData({ diff: metaData, priorFiles, meta: true }); expect(updatedFilesList.length).toEqual(2); expect(updatedFilesList[0]).toEqual(fileMock); @@ -641,14 +548,13 @@ describe('DiffsStoreUtils', () => { const fileMock = getDiffFileMock(); const metaMock = getDiffMetadataMock(); const priorFiles = [{ ...fileMock }]; - const updatedFilesList = utils.prepareDiffData(metaMock, priorFiles); + const updatedFilesList = utils.prepareDiffData({ diff: metaMock, priorFiles, meta: true }); expect(updatedFilesList).toEqual([ fileMock, { ...metaMock.diff_files[0], - highlighted_diff_lines: [], - parallel_diff_lines: [], + [INLINE_DIFF_LINES_KEY]: [], }, ]); }); @@ -1217,30 +1123,19 @@ describe('DiffsStoreUtils', () => { it('converts inline diff lines to parallel diff lines', () => { const file = getDiffFileMock(); - expect(utils.parallelizeDiffLines(file.highlighted_diff_lines)).toEqual( + expect(utils.parallelizeDiffLines(file[INLINE_DIFF_LINES_KEY])).toEqual( file.parallel_diff_lines, ); }); - /** - * What's going on here? - * - * The inline version of parallelizeDiffLines simply keeps the difflines - * in the same order they are received as opposed to shuffling them - * to be "side by side". - * - * This keeps the underlying data structure the same which simplifies - * the components, but keeps the changes grouped together as users - * expect when viewing changes inline. - */ - it('converts inline diff lines to inline diff lines with a parallel structure', () => { + it('converts inline diff lines', () => { const file = getDiffFileMock(); const files = utils.parallelizeDiffLines(file.highlighted_diff_lines, true); expect(files[5].left).toEqual(file.parallel_diff_lines[5].left); expect(files[5].right).toBeNull(); - expect(files[6].left).toBeNull(); - expect(files[6].right).toEqual(file.parallel_diff_lines[5].right); + expect(files[6].left).toEqual(file.parallel_diff_lines[5].right); + expect(files[6].right).toBeNull(); }); }); }); diff --git a/spec/frontend/diffs/utils/diff_file_spec.js b/spec/frontend/diffs/utils/diff_file_spec.js new file mode 100644 index 00000000000..2e6247b8c07 --- /dev/null +++ b/spec/frontend/diffs/utils/diff_file_spec.js @@ -0,0 +1,126 @@ +import { prepareRawDiffFile } from '~/diffs/utils/diff_file'; + +function getDiffFiles() { + return [ + { + blob: { + id: 'C0473471', + }, + file_hash: 'ABC', // This file is just a normal file + file_identifier_hash: 'ABC1', + }, + { + blob: { + id: 'C0473472', + }, + file_hash: 'DEF', // This file replaces a symlink + file_identifier_hash: 'DEF1', + a_mode: '0', + b_mode: '0755', + }, + { + blob: { + id: 'C0473473', + }, + file_hash: 'DEF', // This symlink is replaced by a file + file_identifier_hash: 'DEF2', + a_mode: '120000', + b_mode: '0', + }, + { + blob: { + id: 'C0473474', + }, + file_hash: 'GHI', // This symlink replaces a file + file_identifier_hash: 'GHI1', + a_mode: '0', + b_mode: '120000', + }, + { + blob: { + id: 'C0473475', + }, + file_hash: 'GHI', // This file is replaced by a symlink + file_identifier_hash: 'GHI2', + a_mode: '0755', + b_mode: '0', + }, + ]; +} +function makeBrokenSymlinkObject(replaced, wasSymbolic, isSymbolic, wasReal, isReal) { + return { + replaced, + wasSymbolic, + isSymbolic, + wasReal, + isReal, + }; +} + +describe('diff_file utilities', () => { + describe('prepareRawDiffFile', () => { + let files; + + beforeEach(() => { + files = getDiffFiles(); + }); + + it.each` + fileIndex | description | brokenSymlink + ${0} | ${'a file that is not symlink-adjacent'} | ${false} + ${1} | ${'a file that replaces a symlink'} | ${makeBrokenSymlinkObject(false, false, false, false, true)} + ${2} | ${'a symlink that is replaced by a file'} | ${makeBrokenSymlinkObject(true, true, false, false, false)} + ${3} | ${'a symlink that replaces a file'} | ${makeBrokenSymlinkObject(false, false, true, false, false)} + ${4} | ${'a file that is replaced by a symlink'} | ${makeBrokenSymlinkObject(true, false, false, true, false)} + `( + 'properly marks $description with the correct .brokenSymlink value', + ({ fileIndex, brokenSymlink }) => { + const preppedRaw = prepareRawDiffFile({ + file: files[fileIndex], + allFiles: files, + }); + + expect(preppedRaw.brokenSymlink).toStrictEqual(brokenSymlink); + }, + ); + + it.each` + fileIndex | id + ${0} | ${'8dcd585e-a421-4dab-a04e-6f88c81b7b4c'} + ${1} | ${'3f178b78-392b-44a4-bd7d-5d6192208a97'} + ${2} | ${'3d9e1354-cddf-4a11-8234-f0413521b2e5'} + ${3} | ${'460f005b-d29d-43c1-9a08-099a7c7f08de'} + ${4} | ${'d8c89733-6ce1-4455-ae3d-f8aad6ee99f9'} + `('sets the file id properly { id: $id } on normal diff files', ({ fileIndex, id }) => { + const preppedFile = prepareRawDiffFile({ + file: files[fileIndex], + allFiles: files, + }); + + expect(preppedFile.id).toBe(id); + }); + + it('does not set the `id` property for metadata diff files', () => { + const preppedFile = prepareRawDiffFile({ + file: files[0], + allFiles: files, + meta: true, + }); + + expect(preppedFile).not.toHaveProp('id'); + }); + + it('does not set the id property if the file is missing a `blob.id`', () => { + const fileMissingContentSha = { ...files[0] }; + + delete fileMissingContentSha.blob.id; + + const preppedFile = prepareRawDiffFile({ + file: fileMissingContentSha, + allFiles: files, + }); + + expect(preppedFile).not.toHaveProp('id'); + }); + }); +}); diff --git a/spec/frontend/diffs/utils/preferences_spec.js b/spec/frontend/diffs/utils/preferences_spec.js new file mode 100644 index 00000000000..a48db1d7512 --- /dev/null +++ b/spec/frontend/diffs/utils/preferences_spec.js @@ -0,0 +1,40 @@ +import Cookies from 'js-cookie'; +import { getParameterValues } from '~/lib/utils/url_utility'; + +import { fileByFile } from '~/diffs/utils/preferences'; +import { + DIFF_FILE_BY_FILE_COOKIE_NAME, + DIFF_VIEW_FILE_BY_FILE, + DIFF_VIEW_ALL_FILES, +} from '~/diffs/constants'; + +jest.mock('~/lib/utils/url_utility'); + +describe('diffs preferences', () => { + describe('fileByFile', () => { + it.each` + result | preference | cookie | searchParam + ${false} | ${false} | ${undefined} | ${undefined} + ${true} | ${true} | ${undefined} | ${undefined} + ${true} | ${false} | ${DIFF_VIEW_FILE_BY_FILE} | ${undefined} + ${false} | ${true} | ${DIFF_VIEW_ALL_FILES} | ${undefined} + ${true} | ${false} | ${undefined} | ${[DIFF_VIEW_FILE_BY_FILE]} + ${false} | ${true} | ${undefined} | ${[DIFF_VIEW_ALL_FILES]} + ${true} | ${false} | ${DIFF_VIEW_FILE_BY_FILE} | ${[DIFF_VIEW_FILE_BY_FILE]} + ${true} | ${true} | ${DIFF_VIEW_ALL_FILES} | ${[DIFF_VIEW_FILE_BY_FILE]} + ${false} | ${false} | ${DIFF_VIEW_ALL_FILES} | ${[DIFF_VIEW_ALL_FILES]} + ${false} | ${true} | ${DIFF_VIEW_FILE_BY_FILE} | ${[DIFF_VIEW_ALL_FILES]} + `( + 'should return $result when { preference: $preference, cookie: $cookie, search: $searchParam }', + ({ result, preference, cookie, searchParam }) => { + if (cookie) { + Cookies.set(DIFF_FILE_BY_FILE_COOKIE_NAME, cookie); + } + + getParameterValues.mockReturnValue(searchParam); + + expect(fileByFile(preference)).toBe(result); + }, + ); + }); +}); |