Skip to content

LG-7450 Implement "Try Again" Button Functionality on the First Error Page#7284

Merged
gangelo merged 5 commits intomainfrom
lg-7450-implement-try-again-button-functionality-on-the-first-error-page
Nov 23, 2022
Merged

LG-7450 Implement "Try Again" Button Functionality on the First Error Page#7284
gangelo merged 5 commits intomainfrom
lg-7450-implement-try-again-button-functionality-on-the-first-error-page

Conversation

@gangelo
Copy link
Contributor

@gangelo gangelo commented Nov 3, 2022 •
edited
Loading

Display and implement a "Try again" button so that an Inherited Proofing user can attempt to retrieve their PII from their Service Provider a second time, if the first attempt fails.

🎫 Ticket

https://cm-jira.usa.gov/browse/LG-7450

🛠 Summary of changes

  • Add "Try again" button to the "We could not retrieve your information from..." UI. Allows user to try a second time to retrieve their user PII from their respective service provider. IP forces the user to acknowledge (checkbox) retrieval of their PII from the service provider in one step, and retrieves the PII in a subsequent ajax/async step. Subsequent retries do not have to be acknowledged (checkbox).
  • Implement error handling and bubbling up from service to form class for VA if errors are encountered during the request or errors encountered decrypting the payload returned from the service provider.
  • Implement throttling for PII retrieval from the service provider to 2 tries, and a reattempt window of 1 minute.
  • Implement automated tests.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Automated testing.
  • Manual testing in the UI using Sinatra app.

👀 Screenshots

Before: There are no before images.
After:

image
image

🚀 Notes for Deployment

N/A

@gangelo gangelo added the inherited proofing Pull Requests for the Inherited Proofing feature label Nov 3, 2022
@gangelo gangelo self-assigned this Nov 3, 2022
@gangelo gangelo force-pushed the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch 7 times, most recently from b9af9d0 to 82e6d77 Compare November 4, 2022 20:47
gangelo
gangelo commented Nov 5, 2022
View reviewed changes
gangelo
gangelo commented Nov 5, 2022
View reviewed changes
@gangelo gangelo force-pushed the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch 6 times, most recently from c2a5103 to f13a88a Compare November 7, 2022 19:40
gangelo
gangelo commented Nov 7, 2022
View reviewed changes
gangelo
gangelo commented Nov 7, 2022
View reviewed changes
@gangelo gangelo force-pushed the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch 11 times, most recently from 49577f2 to 46e6402 Compare November 10, 2022 13:04
@gangelo gangelo force-pushed the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch 7 times, most recently from 23c7639 to 070a5c1 Compare November 16, 2022 14:07
jmhooper
jmhooper reviewed Nov 16, 2022
View reviewed changes
@gangelo gangelo force-pushed the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch 4 times, most recently from c27c007 to ef1a7e4 Compare November 21, 2022 17:22
zachmargolis
zachmargolis reviewed Nov 21, 2022
View reviewed changes
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.

I think this PR is mostly good to go, some small suggestions, but I'd like to see this dynamic accessor stuff cleaned up or a ticket filed to clean it up before I approve

@gangelo
Implement try again logic for Inherited Proofing
4f6bc14 
changelog: Upcoming Features, Inherited Proofing, LG-7450 Implement "Try Again" Button Functionality on the First Error Page

Refactor code to enqueue user PII job to mixin

- To keep things DRY as this will be used in the verify_wait
step when the user clicks the "Retry" button on the
IP Warning UI.

Refactor to use UserPiiJobInitiator mixin

Add translations for request/response errors

- Related to inherited proofing service provider requests and responses;
this, so that we can 1) record the error and 2) display something to
the user. Errors displayed to the user are "safe" errors, revealing
no information that could be used by those having ill intentions.

Refactor VA form and service classes

- To handle network errors encountered during the request to
the service provider and decrypting response data returned
from the service provider request.

Allow va mock proofer to return meaningful error

- So that we can identify the error in the va Form class as such
and notify our Flow that an error has indeed occured. The error
added to the mock proofer is a network-related error, not a
va api related (returned in a response) error.

Enable flow step to hook into "try again" logic

- Which exists in the base class.

Enable flow step to hook into "try again" logic

DRY VerifyBaseStep for Inherited Proofing usage

Set up throttling for Inherited Proofing

Change the path the "Try again" button uses

- To point to the :verify_wait flow step so the user can try again.

Automated tests

Refactor inherited_proofing_cancel_spec.rb

- To use InheritedProofingWithServiceProviderHelper.
@gangelo
Address PR feedback
e607c9c 
- Remove unnecessary call to super in FormBase class.
- Remove #extra method in FormBase.
- Rename add_service_error_if method to add_service_error and add
conditional callback if: :service_error?
- Fix spec to check actual :service_error hasn key value instead of
converting the hash to a string and checking that.
@gangelo
Add missing failure method to BaseFlow
9297dc8 
- This seemed a better option than making BaseFlow an abstract class raising
NoMethodError, since it looks like the logic would be the same anyhow.
@gangelo
Create step action to retry user pii retrieval
4ea5e57
@gangelo gangelo force-pushed the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch from ef1a7e4 to 4ea5e57 Compare November 23, 2022 11:35
@gangelo gangelo requested a review from zachmargolis November 23, 2022 13:15
@gangelo gangelo merged commit 6acbb30 into main Nov 23, 2022
@gangelo gangelo deleted the lg-7450-implement-try-again-button-functionality-on-the-first-error-page branch November 23, 2022 17:16
@mdiarra3 mdiarra3 mentioned this pull request Nov 28, 2022
Merged
mdiarra3 pushed a commit that referenced this pull request Nov 28, 2022
@gangelo @mdiarra3
LG-7450 Implement "Try Again" Button Functionality on the First Error…
91107eb 
… Page (#7284)

* Implement try again logic for Inherited Proofing

changelog: Upcoming Features, Inherited Proofing, LG-7450 Implement "Try Again" Button Functionality on the First Error Page

Refactor code to enqueue user PII job to mixin

- To keep things DRY as this will be used in the verify_wait
step when the user clicks the "Retry" button on the
IP Warning UI.

Refactor to use UserPiiJobInitiator mixin

Add translations for request/response errors

- Related to inherited proofing service provider requests and responses;
this, so that we can 1) record the error and 2) display something to
the user. Errors displayed to the user are "safe" errors, revealing
no information that could be used by those having ill intentions.

Refactor VA form and service classes

- To handle network errors encountered during the request to
the service provider and decrypting response data returned
from the service provider request.

Allow va mock proofer to return meaningful error

- So that we can identify the error in the va Form class as such
and notify our Flow that an error has indeed occured. The error
added to the mock proofer is a network-related error, not a
va api related (returned in a response) error.

Enable flow step to hook into "try again" logic

- Which exists in the base class.

Enable flow step to hook into "try again" logic

DRY VerifyBaseStep for Inherited Proofing usage

Set up throttling for Inherited Proofing

Change the path the "Try again" button uses

- To point to the :verify_wait flow step so the user can try again.

Automated tests

Refactor inherited_proofing_cancel_spec.rb

- To use InheritedProofingWithServiceProviderHelper.

* Address PR feedback

- Remove unnecessary call to super in FormBase class.
- Remove #extra method in FormBase.
- Rename add_service_error_if method to add_service_error and add
conditional callback if: :service_error?
- Fix spec to check actual :service_error hasn key value instead of
converting the hash to a string and checking that.

* Add missing failure method to BaseFlow

- This seemed a better option than making BaseFlow an abstract class raising
NoMethodError, since it looks like the logic would be the same anyhow.

* Create step action to retry user pii retrieval

* Address latest PR feedback
This was referenced Nov 29, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduth aduth Awaiting requested review from aduth

@jmhooper jmhooper Awaiting requested review from jmhooper

1 more reviewer

@zachmargolis zachmargolis zachmargolis approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@gangelo gangelo

Labels

inherited proofing Pull Requests for the Inherited Proofing feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gangelo @zachmargolis @jmhooper