Remove $filter variables holding hook names.

[tw2113]

There are currently four files that create a $filter variable that holds the filter hook name and are passed in to apply_filters(). The four files are bp-activity-notifications, bp-groups-notifications, bp-friends-notifications, and bp-messages-notifications because of how these are done, it's notably more difficult to set up documentation for the hooks, as well as makes things a bit more complex than they need to be.

I think a more optimal solution is to make parts of the hook names variable, depending on context in the surrounding code, instead of the whole hook. Think wp_insert_{$post->post_type}

For the activity one, it's simple enough as there's only two to deal with, and it's bp_activity_multiple_at_mentions_notification vs bp_activity_single_at_mentions_notification. In place of the whole hook, I have a diff ready to be created that changes $filter to $amount, and the apply_filters() become bp_activity_' . $amount . '_at_mentions_notification or bp_activity_{$amount}_at_mentions_notification depending on your preference for single vs double quotes.

Messages will be the same situation, except the hook is bp_messages_{$amount}_new_message_notification

Friends will be a bit more, except there will be two spots to change out. bp_friends_{$amount}_friendship_{$action}_notification with $amount being the same as above, and $action being "accepted" or "request".

Lastly, the Group notifications actually does the variable needlessly, as the apply_filters calls are all done within their respective if/else blocks. It'd be better to just remove that usage completely.

[DJPaul]

I think if this kind of change makes the documenting easier, then let's do it, as long as the overall filter names don't change, or the arguments passed to them.

We could also make sure the complete action names (without variables) are contained within the documentation added, please, or at least nearby in a separate code comment, so people can find them easily when doing a "find in files".

[tw2113]

Agreed with keeping a "complete" version of each nearby. I'll get to doing some patches for this soon.

[tw2113]

If we can get the bp-friends-notifications diff file committed soon, I'll be able continue finishing up the Friends component hooks docs ticket #5942

[boonebgorges]

Change looks fine. I'm going to add unit tests.

[boonebgorges]

In 9153:

Assemble the filters in friends_format_notifications() dynamically.

This makes them easier to document.

Props tw2113.
See #5970.

[Mamaduka]

Assembles filters dynamically in groups_format_notifications().

Also include hook documentation for those filters.

[boonebgorges]

Thanks, Mamaduka! I will feel more comfortable committing these if we have unit tests first, along the lines of what I wrote here: ​https://buddypress.trac.wordpress.org/browser/trunk/tests/phpunit/testcases/friends/notifications.php?rev=9153 If you write them, it'll get done faster, wink wink :)

[Mamaduka]

boonebgorges,

Wasn't sure how to write unit tests for this this. Thanks for the reference will add patch for tests well.

[boonebgorges]

Excellent, thanks! You should be able to copy those other tests more or less directly. The important thing is just to verify that the filters being fired are exactly the same pre- and post-patch. Thanks!

[Mamaduka]

boonebgorges,

Attached updated patch with tests.

[boonebgorges]

In 9170:

Assemble group notification hooks dynamically.

This makes them easier to document using WP's filter documentation standards.

Props Mamaduka.
See #5970.

[tw2113]

Filter name changes.

[tw2113]

Filter name changes unit tests.

[tw2113]

Filter name changes + unit tests for them. The tests are passing, but if possible, I'd appreciate someone looking them over to see if there's anything notable needing changed.

Once these are in place, I'll be able to continue with #5945

[boonebgorges]

In 9208:

Assemble message notification filters dynamically.

This allows us to centralize the documentation.

Props tw2113.
See #5970.