Merge branch '338821-fj-move-project-feature-validation-to-featurable-concern' into 'master'

Fix bug validating EE project features

See merge request gitlab-org/gitlab!68523

app/models/concerns/featurable.rb

@@ -83,6 +83,10 @@ module Featurable
     end
   end
 
+  included do
+    validate :allowed_access_levels
+  end
+
   def access_level(feature)
     public_send(self.class.access_level_attribute(feature)) # rubocop:disable GitlabSecurity/PublicSend
   end
@@ -94,4 +98,21 @@ module Featurable
   def string_access_level(feature)
     self.class.str_from_access_level(access_level(feature))
   end
+
+  private
+
+  def allowed_access_levels
+    validator = lambda do |field|
+      level = public_send(field) || ENABLED # rubocop:disable GitlabSecurity/PublicSend
+      not_allowed = level > ENABLED
+      self.errors.add(field, "cannot have public visibility level") if not_allowed
+    end
+
+    (self.class.available_features - feature_validation_exclusion).each {|f| validator.call("#{f}_access_level")}
+  end
+
+  # Features that we should exclude from the validation
+  def feature_validation_exclusion
+    []
+  end
 end

app/models/project_feature.rb

@@ -54,7 +54,6 @@ class ProjectFeature < ApplicationRecord
   validates :project, presence: true
 
   validate :repository_children_level
-  validate :allowed_access_levels
 
   default_value_for :builds_access_level, value: ENABLED, allows_nil: false
   default_value_for :issues_access_level, value: ENABLED, allows_nil: false
@@ -110,17 +109,6 @@ class ProjectFeature < ApplicationRecord
     %i(merge_requests_access_level builds_access_level).each(&validator)
   end
 
-  # Validates access level for other than pages cannot be PUBLIC
-  def allowed_access_levels
-    validator = lambda do |field|
-      level = public_send(field) || ENABLED # rubocop:disable GitlabSecurity/PublicSend
-      not_allowed = level > ENABLED
-      self.errors.add(field, "cannot have public visibility level") if not_allowed
-    end
-
-    (FEATURES - %i(pages)).each {|f| validator.call("#{f}_access_level")}
-  end
-
   def get_permission(user, feature)
     case access_level(feature)
     when DISABLED
@@ -142,6 +130,10 @@ class ProjectFeature < ApplicationRecord
 
     project.team.member?(user, ProjectFeature.required_minimum_access_level(feature))
   end
+
+  def feature_validation_exclusion
+    %i(pages)
+  end
 end
 
 ProjectFeature.prepend_mod_with('ProjectFeature')

ee/spec/models/project_feature_spec.rb

@@ -54,4 +54,8 @@ RSpec.describe ProjectFeature do
       end
     end
   end
+
+  it_behaves_like 'access level validation', ProjectFeature::EE_FEATURES do
+    let(:container_features) { project.project_feature }
+  end
 end

spec/models/concerns/featurable_spec.rb

@@ -30,8 +30,11 @@ RSpec.describe Featurable do
 
   describe '.set_available_features' do
     let!(:klass) do
-      Class.new do
+      Class.new(ApplicationRecord) do
         include Featurable
+
+        self.table_name = 'project_features'
+
         set_available_features %i(feature1 feature2)
 
         def feature1_access_level

spec/models/project_feature_spec.rb

@@ -41,18 +41,15 @@ RSpec.describe ProjectFeature do
     end
   end
 
-  context 'public features' do
-    features = ProjectFeature::FEATURES - %i(pages)
+  it_behaves_like 'access level validation', ProjectFeature::FEATURES - %i(pages) do
+    let(:container_features) { project.project_feature }
+  end
 
-    features.each do |feature|
-      it "does not allow public access level for #{feature}" do
-        project_feature = project.project_feature
-        field = "#{feature}_access_level".to_sym
-        project_feature.update_attribute(field, ProjectFeature::PUBLIC)
+  it 'allows public access level for :pages feature' do
+    project_feature = project.project_feature
+    project_feature.pages_access_level = ProjectFeature::PUBLIC
 
-        expect(project_feature.valid?).to be_falsy, "#{field} failed"
-      end
-    end
+    expect(project_feature.valid?).to be_truthy
   end
 
   describe 'default pages access level' do

spec/support/shared_examples/models/concerns/featurable_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'access level validation' do |features|
+  features.each do |feature|
+    it "does not allow public access level for #{feature}" do
+      field = "#{feature}_access_level".to_sym
+      container_features.update_attribute(field, ProjectFeature::PUBLIC)
+
+      expect(container_features.valid?).to be_falsy, "#{field} failed"
+    end
+  end
+end