From 752d61e6b6a79bcf1eef1e1455105e9fb2060672 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Wed, 26 Feb 2025 15:47:20 +1100 Subject: [PATCH 01/11] Add internal event tracking to placeholder reassignment This adds internal event tracking to several events in placeholder reassignment. Changelog: other --- ...assign_placeholder_user_records_service.rb | 15 +++++++++++- .../accept_reassignment_service.rb | 3 +++ .../import/source_users/base_service.rb | 15 ++++++++++++ .../cancel_reassignment_service.rb | 2 ++ .../keep_all_as_placeholder_service.rb | 2 ++ .../keep_as_placeholder_service.rb | 2 ++ .../import/source_users/reassign_service.rb | 2 ++ .../reject_reassignment_service.rb | 1 + config/events/keep_all_as_placeholder.yml | 16 +++++++++++++ config/events/keep_as_placeholder.yml | 21 +++++++++++++++++ .../placeholder_reassignment_accepted.yml | 23 +++++++++++++++++++ .../placeholder_reassignment_canceled.yml | 23 +++++++++++++++++++ .../placeholder_reassignment_completed.yml | 22 ++++++++++++++++++ .../placeholder_reassignment_failed.yml | 23 +++++++++++++++++++ .../placeholder_reassignment_proposed.yml | 23 +++++++++++++++++++ .../placeholder_reassignment_rejected.yml | 23 +++++++++++++++++++ config/events/placeholder_user_created.yml | 20 ++++++++++++++++ lib/gitlab/import/placeholder_user_creator.rb | 10 ++++++++ .../import/placeholder_user_creator_spec.rb | 14 ++++++++++- ...n_placeholder_user_records_service_spec.rb | 14 +++++++++++ .../accept_reassignment_service_spec.rb | 15 ++++++++++++ .../cancel_reassignment_service_spec.rb | 8 +++++++ .../keep_all_as_placeholder_service_spec.rb | 3 +++ .../keep_as_placeholder_service_spec.rb | 8 +++++++ .../source_users/reassign_service_spec.rb | 15 ++++++++++++ .../reject_reassignment_service_spec.rb | 4 ++++ 26 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 config/events/keep_all_as_placeholder.yml create mode 100644 config/events/keep_as_placeholder.yml create mode 100644 config/events/placeholder_reassignment_accepted.yml create mode 100644 config/events/placeholder_reassignment_canceled.yml create mode 100644 config/events/placeholder_reassignment_completed.yml create mode 100644 config/events/placeholder_reassignment_failed.yml create mode 100644 config/events/placeholder_reassignment_proposed.yml create mode 100644 config/events/placeholder_reassignment_rejected.yml create mode 100644 config/events/placeholder_user_created.yml diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb index 15ff07678ca8da..b09732f436e0ca 100644 --- a/app/services/import/reassign_placeholder_user_records_service.rb +++ b/app/services/import/reassign_placeholder_user_records_service.rb @@ -2,6 +2,8 @@ module Import class ReassignPlaceholderUserRecordsService + include Gitlab::InternalEventsTracking + MEMBER_SELECT_BATCH_SIZE = 100 MEMBER_DELETE_BATCH_SIZE = 1_000 GROUP_FINDER_MEMBER_RELATIONS = %i[direct inherited shared_from_groups].freeze @@ -19,6 +21,7 @@ class ReassignPlaceholderUserRecordsService def initialize(import_source_user) @import_source_user = import_source_user @reassigned_by_user = import_source_user.reassigned_by_user + @placeholder_user_id = import_source_user.placeholder_user_id @unavailable_tables = [] @project_membership_created = false end @@ -48,6 +51,16 @@ def execute import_source_user.complete! + track_internal_event( + 'placeholder_reassignment_completed', + namespace: import_source_user.namespace, + additional_properties: { + label: 'placeholder_user_id', + value: placeholder_user_id, + reassign_to_user_id: import_source_user.reassign_to_user_id + } + ) + ServiceResponse.success( message: s_('Import|Placeholder user record reassignment complete'), payload: import_source_user @@ -56,7 +69,7 @@ def execute private - attr_accessor :import_source_user, :reassigned_by_user, :unavailable_tables + attr_accessor :import_source_user, :reassigned_by_user, :unavailable_tables, :placeholder_user_id def warn_about_any_risky_reassignments warn_about_reassign_to_admin if import_source_user.reassign_to_user.admin? # rubocop:disable Cop/UserAdmin -- Not authentication related diff --git a/app/services/import/source_users/accept_reassignment_service.rb b/app/services/import/source_users/accept_reassignment_service.rb index b6c8810f9a101a..a7bf6b5009d1ed 100644 --- a/app/services/import/source_users/accept_reassignment_service.rb +++ b/app/services/import/source_users/accept_reassignment_service.rb @@ -23,8 +23,11 @@ def execute if accept_successful Import::ReassignPlaceholderUserRecordsWorker.perform_async(import_source_user.id) + track_reassignment_event('placeholder_reassignment_accepted', current_user, import_source_user) + ServiceResponse.success(payload: import_source_user) else + track_reassignment_event('placeholder_reassignment_failed', current_user, import_source_user) ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/app/services/import/source_users/base_service.rb b/app/services/import/source_users/base_service.rb index f2f880da32f1c7..2b1f1e4d8caeac 100644 --- a/app/services/import/source_users/base_service.rb +++ b/app/services/import/source_users/base_service.rb @@ -3,6 +3,8 @@ module Import module SourceUsers class BaseService + include Gitlab::InternalEventsTracking + private attr_reader :import_source_user, :current_user @@ -25,6 +27,19 @@ def error_invalid_status def send_user_reassign_email Notify.import_source_user_reassign(import_source_user.id).deliver_later end + + def track_reassignment_event(event_name, current_user, import_source_user) + track_internal_event( + event_name, + user: current_user, + namespace: import_source_user.namespace, + additional_properties: { + label: 'placeholder_user_id', + value: import_source_user.placeholder_user_id, + reassign_to_user_id: import_source_user.reassign_to_user_id || nil + } + ) + end end end end diff --git a/app/services/import/source_users/cancel_reassignment_service.rb b/app/services/import/source_users/cancel_reassignment_service.rb index b3ee14fee7db77..d04c6848c5d64a 100644 --- a/app/services/import/source_users/cancel_reassignment_service.rb +++ b/app/services/import/source_users/cancel_reassignment_service.rb @@ -25,6 +25,8 @@ def execute return error_invalid_status if invalid_status if cancel_successful + track_reassignment_event('placeholder_reassignment_canceled', current_user, import_source_user) + ServiceResponse.success(payload: import_source_user) else ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) diff --git a/app/services/import/source_users/keep_all_as_placeholder_service.rb b/app/services/import/source_users/keep_all_as_placeholder_service.rb index 8a377097001c17..9f43ea35e7839c 100644 --- a/app/services/import/source_users/keep_all_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_all_as_placeholder_service.rb @@ -25,6 +25,8 @@ def execute ) end + track_internal_event('keep_all_as_placeholder', namespace: namespace, user: current_user) + ServiceResponse.success(payload: updated_source_user_count) end diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index 3773dc622c4840..c2aef0292b300c 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -13,6 +13,8 @@ def execute return error_invalid_status unless import_source_user.reassignable_status? if keep_as_placeholder + track_reassignment_event('keep_as_placeholder', current_user, import_source_user) + ServiceResponse.success(payload: import_source_user) else ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) diff --git a/app/services/import/source_users/reassign_service.rb b/app/services/import/source_users/reassign_service.rb index 597746cec0aef5..cf12a2c8df4f21 100644 --- a/app/services/import/source_users/reassign_service.rb +++ b/app/services/import/source_users/reassign_service.rb @@ -28,9 +28,11 @@ def execute if reassign_successful send_user_reassign_email + track_reassignment_event('placeholder_reassignment_proposed', current_user, import_source_user) ServiceResponse.success(payload: import_source_user) else + track_reassignment_event('placeholder_reassignment_failed', current_user, import_source_user) ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/app/services/import/source_users/reject_reassignment_service.rb b/app/services/import/source_users/reject_reassignment_service.rb index 212354bb7feaf1..2cbd4f5a7e92cd 100644 --- a/app/services/import/source_users/reject_reassignment_service.rb +++ b/app/services/import/source_users/reject_reassignment_service.rb @@ -25,6 +25,7 @@ def execute if reject_successful send_user_reassign_rejected_email + track_reassignment_event('placeholder_reassignment_rejected', current_user, import_source_user) ServiceResponse.success(payload: import_source_user) else diff --git a/config/events/keep_all_as_placeholder.yml b/config/events/keep_all_as_placeholder.yml new file mode 100644 index 00000000000000..1cbc6b0de1c3e1 --- /dev/null +++ b/config/events/keep_all_as_placeholder.yml @@ -0,0 +1,16 @@ +--- +description: keep all as placeholder +internal_events: true +action: keep_all_as_placeholder +identifiers: +- namespace +- user +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/keep_as_placeholder.yml b/config/events/keep_as_placeholder.yml new file mode 100644 index 00000000000000..5cfbcd9e036ea2 --- /dev/null +++ b/config/events/keep_as_placeholder.yml @@ -0,0 +1,21 @@ +--- +description: keep as placeholder +internal_events: true +action: keep_as_placeholder +identifiers: +- namespace +- user +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/placeholder_reassignment_accepted.yml b/config/events/placeholder_reassignment_accepted.yml new file mode 100644 index 00000000000000..51c911ea8df5bf --- /dev/null +++ b/config/events/placeholder_reassignment_accepted.yml @@ -0,0 +1,23 @@ +--- +description: placeholder reassignment accepted +internal_events: true +action: placeholder_reassignment_accepted +identifiers: +- namespace +- user +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID + reassign_to_user_id: + description: id of the User the Placeholder was reassigned to +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/placeholder_reassignment_canceled.yml b/config/events/placeholder_reassignment_canceled.yml new file mode 100644 index 00000000000000..4b37dcc41083bc --- /dev/null +++ b/config/events/placeholder_reassignment_canceled.yml @@ -0,0 +1,23 @@ +--- +description: placeholder reassignment canceled +internal_events: true +action: placeholder_reassignment_canceled +identifiers: +- namespace +- user +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID + reassign_to_user_id: + description: id of the User the Placeholder was canceled being reassigned to +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate \ No newline at end of file diff --git a/config/events/placeholder_reassignment_completed.yml b/config/events/placeholder_reassignment_completed.yml new file mode 100644 index 00000000000000..5522b5b57661b8 --- /dev/null +++ b/config/events/placeholder_reassignment_completed.yml @@ -0,0 +1,22 @@ +--- +description: placeholder reassignment completed +internal_events: true +action: placeholder_reassignment_completed +identifiers: +- namespace +additional_properties: + label: + description: The now deleted User id of the Placeholder user who was reassigned + value: + description: PLACEHOLDER_USER_ID + reassign_to_user_id: + description: id of the User the Placeholder has been reassigned to +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate \ No newline at end of file diff --git a/config/events/placeholder_reassignment_failed.yml b/config/events/placeholder_reassignment_failed.yml new file mode 100644 index 00000000000000..f263bb50882505 --- /dev/null +++ b/config/events/placeholder_reassignment_failed.yml @@ -0,0 +1,23 @@ +--- +description: placeholder reassignment failed +internal_events: true +action: placeholder_reassignment_failed +identifiers: +- namespace +- user +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID + reassign_to_user_id: + description: id of the User the Placeholder failed being reassigned to +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate \ No newline at end of file diff --git a/config/events/placeholder_reassignment_proposed.yml b/config/events/placeholder_reassignment_proposed.yml new file mode 100644 index 00000000000000..e8f78d222a720c --- /dev/null +++ b/config/events/placeholder_reassignment_proposed.yml @@ -0,0 +1,23 @@ +--- +description: placeholder reassignment proposed +internal_events: true +action: placeholder_reassignment_proposed +identifiers: +- namespace +- user +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID + reassign_to_user_id: + description: id of the User the Placeholder will be reassigned to +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/placeholder_reassignment_rejected.yml b/config/events/placeholder_reassignment_rejected.yml new file mode 100644 index 00000000000000..7a8cf33da8897c --- /dev/null +++ b/config/events/placeholder_reassignment_rejected.yml @@ -0,0 +1,23 @@ +--- +description: placeholder reassignment rejected +internal_events: true +action: placeholder_reassignment_rejected +identifiers: +- namespace +- user +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID + reassign_to_user_id: + description: id of the User the Placeholder reassignment was rejected by +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/placeholder_user_created.yml b/config/events/placeholder_user_created.yml new file mode 100644 index 00000000000000..7b5e6883f71532 --- /dev/null +++ b/config/events/placeholder_user_created.yml @@ -0,0 +1,20 @@ +--- +description: placeholder user created +internal_events: true +action: placeholder_user_created +identifiers: +- namespace +additional_properties: + label: + description: The User id of the Placeholder user + value: + description: PLACEHOLDER_USER_ID +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index a58a78e66e0f3d..f512d7cb09b8a1 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -3,6 +3,8 @@ module Gitlab module Import class PlaceholderUserCreator + include ::Gitlab::InternalEventsTracking + delegate :import_type, :namespace, :source_user_identifier, :source_name, :source_username, to: :source_user, private: true @@ -38,6 +40,14 @@ def execute user.save! log_placeholder_user_creation(user) + track_internal_event( + 'placeholder_user_created', + namespace: source_user.namespace, + additional_properties: { + label: 'placeholder_user_id', + value: user.id + } + ) user end diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index ba40277895ca07..e10165b417104c 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -53,8 +53,9 @@ } end - it 'logs placeholder user creation' do + it 'logs and tracks placeholder user creation' do allow(::Import::Framework::Logger).to receive(:info) + allow(service).to receive(:track_internal_event) service.execute @@ -67,6 +68,17 @@ user_id: User.last.id ) ) + + expect(service) + .to have_received(:track_internal_event) + .with( + 'placeholder_user_created', + namespace: source_user.namespace, + additional_properties: { + label: 'placeholder_user_id', + value: User.last.id + } + ) end context 'when there are non-unique usernames on the same import source' do diff --git a/spec/services/import/reassign_placeholder_user_records_service_spec.rb b/spec/services/import/reassign_placeholder_user_records_service_spec.rb index 9d54f1e1322a90..1512cebbb6a3a1 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -122,10 +122,23 @@ describe '#execute', :aggregate_failures do before do allow(service).to receive_messages(db_health_check!: nil, db_table_unavailable?: false) + allow(service).to receive(:track_internal_event) end shared_examples 'a successful reassignment' do it 'completes the reassignment' do + expect(service) + .to receive(:track_internal_event) + .with( + 'placeholder_reassignment_completed', + namespace: namespace, + additional_properties: { + label: 'placeholder_user_id', + value: source_user.placeholder_user_id, + reassign_to_user_id: source_user.reassign_to_user_id + } + ) + service.execute expect(source_user.reload).to be_completed @@ -659,6 +672,7 @@ def expect_skipped_membership_log(message, placeholder_membership, existing_memb context 'when database is healthy' do before do allow(service).to receive_messages(db_health_check!: nil, db_table_unhealthy: false) + allow(service).to receive(:track_internal_event) end it 'checks all tables and individual tables' do diff --git a/spec/services/import/source_users/accept_reassignment_service_spec.rb b/spec/services/import/source_users/accept_reassignment_service_spec.rb index bb9a2b9b1c79f6..75bde7fc9dda41 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -11,6 +11,10 @@ end describe '#execute' do + before do + allow(service).to receive(:track_reassignment_event) + end + it 'returns success' do expect(service.execute).to be_success end @@ -27,6 +31,14 @@ service.execute end + it 'tracks the reassignment event' do + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_accepted', current_user, import_source_user) + + service.execute + end + shared_examples 'current user does not have permission to accept reassignment' do it 'returns error no permissions' do result = service.execute @@ -71,6 +83,9 @@ it 'returns transition error' do expect(Import::ReassignPlaceholderUserRecordsWorker).not_to receive(:perform_async) + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_failed', current_user, import_source_user) result = service.execute diff --git a/spec/services/import/source_users/cancel_reassignment_service_spec.rb b/spec/services/import/source_users/cancel_reassignment_service_spec.rb index 08544d1b2bf27d..8fb05fb740149d 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -15,7 +15,15 @@ describe '#execute' do context 'when cancelation is successful' do + before do + allow(service).to receive(:track_reassignment_event) + end + it 'returns success' do + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_canceled', current_user, import_source_user) + result = service.execute expect(result).to be_success diff --git a/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb index 3ec41a4f5eb702..25ee60698c6ab5 100644 --- a/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb @@ -30,6 +30,9 @@ reassignable_source_users = [ source_user_pending_assignment_1, source_user_pending_assignment_2, source_user_rejected ] + expect(service) + .to receive(:track_internal_event) + .with('keep_all_as_placeholder', namespace: namespace, user: current_user) service.execute diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index a59f35761555ef..34676ccffba2d1 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -10,7 +10,15 @@ describe '#execute' do context 'when reassignment is successful' do + before do + allow(service).to receive(:track_reassignment_event) + end + it 'returns success' do + expect(service) + .to receive(:track_reassignment_event) + .with('keep_as_placeholder', current_user, import_source_user) + result = service.execute expect(result).to be_success diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index 64b2c9e636367e..aadf0f187af1c1 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -13,6 +13,9 @@ context 'when reassignment is successful' do it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_proposed', current_user, import_source_user) result = service.execute @@ -105,6 +108,9 @@ it 'assigns the user' do expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_proposed', current_user, import_source_user) expect(service.execute).to be_success end @@ -116,9 +122,18 @@ allow(import_source_user).to receive(:reassign).and_return(false) allow(import_source_user).to receive(:errors).and_return(instance_double(ActiveModel::Errors, full_messages: ['Error'])) + allow(service).to receive(:track_reassignment_event) end it_behaves_like 'an error response', 'active record', error: ['Error'] + + it 'tracks a reassignment event' do + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_failed', current_user, import_source_user) + + service.execute + end end end end diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index 1b8aba193980e8..0ffc855d3ed20b 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -16,10 +16,14 @@ before do allow(message_delivery).to receive(:deliver_now) allow(Notify).to receive(:import_source_user_rejected).and_return(message_delivery) + allow(service).to receive(:track_reassignment_event) end it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_rejected, :deliver_now) + expect(service) + .to receive(:track_reassignment_event) + .with('placeholder_reassignment_rejected', current_user, import_source_user) expect(service.execute).to be_success end -- GitLab From 89aee5bfa04b22fe42ade7e7c31c2c9b2423b8a8 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Fri, 28 Feb 2025 09:27:48 +1100 Subject: [PATCH 02/11] Update keep as placeholder event call --- .../source_users/keep_as_placeholder_service.rb | 10 +++++++++- .../source_users/keep_as_placeholder_service_spec.rb | 12 ++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index c2aef0292b300c..cd330a3e8acdaa 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -13,7 +13,15 @@ def execute return error_invalid_status unless import_source_user.reassignable_status? if keep_as_placeholder - track_reassignment_event('keep_as_placeholder', current_user, import_source_user) + track_internal_event( + 'keep_as_placeholder', + user: current_user, + namespace: import_source_user.namespace, + additional_properties: { + label: 'placeholder_user_id', + value: import_source_user.placeholder_user_id + } + ) ServiceResponse.success(payload: import_source_user) else diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 34676ccffba2d1..3fd6940e883a32 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -16,8 +16,16 @@ it 'returns success' do expect(service) - .to receive(:track_reassignment_event) - .with('keep_as_placeholder', current_user, import_source_user) + .to receive(:track_internal_event) + .with( + 'keep_as_placeholder', + user: current_user, + namespace: import_source_user.namespace, + additional_properties: { + label: 'placeholder_user_id', + value: import_source_user.placeholder_user_id + } + ) result = service.execute -- GitLab From 215662464526ddbfec4396098776adefea4d33e7 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Mon, 3 Mar 2025 16:55:56 +1000 Subject: [PATCH 03/11] Update event names, update specs --- ...assign_placeholder_user_records_service.rb | 21 ++++++++------- .../accept_reassignment_service.rb | 4 +-- .../import/source_users/base_service.rb | 7 +++-- .../cancel_reassignment_service.rb | 17 +++++++++++- .../keep_as_placeholder_service.rb | 21 ++++++++------- .../import/source_users/reassign_service.rb | 4 +-- .../reject_reassignment_service.rb | 2 +- ...ml => accept_placeholder_reassignment.yml} | 12 ++++----- ...ml => cancel_placeholder_reassignment.yml} | 12 ++++----- .../complete_placeholder_reassignment.yml | 20 ++++++++++++++ ...reated.yml => create_placeholder_user.yml} | 8 +++--- ...ml => failed_placeholder_reassignment.yml} | 12 ++++----- config/events/keep_all_as_placeholder.yml | 2 +- config/events/keep_as_placeholder.yml | 6 ++--- .../placeholder_reassignment_canceled.yml | 23 ---------------- .../placeholder_reassignment_completed.yml | 22 ---------------- .../placeholder_reassignment_failed.yml | 23 ---------------- .../propose_placeholder_reassignment.yml | 21 +++++++++++++++ .../reject_placeholder_reassignment.yml | 21 +++++++++++++++ lib/gitlab/import/placeholder_user_creator.rb | 19 ++++++++------ .../import/placeholder_user_creator_spec.rb | 5 ++-- ...n_placeholder_user_records_service_spec.rb | 7 +++-- .../accept_reassignment_service_spec.rb | 26 +++++++++++++++---- .../cancel_reassignment_service_spec.rb | 14 +++++++--- .../keep_as_placeholder_service_spec.rb | 3 +-- .../source_users/reassign_service_spec.rb | 18 +++++++++---- .../reject_reassignment_service_spec.rb | 14 +++++++--- 27 files changed, 204 insertions(+), 160 deletions(-) rename config/events/{placeholder_reassignment_accepted.yml => accept_placeholder_reassignment.yml} (51%) rename config/events/{placeholder_reassignment_proposed.yml => cancel_placeholder_reassignment.yml} (50%) create mode 100644 config/events/complete_placeholder_reassignment.yml rename config/events/{placeholder_user_created.yml => create_placeholder_user.yml} (62%) rename config/events/{placeholder_reassignment_rejected.yml => failed_placeholder_reassignment.yml} (50%) delete mode 100644 config/events/placeholder_reassignment_canceled.yml delete mode 100644 config/events/placeholder_reassignment_completed.yml delete mode 100644 config/events/placeholder_reassignment_failed.yml create mode 100644 config/events/propose_placeholder_reassignment.yml create mode 100644 config/events/reject_placeholder_reassignment.yml diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb index b09732f436e0ca..e6543340c8f97d 100644 --- a/app/services/import/reassign_placeholder_user_records_service.rb +++ b/app/services/import/reassign_placeholder_user_records_service.rb @@ -51,15 +51,7 @@ def execute import_source_user.complete! - track_internal_event( - 'placeholder_reassignment_completed', - namespace: import_source_user.namespace, - additional_properties: { - label: 'placeholder_user_id', - value: placeholder_user_id, - reassign_to_user_id: import_source_user.reassign_to_user_id - } - ) + track_reassignment_completed ServiceResponse.success( message: s_('Import|Placeholder user record reassignment complete'), @@ -300,6 +292,17 @@ def log_create_membership_skipped(message, placeholder_membership, existing_memb ) end + def track_reassignment_completed + track_internal_event( + 'complete_placeholder_reassignment', + namespace: import_source_user.namespace, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: import_source_user.reassign_to_user_id.to_s + } + ) + end + def logger Framework::Logger end diff --git a/app/services/import/source_users/accept_reassignment_service.rb b/app/services/import/source_users/accept_reassignment_service.rb index a7bf6b5009d1ed..a1e46263913dd0 100644 --- a/app/services/import/source_users/accept_reassignment_service.rb +++ b/app/services/import/source_users/accept_reassignment_service.rb @@ -23,11 +23,11 @@ def execute if accept_successful Import::ReassignPlaceholderUserRecordsWorker.perform_async(import_source_user.id) - track_reassignment_event('placeholder_reassignment_accepted', current_user, import_source_user) + track_reassignment_event('accept_placeholder_reassignment') ServiceResponse.success(payload: import_source_user) else - track_reassignment_event('placeholder_reassignment_failed', current_user, import_source_user) + track_reassignment_event('failed_placeholder_reassignment') ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/app/services/import/source_users/base_service.rb b/app/services/import/source_users/base_service.rb index 2b1f1e4d8caeac..01502c18c52ccd 100644 --- a/app/services/import/source_users/base_service.rb +++ b/app/services/import/source_users/base_service.rb @@ -28,15 +28,14 @@ def send_user_reassign_email Notify.import_source_user_reassign(import_source_user.id).deliver_later end - def track_reassignment_event(event_name, current_user, import_source_user) + def track_reassignment_event(event_name) track_internal_event( event_name, user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: 'placeholder_user_id', - value: import_source_user.placeholder_user_id, - reassign_to_user_id: import_source_user.reassign_to_user_id || nil + label: import_source_user.placeholder_user_id.to_s, + property: import_source_user.reassign_to_user_id.to_s } ) end diff --git a/app/services/import/source_users/cancel_reassignment_service.rb b/app/services/import/source_users/cancel_reassignment_service.rb index d04c6848c5d64a..f657639417aa9a 100644 --- a/app/services/import/source_users/cancel_reassignment_service.rb +++ b/app/services/import/source_users/cancel_reassignment_service.rb @@ -3,8 +3,11 @@ module Import module SourceUsers class CancelReassignmentService < BaseService + attr_reader :reassign_to_user_id + def initialize(import_source_user, current_user:) @import_source_user = import_source_user + @reassign_to_user_id = import_source_user.reassign_to_user_id @current_user = current_user end @@ -25,7 +28,7 @@ def execute return error_invalid_status if invalid_status if cancel_successful - track_reassignment_event('placeholder_reassignment_canceled', current_user, import_source_user) + track_reassignment_cancellation ServiceResponse.success(payload: import_source_user) else @@ -40,6 +43,18 @@ def cancel_reassignment import_source_user.reassigned_by_user = nil import_source_user.cancel_reassignment end + + def track_reassignment_cancellation + track_internal_event( + 'cancel_placeholder_reassignment', + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: reassign_to_user_id.to_s + } + ) + end end end end diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index cd330a3e8acdaa..cd52b00d37aad6 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -13,15 +13,7 @@ def execute return error_invalid_status unless import_source_user.reassignable_status? if keep_as_placeholder - track_internal_event( - 'keep_as_placeholder', - user: current_user, - namespace: import_source_user.namespace, - additional_properties: { - label: 'placeholder_user_id', - value: import_source_user.placeholder_user_id - } - ) + track_keep_as_placeholder ServiceResponse.success(payload: import_source_user) else @@ -36,6 +28,17 @@ def keep_as_placeholder import_source_user.reassigned_by_user = current_user import_source_user.keep_as_placeholder end + + def track_keep_as_placeholder + track_internal_event( + 'keep_as_placeholder', + user: current_user, + namespace: import_source_user.namespace, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s + } + ) + end end end end diff --git a/app/services/import/source_users/reassign_service.rb b/app/services/import/source_users/reassign_service.rb index cf12a2c8df4f21..723aefdddf1e88 100644 --- a/app/services/import/source_users/reassign_service.rb +++ b/app/services/import/source_users/reassign_service.rb @@ -28,11 +28,11 @@ def execute if reassign_successful send_user_reassign_email - track_reassignment_event('placeholder_reassignment_proposed', current_user, import_source_user) + track_reassignment_event('propose_placeholder_reassignment') ServiceResponse.success(payload: import_source_user) else - track_reassignment_event('placeholder_reassignment_failed', current_user, import_source_user) + track_reassignment_event('fail_placeholder_reassignment') ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/app/services/import/source_users/reject_reassignment_service.rb b/app/services/import/source_users/reject_reassignment_service.rb index 2cbd4f5a7e92cd..78f698ebbf8fd1 100644 --- a/app/services/import/source_users/reject_reassignment_service.rb +++ b/app/services/import/source_users/reject_reassignment_service.rb @@ -25,7 +25,7 @@ def execute if reject_successful send_user_reassign_rejected_email - track_reassignment_event('placeholder_reassignment_rejected', current_user, import_source_user) + track_reassignment_event('reject_placeholder_reassignment') ServiceResponse.success(payload: import_source_user) else diff --git a/config/events/placeholder_reassignment_accepted.yml b/config/events/accept_placeholder_reassignment.yml similarity index 51% rename from config/events/placeholder_reassignment_accepted.yml rename to config/events/accept_placeholder_reassignment.yml index 51c911ea8df5bf..d0d604a769ca1f 100644 --- a/config/events/placeholder_reassignment_accepted.yml +++ b/config/events/accept_placeholder_reassignment.yml @@ -1,17 +1,15 @@ --- -description: placeholder reassignment accepted +description: Tracks when a placeholder user reassignment is accepted internal_events: true -action: placeholder_reassignment_accepted +action: accept_placeholder_reassignment identifiers: - namespace - user additional_properties: label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID - reassign_to_user_id: - description: id of the User the Placeholder was reassigned to + description: id of the placeholder user + property: + description: id of the user that is replacing the placeholder product_group: import_and_integrate product_categories: - importers diff --git a/config/events/placeholder_reassignment_proposed.yml b/config/events/cancel_placeholder_reassignment.yml similarity index 50% rename from config/events/placeholder_reassignment_proposed.yml rename to config/events/cancel_placeholder_reassignment.yml index e8f78d222a720c..8ec49096f42f78 100644 --- a/config/events/placeholder_reassignment_proposed.yml +++ b/config/events/cancel_placeholder_reassignment.yml @@ -1,17 +1,15 @@ --- -description: placeholder reassignment proposed +description: Tracks when a placeholder user reassignment is canceled internal_events: true -action: placeholder_reassignment_proposed +action: cancel_placeholder_reassignment identifiers: - namespace - user additional_properties: label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID - reassign_to_user_id: - description: id of the User the Placeholder will be reassigned to + description: id of the placeholder user + property: + description: id of the user that was to replace the placeholder product_group: import_and_integrate product_categories: - importers diff --git a/config/events/complete_placeholder_reassignment.yml b/config/events/complete_placeholder_reassignment.yml new file mode 100644 index 00000000000000..76904771bd848a --- /dev/null +++ b/config/events/complete_placeholder_reassignment.yml @@ -0,0 +1,20 @@ +--- +description: Tracks when a placeholder user reassignment is completed +internal_events: true +action: complete_placeholder_reassignment +identifiers: +- namespace +additional_properties: + label: + description: id of the placeholder user + property: + description: id of the user that replaced the placeholder +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/placeholder_user_created.yml b/config/events/create_placeholder_user.yml similarity index 62% rename from config/events/placeholder_user_created.yml rename to config/events/create_placeholder_user.yml index 7b5e6883f71532..cec76a84e1a682 100644 --- a/config/events/placeholder_user_created.yml +++ b/config/events/create_placeholder_user.yml @@ -1,14 +1,12 @@ --- -description: placeholder user created +description: Tracks when a placeholder user is created internal_events: true -action: placeholder_user_created +action: create_placeholder_user identifiers: - namespace additional_properties: label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID + description: id of the placeholder user product_group: import_and_integrate product_categories: - importers diff --git a/config/events/placeholder_reassignment_rejected.yml b/config/events/failed_placeholder_reassignment.yml similarity index 50% rename from config/events/placeholder_reassignment_rejected.yml rename to config/events/failed_placeholder_reassignment.yml index 7a8cf33da8897c..06e05594801147 100644 --- a/config/events/placeholder_reassignment_rejected.yml +++ b/config/events/failed_placeholder_reassignment.yml @@ -1,17 +1,15 @@ --- -description: placeholder reassignment rejected +description: Tracks when a placeholder user reassignment fails internal_events: true -action: placeholder_reassignment_rejected +action: failed_placeholder_reassignment identifiers: - namespace - user additional_properties: label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID - reassign_to_user_id: - description: id of the User the Placeholder reassignment was rejected by + description: id of the placeholder user + property: + description: id of the user that did not replace the placeholder product_group: import_and_integrate product_categories: - importers diff --git a/config/events/keep_all_as_placeholder.yml b/config/events/keep_all_as_placeholder.yml index 1cbc6b0de1c3e1..c27106a93ac0cb 100644 --- a/config/events/keep_all_as_placeholder.yml +++ b/config/events/keep_all_as_placeholder.yml @@ -1,5 +1,5 @@ --- -description: keep all as placeholder +description: Tracks when all placeholder users are kept as placeholders and not reassigned to existing users internal_events: true action: keep_all_as_placeholder identifiers: diff --git a/config/events/keep_as_placeholder.yml b/config/events/keep_as_placeholder.yml index 5cfbcd9e036ea2..a44073d615dc23 100644 --- a/config/events/keep_as_placeholder.yml +++ b/config/events/keep_as_placeholder.yml @@ -1,5 +1,5 @@ --- -description: keep as placeholder +description: Tracks when a placeholder user is kept as a placeholder and not reassigned to an existing user internal_events: true action: keep_as_placeholder identifiers: @@ -7,9 +7,7 @@ identifiers: - user additional_properties: label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID + description: id of the placeholder user product_group: import_and_integrate product_categories: - importers diff --git a/config/events/placeholder_reassignment_canceled.yml b/config/events/placeholder_reassignment_canceled.yml deleted file mode 100644 index 4b37dcc41083bc..00000000000000 --- a/config/events/placeholder_reassignment_canceled.yml +++ /dev/null @@ -1,23 +0,0 @@ ---- -description: placeholder reassignment canceled -internal_events: true -action: placeholder_reassignment_canceled -identifiers: -- namespace -- user -additional_properties: - label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID - reassign_to_user_id: - description: id of the User the Placeholder was canceled being reassigned to -product_group: import_and_integrate -product_categories: -- importers -milestone: '17.10' -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 -tiers: -- free -- premium -- ultimate \ No newline at end of file diff --git a/config/events/placeholder_reassignment_completed.yml b/config/events/placeholder_reassignment_completed.yml deleted file mode 100644 index 5522b5b57661b8..00000000000000 --- a/config/events/placeholder_reassignment_completed.yml +++ /dev/null @@ -1,22 +0,0 @@ ---- -description: placeholder reassignment completed -internal_events: true -action: placeholder_reassignment_completed -identifiers: -- namespace -additional_properties: - label: - description: The now deleted User id of the Placeholder user who was reassigned - value: - description: PLACEHOLDER_USER_ID - reassign_to_user_id: - description: id of the User the Placeholder has been reassigned to -product_group: import_and_integrate -product_categories: -- importers -milestone: '17.10' -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 -tiers: -- free -- premium -- ultimate \ No newline at end of file diff --git a/config/events/placeholder_reassignment_failed.yml b/config/events/placeholder_reassignment_failed.yml deleted file mode 100644 index f263bb50882505..00000000000000 --- a/config/events/placeholder_reassignment_failed.yml +++ /dev/null @@ -1,23 +0,0 @@ ---- -description: placeholder reassignment failed -internal_events: true -action: placeholder_reassignment_failed -identifiers: -- namespace -- user -additional_properties: - label: - description: The User id of the Placeholder user - value: - description: PLACEHOLDER_USER_ID - reassign_to_user_id: - description: id of the User the Placeholder failed being reassigned to -product_group: import_and_integrate -product_categories: -- importers -milestone: '17.10' -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 -tiers: -- free -- premium -- ultimate \ No newline at end of file diff --git a/config/events/propose_placeholder_reassignment.yml b/config/events/propose_placeholder_reassignment.yml new file mode 100644 index 00000000000000..dd780ae64cfcd6 --- /dev/null +++ b/config/events/propose_placeholder_reassignment.yml @@ -0,0 +1,21 @@ +--- +description: Tracks when a placeholder user reassignment is proposed +internal_events: true +action: propose_placeholder_reassignment +identifiers: +- namespace +- user +additional_properties: + label: + description: id of the placeholder user + property: + description: id of the user that is replacing the placeholder +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/reject_placeholder_reassignment.yml b/config/events/reject_placeholder_reassignment.yml new file mode 100644 index 00000000000000..9ad150a70744cb --- /dev/null +++ b/config/events/reject_placeholder_reassignment.yml @@ -0,0 +1,21 @@ +--- +description: Tracks when a placeholder user reassignment is rejected +internal_events: true +action: reject_placeholder_reassignment +identifiers: +- namespace +- user +additional_properties: + label: + description: id of the placeholder user + property: + description: id of the user that was to replace the placeholder +product_group: import_and_integrate +product_categories: +- importers +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index f512d7cb09b8a1..c78f7dd6e9a23e 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -40,14 +40,7 @@ def execute user.save! log_placeholder_user_creation(user) - track_internal_event( - 'placeholder_user_created', - namespace: source_user.namespace, - additional_properties: { - label: 'placeholder_user_id', - value: user.id - } - ) + track_placeholder_user_creation(source_user, user) user end @@ -98,6 +91,16 @@ def log_placeholder_user_creation(user) user_id: user.id ) end + + def track_placeholder_user_creation(source_user, user) + track_internal_event( + 'create_placeholder_user', + namespace: source_user.namespace, + additional_properties: { + label: user.id.to_s + } + ) + end end end end diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index e10165b417104c..8a6080d49e5615 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -72,11 +72,10 @@ expect(service) .to have_received(:track_internal_event) .with( - 'placeholder_user_created', + 'create_placeholder_user', namespace: source_user.namespace, additional_properties: { - label: 'placeholder_user_id', - value: User.last.id + label: User.last.id.to_s } ) end diff --git a/spec/services/import/reassign_placeholder_user_records_service_spec.rb b/spec/services/import/reassign_placeholder_user_records_service_spec.rb index 1512cebbb6a3a1..cf6e5f292360b7 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -130,12 +130,11 @@ expect(service) .to receive(:track_internal_event) .with( - 'placeholder_reassignment_completed', + 'complete_placeholder_reassignment', namespace: namespace, additional_properties: { - label: 'placeholder_user_id', - value: source_user.placeholder_user_id, - reassign_to_user_id: source_user.reassign_to_user_id + label: source_user.placeholder_user_id.to_s, + property: source_user.reassign_to_user_id.to_s } ) diff --git a/spec/services/import/source_users/accept_reassignment_service_spec.rb b/spec/services/import/source_users/accept_reassignment_service_spec.rb index 75bde7fc9dda41..c1df75d86f5417 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -12,7 +12,7 @@ describe '#execute' do before do - allow(service).to receive(:track_reassignment_event) + allow(service).to receive(:track_internal_event) end it 'returns success' do @@ -33,8 +33,16 @@ it 'tracks the reassignment event' do expect(service) - .to receive(:track_reassignment_event) - .with('placeholder_reassignment_accepted', current_user, import_source_user) + .to receive(:track_internal_event) + .with( + 'accept_placeholder_reassignment', + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: import_source_user.reassign_to_user_id.to_s + } + ) service.execute end @@ -84,8 +92,16 @@ it 'returns transition error' do expect(Import::ReassignPlaceholderUserRecordsWorker).not_to receive(:perform_async) expect(service) - .to receive(:track_reassignment_event) - .with('placeholder_reassignment_failed', current_user, import_source_user) + .to receive(:track_internal_event) + .with( + 'failed_placeholder_reassignment', + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: import_source_user.reassign_to_user_id.to_s + } + ) result = service.execute diff --git a/spec/services/import/source_users/cancel_reassignment_service_spec.rb b/spec/services/import/source_users/cancel_reassignment_service_spec.rb index 8fb05fb740149d..46c08cd0e5cde0 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -16,13 +16,21 @@ describe '#execute' do context 'when cancelation is successful' do before do - allow(service).to receive(:track_reassignment_event) + allow(service).to receive(:track_internal_event) end it 'returns success' do expect(service) - .to receive(:track_reassignment_event) - .with('placeholder_reassignment_canceled', current_user, import_source_user) + .to receive(:track_internal_event) + .with( + 'cancel_placeholder_reassignment', + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: user.id.to_s + } + ) result = service.execute diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 3fd6940e883a32..17f60b6628183b 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -22,8 +22,7 @@ user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: 'placeholder_user_id', - value: import_source_user.placeholder_user_id + label: import_source_user.placeholder_user_id.to_s } ) diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index aadf0f187af1c1..29dfeac8ba58ef 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -14,8 +14,16 @@ it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) expect(service) - .to receive(:track_reassignment_event) - .with('placeholder_reassignment_proposed', current_user, import_source_user) + .to receive(:track_internal_event) + .with( + 'propose_placeholder_reassignment', + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: assignee_user.id.to_s + } + ) result = service.execute @@ -110,7 +118,7 @@ expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) expect(service) .to receive(:track_reassignment_event) - .with('placeholder_reassignment_proposed', current_user, import_source_user) + .with('propose_placeholder_reassignment') expect(service.execute).to be_success end @@ -122,7 +130,7 @@ allow(import_source_user).to receive(:reassign).and_return(false) allow(import_source_user).to receive(:errors).and_return(instance_double(ActiveModel::Errors, full_messages: ['Error'])) - allow(service).to receive(:track_reassignment_event) + allow(service).to receive(:track_internal_event) end it_behaves_like 'an error response', 'active record', error: ['Error'] @@ -130,7 +138,7 @@ it 'tracks a reassignment event' do expect(service) .to receive(:track_reassignment_event) - .with('placeholder_reassignment_failed', current_user, import_source_user) + .with('fail_placeholder_reassignment') service.execute end diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index 0ffc855d3ed20b..8e733d39385f93 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -16,14 +16,22 @@ before do allow(message_delivery).to receive(:deliver_now) allow(Notify).to receive(:import_source_user_rejected).and_return(message_delivery) - allow(service).to receive(:track_reassignment_event) + allow(service).to receive(:track_internal_event) end it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_rejected, :deliver_now) expect(service) - .to receive(:track_reassignment_event) - .with('placeholder_reassignment_rejected', current_user, import_source_user) + .to receive(:track_internal_event) + .with( + 'reject_placeholder_reassignment', + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: import_source_user.reassign_to_user_id.to_s + } + ) expect(service.execute).to be_success end -- GitLab From 94062304971974b12e38c3cc92c97263e3acc601 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Tue, 4 Mar 2025 10:48:27 +1000 Subject: [PATCH 04/11] Update event names to include user --- .../import/reassign_placeholder_user_records_service.rb | 8 ++++---- .../import/source_users/accept_reassignment_service.rb | 4 ++-- .../import/source_users/cancel_reassignment_service.rb | 2 +- .../source_users/keep_all_as_placeholder_service.rb | 2 +- .../import/source_users/keep_as_placeholder_service.rb | 2 +- app/services/import/source_users/reassign_service.rb | 2 +- .../import/source_users/reject_reassignment_service.rb | 2 +- ...nment.yml => accept_placeholder_user_reassignment.yml} | 2 +- ...nment.yml => cancel_placeholder_user_reassignment.yml} | 2 +- ...ent.yml => complete_placeholder_user_reassignment.yml} | 2 +- ...nment.yml => failed_placeholder_user_reassignment.yml} | 2 +- ...s_placeholder.yml => keep_all_as_placeholder_user.yml} | 2 +- ...ep_as_placeholder.yml => keep_as_placeholder_user.yml} | 0 ...ment.yml => propose_placeholder_user_reassignment.yml} | 2 +- ...nment.yml => reject_placeholder_user_reassignment.yml} | 2 +- .../reassign_placeholder_user_records_service_spec.rb | 2 +- .../source_users/accept_reassignment_service_spec.rb | 4 ++-- .../source_users/cancel_reassignment_service_spec.rb | 2 +- .../source_users/keep_all_as_placeholder_service_spec.rb | 2 +- .../source_users/keep_as_placeholder_service_spec.rb | 2 +- .../services/import/source_users/reassign_service_spec.rb | 4 ++-- .../source_users/reject_reassignment_service_spec.rb | 2 +- 22 files changed, 27 insertions(+), 27 deletions(-) rename config/events/{accept_placeholder_reassignment.yml => accept_placeholder_user_reassignment.yml} (91%) rename config/events/{cancel_placeholder_reassignment.yml => cancel_placeholder_user_reassignment.yml} (91%) rename config/events/{complete_placeholder_reassignment.yml => complete_placeholder_user_reassignment.yml} (90%) rename config/events/{failed_placeholder_reassignment.yml => failed_placeholder_user_reassignment.yml} (91%) rename config/events/{keep_all_as_placeholder.yml => keep_all_as_placeholder_user.yml} (90%) rename config/events/{keep_as_placeholder.yml => keep_as_placeholder_user.yml} (100%) rename config/events/{propose_placeholder_reassignment.yml => propose_placeholder_user_reassignment.yml} (91%) rename config/events/{reject_placeholder_reassignment.yml => reject_placeholder_user_reassignment.yml} (91%) diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb index e6543340c8f97d..c536e54d9bf9f7 100644 --- a/app/services/import/reassign_placeholder_user_records_service.rb +++ b/app/services/import/reassign_placeholder_user_records_service.rb @@ -51,7 +51,7 @@ def execute import_source_user.complete! - track_reassignment_completed + track_reassignment_complete ServiceResponse.success( message: s_('Import|Placeholder user record reassignment complete'), @@ -292,12 +292,12 @@ def log_create_membership_skipped(message, placeholder_membership, existing_memb ) end - def track_reassignment_completed + def track_reassignment_complete track_internal_event( - 'complete_placeholder_reassignment', + 'complete_placeholder_user_reassignment', namespace: import_source_user.namespace, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, + label: placeholder_user_id.to_s, property: import_source_user.reassign_to_user_id.to_s } ) diff --git a/app/services/import/source_users/accept_reassignment_service.rb b/app/services/import/source_users/accept_reassignment_service.rb index a1e46263913dd0..39ff3ea55acc64 100644 --- a/app/services/import/source_users/accept_reassignment_service.rb +++ b/app/services/import/source_users/accept_reassignment_service.rb @@ -23,11 +23,11 @@ def execute if accept_successful Import::ReassignPlaceholderUserRecordsWorker.perform_async(import_source_user.id) - track_reassignment_event('accept_placeholder_reassignment') + track_reassignment_event('accept_placeholder_user_reassignment') ServiceResponse.success(payload: import_source_user) else - track_reassignment_event('failed_placeholder_reassignment') + track_reassignment_event('failed_placeholder_user_reassignment') ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/app/services/import/source_users/cancel_reassignment_service.rb b/app/services/import/source_users/cancel_reassignment_service.rb index f657639417aa9a..649a6f603fc744 100644 --- a/app/services/import/source_users/cancel_reassignment_service.rb +++ b/app/services/import/source_users/cancel_reassignment_service.rb @@ -46,7 +46,7 @@ def cancel_reassignment def track_reassignment_cancellation track_internal_event( - 'cancel_placeholder_reassignment', + 'cancel_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { diff --git a/app/services/import/source_users/keep_all_as_placeholder_service.rb b/app/services/import/source_users/keep_all_as_placeholder_service.rb index 9f43ea35e7839c..f2f08fcc184f32 100644 --- a/app/services/import/source_users/keep_all_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_all_as_placeholder_service.rb @@ -25,7 +25,7 @@ def execute ) end - track_internal_event('keep_all_as_placeholder', namespace: namespace, user: current_user) + track_internal_event('keep_all_as_placeholder_user', namespace: namespace, user: current_user) ServiceResponse.success(payload: updated_source_user_count) end diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index cd52b00d37aad6..a24c53867d24c1 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -31,7 +31,7 @@ def keep_as_placeholder def track_keep_as_placeholder track_internal_event( - 'keep_as_placeholder', + 'keep_as_placeholder_user', user: current_user, namespace: import_source_user.namespace, additional_properties: { diff --git a/app/services/import/source_users/reassign_service.rb b/app/services/import/source_users/reassign_service.rb index 723aefdddf1e88..88b303310f10f8 100644 --- a/app/services/import/source_users/reassign_service.rb +++ b/app/services/import/source_users/reassign_service.rb @@ -28,7 +28,7 @@ def execute if reassign_successful send_user_reassign_email - track_reassignment_event('propose_placeholder_reassignment') + track_reassignment_event('propose_placeholder_user_reassignment') ServiceResponse.success(payload: import_source_user) else diff --git a/app/services/import/source_users/reject_reassignment_service.rb b/app/services/import/source_users/reject_reassignment_service.rb index 78f698ebbf8fd1..abb0113ab55563 100644 --- a/app/services/import/source_users/reject_reassignment_service.rb +++ b/app/services/import/source_users/reject_reassignment_service.rb @@ -25,7 +25,7 @@ def execute if reject_successful send_user_reassign_rejected_email - track_reassignment_event('reject_placeholder_reassignment') + track_reassignment_event('reject_placeholder_user_reassignment') ServiceResponse.success(payload: import_source_user) else diff --git a/config/events/accept_placeholder_reassignment.yml b/config/events/accept_placeholder_user_reassignment.yml similarity index 91% rename from config/events/accept_placeholder_reassignment.yml rename to config/events/accept_placeholder_user_reassignment.yml index d0d604a769ca1f..fc049e5c2037c9 100644 --- a/config/events/accept_placeholder_reassignment.yml +++ b/config/events/accept_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment is accepted internal_events: true -action: accept_placeholder_reassignment +action: accept_placeholder_user_reassignment identifiers: - namespace - user diff --git a/config/events/cancel_placeholder_reassignment.yml b/config/events/cancel_placeholder_user_reassignment.yml similarity index 91% rename from config/events/cancel_placeholder_reassignment.yml rename to config/events/cancel_placeholder_user_reassignment.yml index 8ec49096f42f78..ef61aeb8c632a0 100644 --- a/config/events/cancel_placeholder_reassignment.yml +++ b/config/events/cancel_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment is canceled internal_events: true -action: cancel_placeholder_reassignment +action: cancel_placeholder_user_reassignment identifiers: - namespace - user diff --git a/config/events/complete_placeholder_reassignment.yml b/config/events/complete_placeholder_user_reassignment.yml similarity index 90% rename from config/events/complete_placeholder_reassignment.yml rename to config/events/complete_placeholder_user_reassignment.yml index 76904771bd848a..d38441be388af9 100644 --- a/config/events/complete_placeholder_reassignment.yml +++ b/config/events/complete_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment is completed internal_events: true -action: complete_placeholder_reassignment +action: complete_placeholder_user_reassignment identifiers: - namespace additional_properties: diff --git a/config/events/failed_placeholder_reassignment.yml b/config/events/failed_placeholder_user_reassignment.yml similarity index 91% rename from config/events/failed_placeholder_reassignment.yml rename to config/events/failed_placeholder_user_reassignment.yml index 06e05594801147..bb7ef9181b3b5b 100644 --- a/config/events/failed_placeholder_reassignment.yml +++ b/config/events/failed_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment fails internal_events: true -action: failed_placeholder_reassignment +action: failed_placeholder_user_reassignment identifiers: - namespace - user diff --git a/config/events/keep_all_as_placeholder.yml b/config/events/keep_all_as_placeholder_user.yml similarity index 90% rename from config/events/keep_all_as_placeholder.yml rename to config/events/keep_all_as_placeholder_user.yml index c27106a93ac0cb..50b494e42c2243 100644 --- a/config/events/keep_all_as_placeholder.yml +++ b/config/events/keep_all_as_placeholder_user.yml @@ -1,7 +1,7 @@ --- description: Tracks when all placeholder users are kept as placeholders and not reassigned to existing users internal_events: true -action: keep_all_as_placeholder +action: keep_all_as_placeholder_user identifiers: - namespace - user diff --git a/config/events/keep_as_placeholder.yml b/config/events/keep_as_placeholder_user.yml similarity index 100% rename from config/events/keep_as_placeholder.yml rename to config/events/keep_as_placeholder_user.yml diff --git a/config/events/propose_placeholder_reassignment.yml b/config/events/propose_placeholder_user_reassignment.yml similarity index 91% rename from config/events/propose_placeholder_reassignment.yml rename to config/events/propose_placeholder_user_reassignment.yml index dd780ae64cfcd6..2017f394e657c8 100644 --- a/config/events/propose_placeholder_reassignment.yml +++ b/config/events/propose_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment is proposed internal_events: true -action: propose_placeholder_reassignment +action: propose_placeholder_user_reassignment identifiers: - namespace - user diff --git a/config/events/reject_placeholder_reassignment.yml b/config/events/reject_placeholder_user_reassignment.yml similarity index 91% rename from config/events/reject_placeholder_reassignment.yml rename to config/events/reject_placeholder_user_reassignment.yml index 9ad150a70744cb..a3ebb6d4a902ec 100644 --- a/config/events/reject_placeholder_reassignment.yml +++ b/config/events/reject_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment is rejected internal_events: true -action: reject_placeholder_reassignment +action: reject_placeholder_user_reassignment identifiers: - namespace - user diff --git a/spec/services/import/reassign_placeholder_user_records_service_spec.rb b/spec/services/import/reassign_placeholder_user_records_service_spec.rb index cf6e5f292360b7..f21fedad487365 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -130,7 +130,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'complete_placeholder_reassignment', + 'complete_placeholder_user_reassignment', namespace: namespace, additional_properties: { label: source_user.placeholder_user_id.to_s, diff --git a/spec/services/import/source_users/accept_reassignment_service_spec.rb b/spec/services/import/source_users/accept_reassignment_service_spec.rb index c1df75d86f5417..81483b6375b72f 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -35,7 +35,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'accept_placeholder_reassignment', + 'accept_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -94,7 +94,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'failed_placeholder_reassignment', + 'failed_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { diff --git a/spec/services/import/source_users/cancel_reassignment_service_spec.rb b/spec/services/import/source_users/cancel_reassignment_service_spec.rb index 46c08cd0e5cde0..87c4682687b5f4 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -23,7 +23,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'cancel_placeholder_reassignment', + 'cancel_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { diff --git a/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb index 25ee60698c6ab5..6102725c1042e7 100644 --- a/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb @@ -32,7 +32,7 @@ ] expect(service) .to receive(:track_internal_event) - .with('keep_all_as_placeholder', namespace: namespace, user: current_user) + .with('keep_all_as_placeholder_user', namespace: namespace, user: current_user) service.execute diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 17f60b6628183b..2d7fc535297687 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -18,7 +18,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'keep_as_placeholder', + 'keep_as_placeholder_user', user: current_user, namespace: import_source_user.namespace, additional_properties: { diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index 29dfeac8ba58ef..8c6041524bed70 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -16,7 +16,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'propose_placeholder_reassignment', + 'propose_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -118,7 +118,7 @@ expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) expect(service) .to receive(:track_reassignment_event) - .with('propose_placeholder_reassignment') + .with('propose_placeholder_user_reassignment') expect(service.execute).to be_success end diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index 8e733d39385f93..1600ed9f294afd 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -24,7 +24,7 @@ expect(service) .to receive(:track_internal_event) .with( - 'reject_placeholder_reassignment', + 'reject_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { -- GitLab From 6d7616237feb98e0b0fc5b6c91eedb67c102a076 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Tue, 4 Mar 2025 14:35:31 +1000 Subject: [PATCH 05/11] Use internal events helper in specs --- .../accept_reassignment_service.rb | 2 +- .../import/source_users/reassign_service.rb | 2 +- ...=> fail_placeholder_user_reassignment.yml} | 2 +- config/events/keep_as_placeholder_user.yml | 2 +- .../import/placeholder_user_creator_spec.rb | 20 ++++----- ...n_placeholder_user_records_service_spec.rb | 9 +--- .../accept_reassignment_service_spec.rb | 19 +++------ .../cancel_reassignment_service_spec.rb | 18 ++------ .../keep_all_as_placeholder_service_spec.rb | 8 ++-- .../keep_as_placeholder_service_spec.rb | 13 ++---- .../source_users/reassign_service_spec.rb | 41 +++++++++++-------- .../reject_reassignment_service_spec.rb | 13 +++--- 12 files changed, 57 insertions(+), 92 deletions(-) rename config/events/{failed_placeholder_user_reassignment.yml => fail_placeholder_user_reassignment.yml} (91%) diff --git a/app/services/import/source_users/accept_reassignment_service.rb b/app/services/import/source_users/accept_reassignment_service.rb index 39ff3ea55acc64..99833cdb01f24a 100644 --- a/app/services/import/source_users/accept_reassignment_service.rb +++ b/app/services/import/source_users/accept_reassignment_service.rb @@ -27,7 +27,7 @@ def execute ServiceResponse.success(payload: import_source_user) else - track_reassignment_event('failed_placeholder_user_reassignment') + track_reassignment_event('fail_placeholder_user_reassignment') ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/app/services/import/source_users/reassign_service.rb b/app/services/import/source_users/reassign_service.rb index 88b303310f10f8..6779359fcbc52f 100644 --- a/app/services/import/source_users/reassign_service.rb +++ b/app/services/import/source_users/reassign_service.rb @@ -32,7 +32,7 @@ def execute ServiceResponse.success(payload: import_source_user) else - track_reassignment_event('fail_placeholder_reassignment') + track_reassignment_event('fail_placeholder_user_reassignment') ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) end end diff --git a/config/events/failed_placeholder_user_reassignment.yml b/config/events/fail_placeholder_user_reassignment.yml similarity index 91% rename from config/events/failed_placeholder_user_reassignment.yml rename to config/events/fail_placeholder_user_reassignment.yml index bb7ef9181b3b5b..7253eeac9c4220 100644 --- a/config/events/failed_placeholder_user_reassignment.yml +++ b/config/events/fail_placeholder_user_reassignment.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user reassignment fails internal_events: true -action: failed_placeholder_user_reassignment +action: fail_placeholder_user_reassignment identifiers: - namespace - user diff --git a/config/events/keep_as_placeholder_user.yml b/config/events/keep_as_placeholder_user.yml index a44073d615dc23..9a6877d78cac44 100644 --- a/config/events/keep_as_placeholder_user.yml +++ b/config/events/keep_as_placeholder_user.yml @@ -1,7 +1,7 @@ --- description: Tracks when a placeholder user is kept as a placeholder and not reassigned to an existing user internal_events: true -action: keep_as_placeholder +action: keep_as_placeholder_user identifiers: - namespace - user diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index 8a6080d49e5615..d57cb8fca31b2e 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -55,9 +55,15 @@ it 'logs and tracks placeholder user creation' do allow(::Import::Framework::Logger).to receive(:info) - allow(service).to receive(:track_internal_event) - service.execute + expect { service.execute } + .to trigger_internal_events('create_placeholder_user') + .with( + namespace: namespace, + additional_properties: { + label: User.maximum(:id).next.to_s + } + ) expect(::Import::Framework::Logger).to have_received(:info).with( hash_including( @@ -68,16 +74,6 @@ user_id: User.last.id ) ) - - expect(service) - .to have_received(:track_internal_event) - .with( - 'create_placeholder_user', - namespace: source_user.namespace, - additional_properties: { - label: User.last.id.to_s - } - ) end context 'when there are non-unique usernames on the same import source' do diff --git a/spec/services/import/reassign_placeholder_user_records_service_spec.rb b/spec/services/import/reassign_placeholder_user_records_service_spec.rb index f21fedad487365..d58262dbb54f3f 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -122,15 +122,13 @@ describe '#execute', :aggregate_failures do before do allow(service).to receive_messages(db_health_check!: nil, db_table_unavailable?: false) - allow(service).to receive(:track_internal_event) end shared_examples 'a successful reassignment' do it 'completes the reassignment' do - expect(service) - .to receive(:track_internal_event) + expect { service.execute } + .to trigger_internal_events('complete_placeholder_user_reassignment') .with( - 'complete_placeholder_user_reassignment', namespace: namespace, additional_properties: { label: source_user.placeholder_user_id.to_s, @@ -138,8 +136,6 @@ } ) - service.execute - expect(source_user.reload).to be_completed end @@ -671,7 +667,6 @@ def expect_skipped_membership_log(message, placeholder_membership, existing_memb context 'when database is healthy' do before do allow(service).to receive_messages(db_health_check!: nil, db_table_unhealthy: false) - allow(service).to receive(:track_internal_event) end it 'checks all tables and individual tables' do diff --git a/spec/services/import/source_users/accept_reassignment_service_spec.rb b/spec/services/import/source_users/accept_reassignment_service_spec.rb index 81483b6375b72f..bb0da72226c2f0 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -11,10 +11,6 @@ end describe '#execute' do - before do - allow(service).to receive(:track_internal_event) - end - it 'returns success' do expect(service.execute).to be_success end @@ -32,10 +28,9 @@ end it 'tracks the reassignment event' do - expect(service) - .to receive(:track_internal_event) + expect { service.execute } + .to trigger_internal_events('accept_placeholder_user_reassignment') .with( - 'accept_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -43,8 +38,6 @@ property: import_source_user.reassign_to_user_id.to_s } ) - - service.execute end shared_examples 'current user does not have permission to accept reassignment' do @@ -88,13 +81,13 @@ context 'when the source user is not awaiting approval' do let(:import_source_user) { create(:import_source_user, :reassignment_in_progress) } + let(:result) { service.execute } it 'returns transition error' do expect(Import::ReassignPlaceholderUserRecordsWorker).not_to receive(:perform_async) - expect(service) - .to receive(:track_internal_event) + expect { result } + .to trigger_internal_events('fail_placeholder_user_reassignment') .with( - 'failed_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -103,8 +96,6 @@ } ) - result = service.execute - expect(result).to be_error expect(result.message).to include('Status cannot transition via "accept"') end diff --git a/spec/services/import/source_users/cancel_reassignment_service_spec.rb b/spec/services/import/source_users/cancel_reassignment_service_spec.rb index 87c4682687b5f4..6e3f784c513456 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -12,18 +12,14 @@ let(:current_user) { user } let(:service) { described_class.new(import_source_user, current_user: current_user) } + let(:result) { service.execute } describe '#execute' do context 'when cancelation is successful' do - before do - allow(service).to receive(:track_internal_event) - end - it 'returns success' do - expect(service) - .to receive(:track_internal_event) + expect { result } + .to trigger_internal_events('cancel_placeholder_user_reassignment') .with( - 'cancel_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -32,8 +28,6 @@ } ) - result = service.execute - expect(result).to be_success expect(result.payload.reload).to eq(import_source_user) expect(result.payload.reassigned_by_user).to eq(nil) @@ -46,8 +40,6 @@ let(:current_user) { create(:user) } it 'returns error no permissions' do - result = service.execute - expect(result).to be_error expect(result.message).to eq('You have insufficient permissions to update the import source user') end @@ -59,8 +51,6 @@ end it 'returns error invalid status' do - result = service.execute - expect(result).to be_error expect(result.message).to eq('Import source user has an invalid status for this operation') end @@ -74,8 +64,6 @@ end it 'returns an error' do - result = service.execute - expect(result).to be_error expect(result.message).to eq(['Error']) end diff --git a/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb index 6102725c1042e7..c5477249e40bbc 100644 --- a/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_all_as_placeholder_service_spec.rb @@ -30,11 +30,9 @@ reassignable_source_users = [ source_user_pending_assignment_1, source_user_pending_assignment_2, source_user_rejected ] - expect(service) - .to receive(:track_internal_event) - .with('keep_all_as_placeholder_user', namespace: namespace, user: current_user) - - service.execute + expect { service.execute } + .to trigger_internal_events('keep_all_as_placeholder_user') + .with(namespace: namespace, user: current_user) reassignable_source_users.each(&:reload) expect(reassignable_source_users.all?(&:keep_as_placeholder?)).to be(true) diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 2d7fc535297687..ad7b642fe28b64 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -7,6 +7,7 @@ let(:user) { import_source_user.namespace.owner } let(:current_user) { user } let(:service) { described_class.new(import_source_user, current_user: current_user) } + let(:result) { service.execute } describe '#execute' do context 'when reassignment is successful' do @@ -15,10 +16,9 @@ end it 'returns success' do - expect(service) - .to receive(:track_internal_event) + expect { result } + .to trigger_internal_events('keep_as_placeholder_user') .with( - 'keep_as_placeholder_user', user: current_user, namespace: import_source_user.namespace, additional_properties: { @@ -26,8 +26,6 @@ } ) - result = service.execute - expect(result).to be_success expect(result.payload.reload).to eq(import_source_user) expect(result.payload.reassign_to_user).to eq(nil) @@ -40,8 +38,6 @@ let(:current_user) { create(:user) } it 'returns error no permissions' do - result = service.execute - expect(result).to be_error expect(result.message).to eq('You have insufficient permissions to update the import source user') end @@ -54,7 +50,6 @@ end it 'returns error invalid status' do - result = service.execute expect(result).to be_error expect(result.message).to eq('Import source user has an invalid status for this operation') end @@ -68,8 +63,6 @@ end it 'returns an error' do - result = service.execute - expect(result).to be_error expect(result.message).to eq(['Error']) end diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index 8c6041524bed70..959c71e4798f13 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -8,15 +8,15 @@ let(:current_user) { user } let(:assignee_user) { create(:user) } let(:service) { described_class.new(import_source_user, assignee_user, current_user: current_user) } + let(:result) { service.execute } describe '#execute' do context 'when reassignment is successful' do it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) - expect(service) - .to receive(:track_internal_event) + expect { result } + .to trigger_internal_events('propose_placeholder_user_reassignment') .with( - 'propose_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -25,8 +25,6 @@ } ) - result = service.execute - expect(result).to be_success expect(result.payload.reload).to eq(import_source_user) expect(result.payload.reassign_to_user).to eq(assignee_user) @@ -39,8 +37,6 @@ it "returns #{desc} error" do expect(Notify).not_to receive(:import_source_user_reassign) - result = service.execute - expect(result).to be_error expect(result.message).to eq(error) end @@ -116,11 +112,18 @@ it 'assigns the user' do expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) - expect(service) - .to receive(:track_reassignment_event) - .with('propose_placeholder_user_reassignment') + expect { result } + .to trigger_internal_events('propose_placeholder_user_reassignment') + .with( + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: assignee_user.id.to_s + } + ) - expect(service.execute).to be_success + expect(result).to be_success end end end @@ -130,17 +133,21 @@ allow(import_source_user).to receive(:reassign).and_return(false) allow(import_source_user).to receive(:errors).and_return(instance_double(ActiveModel::Errors, full_messages: ['Error'])) - allow(service).to receive(:track_internal_event) end it_behaves_like 'an error response', 'active record', error: ['Error'] it 'tracks a reassignment event' do - expect(service) - .to receive(:track_reassignment_event) - .with('fail_placeholder_reassignment') - - service.execute + expect { result } + .to trigger_internal_events('fail_placeholder_user_reassignment') + .with( + namespace: import_source_user.namespace, + user: current_user, + additional_properties: { + label: import_source_user.placeholder_user_id.to_s, + property: assignee_user.id.to_s + } + ) end end end diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index 1600ed9f294afd..238a43703906f6 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -10,21 +10,21 @@ described_class.new(import_source_user, current_user: current_user, reassignment_token: reassignment_token) end + let(:result) { service.execute } + describe '#execute' do let(:message_delivery) { instance_double(ActionMailer::MessageDelivery) } before do allow(message_delivery).to receive(:deliver_now) allow(Notify).to receive(:import_source_user_rejected).and_return(message_delivery) - allow(service).to receive(:track_internal_event) end it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_rejected, :deliver_now) - expect(service) - .to receive(:track_internal_event) + expect { result } + .to trigger_internal_events('reject_placeholder_user_reassignment') .with( - 'reject_placeholder_user_reassignment', namespace: import_source_user.namespace, user: current_user, additional_properties: { @@ -32,7 +32,7 @@ property: import_source_user.reassign_to_user_id.to_s } ) - expect(service.execute).to be_success + expect(result).to be_success end it 'sets the source user to rejected' do @@ -43,7 +43,6 @@ shared_examples 'current user does not have permission to reject reassignment' do it 'returns error no permissions' do result = service.execute - expect(Notify).not_to receive(:import_source_user_rejected) expect(result).to be_error @@ -74,7 +73,6 @@ it 'returns error invalid status' do result = service.execute - expect(result).to be_error expect(result.message).to eq("Import source user has an invalid status for this operation") end @@ -89,7 +87,6 @@ it 'returns an error' do result = service.execute - expect(result).to be_error expect(result.message).to eq(['Error']) end -- GitLab From ffe196b6ac75769031b631718bb483e46af1b8e7 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Wed, 5 Mar 2025 10:02:23 +1000 Subject: [PATCH 06/11] User satisfy to match user id --- spec/lib/gitlab/import/placeholder_user_creator_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index d57cb8fca31b2e..25fed4ebee4d5b 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -61,7 +61,7 @@ .with( namespace: namespace, additional_properties: { - label: User.maximum(:id).next.to_s + label: satisfy { |value| value == User.last.id.to_s } } ) -- GitLab From 8dfa1520b66cd390860b8772a469745c67d4ef95 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Wed, 5 Mar 2025 10:30:59 +1000 Subject: [PATCH 07/11] Use global annoymous ids for labels and properties --- .../reassign_placeholder_user_records_service.rb | 8 ++++---- app/services/import/source_users/base_service.rb | 4 ++-- .../source_users/cancel_reassignment_service.rb | 8 ++++---- .../source_users/keep_as_placeholder_service.rb | 2 +- lib/gitlab/import/placeholder_user_creator.rb | 2 +- .../gitlab/import/placeholder_user_creator_spec.rb | 2 +- ...reassign_placeholder_user_records_service_spec.rb | 4 ++-- .../source_users/accept_reassignment_service_spec.rb | 8 ++++---- .../source_users/cancel_reassignment_service_spec.rb | 4 ++-- .../source_users/keep_as_placeholder_service_spec.rb | 2 +- .../import/source_users/reassign_service_spec.rb | 12 ++++++------ .../source_users/reject_reassignment_service_spec.rb | 4 ++-- 12 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb index c536e54d9bf9f7..b47d77a7d171cc 100644 --- a/app/services/import/reassign_placeholder_user_records_service.rb +++ b/app/services/import/reassign_placeholder_user_records_service.rb @@ -21,7 +21,7 @@ class ReassignPlaceholderUserRecordsService def initialize(import_source_user) @import_source_user = import_source_user @reassigned_by_user = import_source_user.reassigned_by_user - @placeholder_user_id = import_source_user.placeholder_user_id + @placeholder_user = import_source_user.placeholder_user @unavailable_tables = [] @project_membership_created = false end @@ -61,7 +61,7 @@ def execute private - attr_accessor :import_source_user, :reassigned_by_user, :unavailable_tables, :placeholder_user_id + attr_accessor :import_source_user, :reassigned_by_user, :unavailable_tables, :placeholder_user def warn_about_any_risky_reassignments warn_about_reassign_to_admin if import_source_user.reassign_to_user.admin? # rubocop:disable Cop/UserAdmin -- Not authentication related @@ -297,8 +297,8 @@ def track_reassignment_complete 'complete_placeholder_user_reassignment', namespace: import_source_user.namespace, additional_properties: { - label: placeholder_user_id.to_s, - property: import_source_user.reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) } ) end diff --git a/app/services/import/source_users/base_service.rb b/app/services/import/source_users/base_service.rb index 01502c18c52ccd..8a2a735f172aee 100644 --- a/app/services/import/source_users/base_service.rb +++ b/app/services/import/source_users/base_service.rb @@ -34,8 +34,8 @@ def track_reassignment_event(event_name) user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: import_source_user.reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) } ) end diff --git a/app/services/import/source_users/cancel_reassignment_service.rb b/app/services/import/source_users/cancel_reassignment_service.rb index 649a6f603fc744..a679bfe28b82e4 100644 --- a/app/services/import/source_users/cancel_reassignment_service.rb +++ b/app/services/import/source_users/cancel_reassignment_service.rb @@ -3,11 +3,11 @@ module Import module SourceUsers class CancelReassignmentService < BaseService - attr_reader :reassign_to_user_id + attr_reader :reassign_to_user def initialize(import_source_user, current_user:) @import_source_user = import_source_user - @reassign_to_user_id = import_source_user.reassign_to_user_id + @reassign_to_user = import_source_user.reassign_to_user @current_user = current_user end @@ -50,8 +50,8 @@ def track_reassignment_cancellation namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(reassign_to_user) } ) end diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index a24c53867d24c1..d5d82ae0228813 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -35,7 +35,7 @@ def track_keep_as_placeholder user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: import_source_user.placeholder_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user) } ) end diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index c78f7dd6e9a23e..828cd8cf401601 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -97,7 +97,7 @@ def track_placeholder_user_creation(source_user, user) 'create_placeholder_user', namespace: source_user.namespace, additional_properties: { - label: user.id.to_s + label: Gitlab::GlobalAnonymousId.user_id(user) } ) end diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index 25fed4ebee4d5b..bbd213d8dcef37 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -61,7 +61,7 @@ .with( namespace: namespace, additional_properties: { - label: satisfy { |value| value == User.last.id.to_s } + label: satisfy { |value| value == Gitlab::GlobalAnonymousId.user_id(User.last) } } ) diff --git a/spec/services/import/reassign_placeholder_user_records_service_spec.rb b/spec/services/import/reassign_placeholder_user_records_service_spec.rb index d58262dbb54f3f..3cb2e90d1d6e1e 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -131,8 +131,8 @@ .with( namespace: namespace, additional_properties: { - label: source_user.placeholder_user_id.to_s, - property: source_user.reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(source_user.reassign_to_user) } ) diff --git a/spec/services/import/source_users/accept_reassignment_service_spec.rb b/spec/services/import/source_users/accept_reassignment_service_spec.rb index bb0da72226c2f0..8a20457c1783b4 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -34,8 +34,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: import_source_user.reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) } ) end @@ -91,8 +91,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: import_source_user.reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) } ) diff --git a/spec/services/import/source_users/cancel_reassignment_service_spec.rb b/spec/services/import/source_users/cancel_reassignment_service_spec.rb index 6e3f784c513456..ba59a5e80326c5 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -23,8 +23,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: user.id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(user) } ) diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index ad7b642fe28b64..57d827d0bc7530 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -22,7 +22,7 @@ user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: import_source_user.placeholder_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user) } ) diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index 959c71e4798f13..d912ac45375403 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -20,8 +20,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: assignee_user.id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(assignee_user) } ) @@ -118,8 +118,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: assignee_user.id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(assignee_user) } ) @@ -144,8 +144,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: assignee_user.id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(assignee_user) } ) end diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index 238a43703906f6..2a9fe37da34c74 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -28,8 +28,8 @@ namespace: import_source_user.namespace, user: current_user, additional_properties: { - label: import_source_user.placeholder_user_id.to_s, - property: import_source_user.reassign_to_user_id.to_s + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) } ) expect(result).to be_success -- GitLab From 7a635b9da084cdcd609f24f4de122f324430aea0 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Thu, 6 Mar 2025 12:02:42 +1000 Subject: [PATCH 08/11] Add refactor suggestions --- ...reassign_placeholder_user_records_service.rb | 5 ++--- .../import/source_users/base_service.rb | 4 ++-- .../source_users/cancel_reassignment_service.rb | 17 ++++------------- lib/gitlab/import/placeholder_user_creator.rb | 4 ++-- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb index b47d77a7d171cc..b4ee1d33b76ccc 100644 --- a/app/services/import/reassign_placeholder_user_records_service.rb +++ b/app/services/import/reassign_placeholder_user_records_service.rb @@ -21,7 +21,6 @@ class ReassignPlaceholderUserRecordsService def initialize(import_source_user) @import_source_user = import_source_user @reassigned_by_user = import_source_user.reassigned_by_user - @placeholder_user = import_source_user.placeholder_user @unavailable_tables = [] @project_membership_created = false end @@ -61,7 +60,7 @@ def execute private - attr_accessor :import_source_user, :reassigned_by_user, :unavailable_tables, :placeholder_user + attr_accessor :import_source_user, :reassigned_by_user, :unavailable_tables def warn_about_any_risky_reassignments warn_about_reassign_to_admin if import_source_user.reassign_to_user.admin? # rubocop:disable Cop/UserAdmin -- Not authentication related @@ -297,7 +296,7 @@ def track_reassignment_complete 'complete_placeholder_user_reassignment', namespace: import_source_user.namespace, additional_properties: { - label: Gitlab::GlobalAnonymousId.user_id(placeholder_user), + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) } ) diff --git a/app/services/import/source_users/base_service.rb b/app/services/import/source_users/base_service.rb index 8a2a735f172aee..3ae4be121035a2 100644 --- a/app/services/import/source_users/base_service.rb +++ b/app/services/import/source_users/base_service.rb @@ -28,14 +28,14 @@ def send_user_reassign_email Notify.import_source_user_reassign(import_source_user.id).deliver_later end - def track_reassignment_event(event_name) + def track_reassignment_event(event_name, reassign_to_user: import_source_user.reassign_to_user) track_internal_event( event_name, user: current_user, namespace: import_source_user.namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(reassign_to_user) } ) end diff --git a/app/services/import/source_users/cancel_reassignment_service.rb b/app/services/import/source_users/cancel_reassignment_service.rb index a679bfe28b82e4..997508d8a73d9c 100644 --- a/app/services/import/source_users/cancel_reassignment_service.rb +++ b/app/services/import/source_users/cancel_reassignment_service.rb @@ -28,7 +28,10 @@ def execute return error_invalid_status if invalid_status if cancel_successful - track_reassignment_cancellation + track_reassignment_event( + 'cancel_placeholder_user_reassignment', + reassign_to_user: reassign_to_user + ) ServiceResponse.success(payload: import_source_user) else @@ -43,18 +46,6 @@ def cancel_reassignment import_source_user.reassigned_by_user = nil import_source_user.cancel_reassignment end - - def track_reassignment_cancellation - track_internal_event( - 'cancel_placeholder_user_reassignment', - namespace: import_source_user.namespace, - user: current_user, - additional_properties: { - label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(reassign_to_user) - } - ) - end end end end diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index 828cd8cf401601..e15c97e8d43c3b 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -40,7 +40,7 @@ def execute user.save! log_placeholder_user_creation(user) - track_placeholder_user_creation(source_user, user) + track_placeholder_user_creation(user) user end @@ -92,7 +92,7 @@ def log_placeholder_user_creation(user) ) end - def track_placeholder_user_creation(source_user, user) + def track_placeholder_user_creation(user) track_internal_event( 'create_placeholder_user', namespace: source_user.namespace, -- GitLab From 2a93527cf777ecf8f1f1779eb10045ed80146ecc Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Thu, 6 Mar 2025 12:46:59 +1000 Subject: [PATCH 09/11] Add import_type to reassignment tracking events --- .../import/reassign_placeholder_user_records_service.rb | 3 ++- app/services/import/source_users/base_service.rb | 3 ++- .../import/source_users/keep_as_placeholder_service.rb | 3 ++- config/events/accept_placeholder_user_reassignment.yml | 2 ++ config/events/cancel_placeholder_user_reassignment.yml | 2 ++ config/events/complete_placeholder_user_reassignment.yml | 2 ++ config/events/create_placeholder_user.yml | 2 ++ config/events/fail_placeholder_user_reassignment.yml | 2 ++ config/events/keep_as_placeholder_user.yml | 2 ++ config/events/propose_placeholder_user_reassignment.yml | 2 ++ config/events/reject_placeholder_user_reassignment.yml | 2 ++ lib/gitlab/import/placeholder_user_creator.rb | 3 ++- spec/lib/gitlab/import/placeholder_user_creator_spec.rb | 3 ++- .../reassign_placeholder_user_records_service_spec.rb | 3 ++- .../source_users/accept_reassignment_service_spec.rb | 6 ++++-- .../source_users/cancel_reassignment_service_spec.rb | 3 ++- .../source_users/keep_as_placeholder_service_spec.rb | 3 ++- .../import/source_users/reassign_service_spec.rb | 9 ++++++--- .../source_users/reject_reassignment_service_spec.rb | 3 ++- 19 files changed, 44 insertions(+), 14 deletions(-) diff --git a/app/services/import/reassign_placeholder_user_records_service.rb b/app/services/import/reassign_placeholder_user_records_service.rb index b4ee1d33b76ccc..d0cdebb52adbdf 100644 --- a/app/services/import/reassign_placeholder_user_records_service.rb +++ b/app/services/import/reassign_placeholder_user_records_service.rb @@ -297,7 +297,8 @@ def track_reassignment_complete namespace: import_source_user.namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user), + import_type: import_source_user.import_type } ) end diff --git a/app/services/import/source_users/base_service.rb b/app/services/import/source_users/base_service.rb index 3ae4be121035a2..4dbde352bd666d 100644 --- a/app/services/import/source_users/base_service.rb +++ b/app/services/import/source_users/base_service.rb @@ -35,7 +35,8 @@ def track_reassignment_event(event_name, reassign_to_user: import_source_user.re namespace: import_source_user.namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(reassign_to_user), + import_type: import_source_user.import_type } ) end diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index d5d82ae0228813..18413f104d5558 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -35,7 +35,8 @@ def track_keep_as_placeholder user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user) + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: import_source_user.import_type } ) end diff --git a/config/events/accept_placeholder_user_reassignment.yml b/config/events/accept_placeholder_user_reassignment.yml index fc049e5c2037c9..2377654922c362 100644 --- a/config/events/accept_placeholder_user_reassignment.yml +++ b/config/events/accept_placeholder_user_reassignment.yml @@ -10,6 +10,8 @@ additional_properties: description: id of the placeholder user property: description: id of the user that is replacing the placeholder + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/cancel_placeholder_user_reassignment.yml b/config/events/cancel_placeholder_user_reassignment.yml index ef61aeb8c632a0..8e31a044289fa0 100644 --- a/config/events/cancel_placeholder_user_reassignment.yml +++ b/config/events/cancel_placeholder_user_reassignment.yml @@ -10,6 +10,8 @@ additional_properties: description: id of the placeholder user property: description: id of the user that was to replace the placeholder + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/complete_placeholder_user_reassignment.yml b/config/events/complete_placeholder_user_reassignment.yml index d38441be388af9..5894945935a9dc 100644 --- a/config/events/complete_placeholder_user_reassignment.yml +++ b/config/events/complete_placeholder_user_reassignment.yml @@ -9,6 +9,8 @@ additional_properties: description: id of the placeholder user property: description: id of the user that replaced the placeholder + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/create_placeholder_user.yml b/config/events/create_placeholder_user.yml index cec76a84e1a682..62abdc86a85065 100644 --- a/config/events/create_placeholder_user.yml +++ b/config/events/create_placeholder_user.yml @@ -7,6 +7,8 @@ identifiers: additional_properties: label: description: id of the placeholder user + property: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/fail_placeholder_user_reassignment.yml b/config/events/fail_placeholder_user_reassignment.yml index 7253eeac9c4220..75da6a52680526 100644 --- a/config/events/fail_placeholder_user_reassignment.yml +++ b/config/events/fail_placeholder_user_reassignment.yml @@ -10,6 +10,8 @@ additional_properties: description: id of the placeholder user property: description: id of the user that did not replace the placeholder + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/keep_as_placeholder_user.yml b/config/events/keep_as_placeholder_user.yml index 9a6877d78cac44..66de138fbebb63 100644 --- a/config/events/keep_as_placeholder_user.yml +++ b/config/events/keep_as_placeholder_user.yml @@ -8,6 +8,8 @@ identifiers: additional_properties: label: description: id of the placeholder user + property: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/propose_placeholder_user_reassignment.yml b/config/events/propose_placeholder_user_reassignment.yml index 2017f394e657c8..81d14803607f4e 100644 --- a/config/events/propose_placeholder_user_reassignment.yml +++ b/config/events/propose_placeholder_user_reassignment.yml @@ -10,6 +10,8 @@ additional_properties: description: id of the placeholder user property: description: id of the user that is replacing the placeholder + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: - importers diff --git a/config/events/reject_placeholder_user_reassignment.yml b/config/events/reject_placeholder_user_reassignment.yml index a3ebb6d4a902ec..164da7dc5aab2d 100644 --- a/config/events/reject_placeholder_user_reassignment.yml +++ b/config/events/reject_placeholder_user_reassignment.yml @@ -10,6 +10,8 @@ additional_properties: description: id of the placeholder user property: description: id of the user that was to replace the placeholder + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket, gitea product_group: import_and_integrate product_categories: - importers diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index e15c97e8d43c3b..a0421fcce6582b 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -97,7 +97,8 @@ def track_placeholder_user_creation(user) 'create_placeholder_user', namespace: source_user.namespace, additional_properties: { - label: Gitlab::GlobalAnonymousId.user_id(user) + label: Gitlab::GlobalAnonymousId.user_id(user), + property: source_user.import_type } ) end diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index bbd213d8dcef37..870dd3054e17aa 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -61,7 +61,8 @@ .with( namespace: namespace, additional_properties: { - label: satisfy { |value| value == Gitlab::GlobalAnonymousId.user_id(User.last) } + label: satisfy { |value| value == Gitlab::GlobalAnonymousId.user_id(User.last) }, + property: source_user.import_type } ) diff --git a/spec/services/import/reassign_placeholder_user_records_service_spec.rb b/spec/services/import/reassign_placeholder_user_records_service_spec.rb index 3cb2e90d1d6e1e..740ffc707573c4 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -132,7 +132,8 @@ namespace: namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(source_user.reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(source_user.reassign_to_user), + import_type: source_user.import_type } ) diff --git a/spec/services/import/source_users/accept_reassignment_service_spec.rb b/spec/services/import/source_users/accept_reassignment_service_spec.rb index 8a20457c1783b4..764f5aa433e3db 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -35,7 +35,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user), + import_type: import_source_user.import_type } ) end @@ -92,7 +93,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user), + import_type: import_source_user.import_type } ) diff --git a/spec/services/import/source_users/cancel_reassignment_service_spec.rb b/spec/services/import/source_users/cancel_reassignment_service_spec.rb index ba59a5e80326c5..7a3dd82ef50849 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -24,7 +24,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(user) + property: Gitlab::GlobalAnonymousId.user_id(user), + import_type: import_source_user.import_type } ) diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 57d827d0bc7530..07ce3640e0725d 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -22,7 +22,8 @@ user: current_user, namespace: import_source_user.namespace, additional_properties: { - label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user) + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: import_source_user.import_type } ) diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index d912ac45375403..1395825e7d8072 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -21,7 +21,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(assignee_user) + property: Gitlab::GlobalAnonymousId.user_id(assignee_user), + import_type: import_source_user.import_type } ) @@ -119,7 +120,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(assignee_user) + property: Gitlab::GlobalAnonymousId.user_id(assignee_user), + import_type: import_source_user.import_type } ) @@ -145,7 +147,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(assignee_user) + property: Gitlab::GlobalAnonymousId.user_id(assignee_user), + import_type: import_source_user.import_type } ) end diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index 2a9fe37da34c74..eede1983f6bca3 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -29,7 +29,8 @@ user: current_user, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user) + property: Gitlab::GlobalAnonymousId.user_id(import_source_user.reassign_to_user), + import_type: import_source_user.import_type } ) expect(result).to be_success -- GitLab From f5aaeed23da0e23760a5b45b595950e061551f30 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Thu, 6 Mar 2025 20:02:23 +1000 Subject: [PATCH 10/11] Tidy up specs --- .../import/source_users/keep_as_placeholder_service_spec.rb | 4 ---- .../import/source_users/reject_reassignment_service_spec.rb | 3 --- 2 files changed, 7 deletions(-) diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 07ce3640e0725d..7f555e499dba71 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -11,10 +11,6 @@ describe '#execute' do context 'when reassignment is successful' do - before do - allow(service).to receive(:track_reassignment_event) - end - it 'returns success' do expect { result } .to trigger_internal_events('keep_as_placeholder_user') diff --git a/spec/services/import/source_users/reject_reassignment_service_spec.rb b/spec/services/import/source_users/reject_reassignment_service_spec.rb index eede1983f6bca3..7828c6dab310a4 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -43,7 +43,6 @@ shared_examples 'current user does not have permission to reject reassignment' do it 'returns error no permissions' do - result = service.execute expect(Notify).not_to receive(:import_source_user_rejected) expect(result).to be_error @@ -73,7 +72,6 @@ let(:import_source_user) { create(:import_source_user, :reassignment_in_progress) } it 'returns error invalid status' do - result = service.execute expect(result).to be_error expect(result.message).to eq("Import source user has an invalid status for this operation") end @@ -87,7 +85,6 @@ end it 'returns an error' do - result = service.execute expect(result).to be_error expect(result.message).to eq(['Error']) end -- GitLab From a23c383207fec49e1405cc41b79e1065f284eab4 Mon Sep 17 00:00:00 2001 From: carlad-gl Date: Mon, 10 Mar 2025 09:29:25 +1000 Subject: [PATCH 11/11] Add nil property --- .../import/source_users/keep_as_placeholder_service.rb | 3 ++- config/events/create_placeholder_user.yml | 2 ++ config/events/keep_as_placeholder_user.yml | 2 ++ lib/gitlab/import/placeholder_user_creator.rb | 3 ++- spec/lib/gitlab/import/placeholder_user_creator_spec.rb | 3 ++- .../import/source_users/keep_as_placeholder_service_spec.rb | 3 ++- 6 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index 18413f104d5558..9a3a03ca81436f 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -36,7 +36,8 @@ def track_keep_as_placeholder namespace: import_source_user.namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: import_source_user.import_type + property: nil, + import_type: import_source_user.import_type } ) end diff --git a/config/events/create_placeholder_user.yml b/config/events/create_placeholder_user.yml index 62abdc86a85065..d7731dd67d807a 100644 --- a/config/events/create_placeholder_user.yml +++ b/config/events/create_placeholder_user.yml @@ -8,6 +8,8 @@ additional_properties: label: description: id of the placeholder user property: + description: nil (for related placeholder events, this is the reassigned user id) + import_type: description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: diff --git a/config/events/keep_as_placeholder_user.yml b/config/events/keep_as_placeholder_user.yml index 66de138fbebb63..688b7722480b3f 100644 --- a/config/events/keep_as_placeholder_user.yml +++ b/config/events/keep_as_placeholder_user.yml @@ -9,6 +9,8 @@ additional_properties: label: description: id of the placeholder user property: + description: nil (for related placeholder events, this is the reassigned user id) + import_type: description: the import source e.g. gitlab_migration, github, bitbucket product_group: import_and_integrate product_categories: diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb index a0421fcce6582b..040edf7f5d9fb1 100644 --- a/lib/gitlab/import/placeholder_user_creator.rb +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -98,7 +98,8 @@ def track_placeholder_user_creation(user) namespace: source_user.namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(user), - property: source_user.import_type + property: nil, + import_type: source_user.import_type } ) end diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb index 870dd3054e17aa..11a4b5e3c55e8f 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -62,7 +62,8 @@ namespace: namespace, additional_properties: { label: satisfy { |value| value == Gitlab::GlobalAnonymousId.user_id(User.last) }, - property: source_user.import_type + property: nil, + import_type: source_user.import_type } ) diff --git a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb index 7f555e499dba71..9799815e2f8bae 100644 --- a/spec/services/import/source_users/keep_as_placeholder_service_spec.rb +++ b/spec/services/import/source_users/keep_as_placeholder_service_spec.rb @@ -19,7 +19,8 @@ namespace: import_source_user.namespace, additional_properties: { label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), - property: import_source_user.import_type + property: nil, + import_type: import_source_user.import_type } ) -- GitLab