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 3 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
25 changes: 25 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,25 @@
<%= 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' %>

@aduth aduth Oct 18, 2022

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.

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

@rnagilla-gsa rnagilla-gsa Oct 18, 2022

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.

@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_tag: :h3,

@aduth aduth Oct 18, 2022

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.

We should leave out this option so that the default :h2 heading is used, since otherwise there's an accessibility issue, as the heading order increases from h1 to h3.

https://dequeuniversity.com/rules/axe/3.5/heading-order

Suggested change
heading_tag: :h3,

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?),

@aduth aduth Oct 19, 2022

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.

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 %>
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: 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'

@aduth aduth Oct 18, 2022

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.

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

@aduth aduth Oct 18, 2022

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 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.

@rnagilla-gsa rnagilla-gsa Oct 18, 2022

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.

@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.

@rnagilla-gsa rnagilla-gsa Oct 18, 2022

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.

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

@aduth aduth Oct 19, 2022

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.

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).

@rnagilla-gsa rnagilla-gsa Oct 19, 2022

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.

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

@aduth aduth Oct 19, 2022 •
edited
Loading

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.

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

@jscodefix jscodefix Oct 19, 2022

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.

@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