diff --git a/dashboard/app/helpers/users_helper.rb b/dashboard/app/helpers/users_helper.rb index 8eca47c4678d0..54981da4375a4 100644 --- a/dashboard/app/helpers/users_helper.rb +++ b/dashboard/app/helpers/users_helper.rb @@ -19,13 +19,18 @@ def move_sections_and_destroy_source_user(source_user:, destination_user:, takeo # No-op if source_user is nil return true unless source_user.present? + firehose_params = { + source_user: source_user, + destination_user: destination_user, + type: takeover_type, + provider: destination_user.provider, + } + if source_user.has_activity? - # We don't want to destroy an account with progress. - # In theory this should not happen, so we log a Honeybadger error and return. - Honeybadger.notify( - error_class: 'Oauth takeover called for user with progress', - errors_message: "Attempted takeover for account with progress. Cancelling takeover of account with id #{source_user.id} by id #{destination_user.id}" - ) + # We don't want to destroy an account with progress. Log to Redshift and return false. + firehose_params[:type] = "cancelled-#{takeover_type}" + firehose_params[:error] = "Attempted takeover for account with progress." + log_account_takeover_to_firehose(firehose_params) return false end @@ -40,12 +45,7 @@ def move_sections_and_destroy_source_user(source_user:, destination_user:, takeo source_user.destroy! end - log_account_takeover_to_firehose( - source_user: source_user, - destination_user: destination_user, - type: takeover_type, - provider: destination_user.provider - ) + log_account_takeover_to_firehose(firehose_params) true rescue false @@ -94,7 +94,7 @@ def check_and_apply_oauth_takeover(user) end end - def log_account_takeover_to_firehose(source_user:, destination_user:, type:, provider:) + def log_account_takeover_to_firehose(source_user:, destination_user:, type:, provider:, error: nil) FirehoseClient.instance.put_record( study: 'user-soft-delete-audit', event: "#{type}-account-takeover", # Silent or OAuth takeover @@ -103,6 +103,7 @@ def log_account_takeover_to_firehose(source_user:, destination_user:, type:, pro data_string: provider, # OAuth provider data_json: { user_type: destination_user.user_type, + error: error, }.to_json ) end diff --git a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb index 6548a8463bc8e..d207dab9e854b 100644 --- a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb +++ b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb @@ -449,7 +449,7 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase assert oauth_student.has_activity? - Honeybadger.expects(:notify).at_least_once + FirehoseClient.any_instance.expects(:put_record).at_least_once set_oauth_takeover_session_variables(provider, oauth_student) check_and_apply_oauth_takeover(student)