Merge branch 'remove-default-access-levels-ee' into 'master'

Make access level for protected tag factories less conflicting

See merge request !2553

spec/factories/protected_branches.rb

@@ -3,56 +3,80 @@ FactoryGirl.define do
     name
     project
 
-    after(:build) do |protected_branch|
-      protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER)
-      protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER)
-    end
-
     transient do
+      # EE
       authorize_user_to_push nil
       authorize_user_to_merge nil
-
       authorize_group_to_push nil
       authorize_group_to_merge nil
-    end
 
-    trait :remove_default_access_levels do
-      after(:build) do |protected_branch|
-        protected_branch.push_access_levels = []
-        protected_branch.merge_access_levels = []
-      end
+      default_push_level true
+      default_merge_level true
+      default_access_level true
     end
 
     trait :developers_can_push do
-      after(:create) do |protected_branch|
-        protected_branch.push_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
+      transient do
+        default_push_level false
+      end
+
+      after(:build) do |protected_branch|
+        protected_branch.push_access_levels.new(access_level: Gitlab::Access::DEVELOPER)
       end
     end
 
     trait :developers_can_merge do
-      after(:create) do |protected_branch|
-        protected_branch.merge_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
+      transient do
+        default_merge_level false
+      end
+
+      after(:build) do |protected_branch|
+        protected_branch.merge_access_levels.new(access_level: Gitlab::Access::DEVELOPER)
       end
     end
 
     trait :no_one_can_push do
-      after(:create) do |protected_branch|
-        protected_branch.push_access_levels.create!(access_level: Gitlab::Access::NO_ACCESS)
+      transient do
+        default_push_level false
+      end
+
+      after(:build) do |protected_branch|
+        protected_branch.push_access_levels.new(access_level: Gitlab::Access::NO_ACCESS)
       end
     end
 
     trait :masters_can_push do
-      after(:create) do |protected_branch|
-        protected_branch.push_access_levels.create!(access_level: Gitlab::Access::MASTER)
+      transient do
+        default_push_level false
+      end
+
+      after(:build) do |protected_branch|
+        protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER)
       end
     end
 
-    after(:create) do |protected_branch, evaluator|
-      protected_branch.push_access_levels.create!(user: evaluator.authorize_user_to_push) if evaluator.authorize_user_to_push
-      protected_branch.merge_access_levels.create!(user: evaluator.authorize_user_to_merge) if evaluator.authorize_user_to_merge
+    after(:build) do |protected_branch, evaluator|
+      # EE
+      if user = evaluator.authorize_user_to_push
+        protected_branch.push_access_levels.new(user: user)
+      end
+      if user = evaluator.authorize_user_to_merge
+        protected_branch.merge_access_levels.new(user: user)
+      end
+      if group = evaluator.authorize_group_to_push
+        protected_branch.push_access_levels.new(group: group)
+      end
+      if group = evaluator.authorize_group_to_merge
+        protected_branch.merge_access_levels.new(group: group)
+      end
+      next unless protected_branch.merge_access_levels.empty?
 
-      protected_branch.push_access_levels.create!(group: evaluator.authorize_group_to_push) if evaluator.authorize_group_to_push
-      protected_branch.merge_access_levels.create!(group: evaluator.authorize_group_to_merge) if evaluator.authorize_group_to_merge
+      if evaluator.default_access_level && evaluator.default_push_level
+        protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER)
+      end
+      if evaluator.default_access_level && evaluator.default_merge_level
+        protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER)
+      end
     end
   end
 end

spec/factories/protected_tags.rb

@@ -3,42 +3,56 @@ FactoryGirl.define do
     name
     project
 
-    after(:build) do |protected_tag|
-      protected_tag.create_access_levels.new(access_level: Gitlab::Access::MASTER)
-    end
-
     transient do
+      # EE
       authorize_user_to_create nil
       authorize_group_to_create nil
-    end
 
-    trait :remove_default_access_levels do
-      after(:build) do |protected_tag|
-        protected_tag.create_access_levels = []
-      end
+      default_access_level true
     end
 
     trait :developers_can_create do
-      after(:create) do |protected_tag|
-        protected_tag.create_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
+      transient do
+        default_access_level false
+      end
+
+      after(:build) do |protected_tag|
+        protected_tag.create_access_levels.new(access_level: Gitlab::Access::DEVELOPER)
       end
     end
 
     trait :no_one_can_create do
-      after(:create) do |protected_tag|
-        protected_tag.create_access_levels.create!(access_level: Gitlab::Access::NO_ACCESS)
+      transient do
+        default_access_level false
+      end
+
+      after(:build) do |protected_tag|
+        protected_tag.create_access_levels.new(access_level: Gitlab::Access::NO_ACCESS)
       end
     end
 
     trait :masters_can_create do
-      after(:create) do |protected_tag|
-        protected_tag.create_access_levels.create!(access_level: Gitlab::Access::MASTER)
+      transient do
+        default_access_level false
+      end
+
+      after(:build) do |protected_tag|
+        protected_tag.create_access_levels.new(access_level: Gitlab::Access::MASTER)
       end
     end
 
-    after(:create) do |protected_tag, evaluator|
-      protected_tag.create_access_levels.create!(user: evaluator.authorize_user_to_create) if evaluator.authorize_user_to_create
-      protected_tag.create_access_levels.create!(group: evaluator.authorize_group_to_create) if evaluator.authorize_group_to_create
+    after(:build) do |protected_tag, evaluator|
+      # EE
+      if evaluator.authorize_user_to_create
+        protected_tag.create_access_levels.new(user: evaluator.authorize_user_to_create)
+      end
+      if evaluator.authorize_group_to_create
+        protected_tag.create_access_levels.new(group: evaluator.authorize_group_to_create)
+      end
+
+      if evaluator.default_access_level
+        protected_tag.create_access_levels.new(access_level: Gitlab::Access::MASTER)
+      end
     end
   end
 end

spec/lib/gitlab/git_access_spec.rb

@@ -556,7 +556,7 @@ describe Gitlab::GitAccess do
     [%w(feature exact), ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type|
       context do
         before do
-          create(:protected_branch, :remove_default_access_levels, :masters_can_push, name: protected_branch_name, project: project)
+          create(:protected_branch, :masters_can_push, name: protected_branch_name, project: project)
         end
 
         run_permission_checks(permissions_matrix)
@@ -564,7 +564,7 @@ describe Gitlab::GitAccess do
 
       context "when developers are allowed to push into the #{protected_branch_type} protected branch" do
         before do
-          create(:protected_branch, :remove_default_access_levels, :masters_can_push, :developers_can_push, name: protected_branch_name, project: project)
+          create(:protected_branch, :masters_can_push, :developers_can_push, name: protected_branch_name, project: project)
         end
 
         run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
@@ -572,7 +572,7 @@ describe Gitlab::GitAccess do
 
       context "developers are allowed to merge into the #{protected_branch_type} protected branch" do
         before do
-          create(:protected_branch, :remove_default_access_levels, :masters_can_push, :developers_can_merge, name: protected_branch_name, project: project)
+          create(:protected_branch, :masters_can_push, :developers_can_merge, name: protected_branch_name, project: project)
         end
 
         context "when a merge request exists for the given source/target branch" do
@@ -601,7 +601,7 @@ describe Gitlab::GitAccess do
 
       context "when developers are allowed to push and merge into the #{protected_branch_type} protected branch" do
         before do
-          create(:protected_branch, :remove_default_access_levels, :masters_can_push, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project)
+          create(:protected_branch, :masters_can_push, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project)
         end
 
         run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
@@ -612,7 +612,7 @@ describe Gitlab::GitAccess do
           let(:user) { create(:user) }
 
           before do
-            create(:protected_branch, :remove_default_access_levels, authorize_user_to_push: user, name: protected_branch_name, project: project)
+            create(:protected_branch, authorize_user_to_push: user, name: protected_branch_name, project: project)
           end
 
           run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true },
@@ -625,7 +625,7 @@ describe Gitlab::GitAccess do
 
           before do
             create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
-            create(:protected_branch, :remove_default_access_levels, authorize_user_to_merge: user, name: protected_branch_name, project: project)
+            create(:protected_branch, authorize_user_to_merge: user, name: protected_branch_name, project: project)
           end
 
           run_permission_checks(permissions_matrix.deep_merge(admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: true },
@@ -640,7 +640,7 @@ describe Gitlab::GitAccess do
 
           before do
             create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
-            create(:protected_branch, :remove_default_access_levels, authorize_user_to_push: user, authorize_user_to_merge: user, name: protected_branch_name, project: project)
+            create(:protected_branch, authorize_user_to_push: user, authorize_user_to_merge: user, name: protected_branch_name, project: project)
           end
 
           run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true },
@@ -656,7 +656,7 @@ describe Gitlab::GitAccess do
 
           before do
             group.add_master(user)
-            create(:protected_branch, :remove_default_access_levels, authorize_group_to_push: group, name: protected_branch_name, project: project)
+            create(:protected_branch, authorize_group_to_push: group, name: protected_branch_name, project: project)
           end
 
           permissions = permissions_matrix.except(:admin).deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true },
@@ -673,7 +673,7 @@ describe Gitlab::GitAccess do
           before do
             group.add_master(user)
             create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
-            create(:protected_branch, :remove_default_access_levels, authorize_group_to_merge: group, name: protected_branch_name, project: project)
+            create(:protected_branch, authorize_group_to_merge: group, name: protected_branch_name, project: project)
           end
 
           permissions = permissions_matrix.except(:admin).deep_merge(master: { push_protected_branch: false, push_all: false, merge_into_protected_branch: true },
@@ -691,7 +691,7 @@ describe Gitlab::GitAccess do
           before do
             group.add_master(user)
             create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
-            create(:protected_branch, :remove_default_access_levels, authorize_group_to_push: group, authorize_group_to_merge: group, name: protected_branch_name, project: project)
+            create(:protected_branch, authorize_group_to_push: group, authorize_group_to_merge: group, name: protected_branch_name, project: project)
           end
 
           permissions = permissions_matrix.except(:admin).deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true },
@@ -704,7 +704,7 @@ describe Gitlab::GitAccess do
 
       context "when no one is allowed to push to the #{protected_branch_name} protected branch" do
         before do
-          create(:protected_branch, :remove_default_access_levels, :no_one_can_push, name: protected_branch_name, project: project)
+          create(:protected_branch, :no_one_can_push, name: protected_branch_name, project: project)
         end
 
         run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false },

spec/models/protected_branch_spec.rb

@@ -16,8 +16,8 @@ describe ProtectedBranch do
 
       context "while checking uniqueness of a role-based #{human_association_name}" do
         it "allows a single #{human_association_name} for a role (per protected branch)" do
-          first_protected_branch = create(:protected_branch, :remove_default_access_levels)
-          second_protected_branch = create(:protected_branch, :remove_default_access_levels)
+          first_protected_branch = create(:protected_branch, default_access_level: false)
+          second_protected_branch = create(:protected_branch, default_access_level: false)
 
           first_protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
           second_protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
@@ -31,7 +31,7 @@ describe ProtectedBranch do
         end
 
         it "does not count a user-based #{human_association_name} with an `access_level` set" do
-          protected_branch = create(:protected_branch, :remove_default_access_levels)
+          protected_branch = create(:protected_branch, default_access_level: false)
 
           protected_branch.send(association_name) << build(factory_name, user: user, access_level: Gitlab::Access::MASTER)
           protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
@@ -41,7 +41,7 @@ describe ProtectedBranch do
 
         it "does not count a group-based #{human_association_name} with an `access_level` set" do
           group = create(:group)
-          protected_branch = create(:protected_branch, :remove_default_access_levels)
+          protected_branch = create(:protected_branch, default_access_level: false)
 
           protected_branch.send(association_name) << build(factory_name, group: group, access_level: Gitlab::Access::MASTER)
           protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
@@ -52,8 +52,8 @@ describe ProtectedBranch do
 
       context "while checking uniqueness of a user-based #{human_association_name}" do
         it "allows a single #{human_association_name} for a user (per protected branch)" do
-          first_protected_branch = create(:protected_branch, :remove_default_access_levels)
-          second_protected_branch = create(:protected_branch, :remove_default_access_levels)
+          first_protected_branch = create(:protected_branch, default_access_level: false)
+          second_protected_branch = create(:protected_branch, default_access_level: false)
 
           first_protected_branch.send(association_name) << build(factory_name, user: user)
           second_protected_branch.send(association_name) << build(factory_name, user: user)
@@ -67,7 +67,7 @@ describe ProtectedBranch do
         end
 
         it "ignores the `access_level` while validating a user-based #{human_association_name}" do
-          protected_branch = create(:protected_branch, :remove_default_access_levels)
+          protected_branch = create(:protected_branch, default_access_level: false)
 
           protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
           protected_branch.send(association_name) << build(factory_name, user: user, access_level: Gitlab::Access::MASTER)
@@ -80,8 +80,8 @@ describe ProtectedBranch do
         let(:group) { create(:group) }
 
         it "allows a single #{human_association_name} for a group (per protected branch)" do
-          first_protected_branch = create(:protected_branch, :remove_default_access_levels)
-          second_protected_branch = create(:protected_branch, :remove_default_access_levels)
+          first_protected_branch = create(:protected_branch, default_access_level: false)
+          second_protected_branch = create(:protected_branch, default_access_level: false)
 
           first_protected_branch.send(association_name) << build(factory_name, group: group)
           second_protected_branch.send(association_name) << build(factory_name, group: group)
@@ -95,7 +95,7 @@ describe ProtectedBranch do
         end
 
         it "ignores the `access_level` while validating a group-based #{human_association_name}" do
-          protected_branch = create(:protected_branch, :remove_default_access_levels)
+          protected_branch = create(:protected_branch, default_access_level: false)
 
           protected_branch.send(association_name) << build(factory_name, access_level: Gitlab::Access::MASTER)
           protected_branch.send(association_name) << build(factory_name, group: group, access_level: Gitlab::Access::MASTER)

spec/services/ci/create_pipeline_service_spec.rb

@@ -532,7 +532,6 @@ describe Ci::CreatePipelineService do
         context 'when no one can create the tag' do
           let!(:protected_tag) do
             create(:protected_tag,
-                   :remove_default_access_levels,
                    :no_one_can_create,
                    project: project,
                    name: ref)

spec/services/git_push_service_spec.rb

@@ -271,7 +271,6 @@ describe GitPushService do
         stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
 
         create(:protected_branch, :no_one_can_push, :developers_can_merge,
-               :remove_default_access_levels,
                project: project, name: 'master')
         expect(project).to receive(:execute_hooks)
         expect(project.default_branch).to eq("master")