Backport EE refactorings for Protected Tag EE-only functionalityjej-backport-protected-tag-ee-role-refactorings

Improvements and refactorings were made while adding role based permissions for protected tags to EE. This doesn’t backport the feature, but should improve code quality and minimize divergence.

3 files changed, 22 insertions, 21 deletions

diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb
index 62eaec2407f..47e71c58557 100644
--- a/app/models/concerns/protected_ref.rb
+++ b/app/models/concerns/protected_ref.rb
@@ -8,32 +8,44 @@ module ProtectedRef
     validates :project, presence: true
 
     delegate :matching, :matches?, :wildcard?, to: :ref_matcher
+  end
+
+  def commit
+    project.commit(self.name)
+  end
+
+  class_methods do
+    def protected_ref_access_levels(*types)
+      types.each do |type|
+        has_many :"#{type}_access_levels", dependent: :destroy
+
+        validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." }
 
-    def self.protected_ref_accessible_to?(ref, user, action:)
+        accepts_nested_attributes_for :"#{type}_access_levels", allow_destroy: true
+      end
+    end
+
+    def protected_ref_accessible_to?(ref, user, action:)
       access_levels_for_ref(ref, action: action).any? do |access_level|
         access_level.check_access(user)
       end
     end
 
-    def self.developers_can?(action, ref)
+    def developers_can?(action, ref)
       access_levels_for_ref(ref, action: action).any? do |access_level|
         access_level.access_level == Gitlab::Access::DEVELOPER
       end
     end
 
-    def self.access_levels_for_ref(ref, action:)
+    def access_levels_for_ref(ref, action:)
       self.matching(ref).map(&:"#{action}_access_levels").flatten
     end
 
-    def self.matching(ref_name, protected_refs: nil)
+    def matching(ref_name, protected_refs: nil)
       ProtectedRefMatcher.matching(self, ref_name, protected_refs: protected_refs)
     end
   end
 
-  def commit
-    project.commit(self.name)
-  end
-
   private
 
   def ref_matcher
diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index 28b7d5ad072..5f0d0802ac9 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -2,14 +2,7 @@ class ProtectedBranch < ActiveRecord::Base
   include Gitlab::ShellAdapter
   include ProtectedRef
 
-  has_many :merge_access_levels, dependent: :destroy
-  has_many :push_access_levels, dependent: :destroy
-
-  validates :merge_access_levels, length: { is: 1, message: "are restricted to a single instance per protected branch." }
-  validates :push_access_levels, length: { is: 1, message: "are restricted to a single instance per protected branch." }
-
-  accepts_nested_attributes_for :push_access_levels
-  accepts_nested_attributes_for :merge_access_levels
+  protected_ref_access_levels :merge, :push
 
   # Check if branch name is marked as protected in the system
   def self.protected?(project, ref_name)
diff --git a/app/models/protected_tag.rb b/app/models/protected_tag.rb
index 83964095516..f38109c0e52 100644
--- a/app/models/protected_tag.rb
+++ b/app/models/protected_tag.rb
@@ -2,11 +2,7 @@ class ProtectedTag < ActiveRecord::Base
   include Gitlab::ShellAdapter
   include ProtectedRef
 
-  has_many :create_access_levels, dependent: :destroy
-
-  validates :create_access_levels, length: { is: 1, message: "are restricted to a single instance per protected tag." }
-
-  accepts_nested_attributes_for :create_access_levels
+  protected_ref_access_levels :create
 
   def self.protected?(project, ref_name)
     self.matching(ref_name, protected_refs: project.protected_tags).present?