Skip to content

LG-9459: Accessing phone error screens should redirect you if you haven't entered your phone yet#8893

Merged
amirbey merged 16 commits intomainfrom
amirbey/LG-9459-phone-errors-redirect-unsubmitted
Aug 1, 2023
Merged

LG-9459: Accessing phone error screens should redirect you if you haven't entered your phone yet#8893
amirbey merged 16 commits intomainfrom
amirbey/LG-9459-phone-errors-redirect-unsubmitted

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Jul 28, 2023

🎫 Ticket

LG-9459

🛠 Summary of changes

Add before action to phone_errors controller to verify that a user has submitted a phone number before rendering error page.

📜 Testing Plan

  • start identity verification and complete any number steps before phone verification
  • observe the current step (ie: welcome, agreement, doc_auth, etc.)
  • visit a phone errors page (ie: http://example.com/verify/phone/errors/warning in the browser)
  • verify being redirected to the step observed above

repeat the steps ensuring that all phone errors pages have been tested:
http://example.com/verify/phone/errors/warning
http://example.com/verify/phone/errors/failure
http://example.com/verify/phone/errors/jobfail

@amirbey amirbey self-assigned this Jul 28, 2023
@amirbey amirbey changed the title Amirbey/lg 9459 phone errors redirect unsubmitted LG-9459: Accessing phone error screens should redirect you if you haven't entered your phone yet Jul 28, 2023
@amirbey amirbey requested a review from a team July 28, 2023 21:04
@amirbey amirbey marked this pull request as ready for review July 28, 2023 21:04
@amirbey amirbey removed the request for review from a team July 29, 2023 00:54
@amirbey amirbey marked this pull request as draft July 29, 2023 00:54
@amirbey amirbey requested a review from a team July 31, 2023 14:31
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting idea to use RateLimiter! We've been checking idv_session for statuses like this. Do we want to adopt RateLimiter as a new pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right 😅 ... i have reimplemented this using the idv_session ...thanks @soniaconnolly 👍🏿

@amirbey amirbey requested a review from soniaconnolly July 31, 2023 20:37
@amirbey amirbey marked this pull request as ready for review July 31, 2023 22:15
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

Implementation looks good! Some stray code left over.

@amirbey amirbey force-pushed the amirbey/LG-9459-phone-errors-redirect-unsubmitted branch from 5403e3f to eae32e9 Compare August 1, 2023 13:21
@amirbey amirbey requested a review from soniaconnolly August 1, 2023 13:28
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGTM!

@amirbey amirbey merged commit 5a9f5fe into main Aug 1, 2023
@amirbey amirbey deleted the amirbey/LG-9459-phone-errors-redirect-unsubmitted branch August 1, 2023 15:15
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.

2 participants