#6915 closed defect (bug) (fixed)
BP Email template: the unsubscribe link & the account activation email
Reported by: | imath | Owned by: | |
---|---|---|---|
Milestone: | 2.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | dev-feedback has-patch |
Cc: |
Description
While testing the patch on #6913 i've noticed the "Unsubscribe" link was inserted inside the activation email although the user is not activated yet.
For regular WordPress config : clicking on the unsubscribe link displays the login form, when trying to login it displays an error "please activate your account".
For all other configs (BP_SIGNUPS_SKIP_USER_CREATION
&& multisite) the unsubscribe link has an empty href
attribute.
I wonder if it's possible to avoid displaying this unsubscribe link if the email type is core-user-registration-with-blog
or core-user-registration
, and if we should actually do it ?
Attachments (2)
Change History (12)
#3
@
9 years ago
Attached patch passes the 'To' email info into the email template for checking so we can determine if the "Unsubscribe" link should show in emails.
I had to move the 'to'
parameter higher up to the bp_get_email()
function and I had to stash the parameter in the buddypress()
object in order so we can access this info in the email template. Yeah, I know...
To determine if a user should see the "Unsubscribe" link, I created an email template function called bp_email_is_recipient_to_singular_user()
(pardon the crappy name for now!). In this function, I'm using bp_is_user_active()
, which I've patched to determine if a user is pending as well.
If we like this technique, we could also move the tokens to bp_get_email()
so we could potentially do other checks in the email template.
#4
@
9 years ago
The idea with bp_get_email()
was that it encapsulates the query to fetch a WP_Post
and handles that (which is why it sets subject/content/template, because that data comes from the Post). My initial thoughts are that I don't like the idea of moving any other data into that doesn't directly come from that Post. I like the separation of concerns. The recipient details come from another place.
There's no reason why a more custom implementation of our email system could be made by someone who chooses to not use bp_get_email()
. Perhaps they are creating their own bp_send_email
for use in their own plugin, for some reason, and bypass bp_get_email()
entirely (maybe they aren't using Posts to store email content). While this isn't my strongest argument ever, moving any part of recipient information or tokens into bp_get_email()
just means more work for someone to copy out of our functions and paste it somewhere else (and be baffled why all they need isn't in one place to copy from).
If the goal of patch idea is to provide conditional logic around token rendering (plus the blocks of markup around them), I'd rather take the time to consider replacing the basic/simple token parsing logic with a real Mustache template parser. For example, see https://mustache.github.io/mustache.5.html under "Sections" ({{#person}}
etc).
#5
@
9 years ago
- Keywords has-patch added
I'd rather take the time to consider replacing the basic/simple token parsing logic with a real Mustache template parser.
I think this would be very cool and quite easy to integrate.
Only question is what Mustache parser we would want to use.
- mustache.php is linked from the official mustache Github page. Could use Composer for this, but library is 176kb.
- LightnCandy claims to be 2-7 times faster than mustache.php, but requires PHP 5.3 due to namespaces. Library is also 196kb.
For 6915.mustache.patch
, I decided to pick a lesser-known library, php-mustache, that is only 40kb and licensed under the BSD. We can pick and choose any library, I just chose one that was relatively small.
To address the issue in this ticket, I've included some unit tests for bp_core_replace_tokens_in_text()
and have changed the email template to use a mustache section for the unsubscribe block -- {{#unsubscribe}}
. The 'unsubscribe'
token is only added if the user is active. See the changes made to bp_email_set_default_tokens()
and bp_is_user_active()
.
Now, the unsubscribe link will not show up in registration and activation emails.
unsubscribe
will be empty if the recipient can't be found byget_user_by( 'email', ... )
. There's no support for using tokens in a conditional.We could set the default value for this to wp-login.php, so at the very least, people arrive at the log-in form, which is better than a broken link, because they get a message telling them to activate their account first.
Until if/when we have a "real" unsubscribe link that doesn't require you to be logged in, this is probably the easiest and least messy solution I can think of. What do you think?