From 08198f213b07d4c142591a03357ddef2ed166736 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 17:56:12 -0400 Subject: [PATCH 01/12] Add create seats workers These workers are responsible for creating seat assignments when a namespace is transferred. The namespace can be either a group or a project. It ensures that users in the transferred hierarchy have the appropriate seat assignments under the new root namespace. --- .../member_transfers/create_seats_worker.rb | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb new file mode 100644 index 00000000000000..2a95f23cfd175a --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class CreateSeatsWorker + 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! + + def perform(namespace) + return unless namespace + + create_missing_seat_assignments(namespace) + end + + private + + def create_missing_seat_assignments(namespace) + users_ids_in_namespace_tree = collect_user_ids(namespace) + + seat_assignments = users_ids_in_namespace_tree.map do |user_id| + { + namespace_id: namespace.root_ancestor.id, + user_id: user_id, + organization_id: namespace.organization_id || Organizations::Organization::DEFAULT_ORGANIZATION_ID + } + end + + seat_assignments.each_slice(1000) do |batch| + ::GitlabSubscriptions::SeatAssignment.insert_all( + batch, + unique_by: [:namespace_id, :user_id] + ) + end + end + + def collect_user_ids(namespace) + 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.pluck(:user_id) + # rubocop:enable CodeReuse/ActiveRecord + end + end + end + end +end -- GitLab From 6f7dce141bfc139e4241c14a07e99e8f76d8e329 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 17:57:43 -0400 Subject: [PATCH 02/12] Add create worker tests --- .../create_seats_worker_spec.rb | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb new file mode 100644 index 00000000000000..f49622d89dac77 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateSeatsWorker, :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) } + let_it_be(:user_3) { create(:user) } + + describe '#perform' do + context 'with group namespaces' do + let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } + + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: group) + end + + context 'when group namespace has missing seats' do + before do + create(:group_member, group: group, user: user) + create(:group_member, group: group, user: user_2) + create(:group_member, group: group, user: user_3) + end + + it 'creates the missing seats based on the group members' do + expect do + worker.perform(group) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count }.by(2) + end + + it 'creates seat assignments for the missing users' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).not_to include(user_2.id, user_3.id) + + worker.perform(group) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to include(user_2.id, user_3.id) + end + + it 'creates seat assignments for all members' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).not_to match(::Member.in_hierarchy(group).pluck(:user_id)) + + worker.perform(group) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match(::Member.in_hierarchy(group).pluck(:user_id)) + end + end + + context 'when a group namespace does not have missing seat assignments' do + before do + create(:group_member, group: group, user: user) + create(:group_member, group: group, user: user_2) + create(:group_member, group: group, user: user_3) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: group) + create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: group) + end + + it 'does not create seat assignments' do + expect do + worker.perform(group) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) } + end + end + end + + context 'with user namespaces' do + let_it_be_with_refind(:project) { create(:project, namespace: user.namespace) } + + context 'when the user namespace has missing seat assignments' do + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: project.namespace) + create(:project_member, project: project, user: user_2) + create(:project_member, project: project, user: user_3) + end + + it 'creates the missing seats' do + expect do + worker.perform(project) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count }.by(2) + end + + it 'creates seat assignments for the missing users' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + ).not_to include(user_2.id, user_3.id) + + worker.perform(project) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + ).to include(user_2.id, user_3.id) + end + + it 'creates seat assignments for all members' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + ).not_to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) + + worker.perform(project) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + ).to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) + end + end + + context 'when a group namespace does not have missing seat assignments' do + before do + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: project.namespace) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: project.namespace) + create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: project.namespace) + create(:project_member, project: project, user: user_2) + create(:project_member, project: project, user: user_3) + end + + it 'does not create seat assignments' do + expect do + worker.perform(project) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) } + end + end + end + end +end -- GitLab From 037a35158060058760906986e93d0893614c7033 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 18:02:16 -0400 Subject: [PATCH 03/12] Add changes to all queues file --- ee/app/workers/all_queues.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 71eee7a2922b2b..2a88693826e292 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_create_seats + :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateSeatsWorker + :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 -- GitLab From 7364ed97d0740adf90dc6cd7dbb784aa317804ff Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 18:03:35 -0400 Subject: [PATCH 04/12] Add changes to sidekiq queues file --- config/sidekiq_queues.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index a9ed99b46c1bbf..87141c0363f917 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_create_seats + - 1 - - gitlab_subscriptions_self_managed_duo_core_todo_notification - 1 - - gitlab_subscriptions_trials_apply_trial -- GitLab From 8b67c120eb00746e8b1d6d0687b4f38fb589b4f3 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 10:03:56 -0400 Subject: [PATCH 05/12] Add duo review feedback changes --- .../member_transfers/create_seats_worker.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb index 2a95f23cfd175a..97d217e2aa379f 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb @@ -15,9 +15,9 @@ class CreateSeatsWorker idempotent! - def perform(namespace) - return unless namespace + BATCH_SIZE = 1000 + def perform(namespace) create_missing_seat_assignments(namespace) end @@ -34,7 +34,7 @@ def create_missing_seat_assignments(namespace) } end - seat_assignments.each_slice(1000) do |batch| + seat_assignments.each_slice(BATCH_SIZE) do |batch| ::GitlabSubscriptions::SeatAssignment.insert_all( batch, unique_by: [:namespace_id, :user_id] -- GitLab From 7c1792451686982ef717edfd369b6132355b5ea8 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 14:06:34 +0000 Subject: [PATCH 06/12] Add gitlab duo suggested change --- .../seat_assignments/member_transfers/create_seats_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb index 97d217e2aa379f..43a0f964e1f707 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb @@ -30,7 +30,7 @@ def create_missing_seat_assignments(namespace) { namespace_id: namespace.root_ancestor.id, user_id: user_id, - organization_id: namespace.organization_id || Organizations::Organization::DEFAULT_ORGANIZATION_ID + organization_id: namespace.organization_id.presence || Organizations::Organization::DEFAULT_ORGANIZATION_ID } end -- GitLab From a8d469faa7a8d0f988bd3a6342718784f977e41f Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 10:34:33 -0400 Subject: [PATCH 07/12] Fix rubocop error --- .../seat_assignments/member_transfers/create_seats_worker.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb index 43a0f964e1f707..01674e0ff1b91d 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb @@ -30,7 +30,8 @@ def create_missing_seat_assignments(namespace) { namespace_id: namespace.root_ancestor.id, user_id: user_id, - organization_id: namespace.organization_id.presence || Organizations::Organization::DEFAULT_ORGANIZATION_ID + organization_id: namespace.organization_id.presence || + Organizations::Organization::DEFAULT_ORGANIZATION_ID } end -- GitLab From d3dcdd395d7adebfec92c0cfb475579215d914e0 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 12 Jun 2025 22:59:12 -0400 Subject: [PATCH 08/12] Address MR feedback --- .../member_transfers/create_seats_worker.rb | 23 +++++++++--------- .../create_seats_worker_spec.rb | 24 ++++++++++++------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb index 01674e0ff1b91d..767fb4f2721115 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb @@ -17,7 +17,10 @@ class CreateSeatsWorker BATCH_SIZE = 1000 - def perform(namespace) + def perform(namespace_id) + namespace = Namespace.find_by_id(namespace_id) + return unless namespace + create_missing_seat_assignments(namespace) end @@ -43,21 +46,19 @@ def create_missing_seat_assignments(namespace) end end - def collect_user_ids(namespace) - return [] if namespace.nil? + def collect_user_ids(source) + return [] if source.nil? scope = - if namespace.is_a?(Group) - namespace.hierarchy_members - elsif namespace.is_a?(Project) - namespace.project_members + if source.is_a?(Group) + source.hierarchy_members + elsif source.is_a?(Project) + source.project_members else - ::Member.in_hierarchy(namespace) + ::Member.in_hierarchy(source) end - # rubocop:disable CodeReuse/ActiveRecord -- We need this to obtain the user ids of the members - scope.pluck(:user_id) - # rubocop:enable CodeReuse/ActiveRecord + scope.pluck_user_ids end end end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb index f49622d89dac77..5d63aa1a9750fc 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb @@ -9,6 +9,14 @@ let_it_be(:user_3) { create(:user) } describe '#perform' do + context 'when the namespace is nil' do + it 'does nothing' do + expect(worker).not_to receive(:create_missing_seat_assignments) + + worker.perform(non_existing_record_id) + end + end + context 'with group namespaces' do let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } @@ -25,7 +33,7 @@ it 'creates the missing seats based on the group members' do expect do - worker.perform(group) + worker.perform(group.id) end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count }.by(2) end @@ -34,7 +42,7 @@ GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) ).not_to include(user_2.id, user_3.id) - worker.perform(group) + worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) @@ -46,7 +54,7 @@ GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) ).not_to match(::Member.in_hierarchy(group).pluck(:user_id)) - worker.perform(group) + worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) @@ -65,7 +73,7 @@ it 'does not create seat assignments' do expect do - worker.perform(group) + worker.perform(group.id) end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) } end end @@ -83,7 +91,7 @@ it 'creates the missing seats' do expect do - worker.perform(project) + worker.perform(project.namespace.id) end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count }.by(2) end @@ -92,7 +100,7 @@ GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) ).not_to include(user_2.id, user_3.id) - worker.perform(project) + worker.perform(project.namespace.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) @@ -104,7 +112,7 @@ GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) ).not_to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) - worker.perform(project) + worker.perform(project.namespace.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) @@ -123,7 +131,7 @@ it 'does not create seat assignments' do expect do - worker.perform(project) + worker.perform(project.namespace.id) end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) } end end -- GitLab From 3093e8ed0ecc3c3cdcd81e12388e55b5ee34e123 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Fri, 13 Jun 2025 12:28:28 -0400 Subject: [PATCH 09/12] Remove default organization --- .../seat_assignments/member_transfers/create_seats_worker.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb index 767fb4f2721115..3be2f4b698536b 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb @@ -33,8 +33,7 @@ def create_missing_seat_assignments(namespace) { namespace_id: namespace.root_ancestor.id, user_id: user_id, - organization_id: namespace.organization_id.presence || - Organizations::Organization::DEFAULT_ORGANIZATION_ID + organization_id: namespace.organization_id } end -- GitLab From b41f32320868e591741d079b4fbd8b50f048bbc3 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 16 Jun 2025 17:20:10 -0400 Subject: [PATCH 10/12] Remove general purpose worker and add queue yml files --- config/sidekiq_queues.yml | 2 - ee/app/workers/all_queues.yml | 10 -- .../member_transfers/create_seats_worker.rb | 65 -------- .../create_seats_worker_spec.rb | 140 ------------------ 4 files changed, 217 deletions(-) delete mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb delete mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 87141c0363f917..a9ed99b46c1bbf 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -491,8 +491,6 @@ - 1 - - gitlab_subscriptions_seat_assignments_member_transfers_create_project_seats - 1 -- - gitlab_subscriptions_seat_assignments_member_transfers_create_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 2a88693826e292..71eee7a2922b2b 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2483,16 +2483,6 @@ :idempotent: true :tags: [] :queue_namespace: -- :name: gitlab_subscriptions_seat_assignments_member_transfers_create_seats - :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateSeatsWorker - :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/create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb deleted file mode 100644 index 3be2f4b698536b..00000000000000 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -module GitlabSubscriptions - module SeatAssignments - module MemberTransfers - class CreateSeatsWorker - 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(namespace_id) - namespace = Namespace.find_by_id(namespace_id) - return unless namespace - - create_missing_seat_assignments(namespace) - end - - private - - def create_missing_seat_assignments(namespace) - users_ids_in_namespace_tree = collect_user_ids(namespace) - - seat_assignments = users_ids_in_namespace_tree.map do |user_id| - { - namespace_id: namespace.root_ancestor.id, - user_id: user_id, - organization_id: namespace.organization_id - } - end - - seat_assignments.each_slice(BATCH_SIZE) do |batch| - ::GitlabSubscriptions::SeatAssignment.insert_all( - batch, - unique_by: [:namespace_id, :user_id] - ) - end - end - - def collect_user_ids(source) - return [] if source.nil? - - scope = - if source.is_a?(Group) - source.hierarchy_members - elsif source.is_a?(Project) - source.project_members - else - ::Member.in_hierarchy(source) - end - - scope.pluck_user_ids - end - end - end - end -end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb deleted file mode 100644 index 5d63aa1a9750fc..00000000000000 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_seats_worker_spec.rb +++ /dev/null @@ -1,140 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateSeatsWorker, :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) } - let_it_be(:user_3) { create(:user) } - - describe '#perform' do - context 'when the namespace is nil' do - it 'does nothing' do - expect(worker).not_to receive(:create_missing_seat_assignments) - - worker.perform(non_existing_record_id) - end - end - - context 'with group namespaces' do - let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } - - before do - create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: group) - end - - context 'when group namespace has missing seats' do - before do - create(:group_member, group: group, user: user) - create(:group_member, group: group, user: user_2) - create(:group_member, group: group, user: user_3) - end - - it 'creates the missing seats based on the group members' do - expect do - worker.perform(group.id) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count }.by(2) - end - - it 'creates seat assignments for the missing users' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to include(user_2.id, user_3.id) - - worker.perform(group.id) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to include(user_2.id, user_3.id) - end - - it 'creates seat assignments for all members' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to match(::Member.in_hierarchy(group).pluck(:user_id)) - - worker.perform(group.id) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match(::Member.in_hierarchy(group).pluck(:user_id)) - end - end - - context 'when a group namespace does not have missing seat assignments' do - before do - create(:group_member, group: group, user: user) - create(:group_member, group: group, user: user_2) - create(:group_member, group: group, user: user_3) - create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: group) - create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: group) - end - - it 'does not create seat assignments' do - expect do - worker.perform(group.id) - end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) } - end - end - end - - context 'with user namespaces' do - let_it_be_with_refind(:project) { create(:project, namespace: user.namespace) } - - context 'when the user namespace has missing seat assignments' do - before do - create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: project.namespace) - create(:project_member, project: project, user: user_2) - create(:project_member, project: project, user: user_3) - end - - it 'creates the missing seats' do - expect do - worker.perform(project.namespace.id) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count }.by(2) - end - - it 'creates seat assignments for the missing users' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).not_to include(user_2.id, user_3.id) - - worker.perform(project.namespace.id) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).to include(user_2.id, user_3.id) - end - - it 'creates seat assignments for all members' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).not_to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) - - worker.perform(project.namespace.id) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) - end - end - - context 'when a group namespace does not have missing seat assignments' do - before do - create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: project.namespace) - create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: project.namespace) - create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: project.namespace) - create(:project_member, project: project, user: user_2) - create(:project_member, project: project, user: user_3) - end - - it 'does not create seat assignments' do - expect do - worker.perform(project.namespace.id) - end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) } - end - end - end - end -end -- GitLab From 0f2b0934afb398b178de0602a13968632a896e6f Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 14:13:26 -0400 Subject: [PATCH 11/12] Add create seats worker to transfer service --- ee/app/services/ee/groups/transfer_service.rb | 8 +++ .../ee/groups/transfer_service_spec.rb | 60 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/ee/app/services/ee/groups/transfer_service.rb b/ee/app/services/ee/groups/transfer_service.rb index ef2cafd060a1d6..b890a1e35ad62a 100644 --- a/ee/app/services/ee/groups/transfer_service.rb +++ b/ee/app/services/ee/groups/transfer_service.rb @@ -66,6 +66,7 @@ def post_update_hooks(updated_project_ids, old_root_ancestor_id) process_group_associations(old_root_ancestor_id, group) # Epics and WorkItems sync_security_policies(group, current_user) + reconcile_seat_assignments(old_root_ancestor_id, group) end def sync_security_policies(group, current_user) @@ -184,6 +185,13 @@ def remove_project_compliance_frameworks(project) ) end end + + def reconcile_seat_assignments(old_root_ancestor_id, group) + return unless ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) + return if old_root_ancestor_id == group.root_ancestor.id + + GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker.perform_async(group.id) + end end end end diff --git a/ee/spec/services/ee/groups/transfer_service_spec.rb b/ee/spec/services/ee/groups/transfer_service_spec.rb index 04088cb991c60f..29ce463938a664 100644 --- a/ee/spec/services/ee/groups/transfer_service_spec.rb +++ b/ee/spec/services/ee/groups/transfer_service_spec.rb @@ -618,4 +618,64 @@ end end end + + describe 'subscription seat assignments reconciliation' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + + context 'when a top level group becomes subgroup' do + it 'enqueues CreateSeatAssignmentsWorker worker with the group being transferred' do + expect(group.root_ancestor).to eq(group) + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) + .to receive(:perform_async).with(group.id) + + transfer_service.execute(new_group) + expect(group.root_ancestor).to eq(new_group) + end + end + + context 'when a subgroup becomes top level group' do + let(:subgroup) { create(:group, :private, parent: group) } + let(:transfer_service) { described_class.new(subgroup, user) } + + it 'enqueues CreateSeatAssignmentsWorker worker with the new namespace (the group being transferred)' do + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) + .to receive(:perform_async).with(subgroup.id) + + transfer_service.execute(nil) + + expect(subgroup.root_ancestor).to eq(subgroup) + end + end + + context 'when a sub-group becomes a sub-group in a different hierarchy' do + let(:subgroup) { create(:group, :private, parent: group) } + let(:transfer_service) { described_class.new(subgroup, user) } + + it 'enqueues CreateSeatAssignmentsWorker worker with the group being transferred in the params' do + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) + .to receive(:perform_async).with(subgroup.id) + + transfer_service.execute(new_group) + + expect(subgroup.root_ancestor).to eq(new_group) + end + end + + context 'when a sub-group becomes a sub-group in the same hierarchy' do + let(:subgroup_to_be_transferred) { create(:group, :private, parent: group) } + let(:subgroup_to_be_new_parent) { create(:group, :private, parent: group) } + let(:transfer_service) { described_class.new(subgroup_to_be_transferred, user) } + + it 'does not enqueue CreateSeatAssignmentsWorker cleanup worker' do + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) + .not_to receive(:perform_async).with(subgroup_to_be_transferred.id) + + transfer_service.execute(subgroup_to_be_new_parent) + + expect(subgroup_to_be_transferred.root_ancestor).to eq(group) + end + end + end end -- GitLab From 636675afa79c9342fa0bf0f0813ee00489f0578b Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 14:51:18 -0400 Subject: [PATCH 12/12] Add create seats worker to project transfer service --- .../services/ee/projects/transfer_service.rb | 8 +++ .../ee/groups/transfer_service_spec.rb | 8 +-- .../projects/transfer_service_spec.rb | 65 +++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/ee/app/services/ee/projects/transfer_service.rb b/ee/app/services/ee/projects/transfer_service.rb index ede11fcef30899..b71c8a7def7261 100644 --- a/ee/app/services/ee/projects/transfer_service.rb +++ b/ee/app/services/ee/projects/transfer_service.rb @@ -34,6 +34,7 @@ def post_update_hooks(project, old_group) update_compliance_standards_adherence delete_compliance_statuses sync_security_policies + reconcile_seat_assignments(old_namespace, project) end override :remove_paid_features @@ -82,6 +83,13 @@ def transfer_status_data project_namespace_ids: [project.project_namespace_id] ).execute end + + def reconcile_seat_assignments(old_namespace, project) + return unless ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) + return if old_namespace.root_ancestor.id == project.root_ancestor.id + + GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker.perform_async(project.id) + end end end end diff --git a/ee/spec/services/ee/groups/transfer_service_spec.rb b/ee/spec/services/ee/groups/transfer_service_spec.rb index 29ce463938a664..5a978bbab68f47 100644 --- a/ee/spec/services/ee/groups/transfer_service_spec.rb +++ b/ee/spec/services/ee/groups/transfer_service_spec.rb @@ -625,7 +625,7 @@ end context 'when a top level group becomes subgroup' do - it 'enqueues CreateSeatAssignmentsWorker worker with the group being transferred' do + it 'enqueues CreateGroupSeatsWorker worker with the group being transferred' do expect(group.root_ancestor).to eq(group) expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) .to receive(:perform_async).with(group.id) @@ -639,7 +639,7 @@ let(:subgroup) { create(:group, :private, parent: group) } let(:transfer_service) { described_class.new(subgroup, user) } - it 'enqueues CreateSeatAssignmentsWorker worker with the new namespace (the group being transferred)' do + it 'enqueues CreateGroupSeatsWorker worker with the new namespace (the group being transferred)' do expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) .to receive(:perform_async).with(subgroup.id) @@ -653,7 +653,7 @@ let(:subgroup) { create(:group, :private, parent: group) } let(:transfer_service) { described_class.new(subgroup, user) } - it 'enqueues CreateSeatAssignmentsWorker worker with the group being transferred in the params' do + it 'enqueues CreateGroupSeatsWorker worker with the group being transferred in the params' do expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) .to receive(:perform_async).with(subgroup.id) @@ -668,7 +668,7 @@ let(:subgroup_to_be_new_parent) { create(:group, :private, parent: group) } let(:transfer_service) { described_class.new(subgroup_to_be_transferred, user) } - it 'does not enqueue CreateSeatAssignmentsWorker cleanup worker' do + it 'does not enqueue CreateGroupSeatsWorker cleanup worker' do expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker) .not_to receive(:perform_async).with(subgroup_to_be_transferred.id) diff --git a/ee/spec/services/projects/transfer_service_spec.rb b/ee/spec/services/projects/transfer_service_spec.rb index 62705f333946f5..94b81add9bcbb3 100644 --- a/ee/spec/services/projects/transfer_service_spec.rb +++ b/ee/spec/services/projects/transfer_service_spec.rb @@ -274,5 +274,70 @@ def operation subject.execute(other_group) end end + + describe 'subscription seat assignments reconciliation' do + context 'seat assignment creation' do + let_it_be_with_refind(:project_2) { create(:project, namespace: group) } + + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + + context 'when we transfer from user_namespace to group_namespace' do + before do + project.update!(namespace: user.namespace) + end + + it 'enqueues CreateProjectSeatsWorker with the project id' do + expect(project.namespace).to eq(user.namespace) + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker) + .to receive(:perform_async).with(project.id) + + subject.execute(group) + expect(project.namespace).to eq(group) + end + end + + context 'when we transfer from group_namespace to user_namespace' do + it 'enqueues CreateProjectSeatsWorker with the project id' do + expect(project_2.namespace).to eq(group) + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker) + .to receive(:perform_async).with(project_2.id) + + described_class.new(project_2, user).execute(user.namespace) + expect(project_2.namespace).to eq(user.namespace) + end + end + + context 'when we transfer from a group_namespace to another group_namespace' do + let(:new_group) { create(:group, :public, owners: user) } + + it 'enqueues CreateProjectSeatsWorker with the project id' do + expect(project_2.namespace).to eq(group) + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker) + .to receive(:perform_async).with(project_2.id) + + described_class.new(project_2, user).execute(new_group) + + expect(project_2.namespace).to eq(new_group) + end + end + + context 'when a project is transferred to a sub-group in the same hierarchy' do + let(:subgroup_to_be_new_parent) { create(:group, :private, parent: group) } + + it 'does not enqueue CreateProjectSeatsWorker' do + expect(project_2.namespace).to eq(group) + expect(project_2.namespace).to eq(subgroup_to_be_new_parent.root_ancestor) + expect(GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker) + .not_to receive(:perform_async) + + described_class.new(project_2, user).execute(subgroup_to_be_new_parent) + + expect(project_2.namespace.root_ancestor).to eq(group) + end + end + end + end end end -- GitLab