diff options
author | Alfredo Sumaran <alfredo@gitlab.com> | 2016-09-29 20:26:01 -0500 |
---|---|---|
committer | Alfredo Sumaran <alfredo@gitlab.com> | 2016-10-13 14:16:35 -0500 |
commit | 197ac5ebbfd8ca7fbcb79776fd25ca0e213376c4 (patch) | |
tree | a44169352bf81ed518f060b414449089cb748386 | |
parent | a8ac9089afb664e569b34c61dc6782d20d1019d1 (diff) | |
download | gitlab-ce-197ac5ebbfd8ca7fbcb79776fd25ca0e213376c4.tar.gz |
Ability to resolve conflicts for files with `text-editor` as conflict type
7 files changed, 213 insertions, 159 deletions
diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index 49d30f6e9e8..46aad9fe33c 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -7,10 +7,10 @@ template: '#diff-file-editor', data() { return { - originalContent: '', saved: false, loading: false, - fileLoaded: false + fileLoaded: false, + originalContent: '', } }, computed: { @@ -23,43 +23,48 @@ } }, watch: { - loadFile(val) { - const self = this; - + ['file.showEditor'](val) { this.resetEditorContent(); if (!val || this.fileLoaded || this.loading) { - return + return; } + this.loadEditor(); + } + }, + ready() { + if (this.file.loadEditor) { + this.loadEditor(); + } + }, + methods: { + loadEditor() { this.loading = true; $.get(this.file.content_path) .done((file) => { - - let content = self.$el.querySelector('pre'); + let content = this.$el.querySelector('pre'); let fileContent = document.createTextNode(file.content); content.textContent = fileContent.textContent; - self.originalContent = file.content; - self.fileLoaded = true; - self.editor = ace.edit(content); - self.editor.$blockScrolling = Infinity; // Turn off annoying warning - self.editor.on('change', () => { - self.saveDiffResolution(); + this.originalContent = file.content; + this.fileLoaded = true; + this.editor = ace.edit(content); + this.editor.$blockScrolling = Infinity; // Turn off annoying warning + this.editor.on('change', () => { + this.saveDiffResolution(); }); - self.saveDiffResolution(); + this.saveDiffResolution(); }) .fail(() => { console.log('error'); }) .always(() => { - self.loading = false; + this.loading = false; }); - } - }, - methods: { + }, saveDiffResolution() { this.saved = true; diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 index d35e6d8aed6..e1f862797f5 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -13,6 +13,10 @@ INLINE: 'inline', PARALLEL: 'parallel' }; + const CONFLICT_TYPES = { + TEXT: 'text', + TEXT_EDITOR: 'text-editor' + }; global.mergeConflicts.mergeConflictsStore = { state: { @@ -26,8 +30,6 @@ setConflictsData(data) { this.decorateFiles(data.files); - this.setInlineLines(data.files); - this.setParallelLines(data.files); this.state.conflictsData = { files: data.files, @@ -45,90 +47,90 @@ file.resolutionData = {}; file.promptDiscardConfirmation = false; file.resolveMode = DEFAULT_RESOLVE_MODE; - }); - }, - - setInlineLines(files) { - files.forEach((file) => { + file.filePath = this.getFilePath(file); file.iconClass = `fa-${file.blob_icon}`; file.blobPath = file.blob_path; - file.filePath = this.getFilePath(file); - file.inlineLines = []; - file.sections.forEach((section) => { - let currentLineType = 'new'; - const { conflict, lines, id } = section; + if (file.type === CONFLICT_TYPES.TEXT) { + file.showEditor = false; + file.loadEditor = false; - if (conflict) { - file.inlineLines.push(this.getHeadHeaderLine(id)); - } + this.setInlineLine(file); + this.setParallelLine(file); + } else if (file.type === CONFLICT_TYPES.TEXT_EDITOR) { + file.showEditor = true; + file.loadEditor = true; + } + }); + }, - lines.forEach((line) => { - const { type } = line; + setInlineLine(file) { + file.inlineLines = []; - if ((type === 'new' || type === 'old') && currentLineType !== type) { - currentLineType = type; - file.inlineLines.push({ lineType: 'emptyLine', richText: '' }); - } + file.sections.forEach((section) => { + let currentLineType = 'new'; + const { conflict, lines, id } = section; + + if (conflict) { + file.inlineLines.push(this.getHeadHeaderLine(id)); + } - this.decorateLineForInlineView(line, id, conflict); - file.inlineLines.push(line); - }) + lines.forEach((line) => { + const { type } = line; - if (conflict) { - file.inlineLines.push(this.getOriginHeaderLine(id)); + if ((type === 'new' || type === 'old') && currentLineType !== type) { + currentLineType = type; + file.inlineLines.push({ lineType: 'emptyLine', richText: '' }); } - }); + + this.decorateLineForInlineView(line, id, conflict); + file.inlineLines.push(line); + }) + + if (conflict) { + file.inlineLines.push(this.getOriginHeaderLine(id)); + } }); }, - setParallelLines(files) { - files.forEach((file) => { - file.filePath = this.getFilePath(file); - file.iconClass = `fa-${file.blob_icon}`; - file.blobPath = file.blob_path; - file.parallelLines = []; - const linesObj = { left: [], right: [] }; - - file.sections.forEach((section) => { - const { conflict, lines, id } = section; + setParallelLine(file) { + file.parallelLines = []; + const linesObj = { left: [], right: [] }; - if (conflict) { - linesObj.left.push(this.getOriginHeaderLine(id)); - linesObj.right.push(this.getHeadHeaderLine(id)); - } + file.sections.forEach((section) => { + const { conflict, lines, id } = section; - lines.forEach((line) => { - const { type } = line; + if (conflict) { + linesObj.left.push(this.getOriginHeaderLine(id)); + linesObj.right.push(this.getHeadHeaderLine(id)); + } - if (conflict) { - if (type === 'old') { - linesObj.left.push(this.getLineForParallelView(line, id, 'conflict')); - } - else if (type === 'new') { - linesObj.right.push(this.getLineForParallelView(line, id, 'conflict', true)); - } - } - else { - const lineType = type || 'context'; + lines.forEach((line) => { + const { type } = line; - linesObj.left.push (this.getLineForParallelView(line, id, lineType)); - linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); + if (conflict) { + if (type === 'old') { + linesObj.left.push(this.getLineForParallelView(line, id, 'conflict')); + } else if (type === 'new') { + linesObj.right.push(this.getLineForParallelView(line, id, 'conflict', true)); } - }); + } else { + const lineType = type || 'context'; - this.checkLineLengths(linesObj); + linesObj.left.push (this.getLineForParallelView(line, id, lineType)); + linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); + } }); - for (let i = 0, len = linesObj.left.length; i < len; i++) { - file.parallelLines.push([ - linesObj.right[i], - linesObj.left[i] - ]); - } - - return file; + this.checkLineLengths(linesObj); }); + + for (let i = 0, len = linesObj.left.length; i < len; i++) { + file.parallelLines.push([ + linesObj.right[i], + linesObj.left[i] + ]); + } }, setLoadingState(state) { @@ -140,13 +142,12 @@ }, setFailedRequest(message) { - console.log('setFailedRequest'); this.state.hasError = true; this.state.conflictsData.errorMessage = message; }, getConflictsCount() { - if (!this.state.conflictsData.files) { + if (!this.state.conflictsData.files.length) { return 0; } @@ -154,11 +155,15 @@ let count = 0; files.forEach((file) => { - file.sections.forEach((section) => { - if (section.conflict) { - count++; - } - }); + if (file.type === CONFLICT_TYPES.TEXT) { + file.sections.forEach((section) => { + if (section.conflict) { + count++; + } + }); + } else { + count++; + } }); return count; @@ -172,7 +177,7 @@ }, setViewType(viewType) { - this.state.diffView = viewType; + this.state.diffView = viewType; this.state.isParallel = viewType === VIEW_TYPES.PARALLEL; $.cookie('diff_view', viewType, { @@ -253,8 +258,7 @@ for (let i = 0; i < diff; i++) { right.push({ lineType: 'emptyLine', richText: '' }); } - } - else { + } else { const diff = right.length - left.length; for (let i = 0; i < diff; i++) { left.push({ lineType: 'emptyLine', richText: '' }); @@ -268,8 +272,12 @@ }, setFileResolveMode(file, mode) { - // Restore Interactive mode when switching to Edit mode - if (mode === EDIT_RESOLVE_MODE) { + if (mode === INTERACTIVE_RESOLVE_MODE) { + file.showEditor = false; + } else if (mode === EDIT_RESOLVE_MODE) { + // Restore Interactive mode when switching to Edit mode + file.showEditor = true; + file.loadEditor = true; file.resolutionData = {}; this.restoreFileLinesState(file); @@ -287,9 +295,9 @@ }); file.parallelLines.forEach((lines) => { - const left = lines[0]; - const right = lines[1]; - const isLeftMatch = left.hasConflict || left.isHeader; + const left = lines[0]; + const right = lines[1]; + const isLeftMatch = left.hasConflict || left.isHeader; const isRightMatch = right.hasConflict || right.isHeader; if (isLeftMatch || isRightMatch) { @@ -313,14 +321,17 @@ let numberConflicts = 0; let resolvedConflicts = Object.keys(file.resolutionData).length - for (let j = 0, k = file.sections.length; j < k; j++) { - if (file.sections[j].conflict) { - numberConflicts++; + // We only check if + if (file.type === CONFLICT_TYPES.TEXT) { + for (let j = 0, k = file.sections.length; j < k; j++) { + if (file.sections[j].conflict) { + numberConflicts++; + } } - } - if (resolvedConflicts !== numberConflicts) { - unresolved++; + if (resolvedConflicts !== numberConflicts) { + unresolved++; + } } } else if (file.resolveMode === EDIT_RESOLVE_MODE) { // Unlikely to happen since switching to Edit mode saves content automatically. @@ -358,10 +369,15 @@ new_path: file.new_path }; - // Submit only one data for type of editing - if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { - addFile.sections = file.resolutionData; - } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + if (file.type === CONFLICT_TYPES.TEXT) { + + // Submit only one data for type of editing + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + addFile.sections = file.resolutionData; + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + addFile.content = file.content; + } + } else if (file.type === CONFLICT_TYPES.TEXT_EDITOR) { addFile.content = file.content; } @@ -374,39 +390,35 @@ handleSelected(file, sectionId, selection) { Vue.set(file.resolutionData, sectionId, selection); - this.state.conflictsData.files.forEach((file) => { - file.inlineLines.forEach((line) => { - if (line.id === sectionId && (line.hasConflict || line.isHeader)) { - this.markLine(line, selection); - } - }); + file.inlineLines.forEach((line) => { + if (line.id === sectionId && (line.hasConflict || line.isHeader)) { + this.markLine(line, selection); + } + }); - file.parallelLines.forEach((lines) => { - const left = lines[0]; - const right = lines[1]; - const hasSameId = right.id === sectionId || left.id === sectionId; - const isLeftMatch = left.hasConflict || left.isHeader; - const isRightMatch = right.hasConflict || right.isHeader; + file.parallelLines.forEach((lines) => { + const left = lines[0]; + const right = lines[1]; + const hasSameId = right.id === sectionId || left.id === sectionId; + const isLeftMatch = left.hasConflict || left.isHeader; + const isRightMatch = right.hasConflict || right.isHeader; - if (hasSameId && (isLeftMatch || isRightMatch)) { - this.markLine(left, selection); - this.markLine(right, selection); - } - }) + if (hasSameId && (isLeftMatch || isRightMatch)) { + this.markLine(left, selection); + this.markLine(right, selection); + } }); }, markLine(line, selection) { if (selection === 'head' && line.isHead) { - line.isSelected = true; + line.isSelected = true; line.isUnselected = false; - } - else if (selection === 'origin' && line.isOrigin) { - line.isSelected = true; + } else if (selection === 'origin' && line.isOrigin) { + line.isSelected = true; line.isUnselected = false; - } - else { - line.isSelected = false; + } else { + line.isSelected = false; line.isUnselected = true; } }, diff --git a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml index a335470de75..497db7e3f9b 100644 --- a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml @@ -1,10 +1,8 @@ -- if_condition = local_assigns.fetch(:if_condition, '') - -.diff-editor-wrap{ "v-show" => if_condition } +.diff-editor-wrap{ "v-show" => "file.showEditor" } .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } .discard-changes-alert Are you sure to discard your changes? .discard-actions %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel - %diff-file-editor{":file" => "file", ":load-file" => if_condition } + %diff-file-editor{":file" => "file"} diff --git a/app/views/projects/merge_requests/conflicts/_file_actions.html.haml b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml index 124dfde7b22..05af57acf03 100644 --- a/app/views/projects/merge_requests/conflicts/_file_actions.html.haml +++ b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml @@ -1,5 +1,5 @@ .file-actions - .btn-group + .btn-group{"v-if" => "file.type === 'text'"} %button.btn{ ":class" => "{ 'active': file.resolveMode == 'interactive' }", '@click' => "onClickResolveModeButton(file, 'interactive')", type: 'button' } diff --git a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml index 7120b6ff48d..60ac21d26c3 100644 --- a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml +++ b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml @@ -4,23 +4,26 @@ %i.fa.fa-fw{":class" => "file.iconClass"} %strong {{file.filePath}} = render partial: 'projects/merge_requests/conflicts/file_actions' - .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } - %table - %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.new_line{":class" => class_bindings} - %a {{line.new_line}} - %td.diff-line-num.old_line{":class" => class_bindings} - %a {{line.old_line}} - %td.line_content{":class" => class_bindings} - {{{line.richText}}} + %template{"v-if" => "file.type === 'text'"} + .diff-content.diff-wrap-lines + .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } + %table + %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} + %template{"v-if" => "!line.isHeader"} + %td.diff-line-num.new_line{":class" => class_bindings} + %a {{line.new_line}} + %td.diff-line-num.old_line{":class" => class_bindings} + %a {{line.old_line}} + %td.line_content{":class" => class_bindings} + {{{line.richText}}} - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{{line.richText}}} - %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } - {{line.buttonTitle}} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.resolveMode === 'edit' && !isParallel" } + %template{"v-if" => "line.isHeader"} + %td.diff-line-num.header{":class" => class_bindings} + %td.diff-line-num.header{":class" => class_bindings} + %td.line_content.header{":class" => class_bindings} + %strong {{{line.richText}}} + %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } + {{line.buttonTitle}} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor' + %template{"v-else" => true} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor' diff --git a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml index 18c830ddb17..7ed1485fc01 100644 --- a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml +++ b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml @@ -22,4 +22,4 @@ {{line.lineNumber}} %td.line_content.parallel{":class" => class_bindings} {{{line.richText}}} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.resolveMode === 'edit' && isParallel" } + = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.loadFile && isParallel" } diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index f2ff000486b..0e5507a0210 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -37,7 +37,7 @@ feature 'Merge request conflict resolution', js: true, feature: true do end end - context 'when in inline mode' do + context 'when in inline mode' do it 'resolves files manually' do within find('.files-wrapper .diff-file.inline-view', text: 'files/ruby/popen.rb') do click_button 'Edit inline' @@ -66,6 +66,42 @@ feature 'Merge request conflict resolution', js: true, feature: true do end end + context 'when a merge request can be resolved in the UI' do + let(:merge_request) { create_merge_request('conflict-contains-conflict-markers') } + + before do + project.team << [user, :developer] + login_as(user) + + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + context 'a conflict contain markers' do + before { click_link('conflicts', href: /\/conflicts\Z/) } + + it 'resolves files manually' do + within find('.files-wrapper .diff-file.inline-view', text: 'files/markdown/ruby-style-guide.md') do + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("Gregor Samsa woke from troubled dreams");') + end + + click_button 'Commit conflict resolution' + wait_for_ajax + + expect(page).to have_content('All merge conflicts were resolved') + + merge_request.reload_diff + + click_on 'Changes' + wait_for_ajax + find('.nothing-here-block', visible: true).click + wait_for_ajax + + expect(page).to have_content('Gregor Samsa woke from troubled dreams') + end + end + end + UNRESOLVABLE_CONFLICTS = { 'conflict-too-large' => 'when the conflicts contain a large file', 'conflict-binary-file' => 'when the conflicts contain a binary file', |