From f5cf243b875c708a1fbd0b2299a9ffdcc5f96915 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 30 Jan 2025 09:14:10 -0500 Subject: [PATCH] Log errors for profile reactivity password submitted changelog: Internal, Analytics, Log errors for profile reactivity password submitted --- .../users/verify_password_controller.rb | 11 ++++---- app/forms/verify_password_form.rb | 7 +++-- app/services/analytics_events.rb | 5 ++-- .../users/verify_password_controller_spec.rb | 26 +++++++++---------- spec/forms/verify_password_form_spec.rb | 8 +++--- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/app/controllers/users/verify_password_controller.rb b/app/controllers/users/verify_password_controller.rb index e592d396eab..6682403bb98 100644 --- a/app/controllers/users/verify_password_controller.rb +++ b/app/controllers/users/verify_password_controller.rb @@ -13,12 +13,13 @@ def new end def update - result = verify_password_form.submit + form = verify_password_form + result = form.submit - analytics.reactivate_account_verify_password_submitted(success: result.success?) + analytics.reactivate_account_verify_password_submitted(**result.to_h) if result.success? - handle_success(result) + handle_success(personal_key: form.personal_key) else flash[:error] = t('errors.messages.password_incorrect') render :new @@ -32,8 +33,8 @@ def confirm_personal_key redirect_to root_url end - def handle_success(result) - user_session[:personal_key] = result.extra[:personal_key] + def handle_success(personal_key:) + user_session[:personal_key] = personal_key reactivate_account_session.clear redirect_to manage_personal_key_url end diff --git a/app/forms/verify_password_form.rb b/app/forms/verify_password_form.rb index 1f9bc897523..0daa72f401e 100644 --- a/app/forms/verify_password_form.rb +++ b/app/forms/verify_password_form.rb @@ -6,7 +6,7 @@ class VerifyPasswordForm validates :password, presence: true validate :validate_password - attr_reader :user, :password, :decrypted_pii + attr_reader :user, :password, :decrypted_pii, :personal_key def initialize(user:, password:, decrypted_pii:) @user = user @@ -16,11 +16,10 @@ def initialize(user:, password:, decrypted_pii:) def submit success = valid? - extra = {} - extra[:personal_key] = reencrypt_pii if success + @personal_key = reencrypt_pii if success - FormResponse.new(success: success, errors: errors, extra: extra) + FormResponse.new(success:, errors:, serialize_error_details_only: true) end private diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 5ebdab10b29..69d768536da 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -6689,8 +6689,9 @@ def reactivate_account_submit # Submission event for the "verify password" page the user sees after entering their personal key. # @param [Boolean] success Whether the form was submitted successfully. - def reactivate_account_verify_password_submitted(success:, **extra) - track_event(:reactivate_account_verify_password_submitted, success: success, **extra) + # @param [Hash] error_details Details for errors that occurred in unsuccessful submission + def reactivate_account_verify_password_submitted(success:, error_details: nil, **extra) + track_event(:reactivate_account_verify_password_submitted, success:, error_details:, **extra) end # Visit event for the "verify password" page the user sees after entering their personal key. diff --git a/spec/controllers/users/verify_password_controller_spec.rb b/spec/controllers/users/verify_password_controller_spec.rb index 4453f125749..e2ef3ca837b 100644 --- a/spec/controllers/users/verify_password_controller_spec.rb +++ b/spec/controllers/users/verify_password_controller_spec.rb @@ -60,18 +60,19 @@ end describe '#update' do - let(:form) { instance_double(VerifyPasswordForm) } - let(:user_params) { { user: { password: user.password } } } - - before do - expect(controller).to receive(:verify_password_form).and_return(form) - end + let(:password) { nil } + let(:user_params) { { user: { password: } } } context 'with valid password' do - let(:response_ok) { FormResponse.new(success: true, errors: {}, extra: recovery_hash) } + let(:password) { user.password } before do - allow(form).to receive(:submit).and_return(response_ok) + pii = Pii::Attributes.new_from_hash(Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN) + ReactivateAccountSession.new( + user:, + user_session: controller.user_session, + ).store_decrypted_pii(pii) + put :update, params: user_params end @@ -86,19 +87,17 @@ expect(response).to redirect_to(manage_personal_key_url) end - it 'sets a new personal key as a flash message' do - expect(controller.user_session[:personal_key]).to eq(key) + it 'sets the new personal key as a user session value' do + expect(controller.user_session[:personal_key]).to match(/^([A-Z0-9]{4}(-|$)){4}/) end end context 'without valid password' do - let(:response_bad) { FormResponse.new(success: false, errors: {}) } + let(:password) { user.password + 'wrong' } render_views before do - allow(form).to receive(:submit).and_return(response_bad) - put :update, params: user_params end @@ -106,6 +105,7 @@ expect(@analytics).to have_logged_event( :reactivate_account_verify_password_submitted, success: false, + error_details: { password: { password_incorrect: true } }, ) end diff --git a/spec/forms/verify_password_form_spec.rb b/spec/forms/verify_password_form_spec.rb index f64973b0caf..d56ddf792a3 100644 --- a/spec/forms/verify_password_form_spec.rb +++ b/spec/forms/verify_password_form_spec.rb @@ -19,7 +19,7 @@ result = form.submit expect(profile.reload.active?).to eq true - expect(result.success?).to eq true + expect(result.to_h).to eq(success: true) end end @@ -40,8 +40,10 @@ result = form.submit expect(profile.reload.active?).to eq false - expect(result.success?).to eq false - expect(result.errors[:password]).to eq [t('errors.messages.password_incorrect')] + expect(result.to_h).to eq( + success: false, + error_details: { password: { password_incorrect: true } }, + ) end end end