diff --git a/app/models/namespace/detail.rb b/app/models/namespace/detail.rb index a5643ab9f790d81dd4b6e637bbcf05cb7869be9c..2660d11171e89248939fa576b71d877bb1194427 100644 --- a/app/models/namespace/detail.rb +++ b/app/models/namespace/detail.rb @@ -11,3 +11,5 @@ class Namespace::Detail < ApplicationRecord self.primary_key = :namespace_id end + +Namespace::Detail.prepend_mod diff --git a/config/feature_flags/development/free_user_cap_over_user_limit_mails.yml b/config/feature_flags/development/free_user_cap_over_user_limit_mails.yml new file mode 100644 index 0000000000000000000000000000000000000000..88b3f59050d1c611e72724f030d58a32413d2136 --- /dev/null +++ b/config/feature_flags/development/free_user_cap_over_user_limit_mails.yml @@ -0,0 +1,8 @@ +--- +name: free_user_cap_over_user_limit_mails +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98438 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/378616 +milestone: '15.9' +type: development +group: group::acquisition +default_enabled: false diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 067c4730f0adac7852c6e7f9cfbc7d002068f18a..433554b7a5afcfcc24a5b3c4590b61e85046d26b 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -335,6 +335,8 @@ - 1 - - migrate_external_diffs - 1 +- - namespaces_free_user_cap_over_limit_notification + - 1 - - namespaces_process_sync_events - 1 - - namespaces_sync_namespace_name diff --git a/db/migrate/20221219112632_add_next_over_limit_check_at_to_namespace_details.rb b/db/migrate/20221219112632_add_next_over_limit_check_at_to_namespace_details.rb new file mode 100644 index 0000000000000000000000000000000000000000..dd2acbfd0bbf7e1ff251922ddbee17f4f1f986b6 --- /dev/null +++ b/db/migrate/20221219112632_add_next_over_limit_check_at_to_namespace_details.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddNextOverLimitCheckAtToNamespaceDetails < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + TABLE_NAME = :namespace_details + COLUMN = :next_over_limit_check_at + + def up + with_lock_retries do + add_column TABLE_NAME, COLUMN, :datetime_with_timezone, null: true + end + end + + def down + with_lock_retries do + remove_column TABLE_NAME, COLUMN + end + end +end diff --git a/db/schema_migrations/20221219112632 b/db/schema_migrations/20221219112632 new file mode 100644 index 0000000000000000000000000000000000000000..0bba0080af799de2c4fc2037e4777d4d7eaabdec --- /dev/null +++ b/db/schema_migrations/20221219112632 @@ -0,0 +1 @@ +400cab0a2d3130dd7406024cf982c7312918019197ae06af06696435f6bb5aaa \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c1a1ac21349722bea6f96e82839932cdcc26165d..bb9ea21db9ba5d73d78efc5f7677a0524f4531d5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18279,7 +18279,8 @@ CREATE TABLE namespace_details ( free_user_cap_over_limt_notified_at timestamp with time zone, free_user_cap_over_limit_notified_at timestamp with time zone, dashboard_notification_at timestamp with time zone, - dashboard_enforcement_at timestamp with time zone + dashboard_enforcement_at timestamp with time zone, + next_over_limit_check_at timestamp with time zone ); CREATE TABLE namespace_limits ( diff --git a/ee/app/finders/namespaces/free_user_cap/enforceable_groups_finder.rb b/ee/app/finders/namespaces/free_user_cap/enforceable_groups_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..895d9e5f6f490a505447b183a4b0577705b5d1c9 --- /dev/null +++ b/ee/app/finders/namespaces/free_user_cap/enforceable_groups_finder.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Namespaces + module FreeUserCap + class EnforceableGroupsFinder + def execute + Group + .in_default_plan + .top_most + .non_public_only + end + end + end +end diff --git a/ee/app/mailers/ee/preview/notify_preview.rb b/ee/app/mailers/ee/preview/notify_preview.rb index 590e4ce1c091549593edbf1feb9c477a2b5b868c..605d39aa46120361d0746e6fc881cd3f928ee737 100644 --- a/ee/app/mailers/ee/preview/notify_preview.rb +++ b/ee/app/mailers/ee/preview/notify_preview.rb @@ -3,6 +3,7 @@ module EE module Preview module NotifyPreview + FREE_USER_CAP_OVERLIMIT_CHECKED_AT = "2022-11-07 14:33:44 UTC" extend ActiveSupport::Concern # We need to define the methods on the prepender directly because: @@ -80,8 +81,8 @@ def user_cap_reached ::Notify.user_cap_reached(user.id).message end - def reached_free_user_limit_email - ::Notify.reached_free_user_limit_email(user, group).message + def over_free_user_limit_email + ::Notify.over_free_user_limit_email(user, group, FREE_USER_CAP_OVERLIMIT_CHECKED_AT).message end def confirmation_instructions_email diff --git a/ee/app/mailers/emails/free_user_cap.rb b/ee/app/mailers/emails/free_user_cap.rb index 85a20c445c8c7ee9fa27dc1bb1a67cc60ef8694c..0c814694c2f79f70600701a467411f973de70f64 100644 --- a/ee/app/mailers/emails/free_user_cap.rb +++ b/ee/app/mailers/emails/free_user_cap.rb @@ -2,13 +2,14 @@ module Emails module FreeUserCap - def reached_free_user_limit_email(user, namespace) + def over_free_user_limit_email(user, namespace, checked_at) email = user.notification_email_or_default @start_trial_url = new_trial_url @upgrade_url = group_billings_url namespace @manage_users_url = group_usage_quotas_url namespace, anchor: 'seats-quota-tab' @namespace_name = namespace.name + @checked_at = Time.zone.parse checked_at.to_s email_with_layout( to: email, diff --git a/ee/app/models/ee/namespace/detail.rb b/ee/app/models/ee/namespace/detail.rb new file mode 100644 index 0000000000000000000000000000000000000000..2609fdc7bb22f13da63c6fa84fce2cea66b0252d --- /dev/null +++ b/ee/app/models/ee/namespace/detail.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module EE + module Namespace + module Detail + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + prepended do + scope :not_over_limit_notified, -> { where free_user_cap_over_limit_notified_at: nil } + + scope :scheduled_for_over_limit_check, -> do + where(next_over_limit_check_at: ..Time.current) + .or(where(next_over_limit_check_at: nil)) + .order(arel_table[:next_over_limit_check_at].asc.nulls_first) + end + + scope :lock_for_over_limit_check, ->(limit, namespace_ids) do + scheduled_for_over_limit_check + .where(namespace_id: namespace_ids) + .limit(limit) + .lock('FOR UPDATE SKIP LOCKED') + end + end + end + end +end diff --git a/ee/app/models/namespaces/free_user_cap.rb b/ee/app/models/namespaces/free_user_cap.rb index c1e183acd79dec8418debc56c1f9b5dfb562fd66..a0e587ac3dcdf7e5016fe313726bec032534182b 100644 --- a/ee/app/models/namespaces/free_user_cap.rb +++ b/ee/app/models/namespaces/free_user_cap.rb @@ -14,6 +14,10 @@ def self.notification_or_enforcement_enabled?(namespace) ::Namespaces::FreeUserCap::Enforcement.new(namespace).enforce_cap? end + def self.over_user_limit_mails_enabled? + ::Feature.enabled?(:free_user_cap_over_user_limit_mails) + end + def self.dashboard_limit ::Gitlab::CurrentSettings.dashboard_limit end diff --git a/ee/app/services/namespaces/free_user_cap/notify_over_limit_groups_service.rb b/ee/app/services/namespaces/free_user_cap/notify_over_limit_groups_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..50821e05ce0dd82de53dc3f3af9f2e77b64c6e00 --- /dev/null +++ b/ee/app/services/namespaces/free_user_cap/notify_over_limit_groups_service.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Namespaces + module FreeUserCap + class NotifyOverLimitGroupsService + ServiceError = Class.new(StandardError) + + attr_accessor :group + + def self.execute(group:) + new(group: group).execute + end + + def initialize(group:) + @group = group.root_ancestor + end + + def execute + return unless enforce_over_limit_mails? + return unless over_limit? + + notify + + ServiceResponse.success + rescue StandardError => e + ServiceResponse.error message: e.message + end + + private + + def over_limit? + Namespaces::FreeUserCap::Enforcement.new(@group).over_limit? + end + + def notify + email_owners + mark_group_as_notified! + end + + def checked_at + @checked_at ||= Time.current + end + + def enforce_over_limit_mails? + ::Namespaces::FreeUserCap.over_user_limit_mails_enabled? && ::Gitlab::CurrentSettings.dashboard_limit_enabled? + end + + def email_owners + @group.owners.each { |owner| ::Notify.over_free_user_limit_email(owner, @group, checked_at).deliver_now } + end + + def mark_group_as_notified! + @group.namespace_details.update free_user_cap_over_limit_notified_at: checked_at + end + end + end +end diff --git a/ee/app/views/notify/reached_free_user_limit_email.html.haml b/ee/app/views/notify/over_free_user_limit_email.html.haml similarity index 72% rename from ee/app/views/notify/reached_free_user_limit_email.html.haml rename to ee/app/views/notify/over_free_user_limit_email.html.haml index fcb7c431f03866d54d22408169db7279f0fbba50..6e4ed090705810499bfda3db1b74fa41ac40462c 100644 --- a/ee/app/views/notify/reached_free_user_limit_email.html.haml +++ b/ee/app/views/notify/over_free_user_limit_email.html.haml @@ -13,7 +13,7 @@ = s_("FreeUserCap|You've reached your member limit!") %p.spaced-lines - = s_("FreeUserCap|Looks like you've reached your limit of %{free_user_limit} members for \"%{namespace_name}\". You can't add any more, but you can manage your existing members, for example, by removing inactive members and replacing them with new members.") % { namespace_name: @namespace_name, free_user_limit: Namespaces::FreeUserCap.dashboard_limit } + = s_("FreeUserCap|It looks like you've reached your limit of %{free_user_limit} members for \"%{namespace_name}\", according to the check we ran on %{date_time}. You can't add any more, but you can manage your existing members, for example, by removing inactive members and replacing them with new members.") % { namespace_name: @namespace_name, free_user_limit: Namespaces::FreeUserCap.dashboard_limit, date_time: l(@checked_at, format: :long) } %p.spaced-lines{ style: 'margin-bottom: 24px;' } - end_link = "".html_safe diff --git a/ee/app/views/notify/over_free_user_limit_email.text.erb b/ee/app/views/notify/over_free_user_limit_email.text.erb new file mode 100644 index 0000000000000000000000000000000000000000..bb1b8864a44f9ead20663643ec0db67f46260054 --- /dev/null +++ b/ee/app/views/notify/over_free_user_limit_email.text.erb @@ -0,0 +1,16 @@ +<%= s_("FreeUserCap|You've reached your member limit!") %> + +<%= s_("FreeUserCap|It looks like you've reached your limit of %{free_user_limit} members for \"%{namespace_name}\", according to the check we ran on %{date_time}. You can't add any more, but you can manage your existing members, for example, by removing inactive members and replacing them with new members.") % { namespace_name: @namespace_name, free_user_limit: Namespaces::FreeUserCap.dashboard_limit, date_time: l(@checked_at, format: :long) } %> + +<%= s_("FreeUserCap|To get more members start a trial:") %> +<%= @start_trial_url %> + +<%= s_("FreeUserCap|Alternatively you can upgrade to GitLab Premium or GitLab Ultimate:") %> +<%= @upgrade_url %> + +<%= s_("FreeUserCap|Manage members:") %> +<%= @manage_users_url %> + +<%= s_("FreeUserCap|Explore paid plans:") %> +<%= @upgrade_url %> + diff --git a/ee/app/views/notify/reached_free_user_limit_email.text.erb b/ee/app/views/notify/reached_free_user_limit_email.text.erb deleted file mode 100644 index 515bd65951faebc1f59f8095d3b1c01f4bfc4985..0000000000000000000000000000000000000000 --- a/ee/app/views/notify/reached_free_user_limit_email.text.erb +++ /dev/null @@ -1,16 +0,0 @@ -<%= s_("FreeUserCap|You've reached your member limit!") %> - -<%= s_("FreeUserCap|Looks like you've reached your limit of %{free_user_limit} members for \"%{namespace_name}\". You can't add any more, but you can manage your existing members, for example, by removing inactive members and replacing them with new members.") % { namespace_name: @namespace_name, free_user_limit: Namespaces::FreeUserCap.dashboard_limit } %> - -<%= s_("FreeUserCap|To get more members start a trial:") %> -<%= @start_trial_url %> - -<%= s_("FreeUserCap|Alternatively you can upgrade to GitLab Premium or GitLab Ultimate:") %> -<%= @upgrade_url %> - -<%= s_("FreeUserCap|Manage members:") %> -<%= @manage_users_url %> - -<%= s_("FreeUserCap|Explore paid plans:") %> -<%= @upgrade_url %> - diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 70f00ae29d1512d5696fb43360bab2194aed5470..b376c660dbff1055d928d24dd2e23e64a971f21e 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -426,6 +426,15 @@ :weight: 1 :idempotent: false :tags: [] +- :name: cronjob:namespaces_free_user_cap_backfill_notification_jobs + :worker_name: Namespaces::FreeUserCap::BackfillNotificationJobsWorker + :feature_category: :user_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: cronjob:projects_disable_legacy_open_source_license_for_inactive_projects :worker_name: Projects::DisableLegacyOpenSourceLicenseForInactiveProjectsWorker :feature_category: :projects @@ -1380,6 +1389,15 @@ :weight: 1 :idempotent: true :tags: [] +- :name: namespaces_free_user_cap_over_limit_notification + :worker_name: Namespaces::FreeUserCap::OverLimitNotificationWorker + :feature_category: :user_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: namespaces_sync_namespace_name :worker_name: Namespaces::SyncNamespaceNameWorker :feature_category: :saas_provisioning diff --git a/ee/app/workers/namespaces/free_user_cap/backfill_notification_jobs_worker.rb b/ee/app/workers/namespaces/free_user_cap/backfill_notification_jobs_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..4d700029964a1c57e1314427f24598db4947a938 --- /dev/null +++ b/ee/app/workers/namespaces/free_user_cap/backfill_notification_jobs_worker.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Namespaces + module FreeUserCap + class BackfillNotificationJobsWorker + include ApplicationWorker + + # rubocop:disable Scalability/CronWorkerContext + # This worker does not perform work scoped to a context + include CronjobQueue + # rubocop:enable Scalability/CronWorkerContext + + feature_category :user_management + urgency :low + data_consistency :always + idempotent! + + def perform(...) + OverLimitNotificationWorker.perform_with_capacity(...) + end + end + end +end diff --git a/ee/app/workers/namespaces/free_user_cap/over_limit_notification_worker.rb b/ee/app/workers/namespaces/free_user_cap/over_limit_notification_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..b375596513e63dcdeeeb99ae32ebd4c96a661919 --- /dev/null +++ b/ee/app/workers/namespaces/free_user_cap/over_limit_notification_worker.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Namespaces + module FreeUserCap + class OverLimitNotificationWorker + include ApplicationWorker + include LimitedCapacity::Worker + + feature_category :user_management + data_consistency :always + sidekiq_options retry: false + idempotent! + + MAX_RUNNING_JOBS = 5 + BATCH_SIZE = 1 + SCHEDULE_BUFFER_IN_HOURS = 24 + + def perform_work(*args) + next_batch.map(&:namespace_id).each do |namespace_id| + notify group_id: namespace_id + end + end + + def remaining_work_count(*args) + (Namespace::Detail.scheduled_for_over_limit_check.limit(MAX_RUNNING_JOBS * BATCH_SIZE).count / BATCH_SIZE).ceil + end + + def max_running_jobs + return 0 unless enforce_over_limit_mails? + + MAX_RUNNING_JOBS + end + + private + + def notify(group_id:, checked_at: Time.current) + return unless enforce_over_limit_mails? + + @group = Group.find_by id: group_id # rubocop: disable CodeReuse/ActiveRecord + + NotifyOverLimitGroupsService.execute(group: @group) if @group + end + + def next_batch + Namespace::Detail.transaction do + Namespace::Detail.find_by_sql next_batch_sql # rubocop: disable CodeReuse/ActiveRecord + end + end + + def next_batch_sql + # The alterntive to setting the timestamp would be using something like + # SET "next_over_limit_check_at" = NOW() + INTERVAL '#{SCHEDULE_BUFFER_IN_HOURS} hours' + # that makes it quite hared to spec, since the DB doesn't freeze time along with ruby + <<~SQL.squish + UPDATE "namespace_details" + SET "next_over_limit_check_at" = to_timestamp(#{schedule.to_i}) + WHERE "namespace_details"."namespace_id" IN (#{locked_item_ids_sql}) + RETURNING * + SQL + end + + def locked_item_ids_sql + Namespace::Detail + .not_over_limit_notified + .lock_for_over_limit_check(BATCH_SIZE, namespace_ids) + .select(:namespace_id) + .to_sql + end + + def namespace_ids + ::Namespaces::FreeUserCap::EnforceableGroupsFinder.new.execute.select :id + end + + def schedule + SCHEDULE_BUFFER_IN_HOURS.hours.from_now + end + + def enforce_over_limit_mails? + ::Namespaces::FreeUserCap.over_user_limit_mails_enabled? && ::Gitlab::CurrentSettings.dashboard_limit_enabled? + end + end + end +end diff --git a/ee/spec/finders/namespaces/free_user_cap/enforceable_groups_finder_spec.rb b/ee/spec/finders/namespaces/free_user_cap/enforceable_groups_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7bfdcccae493abb46a24e72f43fe101922a75365 --- /dev/null +++ b/ee/spec/finders/namespaces/free_user_cap/enforceable_groups_finder_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Interim feature category experimentation_conversion used here while waiting for +# https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/113300 to merge +RSpec.describe Namespaces::FreeUserCap::EnforceableGroupsFinder, feature_category: :experimentation_activation do + subject(:finder) { described_class.new.execute } + + before do + stub_ee_application_setting should_check_namespace_plan: true + end + + describe '#execute', :saas do + let_it_be(:private_free) { create :group_with_plan, :private, plan: :free_plan } + let_it_be(:public_free) { create :group_with_plan, :public, plan: :free_plan } + let_it_be(:private_premium) { create :group_with_plan, :private, plan: :premium_plan } + let_it_be(:private_premium_trial) do + create :group_with_plan, :private, plan: :premium_plan, trial_ends_on: Date.current.advance(months: 1) + end + + context 'with out being previously notified' do + it 'finds private free groups' do + expect(finder.count).to eq(1) + expect(finder.all).to match_array([private_free]) + end + end + + context 'when applicable namespace is not a root namespace' do + let_it_be(:parent) { create :group } + + before do + private_free.update! parent_id: parent.id + end + + it 'finds nothing' do + expect(finder.all).to eq([]) + end + end + end +end diff --git a/ee/spec/mailers/emails/free_user_cap_spec.rb b/ee/spec/mailers/emails/free_user_cap_spec.rb index 854f123d2f0a7ec35f5f06c9ddb0aa63e68e0179..a6598b5943c4ccf63eca55c27b6c3e1a8a2b1f20 100644 --- a/ee/spec/mailers/emails/free_user_cap_spec.rb +++ b/ee/spec/mailers/emails/free_user_cap_spec.rb @@ -7,16 +7,18 @@ let_it_be(:user) { create :user } let_it_be(:namespace) { create :namespace, name: "foobar" } + let(:checked_at) { "1969-04-20 13:37:42 UTC" } - describe "#reached_free_user_limit_email" do - subject(:email) { Notify.reached_free_user_limit_email user, namespace } + describe "#over_free_user_limit_email" do + subject(:email) { Notify.over_free_user_limit_email user, namespace, checked_at } it "sends mail with expected contents" do allow(Namespaces::FreeUserCap).to receive(:dashboard_limit).and_return(10) expect(email).to have_subject("You've reached your member limit!") - expect(email).to have_body_text("Looks like you've reached your limit") + expect(email).to have_body_text("It looks like you've reached your limit") expect(email).to have_body_text("of 10 members") + expect(email).to have_body_text("according to the check we ran on April 20, 1969 13:37") expect(email).to have_body_text("Manage members") expect(email).to have_body_text("Explore paid plans") expect(email).to have_body_text("foobar/-/billings") diff --git a/ee/spec/models/ee/namespace/detail_spec.rb b/ee/spec/models/ee/namespace/detail_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cec9acc5a04884e05573b53e37865b75d381c538 --- /dev/null +++ b/ee/spec/models/ee/namespace/detail_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespace::Detail, type: :model, feature_category: :experimentation_conversion, ee: true do + context 'with scopes' do + def set_schedule(group, time, notified_at = nil) + group.namespace_details.update! next_over_limit_check_at: time, free_user_cap_over_limit_notified_at: notified_at + end + + let_it_be(:group_1) { create :group, :private } + let_it_be(:group_2) { create :group, :private } + let_it_be(:group_3) { create :group, :private } + let_it_be(:group_4) { create :group, :private } + let_it_be(:group_5) { create :group, :private } + + let(:scheduled_items) do + [ + group_1.namespace_details, + group_2.namespace_details, + group_3.namespace_details, + group_5.namespace_details + ] + end + + let(:not_notified_items) do + [ + group_1.namespace_details, + group_2.namespace_details, + group_3.namespace_details, + group_4.namespace_details + ] + end + + let(:namespace_ids) { ::Namespaces::FreeUserCap::EnforceableGroupsFinder.new.execute.select(:id) } + + before do + set_schedule group_1, nil + set_schedule group_2, 2.days.ago + set_schedule group_3, 1.5.days.ago + + set_schedule group_4, 3.days.from_now # not scheduled_item: next_over_limit_check_at too new + set_schedule group_5, 1.day.ago, 2.days.ago # not not_notified_items: has already been notified + end + + context 'for not_over_limit_notified' do + it 'returns only entries that have not been notified of being over limit' do + expect(described_class.not_over_limit_notified).to match_array(not_notified_items) + end + end + + context 'for scheduled_for_over_limit_check' do + it 'returns entries that have been scheduled for over limit checks' do + expect(described_class.scheduled_for_over_limit_check).to eq(scheduled_items) + end + end + + context 'for lock_for_over_limit_check' do + let(:limit) { 2 } + let(:scheduled_items) { [group_1.namespace_details, group_2.namespace_details] } + + subject(:lock_scope_result) { described_class.lock_for_over_limit_check(limit, namespace_ids) } + + it 'only returns scheduled entries up to the limit' do + expect(lock_scope_result).to eq(scheduled_items) + end + end + end +end diff --git a/ee/spec/models/namespaces/free_user_cap_spec.rb b/ee/spec/models/namespaces/free_user_cap_spec.rb index 9cf05bca3e283f1eac9d812d545d54bb32764f82..0ac9898ee8110093ccecb587bc4e5eccaa9ec9eb 100644 --- a/ee/spec/models/namespaces/free_user_cap_spec.rb +++ b/ee/spec/models/namespaces/free_user_cap_spec.rb @@ -32,6 +32,22 @@ end end + describe '.over_user_limit_mails_enabled?' do + subject { described_class.over_user_limit_mails_enabled? } + + context 'when feature flag is enabled' do + it { is_expected.to be_truthy } + end + + context 'when feature flag is disabled' do + it 'reflects the state of the feature flag' do + stub_feature_flags free_user_cap_over_user_limit_mails: false + + expect(subject).to be_falsey + end + end + end + describe '.dashboard_limit' do subject { described_class.dashboard_limit } diff --git a/ee/spec/services/namespaces/free_user_cap/notify_over_limit_groups_service_spec.rb b/ee/spec/services/namespaces/free_user_cap/notify_over_limit_groups_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a817ec4a2cde69bc3aecb870e9a521a92809d930 --- /dev/null +++ b/ee/spec/services/namespaces/free_user_cap/notify_over_limit_groups_service_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Interim feature category experimentation_activation used here while waiting for +# https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/113300 to merge +RSpec.describe Namespaces::FreeUserCap::NotifyOverLimitGroupsService, feature_category: :experimentation_activation, + saas: true do + def create_enforcable_free_group(dashboard_limit: 2) + free_group = create :group_with_plan, :private, plan: :free_plan + create_list :group_member, (dashboard_limit + 1), :active, source: free_group + free_group + end + + let(:frozen_time) { Time.zone.parse "2022-09-22T00:00+0" } + let(:group) { create_enforcable_free_group } + + around do |example| + travel_to(frozen_time) { example.run } + end + + before do + # Change the limit after setup to avoid validation errors during setup + # - We need to create groups that are over the limit which isn't possible + # anymore with net-new namespaces + # @group = create_enforcable_free_group + group.add_owner create(:owner) + + stub_ee_application_setting dashboard_limit: limit + stub_ee_application_setting dashboard_enforcement_limit: limit + stub_ee_application_setting dashboard_limit_enabled: true + stub_ee_application_setting should_check_namespace_plan: true + end + + context "with over limit group" do + let(:limit) { 2 } + + describe '#execute', :saas do + let(:service) { described_class.new group: group } + + it 'records the time of notification in free_user_cap_over_limit_notified_at' do + expect { service.execute } + .to change { group.namespace_details.free_user_cap_over_limit_notified_at } + .from(nil) + .to(Time.zone.parse("2022-09-22T00:00:00.+0")) + + service.execute + end + end + + describe '.execute', :saas do + it 'emails the owner(s) of the group' do + group.owners.each do |owner| + expect(Notify).to receive(:over_free_user_limit_email).with(owner, group, frozen_time).once.and_call_original + end + + described_class.execute group: group + end + end + + describe 'error handling', :saas do + it 'rescues to a ServiceResponse' do + expect(Namespaces::FreeUserCap) + .to receive(:over_user_limit_mails_enabled?).and_raise(StandardError, 'How error doing?') + result = described_class.execute group: group + expect(result).to be_a(ServiceResponse) + expect(result).not_to be_success + end + end + end + + context 'when under the limit' do + let(:limit) { 42 } + + describe '#execute', :saas do + let(:service) { described_class.new group: group } + + it 'exits early' do + expect(Notify).not_to receive(:over_free_user_limit_email) + result = service.execute + expect(result).to be_nil + end + end + end +end diff --git a/ee/spec/workers/namespaces/free_user_cap/backfill_notification_jobs_worker_spec.rb b/ee/spec/workers/namespaces/free_user_cap/backfill_notification_jobs_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d6c2eed248fd171c6f67c51947e50fb6cf63a21e --- /dev/null +++ b/ee/spec/workers/namespaces/free_user_cap/backfill_notification_jobs_worker_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +require 'spec_helper' + +# Interim feature category experimentation_activation used here while waiting for +# https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/113300 to merge +RSpec.describe Namespaces::FreeUserCap::BackfillNotificationJobsWorker, + type: :worker, + feature_category: :experimentation_activation do + let(:worker) { described_class.new } + + describe '#perform' do + it 'adds a OverLimitNotificationWorker to the limited capacity worker pool' do + expect(Namespaces::FreeUserCap::OverLimitNotificationWorker).to receive(:perform_with_capacity) + + worker.perform + end + end +end diff --git a/ee/spec/workers/namespaces/free_user_cap/over_limit_notification_worker_spec.rb b/ee/spec/workers/namespaces/free_user_cap/over_limit_notification_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..38a6f75aaf481a63f27512caed6352f4ed26f17a --- /dev/null +++ b/ee/spec/workers/namespaces/free_user_cap/over_limit_notification_worker_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Interim feature category experimentation_activation used here while waiting for +# https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/113300 to merge +RSpec.describe Namespaces::FreeUserCap::OverLimitNotificationWorker, feature_category: :experimentation_activation, + type: :worker, + ee: true do + let(:frozen_time) { Time.zone.parse "2022-09-22T00:00+0" } + + around do |example| + travel_to(frozen_time) { example.run } + end + + describe '#perform', :saas do + subject(:worker) { described_class.new.perform } + + let(:group) { create :group_with_plan, :private, plan: :free_plan } + let(:owner) { create :owner } + + before do + group.add_owner owner + group.namespace_details.update! next_over_limit_check_at: 2.days.ago + end + + context 'when on gitlab.com', :saas do + before do + stub_ee_application_setting should_check_namespace_plan: true + stub_ee_application_setting dashboard_limit_enabled: true + end + + it 'runs notifiy service and marks next check for group' do + expect(::Namespaces::FreeUserCap::NotifyOverLimitGroupsService).to receive(:execute) + + next_check_time = frozen_time + described_class::SCHEDULE_BUFFER_IN_HOURS.hours + + expect { subject }.to change { group.reload.namespace_details.next_over_limit_check_at }.to(next_check_time) + end + end + + context 'with feature flags enabled/disabled' do + where( + limit_enabled: [true, false, false], + free_user_cap_over_user_limit_mails: [false, true, false] + ) + + before do + stub_ee_application_setting dashboard_limit_enabled: limit_enabled + stub_ee_application_setting should_check_namespace_plan: true + stub_feature_flags free_user_cap_over_user_limit_mails: free_user_cap_over_user_limit_mails + end + + with_them do + it 'triggers mail the namespace owners', :aggregate_failures do + if limit_enabled && free_user_cap_over_user_limit_mails + expect(::Namespaces::FreeUserCap::NotifyOverLimitGroupsService).to receive(:execute) + expect(described_class.new.max_running_jobs).to eq(5) + else + expect(::Namespaces::FreeUserCap::NotifyOverLimitGroupsService).not_to receive(:execute) + expect(described_class.new.max_running_jobs).to eq(0) + end + + subject + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a4f3abb69e87df2f4ccb7828d164e474d0c2d995..642ca5e76d1e07089dc5f7668187c2d3d0a55966 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17937,7 +17937,7 @@ msgstr "" msgid "FreeUserCap|Explore paid plans:" msgstr "" -msgid "FreeUserCap|Looks like you've reached your limit of %{free_user_limit} members for \"%{namespace_name}\". You can't add any more, but you can manage your existing members, for example, by removing inactive members and replacing them with new members." +msgid "FreeUserCap|It looks like you've reached your limit of %{free_user_limit} members for \"%{namespace_name}\", according to the check we ran on %{date_time}. You can't add any more, but you can manage your existing members, for example, by removing inactive members and replacing them with new members." msgstr "" msgid "FreeUserCap|Manage members" diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index c444e1f383cdb69918cd442282c6b4c3a1d71d59..6e306cd18ac3d190d078cb450bad2c6aea8a99e9 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -363,6 +363,7 @@ 'Onboarding::PipelineCreatedWorker' => 3, 'Onboarding::ProgressWorker' => 3, 'Onboarding::UserAddedWorker' => 3, + 'Namespaces::FreeUserCap::OverLimitNotificationWorker' => false, 'Namespaces::RefreshRootStatisticsWorker' => 3, 'Namespaces::RootStatisticsWorker' => 3, 'Namespaces::ScheduleAggregationWorker' => 3,