From 311b0269b4eb9839fa63f80c8d7a58f32b8138a0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 18 Nov 2021 13:16:36 +0000 Subject: Add latest changes from gitlab-org/gitlab@14-5-stable-ee --- spec/frontend/diffs/components/app_spec.js | 34 ++---- .../diffs/components/diff_discussions_spec.js | 4 + .../diffs/components/diff_file_header_spec.js | 23 ++-- .../diffs/components/diff_line_note_form_spec.js | 89 +++++++++----- spec/frontend/diffs/components/tree_list_spec.js | 4 +- spec/frontend/diffs/store/actions_spec.js | 51 +++++--- spec/frontend/diffs/store/mutations_spec.js | 24 +++- spec/frontend/diffs/utils/diff_line_spec.js | 30 +++++ spec/frontend/diffs/utils/discussions_spec.js | 133 +++++++++++++++++++++ spec/frontend/diffs/utils/file_reviews_spec.js | 24 ++-- 10 files changed, 329 insertions(+), 87 deletions(-) create mode 100644 spec/frontend/diffs/utils/diff_line_spec.js create mode 100644 spec/frontend/diffs/utils/discussions_spec.js (limited to 'spec/frontend/diffs') diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 0527c2153f4..d50ac0529d6 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -388,15 +388,24 @@ describe('diffs/components/app', () => { wrapper.vm.jumpToFile(+1); - expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['diffs/scrollToFile', '222.js']); + expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual([ + 'diffs/scrollToFile', + { path: '222.js' }, + ]); store.state.diffs.currentDiffFileId = '222'; wrapper.vm.jumpToFile(+1); - expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['diffs/scrollToFile', '333.js']); + expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual([ + 'diffs/scrollToFile', + { path: '333.js' }, + ]); store.state.diffs.currentDiffFileId = '333'; wrapper.vm.jumpToFile(-1); - expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['diffs/scrollToFile', '222.js']); + expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual([ + 'diffs/scrollToFile', + { path: '222.js' }, + ]); }); it('does not jump to previous file from the first one', async () => { @@ -702,23 +711,4 @@ describe('diffs/components/app', () => { ); }); }); - - describe('fluid layout', () => { - beforeEach(() => { - setFixtures( - '
', - ); - }); - - it('removes limited container classes when on diffs tab', () => { - createComponent({ isFluidLayout: false, shouldShow: true }, () => {}, { - glFeatures: { mrChangesFluidLayout: true }, - }); - - const containerClassList = document.querySelector('.merge-request-container').classList; - - expect(containerClassList).not.toContain('container-limited'); - expect(containerClassList).not.toContain('limit-container-width'); - }); - }); }); diff --git a/spec/frontend/diffs/components/diff_discussions_spec.js b/spec/frontend/diffs/components/diff_discussions_spec.js index bd6f4cd2545..c847a79435a 100644 --- a/spec/frontend/diffs/components/diff_discussions_spec.js +++ b/spec/frontend/diffs/components/diff_discussions_spec.js @@ -1,6 +1,7 @@ import { GlIcon } from '@gitlab/ui'; import { mount, createLocalVue } from '@vue/test-utils'; import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; +import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions'; import { createStore } from '~/mr_notes/stores'; import DiscussionNotes from '~/notes/components/discussion_notes.vue'; import NoteableDiscussion from '~/notes/components/noteable_discussion.vue'; @@ -19,6 +20,9 @@ describe('DiffDiscussions', () => { store = createStore(); wrapper = mount(localVue.extend(DiffDiscussions), { store, + provide: { + discussionObserverHandler: discussionIntersectionObserverHandlerFactory(), + }, propsData: { discussions: getDiscussionsMockData(), ...props, diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js index b16ef8fe6b0..342b4bfcc50 100644 --- a/spec/frontend/diffs/components/diff_file_header_spec.js +++ b/spec/frontend/diffs/components/diff_file_header_spec.js @@ -7,7 +7,7 @@ import { mockTracking, triggerEvent } from 'helpers/tracking_helper'; import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; import { DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE } from '~/diffs/constants'; import { reviewFile } from '~/diffs/store/actions'; -import { SET_MR_FILE_REVIEWS } from '~/diffs/store/mutation_types'; +import { SET_DIFF_FILE_VIEWED, SET_MR_FILE_REVIEWS } from '~/diffs/store/mutation_types'; import { diffViewerModes } from '~/ide/constants'; import { scrollToElement } from '~/lib/utils/common_utils'; import { truncateSha } from '~/lib/utils/text_utility'; @@ -23,6 +23,7 @@ jest.mock('~/lib/utils/common_utils'); const diffFile = Object.freeze( Object.assign(diffDiscussionsMockData.diff_file, { id: '123', + file_hash: 'xyz', file_identifier_hash: 'abc', edit_path: 'link:/to/edit/path', blob: { @@ -58,7 +59,7 @@ describe('DiffFileHeader component', () => { toggleFileDiscussions: jest.fn(), toggleFileDiscussionWrappers: jest.fn(), toggleFullDiff: jest.fn(), - toggleActiveFileByHash: jest.fn(), + setCurrentFileHash: jest.fn(), setFileCollapsedByUser: jest.fn(), reviewFile: jest.fn(), }, @@ -240,18 +241,19 @@ describe('DiffFileHeader component', () => { }); describe('for any file', () => { - const otherModes = Object.keys(diffViewerModes).filter((m) => m !== 'mode_changed'); + const allModes = Object.keys(diffViewerModes).map((m) => [m]); - it('for mode_changed file mode displays mode changes', () => { + it.each(allModes)('for %s file mode displays mode changes', (mode) => { createComponent({ props: { diffFile: { ...diffFile, + mode_changed: true, a_mode: 'old-mode', b_mode: 'new-mode', viewer: { ...diffFile.viewer, - name: diffViewerModes.mode_changed, + name: diffViewerModes[mode], }, }, }, @@ -259,13 +261,14 @@ describe('DiffFileHeader component', () => { expect(findModeChangedLine().text()).toMatch(/old-mode.+new-mode/); }); - it.each(otherModes.map((m) => [m]))( + it.each(allModes.filter((m) => m[0] !== 'mode_changed'))( 'for %s file mode does not display mode changes', (mode) => { createComponent({ props: { diffFile: { ...diffFile, + mode_changed: false, a_mode: 'old-mode', b_mode: 'new-mode', viewer: { @@ -553,7 +556,13 @@ describe('DiffFileHeader component', () => { reviewFile, { file, reviewed: true }, {}, - [{ type: SET_MR_FILE_REVIEWS, payload: { [file.file_identifier_hash]: [file.id] } }], + [ + { type: SET_DIFF_FILE_VIEWED, payload: { id: file.file_hash, seen: true } }, + { + type: SET_MR_FILE_REVIEWS, + payload: { [file.file_identifier_hash]: [file.id, `hash:${file.file_hash}`] }, + }, + ], [], ); }); 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 a192f7e2e9a..0ccf996e220 100644 --- a/spec/frontend/diffs/components/diff_line_note_form_spec.js +++ b/spec/frontend/diffs/components/diff_line_note_form_spec.js @@ -1,10 +1,18 @@ import { shallowMount } from '@vue/test-utils'; +import { nextTick } from 'vue'; import DiffLineNoteForm from '~/diffs/components/diff_line_note_form.vue'; import { createStore } from '~/mr_notes/stores'; import NoteForm from '~/notes/components/note_form.vue'; +import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import { noteableDataMock } from '../../notes/mock_data'; import diffFileMockData from '../mock_data/diff_file'; +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal', () => { + return { + confirmAction: jest.fn(), + }; +}); + describe('DiffLineNoteForm', () => { let wrapper; let diffFile; @@ -24,57 +32,68 @@ describe('DiffLineNoteForm', () => { return shallowMount(DiffLineNoteForm, { store, propsData: { - diffFileHash: diffFile.file_hash, - diffLines, - line: diffLines[0], - noteTargetLine: diffLines[0], + ...{ + diffFileHash: diffFile.file_hash, + diffLines, + line: diffLines[1], + range: { start: diffLines[0], end: diffLines[1] }, + noteTargetLine: diffLines[1], + }, + ...(args.props || {}), }, }); }; + const findNoteForm = () => wrapper.findComponent(NoteForm); + describe('methods', () => { beforeEach(() => { wrapper = createComponent(); }); describe('handleCancelCommentForm', () => { + afterEach(() => { + confirmAction.mockReset(); + }); + it('should ask for confirmation when shouldConfirm and isDirty passed as truthy', () => { - jest.spyOn(window, 'confirm').mockReturnValue(false); + confirmAction.mockResolvedValueOnce(false); - wrapper.vm.handleCancelCommentForm(true, true); + findNoteForm().vm.$emit('cancelForm', true, true); - expect(window.confirm).toHaveBeenCalled(); + expect(confirmAction).toHaveBeenCalled(); }); - it('should ask for confirmation when one of the params false', () => { - jest.spyOn(window, 'confirm').mockReturnValue(false); + it('should not ask for confirmation when one of the params false', () => { + confirmAction.mockResolvedValueOnce(false); - wrapper.vm.handleCancelCommentForm(true, false); + findNoteForm().vm.$emit('cancelForm', true, false); - expect(window.confirm).not.toHaveBeenCalled(); + expect(confirmAction).not.toHaveBeenCalled(); - wrapper.vm.handleCancelCommentForm(false, true); + findNoteForm().vm.$emit('cancelForm', false, true); - expect(window.confirm).not.toHaveBeenCalled(); + expect(confirmAction).not.toHaveBeenCalled(); }); - it('should call cancelCommentForm with lineCode', (done) => { - jest.spyOn(window, 'confirm').mockImplementation(() => {}); + it('should call cancelCommentForm with lineCode', async () => { + confirmAction.mockResolvedValueOnce(true); jest.spyOn(wrapper.vm, 'cancelCommentForm').mockImplementation(() => {}); jest.spyOn(wrapper.vm, 'resetAutoSave').mockImplementation(() => {}); - wrapper.vm.handleCancelCommentForm(); - expect(window.confirm).not.toHaveBeenCalled(); - wrapper.vm.$nextTick(() => { - expect(wrapper.vm.cancelCommentForm).toHaveBeenCalledWith({ - lineCode: diffLines[0].line_code, - fileHash: wrapper.vm.diffFileHash, - }); + findNoteForm().vm.$emit('cancelForm', true, true); + + await nextTick(); + + expect(confirmAction).toHaveBeenCalled(); - expect(wrapper.vm.resetAutoSave).toHaveBeenCalled(); + await nextTick(); - done(); + expect(wrapper.vm.cancelCommentForm).toHaveBeenCalledWith({ + lineCode: diffLines[1].line_code, + fileHash: wrapper.vm.diffFileHash, }); + expect(wrapper.vm.resetAutoSave).toHaveBeenCalled(); }); }); @@ -88,13 +107,13 @@ describe('DiffLineNoteForm', () => { start: { line_code: wrapper.vm.commentLineStart.line_code, type: wrapper.vm.commentLineStart.type, - new_line: 1, + new_line: 2, old_line: null, }, end: { line_code: wrapper.vm.line.line_code, type: wrapper.vm.line.type, - new_line: 1, + new_line: 2, old_line: null, }, }; @@ -118,9 +137,25 @@ describe('DiffLineNoteForm', () => { }); }); + describe('created', () => { + it('should use the provided `range` of lines', () => { + wrapper = createComponent(); + + expect(wrapper.vm.lines.start).toBe(diffLines[0]); + expect(wrapper.vm.lines.end).toBe(diffLines[1]); + }); + + it("should fill the internal `lines` data with the provided `line` if there's no provided `range", () => { + wrapper = createComponent({ props: { range: null } }); + + expect(wrapper.vm.lines.start).toBe(diffLines[1]); + expect(wrapper.vm.lines.end).toBe(diffLines[1]); + }); + }); + describe('mounted', () => { it('should init autosave', () => { - const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1'; + const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2'; wrapper = createComponent(); expect(wrapper.vm.autosave).toBeDefined(); diff --git a/spec/frontend/diffs/components/tree_list_spec.js b/spec/frontend/diffs/components/tree_list_spec.js index f316a9fdf01..31044b0818c 100644 --- a/spec/frontend/diffs/components/tree_list_spec.js +++ b/spec/frontend/diffs/components/tree_list_spec.js @@ -113,7 +113,9 @@ describe('Diffs tree list component', () => { wrapper.find('.file-row').trigger('click'); - expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', 'app/index.js'); + expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', { + path: 'app/index.js', + }); }); it('renders as file list when renderTreeList is false', () => { diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 85734e05aeb..b5003a54917 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -99,6 +99,10 @@ describe('DiffsStoreActions', () => { const projectPath = '/root/project'; const dismissEndpoint = '/-/user_callouts'; const showSuggestPopover = false; + const mrReviews = { + a: ['z', 'hash:a'], + b: ['y', 'hash:a'], + }; testAction( setBaseConfig, @@ -110,6 +114,7 @@ describe('DiffsStoreActions', () => { projectPath, dismissEndpoint, showSuggestPopover, + mrReviews, }, { endpoint: '', @@ -131,8 +136,21 @@ describe('DiffsStoreActions', () => { projectPath, dismissEndpoint, showSuggestPopover, + mrReviews, }, }, + { + type: types.SET_DIFF_FILE_VIEWED, + payload: { id: 'z', seen: true }, + }, + { + type: types.SET_DIFF_FILE_VIEWED, + payload: { id: 'a', seen: true }, + }, + { + type: types.SET_DIFF_FILE_VIEWED, + payload: { id: 'y', seen: true }, + }, ], [], done, @@ -190,10 +208,10 @@ describe('DiffsStoreActions', () => { { type: types.SET_RETRIEVING_BATCHES, payload: true }, { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } }, { type: types.SET_BATCH_LOADING_STATE, payload: 'loaded' }, - { type: types.VIEW_DIFF_FILE, payload: 'test' }, + { type: types.SET_CURRENT_DIFF_FILE, payload: 'test' }, { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res2.diff_files } }, { type: types.SET_BATCH_LOADING_STATE, payload: 'loaded' }, - { type: types.VIEW_DIFF_FILE, payload: 'test2' }, + { type: types.SET_CURRENT_DIFF_FILE, payload: 'test2' }, { type: types.SET_RETRIEVING_BATCHES, payload: false }, { type: types.SET_BATCH_LOADING_STATE, payload: 'error' }, ], @@ -307,7 +325,7 @@ describe('DiffsStoreActions', () => { it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => { testAction(setHighlightedRow, 'ABC_123', {}, [ { type: types.SET_HIGHLIGHTED_ROW, payload: 'ABC_123' }, - { type: types.VIEW_DIFF_FILE, payload: 'ABC' }, + { type: types.SET_CURRENT_DIFF_FILE, payload: 'ABC' }, ]); }); }); @@ -890,12 +908,12 @@ describe('DiffsStoreActions', () => { }, }; - scrollToFile({ state, commit, getters }, 'path'); + scrollToFile({ state, commit, getters }, { path: 'path' }); expect(document.location.hash).toBe('#test'); }); - it('commits VIEW_DIFF_FILE', () => { + it('commits SET_CURRENT_DIFF_FILE', () => { const state = { treeEntries: { path: { @@ -904,9 +922,9 @@ describe('DiffsStoreActions', () => { }, }; - scrollToFile({ state, commit, getters }, 'path'); + scrollToFile({ state, commit, getters }, { path: 'path' }); - expect(commit).toHaveBeenCalledWith(types.VIEW_DIFF_FILE, 'test'); + expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, 'test'); }); }); @@ -1428,7 +1446,7 @@ describe('DiffsStoreActions', () => { }); describe('setCurrentDiffFileIdFromNote', () => { - it('commits VIEW_DIFF_FILE', () => { + it('commits SET_CURRENT_DIFF_FILE', () => { const commit = jest.fn(); const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { @@ -1438,10 +1456,10 @@ describe('DiffsStoreActions', () => { setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); - expect(commit).toHaveBeenCalledWith(types.VIEW_DIFF_FILE, '123'); + expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, '123'); }); - it('does not commit VIEW_DIFF_FILE when discussion has no diff_file', () => { + it('does not commit SET_CURRENT_DIFF_FILE when discussion has no diff_file', () => { const commit = jest.fn(); const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { @@ -1454,7 +1472,7 @@ describe('DiffsStoreActions', () => { expect(commit).not.toHaveBeenCalled(); }); - it('does not commit VIEW_DIFF_FILE when diff file does not exist', () => { + it('does not commit SET_CURRENT_DIFF_FILE when diff file does not exist', () => { const commit = jest.fn(); const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { @@ -1469,12 +1487,12 @@ describe('DiffsStoreActions', () => { }); describe('navigateToDiffFileIndex', () => { - it('commits VIEW_DIFF_FILE', (done) => { + it('commits SET_CURRENT_DIFF_FILE', (done) => { testAction( navigateToDiffFileIndex, 0, { diffFiles: [{ file_hash: '123' }] }, - [{ type: types.VIEW_DIFF_FILE, payload: '123' }], + [{ type: types.SET_CURRENT_DIFF_FILE, payload: '123' }], [], done, ); @@ -1523,13 +1541,14 @@ describe('DiffsStoreActions', () => { describe('reviewFile', () => { const file = { id: '123', + file_hash: 'xyz', file_identifier_hash: 'abc', load_collapsed_diff_url: 'gitlab-org/gitlab-test/-/merge_requests/1/diffs', }; it.each` - reviews | diffFile | reviewed - ${{ abc: ['123'] }} | ${file} | ${true} - ${{}} | ${file} | ${false} + reviews | diffFile | reviewed + ${{ abc: ['123', 'hash:xyz'] }} | ${file} | ${true} + ${{}} | ${file} | ${false} `( 'sets reviews ($reviews) to localStorage and state for file $file if it is marked reviewed=$reviewed', ({ reviews, diffFile, reviewed }) => { diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index fc9ba223d5a..c104fcd5fb9 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -633,16 +633,36 @@ describe('DiffsStoreMutations', () => { }); }); - describe('VIEW_DIFF_FILE', () => { + describe('SET_CURRENT_DIFF_FILE', () => { it('updates currentDiffFileId', () => { const state = createState(); - mutations[types.VIEW_DIFF_FILE](state, 'somefileid'); + mutations[types.SET_CURRENT_DIFF_FILE](state, 'somefileid'); expect(state.currentDiffFileId).toBe('somefileid'); }); }); + describe('SET_DIFF_FILE_VIEWED', () => { + let state; + + beforeEach(() => { + state = { + viewedDiffFileIds: { 123: true }, + }; + }); + + it.each` + id | bool | outcome + ${'abc'} | ${true} | ${{ 123: true, abc: true }} + ${'123'} | ${false} | ${{ 123: false }} + `('sets the viewed files list to $bool for the id $id', ({ id, bool, outcome }) => { + mutations[types.SET_DIFF_FILE_VIEWED](state, { id, seen: bool }); + + expect(state.viewedDiffFileIds).toEqual(outcome); + }); + }); + describe('Set highlighted row', () => { it('sets highlighted row', () => { const state = createState(); diff --git a/spec/frontend/diffs/utils/diff_line_spec.js b/spec/frontend/diffs/utils/diff_line_spec.js new file mode 100644 index 00000000000..adcb4a4433c --- /dev/null +++ b/spec/frontend/diffs/utils/diff_line_spec.js @@ -0,0 +1,30 @@ +import { pickDirection } from '~/diffs/utils/diff_line'; + +describe('diff_line utilities', () => { + describe('pickDirection', () => { + const left = { + line_code: 'left', + }; + const right = { + line_code: 'right', + }; + const defaultLine = { + left, + right, + }; + + it.each` + code | pick | line | pickDescription + ${'left'} | ${left} | ${defaultLine} | ${'the left line'} + ${'right'} | ${right} | ${defaultLine} | ${'the right line'} + ${'junk'} | ${left} | ${defaultLine} | ${'the default: the left line'} + ${'junk'} | ${right} | ${{ right }} | ${"the right line if there's no left line to default to"} + ${'right'} | ${left} | ${{ left }} | ${"the left line when there isn't a right line to match"} + `( + 'when provided a line and a line code `$code`, picks $pickDescription', + ({ code, line, pick }) => { + expect(pickDirection({ line, code })).toBe(pick); + }, + ); + }); +}); diff --git a/spec/frontend/diffs/utils/discussions_spec.js b/spec/frontend/diffs/utils/discussions_spec.js new file mode 100644 index 00000000000..9a3d442d943 --- /dev/null +++ b/spec/frontend/diffs/utils/discussions_spec.js @@ -0,0 +1,133 @@ +import { discussionIntersectionObserverHandlerFactory } from '~/diffs/utils/discussions'; + +describe('Diff Discussions Utils', () => { + describe('discussionIntersectionObserverHandlerFactory', () => { + it('creates a handler function', () => { + expect(discussionIntersectionObserverHandlerFactory()).toBeInstanceOf(Function); + }); + + describe('intersection observer handler', () => { + const functions = { + setCurrentDiscussionId: jest.fn(), + getPreviousUnresolvedDiscussionId: jest.fn().mockImplementation((id) => { + return Number(id) - 1; + }), + }; + const defaultProcessableWrapper = { + entry: { + time: 0, + isIntersecting: true, + rootBounds: { + bottom: 0, + }, + boundingClientRect: { + top: 0, + }, + }, + currentDiscussion: { + id: 1, + }, + isFirstUnresolved: false, + isDiffsPage: true, + }; + let handler; + let getMock; + let setMock; + + beforeEach(() => { + functions.setCurrentDiscussionId.mockClear(); + functions.getPreviousUnresolvedDiscussionId.mockClear(); + + defaultProcessableWrapper.functions = functions; + + setMock = functions.setCurrentDiscussionId.mock; + getMock = functions.getPreviousUnresolvedDiscussionId.mock; + handler = discussionIntersectionObserverHandlerFactory(); + }); + + it('debounces multiple simultaneous requests into one queue', () => { + handler(defaultProcessableWrapper); + handler(defaultProcessableWrapper); + handler(defaultProcessableWrapper); + handler(defaultProcessableWrapper); + + expect(setTimeout).toHaveBeenCalledTimes(4); + expect(clearTimeout).toHaveBeenCalledTimes(3); + + // By only advancing to one timer, we ensure it's all being batched into one queue + jest.advanceTimersToNextTimer(); + + expect(functions.setCurrentDiscussionId).toHaveBeenCalledTimes(4); + }); + + it('properly processes, sorts and executes the correct actions for a set of observed intersections', () => { + handler(defaultProcessableWrapper); + handler({ + // This observation is here to be filtered out because it's a scrollDown + ...defaultProcessableWrapper, + entry: { + ...defaultProcessableWrapper.entry, + isIntersecting: false, + boundingClientRect: { top: 10 }, + rootBounds: { bottom: 100 }, + }, + }); + handler({ + ...defaultProcessableWrapper, + entry: { + ...defaultProcessableWrapper.entry, + time: 101, + isIntersecting: false, + rootBounds: { bottom: -100 }, + }, + currentDiscussion: { id: 20 }, + }); + handler({ + ...defaultProcessableWrapper, + entry: { + ...defaultProcessableWrapper.entry, + time: 100, + isIntersecting: false, + boundingClientRect: { top: 100 }, + }, + currentDiscussion: { id: 30 }, + isDiffsPage: false, + }); + handler({ + ...defaultProcessableWrapper, + isFirstUnresolved: true, + entry: { + ...defaultProcessableWrapper.entry, + time: 100, + isIntersecting: false, + boundingClientRect: { top: 200 }, + }, + }); + + jest.advanceTimersToNextTimer(); + + expect(setMock.calls.length).toBe(4); + expect(setMock.calls[0]).toEqual([1]); + expect(setMock.calls[1]).toEqual([29]); + expect(setMock.calls[2]).toEqual([null]); + expect(setMock.calls[3]).toEqual([19]); + + expect(getMock.calls.length).toBe(2); + expect(getMock.calls[0]).toEqual([30, false]); + expect(getMock.calls[1]).toEqual([20, true]); + + [ + setMock.invocationCallOrder[0], + getMock.invocationCallOrder[0], + setMock.invocationCallOrder[1], + setMock.invocationCallOrder[2], + getMock.invocationCallOrder[1], + setMock.invocationCallOrder[3], + ].forEach((order, idx, list) => { + // Compare each invocation sequence to the one before it (except the first one) + expect(list[idx - 1] || -1).toBeLessThan(order); + }); + }); + }); + }); +}); diff --git a/spec/frontend/diffs/utils/file_reviews_spec.js b/spec/frontend/diffs/utils/file_reviews_spec.js index 230ec12409c..ccd27a5ae3e 100644 --- a/spec/frontend/diffs/utils/file_reviews_spec.js +++ b/spec/frontend/diffs/utils/file_reviews_spec.js @@ -11,14 +11,14 @@ import { function getDefaultReviews() { return { - abc: ['123', '098'], + abc: ['123', 'hash:xyz', '098', 'hash:uvw'], }; } describe('File Review(s) utilities', () => { const mrPath = 'my/fake/mr/42'; const storageKey = `${mrPath}-file-reviews`; - const file = { id: '123', file_identifier_hash: 'abc' }; + const file = { id: '123', file_hash: 'xyz', file_identifier_hash: 'abc' }; const storedValue = JSON.stringify(getDefaultReviews()); let reviews; @@ -44,14 +44,14 @@ describe('File Review(s) utilities', () => { }); describe('reviewStatuses', () => { - const file1 = { id: '123', file_identifier_hash: 'abc' }; - const file2 = { id: '098', file_identifier_hash: 'abc' }; + const file1 = { id: '123', hash: 'xyz', file_identifier_hash: 'abc' }; + const file2 = { id: '098', hash: 'uvw', file_identifier_hash: 'abc' }; it.each` mrReviews | files | fileReviews ${{}} | ${[file1, file2]} | ${{ 123: false, '098': false }} - ${{ abc: ['123'] }} | ${[file1, file2]} | ${{ 123: true, '098': false }} - ${{ abc: ['098'] }} | ${[file1, file2]} | ${{ 123: false, '098': true }} + ${{ abc: ['123', 'hash:xyz'] }} | ${[file1, file2]} | ${{ 123: true, '098': false }} + ${{ abc: ['098', 'hash:uvw'] }} | ${[file1, file2]} | ${{ 123: false, '098': true }} ${{ def: ['123'] }} | ${[file1, file2]} | ${{ 123: false, '098': false }} ${{ abc: ['123'], def: ['098'] }} | ${[]} | ${{}} `( @@ -128,7 +128,7 @@ describe('File Review(s) utilities', () => { describe('markFileReview', () => { it("adds a review when there's nothing that already exists", () => { - expect(markFileReview(null, file)).toStrictEqual({ abc: ['123'] }); + expect(markFileReview(null, file)).toStrictEqual({ abc: ['123', 'hash:xyz'] }); }); it("overwrites an existing review if it's for the same file (identifier hash)", () => { @@ -136,15 +136,15 @@ describe('File Review(s) utilities', () => { }); it('removes a review from the list when `reviewed` is `false`', () => { - expect(markFileReview(reviews, file, false)).toStrictEqual({ abc: ['098'] }); + expect(markFileReview(reviews, file, false)).toStrictEqual({ abc: ['098', 'hash:uvw'] }); }); it('adds a new review if the file ID is new', () => { - const updatedFile = { ...file, id: '098' }; - const allReviews = markFileReview({ abc: ['123'] }, updatedFile); + const updatedFile = { ...file, id: '098', file_hash: 'uvw' }; + const allReviews = markFileReview({ abc: ['123', 'hash:xyz'] }, updatedFile); expect(allReviews).toStrictEqual(getDefaultReviews()); - expect(allReviews.abc).toStrictEqual(['123', '098']); + expect(allReviews.abc).toStrictEqual(['123', 'hash:xyz', '098', 'hash:uvw']); }); it.each` @@ -158,7 +158,7 @@ describe('File Review(s) utilities', () => { it('removes the file key if there are no more reviews for it', () => { let updated = markFileReview(reviews, file, false); - updated = markFileReview(updated, { ...file, id: '098' }, false); + updated = markFileReview(updated, { ...file, id: '098', file_hash: 'uvw' }, false); expect(updated).toStrictEqual({}); }); -- cgit v1.2.3