Add filters: groups_user_is_member, groups_user_get_{$name} and groups_group_get_{$name}

[JoryHogeveen]

Allow short circuit the results of:

• a user-group relation
• a groups-user property
• a groups-group property

Enhances integration possibilities with other plugins, in this example, see: https://wordpress.org/support/topic/integrate-groups-with-view-admin-as/#post-8395011

[itthinx]

reversed approach - should be applied on result instead

[JoryHogeveen]

This method is essentially a shortcut which will prevent any unnecessary logic to run and take up resources.
The same method is used in WP core many time.

[proaktion]

I would apply similar considerations related to short-circuit filters as I mentioned in https://github.com/itthinx/groups/pull/59/files#r1450356957 but particularly on this instance there is a lack of context for the filter - in what context is the property accessed - which can lead to erroneous uses of the filter that can have undesired side-effects on the normal processing.

I wonder whether this would still be needed for https://github.com/JoryHogeveen/view-admin-as/blob/master/modules/class-groups.php or if the addition of the (modified) groups_user_is_member filter is sufficient to cover the needs of that module.

[itthinx]

reversed approach - should be applied on result instead (before returned)

[JoryHogeveen]

This method is essentially a shortcut which will prevent any unnecessary logic to run and take up resources.
The same method is used in WP core many time.

[proaktion]

This filter has been added with a slightly different signature

groups/lib/core/class-groups-user.php

Line 388 in 56957ae

$filtered_result = apply_filters( 'groups_user_is_member', $result, $this, $group_id );

and allows to filter the result. The 2.20.0 branch has also had various instances refactored that relied on the Groups_User_Group::read(...) method and which are now using the Groups_User::user_is_member(...) or $group_user->is_member(...) methods where appropriate so that the new filter kicks in. Extensions to Groups that were using the former method, should be updated to use the new ones.

As to the short-circuit filter suggested, which would allow to preemptively return a value before Groups polls its data, although this pattern is used in WordPress core, I see that it's basically done for two reasons: to evaluate something that is needed in a function for further processing (not overriding the actual processing of a return value), or to preemptively provide an alternative way to evaluate the return value (in the form of pre_... filters). For the latter, one could make a case for every single function to provide such a short-circuit filter for the sake of extensibility, but to me that's anti-pattern terrain. Instead, I would rather go the way of using OOP-based extensibility, for example, by introducing factories that allow to provide alternative classes to the default ones. Otherwise we would end up adding pre_... and $result filters on each and every method and property, adding overhead in processing for custom cases. I think that this could be handled better by using proper inheritance and factories.

[itthinx]

reversed approach - should be applied on result instead (before returned)

[JoryHogeveen]

This method is essentially a shortcut which will prevent any unnecessary logic to run and take up resources.
The same method is used in WP core many time.

[proaktion]

See https://github.com/itthinx/groups/pull/59/files#r1450540422

[itthinx]

Reviewing open requests, this seems to have been missed and lost in time. The filters suggested are valid but the points of application need to be reviewed (comments added). We will incorporate the suggestions into the itthinx:PR59-vs-2.19.0 branch and refactor them there for integration into master.