Skip to content

G
gitlab-ce
Commit 4e6af0c3 authored by Jacob Schatz's avatar Jacob Schatz
Browse files
Options

Merge branch 'merge-conflicts-editor-2' into 'master' 

Resolve Merge conflicts in editor

## What does this MR do?

This is the second iteration of https://gitlab.com/gitlab-org/gitlab-ce/issues/20344 which now allows to resolve conflicts manually providing an editor to the user.

## Are there points in the code the reviewer needs to double check?

- Previous implementation has been refactored almost from the ground up.
- Also some components like `parallel-conflict-lines` has been optimized to work well with PhantomJS.
- The app and files required files are loaded only when needed. `merge_conflicts_bundle.js` is only loaded in `/<group>/<project>/merge_requests/<Id>/conflicts` only

## Why was this MR needed?

Some MRs were unable to be solved interactively in the UI, with this MR we let the user to manually fix complex merge request conflicts.

## Screenshots (if relevant)

<img src="/uploads/33a016f025bd590b1fc6eeee1ee11626/Screen_Shot_2016-09-19_at_1.39.39_PM.png" width="800">

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/20344, https://gitlab.com/gitlab-org/gitlab-ce/issues/3567.

See merge request !6374
parents 8dd8985d 28dbfd5c
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 1560 additions and 586 deletions
.gitlab-ci.yml
View file @ 4e6af0c3
......@@ -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
......
CHANGELOG.md
View file @ 4e6af0c3
......@@ -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
......
app/assets/javascripts/dispatcher.js.es6
View file @ 4e6af0c3
......@@ -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();
......
app/assets/javascripts/merge_conflict_data_provider.js.es6 deleted 100644 → 0
View file @ 8dd8985d
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}`;
}
}
app/assets/javascripts/merge_conflict_resolver.js.es6 deleted 100644 → 0
View file @ 8dd8985d
//= 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!');
});
}
}
app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 0 → 100644
View file @ 4e6af0c3
((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 = {}));
app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 0 → 100644
View file @ 4e6af0c3
((global) => {
global.mergeConflicts = global.mergeConflicts || {};
global.mergeConflicts.inlineConflictLines = Vue.extend({
props: {
file: Object
},
mixins: [global.mergeConflicts.utils, global.mergeConflicts.actions],
});
})(window.gl || (window.gl = {}));
app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 0 → 100644
View file @ 4e6af0c3
((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 = {}));
app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 0 → 100644
View file @ 4e6af0c3
((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 = {}));
app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 0 → 100644
View file @ 4e6af0c3
((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 = {}));
app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 0 → 100644
View file @ 4e6af0c3
//= 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!');
});
}
}
})
});
app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 0 → 100644
View file @ 4e6af0c3
((global) => {
global.mergeConflicts = global.mergeConflicts || {};
global.mergeConflicts.actions = {
methods: {
handleSelected(file, sectionId, selection) {
gl.mergeConflicts.mergeConflictsStore.handleSelected(file, sectionId, selection);
}
}
};
})(window.gl || (window.gl = {}));
app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 0 → 100644
View file @ 4e6af0c3
((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 = {}));
app/assets/stylesheets/framework/variables.scss
View file @ 4e6af0c3
......@@ -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;
......
app/assets/stylesheets/pages/merge_conflicts.scss
View file @ 4e6af0c3
......@@ -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;
}
}
app/controllers/application_controller.rb
View file @ 4e6af0c3
......@@ -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
......
app/controllers/projects/merge_requests_controller.rb
View file @ 4e6af0c3
......@@ -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
......
app/models/merge_request.rb
View file @ 4e6af0c3
......@@ -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
......
app/services/merge_requests/resolve_service.rb
View file @ 4e6af0c3
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)
......
app/views/projects/merge_requests/conflicts.html.haml
View file @ 4e6af0c3
- 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'
app/views/projects/merge_requests/conflicts/_commit_stats.html.haml
View file @ 4e6af0c3
.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}}
app/views/projects/merge_requests/conflicts/_file_actions.html.haml 0 → 100644
View file @ 4e6af0c3
.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}}
app/views/projects/merge_requests/conflicts/_inline_view.html.haml deleted 100644 → 0
View file @ 8dd8985d
.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}}
app/views/projects/merge_requests/conflicts/_parallel_view.html.haml deleted 100644 → 0
View file @ 8dd8985d
.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}}}
app/views/projects/merge_requests/conflicts/_submit_form.html.haml
View file @ 4e6af0c3
.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"
app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml 0 → 100644
View file @ 4e6af0c3
%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" }
app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml 0 → 100644
View file @ 4e6af0c3
%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}}
app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml 0 → 100644
View file @ 4e6af0c3
%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}}}
app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml 0 → 100644
View file @ 4e6af0c3
%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"}
config/application.rb
View file @ 4e6af0c3
......@@ -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"
......
config/initializers/metrics.rb
View file @ 4e6af0c3
......@@ -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']
......
config/routes/project.rb
View file @ 4e6af0c3
......@@ -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
......
lib/gitlab/conflict/file.rb
View file @ 4e6af0c3
......@@ -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}\""
......
lib/gitlab/conflict/file_collection.rb
View file @ 4e6af0c3
......@@ -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,
......
lib/gitlab/conflict/parser.rb
View file @ 4e6af0c3
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)
......
lib/gitlab/conflict/resolution_error.rb 0 → 100644
View file @ 4e6af0c3
module Gitlab
module Conflict
class ResolutionError < StandardError
end
end
end
spec/controllers/projects/merge_requests_controller_spec.rb
View file @ 4e6af0c3
......@@ -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
......
spec/features/merge_requests/conflicts_spec.rb
View file @ 4e6af0c3
......@@ -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',
}
......
spec/fixtures/api/schemas/conflicts.json 0 → 100644
View file @ 4e6af0c3
{
"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"}
}
}
]
}
}
}
spec/lib/gitlab/conflict/file_spec.rb
View file @ 4e6af0c3
......@@ -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
spec/models/merge_request_spec.rb
View file @ 4e6af0c3
......@@ -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
......
spec/services/merge_requests/resolve_service_spec.rb
View file @ 4e6af0c3
......@@ -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
spec/support/test_env.rb
View file @ 4e6af0c3
......@@ -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',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7