From 05fdd12fd984ffee0b2c9be3821fbc9a67abc6d4 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Thu, 1 Oct 2015 09:31:48 +0300
Subject: [PATCH] Improve error message when merging fails
---
CHANGELOG | 1 +
.../javascripts/merge_request_widget.js.coffee | 11 ++++++-----
.../projects/merge_requests_controller.rb | 1 +
app/services/merge_requests/merge_service.rb | 4 ++++
.../20150930001110_merge_request_error_field.rb | 5 +++++
db/schema.rb | 3 ++-
spec/services/merge_requests/merge_service_spec.rb | 14 ++++++++++++++
7 files changed, 33 insertions(+), 6 deletions(-)
create mode 100644 db/migrate/20150930001110_merge_request_error_field.rb
diff --git a/CHANGELOG b/CHANGELOG
index 492e4b9aebf..dc8cac6769d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -18,6 +18,7 @@ v 8.1.0 (unreleased)
- Move CI triggers page to project settings area
- Move CI project settings page to CE project settings area
- Fix bug when removed file was not appearing in merge request diff
+ - 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("<h4>" + data.merge_error + "</h4>")
+ 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
--
GitLab