More updates from the last code review.

7 files changed, 20 insertions, 19 deletions

diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index d87a6fffc9c..97d31a4229a 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -11,7 +11,6 @@ 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 :assign_revert_commit_vars, only: [:revert]
   before_action :authorize_edit_tree!, only: [:revert]
 
   def show
@@ -60,6 +59,8 @@ class Projects::CommitController < Projects::ApplicationController
   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.",
diff --git a/app/models/commit.rb b/app/models/commit.rb
index bb997663cf4..b99abb540ea 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -242,7 +242,7 @@ class Commit
   def merged_merge_request
     return @merged_merge_request if defined?(@merged_merge_request)
 
-    @merged_merge_request = merge_commit? && project.merge_requests.find_by(merge_commit_sha: id)
+    @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)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 82d096989cf..1543ef311d7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -538,9 +538,7 @@ class MergeRequest < ActiveRecord::Base
     @merge_commit ||= project.commit(merge_commit_sha) if merge_commit_sha
   end
 
-  def has_been_reverted?(current_user = nil)
-    if merge_commit
-      merge_commit.has_been_reverted?(current_user, self)
-    end
+  def can_be_reverted?(current_user = nil)
+    merge_commit && !merge_commit.has_been_reverted?(current_user, self)
   end
 end
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 4befb260859..648512e5379 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -85,7 +85,7 @@
         = spinner
 
 = render 'shared/issuable/sidebar', issuable: @merge_request
-- if @merge_request.merge_commit && !@merge_request.has_been_reverted?(current_user)
+- if @merge_request.can_be_reverted?
   = render "projects/commit/revert", commit: @merge_request.merge_commit, title: @merge_request.title
 
 :javascript
diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml
index 7320ff0ccc1..473fbff721b 100644
--- a/app/views/projects/merge_requests/show/_mr_title.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_title.html.haml
@@ -17,4 +17,4 @@
           %i.fa.fa-pencil-square-o
           Edit
       - if @merge_request.closed?
-        = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'btn btn-nr btn-grouped btn-reopen reopen-mr-link', title: "Reopen merge request"
+        = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'btn btn-nr btn-grouped btn-reopen reopen-mr-link', title: 'Reopen 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 09b1db5358e..3abae9f0bf6 100644
--- a/app/views/projects/merge_requests/widget/_merged.html.haml
+++ b/app/views/projects/merge_requests/widget/_merged.html.haml
@@ -7,28 +7,19 @@
         by #{link_to_member(@project, @merge_request.merge_event.author, avatar: true)}
         #{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)
-          = revert_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm')
-
       - if !@merge_request.source_branch_exists? || (params[:delete_source] == 'true')
         %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.
-        - if revert_button.present?
-          .btn-group= revert_button
+        = 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.
-          .btn-group
-            = 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
-            = revert_button
+          = 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')