Change: display user's password reset link to the admin when mail server disabled #1371
Conversation
|
Yes, I think that doing it in a similar manner to how we did with the invite link is preferable than showing it briefly in the growl message. Also, when implementing this please show the link only when |
Similarly to when we invite new users, this allows the admin to reset the password herself (or send it manually to the user), in cases where there's some issue sending the e-mail.
vitorbaptista
force-pushed
the
feature/show_admins_the_password_reset_link
branch
from
d6c9041
to
72389b0
Compare
|
@arikfr Please take a look. I wanted to make the link an actual This is in a couple commits just to make it easier to understand the PR, but would expect them to be squashed when merged. |
| <p ng-if="clientConfig.mailSettingsMissing"> | ||
| You can use the following link to reset the password yourself:<br/> | ||
| {{passwordResetLink}} | ||
| </p> |
There was a problem hiding this comment.
- Let's show the original message ("should receive by email...") only if email is enabled (
ng-if="!clientConfig.mailSettingsMissing"). - Change the text here to:
You don't have mail server configured, please send the following link to {{user.name}} to reset their password:. - To have the reset link as an
<a>tag:<a ng-href="passwordResetLink">{{paswordResetLink}}</a>. IIRC, it's a full link so the base href doesn't matter.
|
@arikfr Thanks for the review! Just added your comments in and this is good to go AFAIK. |
vitorbaptista
changed the title
| </div> | ||
|
|
||
| <div ng-if="passwordResetLink && !disablePasswordResetButton" class="alert alert-success"> | ||
| <p> |
There was a problem hiding this comment.
Let's add here ng-if="clientConfig.mailSettingsMissing".
There was a problem hiding this comment.
So then I should remove the ng-if from the "You don't have a mail server configured" and always show that message?
There was a problem hiding this comment.
No. We show one message if there is no mail server configured and the other otherwise. But I just noticed that you have the ng-if backwards: it should be clientConfig.mailSettingsMissing for your message and ! clientConfig.mailSettingsMissing for the original one.
There was a problem hiding this comment.
Oh, I see. Just changed it. To be honest, as a user, I'd prefer to have the link shown regardless if I have an e-mail server configured or not, similar to the user invites. I find them useful even when the user will receive an e-mail, and it also gives admins the ability to change a user's password herself. Happy to go with whatever you think is best, though.
Just to let you know, I'm adding new commits and not changing the original ones just because I think it's easier to review this way, but I expect that you (or me) will squash them before merging.
arikfr
changed the title
|
Thanks! |
vitorbaptista
deleted the
feature/show_admins_the_password_reset_link
branch
…ins_the_password_reset_link Change: display user's password reset link to the admin when mail server disabled
This is work in progress, I'm sending just to ask for some help. Adding the reset link is quite easy, I'm just wondering what would be the best way to display it to the user. We have a similar functionality when creating a new user, where we display the invite URL to the admin inside the template. Would that be the best approach for this?