Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 8 years ago

#6592 closed enhancement (fixed)

Email API and customisation features

Reported by: djpaul's profile DJPaul Owned by: djpaul's profile DJPaul
Milestone: 2.5 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc: hnla, stephen@…, espellcaste@…, tw2113@…

Description

We've wanted to overhaul emails in BuddyPress for the longest time. The problems with our current implementation is that we don't really have an implementation: we just use wp_mail. While the email content is filtered, it still requires a developer to write code to customise the email, when really, the site owner should be able to do that themselves.
Cosmetically, we send plain text emails, and I don't think that is simply good enough for modern expectations.

Even WordPress multisite's network admin has better email editing features (just a textarea, but we don't even have that), and no-one's touched that part of multisite for a very long time.

So, after much discussion with interested parties over the years in dev chats, support forums, and WordCamps, here are the foundations that I propose we build our experience on:

  • Use a Post Type for email templates.
    • Use wp-admin to edit them.
    • Use the WP Customiser to visually customise those emails.
    • Use post_content to store the HTML version of the email, and post_excerpt to store the plain text version.
    • Use a set of printf-like tokens to dynamically replace text in the email (username, links, etc) as required.
  • Use a Taxonomy to store the distinct types of emails we send (e.g. new_user, new_site, new_activity_mention, etc).
    • Use the taxonomy to find the related Email (post type instance).
    • Taxonomy instead of post_meta to (potentially) scale better, and allow advanced use case e.g. an implementation of a basic A/B test for emails.
  • Introduce new HTML email templates into the BP template stack, with a template hierarchy (user type, component).
  • Introduce new classes to represent an email (BP_Email), one for each type of email.
    • Has: filters, callback functions to fetch the token replacements, convenience functions to access WP_Post properties (bp_email->get_subject, etc).
    • Provide basic API for fetching/creating these objects, like we did for the xprofile field types.
  • Create an interface, and a class (that uses it) that takes the assembled BP_Email object, and sends the email with wp_mail().
    • Structure of this class is set with an interface, which will allow easier integration with third-party mail delivery services (e.g. mailchimp). Currently, all of these plugins I've researched replace the wp_mail function which is not ideal.

(I couldn't find an existing ticket for this, despite all the discussion over the years.)

Attachments (8)

6592-core.01.patch (44.2 KB) - added by DJPaul 8 years ago.
6592-posttype.01.patch (21.7 KB) - added by DJPaul 8 years ago.
6592-customiser.01.patch (28.6 KB) - added by DJPaul 8 years ago.
menu-location-01.patch (933 bytes) - added by DJPaul 8 years ago.
6592-register-post-type-on-rootblog-only.patch (3.2 KB) - added by imath 8 years ago.
6592-customizer-on-root-blog-1.patch (2.5 KB) - added by DJPaul 8 years ago.
image.png (95.1 KB) - added by imath 8 years ago.
6592.decode.patch (1.7 KB) - added by r-a-y 8 years ago.
Updated patch due to feedback from Trac.

Download all attachments as: .zip

Change History (109)

#1 @netweb
9 years ago

  • Cc stephen@… added

#2 @imath
9 years ago

Wow this looks really great!!

#3 @espellcaste
9 years ago

  • Cc espellcaste@… added

A similar discussion about it happened on #6460.

Overall, I like your ideas, BuddyPress lacks a good email system and your suggestion seems to apply for both developers and non developers alike.

My only concern would be about complexity, as I stated on #6460, I like Woocommerce way of handling emails, it is simple, developers can apply their own styles (not in the customizer), it is still hard to create different templates (not related to default but part of Woocommerce) but not impossible with some documentation checking.

But your plan/idea goes beyond that and I applaud you for wishing to take BuddyPress to the next level and resolve this long problem with emails.

Count me in for help. I've had some problems in trying to customize and create custom e-mails in BuddyPress recently and would be more willing to help in any way to solve this.

#4 @DJPaul
9 years ago

@espellcaste Thanks for your feedback, and for finding that other email ticket again. We're up to 3 tickets about pretty much the same thing, so if we find more, we should combine them all together. :) The WooCommerce implementation is pretty interesting to review, and I spy they are using the https://github.com/jjriv/emogrifier library (maybe we will too!).

I'd love to have some help working on this. How would you like to help? Let me know, and I'll make sure you can contribute in the way that you want to.

#5 @espellcaste
9 years ago

Yes, they are using emogrifier. But just to reinforce, your idea goes beyond that, it would have a bigger impact on the BuddyPress community because even non developers would be able to use it.

But I'm gonna check with Claudio Sanches, from Woothemes/Automattic, why they didn't go this way, maybe they didn't thought about or maybe there are deeper problems.

*

About me helping, I'm a guy coming from the front-end arena, so I consider myself a junior PHP developer. I think I can be of assistant on these areas:

Front-end
Email creation/templates/layouts
Testing
Not advanced PHP stuff yet (traits, interfaces and etc)

I'm a quick study but I lack the broad view of the code you guys from the core team have. I've been following tickets here for years and I always notice that.

*

I think you should create a github repository, so that we can tackle this as a plugin first and with more works and improvements, later on, maybe in 2.4, we integrate into core.

What do you think?

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


9 years ago

This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.


9 years ago

#8 @DJPaul
9 years ago

Other related tickets: #1949, #4776, #6460.

#9 @timersys
9 years ago

@DJPaul count me in! As commented on the email I sent you the other day, imo regular users will use only one template to wrap all the emails and there is no need really to give a different email template for every email.

On my plugin I use phpmailer because I found more reliable in order to send emails with html + tex plain all together. Check this question for reference

#10 @DJPaul
9 years ago

@timersys That's interesting, thanks for the information.

#11 @DJPaul
9 years ago

I owe everyone following an update, so here goes: my latest code so far is on my Github, here: https://github.com/paulgibbs/BuddyPress/compare/amazing-emails

  • I have put some framework into core for post types
  • I have registered an "Emails" post type.
    • The intention is to use a standard wp-admin editor box to allow people to change the contents of the email.
  • I have replaced all direct calls to wp_mail with a new custom function bp_send_mail that, basically, wraps wp_mail.
    • I surveyed all the mail replacement plugins I could find (like Mandrill's), and they all re-declare wp_mail to implement support for their custom service.
    • Therefore, if wp_mail is redeclared, *or* something has been filtered to get WP to send HTML emails, I'm assuming there's some dark voodoo at work -- and for reasons of trying to be compatible as possible, BP will treat emails the same way it does today (we'll pass it straight through to whatever wp_mail is).
  • bp_send_mail does three things; 1) the compatibility checks as described above, 2) creates a BP_Email object representing the email to be sent (the content, addresses, etc), 3) and sends the email.
    • Sending is achieved by looking for a class that represents some kind of email delivery service. I'm still playing with the class name, but for example, we could have a BP_WPMail class that takes our email, sets up wp_mail appropriately (enables multipart, HTML, etc), then sends the email.
    • The idea is to split the message object away from the delivery implementation.
    • Mandrill, for example, could create a new BP class in their plugin, filter something in BuddyPress, and they'd receive an email object, and it would be up to them to format it and send it to their delivery API -- but they wouldn't be stomping all over BP core's email implementation to do so.
  • There are a number of unit tests in place, but there's more needed.
  • Most of this hasn't been hooked together yet, it's all pretty fragmented still.

Remaining tasks:

  • On new installation/upgrade, auto-create Email objects for each scenario in BP. e.g. received a private message, you got mentioned in an update, etc.
    • Figure out how best to support i18n here. I think it might be a PITA to do well, but we *need* to do it well. :) Biggest problem is what happens if English language email posts are created in the DB, but then later the site uses a German language packs -- the emails are still in English.
  • Implement Email customisation (maybe font colour, background colour, etc -- don't know yet).
    • I am very keen to get @timersys on board to help build out the customisation features. He has great experience and really high-quality code in in his popular Email Templates plugin that we would be foolish to ignore.
  • Finish building out email delivery class/objects/etc and think the implementation through a bit more. Write more unit tests.
  • Implement new email template(s).
  • Implement code that grabs those templates (with theme compat. and child theme support, etc), replaces tokens with real values (names, links, etc), and anything else needed to prepare it for sending. Some of this is already done, and some it I will probably re-use directly from @timersys' plugin.
  • Connect all these separate bits together.

In terms of facilitating contributions from @timersys and @espellcaste and anyone else, fork https://github.com/paulgibbs/buddypress/ (use the "amazing emails" branch) on Git, and send PRs. If Git isn't your thing, you can check out Github repos as SVN, and then run a diff and send normal patch files to me, via this trac ticket.

If you want to help, please co-ordinate with me so we don't duplicate effort -- let me know here or email me or find me in the WordPress Slack (#buddypress) -- and we can work together.

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

#13 @tw2113
8 years ago

  • Cc tw2113@… added

#14 @timersys
8 years ago

Hi @djpaul I took a look to all the commits so far, its looking nice.
How are you planning to show emails in the backend. As a new tab with action for each type of email ( kinda of Components tabs) Or all the messages in the same page?

I could start adding that part while you finish the mailer classes

Also these notifications already exist in some way?
http://i.imgur.com/b4EGluh.jpg

Last edited 8 years ago by timersys (previous) (diff)

#15 @shanebp
8 years ago

Great to see bp_send_mail.
Would you be willing to take into consideration the ideas in #6460 ?
Which are:

  • bp_send_mail would have an apply_filters hook before it calls wp_mail.
  • add a $type arg to function bp_send_email( $email_type, $to, $args ) {

The $type arg would tell you where the call came from, for example 'bp_activity_new_comment_notification_message'

The $type arg would be included in the apply_filters hook.

Then devs could use a single filter hook ( the new one in bp_send_mail ) to filter emails based on where they came from - instead of multiple hooks.

Last edited 8 years ago by shanebp (previous) (diff)

#16 @DJPaul
8 years ago

Hi @shanebp, thanks for taking a look and your feedback. Of course willing to discuss ideas. :)

  • $email_type is meant to represent a unique type of email message. For example, "new_user", "new_private_message". If we sent the exact same email in multiple distinct situations, I'd agree we need a distinguishing parameter. But I'm not sure this situation exists -- can you think of one I'm missing?
  • bp_send_email is still not finished, there will be more filters. :) Filter all the things!
  • If you want filters to programatically change an email, also note the BP_Email class has many filters.
  • But if you have filters in place to want to be able change how the emails are delivered (or not), the new BP_Email_Delivery class interface will be the best place to do that. Hoping to have this fleshed out pretty very soon.

#17 @DJPaul
8 years ago

Hi @timersys

Emails will be show in the backend as a custom post type, so the normal wp-admin -style interface. So not inside the BP admin screens like your mockup suggestions. There was a bug in the code causing the wp-admin menu to not appear, I've fixed it enough for now so that the menu shows up.

It'd be great if you could start implementing Customizer support for the Email post type? Looking at your plugin, I think it'd be sensible for BP to have all the panels etc that you have.

  • In terms of how a person gets from wp-admin/edit.php?post_type=bp-email, I had been thinking a button like this, rather than a discrete menu item as your plugin implements, but maybe we should do both? https://imgur.com/9NN8tlx

thanks!

#18 @timersys
8 years ago

@DjPaul at the end will be one template for all emails or one for each type of notification ? I think I already asked this but Im not sure about the answer.

Regarding your screenshot , is not there where the "move to trash" link exist? Maybe we can place it at the top of the editor next to the "add media" button

#19 @shanebp
8 years ago

@DJPaul

Thanks for the info. And all the work you're doing on this.

So $email_type is in progress.

And it will hold a value to indicate that it came, for example, from the code that handles bp activity new comment notifications ?

#20 @DJPaul
8 years ago

@shanebp everywhere we send an email in BP today will have a unique value for that type of email. So yes, you can rely on it to filter certain types of email, etc.

#21 @DJPaul
8 years ago

@timersys Let's try to ship with 1 template for all emails, but allowing a drop-down box to pick alternative templates on a per-email basis (kind of like what you have in your plugin already).

RE: screenshot - it was just a quick example to indicate placement in that post metabox. Put it where you see is best; worse case we'll move it later.

#22 @timersys
8 years ago

@DJPaul take a look before I do a PR https://github.com/paulgibbs/BuddyPress/compare/amazing-emails...timersys:templates

Lots of renaming moving files and refactoring is most probably needed, Im not used to organize things like you do in BP and I had a hard time deciding where tu put things :)

#23 @DJPaul
8 years ago

I'm looking forward to checking it out!

#24 @DJPaul
8 years ago

  • Milestone changed from 2.4 to 2.5

#25 @DJPaul
8 years ago

Work over on the github branch is proceeding well; I have the basics hooked up end-to-end and it works. It's going to be a lot of work to migrate all BP emails to the new APIs, so i'm doing it one-by-one. Mountains are being moved for backwards compatibility. I am currently drafting code to "install" these emails on update, and once that's done, I'm going to finally review and integrate the work @timersys did his branch.

@timersys I apologise for taking so long, but really looking forward to getting what you have merged in soon.

#26 @DJPaul
8 years ago

  • Owner set to DJPaul
  • Status changed from new to assigned

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

#29 @DJPaul
8 years ago

Progress is going well, but I wanted to share a technical overview to aid with code review:


Researching WP plugins that support services such as Mandrill or Amazon SES showed a common approach of replacing wp_mail(). Some made formatting changes, some just delivered the email to the external service, some did both.

I looked at a few other PHP CMS projects to see how they approached things. I remember looking at Joomla!, Drupal, and a handful of others. I was hoping to find a nice architectural model or technical implementation, but everything looked like wp_mail(). I wanted to split email delivery away from the internal representation of an email itself.

  • Class BP_Email represents an email; the bits you see in your email client like the subject, message, recipients. It also encapsulates custom email headers and tokens.
    • Tokens help personalise the content of an email. Analogous to MailChimp’s merge tags.
  • Class BP_Email_Recipient represents an email recipient. It’s a simple class, and holds an email address and recipient name. If the class is instantiated with a WordPress user ID, then it also has a WP_User reference.
  • Class interface BP_Email_Delivery mandates a bp_email() function, which mail delivery services will use to implement BuddyPress support.
    • Class BP_PHPMailer implements that interface for PHPMailer.
  • Function bp_send_email() is the function we use to send emails.
    • If wp_mail() has been re-implemented, then BuddyPress will fallback and use it (work in progress, may change).


The other major feature that I wanted to implement was user-editable emails. WordPress multisite lets you edit some of its emails in a basic textarea. BuddyPress requires a code filter. Both are crappy solutions.

  • An email post type and an “email type” taxonomy have been added.
  • All of BuddyPress’ emails have been moved into email posts.
    • Tokens are used to personalise the email content (i.e. to add a link to the recipient’s user profile).
    • Each type of email in BuddyPress has been assigned a unique type, and these are mapped to the email post with the “email type” taxonomy.
  • A HTML email template has been added.
  • A Customizer integration allows email appearance changes.
  • Extensive effort has been made to maintain backwards compatibility for existing email filters, but there will be some breakage (some parameters will be removed from some actions).

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

@DJPaul
8 years ago

#31 @DJPaul
8 years ago

6592-core.01.patch implements the basic structure described above, with unit tests.

It does not include the post type or Customizer integration or the replacement function for wp_mail as those still need a bit of work, and will anyway be split into separate patches which will follow subsequently once this base is agreed on.

Questions, code review, feedback, all gratefully received.

Last edited 8 years ago by DJPaul (previous) (diff)

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

#33 @DJPaul
8 years ago

6592-posttype.01.patch implements the post type and taxonomy, email template, and the content of the email.

I have a TODO with the post type that I need help with; I'm unsure about the capability_type, and how to get it to work with our "really" great roles/caps integration. I'm checking our bp_moderate for when we need to show the menu, but we should probably set the capability type to let us and others filter it in advanced ways in the future.

FYI, this patch may/may not work in conjunction with 6592-core.01.patch; I've tried to create separate patches to try to get more reviewers and to represent groups of things to be committed at once. If you want to test the whole thing, you're best to use my git branch: https://github.com/paulgibbs/BuddyPress/tree/amazing-emails

I'm going to post up the Customizer integration in a third (and final) patch; I may end up missing out a few utility functions here and there, but between these patches, the bulk will be here.

Last edited 8 years ago by DJPaul (previous) (diff)

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

#35 @hnla
8 years ago

  • Cc hnla added

#36 @DJPaul
8 years ago

6592-customiser.01.patch implements the Customizer support.

It does what it sounds like it does, but there's also a menu item added in Appearance > Emails to take you to the Customizer for an email post.

Last edited 8 years ago by DJPaul (previous) (diff)

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


8 years ago

#38 @sebd86
8 years ago

Hi @DJPaul

I released a welcome email plugin for BuddyPress two weeks ago after developing it for a client of mine last year. Can you let me know if this would now be included among the emails and if so when the plugin would no longer be necessary.

I'm currently writing up the documentation for it. It does not currently have an admin settings page or any customizer controls to see what it looks like as you change the design.

It is next on my to do list but after reading this post on WPTavern (http://wptavern.com/buddypress-2-5-development-kicks-off-bp-api-dev-chat-begins-this-week), I am now wondering if it would be best to wait on your feedback before proceeding with the development.

I have only put filters in place at the moment to control and customize how it looks should the user wish to change the default settings.

You can view the plugin here: https://wordpress.org/plugins/welcome-buddy/

It is also on Github here: https://github.com/seb86/Welcome-Buddy

Let me know what you think.

Thank you.

regards,

Sebastien Dumont.

#39 @hnla
8 years ago

@DJPaul Just cloned your repo to check this over and it looks really great, been running a few test emails which work very smoothly, the implementation is clear and uncomplicated.

Minor points (probably stating the obvious though, at this stage!)

  • The emails appear to have the recipients name hardcoded i.e always prints as 'Hi Paul Gibbs'
  • In the customizer, currently in areas such as 'footer' the textarea is escaping html where ideally we would want to be running wp_kses_allowed_html so that we could add breaks or paragraphs to allow the user to build the footer they required and add links etc (again sure this is already on radar)
  • The CPT entries could perhaps do with having a snippet of text reminding users that header/footer and overall styling was/is managed from the customizer?
Last edited 8 years ago by hnla (previous) (diff)

#40 @DJPaul
8 years ago

@sebd86 I'll reply to you by email with detail but in a nutshell, I don't think there are plans for BuddyPress core to have a "welcome" email (at the moment).

#41 @boonebgorges
8 years ago

Thanks for your work on this so far, @DJPaul !

I'm compiling some notes with feedback and questions, but I wanted to pop in to address the capability_type question. To the best of my understanding, we should handle it like this: https://github.com/boonebgorges/BuddyPress-1/commit/6cbb240491339073e0df34e2ebc1b4b3694ec65a (note that this is incomplete and sloppy, but it gives you the idea of how to integrate with our meta cap mapping setup)

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


8 years ago

#43 @DJPaul
8 years ago

@hnla Thanks. I've addressed your feedback in Github, except the third bullet (extra snippet of text) -- not convinced by that immediately.

#44 @DJPaul
8 years ago

The direct equivalent for wp_mail() can be seen here: https://github.com/paulgibbs/BuddyPress/blob/amazing-emails/src/bp-core/bp-core-functions.php#L2830 (it's not included on any of the demo patches on this ticket, but it's best read in conjunction with 6592-posttype.01.patch).

#45 @DJPaul
8 years ago

I have just remembered that the Customizer implementation has two points worth noting:

1) It uses add_panel, which was introduced in WP 4.0.
2) I gave up trying to remove the "Widgets" panel that you see when the Email Customizer loads (Appearance > Emails). See #WP35242 the solution suggested there can't be used for us here; with WP 4.4.0 I was able to workaround by setting Customizer->Widgets = non but this broke in WP 4.4.1, so I gave up.

Last edited 8 years ago by DJPaul (previous) (diff)

#46 @DJPaul
8 years ago

Github branch has had updates since last week that aren't in these patches:

  • HTML now allowed in email footer.
  • Finished working on wp_mail detection/handling in bp_send_email, fixed a couple of PHP notices.
  • Added an extra action for plugin developers to hook to to install their emails, which also ties into the Repair tool.
  • Finished replacing placeholder strings in email template ("Hello Paul Gibbs"), renamed some email tokens.
  • Site name now prefixed to email subjects.
  • Various language tweaks to strings to improve readability, mostly on the Tools screen.
  • Added a banner that only shows to non-en_US locales that tells users how to reinstall the emails if they're showing up in the wrong language. (Need this option to support late installation of language packs, etc).
  • Tweak to email template for better support for wide images.
  • Added support for email preheaders via Custom Field; intended for advanced users (couldn't immediately decide if this is something we should support and didn't want to build custom UI).
Last edited 8 years ago by DJPaul (previous) (diff)

#47 @DJPaul
8 years ago

Github branch has had updates since last week that aren't in these patches:

  • Email Type term not shown on post listing screens in wp-admin.
  • Email Type tax not shown in menu in wp-admin.
  • Email Type - new custom metabox, removes input for adding new terms (since you need to write code for these to do anything) to simplify UI.
  • Email Type terms all have descriptions (= the purpose of the email in human-readable terms).
  • Email Type descriptions now shown throughout UI instead of the term slug (the internal ID).
  • BP_Email has renamed all setters to have a set_ prefix.
  • BP_Email has introduced get_ methods for getters, but these just pass to the existing get() method to avoid duplication.

#48 follow-up: @DJPaul
8 years ago

Core team: I would like feedback on https://buddypress.trac.wordpress.org/ticket/6592#comment:45 -- either we bump requirement to WP 4.0 (please) or somehow re-implement add_panel. ?

#49 @imath
8 years ago

I don't mind bumping requirement to 4.0. My only concern is we already announce we were bumping to 3.9 https://bpdevel.wordpress.com/2016/01/05/buddypress-2-5-will-require-wordpress-3-9/

I don't know if it's possible, but another scenario would be to do like @r-a-y did for embeds or like i did for the Avatar UI, check for 4.0 and enable the BP Email feature only if the required version for this feature match. Users having WP < 4.0 could still enjoy the other BuddyPress features.

#50 in reply to: ↑ 48 @boonebgorges
8 years ago

The changes described in 47 all look great to me. (I suggested them, so no big surprise :) )

Replying to DJPaul:

Core team: I would like feedback on https://buddypress.trac.wordpress.org/ticket/6592#comment:45 -- either we bump requirement to WP 4.0 (please) or somehow re-implement add_panel. ?

another scenario would be to do like @r-a-y did for embeds or like i did for the Avatar UI, check for 4.0 and enable the BP Email feature only if the required version for this feature match.

This seems silly, given that Customizer integration is only a small aspect of DJPaul's patch.

If it were the beginning of the dev cycle, I'd say forget it, and bump to 4.0. Given that we already made the announcement, and go to beta in less than 2 weeks, I feel like we should stick to our guns with 3.9. I suggest silently dropping Customizer support in 3.9:

if ( ! method_exists( $wp_customizer, 'add_panel' ) ) {
    return;
}

The Customizer stuff is great, but the rest of the feature works beautifully without it, so I think we can degrade gracefully here. Also, the chances of someone running BP 2.5 on WP 3.9 and wanting to customize the appearance of their emails is practically nil, so I doubt anyone on 3.9 will notice :)

#51 @DJPaul
8 years ago

Good ideas, thanks for the discussion.

#52 @DJPaul
8 years ago

I've added the add_panel check to the branch.

#53 @boonebgorges
8 years ago

@DJPaul - I've had a look over the latest changes, as well as the parts of the patch I hadn't seen earlier, and I think it's good to go.

The mountains you've moved for filter/action backward compatibility - rarely have I seen such true heroism.

Your sanity checks in bp_send_email() for content type and plugged wp_mail() look smart to me.

Green light on my end. Fine work, sir.

#54 @djpaul
8 years ago

In 10470:

Introduce base classes for new Email implementation.

  • Class BP_Email represents an email; the bits you see in your email client like the subject, message, recipients. It also encapsulates custom email headers and tokens.
  • Class BP_Email_Recipient represents an email recipient. It holds an email address and recipient name. If the class is instantiated with a WordPress user ID, then it also has a WP_User reference.
  • Interface BP_Email_Delivery is what mail delivery services will use to implement support for emails sent by BuddyPress.
  • Class BP_PHPMailer implements BP_Email_Delivery for PHPMailer.

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#55 @djpaul
8 years ago

In 10474:

Emails: add post type and taxonomy, and supporting wp-admin customisations.

All of BuddyPress' emails have been moved into email posts. The change includes an installation routine, but this won't be run until the db_version number is bumped in a subsequent commit. Updates to functions using wp_mail will also follow.

Tokens are used to personalise the email content (e.g. to add a link to the recipient’s user profile). Each type of email in BuddyPress has been assigned a unique type, and these are mapped to the email post through a new "email type" taxonomy.

The change includes a new HTML email template based on work by Ted Goas and contributors from the Cerberus email templates project. Ted, thank you very much. Learn more at http://tedgoas.github.io/Cerberus/

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#56 @djpaul
8 years ago

In 10475:

Emails: add Customizer integration.

This integration allows appearance changes to be easily made to emails, and previewed. For example, font size, background and font colors, and more.

A new submenu has been added under wp-admin's "Appearance" menu that takes the current priviledged user directly to the Customizer preview for a randomly-selected email.

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#57 @djpaul
8 years ago

In 10476:

Emails: add tool to reinstall emails.

This will likely see use on upgrade for non-en_US sites, as the language pack could load after the email posts have already been added to the posts table.

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#58 @djpaul
8 years ago

In 10477:

Emails: add default filters for email headers and tokens.

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#59 @djpaul
8 years ago

In 10478:

Emails: introducing bp_send_email, which is our replacement for wp_mail.

As wp_mail is a pluggable function, if we detect it has been re-declared, or if something has filtered to enable HTML emails, BuddyPress will fallback and let wp_mail send the email for us. This maintains compatibility for customised sites until mail delivery plugins are updated to work with BuddyPress (or new plugins are written).

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#60 @djpaul
8 years ago

In 10479:

Emails: refactor notification functions and use bp_send_email to send email.

Backwards compatibility is mostly maintained, though there are a few unavoidable instances where some data passed to filters is no longer sent (an empty string or equivalent is provided to avoid PHP Notices). These will be detailed in a post on bpdevel.wordpress.com in the feature to alert plugin developers.

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#61 @djpaul
8 years ago

In 10480:

Bump db_version to trigger email installation routine.

See #6592. Props timersys, mercime, boonebgorges, hnla, DJPaul.

#62 @Offereins
8 years ago

First, I got to say, mind = blown. So much work, lot's of pretty code. Thank you, sir Paul.

A few questions found me, while browsing through the code:

  • How does BP_Email_Recipient allow for registering multiple recipients at once in BP_Email::set_to|cc|bcc()? It seems to me that it only processes a single [address, name] set.
  • I think it would be very helpful to also have BP_Email::add_to|cc|bcc(). At the moment when filtering the email object, it would be difficult to just add a single extra recipient, since all you can do is reset each entire recipients list.
  • The Customizer link under the Appearance menu is found in the Network admin, while the email posts are managed on the Site level. Perhaps this could be reconsidered.

That's it for now.
This has a lot of potential, so you have done a wonderful job!

Edit: added another question.

Last edited 8 years ago by Offereins (previous) (diff)

#63 @imath
8 years ago

@DJPaul

About

The Customizer link under the Appearance menu is found in the Network admin, while the email posts are managed on the Site level. Perhaps this could be reconsidered.

On a multisite config, having BuddyPress activated on the network, every blog of the network has a new Emails menu. The root blog is listing the emails on the edit screen while all others are listing none.

If it's not possible to have this menu at the network level, i think it's important only the root blog should have the Emails menu & interfaces.

#64 @djpaul
8 years ago

In 10481:

Emails: update set_to, set_bcc, set_cc to correctly handle multiple recipients.

Also tweaks the PHPDoc to make the intended use of the array argument
clearer.

See #6592

#65 @DJPaul
8 years ago

@Offereins + @imath About the multisite menu location. I am not sure how to do this. Since you have the environments set up, can you test menu-location-01.patch please?

#66 @imath
8 years ago

@DJPaul sure, i will test it asap on multisite & multiblog

#67 @imath
8 years ago

@DJPaul The menu to launch the Email customizer wasn't the one i was thinking of actually :)

But, i find it weird to have this Theme submenu into the Network Admin, while anyway the Email Post Type Administration screens are not (& i think they cannot be) inside the Network Administration.

So i suggest to:

  • move the customizer menu on the root blog Administration,
  • make sure the Email post type is only registered for the Root blog, else, it displays the Email menu on each blog of the network

This is what's in the 6592-register-post-type-on-rootblog-only.patch.

#68 @Offereins
8 years ago

+1 for @imath's approach.

#69 @DJPaul
8 years ago

Thanks, imath. I'll test the patch and update it, but if we're going to do this, we also need to not register the taxonomy (which would otherwise be useless). I need to think/check if not registering the post type is safe.

#70 @r-a-y
8 years ago

And also for multisite, I think bp_get_email() will need a call to switch_to_blog( bp_get_root_blog_id() ) so the right email post type is fetched.

I haven't had a chance to review all this awesome work yet, but I hope to do so in the coming days.

Last edited 8 years ago by r-a-y (previous) (diff)

#71 @djpaul
8 years ago

In 10482:

Emails: new param for set_bcc|cc|to() to add to current values

The default behaviour remains to replace the current value.

This will allow plugin developers to easily append a new email
recipient without having to re-add the existing recipients.

See #6592

#72 @DJPaul
8 years ago

I think you're right, @r-a-y -- certainly it'll cover edge cases.

#73 @djpaul
8 years ago

In 10483:

Emails: swap out remaining old function names.

Prior to initial commit in BuddyPress, setter and getter methods were
renamed. These were missed, and the template call was causing PHP
Fatals.

Props mercime

See #6592

#74 @djpaul
8 years ago

In 10484:

Emails: make sure we are on root blog when fetching email posts.

Potential compatibility fix for various multisite configurations,
especially with third-party plugins.

Props r-a-y

See #6592

#75 @DJPaul
8 years ago

RE: register post type only on root blog

I am still thinking about if this should be done. There could be a problem where, if in a multisite environment, a secondary site calls bp_send_email, which would fail because the post type isn't registered. Since r10484 this would currently work because we switch_to_blog to fetch the data from the right tables.

#76 @imath
8 years ago

@DJPaul if you have a doubt, maybe you can just do 'show_ui' => bp_is_root_blog(). At least an empty list won't be shown to child blogs :)

#77 @imath
8 years ago

Also, using the site icon into the email header could be nice ;)

#78 @DJPaul
8 years ago

@imath I have tested using get_posts with post_type that is not registered and it does fetch matching posts from the database, so I think not registering the post type is OK for now. If testing finds some situation where this causes a problem, we can hide the menu item visibility, like you suggested.

#79 @djpaul
8 years ago

In 10485:

Emails: only register email post type for root blog.

Prevents the email post type appearing in other sites' wp-admin areas in a network-activated multisite configuration.
An alternate approach would be to set show_ui to hide the menu item, but the post type may still end up being revealed in other parts of admin screens, or through newer interfaces such as WP's REST API.

Props imath

See #6592

#80 @djpaul
8 years ago

In 10486:

Emails: make sure we are on root blog when redirecting to email customiser, or running the email repair tool.

See #6592

#81 @DJPaul
8 years ago

@imath I have merged some parts of your patch. The move of appearance > emails is ready in 6592-customizer-on-root-blog-1.patch but I thought of a larger problem when I was about to commit it:

When BuddyPress is network-activated, all BuddyPress menus are moved to the Network Admin. e.g. the Groups and Activity management screens. Those items are not accessible from the root Site Admin.

So, should the Emails menus be moved into the Network Admin, and hidden from the Site Admin? For consistency with how everything else works?

This is a little tricky since it's a post type -- we'd probably have to hide the Emails item from the Site Admin, and make the Network Admin menu link to the root Site Admin with the direct link to the Emails item.

#82 @imath
8 years ago

@DJPaul yes, there's no network admin screens for post types.

I think, in this case, i would simply add an admin menu into the network admin > Emails which would redirect to the root blog admin or display a page explaining the Emails can be managed from the root blog.

As you said it's too tricky, i had the same problem for edit-tags and the BP Groups Taxo plugin and here's how i did > https://github.com/imath/bp-groups-taxo/blob/master/bp-groups/bp-groups-admin.php#L330

#83 @imath
8 years ago

@DJPaul fyi i've updated imath-buddypress.wpserveur.net to trunk and tested the BP email feature with an activity mention.

and it looks like the site name is not customized into the email subject, see attached screenshot.

@imath
8 years ago

#85 @djpaul
8 years ago

In 10491:

Emails: move static token replacement function into its own function.

The token replacement, while intended for BP_Email, in theory could be used anywhere else, and having a common function would allow consistent filtering.
It also helps by reducing the amount of code in the BP_Email class, and therefore overall complexity.

See #6592

#86 @djpaul
8 years ago

In 10494:

Emails: fix name of action used to implement backwards compatibility with old BuddyPress.

Props imath

See #6592 and #6869.

#87 @djpaul
8 years ago

In 10500:

Emails: un-deprecate bp_activity_sent_mention_email action.

All functions where we send an email trigger a final action after the email's sent. The recent changes for BP 2.5's email overhaul moved all of these into the 2.5 backwards compatibility file, which is triggered by the new "email sent" actions and filters in bp_send_email().

Subsequent investigation of all similar actions revealed that only bp_activity_sent_mention_email had core functions hooked to it. Also, bp_activity_sent_mention_email was the only action hook that *always* triggered -- the other similar actions did not trigger if the recipient had chosen to disable email notifications. This change moves the action back into its original location, fixing unit tests and notification functionality, but it does not pass the subject/content/message arguments to the action as we no longer have access to those outside of an BP_Email instance.

Follow-up is required to ensure the 2.5 backwards compatibility code is being loaded correctly, or if this was some quirk of our unit test environment.

See #6592

#88 @djpaul
8 years ago

In 10501:

Emails: fix parameter order in backwards compatibility code for old email actions.

See #6592

#89 @djpaul
8 years ago

In 10503:

Tests, Emails: use real email type, rather than dummy.

This will slow the tests a bit as a Post will be queried for, but by using a genuine email type, we trigger the 2.5 backwards compatibility code, allowing us to test both the class and anything we have hooked into it by default.

See #6592

#90 @djpaul
8 years ago

In 10504:

Emails: preserve multiple "to" addresses in the backwards compatibility code.

See #6592

#91 @djpaul
8 years ago

In 10505:

Tests, Emails: update some tests for recent email type change.

In the real world, the activity-at-message email type always has the poster.name token set prior to the call to bp_send_email(), which isn't what we're testing here; so we need to set it here as it's used in an action provided for backwards compatibility.

See #6592

#92 @r-a-y
8 years ago

I ran into some issues with tokens including backslashes and plain-text content not being HTML entity-decoded.

decode.patch is an attempt to fix this. I haven't looked too much into the filter firing yet, but I'm not sure why the 'bp_email_set_content_plaintext' and 'bp_email_set_content_html' filters do not cover tokens. After talking things over with Paul, I was wrong about the filters I was using before ;)

The patch also includes a better "Go to the discussion link" for private messages.

Last edited 8 years ago by r-a-y (previous) (diff)

@r-a-y
8 years ago

Updated patch due to feedback from Trac.

#93 @r-a-y
8 years ago

In 10512:

BP Email: Decode HTML entities for plain-text email content.

See #6592.

#94 @r-a-y
8 years ago

In 10513:

BP Email: Enhancements for private message emails.

  • Use stripslashes() for message subject and content.
  • Use the full message thread URL.

See #6592.

#95 @djpaul
8 years ago

In 10536:

Emails: only register post type and taxonomy on root site.

Prevent the post type appearing on non-root site wp-admin screen in a
network-activated multisite configuration, while retaining single-site
and multiblog compatibility.

A subsequent change will add support for these items to the network
admin for network-activated multisite configuration.

See #6592

#96 @djpaul
8 years ago

In 10538:

Emails: add email menus to network admin for network-activated configs.

This provides consistency with the rest of BuddyPress; namely, that when network activated, you manage all BuddyPress content from the network admin.
With post types and taxonomies, we have to do a little extra work to add links to the root site on the network-admin, because of API limitations in WordPress.

See #6592

#97 @djpaul
8 years ago

In 10539:

Tests: new base class for BP_Email tests.

The email posts were being emptied out before the tests run because of the way the unit tests truncate the database tables.

See #6592

#98 @djpaul
8 years ago

In 10540:

Emails: handle PHP Notices in backwards compatibility code when a "to" recipient has not been set for an email.

This shouldn't normally occur, but can in certain failure modes, and in unit tests.

See #6592

#99 @djpaul
8 years ago

In 10541:

Unit Tests, Emails: don't un-hook core email actions for unit tests.

The missing default properties cause confusion when writing new tests, and the absence could cause a badly-written test to pass.
The change updates the affected unit tests to correctly handle the default properties.

See #6592

#100 @djpaul
8 years ago

In 10556:

Emails: only support non-string format callable functions for token values.

Prevents calling functions if the token value matches a class or function name.

See #6592

#101 @DJPaul
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed
  • Type changed from idea to enhancement

Calling this done! Thanks to everyone who helped build this, and tested it and gave feedback.

Any new issues during beta, please open new tickets.

Note: See TracTickets for help on using tickets.