From 6a29870034def4265b3f7b7cf3c9c74057644aaf Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 17:56:12 -0400 Subject: [PATCH 01/25] 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 3a10bb6f5c637437ed67ac1ea3727f1564aab9bc Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 17:57:43 -0400 Subject: [PATCH 02/25] 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 89844bf8bb8e4c532ca87bf85e0f4eefb9454772 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 18:02:16 -0400 Subject: [PATCH 03/25] 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 694b83dcdd42de..2ab937dcae2db5 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2443,6 +2443,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 31339c07667bd3aa0ed55ea84c1ada0cc91bab06 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 9 Jun 2025 18:03:35 -0400 Subject: [PATCH 04/25] 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 d950a275c85e67..d231f4170b27a9 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -489,6 +489,8 @@ - 1 - - gitlab_subscriptions_seat_assignments_group_links_create_or_update_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 c55d1ecbf44c009fadaaf49c4a0c72618a300f78 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 10:03:56 -0400 Subject: [PATCH 05/25] 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 7ad4d875c0504628bf420772221c4e8c88c8695a Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 14:06:34 +0000 Subject: [PATCH 06/25] 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 fd4093d2cf44305bdb553d7f786c7822102bb5f2 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 10 Jun 2025 10:34:33 -0400 Subject: [PATCH 07/25] 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 80ff97726e01099e9d26d15842e5a553dbf97c05 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 12 Jun 2025 22:59:12 -0400 Subject: [PATCH 08/25] 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 67552dcec0e2c33ac81c6b8a784d454063d727f4 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Fri, 13 Jun 2025 12:28:28 -0400 Subject: [PATCH 09/25] 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 f5c57942bcc89747aac8a7ff74b61883f9dd86de Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 16 Jun 2025 17:06:04 -0400 Subject: [PATCH 10/25] Add base create seat worker and tests --- .../base_create_seats_worker.rb | 38 +++++++++++++++++++ .../base_create_seats_worker_spec.rb | 13 +++++++ 2 files changed, 51 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb new file mode 100644 index 00000000000000..80947f11862604 --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class BaseCreateSeatsWorker # rubocop:disable Scalability/IdempotentWorker -- Idempotent declaration in child + BATCH_SIZE = 1000 + + private + + def create_missing_seat_assignments(source) + user_ids = collect_user_ids(source) + + seat_assignments = user_ids.map do |user_id| + { + namespace_id: source.root_ancestor.id, + user_id: user_id, + organization_id: source.organization_id + } + end + + seat_assignments.each_slice(self.class::BATCH_SIZE) do |batch| + ::GitlabSubscriptions::SeatAssignment.insert_all( + batch, + unique_by: [:namespace_id, :user_id] + ) + end + end + + # Abstract method. Must be defined in child + def collect_user_ids(_source) + raise NotImplementedError, "Subclasses must implement `collect_user_ids`" + end + end + end + end +end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb new file mode 100644 index 00000000000000..010b3e4d0096b7 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::BaseCreateSeatsWorker, :saas, feature_category: :seat_cost_management do + let(:worker) { described_class.new } + + describe '#collect_user_ids' do + it 'raises NotImplementedError' do + expect { worker.send(:collect_user_ids, double) }.to raise_error(NotImplementedError) + end + end +end -- GitLab From 1e16c99fe9f0e2a01dbcde6d7c9bae52105da963 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 16 Jun 2025 17:13:12 -0400 Subject: [PATCH 11/25] Add create group seat worker and tests --- .../create_group_seats_worker.rb | 34 ++++++ .../create_group_seats_worker_spec.rb | 112 ++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb new file mode 100644 index 00000000000000..018e7bba10fc95 --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class CreateGroupSeatsWorker < BaseCreateSeatsWorker + 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(group_id) + group = Group.find_by_id(group_id) + return unless group + + create_missing_seat_assignments(group) + end + + private + + def collect_user_ids(group) + group.members_with_descendants.pluck_user_ids + end + end + end + end +end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb new file mode 100644 index 00000000000000..aaaef90f754d81 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker, :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 group 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 'when there is a group' 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 + 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_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 + end + end +end -- GitLab From 0c384aaebd604a80e25a6d0eb69f1144dade1450 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 16 Jun 2025 17:15:57 -0400 Subject: [PATCH 12/25] Add create project seat worker and tests --- .../create_project_seats_worker.rb | 34 +++++ .../create_project_seats_worker_spec.rb | 124 ++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb new file mode 100644 index 00000000000000..92074a4b284c2f --- /dev/null +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb +module GitlabSubscriptions + module SeatAssignments + module MemberTransfers + class CreateProjectSeatsWorker < BaseCreateSeatsWorker + 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(project_id) + project = Project.find_by_id(project_id) + return unless project + + create_missing_seat_assignments(project) + end + + private + + def collect_user_ids(project) + project.project_members.pluck_user_ids + end + end + end + end +end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb new file mode 100644 index 00000000000000..6a985055e090ec --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker, :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 + it 'does nothing' do + expect(worker).not_to receive(:create_missing_seat_assignments) + + worker.perform(non_existing_record_id) + end + + context 'when the project root namespace is a user namespace' do + let_it_be_with_refind(:project) { create(:project, namespace: user.namespace) } + + before do + create(:project_member, project: project, user: user_2) + create(:project_member, project: project, user: user_3) + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user.namespace) + end + + it 'creates the missing seat assignments' do + expect do + worker.perform(project.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.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.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + ).to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) + end + + context 'when a user namespace does not have missing seat assignments' do + before do + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: user.namespace) + create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: user.namespace) + end + + it 'does not create duplicates if seat assignments are already reconciled' do + expect do + worker.perform(project.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count } + end + end + end + + context 'when the project namespace is a group' do + let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be_with_refind(:project) { create(:project, namespace: group) } + + before do + create(:project_member, project: project, user: user) + create(:project_member, project: project, user: user_2) + create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: group) + end + + it 'creates seat assignments for missing users' do + expect do + worker.perform(project.id) + end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count }.by(1) + 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(project.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to include(user.id, user_2.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(project.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match(::Member.in_hierarchy(group).pluck(:user_id)) + end + + context 'when a group namespace does not have missing seat assignments' do + before do + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: group) + end + + it 'does not create duplicates if seat assignments are already reconciled' do + expect do + worker.perform(project.id) + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count } + end + end + end + end +end -- GitLab From 7cc730a811bb0861f4cd117b901a2feeaeb3cdd2 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Mon, 16 Jun 2025 17:20:10 -0400 Subject: [PATCH 13/25] Remove general purpose worker and add queue yml files --- config/sidekiq_queues.yml | 4 +- ee/app/workers/all_queues.yml | 14 +- .../member_transfers/create_seats_worker.rb | 65 -------- .../create_seats_worker_spec.rb | 140 ------------------ 4 files changed, 15 insertions(+), 208 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 d231f4170b27a9..88c320f34a9754 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -489,7 +489,9 @@ - 1 - - gitlab_subscriptions_seat_assignments_group_links_create_or_update_seats - 1 -- - gitlab_subscriptions_seat_assignments_member_transfers_create_seats +- - gitlab_subscriptions_seat_assignments_member_transfers_create_group_seats + - 1 +- - gitlab_subscriptions_seat_assignments_member_transfers_create_project_seats - 1 - - gitlab_subscriptions_self_managed_duo_core_todo_notification - 1 diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 2ab937dcae2db5..87ecf9e69b03e5 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2443,8 +2443,18 @@ :idempotent: true :tags: [] :queue_namespace: -- :name: gitlab_subscriptions_seat_assignments_member_transfers_create_seats - :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateSeatsWorker +- :name: gitlab_subscriptions_seat_assignments_member_transfers_create_group_seats + :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker + :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_create_project_seats + :worker_name: GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateProjectSeatsWorker :feature_category: :seat_cost_management :has_external_dependencies: false :urgency: :low 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 7280d050d3005b2261ccf84cf5928dd1d74976d1 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 17 Jun 2025 10:58:58 -0400 Subject: [PATCH 14/25] Move constant to children --- .../member_transfers/base_create_seats_worker.rb | 8 +++----- .../member_transfers/create_group_seats_worker.rb | 3 ++- .../member_transfers/create_project_seats_worker.rb | 3 ++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb index 80947f11862604..b7ae1a99efdcf5 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb @@ -1,12 +1,10 @@ # frozen_string_literal: true -# app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +# rubocop:disable Scalability/IdempotentWorker -- Idempotent declaration in child module GitlabSubscriptions module SeatAssignments module MemberTransfers - class BaseCreateSeatsWorker # rubocop:disable Scalability/IdempotentWorker -- Idempotent declaration in child - BATCH_SIZE = 1000 - + class BaseCreateSeatsWorker private def create_missing_seat_assignments(source) @@ -28,7 +26,6 @@ def create_missing_seat_assignments(source) end end - # Abstract method. Must be defined in child def collect_user_ids(_source) raise NotImplementedError, "Subclasses must implement `collect_user_ids`" end @@ -36,3 +33,4 @@ def collect_user_ids(_source) end end end +# rubocop:enable Scalability/IdempotentWorker diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb index 018e7bba10fc95..4292a1d997f95f 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb module GitlabSubscriptions module SeatAssignments module MemberTransfers @@ -16,6 +15,8 @@ class CreateGroupSeatsWorker < BaseCreateSeatsWorker idempotent! + BATCH_SIZE = 1000 + def perform(group_id) group = Group.find_by_id(group_id) return unless group diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb index 92074a4b284c2f..e63b336fa06e67 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb module GitlabSubscriptions module SeatAssignments module MemberTransfers @@ -16,6 +15,8 @@ class CreateProjectSeatsWorker < BaseCreateSeatsWorker idempotent! + BATCH_SIZE = 1000 + def perform(project_id) project = Project.find_by_id(project_id) return unless project -- GitLab From b85fa15a744d350d2ee67313d1f777d78016faaf Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 18 Jun 2025 13:32:48 -0400 Subject: [PATCH 15/25] Implement MR review feedback --- .../base_create_seats_worker.rb | 15 ++++++- .../create_group_seats_worker.rb | 13 ++---- .../create_project_seats_worker.rb | 11 ++--- .../base_create_seats_worker_spec.rb | 13 ------ .../create_group_seats_worker_spec.rb | 41 +++++++++++++++++++ 5 files changed, 62 insertions(+), 31 deletions(-) delete mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb index b7ae1a99efdcf5..700ff95c4d4486 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb @@ -5,6 +5,15 @@ module GitlabSubscriptions module SeatAssignments module MemberTransfers class BaseCreateSeatsWorker + BATCH_SIZE = 1000 + + def perform(source_id) + source = find_source_by_id(source_id) + return unless source + + create_missing_seat_assignments(source) + end + private def create_missing_seat_assignments(source) @@ -27,7 +36,11 @@ def create_missing_seat_assignments(source) end def collect_user_ids(_source) - raise NotImplementedError, "Subclasses must implement `collect_user_ids`" + raise NotImplementedError + end + + def find_source_by_id(_source_id) + raise NotImplementedError end end end diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb index 4292a1d997f95f..47e4ad6c6ecda0 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb @@ -15,19 +15,14 @@ class CreateGroupSeatsWorker < BaseCreateSeatsWorker idempotent! - BATCH_SIZE = 1000 - - def perform(group_id) - group = Group.find_by_id(group_id) - return unless group + private - create_missing_seat_assignments(group) + def find_source_by_id(group_id) + Group.find_by_id(group_id) end - private - def collect_user_ids(group) - group.members_with_descendants.pluck_user_ids + Member.for_self_and_descendants(group).pluck_user_ids end end end diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb index e63b336fa06e67..769f2f6e675298 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb @@ -15,17 +15,12 @@ class CreateProjectSeatsWorker < BaseCreateSeatsWorker idempotent! - BATCH_SIZE = 1000 - - def perform(project_id) - project = Project.find_by_id(project_id) - return unless project + private - create_missing_seat_assignments(project) + def find_source_by_id(project_id) + Project.find_by_id(project_id) end - private - def collect_user_ids(project) project.project_members.pluck_user_ids end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb deleted file mode 100644 index 010b3e4d0096b7..00000000000000 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::BaseCreateSeatsWorker, :saas, feature_category: :seat_cost_management do - let(:worker) { described_class.new } - - describe '#collect_user_ids' do - it 'raises NotImplementedError' do - expect { worker.send(:collect_user_ids, double) }.to raise_error(NotImplementedError) - end - end -end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb index aaaef90f754d81..7f1995dd5dc8a3 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb @@ -77,6 +77,34 @@ end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) } end end + + context 'when a group has a project with members' do + let_it_be(:project) { create(:project, namespace: group) } + let_it_be(:user_4) { create(:user) } + let_it_be(:user_5) { create(:user) } + + 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(:project_member, project: project, user: user_4) + create(:project_member, project: project, user: user_5) + create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: group) + create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: group) + end + + it 'creates seat assignments for the project members' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).not_to include(user_4.id, user_5.id) + + worker.perform(group.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to include(user_4.id, user_5.id) + end + end end end @@ -107,6 +135,19 @@ 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 + + context 'when a group has a child project' do + let_it_be(:user_5) { create(:user) } + let_it_be(:project) { create(:project, namespace: child_group_c) } + + before do + create(:project_member, project: project, user: user_5) + end + + it 'takes the ids of the group and its project' do + expect(worker.send(:collect_user_ids, child_group_c)).to contain_exactly(user_4.id, user_5.id) + end + end end end end -- GitLab From 8bd7cea59034fe8772454eb90dcdf48cb51350f8 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 2 Jul 2025 21:43:30 -0400 Subject: [PATCH 16/25] Implement suggested changes to base worker --- .../member_transfers/base_create_seats_worker.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb index 700ff95c4d4486..1b9ef51a3017aa 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb @@ -5,7 +5,7 @@ module GitlabSubscriptions module SeatAssignments module MemberTransfers class BaseCreateSeatsWorker - BATCH_SIZE = 1000 + BATCH_SIZE = 100 def perform(source_id) source = find_source_by_id(source_id) @@ -18,16 +18,18 @@ def perform(source_id) def create_missing_seat_assignments(source) user_ids = collect_user_ids(source) + namespace_id = ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) ? source.root_ancestor.id : nil + organization_id = source.root_ancestor.organization_id seat_assignments = user_ids.map do |user_id| { - namespace_id: source.root_ancestor.id, + namespace_id: namespace_id, user_id: user_id, - organization_id: source.organization_id + organization_id: organization_id } end - seat_assignments.each_slice(self.class::BATCH_SIZE) do |batch| + seat_assignments.each_slice(BATCH_SIZE) do |batch| ::GitlabSubscriptions::SeatAssignment.insert_all( batch, unique_by: [:namespace_id, :user_id] -- GitLab From 7d7552913fc198e4e26c758e88fbf3543e9b9c70 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 2 Jul 2025 21:45:50 -0400 Subject: [PATCH 17/25] Improve tests for the create group seats worker --- .../create_group_seats_worker_spec.rb | 173 +++++++++++++----- 1 file changed, 132 insertions(+), 41 deletions(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb index 7f1995dd5dc8a3..6e1c7301c976cb 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb @@ -4,11 +4,15 @@ RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::CreateGroupSeatsWorker, :saas, feature_category: :seat_cost_management do let(:worker) { described_class.new } - let_it_be(:user) { create(:user, :with_namespace) } + let_it_be(:user) { create(:user) } let_it_be(:user_2) { create(:user) } let_it_be(:user_3) { create(:user) } describe '#perform' do + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + context 'when the group is nil' do it 'does nothing' do expect(worker).not_to receive(:create_missing_seat_assignments) @@ -17,7 +21,7 @@ end end - context 'when there is a group' do + context 'with a root group' do let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } before do @@ -31,12 +35,6 @@ 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) @@ -52,13 +50,13 @@ 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)) + ).not_to match([user.id, user_2.id, user_3.id]) worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match(::Member.in_hierarchy(group).pluck(:user_id)) + ).to match([user.id, user_2.id, user_3.id]) end end @@ -78,7 +76,7 @@ end end - context 'when a group has a project with members' do + context 'when a root group has a project with members' do let_it_be(:project) { create(:project, namespace: group) } let_it_be(:user_4) { create(:user) } let_it_be(:user_5) { create(:user) } @@ -105,47 +103,140 @@ ).to include(user_4.id, user_5.id) end 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_4) { create(:user) } + context 'when a group has a parent root group' do + let_it_be_with_refind(:child_group) { create(:group, parent: group) } + let_it_be(:user_4) { create(:user) } + let_it_be(:user_5) { 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 + before do + create(:group_member, group: group, user: user_4) + create(:group_member, group: group, user: user_5) + create(:group_member, group: child_group, user: user_2) + create(:group_member, group: child_group, user: user_3) + 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 'creates seat assignments in the root group for the members of the group being passed' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id]) - 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 + worker.perform(child_group.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id, user_2.id, user_3.id]) + end + + it 'does not create seats for members of the root group if a child group is passed' do + worker.perform(child_group.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).not_to include(user_4.id, user_5.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) + it 'does not assign seats to the child group' do + worker.perform(child_group.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(child_group) + ).to be_empty + end end - context 'when a group has a child project' do - let_it_be(:user_5) { create(:user) } - let_it_be(:project) { create(:project, namespace: child_group_c) } + context 'with nested hierarchy' do + let_it_be(:child_group_a) { create(:group, parent: group) } + let_it_be(:child_group_b) { create(:group, parent: group) } + let_it_be(:child_group_c) { create(:group, parent: child_group_a) } + let_it_be(:user_4) { create(:user) } before do - create(:project_member, project: project, user: user_5) + create(:group_member, group: 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 + + context 'when a leaf group is passed' do + it 'creates seat assignments in the root group only for the members of the leaf group' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id]) + + worker.perform(child_group_c.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id, user_4.id]) + end + end + + context 'when a subhierarchy is passed' do + it 'creates seat assignments in the root group only for the members of subhierarchy' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id]) + + worker.perform(child_group_a.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id, user_2.id, user_4.id]) + end + end + + context 'when a sibling group is passed' do + it 'creates seat assignments in the root group only for the members of sibling' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id]) + + worker.perform(child_group_b.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id, user_3.id]) + end + end + + context 'when the root group is passed' do + let_it_be(:user_5) { create(:user) } + let_it_be(:user_6) { create(:user) } + + before do + create(:group_member, group: group, user: user_5) + create(:group_member, group: group, user: user_6) + end + + it 'creates seat assignments in the root group for all members of the hierarchy' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id]) + + worker.perform(group.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user.id, user_2.id, user_3.id, user_4.id, user_5.id, user_6.id]) + end + end + end + + context 'with an SM instance' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + 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 'takes the ids of the group and its project' do - expect(worker.send(:collect_user_ids, child_group_c)).to contain_exactly(user_4.id, user_5.id) + it 'creates seats with nil namespace_id' do + worker.perform(group.id) + + expect( + GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3]).pluck(:namespace_id) + ).to match([nil, nil]) end end end -- GitLab From a40b5627f399f67c9acee39734bc9a32355ce356 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Wed, 2 Jul 2025 21:46:34 -0400 Subject: [PATCH 18/25] Improve tests for the create project seats worker --- .../create_project_seats_worker_spec.rb | 114 ++++++++++-------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb index 6a985055e090ec..f085d5866bae3b 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -9,14 +9,15 @@ let_it_be(:user_3) { create(:user) } describe '#perform' do - it 'does nothing' do + it 'does nothing when there is no project' do expect(worker).not_to receive(:create_missing_seat_assignments) worker.perform(non_existing_record_id) end context 'when the project root namespace is a user namespace' do - let_it_be_with_refind(:project) { create(:project, namespace: user.namespace) } + let_it_be(:project) { create(:project, namespace: user.namespace) } + let_it_be(:user_4) { create(:user, namespace: user.namespace) } before do create(:project_member, project: project, user: user_2) @@ -24,13 +25,7 @@ create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user.namespace) end - it 'creates the missing seat assignments' do - expect do - worker.perform(project.id) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count }.by(2) - end - - it 'creates seat assignments for the missing users' do + it 'creates seat assignments in the user namespace only for the only members of the project' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) ).not_to include(user_2.id, user_3.id) @@ -39,19 +34,7 @@ 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.id) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).to match(::Member.in_hierarchy(user.namespace).pluck(:user_id)) + ).to match([user.id, user_2.id, user_3.id]) end context 'when a user namespace does not have missing seat assignments' do @@ -66,25 +49,33 @@ end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count } end end + + context 'with an SM instance' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + + it 'creates seats with nil namespace' do + worker.perform(project.id) + + expect( + GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3.id]).pluck(:namespace_id) + ).to match([nil, nil]) + end + end end - context 'when the project namespace is a group' do + context 'when the project root namespace is a group' do let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } - let_it_be_with_refind(:project) { create(:project, namespace: group) } + let_it_be(:project) { create(:project, namespace: group) } before do - create(:project_member, project: project, user: user) + create(:group_member, group: group, user: user) create(:project_member, project: project, user: user_2) - create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: group) - end - - it 'creates seat assignments for missing users' do - expect do - worker.perform(project.id) - end.to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count }.by(1) + create(:project_member, project: project, user: user_3) end - it 'creates seat assignments for the missing users' do + it 'creates seat assignments in the root group only for the members of the project' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) ).not_to include(user_2.id, user_3.id) @@ -93,32 +84,59 @@ expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to include(user.id, user_2.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(project.id) - - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match(::Member.in_hierarchy(group).pluck(:user_id)) + ).to match([user_2.id, user_3.id]) end - context 'when a group namespace does not have missing seat assignments' do + context 'when the root group namespace already has seats for the project members' do before do 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 duplicates if seat assignments are already reconciled' do + it 'does not create duplicate seats' do expect do worker.perform(project.id) end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count } end end + + context 'with an SM instance' do + before do + stub_saas_features(gitlab_com_subscriptions: false) + end + + it 'creates seats with nil namespace' do + worker.perform(project.id) + + expect( + GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3.id]).pluck(:namespace_id) + ).to match([nil, nil]) + end + end + end + + context 'when the project is part of a group hierarchy' do + let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be(:child_group) { create(:group, parent: group) } + let_it_be(:project) { create(:project, namespace: child_group) } + + before do + create(:group_member, group: group, user: user) + create(:group_member, group: child_group, user: user_2) + create(:project_member, project: project, user: user_3) + end + + it 'creates seat assignments in the root group only for the members of the project' do + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).not_to include(user_3.id) + + worker.perform(project.id) + + expect( + GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) + ).to match([user_3.id]) + end end end end -- GitLab From 719cca4eb52899a17d4eb00b2b23f85f37cdf177 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 3 Jul 2025 11:19:43 -0400 Subject: [PATCH 19/25] Address danger suggestions --- .../create_group_seats_worker_spec.rb | 26 +++++++++---------- .../create_project_seats_worker_spec.rb | 10 +++---- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb index 6e1c7301c976cb..45e0ffe90dfe6d 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb @@ -50,13 +50,13 @@ it 'creates seat assignments for all members' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to match([user.id, user_2.id, user_3.id]) + ).not_to match_array([user.id, user_2.id, user_3.id]) worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id, user_2.id, user_3.id]) + ).to match_array([user.id, user_2.id, user_3.id]) end end @@ -119,13 +119,13 @@ it 'creates seat assignments in the root group for the members of the group being passed' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id]) + ).to match_array([user.id]) worker.perform(child_group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id, user_2.id, user_3.id]) + ).to match_array([user.id, user_2.id, user_3.id]) end it 'does not create seats for members of the root group if a child group is passed' do @@ -162,13 +162,13 @@ it 'creates seat assignments in the root group only for the members of the leaf group' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id]) + ).to match_array([user.id]) worker.perform(child_group_c.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id, user_4.id]) + ).to match_array([user.id, user_4.id]) end end @@ -176,13 +176,13 @@ it 'creates seat assignments in the root group only for the members of subhierarchy' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id]) + ).to match_array([user.id]) worker.perform(child_group_a.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id, user_2.id, user_4.id]) + ).to match_array([user.id, user_2.id, user_4.id]) end end @@ -190,13 +190,13 @@ it 'creates seat assignments in the root group only for the members of sibling' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id]) + ).to match_array([user.id]) worker.perform(child_group_b.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id, user_3.id]) + ).to match_array([user.id, user_3.id]) end end @@ -212,13 +212,13 @@ it 'creates seat assignments in the root group for all members of the hierarchy' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id]) + ).to match_array([user.id]) worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user.id, user_2.id, user_3.id, user_4.id, user_5.id, user_6.id]) + ).to match_array([user.id, user_2.id, user_3.id, user_4.id, user_5.id, user_6.id]) end end end @@ -236,7 +236,7 @@ expect( GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3]).pluck(:namespace_id) - ).to match([nil, nil]) + ).to match_array([nil, nil]) end end end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb index f085d5866bae3b..9e34de29a5720a 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -34,7 +34,7 @@ expect( GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).to match([user.id, user_2.id, user_3.id]) + ).to match_array([user.id, user_2.id, user_3.id]) end context 'when a user namespace does not have missing seat assignments' do @@ -60,7 +60,7 @@ expect( GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3.id]).pluck(:namespace_id) - ).to match([nil, nil]) + ).to match_array([nil, nil]) end end end @@ -84,7 +84,7 @@ expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user_2.id, user_3.id]) + ).to match_array([user_2.id, user_3.id]) end context 'when the root group namespace already has seats for the project members' do @@ -110,7 +110,7 @@ expect( GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3.id]).pluck(:namespace_id) - ).to match([nil, nil]) + ).to match_array([nil, nil]) end end end @@ -135,7 +135,7 @@ expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match([user_3.id]) + ).to match_array([user_3.id]) end end end -- GitLab From a2941360736a143b0dc384faa0c27b9100384284 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 10 Jul 2025 15:14:17 -0400 Subject: [PATCH 20/25] Implement MR suggestions --- .../base_create_seats_worker.rb | 2 +- .../create_group_seats_worker_spec.rb | 99 +++---------------- .../create_project_seats_worker_spec.rb | 36 +------ 3 files changed, 20 insertions(+), 117 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb index 1b9ef51a3017aa..9609eac83ac2d4 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb @@ -18,7 +18,7 @@ def perform(source_id) def create_missing_seat_assignments(source) user_ids = collect_user_ids(source) - namespace_id = ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) ? source.root_ancestor.id : nil + namespace_id = source.root_ancestor.id organization_id = source.root_ancestor.organization_id seat_assignments = user_ids.map do |user_id| diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb index 45e0ffe90dfe6d..9be933ecadba57 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb @@ -9,10 +9,6 @@ let_it_be(:user_3) { create(:user) } describe '#perform' do - before do - stub_saas_features(gitlab_com_subscriptions: true) - end - context 'when the group is nil' do it 'does nothing' do expect(worker).not_to receive(:create_missing_seat_assignments) @@ -22,10 +18,10 @@ end context 'with a root group' do - let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be_with_refind(:group) { create(:group) } before do - create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: group) + create(:gitlab_subscription_seat_assignment, user: user, namespace: group) end context 'when group namespace has missing seats' do @@ -36,10 +32,6 @@ 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( @@ -48,10 +40,6 @@ end it 'creates seat assignments for all members' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to match_array([user.id, user_2.id, user_3.id]) - worker.perform(group.id) expect( @@ -78,50 +66,34 @@ context 'when a root group has a project with members' do let_it_be(:project) { create(:project, namespace: group) } - let_it_be(:user_4) { create(:user) } - let_it_be(:user_5) { create(:user) } 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(:project_member, project: project, user: user_4) - create(:project_member, project: project, user: user_5) - create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: group) - create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: group) + create(:project_member, project: project, user: user_2) + create(:project_member, project: project, user: user_3) end it 'creates seat assignments for the project members' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to include(user_4.id, user_5.id) - worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to include(user_4.id, user_5.id) + ).to include(user_2.id, user_3.id) end end context 'when a group has a parent root group' do - let_it_be_with_refind(:child_group) { create(:group, parent: group) } + let_it_be_with_refind(:transferred_group) { create(:group, parent: group) } let_it_be(:user_4) { create(:user) } - let_it_be(:user_5) { create(:user) } before do create(:group_member, group: group, user: user_4) - create(:group_member, group: group, user: user_5) - create(:group_member, group: child_group, user: user_2) - create(:group_member, group: child_group, user: user_3) + create(:group_member, group: transferred_group, user: user_2) + create(:group_member, group: transferred_group, user: user_3) end it 'creates seat assignments in the root group for the members of the group being passed' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match_array([user.id]) - - worker.perform(child_group.id) + worker.perform(transferred_group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) @@ -129,18 +101,18 @@ end it 'does not create seats for members of the root group if a child group is passed' do - worker.perform(child_group.id) + worker.perform(transferred_group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to include(user_4.id, user_5.id) + ).not_to include(user_4.id) end it 'does not assign seats to the child group' do - worker.perform(child_group.id) + worker.perform(transferred_group.id) expect( - GitlabSubscriptions::SeatAssignment.by_namespace(child_group) + GitlabSubscriptions::SeatAssignment.by_namespace(transferred_group) ).to be_empty end end @@ -160,10 +132,6 @@ context 'when a leaf group is passed' do it 'creates seat assignments in the root group only for the members of the leaf group' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match_array([user.id]) - worker.perform(child_group_c.id) expect( @@ -173,11 +141,7 @@ end context 'when a subhierarchy is passed' do - it 'creates seat assignments in the root group only for the members of subhierarchy' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match_array([user.id]) - + it 'creates seat assignments in the root group only for members of the subhierarchy' do worker.perform(child_group_a.id) expect( @@ -188,10 +152,6 @@ context 'when a sibling group is passed' do it 'creates seat assignments in the root group only for the members of sibling' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match_array([user.id]) - worker.perform(child_group_b.id) expect( @@ -201,44 +161,15 @@ end context 'when the root group is passed' do - let_it_be(:user_5) { create(:user) } - let_it_be(:user_6) { create(:user) } - - before do - create(:group_member, group: group, user: user_5) - create(:group_member, group: group, user: user_6) - end - it 'creates seat assignments in the root group for all members of the hierarchy' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match_array([user.id]) - worker.perform(group.id) expect( GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).to match_array([user.id, user_2.id, user_3.id, user_4.id, user_5.id, user_6.id]) + ).to match_array([user.id, user_2.id, user_3.id, user_4.id]) end end end - - context 'with an SM instance' do - before do - stub_saas_features(gitlab_com_subscriptions: false) - 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 seats with nil namespace_id' do - worker.perform(group.id) - - expect( - GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3]).pluck(:namespace_id) - ).to match_array([nil, nil]) - end - end end end end diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb index 9e34de29a5720a..5b11cffe83983a 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -17,7 +17,7 @@ context 'when the project root namespace is a user namespace' do let_it_be(:project) { create(:project, namespace: user.namespace) } - let_it_be(:user_4) { create(:user, namespace: user.namespace) } + let_it_be(:user_4) { create(:user, namespace: project.namespace) } before do create(:project_member, project: project, user: user_2) @@ -25,7 +25,7 @@ create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user.namespace) end - it 'creates seat assignments in the user namespace only for the only members of the project' do + it 'creates seat assignments in the user namespace only for members of the project' do expect( GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) ).not_to include(user_2.id, user_3.id) @@ -49,24 +49,10 @@ end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count } end end - - context 'with an SM instance' do - before do - stub_saas_features(gitlab_com_subscriptions: false) - end - - it 'creates seats with nil namespace' do - worker.perform(project.id) - - expect( - GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3.id]).pluck(:namespace_id) - ).to match_array([nil, nil]) - end - end end context 'when the project root namespace is a group' do - let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be_with_refind(:group) { create(:group) } let_it_be(:project) { create(:project, namespace: group) } before do @@ -99,24 +85,10 @@ end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(group).count } end end - - context 'with an SM instance' do - before do - stub_saas_features(gitlab_com_subscriptions: false) - end - - it 'creates seats with nil namespace' do - worker.perform(project.id) - - expect( - GitlabSubscriptions::SeatAssignment.where(user_id: [user_2.id, user_3.id]).pluck(:namespace_id) - ).to match_array([nil, nil]) - end - end end context 'when the project is part of a group hierarchy' do - let_it_be_with_refind(:group) { create(:group_with_plan, plan: :ultimate_plan) } + let_it_be_with_refind(:group) { create(:group) } let_it_be(:child_group) { create(:group, parent: group) } let_it_be(:project) { create(:project, namespace: child_group) } -- GitLab From 22d0371468fe7bbfea92cab0de75b6017d4b7005 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 15 Jul 2025 17:46:17 -0400 Subject: [PATCH 21/25] Add missing review feedback --- .../create_project_seats_worker_spec.rb | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb index 5b11cffe83983a..3dfa99e7e857be 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -17,36 +17,31 @@ context 'when the project root namespace is a user namespace' do let_it_be(:project) { create(:project, namespace: user.namespace) } - let_it_be(:user_4) { create(:user, namespace: project.namespace) } before do create(:project_member, project: project, user: user_2) create(:project_member, project: project, user: user_3) - create(:gitlab_subscription_seat_assignment, :active, user: user, namespace: user.namespace) + create(:gitlab_subscription_seat_assignment, user: user, namespace: project.namespace) end it 'creates seat assignments in the user namespace only for members of the project' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) - ).not_to include(user_2.id, user_3.id) - worker.perform(project.id) expect( - GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).pluck(:user_id) + GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).pluck(:user_id) ).to match_array([user.id, user_2.id, user_3.id]) end context 'when a user namespace does not have missing seat assignments' do before do - create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: user.namespace) - create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: user.namespace) + create(:gitlab_subscription_seat_assignment, user: user_2, namespace: project.namespace) + create(:gitlab_subscription_seat_assignment, user: user_3, namespace: project.namespace) end it 'does not create duplicates if seat assignments are already reconciled' do expect do worker.perform(project.id) - end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(user.namespace).count } + end.not_to change { GitlabSubscriptions::SeatAssignment.by_namespace(project.namespace).count } end end end @@ -62,10 +57,6 @@ end it 'creates seat assignments in the root group only for the members of the project' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to include(user_2.id, user_3.id) - worker.perform(project.id) expect( @@ -75,8 +66,8 @@ context 'when the root group namespace already has seats for the project members' do before do - create(:gitlab_subscription_seat_assignment, :active, user: user_2, namespace: group) - create(:gitlab_subscription_seat_assignment, :active, user: user_3, namespace: group) + create(:gitlab_subscription_seat_assignment, user: user_2, namespace: group) + create(:gitlab_subscription_seat_assignment, user: user_3, namespace: group) end it 'does not create duplicate seats' do @@ -99,10 +90,6 @@ end it 'creates seat assignments in the root group only for the members of the project' do - expect( - GitlabSubscriptions::SeatAssignment.by_namespace(group).pluck(:user_id) - ).not_to include(user_3.id) - worker.perform(project.id) expect( -- GitLab From b72c7dced617b21fd92735ec09d98e2aa645833e Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 17 Jul 2025 20:11:08 -0400 Subject: [PATCH 22/25] Add database review suggestion --- .../base_create_seats_worker.rb | 19 +++++++++---------- .../create_group_seats_worker.rb | 2 +- .../create_project_seats_worker.rb | 2 +- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb index 9609eac83ac2d4..4ccca787a69505 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker.rb @@ -17,21 +17,20 @@ def perform(source_id) private def create_missing_seat_assignments(source) - user_ids = collect_user_ids(source) namespace_id = source.root_ancestor.id organization_id = source.root_ancestor.organization_id - seat_assignments = user_ids.map do |user_id| - { - namespace_id: namespace_id, - user_id: user_id, - organization_id: organization_id - } - end + collect_user_ids(source).each_batch(of: BATCH_SIZE) do |users| + seat_assignments = users.pluck_user_ids.map do |user_id| + { + namespace_id: namespace_id, + user_id: user_id, + organization_id: organization_id + } + end - seat_assignments.each_slice(BATCH_SIZE) do |batch| ::GitlabSubscriptions::SeatAssignment.insert_all( - batch, + seat_assignments, unique_by: [:namespace_id, :user_id] ) end diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb index 47e4ad6c6ecda0..dee6c1ac8d6b49 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker.rb @@ -22,7 +22,7 @@ def find_source_by_id(group_id) end def collect_user_ids(group) - Member.for_self_and_descendants(group).pluck_user_ids + Member.for_self_and_descendants(group) end end end diff --git a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb index 769f2f6e675298..81ca2da14fabfd 100644 --- a/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb +++ b/ee/app/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker.rb @@ -22,7 +22,7 @@ def find_source_by_id(project_id) end def collect_user_ids(project) - project.project_members.pluck_user_ids + project.project_members end end end -- GitLab From 259082343a998d37d5cf96a5c994eda07affeddd Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 17 Jul 2025 20:13:27 -0400 Subject: [PATCH 23/25] Improve context description --- .../member_transfers/create_project_seats_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb index 3dfa99e7e857be..9d4c46a4e24c80 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -78,7 +78,7 @@ end end - context 'when the project is part of a group hierarchy' do + context 'when the project belongs to a subgroup' do let_it_be_with_refind(:group) { create(:group) } let_it_be(:child_group) { create(:group, parent: group) } let_it_be(:project) { create(:project, namespace: child_group) } -- GitLab From cb27d983485be256a067c1633a7afe1efb7042c1 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Tue, 29 Jul 2025 14:32:23 -0400 Subject: [PATCH 24/25] Add test for missing coverage --- .../base_create_seats_worker_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb new file mode 100644 index 00000000000000..e3b53f266372c9 --- /dev/null +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/base_create_seats_worker_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSubscriptions::SeatAssignments::MemberTransfers::BaseCreateSeatsWorker, :saas, feature_category: :seat_cost_management do + let(:worker) { described_class.new } + + describe '#collect_user_ids' do + it 'raises NotImplementedError' do + expect { worker.send(:collect_user_ids, double) }.to raise_error(NotImplementedError) + end + end + + describe '#find_source_by_id' do + it 'raises NotImplementedError' do + expect { worker.send(:find_source_by_id, double) }.to raise_error(NotImplementedError) + end + end +end -- GitLab From b697b8cc65eee2e1e9071294fd114bd6bfe1a4b9 Mon Sep 17 00:00:00 2001 From: Jorge Cook Date: Thu, 31 Jul 2025 18:09:04 -0400 Subject: [PATCH 25/25] Add tests for idempotency and data consistency --- .../create_group_seats_worker_spec.rb | 12 ++++++++++++ .../create_project_seats_worker_spec.rb | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb index 9be933ecadba57..cb49a57ef2e2a8 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_group_seats_worker_spec.rb @@ -8,6 +8,18 @@ let_it_be(:user_2) { create(:user) } let_it_be(:user_3) { create(:user) } + it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed + + it_behaves_like 'an idempotent worker' do + let_it_be(:group) { create(:group) } + + before do + create(:group_member, group: group, user: user) + end + + let(:job_args) { [group.id] } + end + describe '#perform' do context 'when the group is nil' do it 'does nothing' do diff --git a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb index 9d4c46a4e24c80..8255beac17cfa9 100644 --- a/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb +++ b/ee/spec/workers/gitlab_subscriptions/seat_assignments/member_transfers/create_project_seats_worker_spec.rb @@ -8,6 +8,18 @@ let_it_be(:user_2) { create(:user) } let_it_be(:user_3) { create(:user) } + it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed + + it_behaves_like 'an idempotent worker' do + let_it_be(:project) { create(:project) } + + before do + create(:project_member, project: project, user: user) + end + + let(:job_args) { [project.id] } + end + describe '#perform' do it 'does nothing when there is no project' do expect(worker).not_to receive(:create_missing_seat_assignments) -- GitLab