-
Notifications
You must be signed in to change notification settings - Fork 984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2fa disabling ask password #6408
2fa disabling ask password #6408
Conversation
Hi @yeraydiazdiaz, thanks for this PR 😁 A couple of items: Could we please update the error text (when a user enters an incorrect password) to be more meaningful than 'invalid credentials'? Maybe something like: Can we also please use this functionality for the 'remove API token' modal as well? Thanks! |
@yeraydiazdiaz is this ready for re-review? |
Yes, it is. Here is a capture of the changes requested by @nlhkabu |
Remove unused `index` parameter
Allow `username` to be set as an instance variable
Avoid passing the username to constructor simplifying base class
c1af5ac
to
46fc81f
Compare
I've implemented the change suggested by @di and rebased on master, this is ready for re-review. |
I merged master onto this branch and had to update the some of the tests, I believe everything's correct but @woodruffw could you review again just in case? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you @yeraydiazdiaz ! 🎉 |
Fixes #5825
Here's a capture of the new functionality: