From f93195756b3302f29d8ef3040751393662778a90 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 13:32:35 -0400 Subject: [PATCH 01/14] Add seat removal worker --- .../member_transfers/remove_seats_worker.rb | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb new file mode 100644 index 00000000000000..34542f80afe808 --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class RemoveSeatsWorker + 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! + + BATCH_SIZE = 1000 + + def perform(old_root_namespace_id, namespace) + return unless namespace + + remove_outdated_seat_assignments(old_root_namespace_id, namespace) + end + + private + + def remove_outdated_seat_assignments(old_root_namespace_id, namespace) + old_root_namespace = Namespace.find_by_id(old_root_namespace_id) + + user_ids_in_transferred_namespace_tree = collect_user_ids(namespace) + + if top_group_to_subgroup?(old_root_namespace_id, namespace.id, namespace.root_ancestor.id) + delete_seats(old_root_namespace, user_ids_in_transferred_namespace_tree) + return + end + + member_ids_in_old_root_namespace = collect_user_ids( + old_root_namespace, + filter_user_ids: user_ids_in_transferred_namespace_tree + ) + + outdated_seat_user_ids = user_ids_in_transferred_namespace_tree.reject do |id| + member_ids_in_old_root_namespace.include?(id) + end + + delete_seats(old_root_namespace, outdated_seat_user_ids) + end + + def delete_seats(namespace, user_ids) + return if user_ids.empty? + + # rubocop:disable CodeReuse/ActiveRecord -- We need this to delete the outdated seat assignments + user_ids.each_slice(BATCH_SIZE) do |batch| + ::GitlabSubscriptions::SeatAssignment.where( + namespace_id: namespace.id, + user_id: batch + ).delete_all + end + # rubocop:enable CodeReuse/ActiveRecord + end + + def top_group_to_subgroup?(old_root_namespace_id, namespace_id, new_root_namespace_id) + old_root_namespace_id == namespace_id && new_root_namespace_id != old_root_namespace_id + end + + def collect_user_ids(namespace, filter_user_ids: nil) + return [] if namespace.nil? + + scope = + if namespace.is_a?(Group) + namespace.hierarchy_members + elsif namespace.is_a?(Project) + namespace.project_members + else + ::Member.in_hierarchy(namespace) + end + + # rubocop:disable CodeReuse/ActiveRecord -- We need this to obtain the user ids of the members + scope = scope.where(user_id: filter_user_ids) if filter_user_ids + scope.pluck(:user_id) + # rubocop:enable CodeReuse/ActiveRecord + end + end + end + end +end -- GitLab From 9b1405ba3112da8d958a613634052b7fafad3433 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 13:35:14 -0400 Subject: [PATCH 02/14] Add seat removal worker tests --- .../remove_seats_worker_spec.rb | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb new file mode 100644 index 00000000000000..362efd4a109284 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb @@ -0,0 +1,158 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker, :saas, feature_category: :seat_cost_management do + let(:worker) { described_class.new } + let_it_be(:user) { create(:user, :with_namespace) } + let_it_be(:user_2) { create(:user) } + + describe '#perform' do + context 'with group namespaces' do + 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 + + context 'when a root group namespace has outdated seat assignments after a subgroup transfer' do + let_it_be(:remaining_member) { create(:group_member, group: root_group, user: user) } + 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) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-1) + end + + it 'leaves seat assignments of group members' do + worker.perform(root_group.id, transferred_group) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to match([remaining_member.user_id]) + end + end + + context 'when a root namespace is transferred and becomes a subgroup of another group' 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 seat assignments from old root namespace' do + expect do + worker.perform(root_group.id, root_group) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-2) + end + end + + context 'when a subgroup is transferred within the same hierarchy' do + let_it_be(:parent_subgroup) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } + let_it_be(:subgroup) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } + + before do + create(:group_member, group: subgroup, user: user) + create(:group_member, group: subgroup, user: user_2) + subgroup.update!(parent: parent_subgroup) + end + + it 'does not remove seats' do + expect do + worker.perform(root_group.id, subgroup) + 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) + 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) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) + ).to match(::Member.in_hierarchy(root_group).pluck(:user_id).uniq) + end + end + end + + context 'with projects in user namespaces' do + 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) + create(:project_member, project: project, user: user_2) + project.update!(namespace: root_group) + end + + context 'when the user namespace still has members via another project' do + let_it_be(:project_2) { create(:project, namespace: user.namespace) } + + it 'removes outdated seat assignments and leaves the seat of the remaining member' do + expect do + worker.perform(user.namespace.id, project) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count }.by(-1) + end + + it 'leaves the seat of the remaining member' do + worker.perform(user.namespace.id, project) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + ).to match(project_2.project_members.pluck(:user_id)) + end + + context 'when there are no outdated seat assignments' do + before do + create(:project_member, project: project_2, user: user_2) + end + + it 'does not remove seat assignments' do + expect do + worker.perform(user.namespace.id, project) + 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) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(project_2.namespace).pluck(:user_id) + ).to match(::Member.in_hierarchy(user.namespace).pluck(:user_id).uniq) + end + end + end + + context 'when the user namespace has no remaining members after project transfer' do + it 'removes outdated seat assignments and leaves the seat of the remaining member' do + worker.perform(user.namespace.id, project) + + expect(GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace)).to be_empty + end + end + end + end + end +end -- GitLab From e0b3e63ed6f99fbf59e44093184ee1713da7afbf Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 13:36:18 -0400 Subject: [PATCH 03/14] Add sidekiq and all queues yml files --- config/sidekiq_queues.yml | 2 ++ ee/app/workers/all_queues.yml | 10 ++++++++++ spec/requests/api/group_variables_spec.rb | 1 + 3 files changed, 13 insertions(+) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index a9ed99b46c1bbf..908830d4a7208b 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -491,6 +491,8 @@ - 1 - - gitlab_subscriptions_seat_assignments_member_transfers_create_project_seats - 1 +- - gitlab_subscriptions_seat_assignments_member_transfers_remove_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 71eee7a2922b2b..f04f53b3f0b020 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2483,6 +2483,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: gitlab_subscriptions_seat_assignments_member_transfers_remove_seats + :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker + :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/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 103a1800c4c293..e0e42b7596fe63 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -201,6 +201,7 @@ end it 'returns a variable limit error' do + expect do post api("/groups/#{group.id}/variables", user), params: { key: 'TOO_MANY_VARS', value: 'too many' } end.not_to change { group.variables.count } -- GitLab From c438af8bac93f3c204257b25543ed28dfc82089a Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 17:46:16 -0400 Subject: [PATCH 04/14] Fix failing test --- .../remove_seats_worker_spec.rb | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb index 362efd4a109284..8db0d4f756dc60 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' -RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker, :saas, feature_category: :seat_cost_management do +RSpec.describe ::GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker, :saas, feature_category: :seat_cost_management do let(:worker) { described_class.new } - let_it_be(:user) { create(:user, :with_namespace) } + let_it_be_with_refind(:user) { create(:user, :with_namespace) } let_it_be(:user_2) { create(:user) } describe '#perform' do @@ -96,60 +96,64 @@ end context 'with projects in user namespaces' do - let_it_be_with_refind(:project) { create(:project, namespace: user.namespace) } + 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) - create(:project_member, project: project, user: user_2) - project.update!(namespace: root_group) + 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(:project_2) { create(:project, namespace: user.namespace) } + 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(user.namespace.id, project) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count }.by(-1) + worker.perform(project.namespace.id, project_2) + 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) + worker.perform(user_namespace, project_2) expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).to match(project_2.project_members.pluck(:user_id)) + GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace).pluck(:user_id) + ).to match(project.project_members.pluck(:user_id)) end context 'when there are no outdated seat assignments' do before do - create(:project_member, project: project_2, user: user_2) + create(:project_member, project: project, user: user_2) end it 'does not remove seat assignments' do expect do - worker.perform(user.namespace.id, project) - end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count } + worker.perform(user_namespace.id, project_2) + 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) + worker.perform(user_namespace.id, project_2) expect( - GitlabSubscriptions::SeatAssignment.by_namespace(project_2.namespace).pluck(:user_id) - ).to match(::Member.in_hierarchy(user.namespace).pluck(:user_id).uniq) + GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) + ).to match(::Member.in_hierarchy(user_namespace).pluck(:user_id).uniq) end end end context 'when the user namespace has no remaining members after project transfer' do it 'removes outdated seat assignments and leaves the seat of the remaining member' do - worker.perform(user.namespace.id, project) + worker.perform(user_namespace.id, project_2) - expect(GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace)).to be_empty + expect(GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace)).to be_empty end end end -- GitLab From c885a948492c43547f560f0abbc80086de2dd12b Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 11 Jun 2025 21:33:16 -0400 Subject: [PATCH 05/14] Rearrange seat removal tests --- .../remove_seats_worker_spec.rb | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb index 8db0d4f756dc60..d38cc20d8b2e49 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb @@ -5,7 +5,7 @@ RSpec.describe ::GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker, :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(:user_2) { create(:user) } + let_it_be_with_refind(:user_2) { create(:user) } describe '#perform' do context 'with group namespaces' do @@ -127,34 +127,45 @@ 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) } - context 'when there are no outdated seat assignments' do - before do - create(:project_member, project: project, user: user_2) - end + 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) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace).count } + end - it 'does not remove seat assignments' do - expect do - worker.perform(user_namespace.id, project_2) - 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) - it 'ensures project members match the seat assignments for the project namespace' do - worker.perform(user_namespace.id, project_2) + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) + ).to match(::Member.in_hierarchy(user_namespace).pluck(:user_id).uniq) + end + end - 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 - context 'when the user namespace has no remaining members after project transfer' do - it 'removes outdated seat assignments and leaves the seat of the remaining member' do - worker.perform(user_namespace.id, project_2) + it 'removes outdated seat assignments and leaves the seat of the remaining member' do + worker.perform(user_namespace.id, project) - expect(GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace)).to be_empty - end + expect(GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace)).to be_empty end end end -- GitLab From 372056209a1266e6df5469e96a4935d46bc7af1d Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 17 Jun 2025 17:33:59 -0400 Subject: [PATCH 06/14] Add base remove seats worker and tests --- .../base_remove_seats_worker.rb | 63 +++++++++++++++++++ .../base_remove_seats_worker_spec.rb | 15 +++++ 2 files changed, 78 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker.rb create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb 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 00000000000000..2342711ccca0c5 --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +# rubocop:disable Scalability/IdempotentWorker -- Idempotent worker declaration in children +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class BaseRemoveSeatsWorker + 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 + + def top_group_to_subgroup?(old_root_namespace_id, current_namespace_id, new_root_namespace_id) + old_root_namespace_id == current_namespace_id && new_root_namespace_id != old_root_namespace_id + end + + def collect_group_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 + + def collect_project_user_ids(project, filter_user_ids: nil) + scope = project.project_members + scope = scope.where(user_id: filter_user_ids) if filter_user_ids + scope.pluck_user_ids + end + + def collect_user_namespace_user_ids(user_namespace, filter_user_ids: nil) + scope = Member.in_hierarchy(user_namespace) + scope = scope.where(user_id: filter_user_ids) if filter_user_ids + scope.pluck_user_ids + end + # rubocop: enable CodeReuse/ActiveRecord + + def collect_user_ids(source, filter_user_ids: nil) + return [] unless source + + case source + when Group + collect_group_user_ids(source, filter_user_ids: filter_user_ids) + else + collect_user_namespace_user_ids(source, filter_user_ids: filter_user_ids) + end + end + + def remove_outdated_seat_assignments(old_root_namespace_id, source) + raise NotImplementedError, "#{self.class.name} must implement #remove_outdated_seat_assignments" + end + end + end + end +end +# rubocop:enable Scalability/IdempotentWorker diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb new file mode 100644 index 00000000000000..c8b26cc5390809 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::GitlabSubscriptions::SeatAssignments::MemberTransfers::BaseRemoveSeatsWorker, :saas, feature_category: :seat_cost_management do + let(:worker) { described_class.new } + + describe '#remove_outdated_seat_assignments' do + it 'raises NotImplementedError' do + expect do + worker.send(:remove_outdated_seat_assignments, 1, double) + end.to raise_error(NotImplementedError) + end + end +end -- GitLab From 5e1d0cc08403190aeccc876f4b5279b3be4aab55 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 17 Jun 2025 17:37:58 -0400 Subject: [PATCH 07/14] Add remove group seats worker and tests --- .../remove_group_seats_worker.rb | 56 ++++++++ .../remove_group_seats_worker_spec.rb | 128 ++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker.rb create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker_spec.rb 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 00000000000000..6656265ddd5220 --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker.rb @@ -0,0 +1,56 @@ +# 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! + + BATCH_SIZE = 1000 + + def perform(old_root_namespace_id, group_id) + group = Group.find_by_id(group_id) + return unless group + + remove_outdated_seat_assignments(old_root_namespace_id, group) + end + + private + + def remove_outdated_seat_assignments(old_root_namespace_id, group) + old_root_namespace = Namespace.find_by_id(old_root_namespace_id) + return unless old_root_namespace + + user_ids_in_transferred_group_hierarchy = collect_group_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 + + member_ids_in_old_root_namespace = collect_user_ids( + old_root_namespace, + filter_user_ids: user_ids_in_transferred_group_hierarchy + ) + + member_ids_set = member_ids_in_old_root_namespace.to_set + + outdated_user_ids = user_ids_in_transferred_group_hierarchy.reject do |id| + member_ids_set.include?(id) + end + + delete_seats(old_root_namespace, outdated_user_ids) + end + 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 00000000000000..ede10a31c39979 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_group_seats_worker_spec.rb @@ -0,0 +1,128 @@ +# 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' 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_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 + end + end +end -- GitLab From 0c2d724426c900ff5da5432618e0265b80581d72 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 17 Jun 2025 17:40:49 -0400 Subject: [PATCH 08/14] Add remove project seats worker and tests --- .../remove_project_seats_worker.rb | 56 +++++ .../remove_project_seats_worker_spec.rb | 229 ++++++++++++++++++ 2 files changed, 285 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker.rb create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_project_seats_worker_spec.rb 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 00000000000000..a39e25a7ea4628 --- /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! + + BATCH_SIZE = 1000 + + def perform(old_root_namespace_id, project_id) + project = Project.find_by_id(project_id) + return unless project + + remove_outdated_seat_assignments(old_root_namespace_id, project) + end + + private + + 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_project_user_ids(project) + + if top_group_to_subgroup?(old_root_namespace.id, project.namespace.id, project.namespace.root_ancestor.id) + delete_seats(old_root_namespace, user_ids_in_transferred_project) + return + end + + member_ids_in_old_root_namespace = collect_user_ids( + old_root_namespace, + filter_user_ids: user_ids_in_transferred_project + ) + + member_ids_set = member_ids_in_old_root_namespace.to_set + + outdated_user_ids = user_ids_in_transferred_project.reject do |id| + member_ids_set.include?(id) + end + + delete_seats(old_root_namespace, outdated_user_ids) + 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 00000000000000..4734343abeda80 --- /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 -- GitLab From ebbe6d648d7cc17b2eca5dc674c06b8be57aa610 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 17 Jun 2025 17:41:55 -0400 Subject: [PATCH 09/14] Add queues yml files --- config/sidekiq_queues.yml | 4 ++++ ee/app/workers/all_queues.yml | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 908830d4a7208b..a4defe4e763406 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_seat_assignments_member_transfers_remove_seats - 1 - - gitlab_subscriptions_self_managed_duo_core_todo_notification diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index f04f53b3f0b020..e986ddc7875cb7 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_seat_assignments_member_transfers_remove_seats :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker :feature_category: :seat_cost_management -- GitLab From 90442a56b93dfe616923a38ea9ee8d6eb9b2722a Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 11:33:46 -0400 Subject: [PATCH 10/14] Cleanup base worker --- .../base_remove_seats_worker.rb | 33 ++----------------- 1 file changed, 2 insertions(+), 31 deletions(-) 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 index 2342711ccca0c5..448bb7085762ad 100644 --- 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 @@ -18,43 +18,14 @@ def delete_seats(source, user_ids) ).delete_all end end - - def top_group_to_subgroup?(old_root_namespace_id, current_namespace_id, new_root_namespace_id) - old_root_namespace_id == current_namespace_id && new_root_namespace_id != old_root_namespace_id - end - - def collect_group_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 - - def collect_project_user_ids(project, filter_user_ids: nil) - scope = project.project_members - scope = scope.where(user_id: filter_user_ids) if filter_user_ids - scope.pluck_user_ids - end - - def collect_user_namespace_user_ids(user_namespace, filter_user_ids: nil) - scope = Member.in_hierarchy(user_namespace) - scope = scope.where(user_id: filter_user_ids) if filter_user_ids - scope.pluck_user_ids - end # rubocop: enable CodeReuse/ActiveRecord def collect_user_ids(source, filter_user_ids: nil) - return [] unless source - - case source - when Group - collect_group_user_ids(source, filter_user_ids: filter_user_ids) - else - collect_user_namespace_user_ids(source, filter_user_ids: filter_user_ids) - end + raise NotImplementedError end def remove_outdated_seat_assignments(old_root_namespace_id, source) - raise NotImplementedError, "#{self.class.name} must implement #remove_outdated_seat_assignments" + raise NotImplementedError end end end -- GitLab From d9a87e798c69a1e76a1ed84b237b847c39f7c2fb Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 11:36:16 -0400 Subject: [PATCH 11/14] Refine remove group seats worker and tests --- .../remove_group_seats_worker.rb | 16 ++++++++++++++-- .../remove_group_seats_worker_spec.rb | 14 ++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) 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 index 6656265ddd5220..62b45e289e9ef5 100644 --- 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 @@ -27,10 +27,10 @@ def perform(old_root_namespace_id, group_id) private def remove_outdated_seat_assignments(old_root_namespace_id, group) - old_root_namespace = Namespace.find_by_id(old_root_namespace_id) + old_root_namespace = Group.find_by_id(old_root_namespace_id) return unless old_root_namespace - user_ids_in_transferred_group_hierarchy = collect_group_user_ids(group) + 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) @@ -50,6 +50,18 @@ def remove_outdated_seat_assignments(old_root_namespace_id, group) 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 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 index ede10a31c39979..3c1edc2a0999a1 100644 --- 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 @@ -17,7 +17,7 @@ end describe '#perform' do - context 'when a root group is transferred to become a subgroup of another group' 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 @@ -83,7 +83,7 @@ end end - describe '#collect_user_ids' do + 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) } @@ -123,6 +123,16 @@ 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 -- GitLab From 931fd1f8ec1d9d6e199d88ead408c5c239f8ccdf Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 11:39:58 -0400 Subject: [PATCH 12/14] Refine remove project seats worker and tests --- .../remove_project_seats_worker.rb | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) 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 index a39e25a7ea4628..60e533b7519adb 100644 --- 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 @@ -30,12 +30,7 @@ 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_project_user_ids(project) - - if top_group_to_subgroup?(old_root_namespace.id, project.namespace.id, project.namespace.root_ancestor.id) - delete_seats(old_root_namespace, user_ids_in_transferred_project) - return - end + user_ids_in_transferred_project = collect_user_ids(project) member_ids_in_old_root_namespace = collect_user_ids( old_root_namespace, @@ -50,6 +45,25 @@ def remove_outdated_seat_assignments(old_root_namespace_id, 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 -- GitLab From a7870ffa2d8cde15879b20d7047a44adf1879ad8 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 12:16:00 -0400 Subject: [PATCH 13/14] Move logic to base worker --- .../base_remove_seats_worker.rb | 26 ++++++++++++++++++- .../remove_group_seats_worker.rb | 24 ++++------------- .../remove_project_seats_worker.rb | 22 +++------------- .../base_remove_seats_worker_spec.rb | 15 ----------- 4 files changed, 34 insertions(+), 53 deletions(-) delete mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb 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 index 448bb7085762ad..b7ce4ba96c484c 100644 --- 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 @@ -5,6 +5,15 @@ 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 @@ -20,11 +29,26 @@ def delete_seats(source, user_ids) end # rubocop: enable CodeReuse/ActiveRecord + def collect_outdated_user_ids(old_root_source, user_ids_in_transferred_group_hierarchy) + member_ids_in_old_root_source = collect_user_ids( + old_root_source, + filter_user_ids: user_ids_in_transferred_group_hierarchy + ).to_set + + user_ids_in_transferred_group_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_namespace_id, source) + def remove_outdated_seat_assignments(old_root_source_id, source) + raise NotImplementedError + end + + def find_source_by_id(source_id) raise NotImplementedError end end 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 index 62b45e289e9ef5..3abf5a21c35b83 100644 --- 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 @@ -15,19 +15,14 @@ class RemoveGroupSeatsWorker < BaseRemoveSeatsWorker idempotent! - BATCH_SIZE = 1000 - - def perform(old_root_namespace_id, group_id) - group = Group.find_by_id(group_id) - return unless group + private - remove_outdated_seat_assignments(old_root_namespace_id, group) + def find_source_by_id(group_id) + Group.find_by_id(group_id) end - private - def remove_outdated_seat_assignments(old_root_namespace_id, group) - old_root_namespace = Group.find_by_id(old_root_namespace_id) + 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) @@ -37,16 +32,7 @@ def remove_outdated_seat_assignments(old_root_namespace_id, group) return end - member_ids_in_old_root_namespace = collect_user_ids( - old_root_namespace, - filter_user_ids: user_ids_in_transferred_group_hierarchy - ) - - member_ids_set = member_ids_in_old_root_namespace.to_set - - outdated_user_ids = user_ids_in_transferred_group_hierarchy.reject do |id| - member_ids_set.include?(id) - 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 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 index 60e533b7519adb..94675806257878 100644 --- 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 @@ -15,33 +15,19 @@ class RemoveProjectSeatsWorker < BaseRemoveSeatsWorker idempotent! - BATCH_SIZE = 1000 - - def perform(old_root_namespace_id, project_id) - project = Project.find_by_id(project_id) - return unless project + private - remove_outdated_seat_assignments(old_root_namespace_id, project) + def find_source_by_id(project_id) + Project.find_by_id(project_id) end - private - 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) - member_ids_in_old_root_namespace = collect_user_ids( - old_root_namespace, - filter_user_ids: user_ids_in_transferred_project - ) - - member_ids_set = member_ids_in_old_root_namespace.to_set - - outdated_user_ids = user_ids_in_transferred_project.reject do |id| - member_ids_set.include?(id) - end + outdated_user_ids = collect_outdated_user_ids(old_root_namespace, user_ids_in_transferred_project) delete_seats(old_root_namespace, outdated_user_ids) end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb deleted file mode 100644 index c8b26cc5390809..00000000000000 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_remove_seats_worker_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::GitlabSubscriptions::SeatAssignments::MemberTransfers::BaseRemoveSeatsWorker, :saas, feature_category: :seat_cost_management do - let(:worker) { described_class.new } - - describe '#remove_outdated_seat_assignments' do - it 'raises NotImplementedError' do - expect do - worker.send(:remove_outdated_seat_assignments, 1, double) - end.to raise_error(NotImplementedError) - end - end -end -- GitLab From 9443030a60c896fa91c790c2e96485aa237d0c65 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 12:22:23 -0400 Subject: [PATCH 14/14] Remove old worker implementation --- config/sidekiq_queues.yml | 2 - ee/app/workers/all_queues.yml | 10 - .../base_remove_seats_worker.rb | 6 +- .../member_transfers/remove_seats_worker.rb | 87 --------- .../remove_seats_worker_spec.rb | 173 ------------------ spec/requests/api/group_variables_spec.rb | 1 - 6 files changed, 3 insertions(+), 276 deletions(-) delete mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb delete mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index a4defe4e763406..193127a21a82f8 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -495,8 +495,6 @@ - 1 - - gitlab_subscriptions_seat_assignments_member_transfers_remove_project_seats - 1 -- - gitlab_subscriptions_seat_assignments_member_transfers_remove_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 e986ddc7875cb7..20e28393042259 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2503,16 +2503,6 @@ :idempotent: true :tags: [] :queue_namespace: -- :name: gitlab_subscriptions_seat_assignments_member_transfers_remove_seats - :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker - :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 index b7ce4ba96c484c..cc8c54a9f52d76 100644 --- 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 @@ -29,13 +29,13 @@ def delete_seats(source, user_ids) end # rubocop: enable CodeReuse/ActiveRecord - def collect_outdated_user_ids(old_root_source, user_ids_in_transferred_group_hierarchy) + 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_group_hierarchy + filter_user_ids: user_ids_in_transferred_source_hierarchy ).to_set - user_ids_in_transferred_group_hierarchy.reject do |id| + user_ids_in_transferred_source_hierarchy.reject do |id| member_ids_in_old_root_source.include?(id) end end diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb deleted file mode 100644 index 34542f80afe808..00000000000000 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker.rb +++ /dev/null @@ -1,87 +0,0 @@ -# frozen_string_literal: true - -module GitlabSubscriptions - module SeatAssignments - module MemberTransfers - class RemoveSeatsWorker - 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! - - BATCH_SIZE = 1000 - - def perform(old_root_namespace_id, namespace) - return unless namespace - - remove_outdated_seat_assignments(old_root_namespace_id, namespace) - end - - private - - def remove_outdated_seat_assignments(old_root_namespace_id, namespace) - old_root_namespace = Namespace.find_by_id(old_root_namespace_id) - - user_ids_in_transferred_namespace_tree = collect_user_ids(namespace) - - if top_group_to_subgroup?(old_root_namespace_id, namespace.id, namespace.root_ancestor.id) - delete_seats(old_root_namespace, user_ids_in_transferred_namespace_tree) - return - end - - member_ids_in_old_root_namespace = collect_user_ids( - old_root_namespace, - filter_user_ids: user_ids_in_transferred_namespace_tree - ) - - outdated_seat_user_ids = user_ids_in_transferred_namespace_tree.reject do |id| - member_ids_in_old_root_namespace.include?(id) - end - - delete_seats(old_root_namespace, outdated_seat_user_ids) - end - - def delete_seats(namespace, user_ids) - return if user_ids.empty? - - # rubocop:disable CodeReuse/ActiveRecord -- We need this to delete the outdated seat assignments - user_ids.each_slice(BATCH_SIZE) do |batch| - ::GitlabSubscriptions::SeatAssignment.where( - namespace_id: namespace.id, - user_id: batch - ).delete_all - end - # rubocop:enable CodeReuse/ActiveRecord - end - - def top_group_to_subgroup?(old_root_namespace_id, namespace_id, new_root_namespace_id) - old_root_namespace_id == namespace_id && new_root_namespace_id != old_root_namespace_id - end - - def collect_user_ids(namespace, filter_user_ids: nil) - return [] if namespace.nil? - - scope = - if namespace.is_a?(Group) - namespace.hierarchy_members - elsif namespace.is_a?(Project) - namespace.project_members - else - ::Member.in_hierarchy(namespace) - end - - # rubocop:disable CodeReuse/ActiveRecord -- We need this to obtain the user ids of the members - scope = scope.where(user_id: filter_user_ids) if filter_user_ids - scope.pluck(:user_id) - # rubocop:enable CodeReuse/ActiveRecord - end - end - end - end -end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb deleted file mode 100644 index d38cc20d8b2e49..00000000000000 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/remove_seats_worker_spec.rb +++ /dev/null @@ -1,173 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::GitlabSubscriptions::SeatAssignments::MemberTransfers::RemoveSeatsWorker, :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 'with group namespaces' do - 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 - - context 'when a root group namespace has outdated seat assignments after a subgroup transfer' do - let_it_be(:remaining_member) { create(:group_member, group: root_group, user: user) } - 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) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-1) - end - - it 'leaves seat assignments of group members' do - worker.perform(root_group.id, transferred_group) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) - ).to match([remaining_member.user_id]) - end - end - - context 'when a root namespace is transferred and becomes a subgroup of another group' 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 seat assignments from old root namespace' do - expect do - worker.perform(root_group.id, root_group) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(root_group).count }.by(-2) - end - end - - context 'when a subgroup is transferred within the same hierarchy' do - let_it_be(:parent_subgroup) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } - let_it_be(:subgroup) { create(:group_with_plan, plan: :ultimate_plan, parent: root_group) } - - before do - create(:group_member, group: subgroup, user: user) - create(:group_member, group: subgroup, user: user_2) - subgroup.update!(parent: parent_subgroup) - end - - it 'does not remove seats' do - expect do - worker.perform(root_group.id, subgroup) - 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) - 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) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(root_group).pluck(:user_id) - ).to match(::Member.in_hierarchy(root_group).pluck(:user_id).uniq) - end - end - end - - context 'with projects in user namespaces' 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) - 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, project_2) - - 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) - 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) - - 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 outdated seat assignments and leaves the seat of the remaining member' do - worker.perform(user_namespace.id, project) - - expect(GitlabSubscriptions::SeatAssignment.by_namespace(user_namespace)).to be_empty - end - end - end - end -end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index e0e42b7596fe63..103a1800c4c293 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -201,7 +201,6 @@ end it 'returns a variable limit error' do - expect do post api("/groups/#{group.id}/variables", user), params: { key: 'TOO_MANY_VARS', value: 'too many' } end.not_to change { group.variables.count } -- GitLab