diff --git a/ee/app/controllers/groups/usage_quotas_controller.rb b/ee/app/controllers/groups/usage_quotas_controller.rb index ba24a196870cf709fc39a9e544015dad29a4a0a7..7edfd1c5a361b56459b5d61e53b4aa33d994e691 100644 --- a/ee/app/controllers/groups/usage_quotas_controller.rb +++ b/ee/app/controllers/groups/usage_quotas_controller.rb @@ -19,7 +19,7 @@ def index end def pending_members - render_404 unless ::Feature.enabled?(:saas_user_caps, @group.root_ancestor, default_enabled: :yaml) + render_404 unless @group.user_cap_available? @hide_search_settings = true end diff --git a/ee/app/helpers/ee/namespace_user_cap_reached_alert_helper.rb b/ee/app/helpers/ee/namespace_user_cap_reached_alert_helper.rb index 5a212b6838b66af2d72886548fc22617186e0e8b..b5074cef362afacada81ec024aa44194023baf37 100644 --- a/ee/app/helpers/ee/namespace_user_cap_reached_alert_helper.rb +++ b/ee/app/helpers/ee/namespace_user_cap_reached_alert_helper.rb @@ -4,10 +4,8 @@ module EE module NamespaceUserCapReachedAlertHelper def display_namespace_user_cap_reached_alert?(namespace) root_namespace = namespace.root_ancestor - return false unless ::Feature.enabled?(:saas_user_caps, root_namespace, default_enabled: :yaml) - - return false if root_namespace.user_namespace? + return false unless root_namespace.user_cap_available? return false if alert_has_been_dismissed?(root_namespace) can?(current_user, :admin_namespace, root_namespace) && root_namespace.user_cap_reached?(use_cache: true) diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 2252644a114f14eb7ed6033184774d832baa5c11..4197df01429efcce56eae8eab3d1deb829fc1862 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -512,7 +512,7 @@ def iteration_cadences_feature_flag_enabled? end def user_cap_reached?(use_cache: false) - return false unless ::Feature.enabled?(:saas_user_caps, root_ancestor, default_enabled: :yaml) + return false unless user_cap_available? user_cap = root_ancestor.namespace_settings&.new_user_signups_cap return false unless user_cap diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index f09a8626d69d8ac30ba77e0239052f82616a044d..c95ae4db21f667ea9f5fdbcbc755ae976d8b82ae 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -447,6 +447,13 @@ def root_storage_size klass.new(self) end + def user_cap_available? + return false unless group_namespace? + return false unless ::Gitlab.com? + + ::Feature.enabled?(:saas_user_caps, root_ancestor, default_enabled: :yaml) + end + private def any_project_with_shared_runners_enabled_with_cte? diff --git a/ee/app/views/groups/_user_caps_setting.html.haml b/ee/app/views/groups/_user_caps_setting.html.haml index 52a38db98e5cccbaa1d2556129e9a3121e1a7444..bca74c98e5838cced98f743ca3204fe2c260d07b 100644 --- a/ee/app/views/groups/_user_caps_setting.html.haml +++ b/ee/app/views/groups/_user_caps_setting.html.haml @@ -1,4 +1,4 @@ -- return unless Feature.enabled?(:saas_user_caps, group.root_ancestor) +- return unless group.user_cap_available? .form-group = f.label :new_user_signups_cap, _('User cap'), class: 'gl-font-weight-bold' diff --git a/ee/app/views/groups/usage_quotas/index.html.haml b/ee/app/views/groups/usage_quotas/index.html.haml index a36bb6a0b874dd7cb49046de941900be8c0acb0e..541a725aac717309baf67693e446eb1b72fbddd9 100644 --- a/ee/app/views/groups/usage_quotas/index.html.haml +++ b/ee/app/views/groups/usage_quotas/index.html.haml @@ -1,6 +1,6 @@ - page_title s_("UsageQuota|Usage") - url_to_purchase_storage = buy_storage_path(@group) if purchase_storage_link_enabled?(@group) -- pending_members_page_path = pending_members_group_usage_quotas_path(@group) if Feature.enabled?(:saas_user_caps, @group.root_ancestor) +- pending_members_page_path = pending_members_group_usage_quotas_path(@group) if @group.user_cap_available? - pending_members_count = Member.in_hierarchy(@group).with_state("awaiting").count - if show_product_purchase_success_alert? diff --git a/ee/spec/controllers/groups/usage_quotas_controller_spec.rb b/ee/spec/controllers/groups/usage_quotas_controller_spec.rb index a487b0404e79cac1a5981e9ff7b94a2ad97de5e3..3c14fd3a27fb3f7be887acedf584135b4a9cfa27 100644 --- a/ee/spec/controllers/groups/usage_quotas_controller_spec.rb +++ b/ee/spec/controllers/groups/usage_quotas_controller_spec.rb @@ -37,4 +37,30 @@ end end end + + describe 'GET #pending_members' do + let(:feature_available) { true } + + before do + allow_next_found_instance_of(Group) do |group| + allow(group).to receive(:user_cap_available?).and_return(feature_available) + end + end + + it 'renders the pending members index' do + get :pending_members, params: { group_id: group } + + expect(response).to render_template 'groups/usage_quotas/pending_members' + end + + context 'when user cap feature is unavailable' do + let(:feature_available) { false } + + it 'returns 404' do + get :pending_members, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end diff --git a/ee/spec/features/groups/group_settings_spec.rb b/ee/spec/features/groups/group_settings_spec.rb index ad63c51ece27c6f40223cded036bace18bcb1f36..599fc83daafc52348bd959e0ecf89acedf8582b7 100644 --- a/ee/spec/features/groups/group_settings_spec.rb +++ b/ee/spec/features/groups/group_settings_spec.rb @@ -409,9 +409,18 @@ def update_email_domains(new_domain) end describe 'user caps settings' do - context 'when :saas_user_caps feature flag is off' do + let(:user_cap_available) { true } + + before do + allow_next_found_instance_of(Group) do |instance| + allow(instance).to receive(:user_cap_available?).and_return user_cap_available + end + end + + context 'when user cap feature is unavailable' do + let(:user_cap_available) { false } + before do - stub_feature_flags(saas_user_caps: false) visit edit_group_path(group) end @@ -420,11 +429,10 @@ def update_email_domains(new_domain) end end - context 'when :saas_user_caps feature flag is on', :js do + context 'when user cap feature is available', :js do let(:user_caps_selector) { '[name="group[new_user_signups_cap]"]' } before do - stub_feature_flags(saas_user_caps: true) visit edit_group_path(group) end @@ -479,7 +487,6 @@ def fill_in_new_user_signups_cap(new_user_signups_cap_value) end before do - stub_feature_flags(saas_user_caps: true) group.namespace_settings.update!(new_user_signups_cap: group.group_members.count) end diff --git a/ee/spec/features/namespace_user_cap_reached_alert_spec.rb b/ee/spec/features/namespace_user_cap_reached_alert_spec.rb index eac13c9158641b31d0b09ed437ea1cf52ba6235f..76ea94688f47f8e62e1174d4924bc98181ba3541 100644 --- a/ee/spec/features/namespace_user_cap_reached_alert_spec.rb +++ b/ee/spec/features/namespace_user_cap_reached_alert_spec.rb @@ -24,6 +24,8 @@ context 'with an exceeded user cap' do before do + allow(Gitlab).to receive(:com?).and_return(true) + stub_cache(group) end diff --git a/ee/spec/helpers/ee/namespace_user_cap_reached_alert_helper_spec.rb b/ee/spec/helpers/ee/namespace_user_cap_reached_alert_helper_spec.rb index 9a2d64e8543a278954694bda8e80c67565fdfcf9..19d6d5469d5dd3172808ef8ee80cc19c17780999 100644 --- a/ee/spec/helpers/ee/namespace_user_cap_reached_alert_helper_spec.rb +++ b/ee/spec/helpers/ee/namespace_user_cap_reached_alert_helper_spec.rb @@ -23,19 +23,23 @@ before do allow(helper).to receive(:can?).with(owner, :admin_namespace, group).and_return(true) allow(helper).to receive(:can?).with(developer, :admin_namespace, group).and_return(false) + allow(group).to receive(:user_cap_available?).and_return(true) + stub_cache(group) end + subject(:display_alert?) { helper.display_namespace_user_cap_reached_alert?(group) } + it 'returns true when the user cap is reached for a user who can admin the namespace' do sign_in(owner) - expect(helper.display_namespace_user_cap_reached_alert?(group)).to be true + expect(display_alert?).to be true end it 'returns false when the user cap is reached for a user who cannot admin the namespace' do sign_in(developer) - expect(helper.display_namespace_user_cap_reached_alert?(group)).to be false + expect(display_alert?).to be false end it 'does not trigger reactive caching if there is no user cap set' do @@ -44,7 +48,15 @@ sign_in(owner) expect(group).not_to receive(:with_reactive_cache) - expect(helper.display_namespace_user_cap_reached_alert?(group)).to be false + expect(display_alert?).to be false + end + + it 'returns false when the user cap feature is unavailable' do + allow(group).to receive(:user_cap_available?).and_return(false) + + sign_in(owner) + + expect(display_alert?).to be false end def sign_in(user) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 5ee6bc15c74ad6c3262c77d2edd6c465090a434b..18bb960f1e451fdfda8aa2551cceba87e02a9e36 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -1495,63 +1495,77 @@ def webhook_headers describe '#user_cap_reached?' do subject(:user_cap_reached_for_group?) { group.user_cap_reached? } - context 'when the :saas_user_caps feature flag is not enabled' do + context 'when user cap feature is not available' do + before do + allow(group).to receive(:user_cap_available?).and_return(false) + end + it { is_expected.to be_falsey } end - context 'when the :saas_user_caps feature flag is enabled' do + context 'when user cap feature is available' do before do - stub_feature_flags(saas_user_caps: true) + allow(group).to receive(:user_cap_available?).and_return(true) end - let(:new_user_signups_cap) { nil } + context 'when the :saas_user_caps feature flag is not enabled' do + it { is_expected.to be_falsey } + end - shared_context 'returning the right value for user_cap_reached?' do + context 'when the :saas_user_caps feature flag is enabled' do before do - root_group.namespace_settings.update!(new_user_signups_cap: new_user_signups_cap) + stub_feature_flags(saas_user_caps: true) end - context 'when no user cap has been set to that root ancestor' do - it { is_expected.to be_falsey } - end - - context 'when a user cap has been set to that root ancestor' do - let(:new_user_signups_cap) { 100 } + let(:new_user_signups_cap) { nil } + shared_examples 'returning the right value for user_cap_reached?' do before do - allow(root_group).to receive(:billable_members_count).and_return(billable_members_count) - allow(group).to receive(:root_ancestor).and_return(root_group) + root_group.namespace_settings.update!(new_user_signups_cap: new_user_signups_cap) end - context 'when this cap is higher than the number of billable members' do - let(:billable_members_count) { new_user_signups_cap - 10 } - + context 'when no user cap has been set to that root ancestor' do it { is_expected.to be_falsey } end - context 'when this cap is the same as the number of billable members' do - let(:billable_members_count) { new_user_signups_cap } + context 'when a user cap has been set to that root ancestor' do + let(:new_user_signups_cap) { 100 } - it { is_expected.to be_truthy } - end + before do + allow(root_group).to receive(:billable_members_count).and_return(billable_members_count) + allow(group).to receive(:root_ancestor).and_return(root_group) + end - context 'when this cap is lower than the number of billable members' do - let(:billable_members_count) { new_user_signups_cap + 10 } + context 'when this cap is higher than the number of billable members' do + let(:billable_members_count) { new_user_signups_cap - 10 } - it { is_expected.to be_truthy } + it { is_expected.to be_falsey } + end + + context 'when this cap is the same as the number of billable members' do + let(:billable_members_count) { new_user_signups_cap } + + it { is_expected.to be_truthy } + end + + context 'when this cap is lower than the number of billable members' do + let(:billable_members_count) { new_user_signups_cap + 10 } + + it { is_expected.to be_truthy } + end end end - end - context 'when this group has no root ancestor' do - it_behaves_like 'returning the right value for user_cap_reached?' do - let(:root_group) { group } + context 'when this group has no root ancestor' do + it_behaves_like 'returning the right value for user_cap_reached?' do + let(:root_group) { group } + end end - end - context 'when this group has a root ancestor' do - it_behaves_like 'returning the right value for user_cap_reached?' do - let(:root_group) { create(:group, children: [group]) } + context 'when this group has a root ancestor' do + it_behaves_like 'returning the right value for user_cap_reached?' do + let(:root_group) { create(:group, children: [group]) } + end end end end diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index df39de2ab0b7a373b7c436b50c161a916729981b..922c429ab4a8bef2b72a480f0de44c3faca1042a 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -1960,6 +1960,48 @@ end end + describe '#user_cap_available?' do + let_it_be(:namespace) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: namespace) } + + let(:gitlab_com?) { true } + + subject(:user_cap_available?) { namespace.user_cap_available? } + + before do + allow(::Gitlab).to receive(:com?).and_return(gitlab_com?) + end + + context 'when not on Gitlab.com' do + let(:gitlab_com?) { false } + + it { is_expected.to be false } + end + + context 'when :saas_user_caps is disabled' do + before do + stub_feature_flags(saas_user_caps: false) + end + + it { is_expected.to be false } + end + + context 'when :saas_user_caps is enabled' do + before do + stub_feature_flags(saas_user_caps: true) + end + + it { is_expected.to be true } + + context 'when the namespace is not a group' do + let(:user) { create(:user) } + let(:namespace) { user.namespace } + + it { is_expected.to be false } + end + end + end + def create_project(repository_size:, lfs_objects_size:, repository_size_limit:) create(:project, namespace: namespace, repository_size_limit: repository_size_limit).tap do |project| create(:project_statistics, project: project, repository_size: repository_size, lfs_objects_size: lfs_objects_size) diff --git a/ee/spec/views/shared/_namespace_user_cap_reached_alert.html.haml_spec.rb b/ee/spec/views/shared/_namespace_user_cap_reached_alert.html.haml_spec.rb index a5bf26cebf8e4b5cd1a21c68752b57e01125d064..6818067fa8194dcd3d040e243c50906534954de9 100644 --- a/ee/spec/views/shared/_namespace_user_cap_reached_alert.html.haml_spec.rb +++ b/ee/spec/views/shared/_namespace_user_cap_reached_alert.html.haml_spec.rb @@ -20,6 +20,7 @@ before do allow(view).to receive(:current_user).and_return(owner) + allow(Gitlab).to receive(:com?).and_return(true) stub_cache(group) stub_cache(other_group) end