diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0fcb3063f29..e9d07614785 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -193,18 +193,18 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def revert - target_branch_exists = @merge_request.target_branch_exists? - url_params = { merge_request: { - source_branch: @merge_request.revert_branch_name, - target_branch: @merge_request.target_branch, - source_project_id: @merge_request.target_project_id, - target_project_id: @merge_request.target_project_id, - description: @merge_request.revert_description - }} + url_params = { + merge_request: { source_branch: @merge_request.revert_branch_name, + target_branch: @merge_request.target_branch, + source_project_id: @merge_request.target_project_id, + target_project_id: @merge_request.target_project_id, + description: @merge_request.revert_description } + } - if target_branch_exists + if @merge_request.target_branch_exists? && @merge_request.merge_commit_sha.present? @repository.revert_merge(current_user, @merge_request.merge_commit_sha, - @merge_request.revert_branch_name, @merge_request.revert_title) + @merge_request.revert_branch_name, @merge_request.target_branch, + @merge_request.revert_title) redirect_to new_namespace_project_merge_request_url(@project.namespace, @project, url_params) else diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b7e7d0c5a3b..44a62143cf7 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -135,7 +135,7 @@ module ApplicationHelper # Skip if user removed branch right after that return false unless project.repository.branch_names.include?(event.branch_name) - return false if Gitlab::Git::REVERT_BRANCH_PATTERN === event.branch_name + return false if event.branch_name =~ Gitlab::Git::REVERT_BRANCH_PATTERN true end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 15ebce359ce..4a7d57cbe10 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -535,11 +535,11 @@ class MergeRequest < ActiveRecord::Base end def revert_branch_name - "revert-#{iid}-#{target_branch}" + "revert-#{iid}-#{source_branch}" end def revert_title - "Revert \"#{title}\"" + "Revert \"#{title}\"" end def revert_description diff --git a/app/models/repository.rb b/app/models/repository.rb index 872066a02e4..cca7afadbec 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -622,25 +622,27 @@ class Repository merge_commit_sha end - def revert_merge(user, merge_commit_id, new_branch_name, commit_message) + def revert_merge(user, merge_commit_id, new_branch_name, target_branch, commit_message) # branch exists and it's highly probable that it has the revert commit return if find_branch(new_branch_name) - add_branch(user, new_branch_name, merge_commit_id) + target_sha = find_branch(target_branch).target - new_index = rugged.revert_commit(merge_commit_id, merge_commit_id, mainline: 1) - committer = user_to_committer(user) + commit_with_hooks(user, new_branch_name) do |ref| + new_index = rugged.revert_commit(merge_commit_id, target_sha, mainline: 1) + committer = user_to_committer(user) - options = { - message: commit_message, - author: committer, - committer: committer, - tree: new_index.write_tree(rugged), - parents: [rugged.lookup(merge_commit_id)], - update_ref: "refs/heads/#{new_branch_name}" - } + options = { + message: commit_message, + author: committer, + committer: committer, + tree: new_index.write_tree(rugged), + parents: [rugged.lookup(target_sha)], + update_ref: ref + } - Rugged::Commit.create(rugged, options) + Rugged::Commit.create(rugged, options) + end end def merged_to_root_ref?(branch_name) diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index e6be1a5a19b..5d29181631a 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -18,5 +18,5 @@ Edit - if @merge_request.closed? = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'btn btn-nr btn-grouped btn-reopen reopen-mr-link', title: "Reopen merge request" - - if @merge_request.merged? + - if @merge_request.merged? && @merge_request.merge_commit_sha.present? = link_to 'Revert', revert_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), method: :post, class: "btn btn-grouped btn-close", title: "Revert merge request" diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 420a6a5bed3..9fc9a873575 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -10,7 +10,6 @@ describe Repository, models: true do { message: 'Test message', committer: author, author: author } end let(:merge_commit_id) do - master_commit = repository.commit('master') source_sha = repository.find_branch('feature').target repository.merge(user, source_sha, 'master', commit_options) end