diff --git a/CHANGELOG b/CHANGELOG index d8e0abbdf86..981a93a2405 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,7 @@ v 8.1.0 (unreleased) - Move CI project settings page to CE project settings area - Fix bug when removed file was not appearing in merge request diff - Note the original location of a moved project when notifying users of the move + - Improve error message when merging fails v 8.0.3 - Fix URL shown in Slack notifications diff --git a/app/assets/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee index 995a2f24093..3176e5a8965 100644 --- a/app/assets/javascripts/merge_request_widget.js.coffee +++ b/app/assets/javascripts/merge_request_widget.js.coffee @@ -15,11 +15,12 @@ class @MergeRequestWidget type: 'GET' url: $('.merge-request').data('url') success: (data) => - switch data.state - when 'merged' - location.reload() - else - setTimeout(merge_request_widget.mergeInProgress, 2000) + if data.state == "merged" + location.reload() + else if data.merge_error + $('.mr-widget-body').html("

" + data.merge_error + "

") + else + setTimeout(merge_request_widget.mergeInProgress, 2000) dataType: 'json' getMergeStatus: -> diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7574842cd43..7570934e727 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -150,6 +150,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController return access_denied! unless @merge_request.can_be_merged_by?(current_user) if @merge_request.mergeable? + @merge_request.update(merge_error: nil) MergeWorker.perform_async(@merge_request.id, current_user.id, params) @status = true else diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 98a67c0bc99..fcc0f2a6a8d 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -38,6 +38,10 @@ module MergeRequests } repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) + rescue Exception => e + merge_request.update(merge_error: "Something went wrong during merge") + Rails.logger.error(e.message) + return false end def after_merge diff --git a/db/migrate/20150930001110_merge_request_error_field.rb b/db/migrate/20150930001110_merge_request_error_field.rb new file mode 100644 index 00000000000..c2ee498ef3f --- /dev/null +++ b/db/migrate/20150930001110_merge_request_error_field.rb @@ -0,0 +1,5 @@ +class MergeRequestErrorField < ActiveRecord::Migration + def up + add_column :merge_requests, :merge_error, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 4ce6cee86e5..0aac301587e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150930095736) do +ActiveRecord::Schema.define(version: 20150930001110) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -453,6 +453,7 @@ ActiveRecord::Schema.define(version: 20150930095736) do t.integer "position", default: 0 t.datetime "locked_at" t.integer "updated_by_id" + t.string "merge_error" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 7b564d34d7b..7483f51de03 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -35,5 +35,19 @@ describe MergeRequests::MergeService do expect(note.note).to include 'Status changed to merged' end end + + context "error handling" do + let(:service) { MergeRequests::MergeService.new(project, user, {}) } + + it 'saves error if there is an exception' do + allow(service).to receive(:repository).and_raise("error") + + allow(service).to receive(:execute_hooks) + + service.execute(merge_request, 'Awesome message') + + expect(merge_request.merge_error).to eq("Something went wrong during merge") + end + end end end