bp_notifications_get_notifications_for_user() bug

[rogercoathup]

We pass an array to apply_filters_ref_array in bp_notifications_get_notifications_for_user()

i.e.

<?php
$ref_array = array(
        $component_action_name,
        $component_action_items[0]->item_id,
        $component_action_items[0]->secondary_item_id,
        $action_item_count,
        $format
);

The first parameter should be modifiable content for filtering, not the action name.

With the current setup, if a hook returns content, the action name is overwritten in apply_filters_ref_array() and invalid in any subsequent hooks hooked on bp_notifications_get_notifications_for_user.

[r-a-y]

Related: #6669, ​#bbPress2665.

The 'bp_notifications_get_notifications_for_user' filter is quite weird indeed. You either have to return a string of the content or array( 'text' => 'CONTENT', 'link' => 'LINK' ) depending on the $format. I think this needs better documentation.

If a plugin does change the value of $component_action_name, then no other plugin can do checks against $component_action_name. comment:8:ticket:6669 goes into the problem as well.

We probably need to add some additional parameters so the action is passed again for better checks:

<?php
$ref_array = array(
        $component_action_name,
        $component_action_items[0]->item_id,
        $component_action_items[0]->secondary_item_id,
        $action_item_count,
        $format,
        $component_action_name, // Duplicate this since this could be changed by plugins.
        $component_name,        // Component name is a good parameter to do checks against as well.
);

Unfortunately, this does not fix the issue. We might need to think about deprecating this filter for something else.

---

For an alternative way to formatting your custom notifications, read the latter half of comment:4:ticket:6669.

[r-a-y]

01.patch modifies the 'bp_notifications_get_notifications_for_user' filter to do the following:

• Introduces two new parameters - $component_action_name and $component_name as the 6th and 7th parameters to the filter.
• Deprecates the first parameter in the filter. It is recommended to do notification action checks against the 6th parameter from now on.
• Adds proper documentation for the filter.

[rogercoathup]

Thanks @r-a-y -- in this case, our component is derived from BP_Component, but I hadn't realised the class supported a notification callback to format notifications

[DJPaul]

Looks good, @r-a-y.

[r-a-y]

In 10725:

Notifications: Introduce two new parameters to the 'bp_notifications_get_notifications_for_user' filter.

This commit:

• Adds $component_action_name and $component_name as the 6th and 7th parameters to the filter.
• Updates the DocBlock for the filter.

In the DocBlock, we are deprecating the first parameter, which is the
component action name. It is recommended to do notification action name
checks against the 6th parameter from now on.

The reason why we are deprecating the first parameter is due to potential
plugin conflicts. If a plugin is doing a specific check on the notification
action and changes the return value for the filter, then no other plugin
can do checks for that same notification action. Also see #6669.

Fixes #7020.