diff options
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 18 | ||||
| -rw-r--r-- | lib/gitlab/conflict/file.rb | 10 | ||||
| -rw-r--r-- | lib/gitlab/conflict/parser.rb | 7 | ||||
| -rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 47 |
4 files changed, 69 insertions, 13 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3205d7cb8e4..6772672992c 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -139,15 +139,25 @@ class Projects::MergeRequestsController < Projects::ApplicationController render 'show' end - format.json { render json: Gitlab::Conflict::FileCollection.new(@merge_request) } + format.json do + begin + render json: Gitlab::Conflict::FileCollection.new(@merge_request) + rescue Gitlab::Conflict::Parser::ParserError => e + render json: { message: 'Unable to resolve conflicts in the web interface for this merge request' } + end + end end end def resolve_conflicts - Gitlab::Conflict::FileCollection.new(@merge_request).resolve_conflicts!(params[:merge_request], nil, user: current_user) + begin + 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.' + redirect_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), + notice: 'Merge conflicts resolved. The merge request can now be merged.' + rescue Gitlab::Conflict::File::MissingResolution => e + render status: :bad_request, json: { message: e.message } + end end def builds diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index 9ba8ae0367f..598bf2a2612 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -35,23 +35,23 @@ module Gitlab end def resolve_lines(resolution) - current_section = nil + section_id = nil lines.map do |line| unless line.type - current_section = nil + section_id = nil next line end - current_section ||= resolution[line_code(line)] + section_id ||= line_code(line) - case current_section + case resolution[section_id] when 'ours' next unless line.type == 'new' when 'theirs' next unless line.type == 'old' else - raise MissingResolution + raise MissingResolution, "Missing resolution for section ID: #{section_id}" end line diff --git a/lib/gitlab/conflict/parser.rb b/lib/gitlab/conflict/parser.rb index 0aa85202d56..8ab7b6499aa 100644 --- a/lib/gitlab/conflict/parser.rb +++ b/lib/gitlab/conflict/parser.rb @@ -1,10 +1,13 @@ module Gitlab module Conflict class Parser - class UnexpectedDelimiter < StandardError + class ParserError < StandardError end - class MissingEndDelimiter < StandardError + class UnexpectedDelimiter < ParserError + end + + class MissingEndDelimiter < ParserError end def parse(text, our_path:, their_path:, parent: nil) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index a876a440ab8..bca5d8c0655 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -530,8 +530,13 @@ describe Projects::MergeRequestsController do end describe 'GET conflicts' do - context 'as JSON' do + let(:json_response) { JSON.parse(response.body) } + + 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) + get :conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, project_id: merge_request_with_conflicts.project.to_param, @@ -539,7 +544,23 @@ describe Projects::MergeRequestsController do format: 'json' end - let(:json_response) { JSON.parse(response.body) } + it 'returns a 200 status code' do + expect(response).to have_http_status(:ok) + end + + it 'returns JSON with a message' do + expect(json_response.keys).to contain_exactly('message') + end + end + + context 'with valid conflicts' do + before do + get :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' + end it 'includes meta info about the MR' do expect(json_response['commit_message']).to include('Merge branch') @@ -589,6 +610,9 @@ describe Projects::MergeRequestsController do 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(params) post :resolve_conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, @@ -607,6 +631,7 @@ describe Projects::MergeRequestsController do end it 'creates a new commit on the branch' do + expect(original_head_sha).not_to eq(merge_request_with_conflicts.source_branch_head.sha) expect(merge_request_with_conflicts.source_branch_head.message).to include('Merge branch') end @@ -614,5 +639,23 @@ describe Projects::MergeRequestsController 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 + + context 'when sections are missing' do + before do + resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_4_4' => 'ours') + 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 first missing section' do + expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9') + 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 end end |