Skip to content
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

[5.7] Add line about expiring password reset in notification email #27324

Merged
merged 1 commit into from
Jan 28, 2019
Merged

[5.7] Add line about expiring password reset in notification email #27324

merged 1 commit into from
Jan 28, 2019

Conversation

michaeldyrynda
Copy link
Contributor

@michaeldyrynda michaeldyrynda commented Jan 28, 2019

I've recently started using the password resets in one of our apps at work, and the most common feedback we receive is that "it doesn't work". This is always because the link is expired.

Per suggestions from Postmark, I've added a line that pulls the expiry config value in to the notification email.

I'm not sure of the significance of using transChoice over getFromJson here, but I couldn't get the choice to work otherwise.

Happy to make changes if anybody else (ping @themsaid) has feedback on another approach; I don't want to have a line that doesn't work for those using JSON language files if the transChoice method doesn't work with that properly.

@GrahamCampbell GrahamCampbell changed the title Add line about expiring password reset in notification email [5.7] Add line about expiring password reset in notification email Jan 28, 2019
@taylorotwell
Copy link
Member

taylorotwell commented Jan 28, 2019

There is probably another way to phrase it so that you could use getFromJson... Something like:

"Minutes until password reset link expires: #"

@taylorotwell taylorotwell merged commit 607caff into laravel:5.7 Jan 28, 2019
@taylorotwell
Copy link
Member

I just used getFromJson. In theory it will read weird if the expiry is set to 1 minute but I think that's probably not going to happen in practice. Setting the expire link to 1 minute would never be a good idea in production just due to slight email delivery delays, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants