Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-02-19 21:39:31 +0300
committerRobert Speicher <robert@gitlab.com>2016-02-19 21:39:31 +0300
commita5c7aea3a95e14fd02ceea9b1ff6678b3ce37e68 (patch)
tree3d830cea95dbd23815f77d709f02815bb6bf130f
parent803d6f8f050b3cef1c8f01439385ac736977bddb (diff)
parentcc5ff3b5a4d63dd079e02029be36646261770f66 (diff)
Merge branch 'issue_3409' into 'master'
Add ability to revert changes introduced by Merge Requests or Commits Closes #3409 See merge request !1990
-rw-r--r--CHANGELOG1
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock10
-rw-r--r--app/assets/stylesheets/pages/merge_requests.scss2
-rw-r--r--app/controllers/application_controller.rb9
-rw-r--r--app/controllers/concerns/creates_commit.rb53
-rw-r--r--app/controllers/projects/commit_controller.rb44
-rw-r--r--app/helpers/commits_helper.rb31
-rw-r--r--app/helpers/tree_helper.rb3
-rw-r--r--app/models/commit.rb38
-rw-r--r--app/models/merge_request.rb9
-rw-r--r--app/models/repository.rb37
-rw-r--r--app/services/commits/revert_service.rb58
-rw-r--r--app/services/merge_requests/build_service.rb2
-rw-r--r--app/services/merge_requests/merge_service.rb3
-rw-r--r--app/views/projects/commit/_commit_box.html.haml2
-rw-r--r--app/views/projects/commit/_revert.html.haml31
-rw-r--r--app/views/projects/commit/show.html.haml2
-rw-r--r--app/views/projects/merge_requests/_show.html.haml2
-rw-r--r--app/views/projects/merge_requests/widget/_merged.html.haml14
-rw-r--r--app/views/projects/merge_requests/widget/_merged_buttons.haml11
-rw-r--r--config/routes.rb1
-rw-r--r--db/migrate/20160129155512_add_merge_commit_sha_to_merge_requests.rb5
-rw-r--r--db/schema.rb1
-rw-r--r--doc/workflow/README.md1
-rw-r--r--doc/workflow/img/revert_changes_commit.pngbin0 -> 360098 bytes
-rw-r--r--doc/workflow/img/revert_changes_commit_modal.pngbin0 -> 327932 bytes
-rw-r--r--doc/workflow/img/revert_changes_mr.pngbin0 -> 367881 bytes
-rw-r--r--doc/workflow/img/revert_changes_mr_modal.pngbin0 -> 335010 bytes
-rw-r--r--doc/workflow/revert_changes.md64
-rw-r--r--features/project/commits/revert.feature28
-rw-r--r--features/project/merge_requests/revert.feature30
-rw-r--r--features/steps/project/commits/revert.rb40
-rw-r--r--features/steps/project/merge_requests/revert.rb56
-rw-r--r--spec/controllers/commit_controller_spec.rb49
-rw-r--r--spec/factories/merge_requests.rb1
-rw-r--r--spec/models/merge_request_spec.rb1
-rw-r--r--spec/models/repository_spec.rb24
38 files changed, 629 insertions, 36 deletions
diff --git a/CHANGELOG b/CHANGELOG
index d7e7795dcb1..b4578592600 100644
--- a/CHANGELOG
+++ b/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
diff --git a/Gemfile b/Gemfile
index f7215010a1e..1602ef871c7 100644
--- a/Gemfile
+++ b/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
diff --git a/Gemfile.lock b/Gemfile.lock
index 10750805ca4..4681adf2bd0 100644
--- a/Gemfile.lock
+++ b/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)
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 6b497cd56ed..12fb030760c 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -237,3 +237,5 @@
}
}
}
+
+
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2c329b60a19..fb74919ea23 100644
--- a/app/controllers/application_controller.rb
+++ b/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
diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb
index b9eb0a22f88..787416c17ab 100644
--- a/app/controllers/concerns/creates_commit.rb
+++ b/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
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 36951b91372..97d31a4229a 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/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
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 1d14ee52cfc..7ff539118d3 100644
--- a/app/helpers/commits_helper.rb
+++ b/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
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index 2ad7c80dae0..4920ca5af6e 100644
--- a/app/helpers/tree_helper.rb
+++ b/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)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 23b771aebb7..b99abb540ea 100644
--- a/app/models/commit.rb
+++ b/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
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ea2b0e075f6..1543ef311d7 100644
--- a/app/models/merge_request.rb
+++ b/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
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 73aa67d46ec..be30a3b0906 100644
--- a/app/models/repository.rb
+++ b/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
diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb
new file mode 100644
index 00000000000..43d1c766e35
--- /dev/null
+++ b/app/services/commits/revert_service.rb
@@ -0,0 +1,58 @@
+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
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index a9b29f9654d..c0700d953dd 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/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
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index e8bef250d8b..9a58383b398 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/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)
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index bbe820b8842..71995fcc487 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/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
diff --git a/app/views/projects/commit/_revert.html.haml b/app/views/projects/commit/_revert.html.haml
new file mode 100644
index 00000000000..52ca3ed5b14
--- /dev/null
+++ b/app/views/projects/commit/_revert.html.haml
@@ -0,0 +1,31 @@
+#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'))
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index 05dbe5ebea4..21e186120c3 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/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
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index da67645bc2b..648512e5379 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/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;
diff --git a/app/views/projects/merge_requests/widget/_merged.html.haml b/app/views/projects/merge_requests/widget/_merged.html.haml
index d1d602eecdc..3abae9f0bf6 100644
--- a/app/views/projects/merge_requests/widget/_merged.html.haml
+++ b/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}'.
diff --git a/app/views/projects/merge_requests/widget/_merged_buttons.haml b/app/views/projects/merge_requests/widget/_merged_buttons.haml
new file mode 100644
index 00000000000..85a3a6ba9e2
--- /dev/null
+++ b/app/views/projects/merge_requests/widget/_merged_buttons.haml
@@ -0,0 +1,11 @@
+- 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')
diff --git a/config/routes.rb b/config/routes.rb
index c6541a8979c..0ed3f1731f8 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -502,6 +502,7 @@ Rails.application.routes.draw do
get :builds
post :cancel_builds
post :retry_builds
+ post :revert
end
end
diff --git a/db/migrate/20160129155512_add_merge_commit_sha_to_merge_requests.rb b/db/migrate/20160129155512_add_merge_commit_sha_to_merge_requests.rb
new file mode 100644
index 00000000000..f0d94226514
--- /dev/null
+++ b/db/migrate/20160129155512_add_merge_commit_sha_to_merge_requests.rb
@@ -0,0 +1,5 @@
+class AddMergeCommitShaToMergeRequests < ActiveRecord::Migration
+ def change
+ add_column :merge_requests, :merge_commit_sha, :string
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2d1c6dbf0e4..344c384b6d0 100644
--- a/db/schema.rb
+++ b/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
diff --git a/doc/workflow/README.md b/doc/workflow/README.md
index bf62ab41053..5199ff9390a 100644
--- a/doc/workflow/README.md
+++ b/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)
diff --git a/doc/workflow/img/revert_changes_commit.png b/doc/workflow/img/revert_changes_commit.png
new file mode 100644
index 00000000000..d84211e20db
--- /dev/null
+++ b/doc/workflow/img/revert_changes_commit.png
Binary files differ
diff --git a/doc/workflow/img/revert_changes_commit_modal.png b/doc/workflow/img/revert_changes_commit_modal.png
new file mode 100644
index 00000000000..e94d151a2af
--- /dev/null
+++ b/doc/workflow/img/revert_changes_commit_modal.png
Binary files differ
diff --git a/doc/workflow/img/revert_changes_mr.png b/doc/workflow/img/revert_changes_mr.png
new file mode 100644
index 00000000000..7adad88463b
--- /dev/null
+++ b/doc/workflow/img/revert_changes_mr.png
Binary files differ
diff --git a/doc/workflow/img/revert_changes_mr_modal.png b/doc/workflow/img/revert_changes_mr_modal.png
new file mode 100644
index 00000000000..9da78f84828
--- /dev/null
+++ b/doc/workflow/img/revert_changes_mr_modal.png
Binary files differ
diff --git a/doc/workflow/revert_changes.md b/doc/workflow/revert_changes.md
new file mode 100644
index 00000000000..399366b0cdc
--- /dev/null
+++ b/doc/workflow/revert_changes.md
@@ -0,0 +1,64 @@
+# 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"
diff --git a/features/project/commits/revert.feature b/features/project/commits/revert.feature
new file mode 100644
index 00000000000..7a2effafe03
--- /dev/null
+++ b/features/project/commits/revert.feature
@@ -0,0 +1,28 @@
+@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
diff --git a/features/project/merge_requests/revert.feature b/features/project/merge_requests/revert.feature
new file mode 100644
index 00000000000..d767b088883
--- /dev/null
+++ b/features/project/merge_requests/revert.feature
@@ -0,0 +1,30 @@
+@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
diff --git a/features/steps/project/commits/revert.rb b/features/steps/project/commits/revert.rb
new file mode 100644
index 00000000000..94a5d4e2e4d
--- /dev/null
+++ b/features/steps/project/commits/revert.rb
@@ -0,0 +1,40 @@
+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
diff --git a/features/steps/project/merge_requests/revert.rb b/features/steps/project/merge_requests/revert.rb
new file mode 100644
index 00000000000..c5a4cfce6f0
--- /dev/null
+++ b/features/steps/project/merge_requests/revert.rb
@@ -0,0 +1,56 @@
+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
diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb
index 7793bf1e421..bbe400dad88 100644
--- a/spec/controllers/commit_controller_spec.rb
+++ b/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
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 0c6a881f868..00de7bb5294 100644
--- a/spec/factories/merge_requests.rb
+++ b/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
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index f35b48601ad..c51f34034d7 100644
--- a/spec/models/merge_request_spec.rb
+++ b/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'
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index ed91b62c534..9242f755449 100644
--- a/spec/models/repository_spec.rb
+++ b/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