-
Notifications
You must be signed in to change notification settings - Fork 297
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
Set type of email notification #947
base: 8.next
Are you sure you want to change the base?
Conversation
Without this no email is sent. This has become necessary after the following change from CakeDC#685 https://github.com/CakeDC/users/pull/685/files#diff-087e5d4ddeb9b25aa37f089c2ce923ee418537310623b29d393404528f1797a3R73-R80
By the way, I haven't checked whether there are other places to adjust. |
src/Shell/UsersShell.php
Outdated
@@ -239,6 +239,7 @@ public function passwordEmail() | |||
'expiration' => Configure::read('Users.Token.expiration'), | |||
'checkActive' => false, | |||
'sendEmail' => true, | |||
'type' => 'passord', |
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.
this looks like a typo?
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.
:embarrassing: Fixed
Please could you describe the use case when email is not being sent? I'd like to cover it with a unit test. Thanks, |
I think we tried to send the reset password link through the CLI, thus the adjusted Shell class, but no mail was sent due to the recent change mentioned. |
Thank you, we'll try to reproduce the issue in a unit test and fix it, thanks! |
well, you fix it already ;) I mean merge it |
Without this no email is sent.
This has become necessary after the following change from #685
https://github.com/CakeDC/users/pull/685/files#diff-087e5d4ddeb9b25aa37f089c2ce923ee418537310623b29d393404528f1797a3R73-R80