Skip to content

Comments

LG-9866 remove route to 404 pg#8499

Merged
svalexander merged 8 commits intomainfrom
shannon/lg-9866-remove-404
Jun 13, 2023
Merged

LG-9866 remove route to 404 pg#8499
svalexander merged 8 commits intomainfrom
shannon/lg-9866-remove-404

Conversation

@svalexander
Copy link
Contributor

@svalexander svalexander commented May 26, 2023

🎫 Ticket

LG-9866

🛠 Summary of changes

Remove 404 before action for verify info page.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Go through ipp flow
  • After arriving at /verify (the verify info page) change url to /verify_info
  • Confirm that verify info shows/404 page is not visible

@svalexander svalexander marked this pull request as draft May 26, 2023 21:30
module Actions
module InPerson
class CancelUpdateAddressAction < Idv::Steps::DocAuthBaseStep
include Idv::Steps::TempMaybeRedirectToVerifyInfoHelper
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@soniaconnolly should this be removed in conjunction with the removal of the 404 before action?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it looks like you don't want to delete this until the final PR that deletes the feature flag. If then. Looks like it's managing when a step redirects to the in person VerifyInfoController. I would make a separate PR to replace this with a before_action or similar mechanism once the in person VerifyInfoController is completely live in production, after the feature flag is deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I'll leave this removal out of this pr then

@svalexander svalexander requested a review from gina-yamada May 30, 2023 21:15
@gina-yamada
Copy link
Contributor

@svalexander I think your code changes are good. We just need to solidify our process to know when we want to merge this into main (or another env for testing)- and how/when this will get promoted.

Copy link
Contributor

@gina-yamada gina-yamada 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 would pull in main and make sure branch is up to date before merging

UPDATE 06-12-23 @svalexander I found some bugs with analytics. Please wait to merge this PR. I will give you a green light when we are ready! Thanks

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

@svalexander svalexander marked this pull request as ready for review June 13, 2023 21:46
@svalexander svalexander changed the title DO NOT MERGE: LG-9866 remove route to 404 pg LG-9866 remove route to 404 pg Jun 13, 2023
@svalexander svalexander merged commit aad9950 into main Jun 13, 2023
@svalexander svalexander deleted the shannon/lg-9866-remove-404 branch June 13, 2023 21:46
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