diff --git a/app/models/concerns/limitable.rb b/app/models/concerns/limitable.rb index 41efea65c5a38112827168c7694372dcc4455426..fab1aa21634deeecafd7680d8cc2b23ff85beddb 100644 --- a/app/models/concerns/limitable.rb +++ b/app/models/concerns/limitable.rb @@ -9,6 +9,7 @@ module Limitable class_attribute :limit_relation class_attribute :limit_name class_attribute :limit_feature_flag + class_attribute :limit_feature_flag_for_override # Allows selectively disabling by actor (as per https://docs.gitlab.com/ee/development/feature_flags/#selectively-disable-by-actor) self.limit_name = self.name.demodulize.tableize validate :validate_plan_limit_not_exceeded, on: :create @@ -28,6 +29,7 @@ def validate_scoped_plan_limit_not_exceeded scope_relation = self.public_send(limit_scope) # rubocop:disable GitlabSecurity/PublicSend return unless scope_relation return if limit_feature_flag && ::Feature.disabled?(limit_feature_flag, scope_relation, default_enabled: :yaml) + return if limit_feature_flag_for_override && ::Feature.enabled?(limit_feature_flag_for_override, scope_relation, default_enabled: :yaml) relation = limit_relation ? self.public_send(limit_relation) : self.class.where(limit_scope => scope_relation) # rubocop:disable GitlabSecurity/PublicSend limits = scope_relation.actual_limits diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 2a020d6efb2bd9191ff33dfb062317cb07cd4acd..a0de43abe85c43c1ee16fbb6fd04e789ba826560 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -32,6 +32,7 @@ class MarkUsedFeatureFlags < RuboCop::Cop::Cop SELF_METHODS = %i[ push_frontend_feature_flag limit_feature_flag= + limit_feature_flag_for_override= ].freeze + EXPERIMENT_METHODS + RUGGED_METHODS + WORKER_METHODS RESTRICT_ON_SEND = FEATURE_METHODS + EXPERIMENTATION_METHODS + GRAPHQL_METHODS + SELF_METHODS diff --git a/spec/models/concerns/limitable_spec.rb b/spec/models/concerns/limitable_spec.rb index 6b25ed39efbd47afb6a308e565fd0eb3ad58e26e..850282d54c7309868e1c41c703c826a66d50ad93 100644 --- a/spec/models/concerns/limitable_spec.rb +++ b/spec/models/concerns/limitable_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' -require 'active_model' +require 'spec_helper' RSpec.describe Limitable do let(:minimal_test_class) do @@ -17,7 +16,7 @@ def self.name end before do - stub_const("MinimalTestClass", minimal_test_class) + stub_const('MinimalTestClass', minimal_test_class) end it { expect(MinimalTestClass.limit_name).to eq('test_classes') } @@ -37,25 +36,50 @@ def self.name instance.valid?(:create) end - context 'with custom relation' do - before do - MinimalTestClass.limit_relation = :custom_relation + context 'with custom relation and feature flags' do + using RSpec::Parameterized::TableSyntax + + where(:limit_feature_flag, :limit_feature_flag_value, :limit_feature_flag_for_override, :limit_feature_flag_override_value, :expect_limit_applied?) do + nil | nil | nil | nil | true + :some_feature_flag | false | nil | nil | false + :some_feature_flag | true | nil | nil | true + :some_feature_flag | true | :some_feature_flag_disable | false | true + :some_feature_flag | false | :some_feature_flag_disable | false | false + :some_feature_flag | false | :some_feature_flag_disable | true | false + :some_feature_flag | true | :some_feature_flag_disable | true | false end - it 'triggers custom limit_relation' do - instance = MinimalTestClass.new + with_them do + let(:instance) { MinimalTestClass.new } - def instance.project - @project ||= Object.new - end + before do + def instance.project + @project ||= stub_feature_flag_gate('CustomActor') + end + + stub_feature_flags("#{limit_feature_flag}": limit_feature_flag_value ? [instance.project] : false) if limit_feature_flag + stub_feature_flags("#{limit_feature_flag_for_override}": limit_feature_flag_override_value ? [instance.project] : false) if limit_feature_flag_for_override + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check - limits = Object.new - custom_relation = Object.new - expect(instance).to receive(:custom_relation).and_return(custom_relation) - expect(instance.project).to receive(:actual_limits).and_return(limits) - expect(limits).to receive(:exceeded?).with(instance.class.name.demodulize.tableize, custom_relation).and_return(false) + MinimalTestClass.limit_relation = :custom_relation + MinimalTestClass.limit_feature_flag = limit_feature_flag + MinimalTestClass.limit_feature_flag_for_override = limit_feature_flag_for_override + end - instance.valid?(:create) + it 'acts according to the feature flag settings' do + limits = Object.new + custom_relation = Object.new + if expect_limit_applied? + expect(instance).to receive(:custom_relation).and_return(custom_relation) + expect(instance.project).to receive(:actual_limits).and_return(limits) + expect(limits).to receive(:exceeded?).with(instance.class.name.demodulize.tableize, custom_relation).and_return(false) + else + expect(instance).not_to receive(:custom_relation) + end + + instance.valid?(:create) + end end end end diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb index 968cafc57d4cce8f04b5ef129a2ed43aa869b309..30028a1f1aa7259e5264f29185d6c7d6c061aa7b 100644 --- a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb +++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb @@ -194,6 +194,10 @@ class Foo < ApplicationRecord include_examples 'sets flag as used', 'self.limit_feature_flag = :foo', 'foo' end + describe 'self.limit_feature_flag_for_override = :foo' do + include_examples 'sets flag as used', 'self.limit_feature_flag_for_override = :foo', 'foo' + end + describe 'FEATURE_FLAG = :foo' do include_examples 'sets flag as used', 'FEATURE_FLAG = :foo', 'foo' end