Merge branch 'rd-fix-deleting-existing-branches-with-push-rule-config' into 'master'

Fix a bug where branch could not be delete due to a push rule config

Closes #4078

See merge request gitlab-org/gitlab-ee!3900

changelogs/unreleased-ee/4078-fix-deleting-existing-branches-with-push-rule-config.yml

+---
+title: Fix a bug where branch could not be delete due to a push rule config
+merge_request: 3900
+author:
+type: fixed

lib/gitlab/checks/change_access.rb

@@ -195,13 +195,18 @@ module Gitlab
       end
 
       def branch_name_allowed_by_push_rule?(push_rule)
-        return true unless push_rule
-        return true if @branch_name.blank?
-        return true if @branch_name == @project.default_branch
+        return true if skip_branch_name_push_rule?(push_rule)
 
         push_rule.branch_name_allowed?(@branch_name)
       end
 
+      def skip_branch_name_push_rule?(push_rule)
+        push_rule.nil? ||
+          deletion? ||
+          @branch_name.blank? ||
+          @branch_name == @project.default_branch
+      end
+
       def tag_deletion_denied_by_push_rule?(push_rule)
         push_rule.try(:deny_delete_tag) &&
           !updated_from_web? &&

spec/services/delete_branch_service_spec.rb

@@ -6,20 +6,24 @@ describe DeleteBranchService do
   let(:user) { create(:user) }
   let(:service) { described_class.new(project, user) }
 
+  shared_examples 'a deleted branch' do |branch_name|
+    it 'removes the branch' do
+      expect(branch_exists?(branch_name)).to be true
+
+      result = service.execute(branch_name)
+
+      expect(result[:status]).to eq :success
+      expect(branch_exists?(branch_name)).to be false
+    end
+  end
+
   describe '#execute' do
     context 'when user has access to push to repository' do
       before do
         project.add_developer(user)
       end
 
-      it 'removes the branch' do
-        expect(branch_exists?('feature')).to be true
-
-        result = service.execute('feature')
-
-        expect(result[:status]).to eq :success
-        expect(branch_exists?('feature')).to be false
-      end
+      it_behaves_like 'a deleted branch', 'feature'
     end
 
     context 'when user does not have access to push to repository' do
@@ -33,6 +37,15 @@ describe DeleteBranchService do
         expect(branch_exists?('feature')).to be true
       end
     end
+
+    context 'when there is a push rule matching the branch name' do
+      before do
+        project.add_developer(user)
+        create(:push_rule, branch_name_regex: '^(w*)$')
+      end
+
+      it_behaves_like 'a deleted branch', 'add-pdf-file'
+    end
   end
 
   def branch_exists?(branch_name)