Conversation
|
Depends on #885 |
|
Can we get status on this? It needs to be merged so we can resolve #1247 |
|
@jkim1217 The PR this depends on was just approved last week. Someone with write access to the repo needs to merge it. Then I can go through and fix all the merge conflicts in this PR and tag this ready for review |
|
I just merged #885. I didn't know you didn't have merge access. We should fix that right away. I was wondering why you didn't merge it yourself when I said "feel free to squash and merge". I don't get a notification when a squash happens, so if it's ready to merge, please make sure to add a comment on the PR, which should notify folks. |
el-mapache
force-pushed
the
add-totp-presenter
branch
3 times, most recently
from
3276dba to
255763a
Compare
el-mapache
added
status - ready for review
and removed
status - work in progress
labels
**Why**: To consolidate logic of how we show fallback otp links and associated help text to the user
el-mapache
force-pushed
the
add-totp-presenter
branch
2 times, most recently
from
f441e74 to
177b67d
Compare
jessieay
left a comment
jessieay
left a comment
There was a problem hiding this comment.
Couple of small comments for your consideration, but LGTM!
There was a problem hiding this comment.
since we are always using the default, should we just remove this arg altogether and change references to data_model to be otp_phone_view_data ?
There was a problem hiding this comment.
This will change in an upcoming PR, but its fine to remove for now! Thanks
There was a problem hiding this comment.
small typo, should be "authenticator"
There was a problem hiding this comment.
thanks for catching this!
**Why**: To consolidate logic for displaying otp fallback links and help text
el-mapache
force-pushed
the
add-totp-presenter
branch
from
177b67d to
b7a2848
Compare
|
@el-mapache where are we on this one? |
|
@monfresh did you want to look over this as well? I see we are awaiting a requested review! |
|
@pkarman Waiting for additional reviews, I think? |
* Adds presenters for phone view **Why**: To consolidate logic of how we show fallback otp links and associated help text to the user * Adds authenticator presenter **Why**: To consolidate logic for displaying otp fallback links and help text
* Adds presenters for phone view **Why**: To consolidate logic of how we show fallback otp links and associated help text to the user * Adds authenticator presenter **Why**: To consolidate logic for displaying otp fallback links and help text
* Adds presenters for phone view **Why**: To consolidate logic of how we show fallback otp links and associated help text to the user * Adds authenticator presenter **Why**: To consolidate logic for displaying otp fallback links and help text
5 participants
Adds presenter class and logic for totp verification view