diff --git a/ee/app/models/gitlab_subscription.rb b/ee/app/models/gitlab_subscription.rb
index 595a465abf5f24909c8d155e1b53f90c2189d9c8..d40aa6e41f1971836ffefc84e0d2271ccc3a1e92 100644
--- a/ee/app/models/gitlab_subscription.rb
+++ b/ee/app/models/gitlab_subscription.rb
@@ -16,6 +16,9 @@ class GitlabSubscription < ApplicationRecord
after_save :set_prevent_sharing_groups_outside_hierarchy
+ # Needs to run after_commit because workers can be spawned that can't be run within a transaction
+ after_commit :activate_users_post_subscription_upgrade
+
after_commit :index_namespace, on: [:create, :update]
after_destroy_commit :log_previous_state_for_destroy
@@ -214,4 +217,15 @@ def set_prevent_sharing_groups_outside_hierarchy
# going from paid to free - prevent
namespace.update_attribute(:prevent_sharing_groups_outside_hierarchy, prevent_sharing)
end
+
+ # When free groups where `free_user_cap` was enabled upgrade to a paid
+ # plan we want to activate all the memberships that got set to
+ # awaiting due to the limit.
+ def activate_users_post_subscription_upgrade
+ # rubocop: disable CodeReuse/ServiceClass
+ GitlabSubscriptions::ActivateAwaitingUsersService
+ .new(gitlab_subscription: self, previous_plan_id: hosted_plan_id_before_last_save)
+ .execute
+ # rubocop: enable CodeReuse/ServiceClass
+ end
end
diff --git a/ee/app/services/gitlab_subscriptions/activate_awaiting_users_service.rb b/ee/app/services/gitlab_subscriptions/activate_awaiting_users_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c7f4dcff93ab85edcba71167a19d7b4a2616b8cf
--- /dev/null
+++ b/ee/app/services/gitlab_subscriptions/activate_awaiting_users_service.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module GitlabSubscriptions
+ class ActivateAwaitingUsersService
+ def initialize(gitlab_subscription:, previous_plan_id:)
+ @gitlab_subscription = gitlab_subscription
+ @previous_plan_id = previous_plan_id
+ end
+
+ def execute
+ return unless previous_plan_id
+ return unless namespace.group_namespace?
+ return unless ::Namespaces::FreeUserCap::Standard.new(namespace).feature_enabled?
+ return unless upgrade_from_free_to_paid?
+
+ activate
+ end
+
+ private
+
+ attr_reader :gitlab_subscription, :previous_plan_id
+
+ def namespace
+ @namespace ||= gitlab_subscription.namespace
+ end
+
+ def upgrade_from_free_to_paid?
+ !Plan.find(previous_plan_id).paid? && Plan.find(gitlab_subscription.hosted_plan_id).paid?
+ end
+
+ def activate
+ awaiting_user_ids = namespace.awaiting_user_ids
+ return if awaiting_user_ids.empty?
+ return if awaiting_user_ids.count > gitlab_subscription.seats_remaining
+
+ ::Members::ActivateService
+ .for_users(namespace, users: awaiting_user_ids)
+ .execute(current_user: User.automation_bot, skip_authorization: true)
+ end
+ end
+end
diff --git a/ee/app/services/members/activate_service.rb b/ee/app/services/members/activate_service.rb
index 9cc70d3cf0af1b61bff0947e76a785e70cf7bc26..34b69c9686c6b455c8ab5f3989d45ca9b42d38a3 100644
--- a/ee/app/services/members/activate_service.rb
+++ b/ee/app/services/members/activate_service.rb
@@ -39,8 +39,9 @@ def initialize(group, memberships:)
private_class_method :new
- def execute(current_user:)
+ def execute(current_user:, skip_authorization: false)
@current_user = current_user
+ @skip_authorization = skip_authorization
return error(_('You do not have permission to approve a member'), :forbidden) unless allowed?
return error(_('There is no seat left to activate the member')) unless has_capacity_left?
@@ -56,7 +57,7 @@ def execute(current_user:)
private
- attr_reader :group, :memberships, :current_user, :affected_members
+ attr_reader :group, :memberships, :current_user, :affected_members, :skip_authorization
def activate_memberships
@affected_members = memberships.to_a
@@ -65,6 +66,8 @@ def activate_memberships
end
def allowed?
+ return true if skip_authorization
+
can?(current_user, :admin_group_member, group)
end
diff --git a/ee/spec/models/gitlab_subscription_spec.rb b/ee/spec/models/gitlab_subscription_spec.rb
index 7b2c1d1b95b4cac9398066cd7092c9e992faea65..c51f17edb15de15d8a8848fac4185dbfb7c03834 100644
--- a/ee/spec/models/gitlab_subscription_spec.rb
+++ b/ee/spec/models/gitlab_subscription_spec.rb
@@ -420,93 +420,111 @@
end
describe 'callbacks' do
- context 'after_commit :index_namespace', :saas do
- let_it_be(:namespace) { create(:namespace) }
-
- let(:gitlab_subscription) { build(:gitlab_subscription, plan, namespace: namespace) }
- let(:expiration_date) { Date.today + 10 }
- let(:plan) { :bronze }
+ context 'after_commit', :saas do
+ context 'activate_users_post_subscription_upgrade' do
+ let_it_be(:gitlab_subscription) { create(:gitlab_subscription, hosted_plan: free_plan) }
+
+ it 'calls ActivateAwaitingUsersService' do
+ expect_next_instance_of(
+ GitlabSubscriptions::ActivateAwaitingUsersService,
+ gitlab_subscription: gitlab_subscription,
+ previous_plan_id: free_plan.id
+ ) do |instance|
+ expect(instance).to receive(:execute)
+ end
- before do
- gitlab_subscription.end_date = expiration_date
+ gitlab_subscription.update!(hosted_plan: ultimate_plan)
+ end
end
- it 'indexes the namespace' do
- expect(ElasticsearchIndexedNamespace).to receive(:safe_find_or_create_by!).with(namespace_id: gitlab_subscription.namespace_id)
+ context 'index_namespace' do
+ let_it_be(:namespace) { create(:namespace) }
- gitlab_subscription.save!
- end
+ let(:gitlab_subscription) { build(:gitlab_subscription, plan, namespace: namespace) }
+ let(:expiration_date) { Date.today + 10 }
+ let(:plan) { :bronze }
- context 'when seats is 0' do
- let(:gitlab_subscription) { build(:gitlab_subscription, namespace: namespace, seats: 0) }
+ before do
+ gitlab_subscription.end_date = expiration_date
+ end
- it 'does not index the namespace' do
- expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
+ it 'indexes the namespace' do
+ expect(ElasticsearchIndexedNamespace).to receive(:safe_find_or_create_by!).with(namespace_id: gitlab_subscription.namespace_id)
gitlab_subscription.save!
end
- end
- context 'when it is a trial' do
- let(:seats) { 10 }
- let(:gitlab_subscription) { build(:gitlab_subscription, :active_trial, namespace: namespace, seats: seats) }
+ context 'when seats is 0' do
+ let(:gitlab_subscription) { build(:gitlab_subscription, namespace: namespace, seats: 0) }
- it 'indexes the namespace' do
- expect(ElasticsearchIndexedNamespace).to receive(:safe_find_or_create_by!).with(namespace_id: gitlab_subscription.namespace_id)
+ it 'does not index the namespace' do
+ expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
- gitlab_subscription.save!
+ gitlab_subscription.save!
+ end
end
- context 'when seats is zero' do
- let(:seats) { 0 }
+ context 'when it is a trial' do
+ let(:seats) { 10 }
+ let(:gitlab_subscription) { build(:gitlab_subscription, :active_trial, namespace: namespace, seats: seats) }
it 'indexes the namespace' do
expect(ElasticsearchIndexedNamespace).to receive(:safe_find_or_create_by!).with(namespace_id: gitlab_subscription.namespace_id)
gitlab_subscription.save!
end
- end
- context 'when in free plan' do
- let(:gitlab_subscription) { build(:gitlab_subscription, :active_trial, namespace: namespace, seats: seats, hosted_plan_id: nil) }
+ context 'when seats is zero' do
+ let(:seats) { 0 }
- it 'does not index the namespace' do
- expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
+ it 'indexes the namespace' do
+ expect(ElasticsearchIndexedNamespace).to receive(:safe_find_or_create_by!).with(namespace_id: gitlab_subscription.namespace_id)
- gitlab_subscription.save!
+ gitlab_subscription.save!
+ end
end
- end
- end
- context 'when not ::Gitlab.com?' do
- before do
- allow(::Gitlab).to receive(:com?).and_return(false)
+ context 'when in free plan' do
+ let(:gitlab_subscription) { build(:gitlab_subscription, :active_trial, namespace: namespace, seats: seats, hosted_plan_id: nil) }
+
+ it 'does not index the namespace' do
+ expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
+
+ gitlab_subscription.save!
+ end
+ end
end
- it 'does not index the namespace' do
- expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
+ context 'when not ::Gitlab.com?' do
+ before do
+ allow(::Gitlab).to receive(:com?).and_return(false)
+ end
+
+ it 'does not index the namespace' do
+ expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
- gitlab_subscription.save!
+ gitlab_subscription.save!
+ end
end
- end
- context 'when the plan has expired' do
- let(:expiration_date) { Date.today - 8.days }
+ context 'when the plan has expired' do
+ let(:expiration_date) { Date.today - 8.days }
- it 'does not index the namespace' do
- expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
+ it 'does not index the namespace' do
+ expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
- gitlab_subscription.save!
+ gitlab_subscription.save!
+ end
end
- end
- context 'when it is a free plan' do
- let(:plan) { :free }
+ context 'when it is a free plan' do
+ let(:plan) { :free }
- it 'does not index the namespace' do
- expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
+ it 'does not index the namespace' do
+ expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
- gitlab_subscription.save!
+ gitlab_subscription.save!
+ end
end
end
end
diff --git a/ee/spec/services/gitlab_subscriptions/activate_awaiting_users_service_spec.rb b/ee/spec/services/gitlab_subscriptions/activate_awaiting_users_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..6f18d91a0742c025b8fab45019ac3c9708b63b5c
--- /dev/null
+++ b/ee/spec/services/gitlab_subscriptions/activate_awaiting_users_service_spec.rb
@@ -0,0 +1,110 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSubscriptions::ActivateAwaitingUsersService, :saas do
+ shared_examples 'skips user activation' do
+ it 'does not activate users' do
+ execute
+
+ expect(member1.reload).to be_awaiting
+ expect(member2.reload).to be_awaiting
+ end
+ end
+
+ describe '#execute' do
+ let_it_be(:namespace) { create(:group) }
+ let_it_be(:free_plan) { create(:free_plan) }
+ let_it_be(:ultimate_plan) { create(:ultimate_plan) }
+ let_it_be(:user1) { create(:user) }
+ let_it_be(:user2) { create(:user) }
+ let_it_be(:member1) { create(:group_member, :awaiting, user: user1, source: namespace) }
+ let_it_be(:member2) { create(:group_member, :awaiting, user: user2, source: namespace) }
+
+ let(:current_plan) { free_plan }
+ let(:free_user_cap_enabled) { true }
+ let(:seats_remaining) { 2 }
+ let(:gitlab_subscription) do
+ create(:gitlab_subscription, hosted_plan: current_plan, seats: seats_remaining, namespace: namespace)
+ end
+
+ subject(:execute) do
+ described_class.new(gitlab_subscription: gitlab_subscription, previous_plan_id: previous_plan&.id).execute
+ end
+
+ before do
+ allow_next_instance_of(::Namespaces::FreeUserCap::Standard, namespace) do |instance|
+ allow(instance).to receive(:feature_enabled?).and_return(free_user_cap_enabled)
+ end
+ end
+
+ context 'without a previous plan' do
+ let(:previous_plan) { nil }
+
+ context 'when switching to a free plan' do
+ let_it_be(:current_plan) { free_plan }
+
+ it_behaves_like 'skips user activation'
+ end
+
+ context 'when switching to a paid plan' do
+ let_it_be(:current_plan) { ultimate_plan }
+
+ it_behaves_like 'skips user activation'
+ end
+ end
+
+ context 'with a previous free plan' do
+ let(:previous_plan) { free_plan }
+
+ context 'when switching to a paid plan' do
+ let_it_be_with_reload(:current_plan) { ultimate_plan }
+
+ context 'when not a group namespace' do
+ let_it_be(:namespace) { create(:namespace) }
+
+ it_behaves_like 'skips user activation'
+ end
+
+ context 'when there are enough seats remaining' do
+ it 'activates users' do
+ execute
+
+ expect(member1.reload).to be_active
+ expect(member2.reload).to be_active
+ end
+
+ it 'audits the event using the automation bot user as author' do
+ execute
+
+ audit_event = AuditEvent.find_by(target_id: member1.id)
+ expect(audit_event.author).to eq(User.automation_bot)
+ expect(audit_event.details[:custom_message]).to eq('Changed the membership state to active')
+ end
+
+ context 'when free user cap is not enabled' do
+ let(:free_user_cap_enabled) { false }
+
+ it_behaves_like 'skips user activation'
+ end
+ end
+
+ context 'when there are not enough seats remaining' do
+ let(:seats_remaining) { 0 }
+
+ it_behaves_like 'skips user activation'
+ end
+ end
+ end
+
+ context 'with a previous paid plan' do
+ let(:previous_plan) { ultimate_plan }
+
+ context 'changes from paid plan to free plan' do
+ let(:current_plan) { free_plan }
+
+ it_behaves_like 'skips user activation'
+ end
+ end
+ end
+end
diff --git a/ee/spec/services/members/activate_service_spec.rb b/ee/spec/services/members/activate_service_spec.rb
index 2e754583ab8ecae2b3b0da4c05a9d575c0ddad0e..3615c52529f75ec36960bf0c9139ff36e15c8d83 100644
--- a/ee/spec/services/members/activate_service_spec.rb
+++ b/ee/spec/services/members/activate_service_spec.rb
@@ -252,6 +252,22 @@
it_behaves_like 'returns an error', 'You do not have permission to approve a member'
end
+ context 'when current_user is nil' do
+ let_it_be(:current_user) { nil }
+
+ it_behaves_like 'returns an error', 'You do not have permission to approve a member'
+ end
+
+ context 'when skipping authorization' do
+ let_it_be(:current_user) { User.automation_bot }
+
+ let_it_be(:members) { [create(:group_member, :awaiting, group: root_group, user: user)] }
+
+ subject(:execute) { described_class.for_group(root_group).execute(current_user: current_user, skip_authorization: true) }
+
+ it_behaves_like 'successful member activation'
+ end
+
context 'when authorized' do
before do
root_group.add_owner(current_user)