Skip to content

LG-7401 Add spinner button for VerifyInfoController#show#7686

Merged
soniaconnolly merged 5 commits intomainfrom
sonia-lg-7401-rebased
Jan 25, 2023
Merged

LG-7401 Add spinner button for VerifyInfoController#show#7686
soniaconnolly merged 5 commits intomainfrom
sonia-lg-7401-rebased

Conversation

@soniaconnolly
Copy link
Contributor

🎫 Ticket

LG-7401

🛠 Summary of changes

Add the spinner button to verify_info and tests for throttling by ssn and resolution.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Confirm that doc_auth_verify_info_controller_enabled flag is false
  • Run through remote proofing, confirm unchanged
  • Set doc_auth_verify_info_controller_enabled flag to true
  • Run through remote proofing, will see /verify/verify_info url in browser for Verify step
  • Screens should otherwise remain unchanged
  • Confirm other proofing flows also unchanged

eric-gade and others added 5 commits January 23, 2023 15:03
Also adding the wait template

Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov>

[skip changelog]
A separate PR will use this template from other code paths
changelog: Internal, Flow State Machine, add spinner button to VerifyInfo path
changelog: Internal, Flow State Machine, add spinner button to VerifyInfo path
@soniaconnolly soniaconnolly requested review from a team and jmhooper January 24, 2023 19:10
@soniaconnolly soniaconnolly marked this pull request as ready for review January 24, 2023 20:04
analytics.idv_doc_auth_verify_visited(**analytics_arguments)

redirect_to failure_url(:fail) and return if any_throttled?
redirect_to throttled_url and return if any_throttled?
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I am mistaken, we still need to add some throttle logic to either the update action or move this into a before action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that. I looked at adding a before action, noticed that we redirect to different error pages at different times because of throttles, and then got side tracked. Will follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On further looking at code, it looks like throughout the proofing flow, throttling doesn't get checked at the beginning of update/create actions. I wonder if we can address that as a bug separately from removing verify from FSM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a commit on a separate branch and we can decide if we want to pull it in to this branch. c5c474b

Copy link
Contributor

Choose a reason for hiding this comment

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

I am totally fine with a different PR. We can open a bug ticket if we need. I do think we should fix it before deploying this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened a ticket. Checking that by "before deploying" you meant before LG-7402. I'd like to merge this for tomorrow because I have another branch that depends on it. https://cm-jira.usa.gov/browse/LG-8742

Copy link
Contributor

@jmhooper jmhooper left a comment

Choose a reason for hiding this comment

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

Approving with the understanding that we will address the throttle comment in a separate change request

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