Add reactivation warning modal, fallback, specs#1487
Conversation
There was a problem hiding this comment.
What do you think about using the helper method complete_idv_profile_ok(user), which performs lines 43-51?
There was a problem hiding this comment.
That method assumes we want to use the user's old password, in this case we want to use the new one.
There was a problem hiding this comment.
How about we modify that method to take in the password as a second argument, and default it to the user's old password?
There was a problem hiding this comment.
Should we add a expect(current_path).to eq reactivate_account_path here since we're visiting another page below and then manually visiting the expected page later?
There was a problem hiding this comment.
Why remove the JS from this test? Don't we want to have a JS test for the scenario where the reactivation is done with a personal key, as opposed to without a personal key?
There was a problem hiding this comment.
I originally added it for debugging, but I can leave it in.
el-mapache
force-pushed
the
ab-account-reactivate-modal
branch
from
1f47801 to
8b6b4ac
Compare
|
@monfresh updated |
There was a problem hiding this comment.
How about skipping enter_correct_otp_code_for_user(user) by adding allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) at the top of the test?
|
I pulled this down to test it and found some issues. The first should probably be addressed in this PR since it's related to the modal that was introduced here. The other one is probably an existing issue and can be addressed in a follow-up PR.
|
|
@monfresh as far as I know, having only the |
|
Fair enough. |
el-mapache
force-pushed
the
ab-account-reactivate-modal
branch
from
8b6b4ac to
44f551e
Compare
|
Your second point is totally valid, there is a nested html form on this page, and that is a no-no. I'm going to update the PR to change that. |
el-mapache
force-pushed
the
ab-account-reactivate-modal
branch
2 times, most recently
from
39696b3 to
06f9789
Compare
**Why**: The user should know that they must reactivate their account following a password reset. Using a modal to provide 'heads up' information matches our current UX. A fallback alert message is also provided when JS is disabled Spec changes **Why**: Satisfy PR feedback Removes manual otp code entry **Why**: Only one of the specs in `password_recovery_via_recovery_code_spec.rb` needs to go through the otp code entry page Un-nest personal key verification bail out form **Why**: Having a form nested in a form is bad. Add cancel link to no pkey warning modal **Why**: User should have the option to cancel the modal and verify their personal key once they are warned that lack of said key will require identity reverification Add specs for modal cancel and pkey verify cancel **Why**: We should have confidence that a user attempting to close the reverify warning modal or electing to reverify their info once on the personal key verification page can do so
2 participants
Why: The user should know that they must reactivate their account
following a password reset. Using a modal to provide 'heads up'
information matches our current UX. A fallback alert message is also
provided when JS is disabled
Screenshots: