Skip to:

Opened 3 years ago

Last modified 8 months ago

#4689 new defect (bug)

bp_core_email_from_name_filter() is too aggressive

Reported by: r-a-y Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 1.2.2
Component: Component - Core Keywords: needs-patch


bp_core_email_from_name_filter() hooks onto 'wp_mail_from_name'.

This is a little too aggressive because other plugins that send emails like Gravity Forms will use a different 'From' name, but that won't be seen because of BP.

I'd like to propose that we ditch this function and add the fourth parameter to each wp_mail() call in the BP codebase so we can modify the email headers (like the 'From' name) there.

Change History (8)

comment:1 @boonebgorges3 years ago

Big +1.

Instead of adding a headers param to each wp_mail() call, it might be more efficient (and easier to maintain and extend) to have a wrapper bp_mail() function. This might also be helpful if we decide to overhaul email notifications in the future.

comment:2 @DJPaul3 years ago

I think the filter exists because of some old version of WPMU. I am unsure if there's a reason to keep it nowadays. Suggest we just deprecate it, and be done.

See also #3270

comment:3 @r-a-y3 years ago

I knew there was another ticket for this, but couldn't find it! Thanks for finding it, Paul.

I just looked into this briefly and it looks like we can't deprecate the function.

If we don't set the 'From' name, it will default to "WordPress":

I think Boone's approach of creating a wrapper bp_mail() function is more flexible since we would only have to add the mail headers in one place instead of for each wp_mail() call like I originally suggested.

If we're all in favor (favour for Paul!), I'll create a patch.

comment:4 @DJPaul3 years ago

Is there a compelling reason to change the from name? Beyond the fact that we already do.

comment:5 @johnjamesjacoby3 years ago

  • Milestone changed from 1.7 to 1.8

Unhooking this is easy enough to do for those that need to do it. Agree a bp_mail() function makes huge amounts of sense, but too late in 1.7.

Punting to 1.8.

comment:6 @johnjamesjacoby3 years ago

  • Keywords needs-patch added; dev-feedback removed

comment:7 @r-a-y2 years ago

  • Milestone changed from 1.8 to Future Release

This one's going back to Future Release. No time for 1.8.

comment:8 @kashmiri8 months ago

#6160 merged here.

Last edited 8 months ago by kashmiri (previous) (diff)
Note: See TracTickets for help on using tickets.