Merged
Conversation
Contributor
Author
|
Screenshots:
|
Contributor
Author
|
Looks like merging master into my branch melted on Travis |
Contributor
Author
|
It was a bug in the code that I used to fix the reek issue in the |
zachmargolis
approved these changes
Aug 11, 2017
Contributor
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM, one question about naming but not a blocker
Contributor
There was a problem hiding this comment.
prefered_delivery_method seems a little weird to me in that it's different from the current_user.otp_delivery_preference ?
Maybe this should be selected_delivery_method or chosen_delivery_method ?
Contributor
Author
There was a problem hiding this comment.
Good point. Commit to change it pushed just now
jmhooper
force-pushed
the
jmhooper-idv-otp-delivery-method-selection
branch
from
6f41905 to
3132e46
Compare
**Why**: Prior to this commit, the OTP delivery method was the user's preference for their 2FA phone. The users phone number of record may not have the same capabilities as the user's 2FA phone. This commit allows the user to select a delivery method that is different than their 2FA OTP delivery method. Add context awareness to OTP selection form **Why**: We don't want the OTP delivery selection form to update the user's OTP delivery method preference in the IdV context since that is a one-time thing. This has the additional benefit of letting us refactor controllers so they can use the analytics params directly from the form instead of having to merge in the context. Add delivery method arg to phone confirmation **Why**: On screens where the user selects a delivery method that may not correspond to their delivery preference (e.g. IdV phone confirmation), then we want to convey their delivery preference to the `prommpt_to_confirm` method so that it can use that delivery method if it is available. Disable voice OTP in IdV when it's unsupported **Why**: If the user enters a phone that we can't voice call, we want to disable the option to make it clear that it is unavailable.
jmhooper
force-pushed
the
jmhooper-idv-otp-delivery-method-selection
branch
from
3132e46 to
1961656
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow OTP delivery method selection during IdV
Why: Prior to this commit, the OTP delivery method was the user's
preference for their 2FA phone. The users phone number of record may not
have the same capabilities as the user's 2FA phone. This commit allows
the user to select a delivery method that is different than their 2FA
OTP delivery method.
Disable voice OTP in IdV when it's unsupported
Why: If the user enters a phone that we can't voice call, we want to
disable the option to make it clear that it is unavailable.
Add delivery method arg to phone confirmation
Why: On screens where the user selects a delivery method that may
not correspond to their delivery preference (e.g. IdV phone
confirmation), then we want to convey their delivery preference to the
prommpt_to_confirmmethod so that it can use that delivery method ifit is available.
Add context awareness to OTP selection form
Why: We don't want the OTP delivery selection form to update the
user's OTP delivery method preference in the IdV context since that is a
one-time thing. This has the additional benefit of letting us refactor
controllers so they can use the analytics params directly from the form
instead of having to merge in the context.