-
Notifications
You must be signed in to change notification settings - Fork 166
LG-14010 - More detailed error page for Socure errors #11560
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
Changes from all commits
e5ee223
a90c103
295832a
672446c
0055ba2
cfe26d8
1dcb117
ed93a8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,7 @@ def handle_stored_result(user: current_user, store_in_session: true) | |||||||||||||||||||||||
| successful_response | ||||||||||||||||||||||||
| else | ||||||||||||||||||||||||
| extra = { stored_result_present: stored_result.present? } | ||||||||||||||||||||||||
| failure(I18n.t('doc_auth.errors.general.network_error'), extra) | ||||||||||||||||||||||||
| failure(nil, extra) | ||||||||||||||||||||||||
| end | ||||||||||||||||||||||||
| end | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -22,13 +22,25 @@ def successful_response | |||||||||||||||||||||||
| end | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # copied from Flow::Failure module | ||||||||||||||||||||||||
| def failure(message, extra = nil) | ||||||||||||||||||||||||
| flash[:error] = message | ||||||||||||||||||||||||
| form_response_params = { success: false, errors: { message: message } } | ||||||||||||||||||||||||
| def failure(message = nil, extra = nil) | ||||||||||||||||||||||||
| form_response_params = { success: false } | ||||||||||||||||||||||||
| form_response_params[:errors] = make_error_hash(message) | ||||||||||||||||||||||||
| form_response_params[:extra] = extra unless extra.nil? | ||||||||||||||||||||||||
| FormResponse.new(**form_response_params) | ||||||||||||||||||||||||
| end | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def make_error_hash(message) | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
| Rails.logger.info("make_error_hash: stored_result: #{stored_result.inspect}") | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we intending to log this? |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| error_hash = { message: message || I18n.t('doc_auth.errors.general.network_error') } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if stored_result&.errors&.has_key?(:socure) | ||||||||||||||||||||||||
| error_hash[:socure] = stored_result.errors[:socure] | ||||||||||||||||||||||||
| end | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| error_hash | ||||||||||||||||||||||||
|
Comment on lines
+35
to
+41
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
this seems simpler to me, just a suggestion |
||||||||||||||||||||||||
| end | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def extract_pii_from_doc(user, store_in_session: false) | ||||||||||||||||||||||||
| if defined?(idv_session) # hybrid mobile does not have idv_session | ||||||||||||||||||||||||
| idv_session.had_barcode_read_failure = stored_result.attention_with_barcode? | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Idv | ||
| module SocureErrorsConcern | ||
| def errors | ||
| @presenter = socure_errors_presenter(handle_stored_result) | ||
| end | ||
|
|
||
| def goto_in_person | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. having the controller action inside of the concern feels a bit buried 🤔 i didn't realize this was a controller action until I saw the routes file 😬 verify_info_controller#update has a pattern worth considering |
||
| InPersonEnrollment.find_or_initialize_by( | ||
| user: document_capture_session.user, | ||
| status: :establishing, | ||
| sponsor_id: IdentityConfig.store.usps_ipp_sponsor_id, | ||
| ).save! | ||
|
|
||
| redirect_to idv_in_person_url | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def remaining_attempts | ||
| RateLimiter.new( | ||
| user: document_capture_session.user, | ||
| rate_limit_type: :idv_doc_auth, | ||
| ).remaining_count | ||
| end | ||
|
|
||
| def error_code_for(result) | ||
| if result.errors[:socure] | ||
| result.errors.dig(:socure, :reason_codes).first | ||
| elsif result.errors[:network] | ||
| :network | ||
| else | ||
| # No error information available (shouldn't happen). Default | ||
| # to :network if it does. | ||
| :network | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class SocureErrorPresenter | ||
| include Rails.application.routes.url_helpers | ||
| include ActionView::Helpers::UrlHelper | ||
| include ActionView::Helpers::TranslationHelper | ||
| include LinkHelper | ||
|
|
||
| attr_reader :url_options | ||
|
|
||
| def initialize(error_code:, remaining_attempts:, sp_name:, hybrid_mobile:) | ||
| @error_code = error_code | ||
| @remaining_attempts = remaining_attempts | ||
| @sp_name = sp_name | ||
| @hybrid_mobile = hybrid_mobile | ||
| @url_options = {} | ||
| end | ||
|
|
||
| def heading | ||
| heading_string_for(error_code) | ||
| end | ||
|
|
||
| def body_text | ||
| error_string_for(error_code) | ||
| end | ||
|
|
||
| def rate_limit_text | ||
| if remaining_attempts == 1 | ||
| t('doc_auth.rate_limit_warning.singular_html') | ||
| else | ||
| t('doc_auth.rate_limit_warning.plural_html', remaining_attempts: remaining_attempts) | ||
| end | ||
| end | ||
|
|
||
| def action | ||
| url = hybrid_mobile ? idv_hybrid_mobile_socure_document_capture_path | ||
| : idv_socure_document_capture_path | ||
| { | ||
| text: I18n.t('idv.failure.button.warning'), | ||
| url: url, | ||
| } | ||
| end | ||
|
|
||
| def secondary_action_heading | ||
| I18n.t('in_person_proofing.headings.cta') | ||
| end | ||
|
|
||
| def secondary_action_text | ||
| I18n.t('in_person_proofing.body.cta.prompt_detail') | ||
| end | ||
|
|
||
| def secondary_action | ||
| url = hybrid_mobile ? idv_hybrid_mobile_socure_document_capture_goto_in_person_path | ||
| : idv_socure_document_capture_goto_in_person_path | ||
|
|
||
| { | ||
| text: I18n.t('in_person_proofing.body.cta.button'), | ||
| url: url, | ||
| } | ||
| end | ||
|
|
||
| def troubleshooting_heading | ||
| I18n.t('components.troubleshooting_options.ipp_heading') | ||
| end | ||
|
|
||
| def options | ||
| [ | ||
| { | ||
| url: help_center_redirect_path( | ||
| category: 'verify-your-identity', | ||
| article: 'how-to-add-images-of-your-state-issued-id', | ||
| ), | ||
| text: I18n.t('idv.troubleshooting.options.doc_capture_tips'), | ||
| isExternal: true, | ||
| }, | ||
| { | ||
| url: help_center_redirect_path( | ||
| category: 'verify-your-identity', | ||
| article: 'accepted-identification-documents', | ||
| ), | ||
| text: I18n.t('idv.troubleshooting.options.supported_documents'), | ||
| isExternal: true, | ||
| }, | ||
| { | ||
| url: return_to_sp_failure_to_proof_url(step: 'document_capture'), | ||
| text: t( | ||
| 'idv.failure.verify.fail_link_html', | ||
| sp_name: sp_name, | ||
| ), | ||
| isExternal: true, | ||
| }, | ||
| ] | ||
| end | ||
|
|
||
| def step_indicator_steps | ||
| Idv::StepIndicatorConcern::STEP_INDICATOR_STEPS | ||
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :error_code, :remaining_attempts, :sp_name, :hybrid_mobile | ||
|
|
||
| SOCURE_ERROR_MAP = { | ||
| 'I848' => 'unreadable_id', | ||
| 'I854' => 'unreadable_id', | ||
| 'R810' => 'unreadable_id', | ||
| 'R820' => 'unreadable_id', | ||
| 'R822' => 'unreadable_id', | ||
| 'R823' => 'unreadable_id', | ||
| 'R824' => 'unreadable_id', | ||
| 'R825' => 'unreadable_id', | ||
| 'R826' => 'unreadable_id', | ||
| 'R831' => 'unreadable_id', | ||
| 'R833' => 'unreadable_id', | ||
| 'R838' => 'unreadable_id', | ||
| 'R859' => 'unreadable_id', | ||
| 'R861' => 'unreadable_id', | ||
| 'R863' => 'unreadable_id', | ||
|
|
||
| 'I849' => 'unaccepted_id_type', | ||
| 'R853' => 'unaccepted_id_type', | ||
| 'R862' => 'unaccepted_id_type', | ||
|
|
||
| 'R827' => 'expired_id', | ||
|
|
||
| 'I808' => 'low_resolution', | ||
|
|
||
| 'R845' => 'underage', | ||
|
|
||
| 'I856' => 'id_not_found', | ||
| 'R819' => 'id_not_found', | ||
| }.freeze | ||
|
|
||
| def remapped_error(error_code) | ||
| SOCURE_ERROR_MAP[error_code] || 'unreadable_id' | ||
| end | ||
|
|
||
| def heading_string_for(error_code) | ||
| if error_code == :network | ||
| t('doc_auth.headers.general.network_error') | ||
| else | ||
| # i18n-tasks-use t('doc_auth.headers.unreadable_id') | ||
| # i18n-tasks-use t('doc_auth.headers.unaccepted_id_type') | ||
| # i18n-tasks-use t('doc_auth.headers.expired_id') | ||
| # i18n-tasks-use t('doc_auth.headers.low_resolution') | ||
| # i18n-tasks-use t('doc_auth.headers.underage') | ||
| # i18n-tasks-use t('doc_auth.headers.id_not_found') | ||
| I18n.t("doc_auth.headers.#{remapped_error(error_code)}") | ||
| end | ||
| end | ||
|
|
||
| def error_string_for(error_code) | ||
| if error_code == :network | ||
| t('doc_auth.errors.general.new_network_error') | ||
| elsif remapped_error(error_code) == 'underage' # special handling because it says 'Login.gov' | ||
| I18n.t('doc_auth.errors.underage', app_name: APP_NAME) | ||
| else | ||
| # i18n-tasks-use t('doc_auth.errors.unreadable_id') | ||
| # i18n-tasks-use t('doc_auth.errors.unaccepted_id_type') | ||
| # i18n-tasks-use t('doc_auth.errors.expired_id') | ||
| # i18n-tasks-use t('doc_auth.errors.low_resolution') | ||
| # i18n-tasks-use t('doc_auth.errors.id_not_found') | ||
| I18n.t("doc_auth.errors.#{remapped_error(error_code)}") | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -38,6 +38,7 @@ class DocvResultResponse < DocAuth::Response | |||
| def initialize(http_response:, | ||||
| biometric_comparison_required: false) | ||||
| @http_response = http_response | ||||
|
|
||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| @biometric_comparison_required = biometric_comparison_required | ||||
| @pii_from_doc = read_pii | ||||
|
|
||||
|
|
@@ -99,7 +100,7 @@ def error_messages | |||
| return {} if successful_result? | ||||
|
|
||||
| { | ||||
| reason_codes: get_data(DATA_PATHS[:reason_codes]), | ||||
| socure: { reason_codes: get_data(DATA_PATHS[:reason_codes]) }, | ||||
| } | ||||
| end | ||||
|
|
||||
|
|
||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it appear that failure is only called once passing message as nil. i understand this is following the pattern for defining the failure method throughout the application; however, could it be worth eliminating this failure method definition and instead of calling it use the below
FormResponse.new(
{
success: false,
errors: make_error_hash(message),
extra: extra}
}.compact
)