#7077 closed enhancement (fixed)
Add a global scope option to the BP taxonomy wrapper functions
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | needs-patch |
Cc: |
Description
In a multi-network WP install, wp_user is shared by all networks. BuddyPress can be configured with global member profile tables. However, member type is implemented as a custom taxonomy and each site with BuddyPress active will have it's own copy of member type term object relationships.
This patch allows a site owner to override the default behavior and store term object relationships in the root blog of the primary network instead of the root blog of the current network. This allows member type to be consistent across all networks.
To turn on global scope for a BP custom taxonomy (bp_member_type) one would add the following function and filter to bp-custom.php
function my_custom_bp_taxonomy_scope( $scope ) {
return 'global';
}
add_filter( 'bp_taxonomy_bp_member_type_scope', 'my_custom_bp_taxonomy_scope' );
Attachments (4)
Change History (22)
#2
@
9 years ago
I think with great power comes great responsibility. I was limiting the scope to a strict hierarchy of options, but I like the wide open approach.
We plan on adding wrappers for other taxonomy functions as described in #6028 like wp_insert_term, term_exists, get_terms, etc. We want to provide an admin UI for an academic interests user taxonomy which will be used in user profiles and needs to be stored in the root blog of the primary network since users are global.
#3
@
9 years ago
I'll add a patch with all 3 taxonomy wrapper functions. The function get_site_by_path returns an object, so I've changed the filter function accordingly.
function bp7077_filter_taxonomy_storage_site( $site_id, $taxonomy ) {
if ( 'bp_member_type' === $taxonomy ) {
$main_network = wp_get_network( get_main_network_id() );
$site = get_site_by_path( $main_network->domain, $main_network->path );
}
return $site->site_id;
}
add_filter( 'bp_get_taxonomy_term_site_id', 'bp7077_filter_taxonomy_storage_site', 10, 2 );
#4
@
9 years ago
- Milestone changed from Future Release to 2.6
Thanks @rekmla ! I think this new approach is more flexible and agnostic about your network setup, which IMO is a big win.
The function get_site_by_path returns an object, so I've changed the filter function accordingly.
Ah yes. Can you tell I wrote it off the top of my head? :)
#5
@
9 years ago
bp_get_object_terms()
accepts multiple $taxonomies
, and the $taxonomies
may in certain cases be stored on different sites. I'll update the patch to accommodate this possibility.
#6
@
9 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 10818:
#7
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
As mentioned in Slack, the bp_register_taxonomies()
function now only runs when switching blogs. Since it doesn't run outside of the scope, wp_get_object_terms()
consequently returns an invalid_taxonomy
error for the current blog: the taxonomy isn't registered.
This can be fixed by moving the registration function outside of the switch block. Patch attached.
#8
@
9 years ago
Thanks very much, @Offereins. The first thing I'm seeing is that [10818] contains a variable typo, specifically in bp_get_object_terms()
. I'm going to fix this, and we'll see if that resolves your issue.
#10
@
9 years ago
I'm thinking through this more, and I'm having a hard time imagining why BP would fail to register its taxonomies on the current site, and so would need taxonomy registration as suggested in 7077.4.patch. @Offereins If you can provide any more info about your setup, it'd be very helpful.
I want to be cautious about re-registering taxonomies, as there may be cases where a plugin is doing just-in-time modification of these taxonomies, in such a way that re-registering unnecessarily would cause these changes to be overwritten. It may be better to do something like
$taxonomies_exist = true; foreach ( $taxonomies as $taxonomy ) { if ( ! taxonomy_exists( $taxonomy ) ) { $taxonomies_exist = false; break; } } if ( ! $taxonomies_exist ) { bp_register_taxonomies(); }
though even this is not perfect.
My preference would be to fix whatever problem (assuming it's a problem!) that's preventing the taxonomies from being registered in the first place :)
This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.
9 years ago
#12
@
9 years ago
@Offereins points out privately that the $switched
logic for restore_current_blog()
is backward.
I will follow up here with a fix for that as well as some conclusions from my discussion with him regarding the taxonomy-registration issue.
#13
@
9 years ago
- Keywords needs-patch added; has-patch removed
In addition, @Offereins points out that:
Also, I think the new
bp_get_taxonomy_term_site_id()
should be used inBP_User_Query::get_sql_clause_for_member_types()
when switching blogs
This should be verified for group type queries as well.
#16
@
9 years ago
- Resolution set to fixed
- Status changed from reopened to closed
I've spoken with @Offereins about the underlying issue, and I think we agree that it's due to an oddity of his setup. For the moment, let's not change the way taxonomies are registered in these functions. if anything comes up in the future, we'll look into more dynamic stuff (taxonomy_exists()
).
@rekmla Thanks for the ticket and the patch.
The question of where BuddyPress should store taxonomy data is a legitimate one. Multinetwork + members is the most obvious case, but there are almost certainly other situations where one might legitimately want terms from a given BP taxonomy to be stored on one site, and those from another taxonomy on a different site.
For this reason, the suggested patch seems a bit too specific to the multinetwork use case. It makes your specific use case very easy - a one-line filter - but certain other use cases are difficult.
A more general solution would be to filter the "term storage blog id". See 7077.2.patch. (I only applied the necessary changes to
bp_set_object_terms()
, but you get the idea.) Your implementation would then look like this:I've chosen to pass only the
$taxonomy
to the filter, because I'd think that in the vast majority of cases, this'll be the determining factor as to what gets stored where. But if it'd be useful,$object_id
could be passed as well.@rekmla What do you think? Also pinging @johnjamesjacoby, who has done a lot of thinking about BP in a multi-network environment.