Add item ID to bp_get_member|group_class() filters

[Offereins]

For improved filtering of the bp_get_member_class() and bp_get_group_class() functions filters, send along the current item's ID.

Patch attached.

[Offereins]

Adding the item's ID might also apply to other template tag filters. Perhaps this could be done in a single sweep?

[DJPaul]

There's a fine line between adding global variables to filters, instead of those callbacks just accessing the global variable directly (or using an existing wrapper function).

[dcavins]

Hi @Offereins-

Thanks for your patch. We were discussing this ticket this morning: ​https://wordpress.slack.com/archives/buddypress/p1459867704001333

and leaning toward not fixing it because the $members_template and $groups_template are global variables that can already be accessed in any function hooked to those filters.

What do you think?

[Offereins]

I noticed the talk. I'm not against it, though this was mainly coming from the view that touching globals is unfavorable, to say the least. There are indeed a lot more filters to change if we're going this route. But there is of course the family of bp_get_member_user_id() and the likes to read from the global template, so its fine with me.

Perhaps it is a (small) matter of education/inline docs on why/how the filters differ from the norm that we've come to know from WP and other filters.

[boonebgorges]

touching globals is unfavorable, to say the least.

I agree with this. I've generally advocated with *not* adding the values to the filters just to avoid code churn, but I could easily be persuaded otherwise.

As noted in the chat, I think, on balance, that it would be preferable to pass the member object (group object, etc) to the filter rather than the ID. But this introduces its own complications, such as the fact that, as far as I know, $members_template->member is a stdClass object and not something with proper structure. In any case, we should be consistent.

@Offereins If you want to take this up as a project, I'd encourage you to look through the bp-*-template.php files to get a sense of what the general strategy should be. For example, if there are lots of places where we're already passing IDs to filters in these sorts of functions, then maybe we should continue using IDs. Once we have broad decisions made, someone will need to begin generating patches, maybe one component at a time. These patches should include the necessary filter documentation changes (@since and @param).

[Offereins]

Filter updates for bp-members-template.php

[Offereins]

Filter updates for bp-groups-template.php

[Offereins]

@boonebgorges I'll refrain from adding filter parameters that are accessible through global template variables, to keep the single developer experience.

However, I just uploaded two patches for the members and groups template files. Added filter parameters are variables that are passed as function parameters or those that may have been touched within the scope of the filter's function. These are not accessible through globals or other template vars.

[Offereins]

Filter updates for bp-activity-template.php

[Offereins]

Filter updates for bp-blogs-template.php

[Offereins]

Filter updates for bp-friends-template.php

[Offereins]

Filter updates for bp-messages-template.php

[Offereins]

Filter updates for bp-notifications-template.php

[Offereins]

Filter updates for bp-xprofile-template.php

[Offereins]

Filter updates for bp-members-template.php v2

[boonebgorges]

In 10779:

Pass the $user_id param to the bp_get_last_activity filter.

Props Offereins.
See #6997.

[boonebgorges]

In 10780:

Pass the $group_id to the bp_get_profile_group_name filter.

Props Offereins.
See #6997.

[boonebgorges]

In 10781:

Pass $thread_id to a number of Messages template filters.

Props Offereins.
See #6997.

[boonebgorges]

In 10782:

Pass object IDs to a number of Friends template filters.

Props Offereins.
See #6997.

[boonebgorges]

In 10783:

Pass $user_id to the bp_get_total_blog_count_for_user filter.

Props Offereins.
See #6997.

[boonebgorges]

In 10784:

Pass $user_id to a number of Activity filter templates.

Props Offereins.
See #6997.

[boonebgorges]

In 10785:

Pass parsed function parameters to a number of template filters.

The template filters in question are ones that get their main context from
a template global (say, $members_template), but get auxiliary information
about how to build markup from the function params. As such, it's useful to
have these parameters available via filter.

Props Offereins.
See #6997.

[boonebgorges]

In 10786:

Add missing parameter documentation to bp_loggedin_user_id and bp_displayed_user_id filters.

Props Offereins.
See #6997.

[boonebgorges]

In 10787:

Correct parameter documentation for bp_get_activity_avatar filter.

Props Offereins.
See #6997.

[boonebgorges]

I broke these up into what seemed like meaningful chunks:

• fixes to documentation
• instances where $r or $args is being passed as a chunk to filters
• instances where the object ID is being passed to a filter

I've left out the formatting mods. We follow WP's policy of not making these sorts of edit just to meet our own standards - it causes churn in the changelog that makes debugging and blaming more difficult.

Thanks for your patches, here - there are a couple places where I think that these new params will be really useful.

[Offereins]

@boonebgorges Thank you for taking this up and implementing it! Going through the commits, I noticed you just missed adding the actual $r parameter to the bp_get_member_profile_data filter - you only added the DocBlock.

Also thank you for explaining the code format policy. I just can't help myself scratching that itch when going through the code and suggesting improvements ;)

[boonebgorges]

In 10796:

Pass argument array to bp_get_member_profile_data filter.

Missed in [10785].

Props Offereins.
Fixes #6997.

[boonebgorges]

Going through the commits, I noticed you just missed adding the actual $r parameter to the bp_get_member_profile_data filter - you only added the DocBlock.

Oops! To be fair, you did the same thing in a previous patch in a couple of places :-P