Skip to content

Make access level for protected tag factories less conflicting

username-removed-423915 requested to merge remove-default-access-levels-ee into master

What does this MR do?

Make access level for protected tag factories less conflicting, so we don't use :remove_default_access_levels but { default_access_level: false } instead.

Are there points in the code the reviewer needs to double check?

Checkout the CE version: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13133

Why was this MR needed?

To reduce conflicts and unify the behaviour. Previously, CE was using update! while EE was using create! which is acting completely different...

Result:

Before:

diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb
index b2695e0482..d5ae94fe79 100644
--- a/spec/factories/protected_branches.rb
+++ b/spec/factories/protected_branches.rb
@@ -8,22 +8,51 @@ FactoryGirl.define do
       protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER)
     end
 
+    transient do
+      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
+    end
+
     trait :developers_can_push do
       after(:create) do |protected_branch|
-        protected_branch.push_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER)
+        protected_branch.push_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
       end
     end
 
     trait :developers_can_merge do
       after(:create) do |protected_branch|
-        protected_branch.merge_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER)
+        protected_branch.merge_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
       end
     end
 
     trait :no_one_can_push do
       after(:create) do |protected_branch|
-        protected_branch.push_access_levels.first.update!(access_level: Gitlab::Access::NO_ACCESS)
+        protected_branch.push_access_levels.create!(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)
+      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
+
+      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
+    end
   end
 end
diff --git a/spec/factories/protected_tags.rb b/spec/factories/protected_tags.rb
index d8e90ae1ee..c90180b0ec 100644
--- a/spec/factories/protected_tags.rb
+++ b/spec/factories/protected_tags.rb
@@ -7,16 +7,38 @@ FactoryGirl.define do
       protected_tag.create_access_levels.new(access_level: Gitlab::Access::MASTER)
     end
 
+    transient do
+      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
+    end
+
     trait :developers_can_create do
       after(:create) do |protected_tag|
-        protected_tag.create_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER)
+        protected_tag.create_access_levels.create!(access_level: Gitlab::Access::DEVELOPER)
       end
     end
 
     trait :no_one_can_create do
       after(:create) do |protected_tag|
-        protected_tag.create_access_levels.first.update!(access_level: Gitlab::Access::NO_ACCESS)
+        protected_tag.create_access_levels.create!(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)
+      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
+    end
   end
 end

After:

diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb
index 3dbace4b38..740efb0aa7 100644
--- a/spec/factories/protected_branches.rb
+++ b/spec/factories/protected_branches.rb
@@ -4,6 +4,12 @@ FactoryGirl.define do
     project
 
     transient do
+      # EE
+      authorize_user_to_push nil
+      authorize_user_to_merge nil
+      authorize_group_to_push nil
+      authorize_group_to_merge nil
+
       default_push_level true
       default_merge_level true
       default_access_level true
@@ -50,6 +56,21 @@ FactoryGirl.define do
     end
 
     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?
+
       if evaluator.default_access_level && evaluator.default_push_level
         protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER)
       end
diff --git a/spec/factories/protected_tags.rb b/spec/factories/protected_tags.rb
index 225588b23c..7741f05a6f 100644
--- a/spec/factories/protected_tags.rb
+++ b/spec/factories/protected_tags.rb
@@ -4,6 +4,10 @@ FactoryGirl.define do
     project
 
     transient do
+      # EE
+      authorize_user_to_create nil
+      authorize_group_to_create nil
+
       default_access_level true
     end
 
@@ -38,6 +42,14 @@ FactoryGirl.define do
     end
 
     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
Edited by username-removed-423915

Merge request reports