diff options
Diffstat (limited to 'spec/frontend/diffs')
| -rw-r--r-- | spec/frontend/diffs/components/diff_row_utils_spec.js | 56 | ||||
| -rw-r--r-- | spec/frontend/diffs/mock_data/diff_file.js | 30 | ||||
| -rw-r--r-- | spec/frontend/diffs/store/actions_spec.js | 38 | ||||
| -rw-r--r-- | spec/frontend/diffs/store/utils_spec.js | 54 | ||||
| -rw-r--r-- | spec/frontend/diffs/utils/tree_worker_utils_spec.js | 64 |
5 files changed, 211 insertions, 31 deletions
diff --git a/spec/frontend/diffs/components/diff_row_utils_spec.js b/spec/frontend/diffs/components/diff_row_utils_spec.js index 8b25691ce34..a6f508c73eb 100644 --- a/spec/frontend/diffs/components/diff_row_utils_spec.js +++ b/spec/frontend/diffs/components/diff_row_utils_spec.js @@ -9,6 +9,18 @@ import { const LINE_CODE = 'abc123'; +function problemsClone({ + brokenSymlink = false, + brokenLineCode = false, + fileOnlyMoved = false, +} = {}) { + return { + brokenSymlink, + brokenLineCode, + fileOnlyMoved, + }; +} + describe('isHighlighted', () => { it('should return true if line is highlighted', () => { const line = { line_code: LINE_CODE }; @@ -137,9 +149,12 @@ describe('classNameMapCell', () => { describe('addCommentTooltip', () => { const brokenSymLinkTooltip = - 'Commenting on symbolic links that replace or are replaced by files is currently not supported.'; + 'Commenting on symbolic links that replace or are replaced by files is not supported'; const brokenRealTooltip = - 'Commenting on files that replace or are replaced by symbolic links is currently not supported.'; + 'Commenting on files that replace or are replaced by symbolic links is not supported'; + const lineMovedOrRenamedFileTooltip = + 'Commenting on files that are only moved or renamed is not supported'; + const lineWithNoLineCodeTooltip = 'Commenting on this line is not supported'; const dragTooltip = 'Add a comment to this line or drag for multiple lines'; it('should return default tooltip', () => { @@ -147,24 +162,38 @@ describe('addCommentTooltip', () => { }); it('should return drag comment tooltip when dragging is enabled', () => { - expect(utils.addCommentTooltip({})).toEqual(dragTooltip); + expect(utils.addCommentTooltip({ problems: problemsClone() })).toEqual(dragTooltip); }); it('should return broken symlink tooltip', () => { - expect(utils.addCommentTooltip({ commentsDisabled: { wasSymbolic: true } })).toEqual( - brokenSymLinkTooltip, - ); - expect(utils.addCommentTooltip({ commentsDisabled: { isSymbolic: true } })).toEqual( - brokenSymLinkTooltip, - ); + expect( + utils.addCommentTooltip({ + problems: problemsClone({ brokenSymlink: { wasSymbolic: true } }), + }), + ).toEqual(brokenSymLinkTooltip); + expect( + utils.addCommentTooltip({ problems: problemsClone({ brokenSymlink: { isSymbolic: true } }) }), + ).toEqual(brokenSymLinkTooltip); }); it('should return broken real tooltip', () => { - expect(utils.addCommentTooltip({ commentsDisabled: { wasReal: true } })).toEqual( - brokenRealTooltip, + expect( + utils.addCommentTooltip({ problems: problemsClone({ brokenSymlink: { wasReal: true } }) }), + ).toEqual(brokenRealTooltip); + expect( + utils.addCommentTooltip({ problems: problemsClone({ brokenSymlink: { isReal: true } }) }), + ).toEqual(brokenRealTooltip); + }); + + it('reports a tooltip when the line is in a file that has only been moved or renamed', () => { + expect(utils.addCommentTooltip({ problems: problemsClone({ fileOnlyMoved: true }) })).toEqual( + lineMovedOrRenamedFileTooltip, ); - expect(utils.addCommentTooltip({ commentsDisabled: { isReal: true } })).toEqual( - brokenRealTooltip, + }); + + it("reports a tooltip when the line doesn't have a line code to leave a comment on", () => { + expect(utils.addCommentTooltip({ problems: problemsClone({ brokenLineCode: true }) })).toEqual( + lineWithNoLineCodeTooltip, ); }); }); @@ -211,6 +240,7 @@ describe('mapParallel', () => { discussions: [{}], discussionsExpanded: true, hasForm: true, + problems: problemsClone(), }; const content = { diffFile: {}, diff --git a/spec/frontend/diffs/mock_data/diff_file.js b/spec/frontend/diffs/mock_data/diff_file.js index dd200b0248c..e0e5778e0d5 100644 --- a/spec/frontend/diffs/mock_data/diff_file.js +++ b/spec/frontend/diffs/mock_data/diff_file.js @@ -1,3 +1,11 @@ +function problemsClone() { + return { + brokenSymlink: false, + brokenLineCode: false, + fileOnlyMoved: false, + }; +} + export const getDiffFileMock = () => ({ submodule: false, submodule_link: null, @@ -61,6 +69,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', rich_text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', @@ -71,6 +80,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC2" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', @@ -81,6 +91,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', @@ -91,6 +102,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC4" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', @@ -101,6 +113,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_6', @@ -111,6 +124,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC6" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC6" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_7', @@ -121,6 +135,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC7" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_9', @@ -131,6 +146,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC7" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, { line_code: null, @@ -144,6 +160,7 @@ export const getDiffFileMock = () => ({ old_pos: 3, new_pos: 5, }, + problems: problemsClone(), }, ], parallel_diff_lines: [ @@ -158,6 +175,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', rich_text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -171,6 +189,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC2" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -183,6 +202,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', meta_data: null, + problems: problemsClone(), }, right: { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', @@ -193,6 +213,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -205,6 +226,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC4" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, right: { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', @@ -215,6 +237,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC4" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -227,6 +250,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', meta_data: null, + problems: problemsClone(), }, right: { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', @@ -237,6 +261,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -249,6 +274,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC6" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC6" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, right: { line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_7', @@ -259,6 +285,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC7" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -272,6 +299,7 @@ export const getDiffFileMock = () => ({ text: '<span id="LC7" class="line" lang="plaintext"></span>\n', rich_text: '<span id="LC7" class="line" lang="plaintext"></span>\n', meta_data: null, + problems: problemsClone(), }, }, { @@ -287,6 +315,7 @@ export const getDiffFileMock = () => ({ old_pos: 3, new_pos: 5, }, + problems: problemsClone(), }, right: { line_code: null, @@ -300,6 +329,7 @@ export const getDiffFileMock = () => ({ old_pos: 3, new_pos: 5, }, + problems: problemsClone(), }, }, ], diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index bf75f956d7f..87366cdbfc5 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -183,11 +183,11 @@ describe('DiffsStoreActions', () => { beforeEach(() => { delete noFilesData.diff_files; - - mock.onGet(endpointMetadata).reply(200, diffMetadata); }); it('should fetch diff meta information', () => { + mock.onGet(endpointMetadata).reply(200, diffMetadata); + return testAction( diffActions.fetchDiffFilesMeta, {}, @@ -206,6 +206,40 @@ describe('DiffsStoreActions', () => { [], ); }); + + it('should show a warning on 404 reponse', async () => { + mock.onGet(endpointMetadata).reply(404); + + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + + expect(createAlert).toHaveBeenCalledTimes(1); + expect(createAlert).toHaveBeenCalledWith({ + message: expect.stringMatching( + 'Building your merge request. Wait a few moments, then refresh this page.', + ), + variant: 'warning', + }); + }); + + it('should show no warning on any other status code', async () => { + mock.onGet(endpointMetadata).reply(500); + + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + + expect(createAlert).not.toHaveBeenCalled(); + }); }); describe('fetchCoverageFiles', () => { diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js index 3f870a98396..b5c44b084d8 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -311,9 +311,14 @@ describe('DiffsStoreUtils', () => { describe('prepareLineForRenamedFile', () => { const diffFile = { file_hash: 'file-hash', + brokenSymlink: false, + renamed_file: false, + added_lines: 1, + removed_lines: 1, }; const lineIndex = 4; const sourceLine = { + line_code: 'abc', foo: 'test', rich_text: ' <p>rich</p>', // Note the leading space }; @@ -328,6 +333,12 @@ describe('DiffsStoreUtils', () => { hasForm: false, text: undefined, alreadyPrepared: true, + commentsDisabled: false, + problems: { + brokenLineCode: false, + brokenSymlink: false, + fileOnlyMoved: false, + }, }; let preppedLine; @@ -360,24 +371,35 @@ describe('DiffsStoreUtils', () => { }); it.each` - brokenSymlink - ${false} - ${{}} - ${'anything except `false`'} + brokenSymlink | renamed | added | removed | lineCode | commentsDisabled + ${false} | ${false} | ${0} | ${0} | ${'a'} | ${false} + ${{}} | ${false} | ${1} | ${1} | ${'a'} | ${true} + ${'truthy'} | ${false} | ${1} | ${1} | ${'a'} | ${true} + ${false} | ${true} | ${1} | ${1} | ${'a'} | ${false} + ${false} | ${true} | ${1} | ${0} | ${'a'} | ${false} + ${false} | ${true} | ${0} | ${1} | ${'a'} | ${false} + ${false} | ${true} | ${0} | ${0} | ${'a'} | ${true} `( - "properly assigns each line's `commentsDisabled` as the same value as the parent file's `brokenSymlink` value (`$brokenSymlink`)", - ({ brokenSymlink }) => { - preppedLine = utils.prepareLineForRenamedFile({ - diffViewType: INLINE_DIFF_VIEW_TYPE, - line: sourceLine, + "properly sets a line's `commentsDisabled` to '$commentsDisabled' for file and line settings { brokenSymlink: $brokenSymlink, renamed: $renamed, added: $added, removed: $removed, line_code: $lineCode }", + ({ brokenSymlink, renamed, added, removed, lineCode, commentsDisabled }) => { + const line = { + ...sourceLine, + line_code: lineCode, + }; + const file = { + ...diffFile, + brokenSymlink, + renamed_file: renamed, + added_lines: added, + removed_lines: removed, + }; + const preparedLine = utils.prepareLineForRenamedFile({ index: lineIndex, - diffFile: { - ...diffFile, - brokenSymlink, - }, + diffFile: file, + line, }); - expect(preppedLine.commentsDisabled).toStrictEqual(brokenSymlink); + expect(preparedLine.commentsDisabled).toBe(commentsDisabled); }, ); }); @@ -477,7 +499,7 @@ describe('DiffsStoreUtils', () => { it('adds the `.brokenSymlink` property to each diff file', () => { preparedDiff.diff_files.forEach((file) => { - expect(file).toEqual(expect.objectContaining({ brokenSymlink: false })); + expect(file).toHaveProperty('brokenSymlink', false); }); }); @@ -490,7 +512,7 @@ describe('DiffsStoreUtils', () => { ].flatMap((file) => [...file[INLINE_DIFF_LINES_KEY]]); lines.forEach((line) => { - expect(line.commentsDisabled).toBe(false); + expect(line.problems.brokenSymlink).toBe(false); }); }); }); diff --git a/spec/frontend/diffs/utils/tree_worker_utils_spec.js b/spec/frontend/diffs/utils/tree_worker_utils_spec.js index 8113428f712..4df5fe75004 100644 --- a/spec/frontend/diffs/utils/tree_worker_utils_spec.js +++ b/spec/frontend/diffs/utils/tree_worker_utils_spec.js @@ -35,6 +35,23 @@ describe('~/diffs/utils/tree_worker_utils', () => { file_hash: 'test', }, { + new_path: 'constructor/test/aFile.js', + deleted_file: false, + new_file: true, + removed_lines: 0, + added_lines: 42, + file_hash: 'test', + }, + { + new_path: 'submodule @ abcdef123', + deleted_file: false, + new_file: true, + removed_lines: 0, + added_lines: 1, + submodule: true, + file_hash: 'test', + }, + { new_path: 'package.json', deleted_file: true, new_file: false, @@ -66,6 +83,7 @@ describe('~/diffs/utils/tree_worker_utils', () => { path: 'app/index.js', removedLines: 10, tempFile: false, + submodule: undefined, type: 'blob', tree: [], }, @@ -87,6 +105,7 @@ describe('~/diffs/utils/tree_worker_utils', () => { path: 'app/test/index.js', removedLines: 0, tempFile: true, + submodule: undefined, type: 'blob', tree: [], }, @@ -101,6 +120,7 @@ describe('~/diffs/utils/tree_worker_utils', () => { path: 'app/test/filepathneedstruncating.js', removedLines: 0, tempFile: true, + submodule: undefined, type: 'blob', tree: [], }, @@ -110,6 +130,45 @@ describe('~/diffs/utils/tree_worker_utils', () => { opened: true, }, { + key: 'constructor', + name: 'constructor/test', + opened: true, + path: 'constructor', + tree: [ + { + addedLines: 42, + changed: true, + deleted: false, + fileHash: 'test', + key: 'constructor/test/aFile.js', + name: 'aFile.js', + parentPath: 'constructor/test/', + path: 'constructor/test/aFile.js', + removedLines: 0, + submodule: undefined, + tempFile: true, + tree: [], + type: 'blob', + }, + ], + type: 'tree', + }, + { + key: 'submodule @ abcdef123', + parentPath: '/', + path: 'submodule @ abcdef123', + name: 'submodule @ abcdef123', + type: 'blob', + changed: true, + tempFile: true, + submodule: true, + deleted: false, + fileHash: 'test', + addedLines: 1, + removedLines: 0, + tree: [], + }, + { key: 'package.json', parentPath: '/', path: 'package.json', @@ -117,6 +176,7 @@ describe('~/diffs/utils/tree_worker_utils', () => { type: 'blob', changed: true, tempFile: false, + submodule: undefined, deleted: true, fileHash: 'test', addedLines: 0, @@ -135,6 +195,10 @@ describe('~/diffs/utils/tree_worker_utils', () => { 'app/test', 'app/test/index.js', 'app/test/filepathneedstruncating.js', + 'constructor', + 'constructor/test', + 'constructor/test/aFile.js', + 'submodule @ abcdef123', 'package.json', ]); }); |