Allow replacement of groups object in group component loading

[ddean]

Following up on: http://buddypress.org/community/groups/bp-group-hierarchy/forum/topic/issues-with-bp1-3-trunk-development-version/

A filter in bp-groups-loader.php that would allow $bp->groups->current_group to be replaced or altered before a 404 is sent would allow cleaner extension of the Groups component.

Attached is a patch offering one option for doing this.

[ddean]

groups_object filter proposal

[boonebgorges]

This approach seems sensible to me. Can we mention 'current' in the filter name? Maybe 'groups_get_current_group_object'

[DJPaul]

Is the objective to change which class is used, or to be able to modify the instantiated object?

[ddean]

Replying to boonebgorges:

This approach seems sensible to me. Can we mention 'current' in the filter name? Maybe 'groups_get_current_group_object'

That sounds great. It would better indicate the use of the resulting object.

[ddean]

Replying to DJPaul:

Is the objective to change which class is used, or to be able to modify the instantiated object?

My objective is to change which class is used.

[ddean]

groups_object filter proposal - new filter name

[DJPaul]

Then it needs to filter the *name* of the class to create, not the created object.

[boonebgorges]

Then it needs to filter the *name* of the class to create, not the created object.

Right. But we don't filter class or function names anywhere else in BP or WP (to the best of my knowledge). If you filter the group object, then you can still replace the current group object - you'll just be wasting a few CPU cycles and a DB hit or two.

In any case, I'm fine with a filter on the class name if everyone agrees that it's an OK precedent.

[ddean]

There are pros and cons to each way. This looks like the state of the discussion:

Filter instantiated object:

Pros

• more consistent with other BP and WP filters
• allows plugin authors to modify the group object without subclassing or creating a replacement BP_Groups_Group class

Cons

• uses more system resources

Filter class name:

Pros

• uses fewer system resources
• a better match for this case

Cons

• requires plugin authors to subclass or replace the BP_Groups_Group class in order to use the filter
• breaks with convention for filters in BP and WP

[boonebgorges]

Thanks, ddean. That seems like a fair assessment. All things being equal, I'd prefer to go with filtering the object, for the reasons stated (especially the idea that it won't be necessary to create a BP_Groups_Group subclass, as WP plugin development is generally not required to maintain OOP principles). DJPaul, do you have any strong feelings?

[DJPaul]

Requiring plugin authors to subclass BP_Groups_Group, or use a little OOP, is in no way a downside. We're going to expect certain methods and functions in that class, and if they are not implemented in a custom implementation, we're going to get bug reports which aren't our fault.

If the aim is to change parts of the existing class, sure, use apply_filters_ref_array. If it's to load in your own class, filter the class name before creation. Need a similar filter in similar places throughout BP. Up to you.

[boonebgorges]

I can see use cases each way. Can we go with 3312.diff, which is an implementation of my rallying cry "filters for everyone"?

[ddean]

I was digging around and realized that the bp-groups-functions file might be a better home for this filter. Putting it in groups_get_group() casts a wider net, which goes with DJPaul's idea of having similar filters in other places.

Or is it bad form to assume that BP_Groups_Component::_includes() will have run by the time the globals setup is running?

I'm attaching a proposal.

[ddean]

groups filter proposal - function-based

[DJPaul]

I prefer Boone's patch because the scope is narrower; if a plugin filtered into get_groups, it could perhaps affect another plugin, where there may be something bespoke going on. It fits the scope of this ticket better.

[djpaul]

(In [4735]) Allow replacement of groups object in group component. Fixes #3312, props boonebgorges and ddean