Commit 169070ba authored by Rubén Dávila's avatar Rubén Dávila Committed by Robert Speicher

Some updates from last code review.

parent 69086634
...@@ -238,10 +238,4 @@ ...@@ -238,10 +238,4 @@
} }
} }
// Revert Merge Request modal
#modal-revert-commit {
.js-create-merge-request-container {
line-height: $line-height-base;
}
}
...@@ -130,7 +130,7 @@ module CommitsHelper ...@@ -130,7 +130,7 @@ module CommitsHelper
if can_collaborate_with_project? if can_collaborate_with_project?
content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do
link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', 'data-original-title' => tooltip, class: "btn btn-close btn-#{btn_class}" link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class}"
end end
elsif can?(current_user, :fork_project, @project) elsif can?(current_user, :fork_project, @project)
continue_params = { continue_params = {
...@@ -142,7 +142,7 @@ module CommitsHelper ...@@ -142,7 +142,7 @@ module CommitsHelper
namespace_key: current_user.namespace.id, 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' => tooltip link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', title: tooltip
end end
end end
......
...@@ -228,7 +228,7 @@ class Commit ...@@ -228,7 +228,7 @@ class Commit
end end
def revert_message def revert_message
"Revert \"#{title}\"" + "\n\n#{revert_description}" %Q{Revert "#{title}"\n\n#{revert_description}}
end end
def reverts_commit?(commit) def reverts_commit?(commit)
...@@ -242,7 +242,7 @@ class Commit ...@@ -242,7 +242,7 @@ class Commit
def merged_merge_request def merged_merge_request
return @merged_merge_request if defined?(@merged_merge_request) return @merged_merge_request if defined?(@merged_merge_request)
@merged_merge_request = merge_commit? && MergeRequest.find_by(merge_commit_sha: id) @merged_merge_request = merge_commit? && project.merge_requests.find_by(merge_commit_sha: id)
end end
def has_been_reverted?(current_user = nil, noteable = self) def has_been_reverted?(current_user = nil, noteable = self)
......
...@@ -626,9 +626,9 @@ class Repository ...@@ -626,9 +626,9 @@ class Repository
args << { mainline: 1 } if commit.merge_commit? args << { mainline: 1 } if commit.merge_commit?
revert_index = rugged.revert_commit(*args) revert_index = rugged.revert_commit(*args)
tree_id = revert_index.write_tree(rugged)
return false if revert_index.conflicts? return false if revert_index.conflicts?
tree_id = revert_index.write_tree(rugged)
return false unless diff_exists?(source_sha, tree_id) return false unless diff_exists?(source_sha, tree_id)
commit_with_hooks(user, target_branch) do |ref| commit_with_hooks(user, target_branch) do |ref|
......
...@@ -28,7 +28,7 @@ module Commits ...@@ -28,7 +28,7 @@ module Commits
unless repository.revert(current_user, @commit, revert_into) unless repository.revert(current_user, @commit, revert_into)
error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically. 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." It may have already been reverted, or a more recent commit may have updated some of its content."
raise_error(ReversionError, error_msg) raise ReversionError, error_msg
end end
success success
...@@ -41,19 +41,15 @@ module Commits ...@@ -41,19 +41,15 @@ module Commits
.execute(@commit.revert_branch_name, @target_branch, source_project: @source_project) .execute(@commit.revert_branch_name, @target_branch, source_project: @source_project)
if result[:status] == :error if result[:status] == :error
raise_error(ReversionError, "There was an error creating the source branch: #{result[:message]}") raise ReversionError, "There was an error creating the source branch: #{result[:message]}"
end end
end end
def raise_error(klass, message)
raise klass.new(message)
end
def validate def validate
allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch) allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch)
unless allowed unless allowed
raise_error(ValidationError, "You are not allowed to push into this branch") raise_error('You are not allowed to push into this branch')
end end
true true
......
...@@ -3,11 +3,11 @@ ...@@ -3,11 +3,11 @@
.modal-content .modal-content
.modal-header .modal-header
%a.close{href: "#", "data-dismiss" => "modal"} × %a.close{href: "#", "data-dismiss" => "modal"} ×
%h3.page-title== Revert: "#{title}" %h3.page-title== Revert this #{revert_commit_type(commit)}
.modal-body .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_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 .form-group.branch
= label_tag 'target_branch', 'Revert in branch:', class: 'control-label' = label_tag 'target_branch', 'Revert in branch', class: 'control-label'
.col-sm-10 .col-sm-10
= select_tag "target_branch", grouped_options_refs, class: "select2 select2-sm js-target-branch" = select_tag "target_branch", grouped_options_refs, class: "select2 select2-sm js-target-branch"
- if can?(current_user, :push_code, @project) - if can?(current_user, :push_code, @project)
......
...@@ -13,4 +13,4 @@ ...@@ -13,4 +13,4 @@
diff_refs: @diff_refs diff_refs: @diff_refs
= render "projects/notes/notes_with_form" = render "projects/notes/notes_with_form"
- if can_collaborate_with_project? - if can_collaborate_with_project?
= render "projects/commit/revert", commit_id: @commit.id, title: @commit.title = render "projects/commit/revert", commit: @commit, title: @commit.title
...@@ -86,7 +86,7 @@ ...@@ -86,7 +86,7 @@
= render 'shared/issuable/sidebar', issuable: @merge_request = render 'shared/issuable/sidebar', issuable: @merge_request
- if @merge_request.merge_commit && !@merge_request.has_been_reverted?(current_user) - if @merge_request.merge_commit && !@merge_request.has_been_reverted?(current_user)
= render "projects/commit/revert", commit_id: @merge_request.merge_commit.id, title: @merge_request.title = render "projects/commit/revert", commit: @merge_request.merge_commit, title: @merge_request.title
:javascript :javascript
var merge_request; var merge_request;
......
...@@ -6,24 +6,29 @@ ...@@ -6,24 +6,29 @@
- if @merge_request.merge_event - if @merge_request.merge_event
by #{link_to_member(@project, @merge_request.merge_event.author, avatar: true)} by #{link_to_member(@project, @merge_request.merge_event.author, avatar: true)}
#{time_ago_with_tooltip(@merge_request.merge_event.created_at)} #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}
%div
- revert_button = capture_haml do
- if @merge_request.merge_commit && !@merge_request.has_been_reverted?(current_user) - if @merge_request.merge_commit && !@merge_request.has_been_reverted?(current_user)
= revert_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm') = revert_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm')
%div
- if !@merge_request.source_branch_exists? || (params[:delete_source] == 'true') - if !@merge_request.source_branch_exists? || (params[:delete_source] == 'true')
%p
The changes were merged into 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"}. #{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. The source branch has been removed.
- if revert_button.present?
.btn-group= revert_button
- elsif @merge_request.can_remove_source_branch?(current_user) - elsif @merge_request.can_remove_source_branch?(current_user)
.remove_source_branch_widget .remove_source_branch_widget
%p %p
The changes were merged into 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"}. #{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. 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 .btn-group
%i.fa.fa-times = 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
%i.fa.fa-trash-o
Remove Source Branch Remove Source Branch
= revert_button
.remove_source_branch_widget.failed.hide .remove_source_branch_widget.failed.hide
%p %p
Failed to remove source branch '#{@merge_request.source_branch}'. Failed to remove source branch '#{@merge_request.source_branch}'.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment