Allow resolving conflicts in MR controller
This commit is contained in:
parent
a1c7961217
commit
14a4b17d1c
|
|
@ -10,7 +10,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
|||
before_action :module_enabled
|
||||
before_action :merge_request, only: [
|
||||
:edit, :update, :show, :diffs, :commits, :conflicts, :builds, :merge, :merge_check,
|
||||
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip
|
||||
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts
|
||||
]
|
||||
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
|
||||
before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds]
|
||||
|
|
@ -139,6 +139,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
|||
end
|
||||
end
|
||||
|
||||
def resolve_conflicts
|
||||
Gitlab::Conflict::FileCollection.new(@merge_request).resolve_conflicts!(params[:merge_request], nil, user: current_user)
|
||||
|
||||
redirect_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request),
|
||||
notice: 'Merge conflicts resolved. The merge request can now be merged.'
|
||||
end
|
||||
|
||||
def builds
|
||||
respond_to do |format|
|
||||
format.html do
|
||||
|
|
|
|||
|
|
@ -728,6 +728,7 @@ Rails.application.routes.draw do
|
|||
post :toggle_award_emoji
|
||||
post :remove_wip
|
||||
get :diff_for_path
|
||||
post :resolve_conflicts
|
||||
end
|
||||
|
||||
collection do
|
||||
|
|
|
|||
|
|
@ -1,6 +1,9 @@
|
|||
module Gitlab
|
||||
module Conflict
|
||||
class File
|
||||
class MissingResolution < StandardError
|
||||
end
|
||||
|
||||
CONTEXT_LINES = 3
|
||||
|
||||
attr_reader :merge_file_result, :their_path, :their_ref, :our_path, :our_ref, :repository
|
||||
|
|
@ -21,6 +24,39 @@ module Gitlab
|
|||
their_path: their_path)
|
||||
end
|
||||
|
||||
def resolve!(resolution, index:, rugged:)
|
||||
new_file = resolve_lines(resolution).map(&:text).join("\n")
|
||||
|
||||
oid = rugged.write(new_file, :blob)
|
||||
our_mode = index.conflict_get(our_path)[:ours][:mode]
|
||||
index.add(path: our_path, oid: oid, mode: our_mode)
|
||||
index.conflict_remove(our_path)
|
||||
end
|
||||
|
||||
def resolve_lines(resolution)
|
||||
current_section = nil
|
||||
|
||||
lines.map do |line|
|
||||
unless line.type
|
||||
current_section = nil
|
||||
next line
|
||||
end
|
||||
|
||||
current_section ||= resolution[line_code(line)]
|
||||
|
||||
case current_section
|
||||
when 'ours'
|
||||
next unless line.type == 'new'
|
||||
when 'theirs'
|
||||
next unless line.type == 'old'
|
||||
else
|
||||
raise MissingResolution
|
||||
end
|
||||
|
||||
line
|
||||
end.compact
|
||||
end
|
||||
|
||||
def highlighted_lines
|
||||
return @highlighted_lines if @highlighted_lines
|
||||
|
||||
|
|
@ -77,10 +113,16 @@ module Gitlab
|
|||
match_line.text = "@@ -#{match_line.old_pos},#{lines.last.old_pos} +#{match_line.new_pos},#{lines.last.new_pos} @@"
|
||||
end
|
||||
|
||||
section || { conflict: !no_conflict, lines: lines }
|
||||
section ||= { conflict: !no_conflict, lines: lines }
|
||||
section[:id] = line_code(lines.first) unless no_conflict
|
||||
section
|
||||
end
|
||||
end
|
||||
|
||||
def line_code(line)
|
||||
Gitlab::Diff::LineCode.generate(our_path, line.new_pos, line.old_pos)
|
||||
end
|
||||
|
||||
def as_json(opts = nil)
|
||||
{
|
||||
old_path: their_path,
|
||||
|
|
|
|||
|
|
@ -17,6 +17,26 @@ module Gitlab
|
|||
@merge_index ||= repository.rugged.merge_commits(our_commit, their_commit)
|
||||
end
|
||||
|
||||
def resolve_conflicts!(resolutions, commit_message, user:)
|
||||
rugged = repository.rugged
|
||||
committer = repository.user_to_committer(user)
|
||||
commit_message ||= default_commit_message
|
||||
|
||||
files.each do |file|
|
||||
file.resolve!(resolutions, index: merge_index, rugged: rugged)
|
||||
end
|
||||
|
||||
new_tree = merge_index.write_tree(rugged)
|
||||
|
||||
Rugged::Commit.create(rugged,
|
||||
author: committer,
|
||||
committer: committer,
|
||||
tree: new_tree,
|
||||
message: commit_message,
|
||||
parents: [our_commit, their_commit],
|
||||
update_ref: Gitlab::Git::BRANCH_REF_PREFIX + merge_request.source_branch)
|
||||
end
|
||||
|
||||
def files
|
||||
@files ||= merge_index.conflicts.map do |conflict|
|
||||
their_path = conflict[:theirs][:path]
|
||||
|
|
|
|||
|
|
@ -4,6 +4,11 @@ describe Projects::MergeRequestsController do
|
|||
let(:project) { create(:project) }
|
||||
let(:user) { create(:user) }
|
||||
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
|
||||
let(:merge_request_with_conflicts) do
|
||||
create(:merge_request, source_branch: 'conflict-a', target_branch: 'conflict-b', source_project: project) do |mr|
|
||||
mr.mark_as_unmergeable
|
||||
end
|
||||
end
|
||||
|
||||
before do
|
||||
sign_in(user)
|
||||
|
|
@ -525,12 +530,6 @@ describe Projects::MergeRequestsController do
|
|||
end
|
||||
|
||||
describe 'GET conflicts' do
|
||||
let(:merge_request_with_conflicts) do
|
||||
create(:merge_request, source_branch: 'conflict-a', target_branch: 'conflict-b', source_project: project) do |mr|
|
||||
mr.mark_as_unmergeable
|
||||
end
|
||||
end
|
||||
|
||||
context 'as JSON' do
|
||||
before do
|
||||
get :conflicts,
|
||||
|
|
@ -588,4 +587,32 @@ describe Projects::MergeRequestsController do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'POST resolve_conflicts' do
|
||||
def resolve_conflicts(params)
|
||||
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',
|
||||
merge_request: params
|
||||
end
|
||||
|
||||
context 'with valid params' do
|
||||
before do
|
||||
resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_4_4' => 'ours',
|
||||
'6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'ours',
|
||||
'6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'theirs',
|
||||
'6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'theirs')
|
||||
end
|
||||
|
||||
it 'creates a new commit on the branch' do
|
||||
expect(merge_request_with_conflicts.source_branch_head.message).to include('Merge branch')
|
||||
end
|
||||
|
||||
it 'redirects to the MR show page' do
|
||||
expect(response).to redirect_to([merge_request_with_conflicts.target_project.namespace.becomes(Namespace), merge_request_with_conflicts.target_project, merge_request_with_conflicts])
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -12,6 +12,57 @@ describe Gitlab::Conflict::File, lib: true do
|
|||
let(:merge_file_result) { index.merge_file('files/ruby/regex.rb') }
|
||||
let(:conflict_file) { Gitlab::Conflict::File.new(merge_file_result, conflict, diff_refs: diff_refs, repository: repository) }
|
||||
|
||||
describe '#resolve_lines' do
|
||||
let(:section_keys) { conflict_file.sections.map { |section| section[:id] }.compact }
|
||||
|
||||
context 'when resolving everything to the same side' do
|
||||
let(:resolution_hash) { section_keys.map { |key| [key, 'ours'] }.to_h }
|
||||
let(:resolved_lines) { conflict_file.resolve_lines(resolution_hash) }
|
||||
let(:expected_lines) { conflict_file.lines.reject { |line| line.type == 'old' } }
|
||||
|
||||
it 'has the correct number of lines' do
|
||||
expect(resolved_lines.length).to eq(expected_lines.length)
|
||||
end
|
||||
|
||||
it 'has content matching the chosen lines' do
|
||||
expect(resolved_lines.map(&:text)).to eq(expected_lines.map(&:text))
|
||||
end
|
||||
end
|
||||
|
||||
context 'with mixed resolutions' do
|
||||
let(:resolution_hash) do
|
||||
section_keys.map.with_index { |key, i| [key, i.even? ? 'ours' : 'theirs'] }.to_h
|
||||
end
|
||||
|
||||
let(:resolved_lines) { conflict_file.resolve_lines(resolution_hash) }
|
||||
|
||||
it 'has the correct number of lines' do
|
||||
file_lines = conflict_file.lines.reject { |line| line.type == 'new' }
|
||||
|
||||
expect(resolved_lines.length).to eq(file_lines.length)
|
||||
end
|
||||
|
||||
it 'returns a file containing only the chosen parts of the resolved sections' do
|
||||
expect(resolved_lines.chunk { |line| line.type || 'both' }.map(&:first)).
|
||||
to eq(['both', 'new', 'both', 'old', 'both', 'new', 'both'])
|
||||
end
|
||||
end
|
||||
|
||||
it 'raises MissingResolution when passed a hash without resolutions for all sections' do
|
||||
empty_hash = section_keys.map { |key| [key, nil] }.to_h
|
||||
invalid_hash = section_keys.map { |key| [key, 'invalid'] }.to_h
|
||||
|
||||
expect { conflict_file.resolve_lines({}) }.
|
||||
to raise_error(Gitlab::Conflict::File::MissingResolution)
|
||||
|
||||
expect { conflict_file.resolve_lines(empty_hash) }.
|
||||
to raise_error(Gitlab::Conflict::File::MissingResolution)
|
||||
|
||||
expect { conflict_file.resolve_lines(invalid_hash) }.
|
||||
to raise_error(Gitlab::Conflict::File::MissingResolution)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#highlighted_lines' do
|
||||
def html_to_text(html)
|
||||
CGI.unescapeHTML(ActionView::Base.full_sanitizer.sanitize(html)).delete("\n")
|
||||
|
|
@ -69,5 +120,20 @@ describe Gitlab::Conflict::File, lib: true do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'adds unique IDs to conflict sections, and not to other sections' do
|
||||
section_ids = []
|
||||
|
||||
conflict_file.sections.each do |section|
|
||||
if section[:conflict]
|
||||
expect(section).to have_key(:id)
|
||||
section_ids << section[:id]
|
||||
else
|
||||
expect(section).not_to have_key(:id)
|
||||
end
|
||||
end
|
||||
|
||||
expect(section_ids.uniq).to eq(section_ids)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue