Protected Tags EE merge fixes

app/controllers/projects/protected_branches_controller.rb

@@ -29,6 +29,10 @@ class Projects::ProtectedBranchesController < Projects::ProtectedRefsController
     self.protected_ref = @project.protected_branches.find(params[:id])
   end
 
+  def access_levels
+    [:merge_access_levels, :push_access_levels]
+  end
+
   def protected_ref_params
     params.require(:protected_branch).permit(:name,
                                              merge_access_levels_attributes: [:access_level, :id, :user_id, :_destroy, :group_id],

app/controllers/projects/protected_refs_controller.rb

@@ -28,7 +28,7 @@ class Projects::ProtectedRefsController < Projects::ApplicationController
 
     if protected_ref.valid?
       respond_to do |format|
-        format.json { render json: protected_ref, status: :ok, include: [:merge_access_levels, :push_access_levels] }
+        format.json { render json: protected_ref, status: :ok, include: access_levels }
       end
     else
       respond_to do |format|

app/controllers/projects/protected_tags_controller.rb

@@ -29,6 +29,10 @@ class Projects::ProtectedTagsController < Projects::ProtectedRefsController
     self.protected_ref = @project.protected_tags.find(params[:id])
   end
 
+  def access_levels
+    [:create_access_levels]
+  end
+
   def protected_ref_params
     params.require(:protected_tag).permit(:name, create_access_levels_attributes: [:access_level, :id])
   end

lib/gitlab/checks/change_access.rb

@@ -126,7 +126,7 @@ module Gitlab
         push_rule = project.push_rule
 
         # Prevent tag removal
-        if Gitlab::Git.tag_name(@ref)
+        if @tag_name
           if tag_deletion_denied_by_push_rule?(push_rule)
             return 'You cannot delete a tag'
           end
@@ -134,7 +134,7 @@ module Gitlab
           commit_validation = push_rule.try(:commit_validation?)
 
           # if newrev is blank, the branch was deleted
-          return if Gitlab::Git.blank_ref?(@newrev) || !(commit_validation || validate_path_locks?)
+          return if deletion? || !(commit_validation || validate_path_locks?)
 
           commits.each do |commit|
             if commit_validation
@@ -154,8 +154,8 @@ module Gitlab
       def tag_deletion_denied_by_push_rule?(push_rule)
         push_rule.try(:deny_delete_tag) &&
           protocol != 'web' &&
-          Gitlab::Git.blank_ref?(@newrev) &&
-          protected_tag?(Gitlab::Git.tag_name(@ref))
+          deletion? &&
+          tag_exists?
       end
 
       # If commit does not pass push rule validation the whole push should be rejected.

spec/features/protected_branches/access_control_ee_spec.rb

@@ -38,11 +38,9 @@ RSpec.shared_examples "protected branches > access control > EE" do
 
       click_on "Protect"
 
-      within(".js-protected-branch-edit-form") do
-        set_allowed_to(git_operation, users.map(&:name))
-        set_allowed_to(git_operation, groups.map(&:name))
-        set_allowed_to(git_operation, roles.values)
-      end
+      set_allowed_to(git_operation, users.map(&:name), form: ".js-protected-branch-edit-form")
+      set_allowed_to(git_operation, groups.map(&:name), form: ".js-protected-branch-edit-form")
+      set_allowed_to(git_operation, roles.values, form: ".js-protected-branch-edit-form")
 
       wait_for_ajax
 

spec/features/protected_branches_spec.rb

@@ -9,7 +9,8 @@ feature 'Protected Branches', feature: true, js: true do
 
   before { login_as(user) }
 
-  def set_allowed_to(operation, option = 'Masters')
+  def set_allowed_to(operation, option = 'Masters', form: '#new_protected_branch')
+    within form do
       find(".js-allowed-to-#{operation}").click
       wait_for_ajax
 
@@ -17,6 +18,7 @@ feature 'Protected Branches', feature: true, js: true do
 
       find(".js-allowed-to-#{operation}").click # needed to submit form in some cases
     end
+  end
 
   def set_protected_branch_name(branch_name)
     find(".js-protected-branch-select").click

spec/lib/gitlab/checks/change_access_spec.rb

@@ -144,16 +144,12 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
       end
 
       context 'tag deletion' do
-        let(:changes) do
-          {
-            oldrev: 'be93687618e4b132087f430a4d8fc3a609c9b77c',
-            newrev: '0000000000000000000000000000000000000000',
-            ref: 'refs/tags/v1.0.0'
-          }
-        end
         let(:push_rule) { create(:push_rule, deny_delete_tag: true) }
+        let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' }
+        let(:newrev) { '0000000000000000000000000000000000000000' }
+        let(:ref) { 'refs/tags/v1.0.0' }
 
-        before { allow(user_access).to receive(:can_do_action?).with(:admin_project).and_return(true) }
+        before { project.add_master(user) }
 
         it 'returns an error if the rule denies tag deletion' do
           expect(subject.status).to be(false)