Email taxonomy registered on wrong site in multisite

[needle]

This appears to be an edge case in the upgrade process: when I updated BP via SVN, it just so happened that the first page load thereafter was on a sub-site. This caused the 'bp-email' CPT and "BuddyPress email types" taxonomy to be installed on that sub-site, apparently because bp_update_to_2_5() ran on that site rather than the main site. The CPT and taxonomy are now orphaned on that site and are not removed via the "Reinstall emails (delete and restore from defaults)" tool in Network Admin.

[needle]

To clarify, this when BP is network activated.

[imath]

Thanks for your feedback @needle I confirm and was able to reproduce by switching branches. All emails were created on the subsite

switching to blog seems to create the emails on the right site, but the email taxo is not created.. @DJPaul what about using bp_set_object_terms() instead of wp_set_post_terms() in bp_core_install_emails() ?

Attached patch needs to be improve to also fix the taxo trouble.

[DJPaul]

I would prefer to switch blogs in a higher-level function. We might conceivably have this problem in the future with other content in the future. Maybe we should consider doing a switch_to_blog for the entire installation process. ?

[imath]

@DJPaul yes probably.

But i'm *very* concerned that on my multisite install (WP Trunk), when BuddyPress is network activated, no terms are created, no relationships are created between the emails and the terms...

On this multisite install, if i activate BuddyPress on the root blog only or when i activate BuddyPress on a regular site i have terms added and the relationship between the email and term is created...

[DJPaul]

Replying to imath:

@DJPaul yes probably.

But i'm *very* concerned that on my multisite install (WP Trunk), when BuddyPress is network activated, no terms are created, no relationships are created between the emails and the terms...

On this multisite install, if i activate BuddyPress on the root blog only or when i activate BuddyPress on a regular site i have terms added and the relationship between the email and term is created...

Yes, that's dealt with by #6891. This ticket is about where the posts are added.

[imath]

6892.02.patch is switching blog inside bp_version_updater().

The patch on #6891 is fixing the taxonomy issue when the upgrade happens when on the root blog or in the network admin.

But if we are in the edge case Christian shared here: being in a subsite when upgrading, the terms are not created. Maybe the blog hasn't switched yet when the hook bp_register_taxonomies is fired.

Should we also switch to blog inside bp_register_default_taxonomies() ?? or do something like the following snippet inside bp_core_install_emails() ?

if ( ! taxonomy_exists( bp_get_email_tax_type() ) ) {
   do_action( 'bp_register_taxonomies' );
}

[boonebgorges]

This is going to be a problem whenever we depend on post types, taxonomies, post status, etc that are registered only on the root blog. It's likely that a similar bug could be reproduced with Member Types, for example, under the right circumstances. See ​https://core.trac.wordpress.org/ticket/20541.

In addition to whatever we might do during the upgrade routine, it may be safest to introduce some additional logic into our low-level functions that ensures that taxonomies and post types are registered during switch_to_blog(). See 6892.diff​.

[boonebgorges]

(And in fact, to make sure that the switched environment is always predictable, we might consider a bp_switch_to_root_blog() wrapper that handle the reregistration of content types.)

[imath]

I like @boonebgorges 's idea.

The only trouble is, in the case of emails, it would require some extra wrapper functions, as bp_core_install_emails() is not using one of the "BP Taxonomy functions" but wp_set_post_terms().

So ideally we'd probably need something like bp_insert_post_type( $post_type, $args ), bp_set_post_terms(), bp_get_post_terms() ... in which we would switch/reset blog.

That's, in 03.patch why i've added the switch/reset inside the bp_core_install_emails() as it's used on install or upgrade, and it's the first and only place we need it so far.

Oops, i forgot to mention 03.patch is fixing this bug :)

[DJPaul]

I am reluctant to introduce a helper function for switch_to_blog that registers post types and taxonomies this late into the cycle. The idea seems a bit crazy to me, and maybe over-engineering at this point. But maybe WordPress is just crazy.

[DJPaul]

Also, for the specific instance of installing the emails, bp_set_object_terms isn't adequate because I have to make a call to wp_update_term to update each term's descriptions. Trying to create wrappers for every single taxonomy function in WordPress seems like a massive amount of extra code.

[DJPaul]

Boone's 6892.diff patch is a worthwhile improvement to commit regardless of anything else. However, I've iterated on it in 6892.04.patch to remove the calls to bp_register_post_types() because WP doesn't check if a post type is registered before you use it (at least it doesn't in wp_insert_post and in WP_Query).

[DJPaul]

I like the combination of the following patches, which is my suggestion:

• ​https://buddypress.trac.wordpress.org/attachment/ticket/6892/6892.04.patch
  • To make sure taxonomy installation function is called if someone uses existing BP term wrapper functions.
• ​https://buddypress.trac.wordpress.org/attachment/ticket/6892/6892.02.patch
  • To switch the main installation routine to happen on the root blog.
• ​https://buddypress.trac.wordpress.org/attachment/ticket/6891/6891.01.patch (from another ticket but related to this one)
  • To register the email taxonomy inside the network admin to fix the repair tool.

I think this the lightest way to resolve the current blocker issues, and if there is interest, we can move the discussion about a BP switch_to_blog wrapper and post/taxonomy wrapper functions to another ticket for a future release.

[boonebgorges]

Replying to DJPaul:

I like the combination of the following patches, which is my suggestion:

• ​https://buddypress.trac.wordpress.org/attachment/ticket/6892/6892.04.patch
  • To make sure taxonomy installation function is called if someone uses existing BP term wrapper functions.
• ​https://buddypress.trac.wordpress.org/attachment/ticket/6892/6892.02.patch
  • To switch the main installation routine to happen on the root blog.
• ​https://buddypress.trac.wordpress.org/attachment/ticket/6891/6891.01.patch (from another ticket but related to this one)
  • To register the email taxonomy inside the network admin to fix the repair tool.

I think this the lightest way to resolve the current blocker issues, and if there is interest, we can move the discussion about a BP switch_to_blog wrapper and post/taxonomy wrapper functions to another ticket for a future release.

+1 to all of this. I think we *do* need wrappers, crazy as it may be, but it doesn't need to be done for 2.5.

[imath]

I like results!

6892.03.patch + ​https://buddypress.trac.wordpress.org/attachment/ticket/6891/6891.01.patch is fixing this ticket edge case: running the upgrade routine while on a subblog.

if @DJPaul's combination is also fixing this ticket, i'm fine with it :)

[djpaul]

In 10580:

Core: in BP taxonomy functions, make sure taxonomies are registered.

These functions already use switch_to_blog to ensure we are operating on the root blog context, but as WordPress doesn't update its internal list of registered taxonomies when this happen, core functions that use taxonomy_exists will fail.

By registering our taxonomies if we have to switch blog context, we prevent failures that could otherwise occur in a handful of multisite configurations.

See #6892

Props boonebgorges

[DJPaul]

Upon further consideration, I think we need the extra taxonomy registration check inside a switch_to_blog() in bp_version_updater(), so I've done that in 6892.05.patch which I think is good but I need to test it properly after work before committing.

i.e. like 6892.03.patch but, for the time being, a much lighter change without the proposed shared function.

[djpaul]

In 10582:

Core: be on root blog when installing, and register taxonomies.

Similar to r10580, we needed to make sure the email type taxonomy is
registered when we switch_to_blog to install BuddyPress.

Fixes #6892

Props boonebgorges, DJPaul, imath