Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#6460 closed enhancement

core function for all calls to wp_mail

Reported by: shanebp's profile shanebp Owned by: djpaul's profile DJPaul
Milestone: Priority: normal
Severity: normal Version: 2.2.3
Component: Core Keywords:
Cc: shane@…

Description

There are ~15 calls to wp_mail.
Instead of separate calls, it would be useful to send them all to a core function, maybe bp_wp_mail() which would be the only function that calls wp_mail.

bp_wp_mail would have an apply_filters hook(s) before it calls wp_mail.

As clients increasingly use email tracking & template plugins ( Mandrill, etc), having a single function for all calls to wp_mail would make it easier than a slew of filter functions for all the wp_mail calls.

Currently, all the calls to wp_mail are preceded with apply_filters hooks. Those hooks would remain.

But instead of:

wp_mail( $to, $subject, $message );

Use:

bp_wp_mail( $to, $subject, $message, $type );
// $type tells you where it came from, for example: new_message

And the function is something like:

function bp_wp_mail( $to, $subject, $message, $type ) {
    // add apply_filters hook(s) here
    wp_mail( $to, $subject, $message );
}

Just wanted to float the idea before creating a patch.

Change History (14)

#1 @shanebp
9 years ago

And the call to wp_mail in bp_wp_mail would include all the parameters:

wp_mail( $to, $subject, $message, $headers, $attachments );

#2 @boonebgorges
9 years ago

We've talked in the past about centralizing email functionality - specifically, for the purpose of allowing HTML emails with admin-customizable templates. Maybe @r-a-y will have more to say about it.

To play devil's advocate for a moment: What can be accomplished with a bp_wp_mail() wrapper (plus BP-specific hooks) that couldn't already be done with the hooks in wp_mail()? It seems to me that any customizations you make to email originating from BP (templating, using an external service like Mandrill, etc), you would want to apply to *all* emails coming from your system - including those sent by plugins that don't use bp_wp_mail() as well as those generated by WP core.

#3 @shanebp
9 years ago

What can be accomplished with a bp_wp_mail() wrapper (plus BP-specific hooks) that couldn't already be done with the hooks in wp_mail()

For example: the hooks in wp_mail() don't tell you where in BP the call came from.
That knowledge is useful for several scenarios. For example, assigning different email templates.

#4 @shanebp
9 years ago

Services like Mandrill hook into wp_mail or can be called directly which makes it possible to assign a specific template.

The filters in bp_wp_mail() could allow you to make direct calls to a service.
And maybe return a variable that kills the wp_mail() call.

#5 @r-a-y
9 years ago

For example: the hooks in wp_mail() don't tell you where in BP the call came from.

We could do this by adding a BP-specific email header like X-BuddyPress: mention to each type of BP email, then you would be able to identify this with the 'wp_mail' filter, although it would still be a pain to parse out all email headers for this one header.

This sounds like the most sensible plan for us.

templating

I've had an idea for awhile now to do something like how imath did for Attachments. Allow bp-legacy to bundle email templates. Although boonebgorges does have a point about how "you would want to apply to *all* emails coming from your system" to use the same template.

It seems to me that any customizations you make to email originating from BP (templating, using an external service like Mandrill, etc), you would want to apply to *all* emails coming from your system - including those sent by plugins that don't use bp_wp_mail() as well as those generated by WP core.

These are essentially the main issues in determining whether we should even have a core BP email solution. Or whether it makes sense for these type of ideas to be part of a WP email plugin.

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

#6 @espellcaste
9 years ago

My two cents about this.

I've been dealing with emails in BuddyPress recently as I want custom HTML for some of the transactional emails sent by BuddyPress, which by the way, it's a little hard at first.

For example, the activation key email doesn't have a filter to change the email from plain/text to HTML, forcing the developer to recreate the function in a plugin to accomplish that.

I find the WooCommerce organization for email very interesting and something that BuddyPress could inspire for.

https://github.com/woothemes/woocommerce/blob/master/includes/emails/class-wc-email.php

It's well organized, it's easy for a non-developer to customize and change the content to the way one likes. All the emails are organized in one place, currently it's very hard to have idea of how many emails are sent by BuddyPress, unless you search.

Also, bringing all emails call to the bp_wp_mail wrapper seems a good idea, but doesn't make it easy with the points I'm outlining above.

#7 @shanebp
9 years ago

This sounds like the most sensible plan for us.

So the sensible plan is to avoid a bp mail wrapper and add a header that is a "pain to parse"?

Although boonebgorges does have a point about how "you would want to apply to *all* emails coming from your system" to use the same template.

But you may or may not want to use the same template - which is why mail services allow you to create multiple templates.

I'm not suggesting the inclusion of BP email templates - that is a separate ticket, imo.

Or whether it makes sense for these type of ideas to be part of a WP email plugin.

Again - if you don't know which part of BP made the call to wp_email, how useful is a WP email plugin ?

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

#8 @DJPaul
9 years ago

  • Milestone changed from Awaiting Review to Future Release

I'm positive we have a couple of good trac tickets about mailing things, but as this has some good contributions, going to keep it open while we plot a way forwards.

Best way to get this improved is to help out during our next release cycle (2.4) and spend some time on it. We could chat about it at a dev chat or a hangout sometime if there were a few of us with strong opinions about BuddyPress and emails. :) (Ray and myself definitely are).

#9 @shanebp
9 years ago

We could chat about it at a dev chat or a hangout sometime

Either way is fine by me. Perhaps a slack hangout is better.
Does someone care to add this to an agenda or schedule a chat time?

#11 @DJPaul
9 years ago

  • Milestone changed from Future Release to 2.4
  • Owner set to DJPaul
  • Status changed from new to assigned

#12 @DJPaul
9 years ago

  • Milestone 2.4 deleted
  • Status changed from assigned to closed

Closing in favour of #6460. Everything said here will be taken into account.

#13 @DJPaul
9 years ago

Instead of #6592, doh.

#14 @DJPaul
8 years ago

  • Component changed from API to Core
Note: See TracTickets for help on using tickets.