diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index a9ed99b46c1bbfb45516e1c42d69aa3aadd081eb..193127a21a82f88fdea30a3e1e9d5ea8d4fe787e 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -491,6 +491,10 @@ - 1 - - gitlab_subscriptions_seat_assignments_member_transfers_create_project_seats - 1 +- - gitlab_subscriptions_seat_assignments_member_transfers_remove_group_seats + - 1 +- - gitlab_subscriptions_seat_assignments_member_transfers_remove_project_seats + - 1 - - gitlab_subscriptions_self_managed_duo_core_todo_notification - 1 - - gitlab_subscriptions_trials_apply_trial diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 71eee7a2922b2b5f5c6373792b2b6856029eb850..20e28393042259189cd000433bc821c719b48fe8 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2483,6 +2483,26 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: gitlab_subscriptions_seat_assignments_member_transfers_remove_group_seats + :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveGroupSeatsWorker + :feature_category: :seat_cost_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: +- :name: gitlab_subscriptions_seat_assignments_member_transfers_remove_project_seats + :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveProjectSeatsWorker + :feature_category: :seat_cost_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: gitlab_subscriptions_self_managed_duo_core_todo_notification :worker_name: GitlabSubscriptions::SelfManaged::DuoCoreTodoNotificationWorker :feature_category: :acquisition diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..cc8c54a9f52d7603a0898a7b8746d402809881af --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# rubocop:disable Scalability/IdempotentWorker -- Idempotent worker declaration in children +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class BaseRemoveSeatsWorker + BATCH_SIZE = 1000 + + def perform(old_root_source_id, source_id) + source = find_source_by_id(source_id) + return unless source + + remove_outdated_seat_assignments(old_root_source_id, source) + end + + private + + # rubocop: disable CodeReuse/ActiveRecord -- Needed to use the where clauses + def delete_seats(source, user_ids) + return if user_ids.blank? + + user_ids.each_slice(self.class::BATCH_SIZE) do |batch| + ::GitlabSubscriptions::SeatAssignment.where( + namespace_id: source.id, + user_id: batch + ).delete_all + end + end + # rubocop: enable CodeReuse/ActiveRecord + + def collect_outdated_user_ids(old_root_source, user_ids_in_transferred_source_hierarchy) + member_ids_in_old_root_source = collect_user_ids( + old_root_source, + filter_user_ids: user_ids_in_transferred_source_hierarchy + ).to_set + + user_ids_in_transferred_source_hierarchy.reject do |id| + member_ids_in_old_root_source.include?(id) + end + end + + def collect_user_ids(source, filter_user_ids: nil) + raise NotImplementedError + end + + def remove_outdated_seat_assignments(old_root_source_id, source) + raise NotImplementedError + end + + def find_source_by_id(source_id) + raise NotImplementedError + end + end + end + end +end +# rubocop:enable Scalability/IdempotentWorker diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..3abf5a21c35b83e6f5924c68b5685485f737d044 --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class RemoveGroupSeatsWorker < BaseRemoveSeatsWorker + include ApplicationWorker + + feature_category :seat_cost_management + data_consistency :delayed + urgency :low + + defer_on_database_health_signal :gitlab_main, + [:subscription_seat_assignments, :members], 10.minutes + + idempotent! + + private + + def find_source_by_id(group_id) + Group.find_by_id(group_id) + end + + def remove_outdated_seat_assignments(old_root_namespace_id, group) + old_root_namespace = find_source_by_id(old_root_namespace_id) + return unless old_root_namespace + + user_ids_in_transferred_group_hierarchy = collect_user_ids(group) + + if top_group_to_subgroup?(old_root_namespace.id, group.id, group.root_ancestor.id) + delete_seats(old_root_namespace, user_ids_in_transferred_group_hierarchy) + return + end + + outdated_user_ids = collect_outdated_user_ids(old_root_namespace, user_ids_in_transferred_group_hierarchy) + + delete_seats(old_root_namespace, outdated_user_ids) + end + + # rubocop: disable CodeReuse/ActiveRecord -- Needed to use the where clauses + def collect_user_ids(group, filter_user_ids: nil) + scope = Member.for_self_and_descendants(group) + scope = scope.where(user_id: filter_user_ids) if filter_user_ids + scope.pluck_user_ids + end + # rubocop: enable CodeReuse/ActiveRecord -- Needed to use the where clauses + + def top_group_to_subgroup?(old_root_group_id, current_group_id, new_root_group_id) + old_root_group_id == current_group_id && new_root_group_id != old_root_group_id + end + end + end + end +end diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..94675806257878d6c842e9c5a884fb8e1a4e0bae --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class RemoveProjectSeatsWorker < BaseRemoveSeatsWorker + include ApplicationWorker + + feature_category :seat_cost_management + data_consistency :delayed + urgency :low + + defer_on_database_health_signal :gitlab_main, + [:subscription_seat_assignments, :members], 10.minutes + + idempotent! + + private + + def find_source_by_id(project_id) + Project.find_by_id(project_id) + end + + def remove_outdated_seat_assignments(old_root_namespace_id, project) + old_root_namespace = Namespace.find_by_id(old_root_namespace_id) + return unless old_root_namespace + + user_ids_in_transferred_project = collect_user_ids(project) + + outdated_user_ids = collect_outdated_user_ids(old_root_namespace, user_ids_in_transferred_project) + + delete_seats(old_root_namespace, outdated_user_ids) + end + + # rubocop: disable CodeReuse/ActiveRecord -- Needed to use the where clauses + def collect_user_ids(source, filter_user_ids: nil) + return [] unless source + + scope = + case source + when Group + Member.for_self_and_descendants(source) + when Project + source.project_members + else + Member.in_hierarchy(source) + end + + scope = scope.where(user_id: filter_user_ids) if filter_user_ids.present? + scope.pluck_user_ids + end + # rubocop: enable CodeReuse/ActiveRecord -- Needed to use the where clauses + end + end + end +end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3c1edc2a0999a1bbfa352ccfbf612dd898598d3b --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker_spec.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveGroupSeatsWorker, :saas, feature_category: :seat_cost_management do + let(:worker) { described_class.new } + let_it_be_with_refind(:user) { create(:user) } + let_it_be_with_refind(:user_2) { create(:user) } + let_it_be_with_refind(:root_group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be_with_refind(:transferred_group) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } + + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: root_group) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: root_group) + create(:group_member, group: transferred_group, user: user) + create(:group_member, group: transferred_group, user: user_2) + end + + describe '#perform' do + context 'when a root group is transferred to become a subgroup of another group hierarchy' do + let_it_be(:new_parent_group) { create(:group_with_plan, plan: :ultimate_plan) } + + before do + create(:group_member, group: root_group, user: user) + create(:group_member, group: root_group, user: user_2) + root_group.update!(parent: new_parent_group) + end + + it 'removes all outdated seat assignments from old root namespace' do + expect(GitlabSubscriptions::SeatAssignment.by_namespace(root_group)).not_to be_empty + + worker.perform(root_group.id, root_group.id) + + expect(GitlabSubscriptions::SeatAssignment.by_namespace(root_group)).to be_empty + end + end + + context 'when a subgroup is moved to a different group hierarchy' do + let_it_be(:new_parent_group) { create(:group_with_plan, plan: :ultimate_plan) } + + before do + transferred_group.update!(parent: new_parent_group) + end + + it 'removes outdated seat assignments' do + expect do + worker.perform(root_group.id, transferred_group.id) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-2) + end + end + + context 'when a subgroup is moved within same hierarchy' do + let_it_be(:parent_subgroup) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } + + before do + transferred_group.update!(parent: parent_subgroup) + end + + it 'does not remove seat assignments' do + expect do + worker.perform(root_group.id, transferred_group.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count } + end + end + + context 'when there are no outdated seat assignments' do + let!(:member) { create(:group_member, group: root_group, user: user) } + let!(:member_2) { create(:group_member, group: root_group, user: user_2) } + + it 'does not remove seat assignments' do + expect do + worker.perform(root_group.id, root_group.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count } + end + + it 'has members corresponding to seat assignments for the group' do + worker.perform(root_group.id, root_group.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to match(::Member.in_hierarchy(root_group).pluck(:user_id).uniq) + end + end + end + + describe '#collect_group_user_ids' do + context 'when a group hierarchy has a parent group' do + let_it_be(:root_group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be(:child_group_a) { create(:group, parent: root_group) } + let_it_be(:child_group_b) { create(:group, parent: root_group) } + let_it_be(:child_group_c) { create(:group, parent: child_group_a) } + let_it_be(:user_3) { create(:user) } + let_it_be(:user_4) { create(:user) } + + before do + create(:group_member, group: root_group, user: user) + create(:group_member, group: child_group_a, user: user_2) + create(:group_member, group: child_group_b, user: user_3) + create(:group_member, group: child_group_c, user: user_4) + end + + it 'takes the ids of the whole hierarchy when a root is given' do + expect(worker.send(:collect_user_ids, root_group)).to contain_exactly(user.id, user_2.id, user_3.id, user_4.id) + end + + it 'only takes the ids of the given subhierarchy' do + expect(worker.send(:collect_user_ids, child_group_a)).to contain_exactly(user_2.id, user_4.id) + end + + it 'only takes the ids of the given leaf group' do + expect(worker.send(:collect_user_ids, child_group_b)).to contain_exactly(user_3.id) + expect(worker.send(:collect_user_ids, child_group_c)).to contain_exactly(user_4.id) + end + + it 'returns filtered user IDs when filter_user_ids is provided' do + user_ids = worker.send(:collect_user_ids, child_group_a, filter_user_ids: [user_2.id]) + + expect(user_ids).to contain_exactly(user_2.id) + end + + it 'returns an empty array if no users match the filter' do + user_ids = worker.send(:collect_user_ids, child_group_b, filter_user_ids: [user_4.id]) + + expect(user_ids).to be_empty + end + + context 'when a group has a project' do + let_it_be(:project) { create(:project, namespace: child_group_c) } + let_it_be(:project_user) { create(:user) } + let_it_be(:project_member) { create(:project_member, project: project, user: project_user) } + + it 'takes the ids of the members of the group including the project' do + expect(worker.send(:collect_user_ids, child_group_c)).to contain_exactly(user_4.id, project_user.id) + end + end + end + end +end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4734343abeda808825b2f784b869922918be0766 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker_spec.rb @@ -0,0 +1,229 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveProjectSeatsWorker, :saas, feature_category: :seat_cost_management do + let(:worker) { described_class.new } + let_it_be_with_refind(:user) { create(:user, :with_namespace) } + let_it_be_with_refind(:user_2) { create(:user) } + + describe '#perform' do + context 'when the project root namespace is a user namespace' do + let_it_be_with_refind(:user_namespace) { user.namespace } + let_it_be_with_refind(:project) { create(:project, namespace: user_namespace) } + let_it_be_with_refind(:root_group) { create(:group_with_plan, plan: :ultimate_plan) } + + context 'when the user namespace has outdated seats after project has been transferred to a group' do + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user_namespace) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: user_namespace) + end + + context 'when the user namespace still has members via another project' do + let_it_be_with_refind(:project_2) { create(:project, namespace: user_namespace) } + + before do + create(:project_member, project: project_2, user: user_2) + project_2.update!(namespace: root_group) + end + + it 'removes outdated seat assignments and leaves the seat of the remaining member' do + expect do + worker.perform(project.namespace.id, project_2.id) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).count }.by(-1) + end + + it 'leaves the seat of the remaining member' do + worker.perform(user_namespace.id, project_2.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace).pluck(:user_id) + ).to match(project.project_members.pluck(:user_id)) + end + end + end + + context 'when there are no outdated seat assignments' do + let_it_be_with_refind(:project_2) { create(:project, namespace: user_namespace) } + + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user_namespace) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: user_namespace) + create(:project_member, project: project, user: user_2) + create(:project_member, project: project_2, user: user_2) + project_2.update!(namespace: root_group) + end + + it 'does not remove seat assignments' do + expect do + worker.perform(user_namespace.id, project_2.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace).count } + end + + it 'ensures project members match the seat assignments for the project namespace' do + worker.perform(user_namespace.id, project_2.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) + ).to match(::Member.in_hierarchy(user_namespace).pluck(:user_id).uniq) + end + end + + context 'when the user namespace has no remaining members after project transfer' do + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user_namespace) + project.update!(namespace: root_group) + end + + it 'removes all seat assignments' do + worker.perform(user_namespace.id, project.id) + + expect(GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace)).to be_empty + end + end + end + + context 'when the project root namespace is a group' do + let_it_be_with_refind(:root_group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be_with_refind(:project) { create(:project, namespace: root_group) } + let_it_be_with_refind(:user_3) { create(:user) } + + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: root_group) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: root_group) + create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: root_group) + create(:project_member, project: project, user: user) + create(:project_member, project: project, user: user_2) + create(:project_member, project: project, user: user_3) + end + + context 'when the project is transferred from a group to a user namespace' do + let_it_be_with_refind(:user_namespace) { user.namespace } + + before do + create(:group_member, group: root_group, user: user_3) + project.update!(namespace: user_namespace) + end + + it 'removes outdated seat assignments in the group' do + expect do + worker.perform(root_group.id, project.id) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-2) + end + + it 'leaves seat assignments of old root namespace members' do + worker.perform(root_group.id, project.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to contain_exactly(user_3.id) + end + + context 'when there are no outdated seat assignments' do + before do + create(:group_member, group: root_group, user: user) + create(:group_member, group: root_group, user: user_2) + end + + it 'does not remove seat assignments' do + expect do + worker.perform(root_group.id, project.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count } + end + end + end + + context 'when a project is transferred from a root group to a group in another namespace' do + let_it_be_with_refind(:new_root_group) { create(:group_with_plan, plan: :ultimate_plan) } + + before do + project.update!(namespace: new_root_group) + end + + context 'with remaining group members' do + before do + create(:group_member, group: root_group, user: user_3) + end + + it 'removes outdated seat assignments in the old root group' do + expect do + worker.perform(root_group.id, project.id) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-2) + end + + it 'leaves seat assignments of old root group' do + worker.perform(root_group.id, project.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to contain_exactly(user_3.id) + end + + context 'when there are no outdated seat assignments' do + before do + create(:group_member, group: root_group, user: user) + create(:group_member, group: root_group, user: user_2) + end + + it 'does not remove seat assignments' do + expect do + worker.perform(root_group.id, project.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count } + end + end + end + + context 'when the group has no remaining members after project transfer' do + it 'removes all seat assignments' do + expect(GitlabSubscriptions::SeatAssignment.by_namespace(root_group)).not_to be_empty + + worker.perform(root_group.id, project.id) + + expect(GitlabSubscriptions::SeatAssignment.by_namespace(root_group)).to be_empty + end + end + + context 'when there are remaining members via another project' do + let_it_be_with_refind(:project_2) { create(:project, namespace: root_group) } + let_it_be(:project_member) { create(:project_member, project: project_2, user: user) } + + before do + project.update!(namespace: new_root_group) + end + + it 'removes only the seats of non members' do + expect do + worker.perform(root_group.id, project.id) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-2) + end + + it 'leaves the seat of the members of the other project still in hierarchy' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to contain_exactly(user.id, user_2.id, user_3.id) + + worker.perform(root_group.id, project.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to contain_exactly(project_member.user_id) + end + end + end + + context 'when the project is transferred from a root group to a subgroup in the same hierarchy' do + let_it_be_with_refind(:sub_group) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } + + before do + project.update!(namespace: sub_group) + end + + it 'does not remove seat assignments' do + expect do + worker.perform(root_group.id, project.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count } + end + end + end + end +end