Add some specs plus some refactor.

app/controllers/projects/merge_requests_controller.rb

@@ -199,11 +199,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       target_branch: @merge_request.target_branch,
       source_project_id: @merge_request.target_project_id,
       target_project_id: @merge_request.target_project_id,
-      description: "Reverts #{@merge_request.to_reference}"
+      description: @merge_request.revert_description
     }}
 
     if target_branch_exists
-      @repository.revert_merge(current_user, @merge_request)
+      @repository.revert_merge(current_user, @merge_request.merge_commit_sha,
+                               @merge_request.revert_branch_name, @merge_request.revert_title)
+
       redirect_to new_namespace_project_merge_request_url(@project.namespace, @project, url_params)
     else
       redirect_to namespace_project_merge_request_url(@project.namespace, @project, @merge_request),

app/models/merge_request.rb

@@ -537,4 +537,12 @@ class MergeRequest < ActiveRecord::Base
   def revert_branch_name
     "revert-#{iid}-#{target_branch}"
   end
+
+  def revert_title
+   "Revert \"#{title}\""
+  end
+
+  def revert_description
+    "Reverts #{to_reference}"
+  end
 end

app/models/repository.rb

@@ -622,25 +622,22 @@ class Repository
     merge_commit_sha
   end
 
-  def revert_merge(user, merge_request)
-    revert_branch_name = merge_request.revert_branch_name
-    merge_commit_id = merge_request.merge_commit_sha
-
+  def revert_merge(user, merge_commit_id, new_branch_name, commit_message)
     # branch exists and it's highly probable that it has the revert commit
-    return if find_branch(revert_branch_name)
+    return if find_branch(new_branch_name)
 
-    add_branch(user, revert_branch_name, merge_commit_id)
+    add_branch(user, new_branch_name, merge_commit_id)
 
     new_index = rugged.revert_commit(merge_commit_id, merge_commit_id, mainline: 1)
     committer = user_to_committer(user)
 
     options = {
-      message: "Revert \"#{merge_request.title}\"",
+      message: commit_message,
       author: committer,
       committer: committer,
       tree: new_index.write_tree(rugged),
       parents: [rugged.lookup(merge_commit_id)],
-      update_ref: "refs/heads/#{revert_branch_name}"
+      update_ref: "refs/heads/#{new_branch_name}"
     }
 
     Rugged::Commit.create(rugged, options)

spec/models/repository_spec.rb

@@ -5,6 +5,15 @@ describe Repository, models: true do
 
   let(:repository) { create(:project).repository }
   let(:user) { create(:user) }
+  let(:commit_options) do
+    author = repository.user_to_committer(user)
+    { 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
 
   describe :branch_names_contains do
     subject { repository.branch_names_contains(sample_commit.id) }
@@ -426,4 +435,21 @@ describe Repository, models: true do
 
     it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') }
   end
+
+  describe '#merge' do
+    it 'should merge the code and return the commit id' do
+      expect(merge_commit_id).to be_present
+      expect(repository.blob_at(merge_commit_id, 'files/ruby/feature.rb')).to be_present
+    end
+  end
+
+  describe '#revert_merge' do
+    it 'should revert the changes' do
+      repository.revert_merge(user, merge_commit_id, 'revert-changes', 'Revert changes')
+      source_sha = repository.find_branch('revert-changes').target
+      repository.merge(user, source_sha, 'master', commit_options)
+
+      expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
+    end
+  end
 end