itthinx / groups

Groups provides group-based user membership management, group-based capabilities and content access control. It integrates standard WordPress capabilities and application-specific capabilities along with an extensive API.
GNU General Public License v3.0
49 stars 35 forks source link

Questioning The Value Of The Where Pattern #143

Closed richpeck closed 5 months ago

richpeck commented 11 months ago

I've been using this plugin for a client and have been looking at using one of the groups as a means to exclude posts.

This is not covered by the core functionality and so I have been investigating how to create it myself.

In doing so, I happened to find the code you've used to edit the "where" part of the query.

I am unsure as to why this exists in the form it does?

After looking through your code, and the subsequent entries in the database, why don't you just hook into the pre_get_posts action and change the query to pull in certain meta_query values? To give an example: -

add_action('pre_get_posts', function($query) {

    $group = user_is_trade() ? 9 : 10;

    $query->set('meta_query', array(
        'relation' => 'OR',
        array(
            'key'     => 'groups-read',
            'value'   => $group,
            'compare' => 'IN'
        ),
        array(
            'key'     => 'groups-read',
            'compare' => 'NOT EXISTS'
        )
    ));

});

That's the code I have used to override your where query. It uses a meta_query to check for posts which a) don't have any "groups-read" meta keys (IE they have no group restrictions) and if they do have a key, then its value must be one of the ones stipulated in the code. Obviously, you'd embellish this with the various group ID's in your code etc.

My point is this is not only more succinct, but in line with the Wordpress hooks system. Rather than manually editing the query, you are able to leverage the extensive Wordpress backend to get the result you want.

I'm interested to know why this approach was not taken?

Thanks

itthinx commented 5 months ago

Hi,

Thanks for the considerations, as this was posted when 2.18.0 was the current version, here is the link to the point in question in that version: https://github.com/itthinx/groups/blob/55d272e25989f47d7b33f18733ba113ee63703be/lib/access/class-groups-post-access.php#L314

Among the reasons against using such a meta query as you suggest would be performance considerations and certain control over the outcome that the query would produce. However, at this point, it would be wise to even review the way this query is handled and likely do more processing in PHP and make the overall query less heavy, is possible.

Note taken to review and benchmark possible solutions, closing the issue but feel free to follow up with any thoughts.

Cheers