Merge branch 'issue_3409' into 'master'

Add ability to revert changes introduced by Merge Requests or Commits

Closes #3409 

See merge request !1990

CHANGELOG

@@ -70,6 +70,7 @@ v 8.5.0 (unreleased)
     all Omniauth providers to do so.
   - Allow existing users to auto link their SAML credentials by logging in via SAML
   - Make it possible to erase a build (trace, artifacts) using UI and API
+  - Ability to revert changes from a Merge Request or Commit
 
 v 8.4.4
   - Update omniauth-saml gem to 1.4.2

Gemfile

@@ -50,7 +50,7 @@ gem "browser", '~> 1.0.0'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", '~> 8.1'
+gem "gitlab_git", '~> 8.2'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes

Gemfile.lock

@@ -340,7 +340,7 @@ GEM
       json
     get_process_mem (0.2.0)
     gherkin-ruby (0.3.2)
-    github-linguist (4.7.3)
+    github-linguist (4.7.5)
       charlock_holmes (~> 0.7.3)
       escape_utils (~> 1.1.0)
       mime-types (>= 1.19)
@@ -357,11 +357,11 @@ GEM
       posix-spawn (~> 0.3)
     gitlab_emoji (0.3.1)
       gemojione (~> 2.2, >= 2.2.1)
-    gitlab_git (8.1.0)
+    gitlab_git (8.2.0)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
-      rugged (~> 0.23.3)
+      rugged (~> 0.24.0b13)
     gitlab_meta (7.0)
     gitlab_omniauth-ldap (1.2.1)
       net-ldap (~> 0.9)
@@ -700,7 +700,7 @@ GEM
     rubyntlm (0.5.2)
     rubypants (0.2.0)
     rufus-scheduler (3.1.10)
-    rugged (0.23.3)
+    rugged (0.24.0b13)
     safe_yaml (1.0.4)
     sanitize (2.1.0)
       nokogiri (>= 1.4.4)
@@ -932,7 +932,7 @@ DEPENDENCIES
   github-markup (~> 1.3.1)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab_emoji (~> 0.3.0)
-  gitlab_git (~> 8.1)
+  gitlab_git (~> 8.2)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.1.0)

app/assets/stylesheets/pages/merge_requests.scss

@@ -237,3 +237,5 @@
     }
   }
 }
+
+

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

@@ -13,17 +13,11 @@ module CreatesCommit
     result = service.new(@tree_edit_project, current_user, commit_params).execute
 
     if result[:status] == :success
-      flash[:notice] = success_notice || "Your changes have been successfully committed."
-
-      if create_merge_request?
-        success_path = new_merge_request_path
-        target = different_project? ? "project" : "branch"
-        flash[:notice] << " You can now submit a merge request to get this change into the original #{target}."
-      end
+      update_flash_notice(success_notice)
 
       respond_to do |format|
-        format.html { redirect_to success_path }
-        format.json { render json: { message: "success", filePath: success_path } }
+        format.html { redirect_to final_success_path(success_path) }
+        format.json { render json: { message: "success", filePath: final_success_path(success_path) } }
       end
     else
       flash[:alert] = result[:message]
@@ -41,14 +35,32 @@ module CreatesCommit
   end
 
   def authorize_edit_tree!
-    return if can?(current_user, :push_code, project)
-    return if current_user && current_user.already_forked?(project)
+    return if can_collaborate_with_project?
 
     access_denied!
   end
 
   private
 
+  def update_flash_notice(success_notice)
+    flash[:notice] = success_notice || "Your changes have been successfully committed."
+
+    if create_merge_request?
+      if merge_request_exists?
+        flash[:notice] = nil
+      else
+        target = different_project? ? "project" : "branch"
+        flash[:notice] << " You can now submit a merge request to get this change into the original #{target}."
+      end
+    end
+  end
+
+  def final_success_path(success_path)
+    return success_path unless create_merge_request?
+
+    merge_request_exists? ? existing_merge_request_path : new_merge_request_path
+  end
+
   def new_merge_request_path
     new_namespace_project_merge_request_path(
       @mr_source_project.namespace,
@@ -62,6 +74,19 @@ module CreatesCommit
     )
   end
 
+  def existing_merge_request_path
+    namespace_project_merge_request_path(@mr_target_project.namespace, @mr_target_project, @merge_request)
+  end
+
+  def merge_request_exists?
+    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
+    )
+  end
+
   def different_project?
     @mr_source_project != @mr_target_project
   end
@@ -75,7 +100,7 @@ module CreatesCommit
   end
 
   def set_commit_variables
-    @mr_source_branch = @target_branch
+    @mr_source_branch ||= @target_branch
 
     if can?(current_user, :push_code, @project)
       # Edit file in this project
@@ -89,7 +114,7 @@ module CreatesCommit
       else
         # Merge request to this project
         @mr_target_project = @project
-        @mr_target_branch = @ref
+        @mr_target_branch ||= @ref
       end
     else
       # Edit file in fork
@@ -97,7 +122,7 @@ module CreatesCommit
       # Merge request from fork to this project
       @mr_source_project = @tree_edit_project
       @mr_target_project = @project
-      @mr_target_branch = @ref
+      @mr_target_branch  ||= @ref
     end
   end
 end

app/controllers/projects/commit_controller.rb

@@ -2,6 +2,8 @@
 #
 # Not to be confused with CommitsController, plural.
 class Projects::CommitController < Projects::ApplicationController
+  include CreatesCommit
+
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds]
@@ -9,6 +11,7 @@ class Projects::CommitController < Projects::ApplicationController
   before_action :authorize_read_commit_status!, only: [:builds]
   before_action :commit
   before_action :define_show_vars, only: [:show, :builds]
+  before_action :authorize_edit_tree!, only: [:revert]
 
   def show
     apply_diff_view_cookie!
@@ -55,8 +58,37 @@ class Projects::CommitController < Projects::ApplicationController
     render layout: false
   end
 
+  def revert
+    assign_revert_commit_vars
+
+    return render_404 if @target_branch.blank?
+
+    create_commit(Commits::RevertService, success_notice: "The #{revert_type_title} has been successfully reverted.",
+                                          success_path: successful_revert_path, failure_path: failed_revert_path)
+  end
+
   private
 
+  def revert_type_title
+    @commit.merged_merge_request ? 'merge request' : 'commit'
+  end
+
+  def successful_revert_path
+    return referenced_merge_request_url if @commit.merged_merge_request
+
+    namespace_project_commits_url(@project.namespace, @project, @target_branch)
+  end
+
+  def failed_revert_path
+    return referenced_merge_request_url if @commit.merged_merge_request
+
+    namespace_project_commit_url(@project.namespace, @project, params[:id])
+  end
+
+  def referenced_merge_request_url
+    namespace_project_merge_request_url(@project.namespace, @project, @commit.merged_merge_request)
+  end
+
   def commit
     @commit ||= @project.commit(params[:id])
   end
@@ -79,4 +111,16 @@ class Projects::CommitController < Projects::ApplicationController
 
     @statuses = ci_commit.statuses if ci_commit
   end
+
+  def assign_revert_commit_vars
+    @commit = project.commit(params[:id])
+    @target_branch = params[:target_branch]
+    @mr_source_branch = @commit.revert_branch_name
+    @mr_target_branch = @target_branch
+    @commit_params = {
+      commit: @commit,
+      revert_type_title: revert_type_title,
+      create_merge_request: params[:create_merge_request].present? || different_project?
+    }
+  end
 end

app/helpers/commits_helper.rb

@@ -123,6 +123,37 @@ module CommitsHelper
     )
   end
 
+  def revert_commit_link(commit, continue_to_path, btn_class: nil)
+    return unless current_user
+
+    tooltip = "Revert this #{revert_commit_type(commit)} in a new merge request"
+
+    if can_collaborate_with_project?
+      content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do
+        link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class}"
+      end
+    elsif can?(current_user, :fork_project, @project)
+      continue_params = {
+        to: continue_to_path,
+        notice: edit_in_new_fork_notice + ' Try to revert this commit again.',
+        notice_now: edit_in_new_fork_notice_now
+      }
+      fork_path = namespace_project_forks_path(@project.namespace, @project,
+        namespace_key: current_user.namespace.id,
+        continue: continue_params)
+
+      link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', title: tooltip
+    end
+  end
+
+  def revert_commit_type(commit)
+    if commit.merged_merge_request
+      'merge request'
+    else
+      'commit'
+    end
+  end
+
   protected
 
   # Private: Returns a link to a person. If the person has a matching user and

app/helpers/tree_helper.rb

@@ -56,8 +56,7 @@ module TreeHelper
 
     return false unless on_top_of_branch?(project, ref)
 
-    can?(current_user, :push_code, project) ||
-      (current_user && current_user.already_forked?(project))
+    can_collaborate_with_project?(project)
   end
 
   def tree_edit_branch(project = @project, ref = @ref)

app/models/commit.rb

@@ -215,6 +215,44 @@ class Commit
     ci_commit.try(:status) || :not_found
   end
 
+  def revert_branch_name
+    "revert-#{short_id}"
+  end
+
+  def revert_description
+    if merged_merge_request
+      "This reverts merge request #{merged_merge_request.to_reference}"
+    else
+      "This reverts commit #{sha}"
+    end
+  end
+
+  def revert_message
+    %Q{Revert "#{title}"\n\n#{revert_description}}
+  end
+
+  def reverts_commit?(commit)
+    description.include?(commit.revert_description)
+  end
+
+  def merge_commit?
+    parents.size > 1
+  end
+
+  def merged_merge_request
+    return @merged_merge_request if defined?(@merged_merge_request)
+
+    @merged_merge_request = project.merge_requests.find_by(merge_commit_sha: id) if merge_commit?
+  end
+
+  def has_been_reverted?(current_user = nil, noteable = self)
+    Gitlab::ReferenceExtractor.lazily do
+      noteable.notes.system.flat_map do |note|
+        note.all_references(current_user).commits
+      end
+    end.any? { |commit_ref| commit_ref.reverts_commit?(self) }
+  end
+
   private
 
   def repo_changes

app/models/merge_request.rb

@@ -24,6 +24,7 @@
 #  merge_params              :text
 #  merge_when_build_succeeds :boolean          default(FALSE), not null
 #  merge_user_id             :integer
+#  merge_commit_sha          :string
 #
 
 require Rails.root.join("app/models/commit")
@@ -532,4 +533,12 @@ class MergeRequest < ActiveRecord::Base
 
     [diff_base_commit, last_commit]
   end
+
+  def merge_commit
+    @merge_commit ||= project.commit(merge_commit_sha) if merge_commit_sha
+  end
+
+  def can_be_reverted?(current_user = nil)
+    merge_commit && !merge_commit.has_been_reverted?(current_user, self)
+  end
 end

app/models/repository.rb

@@ -619,6 +619,34 @@ class Repository
     end
   end
 
+  def revert(user, commit, base_branch, target_branch = nil)
+    source_sha    = find_branch(base_branch).target
+    target_branch ||= base_branch
+    args          = [commit.id, source_sha]
+    args          << { mainline: 1 } if commit.merge_commit?
+
+    revert_index = rugged.revert_commit(*args)
+    return false if revert_index.conflicts?
+
+    tree_id = revert_index.write_tree(rugged)
+    return false unless diff_exists?(source_sha, tree_id)
+
+    commit_with_hooks(user, target_branch) do |ref|
+      committer = user_to_committer(user)
+      source_sha = Rugged::Commit.create(rugged,
+        message: commit.revert_message,
+        author: committer,
+        committer: committer,
+        tree: tree_id,
+        parents: [rugged.lookup(source_sha)],
+        update_ref: ref)
+    end
+  end
+
+  def diff_exists?(sha1, sha2)
+    rugged.diff(sha1, sha2).size > 0
+  end
+
   def merged_to_root_ref?(branch_name)
     branch_commit = commit(branch_name)
     root_ref_commit = commit(root_ref)
@@ -700,10 +728,11 @@ class Repository
 
     oldrev = Gitlab::Git::BLANK_SHA
     ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
+    target_branch = find_branch(branch)
     was_empty = empty?
 
-    unless was_empty
-      oldrev = find_branch(branch).target
+    if !was_empty && target_branch
+      oldrev = target_branch.target
     end
 
     with_tmp_ref(oldrev) do |tmp_ref|
@@ -715,7 +744,7 @@ class Repository
       end
 
       GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
-        if was_empty
+        if was_empty || !target_branch
           # Create branch
           rugged.references.create(ref, newrev)
         else
@@ -730,6 +759,8 @@ class Repository
           end
         end
       end
+
+      newrev
     end
   end
 

app/services/commits/revert_service.rb

+module Commits
+  class RevertService < ::BaseService
+    class ValidationError < StandardError; end
+    class ReversionError < StandardError; end
+
+    def execute
+      @source_project = params[:source_project] || @project
+      @target_branch = params[:target_branch]
+      @commit = params[:commit]
+      @create_merge_request = params[:create_merge_request].present?
+
+      validate and commit
+    rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
+           ValidationError, ReversionError => ex
+      error(ex.message)
+    end
+
+    def commit
+      revert_into = @create_merge_request ? @commit.revert_branch_name : @target_branch
+
+      if @create_merge_request
+        # Temporary branch exists and contains the revert commit
+        return success if repository.find_branch(revert_into)
+
+        create_target_branch
+      end
+
+      unless repository.revert(current_user, @commit, revert_into)
+        error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically.
+                     It may have already been reverted, or a more recent commit may have updated some of its content."
+        raise ReversionError, error_msg
+      end
+
+      success
+    end
+
+    private
+
+    def create_target_branch
+      result = CreateBranchService.new(@project, current_user)
+                                  .execute(@commit.revert_branch_name, @target_branch, source_project: @source_project)
+
+      if result[:status] == :error
+        raise ReversionError, "There was an error creating the source branch: #{result[:message]}"
+      end
+    end
+
+    def validate
+      allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch)
+
+      unless allowed
+        raise_error('You are not allowed to push into this branch')
+      end
+
+      true
+    end
+  end
+end

app/services/merge_requests/build_service.rb

@@ -56,7 +56,7 @@ module MergeRequests
       if commits && commits.count == 1
         commit = commits.first
         merge_request.title       = commit.title
-        merge_request.description = commit.description.try(:strip)
+        merge_request.description ||= commit.description.try(:strip)
       else
         merge_request.title = merge_request.source_branch.titleize.humanize
       end

app/services/merge_requests/merge_service.rb

@@ -34,7 +34,8 @@ module MergeRequests
         committer: committer
       }
 
-      repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
+      commit_id = repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
+      merge_request.update(merge_commit_sha: commit_id)
     rescue StandardError => e
       merge_request.update(merge_error: "Something went wrong during merge")
       Rails.logger.error(e.message)

app/views/projects/commit/_commit_box.html.haml

@@ -16,6 +16,8 @@
     = link_to namespace_project_tree_path(@project.namespace, @project, @commit), class: "btn btn-grouped" do
       = icon('files-o')
       Browse Files
+    - unless @commit.has_been_reverted?(current_user)
+      = revert_commit_link(@commit, namespace_project_commit_path(@project.namespace, @project, @commit.id))
   %div
 
 %p

app/views/projects/commit/_revert.html.haml

+#modal-revert-commit.modal
+  .modal-dialog
+    .modal-content
+      .modal-header
+        %a.close{href: "#", "data-dismiss" => "modal"} ×
+        %h3.page-title== Revert this #{revert_commit_type(commit)}
+      .modal-body
+        = form_tag revert_namespace_project_commit_path(@project.namespace, @project, commit.id), method: :post, remote: false, class: 'form-horizontal js-create-dir-form js-requires-input' do
+          .form-group.branch
+            = label_tag 'target_branch', 'Revert in branch', class: 'control-label'
+            .col-sm-10
+              = select_tag "target_branch", grouped_options_refs, class: "select2 select2-sm js-target-branch"
+              - if can?(current_user, :push_code, @project)
+                .js-create-merge-request-container
+                  .checkbox
+                    - nonce = SecureRandom.hex
+                    = label_tag "create_merge_request-#{nonce}" do
+                      = check_box_tag 'create_merge_request', 1, true, class: 'js-create-merge-request', id: "create_merge_request-#{nonce}"
+                      Start a <strong>new merge request</strong> with these changes
+              - else
+                = hidden_field_tag 'create_merge_request', 1
+          .form-actions
+            = submit_tag "Revert", class: 'btn btn-create'
+            = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal"
+
+            - unless can?(current_user, :push_code, @project)
+              .inline.prepend-left-10
+                = commit_in_fork_help
+
+:javascript
+  new NewCommitForm($('.js-create-dir-form'))

app/views/projects/commit/show.html.haml

@@ -12,3 +12,5 @@
 = render "projects/diffs/diffs", diffs: @diffs, project: @project,
          diff_refs: @diff_refs
 = render "projects/notes/notes_with_form"
+- if can_collaborate_with_project?
+  = render "projects/commit/revert", commit: @commit, title: @commit.title

app/views/projects/merge_requests/_show.html.haml

@@ -85,6 +85,8 @@
         = spinner
 
 = render 'shared/issuable/sidebar', issuable: @merge_request
+- if @merge_request.can_be_reverted?
+  = render "projects/commit/revert", commit: @merge_request.merge_commit, title: @merge_request.title
 
 :javascript
   var merge_request;

app/views/projects/merge_requests/widget/_merged.html.haml

@@ -8,20 +8,18 @@
         #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}
     %div
       - if !@merge_request.source_branch_exists? || (params[:delete_source] == 'true')
-        The changes were merged into
-        #{link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch"}.
-        The source branch has been removed.
-
+        %p
+          The changes were merged into
+          #{link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch"}.
+          The source branch has been removed.
+        = render 'projects/merge_requests/widget/merged_buttons'
       - elsif @merge_request.can_remove_source_branch?(current_user)
         .remove_source_branch_widget
           %p
             The changes were merged into
             #{link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch"}.
             You can remove the source branch now.
-          = link_to namespace_project_branch_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request.source_branch), remote: true, method: :delete, class: "btn btn-primary btn-sm remove_source_branch" do
-            %i.fa.fa-times
-            Remove Source Branch
-
+          = render 'projects/merge_requests/widget/merged_buttons', source_branch_exists: true
         .remove_source_branch_widget.failed.hide
           %p
             Failed to remove source branch '#{@merge_request.source_branch}'.

app/views/projects/merge_requests/widget/_merged_buttons.haml

+- source_branch_exists = local_assigns.fetch(:source_branch_exists, false)
+- mr_can_be_reverted = @merge_request.can_be_reverted?
+
+- if source_branch_exists || mr_can_be_reverted
+  .btn-group
+    - if source_branch_exists
+      = link_to namespace_project_branch_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request.source_branch), remote: true, method: :delete, class: "btn btn-default btn-grouped btn-sm remove_source_branch" do
+        = icon('trash-o')
+        Remove Source Branch
+    - if mr_can_be_reverted
+      = revert_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm')

config/routes.rb

@@ -502,6 +502,7 @@ Rails.application.routes.draw do
             get :builds
             post :cancel_builds
             post :retry_builds
+            post :revert
           end
         end
 

db/migrate/20160129155512_add_merge_commit_sha_to_merge_requests.rb

+class AddMergeCommitShaToMergeRequests < ActiveRecord::Migration
+  def change
+    add_column :merge_requests, :merge_commit_sha, :string
+  end
+end

db/schema.rb

@@ -525,6 +525,7 @@ ActiveRecord::Schema.define(version: 20160209130428) do
     t.text     "merge_params"
     t.boolean  "merge_when_build_succeeds", default: false, null: false
     t.integer  "merge_user_id"
+    t.string   "merge_commit_sha"
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

doc/workflow/README.md

@@ -17,6 +17,7 @@
 - [Releases](releases.md)
 - [Milestones](milestones.md)
 - [Merge Requests](merge_requests.md)
+- [Revert changes](revert_changes.md)
 - ["Work In Progress" Merge Requests](wip_merge_requests.md)
 - [Merge When Build Succeeds](merge_when_build_succeeds.md)
 - [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md)

doc/workflow/revert_changes.md

+# Reverting changes
+
+_**Note:** This feature was [introduced][ce-1990] in GitLab 8.5._
+
+---
+
+GitLab implements Git's powerful feature to [revert any commit][git-revert]
+with introducing a **Revert** button in Merge Requests and commit details.
+
+## Reverting a Merge Request
+
+_**Note:** The **Revert** button will only be available for Merge Requests
+created since GitLab 8.5. However, you can still revert a Merge Request
+by reverting the merge commit from the list of Commits page._
+
+After the Merge Request has been merged, a **Revert** button will be available
+to revert the changes introduced by that Merge Request:
+
+![Revert Merge Request](img/revert_changes_mr.png)
+
+---
+
+You can revert the changes directly into the selected branch or you can opt to
+create a new Merge Request with the revert changes:
+
+![Revert Merge Request modal](img/revert_changes_mr_modal.png)
+
+---
+
+After the Merge Request has been reverted, the **Revert** button will not be
+available anymore.
+
+## Reverting a Commit
+
+You can revert a Commit from the Commit details page:
+
+![Revert commit](img/revert_changes_commit.png)
+
+---
+
+Similar to reverting a Merge Request, you can opt to revert the changes
+directly into the target branch or create a new Merge Request to revert the
+changes:
+
+![Revert commit modal](img/revert_changes_commit_modal.png)
+
+---
+
+After the Commit has been reverted, the **Revert** button will not be available
+anymore.
+
+Please note that when reverting merge commits, the mainline will always be the
+first parent. If you want to use a different mainline then you need to do that
+from the command line.
+
+Here is a quick example to revert a merge commit using the second parent as the
+mainline:
+
+```bash
+git revert -m 2 7a39eb0
+```
+
+[ce-1990]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1990 "Revert button Merge Request"
+[git-revert]: https://git-scm.com/docs/git-revert "Git revert documentation"

features/project/commits/revert.feature

+@project_commits
+Feature: Revert Commits
+  Background:
+    Given I sign in as a user
+    And I own a project
+    And I visit my project's commits page
+
+  Scenario: I revert a commit
+    Given I click on commit link
+    And I click on the revert button
+    And I revert the changes directly
+    Then I should see the revert commit notice
+
+  Scenario: I revert a commit that was previously reverted
+    Given I click on commit link
+    And I click on the revert button
+    And I revert the changes directly
+    And I visit my project's commits page
+    And I click on commit link
+    And I click on the revert button
+    And I revert the changes directly
+    Then I should see a revert error
+
+  Scenario: I revert a commit in a new merge request
+    Given I click on commit link
+    And I click on the revert button
+    And I revert the changes in a new merge request
+    Then I should see the new merge request notice

features/project/merge_requests/revert.feature

+@project_merge_requests
+Feature: Revert Merge Requests
+  Background:
+    Given There is an open Merge Request
+      And I am signed in as a developer of the project
+      And I am on the Merge Request detail page
+      And I click on Accept Merge Request
+
+  @javascript
+  Scenario: I revert a merge request
+    Given I click on the revert button
+    And I revert the changes directly
+    Then I should see the revert merge request notice
+
+  @javascript
+  Scenario: I revert a merge request that was previously reverted
+    Given I click on the revert button
+    And I revert the changes directly
+    And I am on the Merge Request detail page
+    And I click on the revert button
+    And I revert the changes directly
+    Then I should see a revert error
+
+  @javascript
+  Scenario: I revert a merge request in a new merge request
+    Given I click on the revert button
+    And I am on the Merge Request detail page
+    And I click on the revert button
+    And I revert the changes in a new merge request
+    Then I should see the new merge request notice

features/steps/project/commits/revert.rb

+class Spinach::Features::RevertCommits < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedProject
+  include SharedPaths
+  include SharedDiffNote
+  include RepoHelpers
+
+  step 'I click on commit link' do
+    visit namespace_project_commit_path(@project.namespace, @project, sample_commit.id)
+  end
+
+  step 'I click on the revert button' do
+    find("a[href='#modal-revert-commit']").click
+  end
+
+  step 'I revert the changes directly' do
+    page.within('#modal-revert-commit') do
+      uncheck 'create_merge_request'
+      click_button 'Revert'
+    end
+  end
+
+  step 'I should see the revert commit notice' do
+    page.should have_content('The commit has been successfully reverted.')
+  end
+
+  step 'I should see a revert error' do
+    page.should have_content('Sorry, we cannot revert this commit automatically.')
+  end
+
+  step 'I revert the changes in a new merge request' do
+    page.within('#modal-revert-commit') do
+      click_button 'Revert'
+    end
+  end
+
+  step 'I should see the new merge request notice' do
+    page.should have_content('The commit has been successfully reverted. You can now submit a merge request to get this change into the original branch.')
+  end
+end

features/steps/project/merge_requests/revert.rb

+class Spinach::Features::RevertMergeRequests < Spinach::FeatureSteps
+  include LoginHelpers
+  include GitlabRoutingHelper
+
+  step 'I click on the revert button' do
+    find("a[href='#modal-revert-commit']").click
+  end
+
+  step 'I revert the changes directly' do
+    page.within('#modal-revert-commit') do
+      uncheck 'create_merge_request'
+      click_button 'Revert'
+    end
+  end
+
+  step 'I should see the revert merge request notice' do
+    page.should have_content('The merge request has been successfully reverted.')
+  end
+
+  step 'I should not see the revert button' do
+    expect(page).not_to have_selector(:xpath, "a[href='#modal-revert-commit']")
+  end
+
+  step 'I am on the Merge Request detail page' do
+    visit merge_request_path(@merge_request)
+  end
+
+  step 'I click on Accept Merge Request' do
+    click_button('Accept Merge Request')
+  end
+
+  step 'I am signed in as a developer of the project' do
+    login_as(@user)
+  end
+
+  step 'There is an open Merge Request' do
+    @user = create(:user)
+    @project = create(:project, :public)
+    @project_member = create(:project_member, user: @user, project: @project, access_level: ProjectMember::DEVELOPER)
+    @merge_request = create(:merge_request, :with_diffs, :simple, source_project: @project)
+  end
+
+  step 'I should see a revert error' do
+    page.should have_content('Sorry, we cannot revert this merge request automatically.')
+  end
+
+  step 'I revert the changes in a new merge request' do
+    page.within('#modal-revert-commit') do
+      click_button 'Revert'
+    end
+  end
+
+  step 'I should see the new merge request notice' do
+    page.should have_content('The merge request has been successfully reverted. You can now submit a merge request to get this change into the original branch.')
+  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/factories/merge_requests.rb

@@ -24,6 +24,7 @@
 #  merge_params              :text
 #  merge_when_build_succeeds :boolean          default(FALSE), not null
 #  merge_user_id             :integer
+#  merge_commit_sha          :string
 #
 
 FactoryGirl.define do

spec/models/merge_request_spec.rb

@@ -24,6 +24,7 @@
 #  merge_params              :text
 #  merge_when_build_succeeds :boolean          default(FALSE), not null
 #  merge_user_id             :integer
+#  merge_commit_sha          :string
 #
 
 require 'spec_helper'

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) do
+    source_sha = repository.find_branch('feature').target
+    merge_commit_id = repository.merge(user, source_sha, 'master', commit_options)
+    repository.commit(merge_commit_id)
+  end
 
   describe :branch_names_contains do
     subject { repository.branch_names_contains(sample_commit.id) }
@@ -426,4 +435,19 @@ 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).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(user, merge_commit, 'master')
+
+      expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
+    end
+  end
 end