Skip to content
Snippets Groups Projects
Commit 05fdd12f authored by Valery Sizov's avatar Valery Sizov
Browse files

Improve error message when merging fails

parent 2714d5b8
No related branches found
No related tags found
No related merge requests found
Loading
@@ -18,6 +18,7 @@ v 8.1.0 (unreleased)
Loading
@@ -18,6 +18,7 @@ v 8.1.0 (unreleased)
- Move CI triggers page to project settings area - Move CI triggers page to project settings area
- Move CI project settings page to CE 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 - Fix bug when removed file was not appearing in merge request diff
- Improve error message when merging fails
   
v 8.0.3 v 8.0.3
- Fix URL shown in Slack notifications - Fix URL shown in Slack notifications
Loading
Loading
Loading
@@ -15,11 +15,12 @@ class @MergeRequestWidget
Loading
@@ -15,11 +15,12 @@ class @MergeRequestWidget
type: 'GET' type: 'GET'
url: $('.merge-request').data('url') url: $('.merge-request').data('url')
success: (data) => success: (data) =>
switch data.state if data.state == "merged"
when 'merged' location.reload()
location.reload() else if data.merge_error
else $('.mr-widget-body').html("<h4>" + data.merge_error + "</h4>")
setTimeout(merge_request_widget.mergeInProgress, 2000) else
setTimeout(merge_request_widget.mergeInProgress, 2000)
dataType: 'json' dataType: 'json'
   
getMergeStatus: -> getMergeStatus: ->
Loading
Loading
Loading
@@ -150,6 +150,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
Loading
@@ -150,6 +150,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
return access_denied! unless @merge_request.can_be_merged_by?(current_user) return access_denied! unless @merge_request.can_be_merged_by?(current_user)
   
if @merge_request.mergeable? if @merge_request.mergeable?
@merge_request.update(merge_error: nil)
MergeWorker.perform_async(@merge_request.id, current_user.id, params) MergeWorker.perform_async(@merge_request.id, current_user.id, params)
@status = true @status = true
else else
Loading
Loading
Loading
@@ -38,6 +38,10 @@ module MergeRequests
Loading
@@ -38,6 +38,10 @@ module MergeRequests
} }
   
repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) 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 end
   
def after_merge def after_merge
Loading
Loading
class MergeRequestErrorField < ActiveRecord::Migration
def up
add_column :merge_requests, :merge_error, :string
end
end
Loading
@@ -11,7 +11,7 @@
Loading
@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
Loading
@@ -453,6 +453,7 @@ ActiveRecord::Schema.define(version: 20150930095736) do
Loading
@@ -453,6 +453,7 @@ ActiveRecord::Schema.define(version: 20150930095736) do
t.integer "position", default: 0 t.integer "position", default: 0
t.datetime "locked_at" t.datetime "locked_at"
t.integer "updated_by_id" t.integer "updated_by_id"
t.string "merge_error"
end end
   
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
Loading
Loading
Loading
@@ -35,5 +35,19 @@ describe MergeRequests::MergeService do
Loading
@@ -35,5 +35,19 @@ describe MergeRequests::MergeService do
expect(note.note).to include 'Status changed to merged' expect(note.note).to include 'Status changed to merged'
end end
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
end end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment