Revert "Revert "Merge branch '18193-developers-can-merge' into 'master' ""

This reverts commit 530f5158e297f3cde27f3566cfe13bad74ba3b50.

See !4892.

Signed-off-by: Rémy Coutable <remy@rymai.me>

15 files changed, 47 insertions, 20 deletions

diff --git a/app/assets/javascripts/protected_branches.js.coffee b/app/assets/javascripts/protected_branches.js.coffee
index 79c2306e4d2..14afef2e2ee 100644
--- a/app/assets/javascripts/protected_branches.js.coffee
+++ b/app/assets/javascripts/protected_branches.js.coffee
@@ -1,18 +1,18 @@
 $ ->
   $(".protected-branches-list :checkbox").change (e) ->
     name = $(this).attr("name")
-    if name == "developers_can_push"
+    if name == "developers_can_push" || name == "developers_can_merge"
       id = $(this).val()
-      checked = $(this).is(":checked")
+      can_push = $(this).is(":checked")
       url = $(this).data("url")
       $.ajax
-        type: "PUT"
+        type: "PATCH"
         url: url
         dataType: "json"
         data:
           id: id
           protected_branch:
-            developers_can_push: checked
+            "#{name}": can_push
 
         success: ->
           row = $(e.target)
diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb
index 80dad758afa..10dca47fded 100644
--- a/app/controllers/projects/protected_branches_controller.rb
+++ b/app/controllers/projects/protected_branches_controller.rb
@@ -50,6 +50,6 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
   end
 
   def protected_branch_params
-    params.require(:protected_branch).permit(:name, :developers_can_push)
+    params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge)
   end
 end
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index 601df5c18df..bfd23aa4e04 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -12,7 +12,7 @@ module BranchesHelper
   def can_push_branch?(project, branch_name)
     return false unless project.repository.branch_exists?(branch_name)
 
-    ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(branch_name)
+    ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name)
   end
 
   def project_branches
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 157901378d3..471e32f3b60 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -552,7 +552,13 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def can_be_merged_by?(user)
-    ::Gitlab::GitAccess.new(user, project, 'web').can_push_to_branch?(target_branch)
+    access = ::Gitlab::UserAccess.new(user, project: project)
+    access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch)
+  end
+
+  def can_be_merged_via_command_line_by?(user)
+    access = ::Gitlab::UserAccess.new(user, project: project)
+    access.can_push_to_branch?(target_branch)
   end
 
   def mergeable_ci_state?
diff --git a/app/models/project.rb b/app/models/project.rb
index d3ae4a2dd0b..b1ef2cb457c 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -838,6 +838,10 @@ class Project < ActiveRecord::Base
     protected_branches.matching(branch_name).any?(&:developers_can_push)
   end
 
+  def developers_can_merge_to_protected_branch?(branch_name)
+    protected_branches.matching(branch_name).any?(&:developers_can_merge)
+  end
+
   def forked?
     !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
   end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 5b670cb4b8f..09487b62f98 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -769,9 +769,9 @@ class Repository
     end
   end
 
-  def merge(user, source_sha, target_branch, options = {})
-    our_commit = rugged.branches[target_branch].target
-    their_commit = rugged.lookup(source_sha)
+  def merge(user, merge_request, options = {})
+    our_commit = rugged.branches[merge_request.target_branch].target
+    their_commit = rugged.lookup(merge_request.diff_head_sha)
 
     raise "Invalid merge target" if our_commit.nil?
     raise "Invalid merge source" if their_commit.nil?
@@ -779,14 +779,16 @@ class Repository
     merge_index = rugged.merge_commits(our_commit, their_commit)
     return false if merge_index.conflicts?
 
-    commit_with_hooks(user, target_branch) do |ref|
+    commit_with_hooks(user, merge_request.target_branch) do |tmp_ref|
       actual_options = options.merge(
         parents: [our_commit, their_commit],
         tree: merge_index.write_tree(rugged),
-        update_ref: ref
+        update_ref: tmp_ref
       )
 
-      Rugged::Commit.create(rugged, actual_options)
+      commit_id = Rugged::Commit.create(rugged, actual_options)
+      merge_request.update(in_progress_merge_commit_sha: commit_id)
+      commit_id
     end
   end
 
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index c578097376a..ed73d8cb8c2 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -23,7 +23,7 @@ module Commits
     private
 
     def check_push_permissions
-      allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
 
       unless allowed
         raise ValidationError.new('You are not allowed to push into this branch')
@@ -31,7 +31,7 @@ module Commits
 
       true
     end
-    
+
     def create_target_branch(new_branch)
       # Temporary branch exists and contains the change commit
       return success if repository.find_branch(new_branch)
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 37c5e321b39..55da949f56a 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -42,7 +42,7 @@ module Files
     end
 
     def validate
-      allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
 
       unless allowed
         raise_error("You are not allowed to push into this branch")
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index a886f35981f..e02b50ff9a2 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -89,7 +89,8 @@ class GitPushService < BaseService
     # Set protection on the default branch if configured
     if current_application_settings.default_branch_protection != PROTECTION_NONE
       developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
-      @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push })
+      developers_can_merge = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? true : false
+      @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push, developers_can_merge: developers_can_merge })
     end
   end
 
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index f1b1d90c457..0dac0614141 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -34,7 +34,7 @@ module MergeRequests
         committer: committer
       }
 
-      commit_id = repository.merge(current_user, merge_request.diff_head_sha, merge_request.target_branch, options)
+      commit_id = repository.merge(current_user, merge_request, options)
       merge_request.update(merge_commit_sha: commit_id)
     rescue GitHooksService::PreReceiveError => e
       merge_request.update(merge_error: e.message)
@@ -43,6 +43,8 @@ module MergeRequests
       merge_request.update(merge_error: "Something went wrong during merge")
       Rails.logger.error(e.message)
       false
+    ensure
+      merge_request.update(in_progress_merge_commit_sha: nil)
     end
 
     def after_merge
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index b11ecd97a57..1daf6bbf553 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -48,7 +48,7 @@ module MergeRequests
     end
 
     def force_push?
-      Gitlab::ForcePushCheck.force_push?(@project, @oldrev, @newrev)
+      Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
     end
 
     # Refresh merge request diff if we push to source or target branch of merge request
diff --git a/app/views/projects/merge_requests/widget/open/_conflicts.html.haml b/app/views/projects/merge_requests/widget/open/_conflicts.html.haml
index 06ab0a3fa00..f000cc38a65 100644
--- a/app/views/projects/merge_requests/widget/open/_conflicts.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_conflicts.html.haml
@@ -4,7 +4,7 @@
 
 %p
   Please resolve these conflicts or
-  - if @merge_request.can_be_merged_by?(current_user)
+  - if @merge_request.can_be_merged_via_command_line_by?(current_user)
     #{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}.
   - else
     ask someone with write access to this repository to merge this request manually.
diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml
index 97cb1a9052b..720d67dff7c 100644
--- a/app/views/projects/protected_branches/_branches_list.html.haml
+++ b/app/views/projects/protected_branches/_branches_list.html.haml
@@ -8,6 +8,7 @@
   .table-responsive
     %table.table.protected-branches-list
       %colgroup
+        %col{ width: "20%" }
         %col{ width: "30%" }
         %col{ width: "25%" }
         %col{ width: "25%" }
@@ -18,6 +19,7 @@
           %th Protected Branch
           %th Commit
           %th Developers Can Push
+          %th Developers Can Merge
           - if can_admin_project
             %th
       %tbody
diff --git a/app/views/projects/protected_branches/_protected_branch.html.haml b/app/views/projects/protected_branches/_protected_branch.html.haml
index 474aec3a97c..7fda7f96047 100644
--- a/app/views/projects/protected_branches/_protected_branch.html.haml
+++ b/app/views/projects/protected_branches/_protected_branch.html.haml
@@ -16,6 +16,8 @@
         (branch was removed from repository)
   %td
     = check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url })
+  %td
+    = check_box_tag("developers_can_merge", protected_branch.id, protected_branch.developers_can_merge, data: { url: url })
   - if can_admin_project
     %td
       = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right"
diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml
index 883d3e3af1e..151e1d64851 100644
--- a/app/views/projects/protected_branches/index.html.haml
+++ b/app/views/projects/protected_branches/index.html.haml
@@ -36,6 +36,14 @@
             = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0"
             %p.light.append-bottom-0
               Allow developers to push to this branch
+
+        .form-group
+          = f.check_box :developers_can_merge, class: "pull-left"
+          .prepend-left-20
+            = f.label :developers_can_merge, "Developers can merge", class: "label-light append-bottom-0"
+            %p.light.append-bottom-0
+              Allow developers to accept merge requests to this branch
         = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true
+
     %hr
     = render "branches_list"