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

Support Unsubscribing from all emails #7844

Open
wants to merge 2 commits into
base: release-2.1
Choose a base branch
from

Conversation

jdarwood007
Copy link
Member

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.

@jdarwood007 jdarwood007 added Compliance Legislative issues such as GDPR Email labels Oct 17, 2023
@jdarwood007 jdarwood007 added this to the The future milestone Oct 17, 2023
@Sesquipedalian
Copy link
Member

Unsubscribing from email notifications is definitely not the same as requesting to delete one's account. They should not be combined.

@jdarwood007
Copy link
Member Author

Think we should leave it out then? I was thinking it would help those who want to delete their account also do that quickly.

@Sesquipedalian
Copy link
Member

Sesquipedalian commented Oct 18, 2023

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.

@jdarwood007
Copy link
Member Author

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.

@Sesquipedalian
Copy link
Member

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?

@jdarwood007
Copy link
Member Author

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.

@Sesquipedalian
Copy link
Member

Sesquipedalian commented Oct 24, 2023

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 {UNSUBSCRIBELINK} tokens in many of the messages. Everywhere that occurs, an unsubscribe token is provided in the message text.

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.

@jdarwood007
Copy link
Member Author

There is a lot of emails that come out without unsubscribe links.

  • Topic watch notifications (New & Replies)
  • Mention notifications
  • New Personal Message

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.

@live627
Copy link
Contributor

live627 commented Feb 1, 2024

Is this included in #7882 ?

@jdarwood007
Copy link
Member Author

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 List-Unsubscribe header for providers like Gmail who will start flagging emails as spam if they send a high enough volume without it now. I think getting opinions on how this was done and if want to redo it for 2.1 and then match it for 3.0.

@live627 live627 added Mail E-mail and removed Email labels Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compliance Legislative issues such as GDPR Mail E-mail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support List-Unsubscribe header
3 participants