-
Notifications
You must be signed in to change notification settings - Fork 255
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
Support Unsubscribing from all emails #7844
base: release-2.1
Are you sure you want to change the base?
Conversation
Unsubscribing from email notifications is definitely not the same as requesting to delete one's account. They should not be combined. |
Think we should leave it out then? I was thinking it would help those who want to delete their account also do that quickly. |
Yes, definitely. Adding the List-Unsubscribe header is an excellent idea. Prompting the user to delete their account just because they don't want certain email messages is not. There's no GDPR requirement to make such a prompt. |
GDPR does require the forum operators to comply with the right to be forgotten rules by deleting the account. This just automates it. I will update the PR to remove the account deletion. Should we bake this more into the notifyAnnouncements logic then to allow them to completely remove all notifications? This also appends to the end of the message "Unsubscribe: ". Ensuring all emails show the unsubscribe text. |
Oh, I know that GDPR requires the ability to delete the account. And we provide that. It just shouldn't be part of unsubscribing from notifications. I thought we already put the unsubscribe link in our notification messages. I quite distinctly remember making both plain text and HTML versions of those links for inclusion. Am I missing or forgetting something here? |
The logic you built only applied to announcements emails. This PR adds it to the sendmail function, effectively ensuring all emails will contain a generic unsubscribe link. This requires them to submit the form and receive the actual unsubscribe link that they then confirm. For a future version, we can rewrite sendmail to do this better. I couldn't fix this in 2.1 since sendmail accepts $to as an array. |
No, it's also there for all the new topic and new reply notification types. If you look at EmailTemplates.english.php, you'll see There are messages that don't include an unsubscribe link, but most of those are all ones where it wouldn't make sense to include one. For example, the account activation message doesn't include an unsubscribe link because it's not something that can be subscribed to or unsubscribed from. We certainly do not want to append an unsubscribe link to every outgoing message. For many of them, that would be nonsensical. We might want to add unsubscribe links to some other notification types (e.g. birthdays) but the way to do that will be to add unsubscribe links to those particular messages and add handlers to Notify.php to accept those particular types of unsubscribe links. |
There is a lot of emails that come out without unsubscribe links.
Just the ones I can easily see in my inbox. Maybe we need to add a param to the sendmail to tell it to not auto add the unsubscribe link? Because emails could contain user generated content, we can't safely parse them for the url existing in the message already. If you want, you can take a crack at this. I want to say for a future version, we can rewrite the sendmail to better pass the to array in a way that allows it to contain things like per email replacement variables that then get emailed out. Essentially make a sendmail as it is a another method and sendmail method that forces this. Internal calls that are in compliance such as announcements and other email templates would be able to bypass this, otherwise logic would force some sort of unsubscribe link. |
Is this included in #7882 ? |
Thank you, I was intending to bring this up. I did not include it. Should be out of scope. That PR is targeted to split up the mail-sending agents (SMTP, SMTP + TLS, and SendMail) into separate APIs. Allowing easier management, delegation, and for future additional agents. Simply think, a customization could implant the API to do emails through SendGrid with ease now, by dropping in the file and updating the mail settings to use it. I don't think additional targets should include it. If we do, it should be at the main mail processing, not the agents. @Sesquipedalian also had concerns I think of how this was implanted for 2.1. Maybe we want to abandon this for 2.1 and redo this for 3.0 the right way? But I do realize we have to have the |
Fixes #7841
Firstly this adds List-Unsubscribe as described in the issue. This helps email clients provide links/information for the user to stop getting emails. As well larger email providers, like Google, requires this for domains sending large volumes of email.
I choose to not use the notifyAnnouncements here as that is tuned to just remove notifications from email announcements. This adds ability to remove notifications and alternatively delete their account. This would also help comply with GDPR that a operator must delete the account.