diff options
44 files changed, 1560 insertions, 586 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ace0de5cad3..76117a48730 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -22,7 +22,7 @@ before_script: - bundle --version - '[ "$USE_BUNDLE_INSTALL" != "true" ] || retry bundle install --without postgres production --jobs $(nproc) "${FLAGS[@]}"' - retry gem install knapsack - - '[ "$SETUP_DB" != "true" ] || bundle exec rake db:drop db:create db:schema:load db:migrate' + - '[ "$SETUP_DB" != "true" ] || bundle exec rake db:drop db:create db:schema:load db:migrate add_limits_mysql' stages: - prepare diff --git a/CHANGELOG.md b/CHANGELOG.md index 37383ea1b72..9dbe1832de0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Update runner version only when updating contacted_at - Add link from system note to compare with previous version - Use gitlab-shell v3.6.6 + - Ability to resolve merge request conflicts with editor !6374 - Add `/projects/visible` API endpoint (Ben Boeckel) - Fix centering of custom header logos (Ashley Dumaine) - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 5be35cf4b41..73691f40c74 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -101,9 +101,6 @@ new ZenMode(); new MergedButtons(); break; - case "projects:merge_requests:conflicts": - window.mcui = new MergeConflictResolver() - break; case 'projects:merge_requests:index': shortcut_handler = new ShortcutsNavigation(); Issuable.init(); diff --git a/app/assets/javascripts/merge_conflict_data_provider.js.es6 b/app/assets/javascripts/merge_conflict_data_provider.js.es6 deleted file mode 100644 index 13ee794ba38..00000000000 --- a/app/assets/javascripts/merge_conflict_data_provider.js.es6 +++ /dev/null @@ -1,347 +0,0 @@ -const HEAD_HEADER_TEXT = 'HEAD//our changes'; -const ORIGIN_HEADER_TEXT = 'origin//their changes'; -const HEAD_BUTTON_TITLE = 'Use ours'; -const ORIGIN_BUTTON_TITLE = 'Use theirs'; - - -class MergeConflictDataProvider { - - getInitialData() { - // TODO: remove reliance on jQuery and DOM state introspection - const diffViewType = $.cookie('diff_view'); - const fixedLayout = $('.content-wrapper .container-fluid').hasClass('container-limited'); - - return { - isLoading : true, - hasError : false, - isParallel : diffViewType === 'parallel', - diffViewType : diffViewType, - fixedLayout : fixedLayout, - isSubmitting : false, - conflictsData : {}, - resolutionData : {} - } - } - - - decorateData(vueInstance, data) { - this.vueInstance = vueInstance; - - if (data.type === 'error') { - vueInstance.hasError = true; - data.errorMessage = data.message; - } - else { - data.shortCommitSha = data.commit_sha.slice(0, 7); - data.commitMessage = data.commit_message; - - this.setParallelLines(data); - this.setInlineLines(data); - this.updateResolutionsData(data); - } - - vueInstance.conflictsData = data; - vueInstance.isSubmitting = false; - - const conflictsText = this.getConflictsCount() > 1 ? 'conflicts' : 'conflict'; - vueInstance.conflictsData.conflictsText = conflictsText; - } - - - updateResolutionsData(data) { - const vi = this.vueInstance; - - data.files.forEach( (file) => { - file.sections.forEach( (section) => { - if (section.conflict) { - vi.$set(`resolutionData['${section.id}']`, false); - } - }); - }); - } - - - setParallelLines(data) { - data.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; - - if (conflict) { - linesObj.left.push(this.getOriginHeaderLine(id)); - linesObj.right.push(this.getHeadHeaderLine(id)); - } - - lines.forEach( (line) => { - const { type } = line; - - 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'; - - linesObj.left.push (this.getLineForParallelView(line, id, lineType)); - linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); - } - }); - - this.checkLineLengths(linesObj); - }); - - for (let i = 0, len = linesObj.left.length; i < len; i++) { - file.parallelLines.push([ - linesObj.right[i], - linesObj.left[i] - ]); - } - - }); - } - - - checkLineLengths(linesObj) { - let { left, right } = linesObj; - - if (left.length !== right.length) { - if (left.length > right.length) { - const diff = left.length - right.length; - for (let i = 0; i < diff; i++) { - right.push({ lineType: 'emptyLine', richText: '' }); - } - } - else { - const diff = right.length - left.length; - for (let i = 0; i < diff; i++) { - left.push({ lineType: 'emptyLine', richText: '' }); - } - } - } - } - - - setInlineLines(data) { - data.files.forEach( (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 (conflict) { - file.inlineLines.push(this.getHeadHeaderLine(id)); - } - - lines.forEach( (line) => { - const { type } = line; - - 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)); - } - }); - }); - } - - - handleSelected(sectionId, selection) { - const vi = this.vueInstance; - - vi.resolutionData[sectionId] = selection; - vi.conflictsData.files.forEach( (file) => { - 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; - - if (hasSameId && (isLeftMatch || isRightMatch)) { - this.markLine(left, selection); - this.markLine(right, selection); - } - }) - }); - } - - - updateViewType(newType) { - const vi = this.vueInstance; - - if (newType === vi.diffViewType || !(newType === 'parallel' || newType === 'inline')) { - return; - } - - vi.diffViewType = newType; - vi.isParallel = newType === 'parallel'; - $.cookie('diff_view', newType, { - path: (gon && gon.relative_url_root) || '/' - }); - $('.content-wrapper .container-fluid') - .toggleClass('container-limited', !vi.isParallel && vi.fixedLayout); - } - - - markLine(line, selection) { - if (selection === 'head' && line.isHead) { - line.isSelected = true; - line.isUnselected = false; - } - else if (selection === 'origin' && line.isOrigin) { - line.isSelected = true; - line.isUnselected = false; - } - else { - line.isSelected = false; - line.isUnselected = true; - } - } - - - getConflictsCount() { - return Object.keys(this.vueInstance.resolutionData).length; - } - - - getResolvedCount() { - let count = 0; - const data = this.vueInstance.resolutionData; - - for (const id in data) { - const resolution = data[id]; - if (resolution) { - count++; - } - } - - return count; - } - - - isReadyToCommit() { - const { conflictsData, isSubmitting } = this.vueInstance - const allResolved = this.getConflictsCount() === this.getResolvedCount(); - const hasCommitMessage = $.trim(conflictsData.commitMessage).length; - - return !isSubmitting && hasCommitMessage && allResolved; - } - - - getCommitButtonText() { - const initial = 'Commit conflict resolution'; - const inProgress = 'Committing...'; - const vue = this.vueInstance; - - return vue ? vue.isSubmitting ? inProgress : initial : initial; - } - - - decorateLineForInlineView(line, id, conflict) { - const { type } = line; - line.id = id; - line.hasConflict = conflict; - line.isHead = type === 'new'; - line.isOrigin = type === 'old'; - line.hasMatch = type === 'match'; - line.richText = line.rich_text; - line.isSelected = false; - line.isUnselected = false; - } - - getLineForParallelView(line, id, lineType, isHead) { - const { old_line, new_line, rich_text } = line; - const hasConflict = lineType === 'conflict'; - - return { - id, - lineType, - hasConflict, - isHead : hasConflict && isHead, - isOrigin : hasConflict && !isHead, - hasMatch : lineType === 'match', - lineNumber : isHead ? new_line : old_line, - section : isHead ? 'head' : 'origin', - richText : rich_text, - isSelected : false, - isUnselected : false - } - } - - - getHeadHeaderLine(id) { - return { - id : id, - richText : HEAD_HEADER_TEXT, - buttonTitle : HEAD_BUTTON_TITLE, - type : 'new', - section : 'head', - isHeader : true, - isHead : true, - isSelected : false, - isUnselected: false - } - } - - - getOriginHeaderLine(id) { - return { - id : id, - richText : ORIGIN_HEADER_TEXT, - buttonTitle : ORIGIN_BUTTON_TITLE, - type : 'old', - section : 'origin', - isHeader : true, - isOrigin : true, - isSelected : false, - isUnselected: false - } - } - - - handleFailedRequest(vueInstance, data) { - vueInstance.hasError = true; - vueInstance.conflictsData.errorMessage = 'Something went wrong!'; - } - - - getCommitData() { - return { - commit_message: this.vueInstance.conflictsData.commitMessage, - sections: this.vueInstance.resolutionData - } - } - - - getFilePath(file) { - const { old_path, new_path } = file; - return old_path === new_path ? new_path : `${old_path} → ${new_path}`; - } - -} diff --git a/app/assets/javascripts/merge_conflict_resolver.js.es6 b/app/assets/javascripts/merge_conflict_resolver.js.es6 deleted file mode 100644 index 7e756433bf5..00000000000 --- a/app/assets/javascripts/merge_conflict_resolver.js.es6 +++ /dev/null @@ -1,82 +0,0 @@ -//= require vue - -class MergeConflictResolver { - - constructor() { - this.dataProvider = new MergeConflictDataProvider() - this.initVue() - } - - - initVue() { - const that = this; - this.vue = new Vue({ - el : '#conflicts', - name : 'MergeConflictResolver', - data : this.dataProvider.getInitialData(), - created : this.fetchData(), - computed : this.setComputedProperties(), - methods : { - handleSelected(sectionId, selection) { - that.dataProvider.handleSelected(sectionId, selection); - }, - handleViewTypeChange(newType) { - that.dataProvider.updateViewType(newType); - }, - commit() { - that.commit(); - } - } - }) - } - - - setComputedProperties() { - const dp = this.dataProvider; - - return { - conflictsCount() { return dp.getConflictsCount() }, - resolvedCount() { return dp.getResolvedCount() }, - readyToCommit() { return dp.isReadyToCommit() }, - commitButtonText() { return dp.getCommitButtonText() } - } - } - - - fetchData() { - const dp = this.dataProvider; - - $.get($('#conflicts').data('conflictsPath')) - .done((data) => { - dp.decorateData(this.vue, data); - }) - .error((data) => { - dp.handleFailedRequest(this.vue, data); - }) - .always(() => { - this.vue.isLoading = false; - - this.vue.$nextTick(() => { - $('#conflicts .js-syntax-highlight').syntaxHighlight(); - }); - - $('.content-wrapper .container-fluid') - .toggleClass('container-limited', !this.vue.isParallel && this.vue.fixedLayout); - }) - } - - - commit() { - this.vue.isSubmitting = true; - - $.post($('#conflicts').data('resolveConflictsPath'), this.dataProvider.getCommitData()) - .done((data) => { - window.location.href = data.redirect_to; - }) - .error(() => { - this.vue.isSubmitting = false; - new Flash('Something went wrong!'); - }); - } - -} 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 new file mode 100644 index 00000000000..5012bdfe997 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -0,0 +1,93 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.diffFileEditor = Vue.extend({ + props: { + file: Object, + onCancelDiscardConfirmation: Function, + onAcceptDiscardConfirmation: Function + }, + data() { + return { + saved: false, + loading: false, + fileLoaded: false, + originalContent: '', + } + }, + computed: { + classObject() { + return { + 'saved': this.saved, + 'is-loading': this.loading + }; + } + }, + watch: { + ['file.showEditor'](val) { + this.resetEditorContent(); + + if (!val || this.fileLoaded || this.loading) { + return; + } + + this.loadEditor(); + } + }, + ready() { + if (this.file.loadEditor) { + this.loadEditor(); + } + }, + methods: { + loadEditor() { + this.loading = true; + + $.get(this.file.content_path) + .done((file) => { + let content = this.$el.querySelector('pre'); + let fileContent = document.createTextNode(file.content); + + content.textContent = fileContent.textContent; + + this.originalContent = file.content; + this.fileLoaded = true; + this.editor = ace.edit(content); + this.editor.$blockScrolling = Infinity; // Turn off annoying warning + this.editor.getSession().setMode(`ace/mode/${file.blob_ace_mode}`); + this.editor.on('change', () => { + this.saveDiffResolution(); + }); + this.saveDiffResolution(); + }) + .fail(() => { + new Flash('Failed to load the file, please try again.'); + }) + .always(() => { + this.loading = false; + }); + }, + saveDiffResolution() { + this.saved = true; + + // This probably be better placed in the data provider + this.file.content = this.editor.getValue(); + this.file.resolveEditChanged = this.file.content !== this.originalContent; + this.file.promptDiscardConfirmation = false; + }, + resetEditorContent() { + if (this.fileLoaded) { + this.editor.setValue(this.originalContent, -1); + } + }, + cancelDiscardConfirmation(file) { + this.onCancelDiscardConfirmation(file); + }, + acceptDiscardConfirmation(file) { + this.onAcceptDiscardConfirmation(file); + } + } + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 b/app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 new file mode 100644 index 00000000000..b4be1c8988d --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 @@ -0,0 +1,12 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.inlineConflictLines = Vue.extend({ + props: { + file: Object + }, + mixins: [global.mergeConflicts.utils, global.mergeConflicts.actions], + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 new file mode 100644 index 00000000000..8b0a8ab2073 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 @@ -0,0 +1,14 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.parallelConflictLine = Vue.extend({ + props: { + file: Object, + line: Object + }, + mixins: [global.mergeConflicts.utils, global.mergeConflicts.actions], + template: '#parallel-conflict-line' + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 new file mode 100644 index 00000000000..eb4cc6a9dac --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 @@ -0,0 +1,15 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.parallelConflictLines = Vue.extend({ + props: { + file: Object + }, + mixins: [global.mergeConflicts.utils], + components: { + 'parallel-conflict-line': gl.mergeConflicts.parallelConflictLine + } + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 new file mode 100644 index 00000000000..da2fb8b1323 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 @@ -0,0 +1,30 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + class mergeConflictsService { + constructor(options) { + this.conflictsPath = options.conflictsPath; + this.resolveConflictsPath = options.resolveConflictsPath; + } + + fetchConflictsData() { + return $.ajax({ + dataType: 'json', + url: this.conflictsPath + }); + } + + submitResolveConflicts(data) { + return $.ajax({ + url: this.resolveConflictsPath, + data: JSON.stringify(data), + contentType: 'application/json', + dataType: 'json', + method: 'POST' + }); + } + }; + + global.mergeConflicts.mergeConflictsService = mergeConflictsService; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 new file mode 100644 index 00000000000..5c5c65f29d4 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -0,0 +1,437 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + const diffViewType = $.cookie('diff_view'); + const HEAD_HEADER_TEXT = 'HEAD//our changes'; + const ORIGIN_HEADER_TEXT = 'origin//their changes'; + const HEAD_BUTTON_TITLE = 'Use ours'; + const ORIGIN_BUTTON_TITLE = 'Use theirs'; + const INTERACTIVE_RESOLVE_MODE = 'interactive'; + const EDIT_RESOLVE_MODE = 'edit'; + const DEFAULT_RESOLVE_MODE = INTERACTIVE_RESOLVE_MODE; + const VIEW_TYPES = { + INLINE: 'inline', + PARALLEL: 'parallel' + }; + const CONFLICT_TYPES = { + TEXT: 'text', + TEXT_EDITOR: 'text-editor' + }; + + global.mergeConflicts.mergeConflictsStore = { + state: { + isLoading: true, + hasError: false, + isSubmitting: false, + isParallel: diffViewType === VIEW_TYPES.PARALLEL, + diffViewType: diffViewType, + conflictsData: {} + }, + + setConflictsData(data) { + this.decorateFiles(data.files); + + this.state.conflictsData = { + files: data.files, + commitMessage: data.commit_message, + sourceBranch: data.source_branch, + targetBranch: data.target_branch, + commitMessage: data.commit_message, + shortCommitSha: data.commit_sha.slice(0, 7), + }; + }, + + decorateFiles(files) { + files.forEach((file) => { + file.content = ''; + file.resolutionData = {}; + file.promptDiscardConfirmation = false; + file.resolveMode = DEFAULT_RESOLVE_MODE; + file.filePath = this.getFilePath(file); + file.iconClass = `fa-${file.blob_icon}`; + file.blobPath = file.blob_path; + + if (file.type === CONFLICT_TYPES.TEXT) { + file.showEditor = false; + file.loadEditor = false; + + this.setInlineLine(file); + this.setParallelLine(file); + } else if (file.type === CONFLICT_TYPES.TEXT_EDITOR) { + file.showEditor = true; + file.loadEditor = true; + } + }); + }, + + setInlineLine(file) { + file.inlineLines = []; + + file.sections.forEach((section) => { + let currentLineType = 'new'; + const { conflict, lines, id } = section; + + if (conflict) { + file.inlineLines.push(this.getHeadHeaderLine(id)); + } + + lines.forEach((line) => { + const { type } = line; + + 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)); + } + }); + }, + + setParallelLine(file) { + file.parallelLines = []; + const linesObj = { left: [], right: [] }; + + file.sections.forEach((section) => { + const { conflict, lines, id } = section; + + if (conflict) { + linesObj.left.push(this.getOriginHeaderLine(id)); + linesObj.right.push(this.getHeadHeaderLine(id)); + } + + lines.forEach((line) => { + const { type } = line; + + 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'; + + linesObj.left.push (this.getLineForParallelView(line, id, lineType)); + linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); + } + }); + + 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) { + this.state.isLoading = state; + }, + + setErrorState(state) { + this.state.hasError = state; + }, + + setFailedRequest(message) { + this.state.hasError = true; + this.state.conflictsData.errorMessage = message; + }, + + getConflictsCount() { + if (!this.state.conflictsData.files.length) { + return 0; + } + + const files = this.state.conflictsData.files; + let count = 0; + + files.forEach((file) => { + if (file.type === CONFLICT_TYPES.TEXT) { + file.sections.forEach((section) => { + if (section.conflict) { + count++; + } + }); + } else { + count++; + } + }); + + return count; + }, + + getConflictsCountText() { + const count = this.getConflictsCount(); + const text = count ? 'conflicts' : 'conflict'; + + return `${count} ${text}`; + }, + + setViewType(viewType) { + this.state.diffView = viewType; + this.state.isParallel = viewType === VIEW_TYPES.PARALLEL; + + $.cookie('diff_view', viewType, { + path: gon.relative_url_root || '/' + }); + }, + + getHeadHeaderLine(id) { + return { + id: id, + richText: HEAD_HEADER_TEXT, + buttonTitle: HEAD_BUTTON_TITLE, + type: 'new', + section: 'head', + isHeader: true, + isHead: true, + isSelected: false, + isUnselected: false + }; + }, + + decorateLineForInlineView(line, id, conflict) { + const { type } = line; + line.id = id; + line.hasConflict = conflict; + line.isHead = type === 'new'; + line.isOrigin = type === 'old'; + line.hasMatch = type === 'match'; + line.richText = line.rich_text; + line.isSelected = false; + line.isUnselected = false; + }, + + getLineForParallelView(line, id, lineType, isHead) { + const { old_line, new_line, rich_text } = line; + const hasConflict = lineType === 'conflict'; + + return { + id, + lineType, + hasConflict, + isHead: hasConflict && isHead, + isOrigin: hasConflict && !isHead, + hasMatch: lineType === 'match', + lineNumber: isHead ? new_line : old_line, + section: isHead ? 'head' : 'origin', + richText: rich_text, + isSelected: false, + isUnselected: false + }; + }, + + getOriginHeaderLine(id) { + return { + id: id, + richText: ORIGIN_HEADER_TEXT, + buttonTitle: ORIGIN_BUTTON_TITLE, + type: 'old', + section: 'origin', + isHeader: true, + isOrigin: true, + isSelected: false, + isUnselected: false + }; + }, + + getFilePath(file) { + const { old_path, new_path } = file; + return old_path === new_path ? new_path : `${old_path} → ${new_path}`; + }, + + checkLineLengths(linesObj) { + let { left, right } = linesObj; + + if (left.length !== right.length) { + if (left.length > right.length) { + const diff = left.length - right.length; + for (let i = 0; i < diff; i++) { + right.push({ lineType: 'emptyLine', richText: '' }); + } + } else { + const diff = right.length - left.length; + for (let i = 0; i < diff; i++) { + left.push({ lineType: 'emptyLine', richText: '' }); + } + } + } + }, + + setPromptConfirmationState(file, state) { + file.promptDiscardConfirmation = state; + }, + + setFileResolveMode(file, 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); + } + + file.resolveMode = mode; + }, + + restoreFileLinesState(file) { + file.inlineLines.forEach((line) => { + if (line.hasConflict || line.isHeader) { + line.isSelected = false; + line.isUnselected = false; + } + }); + + file.parallelLines.forEach((lines) => { + const left = lines[0]; + const right = lines[1]; + const isLeftMatch = left.hasConflict || left.isHeader; + const isRightMatch = right.hasConflict || right.isHeader; + + if (isLeftMatch || isRightMatch) { + left.isSelected = false; + left.isUnselected = false; + right.isSelected = false; + right.isUnselected = false; + } + }); + }, + + isReadyToCommit() { + const files = this.state.conflictsData.files; + const hasCommitMessage = $.trim(this.state.conflictsData.commitMessage).length; + let unresolved = 0; + + for (let i = 0, l = files.length; i < l; i++) { + let file = files[i]; + + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + let numberConflicts = 0; + let resolvedConflicts = Object.keys(file.resolutionData).length + + // We only check for conflicts type 'text' + // since conflicts `text_editor` can´t be resolved in interactive mode + 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++; + } + } + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + + // Unlikely to happen since switching to Edit mode saves content automatically. + // Checking anyway in case the save strategy changes in the future + if (!file.content) { + unresolved++; + continue; + } + } + } + + return !this.state.isSubmitting && hasCommitMessage && !unresolved; + }, + + getCommitButtonText() { + const initial = 'Commit conflict resolution'; + const inProgress = 'Committing...'; + + return this.state ? this.state.isSubmitting ? inProgress : initial : initial; + }, + + getCommitData() { + let commitData = {}; + + commitData = { + commit_message: this.state.conflictsData.commitMessage, + files: [] + }; + + this.state.conflictsData.files.forEach((file) => { + let addFile; + + addFile = { + old_path: file.old_path, + new_path: file.new_path + }; + + 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; + } + + commitData.files.push(addFile); + }); + + return commitData; + }, + + handleSelected(file, sectionId, selection) { + Vue.set(file.resolutionData, sectionId, 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; + + if (hasSameId && (isLeftMatch || isRightMatch)) { + this.markLine(left, selection); + this.markLine(right, selection); + } + }); + }, + + markLine(line, selection) { + if (selection === 'head' && line.isHead) { + line.isSelected = true; + line.isUnselected = false; + } else if (selection === 'origin' && line.isOrigin) { + line.isSelected = true; + line.isUnselected = false; + } else { + line.isSelected = false; + line.isUnselected = true; + } + }, + + setSubmitState(state) { + this.state.isSubmitting = state; + }, + + fileTextTypePresent() { + return this.state.conflictsData.files.some(f => f.type === CONFLICT_TYPES.TEXT); + } + }; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 new file mode 100644 index 00000000000..7fd3749b3e2 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 @@ -0,0 +1,89 @@ +//= require vue +//= require ./merge_conflict_store +//= require ./merge_conflict_service +//= require ./mixins/line_conflict_utils +//= require ./mixins/line_conflict_actions +//= require ./components/diff_file_editor +//= require ./components/inline_conflict_lines +//= require ./components/parallel_conflict_line +//= require ./components/parallel_conflict_lines + +$(() => { + const INTERACTIVE_RESOLVE_MODE = 'interactive'; + const conflictsEl = document.querySelector('#conflicts'); + const mergeConflictsStore = gl.mergeConflicts.mergeConflictsStore; + const mergeConflictsService = new gl.mergeConflicts.mergeConflictsService({ + conflictsPath: conflictsEl.dataset.conflictsPath, + resolveConflictsPath: conflictsEl.dataset.resolveConflictsPath + }); + + gl.MergeConflictsResolverApp = new Vue({ + el: '#conflicts', + data: mergeConflictsStore.state, + components: { + 'diff-file-editor': gl.mergeConflicts.diffFileEditor, + 'inline-conflict-lines': gl.mergeConflicts.inlineConflictLines, + 'parallel-conflict-lines': gl.mergeConflicts.parallelConflictLines + }, + computed: { + conflictsCountText() { return mergeConflictsStore.getConflictsCountText() }, + readyToCommit() { return mergeConflictsStore.isReadyToCommit() }, + commitButtonText() { return mergeConflictsStore.getCommitButtonText() }, + showDiffViewTypeSwitcher() { return mergeConflictsStore.fileTextTypePresent() } + }, + created() { + mergeConflictsService + .fetchConflictsData() + .done((data) => { + if (data.type === 'error') { + mergeConflictsStore.setFailedRequest(data.message); + } else { + mergeConflictsStore.setConflictsData(data); + } + }) + .error(() => { + mergeConflictsStore.setFailedRequest(); + }) + .always(() => { + mergeConflictsStore.setLoadingState(false); + + this.$nextTick(() => { + $(conflictsEl.querySelectorAll('.js-syntax-highlight')).syntaxHighlight(); + }); + }); + }, + methods: { + handleViewTypeChange(viewType) { + mergeConflictsStore.setViewType(viewType); + }, + onClickResolveModeButton(file, mode) { + if (mode === INTERACTIVE_RESOLVE_MODE && file.resolveEditChanged) { + mergeConflictsStore.setPromptConfirmationState(file, true); + return; + } + + mergeConflictsStore.setFileResolveMode(file, mode); + }, + acceptDiscardConfirmation(file) { + mergeConflictsStore.setPromptConfirmationState(file, false); + mergeConflictsStore.setFileResolveMode(file, INTERACTIVE_RESOLVE_MODE); + }, + cancelDiscardConfirmation(file) { + mergeConflictsStore.setPromptConfirmationState(file, false); + }, + commit() { + mergeConflictsStore.setSubmitState(true); + + mergeConflictsService + .submitResolveConflicts(mergeConflictsStore.getCommitData()) + .done((data) => { + window.location.href = data.redirect_to; + }) + .error(() => { + mergeConflictsStore.setSubmitState(false); + new Flash('Failed to save merge conflicts resolutions. Please try again!'); + }); + } + } + }) +}); diff --git a/app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 new file mode 100644 index 00000000000..114a2c5b305 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 @@ -0,0 +1,12 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.actions = { + methods: { + handleSelected(file, sectionId, selection) { + gl.mergeConflicts.mergeConflictsStore.handleSelected(file, sectionId, selection); + } + } + }; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 new file mode 100644 index 00000000000..b846a90ab2a --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 @@ -0,0 +1,18 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.utils = { + methods: { + lineCssClass(line) { + return { + 'head': line.isHead, + 'origin': line.isOrigin, + 'match': line.hasMatch, + 'selected': line.isSelected, + 'unselected': line.isUnselected + }; + } + } + }; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 4c34ed3ebf7..7690d65de8e 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -56,6 +56,7 @@ $border-gray-light: #dcdcdc; $border-gray-normal: #d7d7d7; $border-gray-dark: #c6cacf; +$border-green-extra-light: #9adb84; $border-green-light: #2faa60; $border-green-normal: #2ca05b; $border-green-dark: #279654; diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index 49013d7cac9..eed2b0ab7cc 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -237,4 +237,51 @@ $colors: ( .btn-success .fa-spinner { color: #fff; } + + .editor-wrap { + &.is-loading { + .editor { + display: none; + } + + .loading { + display: block; + } + } + + &.saved { + .editor { + border-top: solid 2px $border-green-extra-light; + } + } + + .editor { + pre { + height: 350px; + border: none; + border-radius: 0; + margin-bottom: 0; + } + } + + .loading { + display: none; + } + } + + .discard-changes-alert { + background-color: $background-color; + text-align: right; + padding: $gl-padding-top $gl-padding; + color: $gl-text-color; + + .discard-actions { + display: inline-block; + margin-left: 10px; + } + } + + .resolve-conflicts-form { + padding-top: $gl-padding; + } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 705824502eb..37600ed875c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -118,7 +118,12 @@ class ApplicationController < ActionController::Base end def render_404 - render file: Rails.root.join("public", "404"), layout: false, status: "404" + respond_to do |format| + format.html do + render file: Rails.root.join("public", "404"), layout: false, status: "404" + end + format.any { head :not_found } + end end def no_cache_headers diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 9207c954335..a39b47b6d95 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -9,15 +9,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :module_enabled before_action :merge_request, only: [ - :edit, :update, :show, :diffs, :commits, :conflicts, :builds, :pipelines, :merge, :merge_check, + :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines, :merge, :merge_check, :ci_status, :ci_environments_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues ] before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines] - before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds, :pipelines] + before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines] before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check] before_action :define_commit_vars, only: [:diffs] before_action :define_diff_comment_vars, only: [:diffs] - before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :pipelines] + before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines] before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines] before_action :apply_diff_view_cookie!, only: [:new_diffs] before_action :build_merge_request, only: [:new, :new_diffs] @@ -33,7 +33,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :authenticate_user!, only: [:assign_related_issues] - before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts] + before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :conflict_for_path, :resolve_conflicts] def index @merge_requests = merge_requests_collection @@ -170,6 +170,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def conflict_for_path + return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui? + + file = @merge_request.conflicts.file_for_path(params[:old_path], params[:new_path]) + + return render_404 unless file + + render json: file, full_content: true + end + def resolve_conflicts return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui? @@ -184,7 +194,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController flash[:notice] = 'All merge conflicts were resolved. The merge request can now be merged.' render json: { redirect_to: namespace_project_merge_request_url(@project.namespace, @project, @merge_request, resolved_conflicts: true) } - rescue Gitlab::Conflict::File::MissingResolution => e + rescue Gitlab::Conflict::ResolutionError => e render status: :bad_request, json: { message: e.message } end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5ccfe11a2a2..8c6905a442d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -871,7 +871,7 @@ class MergeRequest < ActiveRecord::Base # files. conflicts.files.each(&:lines) @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0 - rescue Rugged::OdbError, Gitlab::Conflict::Parser::ParserError, Gitlab::Conflict::FileCollection::ConflictSideMissing + rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing @conflicts_can_be_resolved_in_ui = false end end diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb index 19caa038c44..d22a1d3e0ad 100644 --- a/app/services/merge_requests/resolve_service.rb +++ b/app/services/merge_requests/resolve_service.rb @@ -1,5 +1,8 @@ module MergeRequests class ResolveService < MergeRequests::BaseService + class MissingFiles < Gitlab::Conflict::ResolutionError + end + attr_accessor :conflicts, :rugged, :merge_index, :merge_request def execute(merge_request) @@ -10,8 +13,16 @@ module MergeRequests fetch_their_commit! - conflicts.files.each do |file| - write_resolved_file_to_index(file, params[:sections]) + params[:files].each do |file_params| + conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(conflict_file, file_params) + end + + unless merge_index.conflicts.empty? + missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } + + raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" end commit_params = { @@ -23,8 +34,13 @@ module MergeRequests project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params) end - def write_resolved_file_to_index(file, resolutions) - new_file = file.resolve_lines(resolutions).map(&:text).join("\n") + def write_resolved_file_to_index(file, params) + new_file = if params[:sections] + file.resolve_lines(params[:sections]).map(&:text).join("\n") + elsif params[:content] + file.resolve_content(params[:content]) + end + our_path = file.our_path merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) diff --git a/app/views/projects/merge_requests/conflicts.html.haml b/app/views/projects/merge_requests/conflicts.html.haml index a524936f73c..d9f74d2cbfb 100644 --- a/app/views/projects/merge_requests/conflicts.html.haml +++ b/app/views/projects/merge_requests/conflicts.html.haml @@ -1,11 +1,7 @@ -- class_bindings = "{ | - 'head': line.isHead, | - 'origin': line.isOrigin, | - 'match': line.hasMatch, | - 'selected': line.isSelected, | - 'unselected': line.isUnselected }" - - page_title "Merge Conflicts", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests" +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('merge_conflicts/merge_conflicts_bundle.js') + = page_specific_javascript_tag('lib/ace.js') = render "projects/merge_requests/show/mr_title" .merge-request-details.issuable-details @@ -24,6 +20,21 @@ = render partial: "projects/merge_requests/conflicts/commit_stats" .files-wrapper{"v-if" => "!isLoading && !hasError"} - = render partial: "projects/merge_requests/conflicts/parallel_view", locals: { class_bindings: class_bindings } - = render partial: "projects/merge_requests/conflicts/inline_view", locals: { class_bindings: class_bindings } + .files + .diff-file.file-holder.conflict{"v-for" => "file in conflictsData.files"} + .file-title + %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" => "!isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } + = render partial: "projects/merge_requests/conflicts/components/inline_conflict_lines" + .diff-wrap-lines.code.file-content.js-syntax-highlight{"v-show" => "isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } + = render partial: "projects/merge_requests/conflicts/components/parallel_conflict_lines" + %div{"v-show" => "file.resolveMode === 'edit' || file.type === 'text-editor'"} + = render partial: "projects/merge_requests/conflicts/components/diff_file_editor" + = render partial: "projects/merge_requests/conflicts/submit_form" + +-# Components += render partial: 'projects/merge_requests/conflicts/components/parallel_conflict_line' diff --git a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml index 457c467fba9..5ab3cd96163 100644 --- a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml +++ b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml @@ -1,20 +1,16 @@ .content-block.oneline-block.files-changed{"v-if" => "!isLoading && !hasError"} - .inline-parallel-buttons + .inline-parallel-buttons{"v-if" => "showDiffViewTypeSwitcher"} .btn-group - %a.btn{ | - ":class" => "{'active': !isParallel}", | - "@click" => "handleViewTypeChange('inline')"} + %button.btn{":class" => "{'active': !isParallel}", "@click" => "handleViewTypeChange('inline')"} Inline - %a.btn{ | - ":class" => "{'active': isParallel}", | - "@click" => "handleViewTypeChange('parallel')"} + %button.btn{":class" => "{'active': isParallel}", "@click" => "handleViewTypeChange('parallel')"} Side-by-side .js-toggle-container .commit-stat-summary Showing - %strong.cred {{conflictsCount}} {{conflictsData.conflictsText}} + %strong.cred {{conflictsCountText}} between - %strong {{conflictsData.source_branch}} + %strong {{conflictsData.sourceBranch}} and - %strong {{conflictsData.target_branch}} + %strong {{conflictsData.targetBranch}} diff --git a/app/views/projects/merge_requests/conflicts/_file_actions.html.haml b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml new file mode 100644 index 00000000000..05af57acf03 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml @@ -0,0 +1,12 @@ +.file-actions + .btn-group{"v-if" => "file.type === 'text'"} + %button.btn{ ":class" => "{ 'active': file.resolveMode == 'interactive' }", + '@click' => "onClickResolveModeButton(file, 'interactive')", + type: 'button' } + Interactive mode + %button.btn{ ':class' => "{ 'active': file.resolveMode == 'edit' }", + '@click' => "onClickResolveModeButton(file, 'edit')", + type: 'button' } + Edit inline + %a.btn.view-file.btn-file-option{":href" => "file.blobPath"} + View file @{{conflictsData.shortCommitSha}} diff --git a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml deleted file mode 100644 index 19c7da4b5e3..00000000000 --- a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml +++ /dev/null @@ -1,28 +0,0 @@ -.files{"v-show" => "!isParallel"} - .diff-file.file-holder.conflict.inline-view{"v-for" => "file in conflictsData.files"} - .file-title - %i.fa.fa-fw{":class" => "file.iconClass"} - %strong {{file.filePath}} - .file-actions - %a.btn.view-file.btn-file-option{":href" => "file.blobPath"} - View file @{{conflictsData.shortCommitSha}} - - .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight - %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(line.id, line.section)"} - {{line.buttonTitle}} diff --git a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml deleted file mode 100644 index 2e6f67c2eaf..00000000000 --- a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml +++ /dev/null @@ -1,27 +0,0 @@ -.files{"v-show" => "isParallel"} - .diff-file.file-holder.conflict.parallel-view{"v-for" => "file in conflictsData.files"} - .file-title - %i.fa.fa-fw{":class" => "file.iconClass"} - %strong {{file.filePath}} - .file-actions - %a.btn.view-file.btn-file-option{":href" => "file.blobPath"} - View file @{{conflictsData.shortCommitSha}} - - .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight - %table - %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} - %template{"v-for" => "line in section"} - - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{line.richText}} - %button.btn{"@click" => "handleSelected(line.id, line.section)"} - {{line.buttonTitle}} - - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.old_line{":class" => class_bindings} - {{line.lineNumber}} - %td.line_content.parallel{":class" => class_bindings} - {{{line.richText}}} diff --git a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml index 78bd4133ea2..6ffaa9ad4d2 100644 --- a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml +++ b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml @@ -1,15 +1,16 @@ -.content-block.oneline-block.files-changed - %strong.resolved-count {{resolvedCount}} - of - %strong.total-count {{conflictsCount}} - conflicts have been resolved - - .commit-message-container.form-group - .max-width-marker - %textarea.form-control.js-commit-message{"v-model" => "conflictsData.commitMessage"} - {{{conflictsData.commitMessage}}} - - %button{type: "button", class: "btn btn-success js-submit-button", ":disabled" => "!readyToCommit", "@click" => "commit()"} - %span {{commitButtonText}} - - = link_to "Cancel", namespace_project_merge_request_path(@merge_request.project.namespace, @merge_request.project, @merge_request), class: "btn btn-cancel" +.form-horizontal.resolve-conflicts-form + .form-group + %label.col-sm-2.control-label{ "for" => "commit-message" } + Commit message + .col-sm-10 + .commit-message-container + .max-width-marker + %textarea.form-control.js-commit-message#commit-message{ "v-model" => "conflictsData.commitMessage", "rows" => "5" } + .form-group + .col-sm-offset-2.col-sm-10 + .row + .col-xs-6 + %button{ type: "button", class: "btn btn-success js-submit-button", "@click" => "commit()", ":disabled" => "!readyToCommit" } + %span {{commitButtonText}} + .col-xs-6.text-right + = link_to "Cancel", namespace_project_merge_request_path(@merge_request.project.namespace, @merge_request.project, @merge_request), class: "btn btn-cancel" diff --git a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml new file mode 100644 index 00000000000..3c927d362c2 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml @@ -0,0 +1,13 @@ +%diff-file-editor{"inline-template" => "true", ":file" => "file", ":on-cancel-discard-confirmation" => "cancelDiscardConfirmation", ":on-accept-discard-confirmation" => "acceptDiscardConfirmation"} + .diff-editor-wrap{ "v-show" => "file.showEditor" } + .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } + .discard-changes-alert + Are you sure you want 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 + .editor-wrap{ ":class" => "classObject" } + .loading + %i.fa.fa-spinner.fa-spin + .editor + %pre{ "style" => "height: 350px" } diff --git a/app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml b/app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml new file mode 100644 index 00000000000..f094df7fcaa --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml @@ -0,0 +1,15 @@ +%inline-conflict-lines{ "inline-template" => "true", ":file" => "file"} + %table + %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} + %td.diff-line-num.new_line{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + %a {{line.new_line}} + %td.diff-line-num.old_line{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + %a {{line.old_line}} + %td.line_content{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + {{{line.richText}}} + %td.diff-line-num.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %td.diff-line-num.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %td.line_content.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %strong {{{line.richText}}} + %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } + {{line.buttonTitle}} diff --git a/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml new file mode 100644 index 00000000000..5690bf7419c --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml @@ -0,0 +1,10 @@ +%script{"id" => 'parallel-conflict-line', "type" => "text/x-template"} + %td.diff-line-num.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %td.line_content.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %strong {{line.richText}} + %button.btn{"@click" => "handleSelected(file, line.id, line.section)"} + {{line.buttonTitle}} + %td.diff-line-num.old_line{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + {{line.lineNumber}} + %td.line_content.parallel{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + {{{line.richText}}} diff --git a/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml new file mode 100644 index 00000000000..a8ecdf59393 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml @@ -0,0 +1,4 @@ +%parallel-conflict-lines{"inline-template" => "true", ":file" => "file"} + %table + %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} + %td{"is"=>"parallel-conflict-line", "v-for" => "line in section", ":line" => "line", ":file" => "file"} diff --git a/config/application.rb b/config/application.rb index 962ffe0708d..8a9c539cb43 100644 --- a/config/application.rb +++ b/config/application.rb @@ -89,6 +89,7 @@ module Gitlab config.assets.precompile << "profile/profile_bundle.js" config.assets.precompile << "diff_notes/diff_notes_bundle.js" config.assets.precompile << "boards/boards_bundle.js" + config.assets.precompile << "merge_conflicts/merge_conflicts_bundle.js" config.assets.precompile << "boards/test_utils/simulate_drag.js" config.assets.precompile << "blob_edit/blob_edit_bundle.js" config.assets.precompile << "snippet/snippet_bundle.js" diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index be22085b0df..3b8771543e4 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -67,6 +67,7 @@ if Gitlab::Metrics.enabled? ['app', 'finders'] => ['app', 'finders'], ['app', 'mailers', 'emails'] => ['app', 'mailers'], ['app', 'services', '**'] => ['app', 'services'], + ['lib', 'gitlab', 'conflicts'] => ['lib'], ['lib', 'gitlab', 'diff'] => ['lib'], ['lib', 'gitlab', 'email', 'message'] => ['lib'], ['lib', 'gitlab', 'checks'] => ['lib'] diff --git a/config/routes/project.rb b/config/routes/project.rb index 2cd8c60794a..711a59df744 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -267,6 +267,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: get :commits get :diffs get :conflicts + get :conflict_for_path get :builds get :pipelines get :merge_check diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index dff9e29c6a5..c843315782d 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -4,7 +4,7 @@ module Gitlab include Gitlab::Routing.url_helpers include IconsHelper - class MissingResolution < StandardError + class MissingResolution < ResolutionError end CONTEXT_LINES = 3 @@ -21,12 +21,34 @@ module Gitlab @match_line_headers = {} end + def content + merge_file_result[:data] + end + + def our_blob + @our_blob ||= repository.blob_at(merge_request.diff_refs.head_sha, our_path) + end + + def type + lines unless @type + + @type.inquiry + end + # Array of Gitlab::Diff::Line objects def lines - @lines ||= Gitlab::Conflict::Parser.new.parse(merge_file_result[:data], + return @lines if defined?(@lines) + + begin + @type = 'text' + @lines = Gitlab::Conflict::Parser.new.parse(content, our_path: our_path, their_path: their_path, parent_file: self) + rescue Gitlab::Conflict::Parser::ParserError + @type = 'text-editor' + @lines = nil + end end def resolve_lines(resolution) @@ -53,6 +75,14 @@ module Gitlab end.compact end + def resolve_content(resolution) + if resolution == content + raise MissingResolution, "Resolved content has no changes for file #{our_path}" + end + + resolution + end + def highlight_lines! their_file = lines.reject { |line| line.type == 'new' }.map(&:text).join("\n") our_file = lines.reject { |line| line.type == 'old' }.map(&:text).join("\n") @@ -170,21 +200,39 @@ module Gitlab match_line.text = "@@ -#{match_line.old_pos},#{line.old_pos} +#{match_line.new_pos},#{line.new_pos} @@#{header}" end - def as_json(opts = nil) - { + def as_json(opts = {}) + json_hash = { old_path: their_path, new_path: our_path, blob_icon: file_type_icon_class('file', our_mode, our_path), blob_path: namespace_project_blob_path(merge_request.project.namespace, merge_request.project, - ::File.join(merge_request.diff_refs.head_sha, our_path)), - sections: sections + ::File.join(merge_request.diff_refs.head_sha, our_path)) } + + json_hash.tap do |json_hash| + if opts[:full_content] + json_hash[:content] = content + json_hash[:blob_ace_mode] = our_blob && our_blob.language.try(:ace_mode) + else + json_hash[:sections] = sections if type.text? + json_hash[:type] = type + json_hash[:content_path] = content_path + end + end + end + + def content_path + conflict_for_path_namespace_project_merge_request_path(merge_request.project.namespace, + merge_request.project, + merge_request, + old_path: their_path, + new_path: our_path) end # Don't try to print merge_request or repository. def inspect - instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode].map do |instance_variable| + instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode, :type].map do |instance_variable| value = instance_variable_get("@#{instance_variable}") "#{instance_variable}=\"#{value}\"" diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index bbd0427a2c8..fa5bd4649d4 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -30,6 +30,10 @@ module Gitlab end end + def file_for_path(old_path, new_path) + files.find { |file| file.their_path == old_path && file.our_path == new_path } + end + def as_json(opts = nil) { target_branch: merge_request.target_branch, diff --git a/lib/gitlab/conflict/parser.rb b/lib/gitlab/conflict/parser.rb index 98e842cded3..ddd657903fb 100644 --- a/lib/gitlab/conflict/parser.rb +++ b/lib/gitlab/conflict/parser.rb @@ -1,19 +1,24 @@ module Gitlab module Conflict class Parser - class ParserError < StandardError + class UnresolvableError < StandardError end - class UnexpectedDelimiter < ParserError + class UnmergeableFile < UnresolvableError end - class MissingEndDelimiter < ParserError + class UnsupportedEncoding < UnresolvableError + end + + # Recoverable errors - the conflict can be resolved in an editor, but not with + # sections. + class ParserError < StandardError end - class UnmergeableFile < ParserError + class UnexpectedDelimiter < ParserError end - class UnsupportedEncoding < ParserError + class MissingEndDelimiter < ParserError end def parse(text, our_path:, their_path:, parent_file: nil) diff --git a/lib/gitlab/conflict/resolution_error.rb b/lib/gitlab/conflict/resolution_error.rb new file mode 100644 index 00000000000..a0f2006bc24 --- /dev/null +++ b/lib/gitlab/conflict/resolution_error.rb @@ -0,0 +1,6 @@ +module Gitlab + module Conflict + class ResolutionError < StandardError + end + end +end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index d509f0f2b96..d6980471ea4 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -570,7 +570,7 @@ describe Projects::MergeRequestsController do context 'when the conflicts cannot be resolved in the UI' do before do allow_any_instance_of(Gitlab::Conflict::Parser). - to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnexpectedDelimiter) + to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile) get :conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, @@ -597,6 +597,10 @@ describe Projects::MergeRequestsController do format: 'json' end + it 'matches the schema' do + expect(response).to match_response_schema('conflicts') + end + it 'includes meta info about the MR' do expect(json_response['commit_message']).to include('Merge branch') expect(json_response['commit_sha']).to match(/\h{40}/) @@ -658,26 +662,97 @@ describe Projects::MergeRequestsController do end end + describe 'GET conflict_for_path' do + let(:json_response) { JSON.parse(response.body) } + + def conflict_for_path(path) + get :conflict_for_path, + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project.to_param, + id: merge_request_with_conflicts.iid, + old_path: path, + new_path: path, + format: 'json' + end + + context 'when the conflicts cannot be resolved in the UI' do + before do + allow_any_instance_of(Gitlab::Conflict::Parser). + to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile) + + conflict_for_path('files/ruby/regex.rb') + end + + it 'returns a 404 status code' do + expect(response).to have_http_status(:not_found) + end + end + + context 'when the file does not exist cannot be resolved in the UI' do + before { conflict_for_path('files/ruby/regexp.rb') } + + it 'returns a 404 status code' do + expect(response).to have_http_status(:not_found) + end + end + + context 'with an existing file' do + let(:path) { 'files/ruby/regex.rb' } + + before { conflict_for_path(path) } + + it 'returns a 200 status code' do + expect(response).to have_http_status(:ok) + end + + it 'returns the file in JSON format' do + content = merge_request_with_conflicts.conflicts.file_for_path(path, path).content + + expect(json_response).to include('old_path' => path, + 'new_path' => path, + 'blob_icon' => 'file-text-o', + 'blob_path' => a_string_ending_with(path), + 'blob_ace_mode' => 'ruby', + 'content' => content) + end + end + end + context 'POST resolve_conflicts' do let(:json_response) { JSON.parse(response.body) } let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } - def resolve_conflicts(sections) + def resolve_conflicts(files) post :resolve_conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, project_id: merge_request_with_conflicts.project.to_param, id: merge_request_with_conflicts.iid, format: 'json', - sections: sections, + files: files, commit_message: 'Commit message' end context 'with valid params' do before do - resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin') + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'sections' => { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) end it 'creates a new commit on the branch' do @@ -692,7 +767,23 @@ describe Projects::MergeRequestsController do context 'when sections are missing' do before do - resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head') + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'sections' => { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' + } + } + ] + + resolve_conflicts(resolved_files) end it 'returns a 400 error' do @@ -700,7 +791,71 @@ describe Projects::MergeRequestsController do end it 'has a message with the name of the first missing section' do - expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9') + expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21') + end + + it 'does not create a new commit' do + expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) + end + end + + context 'when files are missing' do + before do + resolved_files = [ + { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) + end + + it 'returns a 400 error' do + expect(response).to have_http_status(:bad_request) + end + + it 'has a message with the name of the missing file' do + expect(json_response['message']).to include('files/ruby/popen.rb') + end + + it 'does not create a new commit' do + expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) + end + end + + context 'when a file has identical content to the conflict' do + before do + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'content' => merge_request_with_conflicts.conflicts.file_for_path('files/ruby/popen.rb', 'files/ruby/popen.rb').content + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) + end + + it 'returns a 400 error' do + expect(response).to have_http_status(:bad_request) + end + + it 'has a message with the path of the problem file' do + expect(json_response['message']).to include('files/ruby/popen.rb') end it 'does not create a new commit' do diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index 759edf8ec80..d258ff52bbb 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -12,29 +12,139 @@ 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-resolvable') } + shared_examples "conflicts are resolved in Interactive mode" do + it 'conflicts are resolved in Interactive mode' do + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do + click_button 'Use ours' + end + + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + all('button', text: 'Use ours').each do |button| + button.click + end + 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 + + within find('.diff-file', text: 'files/ruby/popen.rb') do + expect(page).to have_selector('.line_content.new', text: "vars = { 'PWD' => path }") + expect(page).to have_selector('.line_content.new', text: "options = { chdir: path }") + end + + within find('.diff-file', text: 'files/ruby/regex.rb') do + expect(page).to have_selector('.line_content.new', text: "def username_regexp") + expect(page).to have_selector('.line_content.new', text: "def project_name_regexp") + expect(page).to have_selector('.line_content.new', text: "def path_regexp") + expect(page).to have_selector('.line_content.new', text: "def archive_formats_regexp") + expect(page).to have_selector('.line_content.new', text: "def git_reference_regexp") + expect(page).to have_selector('.line_content.new', text: "def default_regexp") + end + end + end + shared_examples "conflicts are resolved in Edit inline mode" do + it 'conflicts are resolved in Edit inline mode' do + expect(find('#conflicts')).to have_content('popen.rb') + + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do + click_button 'Edit inline' + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("One morning");') + end + + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + click_button 'Edit inline' + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file pre")[1]).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 + + expect(page).to have_content('One morning') + expect(page).to have_content('Gregor Samsa woke from troubled dreams') + end + end + + context 'can be resolved in the UI' do before do project.team << [user, :developer] login_as(user) - - visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - it 'shows a link to the conflict resolution page' do - expect(page).to have_link('conflicts', href: /\/conflicts\Z/) + context 'the conflicts are resolvable' do + let(:merge_request) { create_merge_request('conflict-resolvable') } + + before { visit namespace_project_merge_request_path(project.namespace, project, merge_request) } + + it 'shows a link to the conflict resolution page' do + expect(page).to have_link('conflicts', href: /\/conflicts\Z/) + end + + context 'in Inline view mode' do + before { click_link('conflicts', href: /\/conflicts\Z/) } + + include_examples "conflicts are resolved in Interactive mode" + include_examples "conflicts are resolved in Edit inline mode" + end + + context 'in Parallel view mode' do + before do + click_link('conflicts', href: /\/conflicts\Z/) + click_button 'Side-by-side' + end + + include_examples "conflicts are resolved in Interactive mode" + include_examples "conflicts are resolved in Edit inline mode" + end end - context 'visiting the conflicts resolution page' do - before { click_link('conflicts', href: /\/conflicts\Z/) } + context 'the conflict contain markers' do + let(:merge_request) { create_merge_request('conflict-contains-conflict-markers') } - it 'shows the conflicts' do - begin - expect(find('#conflicts')).to have_content('popen.rb') - rescue Capybara::Poltergeist::JavascriptError - retry + before do + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + click_link('conflicts', href: /\/conflicts\Z/) + end + + it 'conflicts can not be resolved in Interactive mode' do + within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do + expect(page).not_to have_content 'Interactive mode' + expect(page).not_to have_content 'Edit inline' + end + end + + it 'conflicts are resolved in Edit inline mode' do + within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file 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('.click-to-expand').click + wait_for_ajax + + expect(page).to have_content('Gregor Samsa woke from troubled dreams') end end end @@ -42,7 +152,6 @@ feature 'Merge request conflict resolution', js: true, feature: true do UNRESOLVABLE_CONFLICTS = { 'conflict-too-large' => 'when the conflicts contain a large file', 'conflict-binary-file' => 'when the conflicts contain a binary file', - 'conflict-contains-conflict-markers' => 'when the conflicts contain a file with ambiguous conflict markers', 'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another', 'conflict-non-utf8' => 'when the conflicts contain a non-UTF-8 file', } diff --git a/spec/fixtures/api/schemas/conflicts.json b/spec/fixtures/api/schemas/conflicts.json new file mode 100644 index 00000000000..a947783d505 --- /dev/null +++ b/spec/fixtures/api/schemas/conflicts.json @@ -0,0 +1,137 @@ +{ + "type": "object", + "required": [ + "commit_message", + "commit_sha", + "source_branch", + "target_branch", + "files" + ], + "properties": { + "commit_message": {"type": "string"}, + "commit_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"}, + "source_branch": {"type": "string"}, + "target_branch": {"type": "string"}, + "files": { + "type": "array", + "items": { + "oneOf": [ + { "$ref": "#/definitions/conflict-text-with-sections" }, + { "$ref": "#/definitions/conflict-text-for-editor" } + ] + } + } + }, + "definitions": { + "conflict-base": { + "type": "object", + "required": [ + "old_path", + "new_path", + "blob_icon", + "blob_path" + ], + "properties": { + "old_path": {"type": "string"}, + "new_path": {"type": "string"}, + "blob_icon": {"type": "string"}, + "blob_path": {"type": "string"} + } + }, + "conflict-text-for-editor": { + "allOf": [ + {"$ref": "#/definitions/conflict-base"}, + { + "type": "object", + "required": [ + "type", + "content_path" + ], + "properties": { + "type": {"type": {"enum": ["text-editor"]}}, + "content_path": {"type": "string"} + } + } + ] + }, + "conflict-text-with-sections": { + "allOf": [ + {"$ref": "#/definitions/conflict-base"}, + { + "type": "object", + "required": [ + "type", + "content_path", + "sections" + ], + "properties": { + "type": {"type": {"enum": ["text"]}}, + "content_path": {"type": "string"}, + "sections": { + "type": "array", + "items": { + "oneOf": [ + { "$ref": "#/definitions/section-context" }, + { "$ref": "#/definitions/section-conflict" } + ] + } + } + } + } + ] + }, + "section-base": { + "type": "object", + "required": [ + "conflict", + "lines" + ], + "properties": { + "conflict": {"type": "boolean"}, + "lines": { + "type": "array", + "items": { + "type": "object", + "required": [ + "old_line", + "new_line", + "text", + "rich_text" + ], + "properties": { + "type": {"type": "string"}, + "old_line": {"type": "string"}, + "new_line": {"type": "string"}, + "text": {"type": "string"}, + "rich_text": {"type": "string"} + } + } + } + } + }, + "section-context": { + "allOf": [ + {"$ref": "#/definitions/section-base"}, + { + "type": "object", + "properties": { + "conflict": {"enum": [false]} + } + } + ] + }, + "section-conflict": { + "allOf": [ + {"$ref": "#/definitions/section-base"}, + { + "type": "object", + "required": ["id"], + "properties": { + "conflict": {"enum": [true]}, + "id": {"type": "string"} + } + } + ] + } + } +} diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb index 60020487061..648d342ecf8 100644 --- a/spec/lib/gitlab/conflict/file_spec.rb +++ b/spec/lib/gitlab/conflict/file_spec.rb @@ -257,5 +257,16 @@ FILE it 'includes the blob icon for the file' do expect(conflict_file.as_json[:blob_icon]).to eq('file-text-o') end + + context 'with the full_content option passed' do + it 'includes the full content of the conflict' do + expect(conflict_file.as_json(full_content: true)).to have_key(:content) + end + + it 'includes the detected language of the conflict file' do + expect(conflict_file.as_json(full_content: true)[:blob_ace_mode]). + to eq('ruby') + end + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5884b4cff8c..91a423b670c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1155,12 +1155,6 @@ describe MergeRequest, models: true do expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_falsey end - it 'returns a falsey value when the conflicts contain a file with ambiguous conflict markers' do - merge_request = create_merge_request('conflict-contains-conflict-markers') - - expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_falsey - end - it 'returns a falsey value when the conflicts contain a file edited in one branch and deleted in another' do merge_request = create_merge_request('conflict-missing-side') @@ -1172,6 +1166,12 @@ describe MergeRequest, models: true do expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_truthy end + + it 'returns a truthy value when the conflicts have to be resolved in an editor' do + merge_request = create_merge_request('conflict-contains-conflict-markers') + + expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_truthy + end end describe "#forked_source_project_missing?" do diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb index d71932458fa..388abb6a0df 100644 --- a/spec/services/merge_requests/resolve_service_spec.rb +++ b/spec/services/merge_requests/resolve_service_spec.rb @@ -24,15 +24,26 @@ describe MergeRequests::ResolveService do end describe '#execute' do - context 'with valid params' do + context 'with section params' do let(:params) do { - sections: { - '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' - }, + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + sections: { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ], commit_message: 'This is a commit message!' } end @@ -49,7 +60,7 @@ describe MergeRequests::ResolveService do it 'creates a commit with the correct parents' do expect(merge_request.source_branch_head.parents.map(&:id)). to eq(['1450cd639e0bc6721eb02800169e464f212cde06', - '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b']) + '824be604a34828eb682305f0d963056cfac87b2d']) end end @@ -74,8 +85,96 @@ describe MergeRequests::ResolveService do end end - context 'when a resolution is missing' do - let(:invalid_params) { { sections: { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' } } } + context 'with content and sections params' do + let(:popen_content) { "class Popen\nend" } + + let(:params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: popen_content + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ], + commit_message: 'This is a commit message!' + } + end + + before do + MergeRequests::ResolveService.new(project, user, params).execute(merge_request) + end + + it 'creates a commit with the message' do + expect(merge_request.source_branch_head.message).to eq(params[:commit_message]) + end + + it 'creates a commit with the correct parents' do + expect(merge_request.source_branch_head.parents.map(&:id)). + to eq(['1450cd639e0bc6721eb02800169e464f212cde06', + '824be604a34828eb682305f0d963056cfac87b2d']) + end + + it 'sets the content to the content given' do + blob = merge_request.source_project.repository.blob_at(merge_request.source_branch_head.sha, + 'files/ruby/popen.rb') + + expect(blob.data).to eq(popen_content) + end + end + + context 'when a resolution section is missing' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' } + } + ], + commit_message: 'This is a commit message!' + } + end + + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } + + it 'raises a MissingResolution error' do + expect { service.execute(merge_request) }. + to raise_error(Gitlab::Conflict::File::MissingResolution) + end + end + + context 'when the content of a file is unchanged' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + content: merge_request.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content + } + ], + commit_message: 'This is a commit message!' + } + end + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } it 'raises a MissingResolution error' do @@ -83,5 +182,27 @@ describe MergeRequests::ResolveService do to raise_error(Gitlab::Conflict::File::MissingResolution) end end + + context 'when a file is missing' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + } + ], + commit_message: 'This is a commit message!' + } + end + + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } + + it 'raises a MissingFiles error' do + expect { service.execute(merge_request) }. + to raise_error(MergeRequests::ResolveService::MissingFiles) + end + end end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 725299031d0..c79975d8667 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -27,10 +27,10 @@ module TestEnv 'expand-collapse-lines' => '238e82d', 'video' => '8879059', 'crlf-diff' => '5938907', - 'conflict-start' => '75284c7', + 'conflict-start' => '824be60', 'conflict-resolvable' => '1450cd6', 'conflict-binary-file' => '259a6fb', - 'conflict-contains-conflict-markers' => '5e0964c', + 'conflict-contains-conflict-markers' => '78a3086', 'conflict-missing-side' => 'eb227b3', 'conflict-non-utf8' => 'd0a293c', 'conflict-too-large' => '39fa04f', |