diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2018-09-19 15:03:37 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2018-09-19 15:03:37 +0000 |
commit | 38cf0b67b588b6f07adb0bde048ca4f568816598 (patch) | |
tree | 41cb3544a0f79c9a847855072e459d2c3ddc6fbe /spec | |
parent | 9e4e1aee500b44e2568fc439ae53077967ebb0c3 (diff) | |
parent | 90673dbcb82f78c53d071994ca65e3ace7a28f62 (diff) | |
download | gitlab-ce-38cf0b67b588b6f07adb0bde048ca4f568816598.tar.gz |
Merge branch 'mr-legacy-diff-notes' into 'master'
Re-enable legacy diff notes on merge request diffs
Closes #48873
See merge request gitlab-org/gitlab-ce!21652
Diffstat (limited to 'spec')
-rw-r--r-- | spec/features/merge_request/user_posts_diff_notes_spec.rb | 15 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/actions_spec.js | 1 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/mutations_spec.js | 71 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/utils_spec.js | 122 |
4 files changed, 198 insertions, 11 deletions
diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index b6ed3686de2..fa148715855 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -200,23 +200,20 @@ describe 'Merge request > User posts diff notes', :js do end context 'with a new line' do - # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 - xit 'allows commenting' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]').find(:xpath, '..')) + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) end end context 'with an old line' do - # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 - xit 'allows commenting' do - should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]').find(:xpath, '..')) + it 'allows commenting' do + should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) end end context 'with an unchanged line' do - # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 - xit 'allows commenting' do - should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]').find(:xpath, '..')) + it 'allows commenting' do + should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) end end diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index cfb8f862598..fd5c5e104b4 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -157,6 +157,7 @@ describe('DiffsStoreActions', () => { newPath: 'file1', oldLine: 5, oldPath: 'file2', + lineCode: 'ABC_1_1', }, }, }, diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 7eeca6712cc..9a5d8dfbd15 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -162,6 +162,7 @@ describe('DiffsStoreMutations', () => { }; const state = { + latestDiff: true, diffFiles: [ { fileHash: 'ABC', @@ -229,6 +230,76 @@ describe('DiffsStoreMutations', () => { expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2); expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2); }); + + it('should add legacy discussions to the given line', () => { + const diffPosition = { + baseSha: 'ed13df29948c41ba367caa757ab3ec4892509910', + headSha: 'b921914f9a834ac47e6fd9420f78db0f83559130', + newLine: null, + newPath: '500-lines-4.txt', + oldLine: 5, + oldPath: '500-lines-4.txt', + startSha: 'ed13df29948c41ba367caa757ab3ec4892509910', + lineCode: 'ABC_1', + }; + + const state = { + latestDiff: true, + diffFiles: [ + { + fileHash: 'ABC', + parallelDiffLines: [ + { + left: { + lineCode: 'ABC_1', + discussions: [], + }, + right: { + lineCode: 'ABC_1', + discussions: [], + }, + }, + ], + highlightedDiffLines: [ + { + lineCode: 'ABC_1', + discussions: [], + }, + ], + }, + ], + }; + const discussions = [ + { + id: 1, + line_code: 'ABC_1', + diff_discussion: true, + active: true, + }, + { + id: 2, + line_code: 'ABC_1', + diff_discussion: true, + active: true, + }, + ]; + + const diffPositionByLineCode = { + ABC_1: diffPosition, + }; + + mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { + fileHash: 'ABC', + discussions, + diffPositionByLineCode, + }); + + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2); + + expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2); + }); }); describe('REMOVE_LINE_DISCUSSIONS', () => { diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 4b5cf450c68..6138b9701f4 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -3,6 +3,7 @@ import { LINE_POSITION_LEFT, LINE_POSITION_RIGHT, TEXT_DIFF_POSITION_TYPE, + LEGACY_DIFF_NOTE_TYPE, DIFF_NOTE_TYPE, NEW_LINE_TYPE, OLD_LINE_TYPE, @@ -151,6 +152,64 @@ describe('DiffsStoreUtils', () => { data: postData, }); }); + + it('should create legacy note form data', () => { + const diffFile = getDiffFileMock(); + delete diffFile.diffRefs.startSha; + delete diffFile.diffRefs.headSha; + + noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE; + + const options = { + note: 'Hello world!', + noteableData: noteableDataMock, + noteableType: MERGE_REQUEST_NOTEABLE_TYPE, + diffFile, + noteTargetLine: { + lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + metaData: null, + newLine: 3, + oldLine: 1, + }, + diffViewType: PARALLEL_DIFF_VIEW_TYPE, + linePosition: LINE_POSITION_LEFT, + }; + + const position = JSON.stringify({ + base_sha: diffFile.diffRefs.baseSha, + start_sha: undefined, + head_sha: undefined, + old_path: diffFile.oldPath, + new_path: diffFile.newPath, + position_type: TEXT_DIFF_POSITION_TYPE, + old_line: options.noteTargetLine.oldLine, + new_line: options.noteTargetLine.newLine, + }); + + const postData = { + view: options.diffViewType, + line_type: options.linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE, + merge_request_diff_head_sha: undefined, + in_reply_to_discussion_id: '', + note_project_id: '', + target_type: options.noteableType, + target_id: options.noteableData.id, + note: { + noteable_type: options.noteableType, + noteable_id: options.noteableData.id, + commit_id: '', + type: LEGACY_DIFF_NOTE_TYPE, + line_code: options.noteTargetLine.lineCode, + note: options.note, + position, + }, + }; + + expect(utils.getNoteFormData(options)).toEqual({ + endpoint: options.noteableData.create_note_path, + data: postData, + }); + }); }); describe('addLineReferences', () => { @@ -291,13 +350,72 @@ describe('DiffsStoreUtils', () => { it('returns true when the discussion is up to date', () => { expect( - utils.isDiscussionApplicableToLine(discussions.upToDateDiscussion1, diffPosition), + utils.isDiscussionApplicableToLine({ + discussion: discussions.upToDateDiscussion1, + diffPosition, + latestDiff: true, + }), ).toBe(true); }); it('returns false when the discussion is not up to date', () => { expect( - utils.isDiscussionApplicableToLine(discussions.outDatedDiscussion1, diffPosition), + utils.isDiscussionApplicableToLine({ + discussion: discussions.outDatedDiscussion1, + diffPosition, + latestDiff: true, + }), + ).toBe(false); + }); + + it('returns true when line codes match and discussion does not contain position and is not active', () => { + const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: false }; + delete discussion.original_position; + delete discussion.position; + + expect( + utils.isDiscussionApplicableToLine({ + discussion, + diffPosition: { + ...diffPosition, + lineCode: 'ABC_1', + }, + latestDiff: true, + }), + ).toBe(false); + }); + + it('returns true when line codes match and discussion does not contain position and is active', () => { + const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: true }; + delete discussion.original_position; + delete discussion.position; + + expect( + utils.isDiscussionApplicableToLine({ + discussion, + diffPosition: { + ...diffPosition, + lineCode: 'ABC_1', + }, + latestDiff: true, + }), + ).toBe(true); + }); + + it('returns false when not latest diff', () => { + const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: true }; + delete discussion.original_position; + delete discussion.position; + + expect( + utils.isDiscussionApplicableToLine({ + discussion, + diffPosition: { + ...diffPosition, + lineCode: 'ABC_1', + }, + latestDiff: false, + }), ).toBe(false); }); }); |