Protected Tags backend review changes

Added changelog

8 files changed, 73 insertions, 2 deletions

diff --git a/spec/controllers/projects/protected_branches_controller_spec.rb b/spec/controllers/projects/protected_branches_controller_spec.rb
index e378b5714fe..80be135b5d8 100644
--- a/spec/controllers/projects/protected_branches_controller_spec.rb
+++ b/spec/controllers/projects/protected_branches_controller_spec.rb
@@ -3,6 +3,7 @@ require('spec_helper')
 describe Projects::ProtectedBranchesController do
   describe "GET #index" do
     let(:project) { create(:project_empty_repo, :public) }
+
     it "redirects empty repo to projects page" do
       get(:index, namespace_id: project.namespace.to_param, project_id: project)
     end
diff --git a/spec/controllers/projects/protected_tags_controller_spec.rb b/spec/controllers/projects/protected_tags_controller_spec.rb
index ac802981294..64658988b3f 100644
--- a/spec/controllers/projects/protected_tags_controller_spec.rb
+++ b/spec/controllers/projects/protected_tags_controller_spec.rb
@@ -3,6 +3,7 @@ require('spec_helper')
 describe Projects::ProtectedTagsController do
   describe "GET #index" do
     let(:project) { create(:project_empty_repo, :public) }
+
     it "redirects empty repo to projects page" do
       get(:index, namespace_id: project.namespace.to_param, project_id: project)
     end
diff --git a/spec/features/protected_branches/access_control_ce_spec.rb b/spec/features/protected_branches/access_control_ce_spec.rb
index e4aca25a339..eb3cea775da 100644
--- a/spec/features/protected_branches/access_control_ce_spec.rb
+++ b/spec/features/protected_branches/access_control_ce_spec.rb
@@ -2,7 +2,9 @@ RSpec.shared_examples "protected branches > access control > CE" do
   ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
     it "allows creating protected branches that #{access_type_name} can push to" do
       visit namespace_project_protected_branches_path(project.namespace, project)
+
       set_protected_branch_name('master')
+
       within('.new_protected_branch') do
         allowed_to_push_button = find(".js-allowed-to-push")
 
@@ -11,6 +13,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
           within(".dropdown.open .dropdown-menu") { click_on access_type_name }
         end
       end
+
       click_on "Protect"
 
       expect(ProtectedBranch.count).to eq(1)
@@ -19,7 +22,9 @@ RSpec.shared_examples "protected branches > access control > CE" do
 
     it "allows updating protected branches so that #{access_type_name} can push to them" do
       visit namespace_project_protected_branches_path(project.namespace, project)
+
       set_protected_branch_name('master')
+
       click_on "Protect"
 
       expect(ProtectedBranch.count).to eq(1)
@@ -34,6 +39,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
       end
 
       wait_for_ajax
+
       expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to include(access_type_id)
     end
   end
@@ -41,7 +47,9 @@ RSpec.shared_examples "protected branches > access control > CE" do
   ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
     it "allows creating protected branches that #{access_type_name} can merge to" do
       visit namespace_project_protected_branches_path(project.namespace, project)
+
       set_protected_branch_name('master')
+
       within('.new_protected_branch') do
         allowed_to_merge_button = find(".js-allowed-to-merge")
 
@@ -50,6 +58,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
           within(".dropdown.open .dropdown-menu") { click_on access_type_name }
         end
       end
+
       click_on "Protect"
 
       expect(ProtectedBranch.count).to eq(1)
@@ -58,7 +67,9 @@ RSpec.shared_examples "protected branches > access control > CE" do
 
     it "allows updating protected branches so that #{access_type_name} can merge to them" do
       visit namespace_project_protected_branches_path(project.namespace, project)
+
       set_protected_branch_name('master')
+
       click_on "Protect"
 
       expect(ProtectedBranch.count).to eq(1)
@@ -73,6 +84,7 @@ RSpec.shared_examples "protected branches > access control > CE" do
       end
 
       wait_for_ajax
+
       expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to include(access_type_id)
     end
   end
diff --git a/spec/features/protected_tags/access_control_ce_spec.rb b/spec/features/protected_tags/access_control_ce_spec.rb
index 33a07786007..de2556041e7 100644
--- a/spec/features/protected_tags/access_control_ce_spec.rb
+++ b/spec/features/protected_tags/access_control_ce_spec.rb
@@ -2,7 +2,9 @@ RSpec.shared_examples "protected tags > access control > CE" do
   ProtectedTag::CreateAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)|
     it "allows creating protected tags that #{access_type_name} can create" do
       visit namespace_project_protected_tags_path(project.namespace, project)
+
       set_protected_tag_name('master')
+
       within('.new_protected_tag') do
         allowed_to_create_button = find(".js-allowed-to-create")
 
@@ -11,6 +13,7 @@ RSpec.shared_examples "protected tags > access control > CE" do
           within(".dropdown.open .dropdown-menu") { click_on access_type_name }
         end
       end
+
       click_on "Protect"
 
       expect(ProtectedTag.count).to eq(1)
@@ -19,7 +22,9 @@ RSpec.shared_examples "protected tags > access control > CE" do
 
     it "allows updating protected tags so that #{access_type_name} can create them" do
       visit namespace_project_protected_tags_path(project.namespace, project)
+
       set_protected_tag_name('master')
+
       click_on "Protect"
 
       expect(ProtectedTag.count).to eq(1)
@@ -34,6 +39,7 @@ RSpec.shared_examples "protected tags > access control > CE" do
       end
 
       wait_for_ajax
+
       expect(ProtectedTag.last.create_access_levels.map(&:access_level)).to include(access_type_id)
     end
   end
diff --git a/spec/models/protectable_dropdown_spec.rb b/spec/models/protectable_dropdown_spec.rb
index 7f8ef7195e5..4c9bade592b 100644
--- a/spec/models/protectable_dropdown_spec.rb
+++ b/spec/models/protectable_dropdown_spec.rb
@@ -18,6 +18,7 @@ describe ProtectableDropdown, models: true do
       create(:protected_branch, name: 'feat*', project: project)
 
       subject = described_class.new(project.reload, :branches)
+
       expect(subject.protectable_ref_names).to include('feature')
     end
   end
diff --git a/spec/models/protected_tag_spec.rb b/spec/models/protected_tag_spec.rb
index 05ad532935a..51353852a93 100644
--- a/spec/models/protected_tag_spec.rb
+++ b/spec/models/protected_tag_spec.rb
@@ -1,8 +1,6 @@
 require 'spec_helper'
 
 describe ProtectedTag, models: true do
-  subject { build_stubbed(:protected_branch) }
-
   describe 'Associations' do
     it { is_expected.to belong_to(:project) }
   end
diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb
new file mode 100644
index 00000000000..62bdd49a4d7
--- /dev/null
+++ b/spec/services/protected_branches/update_service_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe ProtectedBranches::UpdateService, services: true do
+  let(:protected_branch) { create(:protected_branch) }
+  let(:project) { protected_branch.project }
+  let(:user) { project.owner }
+  let(:params) { { name: 'new protected branch name' } }
+
+  describe '#execute' do
+    subject(:service) { described_class.new(project, user, params) }
+
+    it 'updates a protected branch' do
+      result = service.execute(protected_branch)
+
+      expect(result.reload.name).to eq(params[:name])
+    end
+
+    context 'without admin_project permissions' do
+      let(:user) { create(:user) }
+
+      it "raises error" do
+        expect{ service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+  end
+end
diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb
new file mode 100644
index 00000000000..e78fde4c48d
--- /dev/null
+++ b/spec/services/protected_tags/update_service_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe ProtectedTags::UpdateService, services: true do
+  let(:protected_tag) { create(:protected_tag) }
+  let(:project) { protected_tag.project }
+  let(:user) { project.owner }
+  let(:params) { { name: 'new protected tag name' } }
+
+  describe '#execute' do
+    subject(:service) { described_class.new(project, user, params) }
+
+    it 'updates a protected tag' do
+      result = service.execute(protected_tag)
+
+      expect(result.reload.name).to eq(params[:name])
+    end
+
+    context 'without admin_project permissions' do
+      let(:user) { create(:user) }
+
+      it "raises error" do
+        expect{ service.execute(protected_tag) }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+  end
+end