Extending Messages notification should be improved

[imath]

If you try to add a new notification action (meaning !== 'new_message'), it's quite risky:

• First you get a notice error because in this case the $text var is not defined (which is not so serious)

• Most serious is you can't be sure to only filter your custom action because you need to filter bp_messages_single_new_message_notification which is not in the if ( 'new_message' === $action ) statement and which is not including the needed $action and $format variables.

Maybe we should improve this. See attached patch.

[boonebgorges]

Ugh, this was a pretty ugly one. Good catch, imath. Moving the new_message filter into the if ( 'new_message' === $action ) block, and introducing a new catch-all filter, both seem good to me.

It's a pity that these filters were originally introduced as dynamic. We could greatly simplify the documentation, and greatly decrease the number of filters, if each notification function had a single filter: bp_messages_format_notification (or whatever), which would accept an $action and $amount parameter in addition to whatever params they normally accept. This is not a criticism of 6750.patch​ - it just copies what the rest of BP is doing. Just registering my sadness for the record :)

[imath]

The patch should handle the string format.

[r-a-y]

02.patch handles the string format and also adds another edge case for the 'bp_messages_single_new_message_notification' filter. If $format is an array, there are seven parameters; if $format is a string, there are six...:
​https://buddypress.trac.wordpress.org/browser/tags/2.4.3/src/bp-messages/bp-messages-notifications.php#L212 :(

Not sure how people are using this filter at the moment!

[imath]

02.patch looks good @r-a-y

We just need to make sure the @since is bumped to 2.6.0 for the dynamic filter 'bp_messages_' . $action . '_notification'

[r-a-y]

In 10706:

Messages: Introduce dynamic filter, "bp_messages_{$action}_notification".

This new filter helps plugin developers filter private message
notifications that are not the 'new_message' action.

For backward compatibility, we apply the existing
"bp_messages_{$amount}_new_message_notification" filter just before
adding our new dynamic filter.

Fixes #6750.

Props imath.