Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions app/controllers/concerns/idv_step_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,28 @@ def flow_path
idv_session.flow_path
end

def confirm_hybrid_handoff_needed
setup_for_redo if params[:redo]

if idv_session.skip_hybrid_handoff?
# We previously skipped hybrid handoff. Keep doing that.
idv_session.flow_path = 'standard'
end

if !FeatureManagement.idv_allow_hybrid_flow?
# When hybrid flow is unavailable, skip it.
# But don't store that we skipped it in idv_session, in case it is back to
# available when the user tries to redo document capture.
idv_session.flow_path = 'standard'
end

if idv_session.flow_path == 'standard'
redirect_to idv_document_capture_url
elsif idv_session.flow_path == 'hybrid'
redirect_to idv_link_sent_url
end
end

private

def confirm_ssn_step_complete
Expand Down Expand Up @@ -96,4 +118,16 @@ def extra_analytics_properties
end
extra
end

def setup_for_redo
idv_session.redo_document_capture = true

# If we previously skipped hybrid handoff for the user (because they're on a mobile
# device with a camera), skip it _again_ here.
if idv_session.skip_hybrid_handoff?
idv_session.flow_path = 'standard'
else
idv_session.flow_path = nil
end
end
end
34 changes: 0 additions & 34 deletions app/controllers/idv/hybrid_handoff_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,40 +192,6 @@ def confirm_agreement_step_complete
redirect_to idv_agreement_url
end

def confirm_hybrid_handoff_needed
setup_for_redo if params[:redo]

if idv_session.skip_hybrid_handoff?
# We previously skipped hybrid handoff. Keep doing that.
idv_session.flow_path = 'standard'
end

if !FeatureManagement.idv_allow_hybrid_flow?
# When hybrid flow is unavailable, skip it.
# But don't store that we skipped it in idv_session, in case it is back to
# available when the user tries to redo document capture.
idv_session.flow_path = 'standard'
end

if idv_session.flow_path == 'standard'
redirect_to idv_document_capture_url
elsif idv_session.flow_path == 'hybrid'
redirect_to idv_link_sent_url
end
end

def setup_for_redo
idv_session.redo_document_capture = true

# If we previously skipped hybrid handoff for the user (because they're on a mobile
# device with a camera), skip it _again_ here.
if idv_session.skip_hybrid_handoff?
idv_session.flow_path = 'standard'
else
idv_session.flow_path = nil
end
end

def formatted_destination_phone
raw_phone = params.require(:doc_auth).permit(:phone)
PhoneFormatter.format(raw_phone, country_code: 'US')
Expand Down
21 changes: 21 additions & 0 deletions app/controllers/idv/phone_question_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module Idv
class PhoneQuestionController < ApplicationController
include ActionView::Helpers::DateHelper
include IdvStepConcern
include StepIndicatorConcern

before_action :confirm_verify_info_step_needed
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to think about why this is here and in HybridHandoffController. I think it's because we delete pii_from_doc from the session after the VerifyInfo step, so we need to make sure they're not past that step when they land here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep ... after verify info accessing pii is ☹️ LG-10603

before_action :confirm_agreement_step_complete
before_action :confirm_hybrid_handoff_needed, only: :show

def show
@title = t('doc_auth.headings.phone_question')
end

def confirm_agreement_step_complete
return if idv_session.idv_consent_given

redirect_to idv_agreement_url
end
end
end
32 changes: 32 additions & 0 deletions app/views/idv/phone_question/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<% title @title %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the title never changes, could we have just inlined the text to avoid the indirection?

Suggested change
<% title @title %>
<% title t('doc_auth.headings.phone_question') %>


<% content_for(:pre_flash_content) do %>
<%= render StepIndicatorComponent.new(
steps: Idv::StepIndicatorConcern::STEP_INDICATOR_STEPS,
current_step: :verify_id,
locale_scope: 'idv',
class: 'margin-x-neg-2 margin-top-neg-4 tablet:margin-x-neg-6 tablet:margin-top-neg-4',
) %>
<% end %>

<%= render PageHeadingComponent.new.with_content(@title) %>

<div class="margin-top-4">
<%= button_to(
'',
method: 'get',
class: 'usa-button usa-button--wide usa-button--big',
) do %>
<%= t('doc_auth.buttons.have_phone') %>
<% end %>
</div>

<p class="margin-top-2">

<%= link_to(
t('doc_auth.phone_question.do_not_have'),
'',
) %>
</p>

<%= render 'idv/doc_auth/cancel', step: 'phone_question' %>
4 changes: 4 additions & 0 deletions config/locales/doc_auth/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ en:
buttons:
add_new_photos: Add new photos
continue: Continue
have_phone: Yes, I have a phone
take_or_upload_picture_html: '<lg-take-photo>Take photo</lg-take-photo> or
<lg-upload>Upload photo</lg-upload>'
take_picture: Take photo
Expand Down Expand Up @@ -143,6 +144,7 @@ en:
hybrid_handoff: How would you like to add your ID?
interstitial: We are processing your images
lets_go: How verifying your identity works
phone_question: Do you have a phone you can use to take photos of your ID?
review_issues: Check your images and try again
secure_account: Secure your account
ssn: Enter your Social Security number
Expand Down Expand Up @@ -230,6 +232,8 @@ en:
- '<strong>Verify by mail</strong>: We’ll mail a letter to your home
address. This takes <strong>5 to 10 days</strong>.'
welcome: 'You will need your:'
phone_question:
do_not_have: I don’t have a phone
tips:
document_capture_header_text: 'For best results:'
document_capture_hint: Must be a JPG or PNG
Expand Down
5 changes: 5 additions & 0 deletions config/locales/doc_auth/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ es:
buttons:
add_new_photos: Añadir nuevas fotos
continue: Continuar
have_phone: Sí tengo teléfono
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could you add a comma after "Sí"?

take_or_upload_picture_html: '<lg-take-photo>Toma una foto</lg-take-photo> o
<lg-upload>Sube una foto</lg-upload>'
take_picture: Toma una foto
Expand Down Expand Up @@ -172,6 +173,8 @@ es:
hybrid_handoff: '¿Cómo desea añadir su documento de identidad?'
interstitial: Estamos procesando sus imágenes
lets_go: Cómo funciona la verificación de su identidad
phone_question: ¿Tiene un teléfono con el que pueda tomar fotos de su documento
de identidad?
review_issues: Revise sus imágenes e inténtelo de nuevo
secure_account: Asegure su cuenta
ssn: Ingrese su número de Seguro Social
Expand Down Expand Up @@ -267,6 +270,8 @@ es:
- '<strong>Verificar por correo</strong>: Le enviaremos una carta a su
domicilio. Esto tarda entre <strong>5 y 10 días</strong>.'
welcome: 'Necesitará su:'
phone_question:
do_not_have: No tengo teléfono
tips:
document_capture_header_text: 'Para obtener los mejores resultados:'
document_capture_hint: Debe ser un JPG o PNG
Expand Down
5 changes: 5 additions & 0 deletions config/locales/doc_auth/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ fr:
buttons:
add_new_photos: Ajoutez de nouvelles photos
continue: Continuer
have_phone: Oui j’ai un téléphone
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could you add a comma after "Oui"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i had asked myself the same question 🤔 but after checking how we translate across the site, not using the comma after yes/no is the pattern we use everywhere else

take_or_upload_picture_html: '<lg-take-photo>Prendre une photo</lg-take-photo>
ou <lg-upload>Télécharger une photo</lg-upload>'
take_picture: Prendre une photo
Expand Down Expand Up @@ -178,6 +179,8 @@ fr:
hybrid_handoff: Comment voulez-vous ajouter votre identifiant ?
interstitial: Nous traitons vos images
lets_go: Comment fonctionne la vérification de votre identité
phone_question: Avez-vous un téléphone que vous pouvez utiliser pour prendre des
photos de votre identifiant?
review_issues: Vérifiez vos images et essayez à nouveau
secure_account: Sécuriser votre compte
ssn: Saisissez votre numéro de sécurité sociale
Expand Down Expand Up @@ -276,6 +279,8 @@ fr:
lettre à votre adresse personnelle. Cela prend <strong>5 à 10
jours</strong>.'
welcome: 'Vous aurez besoin de votre:'
phone_question:
do_not_have: Je n’ai pas de téléphone
tips:
document_capture_header_text: 'Pour obtenir les meilleurs résultats:'
document_capture_hint: Doit être un JPG ou PNG
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@
put '/phone_confirmation' => 'otp_verification#update', as: :nil
get '/review' => 'review#new'
put '/review' => 'review#create'
get '/phone_question' => 'phone_question#show'
get '/session/errors/warning' => 'session_errors#warning'
get '/session/errors/state_id_warning' => 'session_errors#state_id_warning'
get '/phone/errors/timeout' => 'phone_errors#timeout'
Expand Down
104 changes: 104 additions & 0 deletions spec/controllers/idv/phone_question_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
require 'rails_helper'

RSpec.describe Idv::PhoneQuestionController do
include IdvHelper

let(:user) { create(:user) }

before do
stub_sign_in(user)
stub_analytics
stub_attempts_tracker
subject.user_session['idv/doc_auth'] = {}
subject.idv_session.idv_consent_given = true
end

describe 'before_actions' do
it 'includes authentication before_action' do
expect(subject).to have_actions(
:before,
:confirm_two_factor_authenticated,
)
end

it 'includes outage before_action' do
expect(subject).to have_actions(
:before,
:check_for_mail_only_outage,
)
end

it 'checks that agreement step is complete' do
expect(subject).to have_actions(
:before,
:confirm_agreement_step_complete,
)
end

it 'checks that hybrid_handoff is needed' do
expect(subject).to have_actions(
:before,
:confirm_hybrid_handoff_needed,
)
end
end

describe '#show' do
it 'renders the show template' do
get :show

expect(response).to render_template :show
end

context 'agreement step is not complete' do
before do
subject.idv_session.idv_consent_given = nil
end

it 'redirects to idv_agreement_url' do
get :show

expect(response).to redirect_to(idv_agreement_url)
end
end

context 'hybrid_handoff already visited' do
it 'redirects to document_capture in standard flow' do
subject.idv_session.flow_path = 'standard'

get :show

expect(response).to redirect_to(idv_document_capture_url)
end

it 'redirects to link_sent in hybrid flow' do
subject.idv_session.flow_path = 'hybrid'

get :show

expect(response).to redirect_to(idv_link_sent_url)
end
end

context 'on mobile device' do
it 'redirects to hybrid_handoff controller' do
subject.idv_session.skip_hybrid_handoff = true

get :show

expect(response).to redirect_to(idv_document_capture_url)
end
end

context 'hybrid flow is not available' do
before do
allow(FeatureManagement).to receive(:idv_allow_hybrid_flow?).and_return(false)
end

it 'redirects the user straight to document capture' do
get :show
expect(response).to redirect_to(idv_document_capture_url)
end
end
end
end
45 changes: 45 additions & 0 deletions spec/features/idv/doc_auth/phone_question_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require 'rails_helper'

RSpec.feature 'phone question step', :js do
include IdvStepHelper
include DocAuthHelper

let(:fake_analytics) { FakeAnalytics.new }

before do
allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics)
sign_in_and_2fa_user
complete_doc_auth_steps_before_hybrid_handoff_step
visit(idv_phone_question_url)
end

it 'contains phone question header' do
expect(page).to have_content(t('doc_auth.headings.phone_question'))
end

it 'contains option to confirm having phone' do
expect(page).to have_content(t('doc_auth.buttons.have_phone'))
end

it 'contains option to confirm not having phone' do
expect(page).to have_content(t('doc_auth.phone_question.do_not_have'))
end

it 'allows user to cancel identify verification' do
click_link t('links.cancel')
expect(page).to have_current_path(idv_cancel_path(step: 'phone_question'))

expect(fake_analytics).to have_logged_event(
'IdV: cancellation visited',
hash_including(step: 'phone_question'),
)

click_spinner_button_and_wait t('idv.cancel.actions.account_page')

expect(current_path).to eq(account_path)
expect(fake_analytics).to have_logged_event(
'IdV: cancellation confirmed',
hash_including(step: 'phone_question'),
)
end
Copy link
Copy Markdown
Contributor

@soniaconnolly soniaconnolly Sep 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could consider adding a spec for the cancel link. Not sure the additional spec would add value or not.

end