Fix build and add specs

app/controllers/application_controller.rb

@@ -25,7 +25,7 @@ class ApplicationController < ActionController::Base
 
   helper_method :abilities, :can?, :current_application_settings
   helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :gitorious_import_enabled?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled?
-  helper_method :repository
+  helper_method :repository, :can_collaborate_with_project?
 
   rescue_from Encoding::CompatibilityError do |exception|
     log_exception(exception)
@@ -410,6 +410,13 @@ class ApplicationController < ActionController::Base
     current_user.nil? && root_path == request.path
   end
 
+  def can_collaborate_with_project?(project = nil)
+    project ||= @project
+
+    can?(current_user, :push_code, project) ||
+      (current_user && current_user.already_forked?(project))
+  end
+
   private
 
   def set_default_sort

app/controllers/concerns/creates_commit.rb

@@ -82,7 +82,9 @@ module CreatesCommit
     return @merge_request if defined?(@merge_request)
 
     @merge_request = @mr_target_project.merge_requests.opened.find_by(
-                       source_branch: @mr_source_branch, target_branch: @mr_target_branch)
+      source_branch: @mr_source_branch,
+      target_branch: @mr_target_branch
+    )
   end
 
   def different_project?

app/controllers/projects/application_controller.rb

@@ -3,7 +3,6 @@ class Projects::ApplicationController < ApplicationController
   before_action :repository
   layout 'project'
 
-  helper_method :can_collaborate_with_project?
   def authenticate_user!
     # Restrict access to Projects area only
     # for non-signed users
@@ -37,11 +36,4 @@ class Projects::ApplicationController < ApplicationController
   def builds_enabled
     return render_404 unless @project.builds_enabled?
   end
-
-  def can_collaborate_with_project?(project = nil)
-    project ||= @project
-
-    can?(current_user, :push_code, project) ||
-      (current_user && current_user.already_forked?(project))
-  end
 end

app/helpers/commits_helper.rb

@@ -128,8 +128,7 @@ module CommitsHelper
 
     if show_modal_condition
       content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do
-        link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip',
-                'data-original-title' => 'Create merge request to revert commit', class: "btn btn-close btn-#{btn_class}"
+        link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', 'data-original-title' => 'Create merge request to revert commit', class: "btn btn-close btn-#{btn_class}"
       end
     else
       continue_params = {
@@ -139,11 +138,9 @@ module CommitsHelper
       }
       fork_path = namespace_project_forks_path(@project.namespace, @project,
         namespace_key: current_user.namespace.id,
-                    continue: continue_params
-                  )
+        continue: continue_params)
 
-      link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post,
-              'data-toggle' => 'tooltip', 'data-original-title' => 'Create merge request to revert commit'
+      link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', 'data-original-title' => 'Create merge request to revert commit'
     end
   end
 

app/models/repository.rb

@@ -642,8 +642,7 @@ class Repository
         committer: committer,
         tree: revert_index.write_tree(rugged),
         parents: [rugged.lookup(source_sha)],
-        update_ref: ref
-      )
+        update_ref: ref)
     end
   end
 

spec/controllers/commit_controller_spec.rb

@@ -143,4 +143,53 @@ describe Projects::CommitController do
       expect(assigns(:tags)).to include("v1.1.0")
     end
   end
+
+  describe '#revert' do
+    context 'when target branch is not provided' do
+      it 'should render the 404 page' do
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            id: commit.id)
+
+        expect(response).not_to be_success
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'when the revert was successful' do
+      it 'should redirect to the commits page' do
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            target_branch: 'master',
+            id: commit.id)
+
+        expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master')
+        expect(flash[:notice]).to eq('The commit has been successfully reverted.')
+      end
+    end
+
+    context 'when the revert failed' do
+      before do
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            target_branch: 'master',
+            id: commit.id)
+      end
+
+      it 'should redirect to the commit page' do
+        # Reverting a commit that has been already reverted.
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            target_branch: 'master',
+            id: commit.id)
+
+        expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, commit.id)
+        expect(flash[:alert]).to match('Sorry, we cannot revert this commit automatically.')
+      end
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -9,9 +9,10 @@ describe Repository, models: true do
     author = repository.user_to_committer(user)
     { message: 'Test message', committer: author, author: author }
   end
-  let(:merge_commit_id) do
+  let(:merge_commit) do
     source_sha = repository.find_branch('feature').target
-    repository.merge(user, source_sha, 'master', commit_options)
+    merge_commit_id = repository.merge(user, source_sha, 'master', commit_options)
+    repository.commit(merge_commit_id)
   end
 
   describe :branch_names_contains do
@@ -437,16 +438,14 @@ describe Repository, models: true do
 
   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
+      expect(merge_commit).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)
+      repository.revert(user, merge_commit, 'master')
 
       expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
     end