Conversation
jmhooper
force-pushed
the
jmhooper-use-enter-password-url
branch
from
3288e75 to
40f4e2e
Compare
soniaconnolly
left a comment
soniaconnolly
left a comment
There was a problem hiding this comment.
LGTM. I tried this out and it works for either route.
I was surprised that going to /verify/review didn't redirect to verify/enter_password. Do you want to add those redirects?
There was a problem hiding this comment.
As far as this PR goes, this is fine. But this code is surprising. Do we really want to redirect out of the RequestLetterController to the EnterPasswordController if the user has sent too many letters? Seems like that would be unexpected, especially if they're rate limited at the Phone step.
There was a problem hiding this comment.
For this PR, this is fine. But it brings up the question, does it still make sense to redirect to EnterPassword here, rather than to Phone step? I think this is left over from the Flow State Machine.
The change in #9375 renamed the review controller to the "enter password" controller. This commit introduced a new path, but did not start using it to support the 50/50 state when that change was deployed. This commit starts using the new path but does not remove the old ones. This is also to prevent 404s in the 50/50 state. A follow up will be needed to remove the old routes after this is deployed. [skip changelog]
jmhooper
force-pushed
the
jmhooper-use-enter-password-url
branch
from
40f4e2e to
dec2364
Compare
In #9411 we moved all traffic from the old `/verify/review` path to the new `/verify/enter_password` path. We left the old paths around so that users getting redirected by old instances would not encounter a 404 error. Once the changes in #9411 has been deployed for a day there should not be any more users visitting `/verify/review` and we can merge this change to remove it. [skip changelog]
In #9411 we moved all traffic from the old `/verify/review` path to the new `/verify/enter_password` path. We left the old paths around so that users getting redirected by old instances would not encounter a 404 error. Once the changes in #9411 has been deployed for a day there should not be any more users visitting `/verify/review` and we can merge this change to remove it. [skip changelog]
2 participants
The change in #9375 renamed the review controller to the "enter password" controller. This commit introduced a new path, but did not start using it to support the 50/50 state when that change was deployed.
This commit starts using the new path but does not remove the old ones. This is also to prevent 404s in the 50/50 state. A follow up will be needed to remove the old routes after this is deployed.