Skip to content

LG-6139: Handle and display password confirm errors#6337

Merged
aduth merged 7 commits intomainfrom
aduth-lg-6139-password-errors
May 13, 2022
Merged

LG-6139: Handle and display password confirm errors#6337
aduth merged 7 commits intomainfrom
aduth-lg-6139-password-errors

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented May 11, 2022

Why: So that the user only proceeds to the next step with a valid submission, and so that they know what they need to do to correct the issue.

Testing Instructions:

  1. Set idv_api_enabled_steps: '["password_confirm","personal_key","personal_key_confirm"]' in local config/application.yml
  2. Sign in
  3. Go to http://localhost:3000/verify
  4. Complete proofing flow up to password confirmation page
  5. Submit the page with an empty field, or an incorrect password
  6. Observe that an error alert message is shown

Screenshot:

In the screenshot below, note that "Before" shows that the application would previously crash, due to assumptions that the API request would have been successful.

Before After
Screen Shot 2022-05-11 at 11 31 58 AM Screen Shot 2022-05-11 at 11 31 35 AM

aduth added 3 commits May 11, 2022 11:18
**Why**: The point of the `type` property is that if/when we log details around the error, the types are uniquely identifying the sort of error that occurred. They should be unique from one another, and locale-independent.
So we can show them in the UI
**Why**: So that the user only proceeds to the next step with a valid submission, and so that they know what they need to do to correct the issue.

changelog: Upcoming Features, Identity Verification, Add password confirmation step
@aduth aduth requested a review from a team May 11, 2022 19:08
aduth added 3 commits May 11, 2022 15:58
This existed previously because the base FormError customized arguments of the constructor to receive only an options object, not a message. Since the base constructor now receives message as the first argument, the override can be removed.
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,3 +1,7 @@
export interface ErrorResponse<Field extends string> {
error: Record<Field, [string, ...string[]]>;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this this [string, ...string[]] type a way to say "string[] with length at least 1"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this this [string, ...string[]] type a way to say "string[] with length at least 1"?

Yes, exactly that. I don't know that it's strictly necessary here, but it seems reasonable to expect that if the response is an error response, there's at least one error message.

Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

LGTM. I updated the description with the correct key (idv_api_enabled_steps vs idv_api_steps_enabled and it worked as described.

rescue ::Api::UserBundleError => err
errors.add(:jwt, "malformed user bundle: #{err.message}", type: :invalid)
rescue JWT::DecodeError
errors.add(:jwt, I18n.t('idv.failure.exceptions.internal_error'), type: :decode_error)
Copy link
Contributor

Choose a reason for hiding this comment

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

🙂

@aduth aduth merged commit 877665b into main May 13, 2022
@aduth aduth deleted the aduth-lg-6139-password-errors branch May 13, 2022 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants