-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update lost password email to use HTML link #31092
Conversation
Codecov Report
@@ Coverage Diff @@
## master #31092 +/- ##
============================================
+ Coverage 62.5% 62.52% +0.01%
- Complexity 18269 18435 +166
============================================
Files 1146 1147 +1
Lines 68397 68416 +19
Branches 1234 1234
============================================
+ Hits 42753 42777 +24
+ Misses 25283 25278 -5
Partials 361 361
Continue to review full report at Codecov.
|
Thanks a lot for this PR! Please note that some of these templates are customizable so I'm not sure if relying on "strip_tags" here would fit all scenarios. A better way would be to use the same approach like for other emails: there are two template files, one called "mail.php" (for HTML) and "altmail.php" (for plain text). So here I suggest to do the same and have "core/templates/lostpassword/email.php" be the HTML email and create a new template "core/templates/lostpassword/altemail.php" with the plain text version. Then wire this up with the code. |
Sure. |
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.
Tested, works 👍
cc @phil-davis @individual-it to include in acceptance tests
Hmm just noticed an extra ">" at the end of the HTML mail: That one doesn't appear for other mails. @joeyberkovitz mind verifying the markup ? I had a quick look but wasn't able to spot it |
I cloned the repository into a separate directory and reinstalled Owncloud. |
Thanks for confirming. Then maybe it was a bug in Mailhog's HTML renderer. |
Mind backporting this to the stable10 branch ? Basically branch off stable10, cherry-pick your commits then submit PR to stable10 after checking it still works. |
I created the backport PR |
Backport stable10 #31144 |
@paurakhsharma is working on acceptance tests in #31168 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Update the lost password email to send both HTML and plain text versions so that URL in email can be clicked.
Related Issue
#30742
Motivation and Context
Solve issue #30742
How Has This Been Tested?
Update associated tests to add HTML support.
Screenshots (if appropriate):
Types of changes
Checklist: