From 0244f146f8fb5794a545685d391afc86ce68d59a Mon Sep 17 00:00:00 2001 From: Carla Drago <cdrago@gitlab.com> Date: Wed, 12 Mar 2025 08:44:44 +1000 Subject: [PATCH] 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 | 16 +++++++ .../accept_reassignment_service.rb | 3 ++ .../import/source_users/base_service.rb | 15 +++++++ .../cancel_reassignment_service.rb | 8 ++++ .../keep_all_as_placeholder_service.rb | 2 + .../keep_as_placeholder_service.rb | 15 +++++++ .../import/source_users/reassign_service.rb | 2 + .../reject_reassignment_service.rb | 1 + .../accept_placeholder_user_reassignment.yml | 23 ++++++++++ .../cancel_placeholder_user_reassignment.yml | 23 ++++++++++ ...complete_placeholder_user_reassignment.yml | 22 +++++++++ config/events/create_placeholder_user.yml | 22 +++++++++ .../fail_placeholder_user_reassignment.yml | 23 ++++++++++ .../events/keep_all_as_placeholder_user.yml | 16 +++++++ config/events/keep_as_placeholder_user.yml | 23 ++++++++++ .../propose_placeholder_user_reassignment.yml | 23 ++++++++++ .../reject_placeholder_user_reassignment.yml | 23 ++++++++++ lib/gitlab/import/placeholder_user_creator.rb | 15 +++++++ .../import/placeholder_user_creator_spec.rb | 13 +++++- ...n_placeholder_user_records_service_spec.rb | 11 ++++- .../accept_reassignment_service_spec.rb | 28 +++++++++++- .../cancel_reassignment_service_spec.rb | 19 +++++--- .../keep_all_as_placeholder_service_spec.rb | 5 ++- .../keep_as_placeholder_service_spec.rb | 18 +++++--- .../source_users/reassign_service_spec.rb | 45 ++++++++++++++++--- .../reject_reassignment_service_spec.rb | 21 ++++++--- 26 files changed, 402 insertions(+), 33 deletions(-) create mode 100644 config/events/accept_placeholder_user_reassignment.yml create mode 100644 config/events/cancel_placeholder_user_reassignment.yml create mode 100644 config/events/complete_placeholder_user_reassignment.yml create mode 100644 config/events/create_placeholder_user.yml create mode 100644 config/events/fail_placeholder_user_reassignment.yml create mode 100644 config/events/keep_all_as_placeholder_user.yml create mode 100644 config/events/keep_as_placeholder_user.yml create mode 100644 config/events/propose_placeholder_user_reassignment.yml create mode 100644 config/events/reject_placeholder_user_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 15ff07678ca8..d0cdebb52adb 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 @@ -48,6 +50,8 @@ def execute import_source_user.complete! + track_reassignment_complete + ServiceResponse.success( message: s_('Import|Placeholder user record reassignment complete'), payload: import_source_user @@ -287,6 +291,18 @@ def log_create_membership_skipped(message, placeholder_membership, existing_memb ) end + def track_reassignment_complete + track_internal_event( + 'complete_placeholder_user_reassignment', + 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), + import_type: import_source_user.import_type + } + ) + 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 b6c8810f9a10..99833cdb01f2 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('accept_placeholder_user_reassignment') + ServiceResponse.success(payload: import_source_user) else + 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/base_service.rb b/app/services/import/source_users/base_service.rb index f2f880da32f1..4dbde352bd66 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, 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(reassign_to_user), + import_type: import_source_user.import_type + } + ) + 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 b3ee14fee7db..997508d8a73d 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 + def initialize(import_source_user, current_user:) @import_source_user = import_source_user + @reassign_to_user = import_source_user.reassign_to_user @current_user = current_user end @@ -25,6 +28,11 @@ def execute return error_invalid_status if invalid_status if cancel_successful + track_reassignment_event( + 'cancel_placeholder_user_reassignment', + reassign_to_user: reassign_to_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 8a377097001c..f2f08fcc184f 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_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 3773dc622c48..9a3a03ca8143 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_keep_as_placeholder + ServiceResponse.success(payload: import_source_user) else ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) @@ -26,6 +28,19 @@ 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', + user: current_user, + namespace: import_source_user.namespace, + additional_properties: { + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: nil, + import_type: import_source_user.import_type + } + ) + 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 597746cec0ae..6779359fcbc5 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('propose_placeholder_user_reassignment') ServiceResponse.success(payload: import_source_user) else + 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/reject_reassignment_service.rb b/app/services/import/source_users/reject_reassignment_service.rb index 212354bb7fea..abb0113ab555 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('reject_placeholder_user_reassignment') ServiceResponse.success(payload: import_source_user) else diff --git a/config/events/accept_placeholder_user_reassignment.yml b/config/events/accept_placeholder_user_reassignment.yml new file mode 100644 index 000000000000..2377654922c3 --- /dev/null +++ b/config/events/accept_placeholder_user_reassignment.yml @@ -0,0 +1,23 @@ +--- +description: Tracks when a placeholder user reassignment is accepted +internal_events: true +action: accept_placeholder_user_reassignment +identifiers: +- namespace +- user +additional_properties: + label: + 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 +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/cancel_placeholder_user_reassignment.yml b/config/events/cancel_placeholder_user_reassignment.yml new file mode 100644 index 000000000000..8e31a044289f --- /dev/null +++ b/config/events/cancel_placeholder_user_reassignment.yml @@ -0,0 +1,23 @@ +--- +description: Tracks when a placeholder user reassignment is canceled +internal_events: true +action: cancel_placeholder_user_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 + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket +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/complete_placeholder_user_reassignment.yml b/config/events/complete_placeholder_user_reassignment.yml new file mode 100644 index 000000000000..5894945935a9 --- /dev/null +++ b/config/events/complete_placeholder_user_reassignment.yml @@ -0,0 +1,22 @@ +--- +description: Tracks when a placeholder user reassignment is completed +internal_events: true +action: complete_placeholder_user_reassignment +identifiers: +- namespace +additional_properties: + label: + 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 +milestone: '17.10' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182679 +tiers: +- free +- premium +- ultimate diff --git a/config/events/create_placeholder_user.yml b/config/events/create_placeholder_user.yml new file mode 100644 index 000000000000..d7731dd67d80 --- /dev/null +++ b/config/events/create_placeholder_user.yml @@ -0,0 +1,22 @@ +--- +description: Tracks when a placeholder user is created +internal_events: true +action: create_placeholder_user +identifiers: +- namespace +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: +- 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/fail_placeholder_user_reassignment.yml b/config/events/fail_placeholder_user_reassignment.yml new file mode 100644 index 000000000000..75da6a526805 --- /dev/null +++ b/config/events/fail_placeholder_user_reassignment.yml @@ -0,0 +1,23 @@ +--- +description: Tracks when a placeholder user reassignment fails +internal_events: true +action: fail_placeholder_user_reassignment +identifiers: +- namespace +- user +additional_properties: + label: + 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 +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_all_as_placeholder_user.yml b/config/events/keep_all_as_placeholder_user.yml new file mode 100644 index 000000000000..50b494e42c22 --- /dev/null +++ b/config/events/keep_all_as_placeholder_user.yml @@ -0,0 +1,16 @@ +--- +description: Tracks when all placeholder users are kept as placeholders and not reassigned to existing users +internal_events: true +action: keep_all_as_placeholder_user +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_user.yml b/config/events/keep_as_placeholder_user.yml new file mode 100644 index 000000000000..688b7722480b --- /dev/null +++ b/config/events/keep_as_placeholder_user.yml @@ -0,0 +1,23 @@ +--- +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_user +identifiers: +- namespace +- user +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: +- 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/propose_placeholder_user_reassignment.yml b/config/events/propose_placeholder_user_reassignment.yml new file mode 100644 index 000000000000..81d14803607f --- /dev/null +++ b/config/events/propose_placeholder_user_reassignment.yml @@ -0,0 +1,23 @@ +--- +description: Tracks when a placeholder user reassignment is proposed +internal_events: true +action: propose_placeholder_user_reassignment +identifiers: +- namespace +- user +additional_properties: + label: + 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 +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_user_reassignment.yml b/config/events/reject_placeholder_user_reassignment.yml new file mode 100644 index 000000000000..164da7dc5aab --- /dev/null +++ b/config/events/reject_placeholder_user_reassignment.yml @@ -0,0 +1,23 @@ +--- +description: Tracks when a placeholder user reassignment is rejected +internal_events: true +action: reject_placeholder_user_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 + import_type: + description: the import source e.g. gitlab_migration, github, bitbucket, gitea +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 a58a78e66e0f..040edf7f5d9f 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,7 @@ def execute user.save! log_placeholder_user_creation(user) + track_placeholder_user_creation(user) user end @@ -88,6 +91,18 @@ def log_placeholder_user_creation(user) user_id: user.id ) end + + def track_placeholder_user_creation(user) + track_internal_event( + 'create_placeholder_user', + namespace: source_user.namespace, + additional_properties: { + label: Gitlab::GlobalAnonymousId.user_id(user), + property: nil, + import_type: source_user.import_type + } + ) + 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 ba40277895ca..11a4b5e3c55e 100644 --- a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -53,10 +53,19 @@ } end - it 'logs placeholder user creation' do + it 'logs and tracks placeholder user creation' do allow(::Import::Framework::Logger).to receive(:info) - service.execute + expect { service.execute } + .to trigger_internal_events('create_placeholder_user') + .with( + namespace: namespace, + additional_properties: { + label: satisfy { |value| value == Gitlab::GlobalAnonymousId.user_id(User.last) }, + property: nil, + import_type: source_user.import_type + } + ) expect(::Import::Framework::Logger).to have_received(:info).with( hash_including( 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 9d54f1e1322a..740ffc707573 100644 --- a/spec/services/import/reassign_placeholder_user_records_service_spec.rb +++ b/spec/services/import/reassign_placeholder_user_records_service_spec.rb @@ -126,7 +126,16 @@ shared_examples 'a successful reassignment' do it 'completes the reassignment' do - service.execute + expect { service.execute } + .to trigger_internal_events('complete_placeholder_user_reassignment') + .with( + namespace: namespace, + additional_properties: { + label: Gitlab::GlobalAnonymousId.user_id(source_user.placeholder_user), + property: Gitlab::GlobalAnonymousId.user_id(source_user.reassign_to_user), + import_type: source_user.import_type + } + ) expect(source_user.reload).to be_completed end 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 bb9a2b9b1c79..764f5aa433e3 100644 --- a/spec/services/import/source_users/accept_reassignment_service_spec.rb +++ b/spec/services/import/source_users/accept_reassignment_service_spec.rb @@ -27,6 +27,20 @@ service.execute end + it 'tracks the reassignment event' do + expect { service.execute } + .to trigger_internal_events('accept_placeholder_user_reassignment') + .with( + 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(import_source_user.reassign_to_user), + import_type: import_source_user.import_type + } + ) + end + shared_examples 'current user does not have permission to accept reassignment' do it 'returns error no permissions' do result = service.execute @@ -68,11 +82,21 @@ 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) - - result = service.execute + expect { result } + .to trigger_internal_events('fail_placeholder_user_reassignment') + .with( + 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(import_source_user.reassign_to_user), + import_type: import_source_user.import_type + } + ) expect(result).to be_error expect(result.message).to include('Status cannot transition via "accept"') 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 08544d1b2bf2..7a3dd82ef508 100644 --- a/spec/services/import/source_users/cancel_reassignment_service_spec.rb +++ b/spec/services/import/source_users/cancel_reassignment_service_spec.rb @@ -12,11 +12,22 @@ 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 it 'returns success' do - result = service.execute + expect { result } + .to trigger_internal_events('cancel_placeholder_user_reassignment') + .with( + 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(user), + import_type: import_source_user.import_type + } + ) expect(result).to be_success expect(result.payload.reload).to eq(import_source_user) @@ -30,8 +41,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 @@ -43,8 +52,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 @@ -58,8 +65,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 3ec41a4f5eb7..c5477249e40b 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,8 +30,9 @@ reassignable_source_users = [ source_user_pending_assignment_1, source_user_pending_assignment_2, source_user_rejected ] - - 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 a59f35761555..9799815e2f8b 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,11 +7,22 @@ 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 it 'returns success' do - result = service.execute + expect { result } + .to trigger_internal_events('keep_as_placeholder_user') + .with( + user: current_user, + namespace: import_source_user.namespace, + additional_properties: { + label: Gitlab::GlobalAnonymousId.user_id(import_source_user.placeholder_user), + property: nil, + import_type: import_source_user.import_type + } + ) expect(result).to be_success expect(result.payload.reload).to eq(import_source_user) @@ -25,8 +36,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 @@ -39,7 +48,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 @@ -53,8 +61,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 64b2c9e63636..1395825e7d80 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -8,13 +8,23 @@ 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) - - result = service.execute + expect { result } + .to trigger_internal_events('propose_placeholder_user_reassignment') + .with( + 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(assignee_user), + import_type: import_source_user.import_type + } + ) expect(result).to be_success expect(result.payload.reload).to eq(import_source_user) @@ -28,8 +38,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 @@ -105,8 +113,19 @@ it 'assigns the user' do expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_later) - - expect(service.execute).to be_success + expect { result } + .to trigger_internal_events('propose_placeholder_user_reassignment') + .with( + 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(assignee_user), + import_type: import_source_user.import_type + } + ) + + expect(result).to be_success end end end @@ -119,6 +138,20 @@ end it_behaves_like 'an error response', 'active record', error: ['Error'] + + it 'tracks a reassignment event' do + expect { result } + .to trigger_internal_events('fail_placeholder_user_reassignment') + .with( + 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(assignee_user), + import_type: import_source_user.import_type + } + ) + 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 1b8aba193980..7828c6dab310 100644 --- a/spec/services/import/source_users/reject_reassignment_service_spec.rb +++ b/spec/services/import/source_users/reject_reassignment_service_spec.rb @@ -10,6 +10,8 @@ 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) } @@ -20,7 +22,18 @@ it 'returns success' do expect(Notify).to receive_message_chain(:import_source_user_rejected, :deliver_now) - expect(service.execute).to be_success + expect { result } + .to trigger_internal_events('reject_placeholder_user_reassignment') + .with( + 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(import_source_user.reassign_to_user), + import_type: import_source_user.import_type + } + ) + expect(result).to be_success end it 'sets the source user to rejected' do @@ -30,8 +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 @@ -61,8 +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 @@ -76,8 +85,6 @@ end it 'returns an error' do - result = service.execute - expect(result).to be_error expect(result.message).to eq(['Error']) end -- GitLab