Skip to content

LG-7449, Add first error page for inherited proofing when no information is received from API call. #7161

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.

Sign up for GitHub

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

Merged
rnagilla-gsa merged 5 commits into from
Oct 19, 2022
Merged

LG-7449, Add first error page for inherited proofing when no information is received from API call. #7161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fdf8afa
LG-7449 Add first error page for inherited proofing when there is no …
rnagilla-gsa Oct 18, 2022
dc07abc
changelog: Upcoming Features,InheritedProofing,AddFirstErrorPage
rnagilla-gsa Oct 18, 2022
cfd07cd
fix lint and normalize yml
rnagilla-gsa Oct 18, 2022
5e922c3
add placeholder method, before filter and remove h2
rnagilla-gsa Oct 18, 2022
af997e3
remove unsed translations
rnagilla-gsa Oct 18, 2022
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
6 changes: 6 additions & 0 deletions app/controllers/idv/inherited_proofing_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module Idv
class InheritedProofingController < ApplicationController
before_action :confirm_two_factor_authenticated

include Flow::FlowStateMachine
include IdvSession
Comment on lines +3 to 6
Copy link
Copy Markdown
Contributor

@aduth aduth Oct 19, 2022

Choose a reason for hiding this comment

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

We may be able to resolve this issue by reordering the includes instead, since IdvSession is already checking that the user is authenticated, but it's not being reached since FlowStateMachine is where the error is happening.

Suggested change
before_action :confirm_two_factor_authenticated
include Flow::FlowStateMachine
include IdvSession
include IdvSession
include Flow::FlowStateMachine

include InheritedProofing404Concern
Expand All @@ -15,5 +17,9 @@ class InheritedProofingController < ApplicationController
def return_to_sp
redirect_to return_to_sp_failure_to_proof_url(step: next_step, location: params[:location])
end

# for errors/no_information
def no_information
end
end
end
24 changes: 24 additions & 0 deletions app/views/idv/inherited_proofing/no_information.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<%= render(
'idv/shared/error',
type: :warning,
title: t('inherited_proofing.errors.cannot_retrieve.title'),
heading: t('inherited_proofing.errors.cannot_retrieve.heading'),
) do %>
<p>
<%= t('inherited_proofing.errors.cannot_retrieve.info') %>
</p>

<%= link_to t('inherited_proofing.buttons.try_again'), root_url, class: 'usa-button usa-button--big usa-button--wide' %>
Copy link
Copy Markdown
Contributor

@aduth aduth Oct 18, 2022

Choose a reason for hiding this comment

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

Should the "Try again" be going to a page for the user to try again, rather than the root URL?

Copy link
Copy Markdown
Contributor Author

@rnagilla-gsa rnagilla-gsa Oct 18, 2022

Choose a reason for hiding this comment

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

@aduth Yes, that work(page which does asynchronous api call) is still in progress and this is a placeholder for now.


<%= render(
'shared/troubleshooting_options',
heading: t('components.troubleshooting_options.default_heading'),
options: [
{
url: 'https://www.va.gov/resources/managing-your-vagov-profile/',
text: t('inherited_proofing.troubleshooting.options.get_va_help'),
new_tab: true,
},
].select(&:present?),
Copy link
Copy Markdown
Contributor

@aduth aduth Oct 19, 2022

Choose a reason for hiding this comment

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

There's only one option, and it's not conditional, so I don't think we need the filtering here.

Suggested change
].select(&:present?),
],

) %>
<% end %>
2 changes: 0 additions & 2 deletions app/views/idv/inherited_proofing/phone.html.erb
View file
Open in desktop

This file was deleted.

7 changes: 7 additions & 0 deletions config/locales/inherited_proofing/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ en:
inherited_proofing:
buttons:
continue: Continue
try_again: Try again
cancel:
actions:
keep_going: No, keep going
Expand All @@ -19,6 +20,12 @@ en:
instructions:
switch_back: Switch back to your computer to finish verifying your identity.
switch_back_image: Arrow pointing from phone to computer
errors:
cannot_retrieve:
heading: We could not retrieve your information from My HealtheVet
info: We are temporarily having trouble retrieving your information. Please try
again.
title: Couldn’t retrieve information
headings:
lets_go: How verifying your identity works
retrieval: We are retrieving your information from My HealtheVet
Expand Down
7 changes: 7 additions & 0 deletions config/locales/inherited_proofing/es.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ es:
inherited_proofing:
buttons:
continue: Continuar
try_again: Inténtelo de nuevo
cancel:
actions:
keep_going: No, continuar
Expand All @@ -20,6 +21,12 @@ es:
switch_back: Regrese a su computadora para continuar con la verificación de su
identidad.
switch_back_image: Flecha que apunta del teléfono a la computadora
errors:
cannot_retrieve:
heading: No hemos podido obtener su información de My HealtheVet
info: Estamos teniendo problemas temporalmente para obtener su información.
Inténtelo de nuevo.
title: to be implemented
headings:
lets_go: to be implemented
retrieval: Estamos recuperando su información de My HealtheVet
Expand Down
7 changes: 7 additions & 0 deletions config/locales/inherited_proofing/fr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ fr:
inherited_proofing:
buttons:
continue: Continuer
try_again: Réessayez
cancel:
actions:
keep_going: Non, continuer
Expand All @@ -21,6 +22,12 @@ fr:
switch_back: Retournez sur votre ordinateur pour continuer à vérifier votre
identité.
switch_back_image: Flèche pointant du téléphone vers l’ordinateur
errors:
cannot_retrieve:
heading: Nous n’avons pas pu récupérer vos informations dans My HealtheVet
info: Nous avons temporairement des difficultés à récupérer vos informations.
Veuillez réessayer.
title: to be implemented
headings:
lets_go: to be implemented
retrieval: Nous récupérons vos informations depuis My HealtheVet.
Expand Down
1 change: 0 additions & 1 deletion config/locales/step_indicator/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ en:
secure_account: Secure your account
verify_id: Verify your ID
verify_info: Verify your personal details
verify_phone: Verify phone
verify_phone_or_address: Verify phone or address
status:
complete: Completed
Expand Down
1 change: 0 additions & 1 deletion config/locales/step_indicator/es.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ es:
secure_account: Proteje tu cuenta
verify_id: Verifica tu identificación
verify_info: Verifica tus datos personales
verify_phone: to implement
verify_phone_or_address: Verifica el teléfono o dirección
status:
complete: Completo
Expand Down
1 change: 0 additions & 1 deletion config/locales/step_indicator/fr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ fr:
secure_account: Sécurisez votre compte
verify_id: Vérifier votre identité
verify_info: Vérifier vos données personnelles
verify_phone: to implement
verify_phone_or_address: Vérifier le téléphone ou l’adresse
status:
complete: Terminé
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@
get '/:step' => 'inherited_proofing#show', as: :step
put '/:step' => 'inherited_proofing#update'
get '/return_to_sp' => 'inherited_proofing#return_to_sp'
get '/errors/no_information' => 'inherited_proofing#no_information'
Copy link
Copy Markdown
Contributor

@aduth aduth Oct 18, 2022

Choose a reason for hiding this comment

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

Even if there's no logic associated with the route, conventionally it's preferred to at least include an empty method for the action in the controller, so that it's more obvious what actions exist when looking at the controller implementation.

class InheritedProofingController
  # ...

  def no_information; end
end

Copy link
Copy Markdown
Contributor

@aduth aduth Oct 18, 2022

Choose a reason for hiding this comment

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

I tried testing this locally by going directly to http://localhost:3000/verify/inherited_proofing/errors/no_information, only to see a 500 error undefined method uuid' for nil:NilClass`. It seems that we need to be adding some validation in the base controller to avoid 500.

Copy link
Copy Markdown
Contributor Author

@rnagilla-gsa rnagilla-gsa Oct 18, 2022

Choose a reason for hiding this comment

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

@aduth I actually first added that method to controller and then removed it thinking that preferred. Ex. no_camera action on doc_auth/errors . I will re add it.

Copy link
Copy Markdown
Contributor Author

@rnagilla-gsa rnagilla-gsa Oct 18, 2022

Choose a reason for hiding this comment

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

We have a future tasks for adding before_filters but I added before_action :confirm_two_factor_authenticated to redirect away from that

Copy link
Copy Markdown
Contributor

@aduth aduth Oct 19, 2022

Choose a reason for hiding this comment

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

With a nested route like this, typically I'd expect this to be broken out to a separate controller, rather than funneling all the routes through a single controller (e.g. InheritedProofingErrorsController).

Copy link
Copy Markdown
Contributor Author

@rnagilla-gsa rnagilla-gsa Oct 19, 2022

Choose a reason for hiding this comment

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

We probably will move this to the other ErrorController and Error handling work that Jeff is working on.

Copy link
Copy Markdown
Contributor

@aduth aduth Oct 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Can you link me to the related ticket which tracks that work?

Copy link
Copy Markdown
Contributor

@jscodefix jscodefix Oct 19, 2022

Choose a reason for hiding this comment

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

@aduth The error handling methodology mentioned here, in this PR conversation, is being worked in LG-7257. You and I had a discussion about the implementation, on the 7th of this month; specifically about creating InheritedProofingErrorsController. I am waiting for this PR code to be merged, so that I can integrate the error controller.

end

namespace :api do
Expand Down