Allow requested table names to be filtered

[christianwach]

Hi Karim,

A fresh PR as promised in #95, this time with added persuasion :-)

First off then, let me try flattery: my collaborators and I love the Groups plugin, so a big thank you for developing and maintaining it!

Secondly, let me say that this kind of filter is by no means unusual - many major WordPress plugins allow the requested table name(s) to be filtered. For example, here's where BuddyPress does this. What this means is that the schema can be altered for different contexts by other plugins. BP Multi Network (FWIW written by Ron Rennick, the "godfather" of WordPress MU) enables multiple BuddyPress communities in WordPress Multi-Network installs. The BuddyPress Multi Network plugin, on the other hand, enables multiple BuddyPress communities in WordPress Multisite installs. My point is that different schemas can be implemented by different plugins to suit the context with one simple filter.

As it stands, the Groups plugin assumes that every sub-site in a multisite network requires its own capabilities management. It creates its standard set of Groups tables per site - which might be described as the reverse default schema that BuddyPress implements (i.e. one community per Multisite) - and enforces separation of groups per site. This is understandable and sensible as a default. But Multisite is a flexible beast and it is by no means a given that each sub-site is used as a "standalone". Many Multisite instances (leaving Multi-Network out of this for the time being) use sub-sites as useful "containers" for various purposes, for example BP Group Blog enables a "blog" for a BuddyPress group where the content may be pulled in to the main site without users being aware that there's a sub-site at all. It's in this kind of context that this PR fits.

So let me describe my own use case, which I don't see as particularly unusual. I have a Multisite install where certain sets of capabilities need to be identical across the entire network. This is because those capabilities are synced with CiviCRM and users need to retain those capabilities regardless of the sub-site they happen to interact with CiviCRM through. CiviCRM implements ACLs internally via its own groups but does not expose those to WordPress by default. Syncing capabilities, therefore, is taken care of via the CiviCRM Groups Sync plugin that I have developed - using Groups as the means by which the capabilities are propagated across the Multisite instance.

In my case I would rather not keep all sub-site Groups tables in sync with the ones on the main site. I could, of course, do that but it seems fragile and contravenes the DRY principle. The filter in this PR therefore gives the Groups plugin the same flexibility that's baked into BuddyPress because I can now restrict Groups to a single point of reference for the user capabilities I'm interested in, i.e. retaining just the tables on the main site:

// Prevent Groups from creating and destroying per-site tables.
remove_action( 'wpmu_new_blog', 'Groups_Controller::wpmu_new_blog', 9, 2 );
remove_action( 'delete_blog', 'Groups_Controller::delete_blog', 10, 2 );

// Prevent Groups from removing users from groups when removed from a blog.
remove_action( 'remove_user_from_blog', 'Groups_User_Group::remove_user_from_blog', 10, 2 );

// Filter the referenced tables.
add_filter( 'groups_get_tablename', 'my_groups_database_name', 10, 3 );

/**
 * Filter the database tables to query.
 *
 * @param str $table_name The constructed table name.
 * @param str $name The requested table name.
 * @param str $prefix The WordPress table prefix.
 * @return str $table_name The modified table name.
 */
function my_groups_database_name( $table_name, $name, $prefix ) {

    global $wpdb;
    static $my_db_prefix = '';

    // Get the main site prefix if we haven't already done so.
    if ( empty( $my_db_prefix ) ) {
        $my_db_prefix = $wpdb->get_blog_prefix( get_main_site_id() );
    }

    // Construct the table name.
    $table_name = $my_db_prefix . GROUPS_TP . $name;

    // --<
    return $table_name;

}

The result (leaving caching and cache-busting aside) is that Groups operates as a "global" container for certain sets of capabilities whilst retaining all the UI loveliness that the Groups plugin provides for managing user capabilities in various locations in WordPress admin.

I am, of course, open to any suggestions you may have for implementing this functionality without the filter in this PR and look forward to your reply!

Cheers, Christian

[christianwach]

@proaktion Hi Karim, have you had a chance to review this?

Cheers, Christian

[christianwach]

@proaktion Hi Karim, it would be great if you could look at this PR.

Thanks, Christian

[christianwach]

@itthinx @proaktion Hi there, here's my monthly ping on this issue... it would be great if someone could engage in a conversation with me about this.

Thanks, Christian

[christianwach]

@itthinx @proaktion Hello again, here's my latest monthly ping on this issue... it would be great if you could talk to me about this.

Thanks, Christian

[christianwach]

@itthinx @proaktion Here's my monthly ping on this issue... it would be great if you could talk to me about this. I don't bite!

Thanks, Christian

[proaktion]

@itthinx @proaktion Here's my monthly ping on this issue... it would be great if you could talk to me about this. I don't bite!

Thanks, Christian

Hi there Christian, I had your PR bookmarked to further review it. Thanks for your patience!

With your explanation (thanks for going into such detail!), the conclusion to which I come is that it would be sufficient to be able to filter $wpdb->prefix which would actually be a sensible feature to have within WP itself ... having a $wpdb->get_tablename( 'foo' ) and/or $wpdb->get_prefix() (both with a filter hook) as a canonical solution to obtaining the name of a table for a given instance would be my proposition. I imagine that there would be tons of different opinions on providing that function and including a filter inside which would allow you to modify the outcome.

Anyhow ... as for Groups, I do not think that allowing to filter the table names it uses would be a good idea. For your purpose, it might be interesting to allow the prefix to be filtered so you can point it towards the instance (e.g. of the main blog) with which you want to work instead of the particular's blog instance. Well that seems to be the theory, I'm not sure about the implications though. You seem to have been testing this already and I would assume that it works for you, right? Did you see any potential issues with pointing to the main blog's prefix in every instance while using Groups?

[christianwach]

@proaktion Hi Karim, a rather late thank you for your considered thoughts on this PR. On reflection, I agree with what you say about the filter implementation in this PR - I understand (from what you say) that the filter is overreaching and that (like BuddyPress) it should only be the prefix that is filterable. With that in mind, I'm closing this and opening a new PR with a more conservative filter for your consideration.

Did you see any potential issues with pointing to the main blog's prefix in every instance while using Groups?

No, there have been no issues - it's working beautifully as a "global" capabilities management method.