Merge branch '273183-consolidate-namespace-storage-app-setting-checks' into 'master'

Consolidate namespace storage app setting check

See merge request gitlab-org/gitlab!46943

ee/app/helpers/ee/namespace_storage_limit_alert_helper.rb

@@ -47,8 +47,7 @@ module EE
     end
 
     def can_purchase_storage_for_namespace?(namespace)
-      ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? &&
-        ::Feature.enabled?(:buy_storage_link) &&
+      ::Feature.enabled?(:buy_storage_link) &&
         namespace.additional_repo_storage_by_namespace_enabled?
     end
 

ee/app/models/ee/namespace.rb

@@ -373,7 +373,9 @@ module EE
     end
 
     def additional_repo_storage_by_namespace_enabled?
-      !::Feature.enabled?(:namespace_storage_limit, self) && ::Feature.enabled?(:additional_repo_storage_by_namespace, self)
+      !::Feature.enabled?(:namespace_storage_limit, self) &&
+        ::Feature.enabled?(:additional_repo_storage_by_namespace, self) &&
+        ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
     end
 
     def root_storage_size

ee/app/models/ee/namespace/root_excess_storage_size.rb

@@ -32,10 +32,7 @@ module EE
     end
 
     def enforce_limit?
-      return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
-      return false unless root_namespace.additional_repo_storage_by_namespace_enabled?
-
-      true
+      root_namespace.additional_repo_storage_by_namespace_enabled?
     end
 
     private

ee/lib/ee/gitlab/repository_size_checker.rb

@@ -26,8 +26,6 @@ module EE
       private
 
       def additional_repo_storage_available?
-        return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
-
         !!namespace&.additional_repo_storage_by_namespace_enabled?
       end
 

ee/spec/controllers/groups/usage_quotas_controller_spec.rb

@@ -9,13 +9,16 @@ RSpec.describe Groups::UsageQuotasController do
   before do
     sign_in(user)
     group.add_owner(user)
+
+    allow_next_found_instance_of(Group) do |group|
+      allow(group).to receive(:additional_repo_storage_by_namespace_enabled?)
+        .and_return(additional_repo_storage_by_namespace_enabled)
+    end
   end
 
   describe 'Pushing the `additionalRepoStorageByNamespace` feature flag to the frontend' do
-    context 'when both flags are true' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: true)
-      end
+    context 'when additional_repo_storage_by_namespace_enabled is false' do
+      let(:additional_repo_storage_by_namespace_enabled) { false }
 
       it 'is disabled' do
         get :index, params: { group_id: group }
@@ -24,10 +27,8 @@ RSpec.describe Groups::UsageQuotasController do
       end
     end
 
-    context 'when `namespace_storage_limit` flag is false' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: false)
-      end
+    context 'when additional_repo_storage_by_namespace_enabled is true' do
+      let(:additional_repo_storage_by_namespace_enabled) { true }
 
       it 'is enabled' do
         get :index, params: { group_id: group }
@@ -35,17 +36,5 @@ RSpec.describe Groups::UsageQuotasController do
         expect(Gon.features).to include('additionalRepoStorageByNamespace' => true)
       end
     end
-
-    context 'when both flags are false' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: false, namespace_storage_limit: false)
-      end
-
-      it 'is disabled' do
-        get :index, params: { group_id: group }
-
-        expect(Gon.features).to include('additionalRepoStorageByNamespace' => false)
-      end
-    end
   end
 end

ee/spec/controllers/profiles/usage_quotas_controller_spec.rb

@@ -18,10 +18,15 @@ RSpec.describe Profiles::UsageQuotasController do
   end
 
   describe 'Pushing the `additionalRepoStorageByNamespace` feature flag to the frontend' do
-    context 'when both flags are true' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: true)
+    before do
+      allow_next_found_instance_of(Namespace) do |namespace|
+        allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
+          .and_return(additional_repo_storage_by_namespace_enabled)
       end
+    end
+
+    context 'when additional_repo_storage_by_namespace_enabled is false' do
+      let(:additional_repo_storage_by_namespace_enabled) { false }
 
       it 'is disabled' do
         get :index
@@ -30,10 +35,8 @@ RSpec.describe Profiles::UsageQuotasController do
       end
     end
 
-    context 'when `namespace_storage_limit` flag is false' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: true, namespace_storage_limit: false)
-      end
+    context 'when additional_repo_storage_by_namespace_enabled is true' do
+      let(:additional_repo_storage_by_namespace_enabled) { true }
 
       it 'is enabled' do
         get :index
@@ -41,17 +44,5 @@ RSpec.describe Profiles::UsageQuotasController do
         expect(Gon.features).to include('additionalRepoStorageByNamespace' => true)
       end
     end
-
-    context 'when both flags are false' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: false, namespace_storage_limit: false)
-      end
-
-      it 'is disabled' do
-        get :index
-
-        expect(Gon.features).to include('additionalRepoStorageByNamespace' => false)
-      end
-    end
   end
 end

ee/spec/helpers/ee/namespace_storage_limit_alert_helper_spec.rb

@@ -112,17 +112,15 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do
       }
     end
 
-    where(:namespace_storage_limit_enabled, :additional_repo_storage_by_namespace_enabled, :service_class_name) do
-      true  | false | Namespaces::CheckStorageSizeService
-      true  | true  | Namespaces::CheckStorageSizeService
-      false | true  | Namespaces::CheckExcessStorageSizeService
-      false | false | Namespaces::CheckStorageSizeService
+    where(:additional_repo_storage_by_namespace_enabled, :service_class_name) do
+      false | Namespaces::CheckStorageSizeService
+      true  | Namespaces::CheckExcessStorageSizeService
     end
 
     with_them do
       before do
-        stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled)
-        stub_feature_flags(additional_repo_storage_by_namespace: additional_repo_storage_by_namespace_enabled)
+        allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
+          .and_return(additional_repo_storage_by_namespace_enabled)
 
         allow(helper).to receive(:current_user).and_return(admin)
         allow_next_instance_of(service_class_name, namespace, admin) do |service|
@@ -203,28 +201,18 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do
 
     let_it_be(:namespace) { build(:namespace) }
 
-    where(
-      auto_storage_allocation_enabled: [true, false],
-      buy_storage_link_enabled: [true, false],
-      namespace_storage_limit_enabled: [true, false],
-      additional_storage_enabled: [true, false]
-    )
+    where(:buy_storage_link_enabled, :additional_repo_storage_by_namespace_enabled, :result) do
+      false | false | false
+      false | true  | false
+      true  | false | false
+      true  | true  | true
+    end
 
     with_them do
-      let(:result) do
-        auto_storage_allocation_enabled &&
-          buy_storage_link_enabled &&
-          !namespace_storage_limit_enabled &&
-          additional_storage_enabled
-      end
-
       before do
-        stub_application_setting(automatic_purchased_storage_allocation: auto_storage_allocation_enabled)
-        stub_feature_flags(
-          namespace_storage_limit: namespace_storage_limit_enabled,
-          additional_repo_storage_by_namespace: additional_storage_enabled,
-          buy_storage_link: buy_storage_link_enabled
-        )
+        stub_feature_flags(buy_storage_link: buy_storage_link_enabled)
+        allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
+          .and_return(additional_repo_storage_by_namespace_enabled)
       end
 
       it { is_expected.to eq(result) }

ee/spec/models/ee/namespace/root_excess_storage_size_spec.rb

@@ -16,40 +16,26 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do
     subject { model.above_size_limit? }
 
     before do
-      allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled }
+      allow(model).to receive(:enforce_limit?) { enforce_limit }
     end
 
     context 'when limit enforcement is off' do
-      let(:storage_allocation_enabled) { false }
+      let(:enforce_limit) { false }
 
       it { is_expected.to eq(false) }
     end
 
     context 'when limit enforcement is on' do
-      let(:storage_allocation_enabled) { true }
+      let(:enforce_limit) { true }
 
-      context 'with feature flag :namespace_storage_limit disabled' do
-        before do
-          stub_feature_flags(namespace_storage_limit: false)
-        end
-
-        context 'when below limit' do
-          it { is_expected.to eq(false) }
-        end
-
-        context 'when above limit' do
-          let(:total_repository_size_excess) { 101.megabytes }
-
-          it { is_expected.to eq(true) }
-        end
+      context 'when below limit' do
+        it { is_expected.to eq(false) }
       end
 
-      context 'with feature flag :additional_repo_storage_by_namespace disabled' do
-        before do
-          stub_feature_flags(additional_repo_storage_by_namespace: false)
-        end
+      context 'when above limit' do
+        let(:total_repository_size_excess) { 101.megabytes }
 
-        it { is_expected.to eq(false) }
+        it { is_expected.to eq(true) }
       end
     end
   end
@@ -99,36 +85,21 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do
   describe '#enforce_limit?' do
     subject { model.enforce_limit? }
 
-    let(:storage_allocation_enabled) { true }
-
     before do
-      allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled }
+      allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
+        .and_return(additional_repo_storage_by_namespace_enabled)
     end
 
-    context 'with application setting is disabled' do
-      let(:storage_allocation_enabled) { false }
+    context 'when additional_repo_storage_by_namespace_enabled is false' do
+      let(:additional_repo_storage_by_namespace_enabled) { false }
 
       it { is_expected.to eq(false) }
     end
 
-    context 'with feature flags (:namespace_storage_limit & :additional_repo_storage_by_namespace) enabled' do
-      it { is_expected.to eq(false) }
-    end
-
     context 'with feature flag :namespace_storage_limit disabled' do
-      before do
-        stub_feature_flags(namespace_storage_limit: false)
-      end
+      let(:additional_repo_storage_by_namespace_enabled) { true }
 
       it { is_expected.to eq(true) }
     end
-
-    context 'with feature flag :additional_repo_storage_by_namespace disabled' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: false)
-      end
-
-      it { is_expected.to eq(false) }
-    end
   end
 end

ee/spec/models/ee/namespace_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Namespace do
+  using RSpec::Parameterized::TableSyntax
+
   include EE::GeoHelpers
 
   let(:namespace) { create(:namespace) }
@@ -1770,12 +1772,48 @@ RSpec.describe Namespace do
     end
   end
 
+  describe '#additional_repo_storage_by_namespace_enabled?' do
+    let_it_be(:namespace) { build(:namespace) }
+
+    subject { namespace.additional_repo_storage_by_namespace_enabled? }
+
+    where(:namespace_storage_limit, :additional_repo_storage_by_namespace, :automatic_purchased_storage_allocation, :result) do
+      false | false | false | false
+      false | false | true  | false
+      false | true  | false | false
+      true  | false | false | false
+      false | true  | true  | true
+      true  | true  | false | false
+      true  | false | true  | false
+      true  | true  | true  | false
+    end
+
+    with_them do
+      before do
+        stub_feature_flags(
+          namespace_storage_limit: namespace_storage_limit,
+          additional_repo_storage_by_namespace: additional_repo_storage_by_namespace
+        )
+        stub_application_setting(automatic_purchased_storage_allocation: automatic_purchased_storage_allocation)
+      end
+
+      it { is_expected.to eq(result) }
+    end
+  end
+
   describe '#root_storage_size' do
     let_it_be(:namespace) { build(:namespace) }
 
     subject { namespace.root_storage_size }
 
-    context 'with feature flag :namespace_storage_limit enabled' do
+    before do
+      allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
+        .and_return(additional_repo_storage_by_namespace_enabled)
+    end
+
+    context 'when additional_repo_storage_by_namespace_enabled is false' do
+      let(:additional_repo_storage_by_namespace_enabled) { false }
+
       it 'initializes a new instance of EE::Namespace::RootStorageSize' do
         expect(EE::Namespace::RootStorageSize).to receive(:new).with(namespace)
 
@@ -1783,10 +1821,8 @@ RSpec.describe Namespace do
       end
     end
 
-    context 'with feature flag :namespace_storage_limit disabled' do
-      before do
-        stub_feature_flags(namespace_storage_limit: false)
-      end
+    context 'when additional_repo_storage_by_namespace_enabled is true' do
+      let(:additional_repo_storage_by_namespace_enabled) { true }
 
       it 'initializes a new instance of EE::Namespace::RootExcessStorageSize' do
         expect(EE::Namespace::RootExcessStorageSize).to receive(:new).with(namespace)

ee/spec/services/ee/post_receive_service_spec.rb

@@ -108,17 +108,15 @@ RSpec.describe PostReceiveService, :geo do
 
     let(:check_storage_size_response) { ServiceResponse.success }
 
-    where(:namespace_storage_limit_enabled, :additional_repo_storage_by_namespace_enabled, :service_class_name) do
-      true  | false | Namespaces::CheckStorageSizeService
-      true  | true  | Namespaces::CheckStorageSizeService
-      false | true  | Namespaces::CheckExcessStorageSizeService
-      false | false | Namespaces::CheckStorageSizeService
+    where(:additional_repo_storage_by_namespace_enabled, :service_class_name) do
+      true  | Namespaces::CheckExcessStorageSizeService
+      false | Namespaces::CheckStorageSizeService
     end
 
     with_them do
       before do
-        stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled)
-        stub_feature_flags(additional_repo_storage_by_namespace: additional_repo_storage_by_namespace_enabled)
+        allow(project.namespace).to receive(:additional_repo_storage_by_namespace_enabled?)
+          .and_return(additional_repo_storage_by_namespace_enabled)
 
         allow_next_instance_of(service_class_name, project.namespace, user) do |service|
           expect(service).to receive(:execute).and_return(check_storage_size_response)

ee/spec/services/namespaces/check_excess_storage_size_service_spec.rb

@@ -8,43 +8,24 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do
   let(:service) { described_class.new(namespace, user) }
   let(:total_repository_size_excess) { 150.megabytes }
   let(:additional_purchased_storage_size) { 100 }
-  let(:namespace_storage_limit_enabled) { false }
-  let(:storage_allocation_enabled) { true }
+  let(:additional_repo_storage_by_namespace_enabled) { true }
   let(:actual_size_limit) { 10.gigabytes }
   let(:locked_project_count) { 1 }
 
   subject(:response) { service.execute }
 
   before do
-    stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled)
-    allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled }
-
+    allow(namespace).to receive(:additional_repo_storage_by_namespace_enabled?).and_return(additional_repo_storage_by_namespace_enabled)
     allow(namespace).to receive(:root_ancestor).and_return(namespace)
     allow(namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess)
     allow(namespace).to receive(:actual_size_limit).and_return(actual_size_limit)
     allow(namespace).to receive(:repository_size_excess_project_count).and_return(locked_project_count)
   end
 
-  context 'without limit enforcement' do
-    context 'with application setting disabled' do
-      let(:storage_allocation_enabled) { false }
-
-      it { is_expected.to be_success }
-    end
-
-    context 'with feature flag :additional_repo_storage_by_namespace disabled' do
-      before do
-        stub_feature_flags(additional_repo_storage_by_namespace: false)
-      end
-
-      it { is_expected.to be_success }
-    end
+  context 'when additional_repo_storage_by_namespace_enabled is false' do
+    let(:additional_repo_storage_by_namespace_enabled) { false }
 
-    context 'with feature flag :namespace_storage_limit enabled' do
-      let(:namespace_storage_limit_enabled) { true }
-
-      it { is_expected.to be_success }
-    end
+    it { is_expected.to be_success }
   end
 
   context 'when additional_purchased_storage_size is set to 0' do
@@ -77,14 +58,7 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do
   end
 
   context 'when not admin of the namespace' do
-    let(:other_namespace) { build(:namespace, additional_purchased_storage_size: additional_purchased_storage_size) }
-
-    subject(:response) { described_class.new(other_namespace, user).execute }
-
-    before do
-      allow(other_namespace).to receive(:root_ancestor).and_return(other_namespace)
-      allow(other_namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess)
-    end
+    let(:user) { build(:user) }
 
     it 'errors and has no payload' do
       expect(response).to be_error