Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing address verification step to a form instead of buttons #1781

Merged
merged 20 commits into from
Dec 4, 2017
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
25 changes: 25 additions & 0 deletions app/controllers/verify/address_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,33 @@ class AddressController < ApplicationController

def index; end

def create
response = Idv::AddressDeliveryMethodForm.new.submit(
address_delivery_params.to_h.symbolize_keys
)

if response.success?
redirect_to address_delivery_destination
else
render :index
end
end

private

def address_delivery_params
params.permit(:address_delivery_method)
end

def address_delivery_destination
destination = address_delivery_params[:address_delivery_method]
if destination == 'phone'
verify_phone_path
elsif destination == 'usps'
verify_usps_path
end
end

def confirm_step_needed
redirect_to verify_review_url if idv_session.address_mechanism_chosen?
end
Expand Down
17 changes: 17 additions & 0 deletions app/forms/idv/address_delivery_method_form.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module Idv
class AddressDeliveryMethodForm
attr_accessor :address_delivery_method

def submit(address_delivery_method: '')
self.address_delivery_method = address_delivery_method

FormResponse.new(success: valid_address_delivery_method?, errors: {})
end

private

def valid_address_delivery_method?
%w[phone usps].include? address_delivery_method
end
end
end
26 changes: 19 additions & 7 deletions app/views/verify/address/index.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,25 @@ h1.h2
p
= decorated_session.verification_method_choice

.clearfix
.sm-col.sm-col-12.md-col-6
= link_to t('idv.buttons.activate_by_phone'), verify_phone_path,
class: 'btn btn-primary mb2 center inline-block'
= form_tag
label.block.p2.border.rounded-md.border-light-blue.mb2 for="address_delivery_method_phone"
.radio
= radio_button_tag 'address_delivery_method', :phone, true
span.indicator
strong.blue.block = t('idv.messages.select_verification_form.phone_header')
span = t('idv.messages.select_verification_form.phone_message')

// USPS verification should always be enabled in prod.
// This check is a convenience for lower envs.
- if FeatureManagement.enable_usps_verification?
.sm-col.sm-col-12.md-col-6
= link_to t('idv.buttons.activate_by_mail'), verify_usps_path,
class: 'btn btn-outline rounded-lg mb2 center'
label.block.p2.border.rounded-md.border-light-blue.mb2 for="address_delivery_method_usps"
.radio
= radio_button_tag 'address_delivery_method', :usps
span.indicator
strong.blue.block = t('idv.messages.select_verification_form.usps_header')
span = t('idv.messages.select_verification_form.usps_message')

= button_tag t('forms.buttons.continue'), type: 'submit',
class: 'btn btn-primary col-6 mb2 p2 rounded-lg inline-block'

= render 'shared/cancel', link: verify_cancel_path
20 changes: 12 additions & 8 deletions config/locales/idv/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
en:
idv:
buttons:
activate_by_mail: Activate by mail
activate_by_phone: Activate by phone
cancel: Cancel and return to your profile
continue: Continue identity verification
continue_plain: Continue
Expand Down Expand Up @@ -149,11 +147,17 @@ en:
financial_info: Where is my financial account information?
info_verified_html: We found records matching your %{phone_message}
intro: Your verified information
select_verification_with_sp: To protect you from identity fraud, you can't use
your account at %{sp_name} until you activate it by entering a confirmation
code.
select_verification_without_sp: In order to protect your account from identity
fraud, your profile will not be activated until you enter a confirmation code.
select_verification_form:
phone_header: Get a code by phone
phone_message: The billing address for your phone must match the address you
gave us.
usps_header: Get a code by letter
usps_message: We'll mail a code to the address you gave us. It will take 5-10
days to reach you.
select_verification_with_sp: To protect you from identity fraud, we will contact
you to confirm that this %{sp_name} account is legitimate.
select_verification_without_sp: To protect you from identity fraud, we will
contact you to confirm that this account is legitimate.
sessions:
no_pii: Do not use real personal information (demo purposes only)
pii: personal information
Expand Down Expand Up @@ -223,7 +227,7 @@ en:
otp_delivery_method: Get a code by phone
phone: Phone number of record
review: Review and submit
select_verification: Activate your account
select_verification: Choose how to confirm your address
session:
phone: Get a code by telephone
review: Encrypt your verified data by entering your password
Expand Down
9 changes: 6 additions & 3 deletions config/locales/idv/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
es:
idv:
buttons:
activate_by_mail: Acitive por email
activate_by_phone: Active por teléfono
cancel: Cancele y regrese a su perfil
continue: Continúe la verificación de identidad
continue_plain: Continúe
Expand Down Expand Up @@ -151,6 +149,11 @@ es:
financial_info: "¿Dónde está la información de mi cuenta financiera?"
info_verified_html: Encontramos registros que coinciden con su %{phone_message}
intro: Su información verificada
select_verification_form:
phone_header: NOT TRANSLATED YET
phone_message: NOT TRANSLATED YET
usps_header: NOT TRANSLATED YET
usps_message: NOT TRANSLATED YET
select_verification_with_sp: Para protegerlo de robo de identidad, no puede
utilizar su cuenta en %{sp_name} hasta que la active ingresando un código
de confirmación.
Expand Down Expand Up @@ -197,9 +200,9 @@ es:
sessions:
fail: Su cuenta está <strong>bloqueada por 24 horas</ strong> antes de que
pueda intentar verificarla de nuevo.
jobfail: NOT TRANSLATED YET
heading: No hemos podido encontrar registros que coincidan con su información
personal.
jobfail: NOT TRANSLATED YET
timeout: NOT TRANSLATED YET
warning: Compruebe la información que ingresó. Los errores comunes son números
incorrectos de Seguro Social, código postal o fecha de nacimiento.
Expand Down
7 changes: 5 additions & 2 deletions config/locales/idv/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
fr:
idv:
buttons:
activate_by_mail: Activer par la poste
activate_by_phone: Activer par téléphone
cancel: Annuler et retourner à votre profil
continue: Continuer la vérification d'identité
continue_plain: Continuer
Expand Down Expand Up @@ -160,6 +158,11 @@ fr:
info_verified_html: Nous avons trouvé des données qui correspondent à votre
%{phone_message}
intro: Vos informations vérifiées
select_verification_form:
phone_header: NOT TRANSLATED YET
phone_message: NOT TRANSLATED YET
usps_header: NOT TRANSLATED YET
usps_message: NOT TRANSLATED YET
select_verification_with_sp: Afin de vous protéger des fraudes d'identité, vous
ne pouvez pas utiliser votre compte au %{sp_name} tant que vous ne l'aurez
pas activé en entrant votre code de confirmation.
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
get '/verify' => 'verify#index'
get '/verify/activated' => 'verify#activated'
get '/verify/address' => 'verify/address#index'
post '/verify/address' => 'verify/address#create'
get '/verify/cancel' => 'verify#cancel'
get '/verify/come_back_later' => 'verify/come_back_later#show'
get '/verify/confirmations' => 'verify/confirmations#show'
Expand Down
9 changes: 0 additions & 9 deletions logstash.conf.example
Original file line number Diff line number Diff line change
@@ -1,9 +0,0 @@
input {
file {
path => "path_to_repo/log/events.log"
}
}
output {
elasticsearch { hosts => ["localhost:9200"] }
stdout { codec => rubydebug }
}
43 changes: 43 additions & 0 deletions spec/forms/idv/address_delivery_method_form_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require 'rails_helper'

describe Idv::AddressDeliveryMethodForm do
subject { Idv::AddressDeliveryMethodForm.new }

describe '#submit' do
context 'when delivery method is phone' do
it 'submits without error' do
response = subject.submit(address_delivery_method: 'phone')

expect(response).to be_a(FormResponse)
expect(response.success?).to eq(true)
end
end

context 'when delivery method is usps' do
it 'submits without error' do
response = subject.submit(address_delivery_method: 'usps')

expect(response).to be_a(FormResponse)
expect(response.success?).to eq(true)
end
end

context 'when delivery method is invalid' do
it 'submits with error' do
response = subject.submit(address_delivery_method: 'nonsense')

expect(response).to be_a(FormResponse)
expect(response.success?).to eq(false)
end
end

context 'when delivery method is blank' do
it 'submits with error' do
response = subject.submit(address_delivery_method: '')

expect(response).to be_a(FormResponse)
expect(response.success?).to eq(false)
end
end
end
end
14 changes: 11 additions & 3 deletions spec/support/features/idv_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def fill_out_idv_previous_address_ok
fill_in 'profile_prev_address1', with: '456 Other Ave'
fill_in 'profile_prev_city', with: 'Elsewhere'
select 'Missouri', from: 'profile_prev_state'
fill_in 'profile_prev_zipcode', with: '12345'
fill_in 'profile_prev_zipcode', with: '64000'
end

def fill_out_idv_previous_address_fail
Expand Down Expand Up @@ -68,11 +68,19 @@ def click_idv_continue
end

def click_idv_address_choose_phone
click_link t('idv.buttons.activate_by_phone')
# we're capturing the click on the label element via the unique "for" attribute
Copy link
Member

Choose a reason for hiding this comment

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

same comment on comments 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should keep it here, because the click handler is a bit unique. There are multiple elements within the label, and clicking the label itself is the only thing that matches how the CSS is actually setting the hidden input to true. Since it's both not immediately clear and a relatively unfamiliar pattern, I think it's worth commenting.

Choose a reason for hiding this comment

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

Having a slack conversation about giving the user an error if nothing is chosen: https://gsa-tts.slack.com/archives/C0NGESUN5/p1511195802000213

Choose a reason for hiding this comment

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

@tbaxter-18f and I just chatted offline and decided that verification via text should be pre-selected as the vast majority of folks will want to choose that. That eliminates the need to provide an error if nothing is selected.

# which matches against the radio button's ID,
# so that we can capture any click within the label.
find("label[for='address_delivery_method_phone']").click
click_on t('forms.buttons.continue')
end

def click_idv_address_choose_usps
click_link t('idv.buttons.activate_by_mail')
# we're capturing the click on the label element via the unique "for" attribute
# which matches against the radio button's ID,
# so that we can capture any click within the label.
find("label[for='address_delivery_method_usps']").click
click_on t('forms.buttons.continue')
end

def choose_idv_otp_delivery_method_sms
Expand Down
2 changes: 1 addition & 1 deletion spec/support/idv_examples/max_attempts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
expect(current_path).to eq verify_session_result_path
end

scenario 'fincance shows failure flash message after max attempts', :email do
scenario 'finance shows failure flash message after max attempts', :email do
Copy link
Member

Choose a reason for hiding this comment

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

nice

visit_idp_from_sp_with_loa3(sp)
register_user

Expand Down
1 change: 0 additions & 1 deletion spec/support/idv_examples/usps_verification_selection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
click_idv_continue

click_idv_address_choose_usps

click_on t('idv.buttons.mail.send')

expect(current_path).to eq verify_review_path
Expand Down