LG-10235: Hide skip MFA after adding F/T#8739
Conversation
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
This should also include hiding the link on the following screen with the 2nd MFA selection.
| end | ||
| end | ||
|
|
||
| describe '#show_skip_additonal_mfa_link?' do |
There was a problem hiding this comment.
- Minor: Typo on "additonal" (should be "additional")
- Since this is implemented in the concern, can we create a concern spec file with one set of tests rather than implementing the tests across all controllers which use the concern?
| attr_reader :mfa_context | ||
|
|
||
| def initialize(mfa_context:, show_skip_additonal_mfa_link: true) | ||
| @mfa_context = mfa_context |
There was a problem hiding this comment.
It doesn't appear we use the MFA context on the presenter? Should it be removed?
Alternatively, we could move the logic for show_skip_additonal_mfa_link? into the presenter?
There was a problem hiding this comment.
There's a trade off between defining the logic of the function show_skip_additional_mfa_link? once or duplicating the logic in the following two files:
app/presenters/mfa_confirmation_presenter.rb
app/presenters/two_factor_options_presenter.rb
Presently the logic is defined in one location (controllers/concerns/mfa_setup_concern.rb). I'm not-so-attached to either style.
The unnecessary mfa_context has been removed and was left over from a change in style.
jc-gsa
force-pushed
the
LG-10235-hide-ft-skip
branch
from
339f97e to
5d2cb09
Compare
jc-gsa
force-pushed
the
LG-10235-hide-ft-skip
branch
from
5d2cb09 to
5518016
Compare
| def initialize(user_agent:, user: nil, | ||
| phishing_resistant_required: false, piv_cac_required: false) | ||
| def initialize(user_agent:, | ||
| user: nil, | ||
| phishing_resistant_required: false, | ||
| piv_cac_required: false, | ||
| show_skip_additional_mfa_link: true) |
There was a problem hiding this comment.
Much more readable as one-per-line 👍
2 participants
🎫 Ticket
LG-10235
🛠 Summary of changes
Please see associated Figma file for expected behavior.