From b993acc07ba3c9033b1c58b7357b67a9e408d5f5 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 1 Nov 2024 10:18:10 -0400 Subject: [PATCH 01/11] changelog: Upcoming Features, Email Selection feature, Add new message for email confirmation --- .../users/email_confirmations_controller.rb | 13 +++++- config/locales/en.yml | 1 + config/locales/es.yml | 1 + config/locales/fr.yml | 1 + config/locales/zh.yml | 1 + .../email_confirmations_controller_spec.rb | 40 +++++++++++++++++++ 6 files changed, 56 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index fdd01844ad0..b5ed30436bc 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -43,7 +43,7 @@ def email_address_already_confirmed? def process_successful_confirmation(email_address) confirm_and_notify(email_address) if current_user - flash[:success] = t('devise.confirmations.confirmed') + flash[:success] = flash_message_for_successful_signed_in_confirmation redirect_to account_url else flash[:success] = t('devise.confirmations.confirmed_but_sign_in') @@ -90,6 +90,17 @@ def message_for_already_confirmed_user end end + def flash_message_for_successful_signed_in_confirmation + if IdentityConfig.store.feature_select_email_to_share_enabled + t( + 'account.emails.confirmed_html', + url: account_connected_accounts_url, + ) + else + t('devise.confirmations.confirmed') + end + end + def email_address_already_confirmed_by_current_user? user_signed_in? && email_confirmation_token_validator.email_address_already_confirmed_by_user?(current_user) diff --git a/config/locales/en.yml b/config/locales/en.yml index dfa8e89d2ab..892e172f502 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -50,6 +50,7 @@ account.email_language.name.es: Español account.email_language.name.fr: Français account.email_language.name.zh: 中文 (简体) account.email_language.updated: Your email language preference has been updated. +account.emails.confirmed_html: You have confirmed your email address. Go to your connected accounts to update the email you share with connected agencies. account.forget_all_browsers.longer_description: Once you choose to ‘forget all browsers,’ we’ll need additional information to know that it’s actually you signing in to your account. We’ll ask for a multi-factor authentication method (such as text/SMS code or a security key) each time you want to access your account. account.index.auth_app_add: Add app account.index.auth_app_disabled: not enabled diff --git a/config/locales/es.yml b/config/locales/es.yml index 87dbd353b81..e4404aecfcb 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -50,6 +50,7 @@ account.email_language.name.es: Español account.email_language.name.fr: Français account.email_language.name.zh: 中文 (简体) account.email_language.updated: Se actualizó su preferencia de idioma del correo electrónico. +account.emails.confirmed_html: Usted confirmó su dirección de correo electrónico. Vaya a Sus cuentas conectadas para actualizar el correo electrónico que proporcionó a las agencias conectadas. account.forget_all_browsers.longer_description: Una vez que elija “Olvidar todos los navegadores”, necesitaremos más información para saber que realmente es usted quien está iniciando sesión en su cuenta. Le pediremos un método de autenticación multifactor (como código de texto o de SMS, o una clave de seguridad) cada vez que desee acceder a su cuenta. account.index.auth_app_add: Agregar aplicación account.index.auth_app_disabled: no habilitada diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 7ed5a60b33b..e5c09c19612 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -50,6 +50,7 @@ account.email_language.name.es: Español account.email_language.name.fr: Français account.email_language.name.zh: 中文 (简体) account.email_language.updated: Votre langue de préférence pour les e-mails a été mise à jour. +account.emails.confirmed_html: Vous avez confirmé votre adresse e-mail. Rendez-vous sur vos comptes connectés pour actualiser l’adresse e-mail que vous communiquez aux organismes connectés. account.forget_all_browsers.longer_description: Une fois que vous aurez choisi d’« oublier tous les navigateurs », nous aurons besoin d’informations supplémentaires pour savoir que c’est bien vous qui vous connectez à votre compte. Nous vous demanderons une méthode d’authentification multi-facteurs (comme un code SMS/texto ou une clé de sécurité) chaque fois que vous souhaiterez accéder à votre compte. account.index.auth_app_add: Ajouter une appli account.index.auth_app_disabled: non activé diff --git a/config/locales/zh.yml b/config/locales/zh.yml index ce88a940dca..d4c6d20f307 100644 --- a/config/locales/zh.yml +++ b/config/locales/zh.yml @@ -50,6 +50,7 @@ account.email_language.name.es: Español account.email_language.name.fr: Français account.email_language.name.zh: 中文 (简体) account.email_language.updated: 你的电邮语言选择已更新。 +account.emails.confirmed_html: 你已确认了你的电邮地址。请到你已连接的账户来更新你与已连接机构所分享的电邮。 account.forget_all_browsers.longer_description: 你选择“忘掉所有浏览器”后，我们将需要额外信息来知道的确是你在登录你自己的账户。每次你要访问自己的账户时，我们都会向你要一个多因素身份证实方法（比如短信/SMS 代码或安全密钥） account.index.auth_app_add: 添加应用程序 account.index.auth_app_disabled: 未启用 diff --git a/spec/controllers/users/email_confirmations_controller_spec.rb b/spec/controllers/users/email_confirmations_controller_spec.rb index 8c984d65584..b7e27a0aebe 100644 --- a/spec/controllers/users/email_confirmations_controller_spec.rb +++ b/spec/controllers/users/email_confirmations_controller_spec.rb @@ -25,6 +25,46 @@ get :create, params: { confirmation_token: email_record.reload.confirmation_token } end + context ' when select email feature is enabled' do + before do + allow(IdentityConfig.store).to receive(:feature_select_email_to_share_enabled). + and_return(true) + end + + it 'renders the proper flash message for signed in user' do + flash_message = t( + 'account.emails.confirmed_html', + url: account_connected_accounts_url, + ) + user = create(:user) + sign_in user + new_email = Faker::Internet.email + + add_email_form = AddUserEmailForm.new + add_email_form.submit(user, email: new_email) + email_record = add_email_form.email_address_record(new_email) + + get :create, params: { confirmation_token: email_record.reload.confirmation_token } + expect(flash[:success]).to eq(flash_message) + end + end + + context 'when select email feature is disabled' do + it 'should render proper flash member' do + flash_message = t('devise.confirmations.confirmed') + user = create(:user) + sign_in user + new_email = Faker::Internet.email + + add_email_form = AddUserEmailForm.new + add_email_form.submit(user, email: new_email) + email_record = add_email_form.email_address_record(new_email) + + get :create, params: { confirmation_token: email_record.reload.confirmation_token } + expect(flash[:success]).to eq(flash_message) + end + end + it 'rejects an otherwise valid token for unconfirmed users' do user = create(:user, :unconfirmed, email_addresses: []) new_email = Faker::Internet.email From 9cf8f51492f9c9186c413dd1a36f82ffc74093a0 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Fri, 1 Nov 2024 10:45:58 -0400 Subject: [PATCH 02/11] update spec to set value to false --- spec/controllers/users/email_confirmations_controller_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/controllers/users/email_confirmations_controller_spec.rb b/spec/controllers/users/email_confirmations_controller_spec.rb index b7e27a0aebe..77244319479 100644 --- a/spec/controllers/users/email_confirmations_controller_spec.rb +++ b/spec/controllers/users/email_confirmations_controller_spec.rb @@ -50,6 +50,10 @@ end context 'when select email feature is disabled' do + before do + allow(IdentityConfig.store).to receive(:feature_select_email_to_share_enabled). + and_return(false) + end it 'should render proper flash member' do flash_message = t('devise.confirmations.confirmed') user = create(:user) From 82e6a4210e660ee54383c345ce994ea6d2cde6fc Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 4 Nov 2024 09:45:41 -0500 Subject: [PATCH 03/11] update edit confirmation emails so it only shows proper message --- .../users/email_confirmations_controller.rb | 3 +- app/controllers/users/emails_controller.rb | 6 ++- app/forms/add_user_email_form.rb | 7 +-- app/mailers/user_mailer.rb | 3 +- app/services/send_add_email_confirmation.rb | 6 ++- .../selected_email/edit.html.erb | 2 +- .../email_confirmations_controller_spec.rb | 54 +++++++++++++------ 7 files changed, 57 insertions(+), 24 deletions(-) diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index b5ed30436bc..e7b28f2f481 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -91,7 +91,8 @@ def message_for_already_confirmed_user end def flash_message_for_successful_signed_in_confirmation - if IdentityConfig.store.feature_select_email_to_share_enabled + if IdentityConfig.store.feature_select_email_to_share_enabled && + params[:from_select_email_flow].to_s == 'true' t( 'account.emails.confirmed_html', url: account_connected_accounts_url, diff --git a/app/controllers/users/emails_controller.rb b/app/controllers/users/emails_controller.rb index e213740f38c..897e20afbbd 100644 --- a/app/controllers/users/emails_controller.rb +++ b/app/controllers/users/emails_controller.rb @@ -12,6 +12,7 @@ class EmailsController < ApplicationController def show analytics.add_email_visit + user_session[:from_select_email_flow] = params[:from_select_email_flow] @add_user_email_form = AddUserEmailForm.new @pending_completions_consent = pending_completions_consent? end @@ -19,7 +20,10 @@ def show def add @add_user_email_form = AddUserEmailForm.new - result = @add_user_email_form.submit(current_user, permitted_params) + result = @add_user_email_form.submit( + current_user, permitted_params, + user_session[:from_select_email_flow] + ) analytics.add_email_request(**result.to_h) if result.success? diff --git a/app/forms/add_user_email_form.rb b/app/forms/add_user_email_form.rb index 99530bb5bb2..60d15978363 100644 --- a/app/forms/add_user_email_form.rb +++ b/app/forms/add_user_email_form.rb @@ -15,10 +15,11 @@ def user @user ||= User.new end - def submit(user, params) + def submit(user, params, from_select_email_flow = nil) @user = user @email = params[:email] @email_address = email_address_record(@email) + @from_select_email_flow = from_select_email_flow if valid? process_successful_submission @@ -42,12 +43,12 @@ def email_address_record(email) private attr_writer :email - attr_reader :success, :email_address + attr_reader :success, :email_address, :from_select_email_flow def process_successful_submission @success = true email_address.save! - SendAddEmailConfirmation.new(user).call(email_address) + SendAddEmailConfirmation.new(user).call(email_address, from_select_email_flow) end def extra_analytics_attributes diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index afb50fab1ae..2ba243c3cd5 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -218,13 +218,14 @@ def verify_by_mail_letter_requested end end - def add_email(token) + def add_email(token, from_select_email_flow = false) with_user_locale(user) do presenter = ConfirmationEmailPresenter.new(user, view_context) @first_sentence = presenter.first_sentence @confirmation_period = presenter.confirmation_period @add_email_url = add_email_confirmation_url( confirmation_token: token, + from_select_email_flow: from_select_email_flow, locale: locale_url_param, ) mail(to: email_address.email, subject: t('user_mailer.add_email.subject')) diff --git a/app/services/send_add_email_confirmation.rb b/app/services/send_add_email_confirmation.rb index cc9eb7dc7d8..04e9f0bc987 100644 --- a/app/services/send_add_email_confirmation.rb +++ b/app/services/send_add_email_confirmation.rb @@ -7,8 +7,9 @@ def initialize(user) @user = user end - def call(email_address) + def call(email_address, from_select_email_flow = false) @email_address = email_address + @from_select_email_flow = from_select_email_flow update_email_address_record send_email end @@ -23,7 +24,7 @@ def confirmation_sent_at email_address.confirmation_sent_at end - attr_reader :email_address + attr_reader :email_address, :from_select_email_flow def update_email_address_record email_address.update!( @@ -59,6 +60,7 @@ def send_email_associated_with_another_account_email def send_confirmation_email UserMailer.with(user: user, email_address: email_address).add_email( confirmation_token, + from_select_email_flow, ).deliver_now_or_later end end diff --git a/app/views/accounts/connected_accounts/selected_email/edit.html.erb b/app/views/accounts/connected_accounts/selected_email/edit.html.erb index f513ad8ddd7..1831815365a 100644 --- a/app/views/accounts/connected_accounts/selected_email/edit.html.erb +++ b/app/views/accounts/connected_accounts/selected_email/edit.html.erb @@ -34,7 +34,7 @@ <% end %> <%= render ButtonComponent.new( - url: add_email_path, + url: add_email_path(from_select_email_flow: true), outline: true, big: true, wide: true, diff --git a/spec/controllers/users/email_confirmations_controller_spec.rb b/spec/controllers/users/email_confirmations_controller_spec.rb index 77244319479..72cd00af0f1 100644 --- a/spec/controllers/users/email_confirmations_controller_spec.rb +++ b/spec/controllers/users/email_confirmations_controller_spec.rb @@ -31,21 +31,45 @@ and_return(true) end - it 'renders the proper flash message for signed in user' do - flash_message = t( - 'account.emails.confirmed_html', - url: account_connected_accounts_url, - ) - user = create(:user) - sign_in user - new_email = Faker::Internet.email - - add_email_form = AddUserEmailForm.new - add_email_form.submit(user, email: new_email) - email_record = add_email_form.email_address_record(new_email) - - get :create, params: { confirmation_token: email_record.reload.confirmation_token } - expect(flash[:success]).to eq(flash_message) + context 'when user is in select email form flow' do + it 'renders the proper flash message' do + flash_message = t( + 'account.emails.confirmed_html', + url: account_connected_accounts_url, + ) + user = create(:user) + sign_in user + new_email = Faker::Internet.email + + add_email_form = AddUserEmailForm.new + add_email_form.submit(user, email: new_email) + email_record = add_email_form.email_address_record(new_email) + + get :create, params: { + confirmation_token: email_record.reload.confirmation_token, + from_select_email_flow: true, + } + expect(flash[:success]).to eq(flash_message) + end + end + + context 'when user is not in email form flow' do + it 'renders proper flash message' do + flash_message = t('devise.confirmations.confirmed') + user = create(:user) + sign_in user + new_email = Faker::Internet.email + + add_email_form = AddUserEmailForm.new + add_email_form.submit(user, email: new_email) + email_record = add_email_form.email_address_record(new_email) + + get :create, params: { + confirmation_token: email_record.reload.confirmation_token, + from_select_email_flow: false, + } + expect(flash[:success]).to eq(flash_message) + end end end From c08d017a8d3bba2b61a3420b1bd50c8e8763cb9b Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 4 Nov 2024 10:25:58 -0500 Subject: [PATCH 04/11] fix rubocop --- .../email_confirmations_controller_spec.rb | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/controllers/users/email_confirmations_controller_spec.rb b/spec/controllers/users/email_confirmations_controller_spec.rb index 72cd00af0f1..92cec3d8415 100644 --- a/spec/controllers/users/email_confirmations_controller_spec.rb +++ b/spec/controllers/users/email_confirmations_controller_spec.rb @@ -40,34 +40,34 @@ user = create(:user) sign_in user new_email = Faker::Internet.email - + add_email_form = AddUserEmailForm.new add_email_form.submit(user, email: new_email) email_record = add_email_form.email_address_record(new_email) - - get :create, params: { - confirmation_token: email_record.reload.confirmation_token, - from_select_email_flow: true, - } + + get :create, params: { + confirmation_token: email_record.reload.confirmation_token, + from_select_email_flow: true, + } expect(flash[:success]).to eq(flash_message) end end - + context 'when user is not in email form flow' do it 'renders proper flash message' do flash_message = t('devise.confirmations.confirmed') user = create(:user) sign_in user new_email = Faker::Internet.email - + add_email_form = AddUserEmailForm.new add_email_form.submit(user, email: new_email) email_record = add_email_form.email_address_record(new_email) - - get :create, params: { - confirmation_token: email_record.reload.confirmation_token, - from_select_email_flow: false, - } + + get :create, params: { + confirmation_token: email_record.reload.confirmation_token, + from_select_email_flow: false, + } expect(flash[:success]).to eq(flash_message) end end From 45c73ed74ab4b825971aca4d2a8d1fcd7327d2f9 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 4 Nov 2024 12:26:44 -0500 Subject: [PATCH 05/11] fix to show when user also signs in after confirming email --- app/controllers/accounts_controller.rb | 7 +++++++ app/controllers/users/email_confirmations_controller.rb | 8 +++++++- app/views/sign_up/completions/show.html.erb | 2 +- app/views/sign_up/select_email/show.html.erb | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 90d78f33bf3..236fb9ed953 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -19,6 +19,13 @@ def show user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) + if session[:from_select_email_flow] + flash[:success] = t( + 'account.emails.confirmed_html', + url: account_connected_accounts_url, + ) + session.delete(:from_select_email_flow) + end end def reauthentication diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index e7b28f2f481..e244ce4a0e0 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -42,8 +42,10 @@ def email_address_already_confirmed? def process_successful_confirmation(email_address) confirm_and_notify(email_address) + store_from_select_email_flow_in_session if current_user flash[:success] = flash_message_for_successful_signed_in_confirmation + session.delete(:from_select_email_flow) redirect_to account_url else flash[:success] = t('devise.confirmations.confirmed_but_sign_in') @@ -92,7 +94,7 @@ def message_for_already_confirmed_user def flash_message_for_successful_signed_in_confirmation if IdentityConfig.store.feature_select_email_to_share_enabled && - params[:from_select_email_flow].to_s == 'true' + session[:from_select_email_flow] t( 'account.emails.confirmed_html', url: account_connected_accounts_url, @@ -110,5 +112,9 @@ def email_address_already_confirmed_by_current_user? def confirmation_params params.permit(:confirmation_token) end + + def store_from_select_email_flow_in_session + session[:from_select_email_flow] = params[:from_select_email_flow].to_s == 'true' + end end end diff --git a/app/views/sign_up/completions/show.html.erb b/app/views/sign_up/completions/show.html.erb index e019e17a33d..2a74d7ca2e3 100644 --- a/app/views/sign_up/completions/show.html.erb +++ b/app/views/sign_up/completions/show.html.erb @@ -49,7 +49,7 @@ <% if @presenter.multiple_emails? %> <%= link_to t('help_text.requested_attributes.change_email_link'), sign_up_select_email_path %> <% else %> - <%= link_to t('account.index.email_add'), add_email_path %> + <%= link_to t('account.index.email_add'), add_email_path(from_select_email_flow: true) %> <% end %>

diff --git a/app/views/sign_up/select_email/show.html.erb b/app/views/sign_up/select_email/show.html.erb index 0403c5d2653..6ff303a728e 100644 --- a/app/views/sign_up/select_email/show.html.erb +++ b/app/views/sign_up/select_email/show.html.erb @@ -30,7 +30,7 @@ <% end %> <%= render ButtonComponent.new( - url: add_email_path, + url: add_email_path(from_select_email_flow: true), outline: true, big: true, wide: true, From e0de99d6bb9047162019b07c4f61963b3c847138 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 4 Nov 2024 14:13:01 -0500 Subject: [PATCH 06/11] add spec for accounts controller flash logic, clean up emails confirmation logic --- app/controllers/accounts_controller.rb | 5 +- .../users/email_confirmations_controller.rb | 14 +----- spec/controllers/accounts_controller_spec.rb | 36 ++++++++++++++ .../email_confirmations_controller_spec.rb | 48 ------------------- 4 files changed, 39 insertions(+), 64 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 236fb9ed953..e6068396f1c 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -19,12 +19,11 @@ def show user: current_user, locked_for_session: pii_locked_for_session?(current_user), ) - if session[:from_select_email_flow] - flash[:success] = t( + if session.delete(:from_select_email_flow) + flash.now[:success] = t( 'account.emails.confirmed_html', url: account_connected_accounts_url, ) - session.delete(:from_select_email_flow) end end diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index e244ce4a0e0..2a478677353 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -44,7 +44,7 @@ def process_successful_confirmation(email_address) confirm_and_notify(email_address) store_from_select_email_flow_in_session if current_user - flash[:success] = flash_message_for_successful_signed_in_confirmation + flash[:success] = t('devise.confirmations.confirmed') session.delete(:from_select_email_flow) redirect_to account_url else @@ -92,18 +92,6 @@ def message_for_already_confirmed_user end end - def flash_message_for_successful_signed_in_confirmation - if IdentityConfig.store.feature_select_email_to_share_enabled && - session[:from_select_email_flow] - t( - 'account.emails.confirmed_html', - url: account_connected_accounts_url, - ) - else - t('devise.confirmations.confirmed') - end - end - def email_address_already_confirmed_by_current_user? user_signed_in? && email_confirmation_token_validator.email_address_already_confirmed_by_user?(current_user) diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index 52818437548..0bae12eabd0 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -80,6 +80,42 @@ end end + context 'when user just added new email through select email flow' do + context 'when user is in select email form flow' do + before do + session[:from_select_email_flow] = true + end + it 'renders the proper flash message' do + flash_message = t( + 'account.emails.confirmed_html', + url: account_connected_accounts_url, + ) + user = create(:user, :fully_registered) + sign_in user + + get :show + + expect(response).to_not be_redirect + expect(flash[:success]).to eq(flash_message) + expect(session[:from_select_email_flow]).to be_nil + end + end + + context 'when user is not in email form flow' do + before do + session[:from_select_email_flow] = false + end + it 'renders proper flash message' do + t('devise.confirmations.confirmed') + user = create(:user, :fully_registered) + sign_in user + + get :show + expect(flash[:success]).to be_nil + end + end + end + context 'when a profile has been deactivated by password reset' do it 'renders the profile and shows a deactivation banner' do user = create( diff --git a/spec/controllers/users/email_confirmations_controller_spec.rb b/spec/controllers/users/email_confirmations_controller_spec.rb index 92cec3d8415..c6a7613e381 100644 --- a/spec/controllers/users/email_confirmations_controller_spec.rb +++ b/spec/controllers/users/email_confirmations_controller_spec.rb @@ -25,54 +25,6 @@ get :create, params: { confirmation_token: email_record.reload.confirmation_token } end - context ' when select email feature is enabled' do - before do - allow(IdentityConfig.store).to receive(:feature_select_email_to_share_enabled). - and_return(true) - end - - context 'when user is in select email form flow' do - it 'renders the proper flash message' do - flash_message = t( - 'account.emails.confirmed_html', - url: account_connected_accounts_url, - ) - user = create(:user) - sign_in user - new_email = Faker::Internet.email - - add_email_form = AddUserEmailForm.new - add_email_form.submit(user, email: new_email) - email_record = add_email_form.email_address_record(new_email) - - get :create, params: { - confirmation_token: email_record.reload.confirmation_token, - from_select_email_flow: true, - } - expect(flash[:success]).to eq(flash_message) - end - end - - context 'when user is not in email form flow' do - it 'renders proper flash message' do - flash_message = t('devise.confirmations.confirmed') - user = create(:user) - sign_in user - new_email = Faker::Internet.email - - add_email_form = AddUserEmailForm.new - add_email_form.submit(user, email: new_email) - email_record = add_email_form.email_address_record(new_email) - - get :create, params: { - confirmation_token: email_record.reload.confirmation_token, - from_select_email_flow: false, - } - expect(flash[:success]).to eq(flash_message) - end - end - end - context 'when select email feature is disabled' do before do allow(IdentityConfig.store).to receive(:feature_select_email_to_share_enabled). From a15fc8e5607cf44e8054bdba041237ccafeb56d4 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 4 Nov 2024 14:42:48 -0500 Subject: [PATCH 07/11] use hidden field instead of user session --- app/controllers/users/email_confirmations_controller.rb | 1 - app/controllers/users/emails_controller.rb | 9 +++------ app/forms/add_user_email_form.rb | 4 ++-- app/views/users/emails/show.html.erb | 1 + 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/controllers/users/email_confirmations_controller.rb b/app/controllers/users/email_confirmations_controller.rb index 2a478677353..7e7e87e9f8b 100644 --- a/app/controllers/users/email_confirmations_controller.rb +++ b/app/controllers/users/email_confirmations_controller.rb @@ -45,7 +45,6 @@ def process_successful_confirmation(email_address) store_from_select_email_flow_in_session if current_user flash[:success] = t('devise.confirmations.confirmed') - session.delete(:from_select_email_flow) redirect_to account_url else flash[:success] = t('devise.confirmations.confirmed_but_sign_in') diff --git a/app/controllers/users/emails_controller.rb b/app/controllers/users/emails_controller.rb index 897e20afbbd..5ad948c2b58 100644 --- a/app/controllers/users/emails_controller.rb +++ b/app/controllers/users/emails_controller.rb @@ -12,7 +12,7 @@ class EmailsController < ApplicationController def show analytics.add_email_visit - user_session[:from_select_email_flow] = params[:from_select_email_flow] + @from_select_email_flow = params[:from_select_email_flow] @add_user_email_form = AddUserEmailForm.new @pending_completions_consent = pending_completions_consent? end @@ -20,10 +20,7 @@ def show def add @add_user_email_form = AddUserEmailForm.new - result = @add_user_email_form.submit( - current_user, permitted_params, - user_session[:from_select_email_flow] - ) + result = @add_user_email_form.submit(current_user, permitted_params) analytics.add_email_request(**result.to_h) if result.success? @@ -109,7 +106,7 @@ def session_email end def permitted_params - params.require(:user).permit(:email) + params.require(:user).permit(:email, :from_select_email_flow) end def check_max_emails_per_account diff --git a/app/forms/add_user_email_form.rb b/app/forms/add_user_email_form.rb index 60d15978363..1ee5ce7d45c 100644 --- a/app/forms/add_user_email_form.rb +++ b/app/forms/add_user_email_form.rb @@ -15,11 +15,11 @@ def user @user ||= User.new end - def submit(user, params, from_select_email_flow = nil) + def submit(user, params) @user = user @email = params[:email] @email_address = email_address_record(@email) - @from_select_email_flow = from_select_email_flow + @from_select_email_flow = params[:from_select_email_flow] if valid? process_successful_submission diff --git a/app/views/users/emails/show.html.erb b/app/views/users/emails/show.html.erb index c300e5ad70e..b8204922d83 100644 --- a/app/views/users/emails/show.html.erb +++ b/app/views/users/emails/show.html.erb @@ -10,6 +10,7 @@ label: t('forms.registration.labels.email'), required: true, ) %> + <%= f.hidden_field(:from_select_email_flow, value: @from_select_email_flow) %> <%= f.submit t('forms.buttons.submit.default') %> <% end %> From 3305181eabb08bc6943cc3becb7389b8abd881fa Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Mon, 4 Nov 2024 15:07:23 -0500 Subject: [PATCH 08/11] add user mailer spec for additional attribute --- spec/mailers/user_mailer_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 7f4b80dd228..71f615af5fe 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -43,6 +43,22 @@ expect(mail.html_part.body).to have_content(add_email_url) expect(mail.html_part.body).to_not have_content(sign_up_create_email_confirmation_url) end + + context 'when user adds email from select email flow' do + let(:token) { SecureRandom.hex } + let(:mail) do + UserMailer.with(user: user, email_address: email_address).add_email(token, true) + end + + it 'renders the add_email_confirmation_url' do + add_email_url = add_email_confirmation_url( + confirmation_token: token, + from_select_email_flow: true, + ) + + expect(mail.html_part.body).to have_content(add_email_url) + end + end end describe '#email_deleted' do From 4c4ae2d191e3487895822424c441b6c443a6983c Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 5 Nov 2024 11:47:41 -0500 Subject: [PATCH 09/11] move back to using session, but also ensure we delete session after submission --- app/controllers/users/emails_controller.rb | 13 +++++++++---- app/forms/add_user_email_form.rb | 11 +++++++---- app/mailers/user_mailer.rb | 4 ++-- app/services/send_add_email_confirmation.rb | 8 ++++---- .../connected_accounts/selected_email/edit.html.erb | 2 +- app/views/sign_up/completions/show.html.erb | 2 +- app/views/sign_up/select_email/show.html.erb | 2 +- app/views/users/emails/show.html.erb | 1 - 8 files changed, 25 insertions(+), 18 deletions(-) diff --git a/app/controllers/users/emails_controller.rb b/app/controllers/users/emails_controller.rb index 5ad948c2b58..548ba6c5ae9 100644 --- a/app/controllers/users/emails_controller.rb +++ b/app/controllers/users/emails_controller.rb @@ -12,15 +12,19 @@ class EmailsController < ApplicationController def show analytics.add_email_visit - @from_select_email_flow = params[:from_select_email_flow] + session[:in_select_email_flow] = params[:in_select_email_flow] @add_user_email_form = AddUserEmailForm.new @pending_completions_consent = pending_completions_consent? end def add - @add_user_email_form = AddUserEmailForm.new + @add_user_email_form = AddUserEmailForm.new( + session[:in_select_email_flow], + ) - result = @add_user_email_form.submit(current_user, permitted_params) + result = @add_user_email_form.submit( + current_user, permitted_params + ) analytics.add_email_request(**result.to_h) if result.success? @@ -95,6 +99,7 @@ def handle_successful_delete end def process_successful_creation + session.delete(:in_select_email_flow) resend_confirmation = params[:user][:resend] session[:email] = @add_user_email_form.email @@ -106,7 +111,7 @@ def session_email end def permitted_params - params.require(:user).permit(:email, :from_select_email_flow) + params.require(:user).permit(:email) end def check_max_emails_per_account diff --git a/app/forms/add_user_email_form.rb b/app/forms/add_user_email_form.rb index 1ee5ce7d45c..cc91ff62fa2 100644 --- a/app/forms/add_user_email_form.rb +++ b/app/forms/add_user_email_form.rb @@ -5,12 +5,16 @@ class AddUserEmailForm include FormAddEmailValidator include ActionView::Helpers::TranslationHelper - attr_reader :email + attr_reader :email, :in_select_email_flow def self.model_name ActiveModel::Name.new(self, nil, 'User') end + def initialize(in_select_email_flow = nil) + @in_select_email_flow = in_select_email_flow + end + def user @user ||= User.new end @@ -19,7 +23,6 @@ def submit(user, params) @user = user @email = params[:email] @email_address = email_address_record(@email) - @from_select_email_flow = params[:from_select_email_flow] if valid? process_successful_submission @@ -43,12 +46,12 @@ def email_address_record(email) private attr_writer :email - attr_reader :success, :email_address, :from_select_email_flow + attr_reader :success, :email_address def process_successful_submission @success = true email_address.save! - SendAddEmailConfirmation.new(user).call(email_address, from_select_email_flow) + SendAddEmailConfirmation.new(user).call(email_address, in_select_email_flow) end def extra_analytics_attributes diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 2ba243c3cd5..804c61ea82c 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -218,14 +218,14 @@ def verify_by_mail_letter_requested end end - def add_email(token, from_select_email_flow = false) + def add_email(token, from_select_email_flow = nil) with_user_locale(user) do presenter = ConfirmationEmailPresenter.new(user, view_context) @first_sentence = presenter.first_sentence @confirmation_period = presenter.confirmation_period @add_email_url = add_email_confirmation_url( confirmation_token: token, - from_select_email_flow: from_select_email_flow, + from_select_email_flow:, locale: locale_url_param, ) mail(to: email_address.email, subject: t('user_mailer.add_email.subject')) diff --git a/app/services/send_add_email_confirmation.rb b/app/services/send_add_email_confirmation.rb index 04e9f0bc987..fbeb8d900a8 100644 --- a/app/services/send_add_email_confirmation.rb +++ b/app/services/send_add_email_confirmation.rb @@ -7,9 +7,9 @@ def initialize(user) @user = user end - def call(email_address, from_select_email_flow = false) + def call(email_address, in_select_email_flow = nil) @email_address = email_address - @from_select_email_flow = from_select_email_flow + @in_select_email_flow = in_select_email_flow update_email_address_record send_email end @@ -24,7 +24,7 @@ def confirmation_sent_at email_address.confirmation_sent_at end - attr_reader :email_address, :from_select_email_flow + attr_reader :email_address, :in_select_email_flow def update_email_address_record email_address.update!( @@ -60,7 +60,7 @@ def send_email_associated_with_another_account_email def send_confirmation_email UserMailer.with(user: user, email_address: email_address).add_email( confirmation_token, - from_select_email_flow, + in_select_email_flow, ).deliver_now_or_later end end diff --git a/app/views/accounts/connected_accounts/selected_email/edit.html.erb b/app/views/accounts/connected_accounts/selected_email/edit.html.erb index 1831815365a..f2a57f126e5 100644 --- a/app/views/accounts/connected_accounts/selected_email/edit.html.erb +++ b/app/views/accounts/connected_accounts/selected_email/edit.html.erb @@ -34,7 +34,7 @@ <% end %> <%= render ButtonComponent.new( - url: add_email_path(from_select_email_flow: true), + url: add_email_path(in_select_email_flow: true), outline: true, big: true, wide: true, diff --git a/app/views/sign_up/completions/show.html.erb b/app/views/sign_up/completions/show.html.erb index 2a74d7ca2e3..b40f9ce2a3c 100644 --- a/app/views/sign_up/completions/show.html.erb +++ b/app/views/sign_up/completions/show.html.erb @@ -49,7 +49,7 @@ <% if @presenter.multiple_emails? %> <%= link_to t('help_text.requested_attributes.change_email_link'), sign_up_select_email_path %> <% else %> - <%= link_to t('account.index.email_add'), add_email_path(from_select_email_flow: true) %> + <%= link_to t('account.index.email_add'), add_email_path(in_select_email_flow: true) %> <% end %>

diff --git a/app/views/sign_up/select_email/show.html.erb b/app/views/sign_up/select_email/show.html.erb index 6ff303a728e..53b0c04e5de 100644 --- a/app/views/sign_up/select_email/show.html.erb +++ b/app/views/sign_up/select_email/show.html.erb @@ -30,7 +30,7 @@ <% end %> <%= render ButtonComponent.new( - url: add_email_path(from_select_email_flow: true), + url: add_email_path(in_select_email_flow: true), outline: true, big: true, wide: true, diff --git a/app/views/users/emails/show.html.erb b/app/views/users/emails/show.html.erb index b8204922d83..c300e5ad70e 100644 --- a/app/views/users/emails/show.html.erb +++ b/app/views/users/emails/show.html.erb @@ -10,7 +10,6 @@ label: t('forms.registration.labels.email'), required: true, ) %> - <%= f.hidden_field(:from_select_email_flow, value: @from_select_email_flow) %> <%= f.submit t('forms.buttons.submit.default') %> <% end %> From fa816a6488e86a1b19e7f337060413461f7caf7c Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 5 Nov 2024 14:33:49 -0500 Subject: [PATCH 10/11] update rubocop and to work for the resend or add different email --- app/controllers/users/emails_controller.rb | 9 ++++++--- app/views/users/emails/verify.html.erb | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/emails_controller.rb b/app/controllers/users/emails_controller.rb index 548ba6c5ae9..b52c7625d8c 100644 --- a/app/controllers/users/emails_controller.rb +++ b/app/controllers/users/emails_controller.rb @@ -76,7 +76,8 @@ def verify if session_email.blank? redirect_to add_email_url else - render :verify, locals: { email: session_email } + render :verify, + locals: { email: session_email, in_select_email_flow: params[:in_select_email_flow] } end end @@ -99,11 +100,13 @@ def handle_successful_delete end def process_successful_creation - session.delete(:in_select_email_flow) resend_confirmation = params[:user][:resend] session[:email] = @add_user_email_form.email - redirect_to add_email_verify_email_url(resend: resend_confirmation) + redirect_to add_email_verify_email_url( + resend: resend_confirmation, + in_select_email_flow: session.delete(:in_select_email_flow), + ) end def session_email diff --git a/app/views/users/emails/verify.html.erb b/app/views/users/emails/verify.html.erb index 471f93d0a34..211b59fc6ee 100644 --- a/app/views/users/emails/verify.html.erb +++ b/app/views/users/emails/verify.html.erb @@ -22,7 +22,7 @@ <%= t('notices.signed_up_and_confirmed.no_email_sent_explanation_start') %> <%= button_to(add_email_resend_path, method: :post, class: 'usa-button usa-button--unstyled', form_class: 'display-inline-block padding-left-1') { t('links.resend') } %> -

<%= t('notices.use_diff_email.text_html', link_html: link_to(t('notices.use_diff_email.link'), add_email_path)) %>

+

<%= t('notices.use_diff_email.text_html', link_html: link_to(t('notices.use_diff_email.link'), add_email_path(in_select_email_flow: in_select_email_flow))) %>

<%= t('devise.registrations.close_window') %>

<% if FeatureManagement.enable_load_testing_mode? && EmailAddress.find_with_email(email) %> From 0cb47641e5ab023d18284ffb5804b2b3259f42c1 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Tue, 5 Nov 2024 15:50:13 -0500 Subject: [PATCH 11/11] fix verify html spec --- spec/views/users/emails/verify.html.erb_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/views/users/emails/verify.html.erb_spec.rb b/spec/views/users/emails/verify.html.erb_spec.rb index 5c74dd8b7ed..f90ae443004 100644 --- a/spec/views/users/emails/verify.html.erb_spec.rb +++ b/spec/views/users/emails/verify.html.erb_spec.rb @@ -4,6 +4,7 @@ let(:email) { 'foo@bar.com' } before do allow(view).to receive(:email).and_return(email) + allow(view).to receive(:in_select_email_flow).and_return(nil) @resend_email_confirmation_form = ResendEmailConfirmationForm.new end