Skip to:
Content

Opened 8 years ago

Closed 8 years ago

#3219 closed defect (bug) (fixed)

BP_ENABLE_MULTIBLOG broken

Reported by: wpmuguru Owned by: boonebgorges
Milestone: 1.5 Priority: major
Severity: Version:
Component: Core Keywords: has-patch dev-feedback
Cc:

Description

In trunk, if you have BP_ENABLE_MULTIBLOG defined the only site that works is the main one. The bp-pages site meta row does not (or is unlikely to) match the page ids in the subsites.

Not sure how to best address that.

Attachments (2)

3219.1.patch (12.8 KB) - added by boonebgorges 8 years ago.
3219.2.patch (15.1 KB) - added by boonebgorges 8 years ago.

Download all attachments as: .zip

Change History (22)

#1 @boonebgorges
8 years ago

  • Milestone changed from Awaiting Review to 1.3

Hm. This should have been fixed in r4254, which moved bp-pages into a sitewide option array, keyed by blog_id. When you visit the Dashboard of the non-root blogs, do you see a BuddyPress related admin nag saying that you need to setup the pages?

#2 @boonebgorges
8 years ago

I've just been playing with this and I realize now why it doesn't work. It's because the Network Admin panel is always on blog_id = 1 (as opposed to the old Super Admin system). This is going to throw a major wrench in things. Ugh.

#3 @boonebgorges
8 years ago

  • Owner set to boonebgorges
  • Priority changed from normal to major
  • Status changed from new to assigned

This is also going to be a blocker for the possibility of multi-network-BP. I've got a fix in the works; will post a patch tomorrow.

#4 @boonebgorges
8 years ago

  • Keywords has-patch dev-feedback added

3219.1.patch is my first attempt at a fix. Here's my thought process and strategy.

Typically, admin screens are loaded in Network Admin when is_multisite() is true. This won't work for BP_ENABLE MULTIBLOG (or for multi-network-BP for that matter) because in those cases BP stuff (or at the very least bp-pages) needs to be blog-specific in order to work properly.

3219.1.patch alters this behavior by hooking admin screens/actions to the individual blog screen when BP_ENABLE_MULTIBLOG is activated. Also, since the admin_hook and do_network_admin values are filtered, it gives things like multi-network-BP a chance to change how admin screens are handled.

This patch has not been super-extensively tested or documented. I'm putting it up here so I can get feedback from the other core devs and from wpmuguru on whether this is a reasonable approach to the problem. Thanks!

#5 @boonebgorges
8 years ago

One more thing I meant to mention above: The 3219.1.patch approach is backward-compatible, in the sense that normal admin hooks and form actions will continue to work, except in the case of BP_ENABLE_MULTIBLOG and multi-network-BP. So plugins *should* update, but if they don't, they'll still work in the majority of cases.

@boonebgorges
8 years ago

#6 @DJPaul
8 years ago

I've read the patch, not tested, but the approach is nice

#7 @wpmuguru
8 years ago

For some reason I didn't get any email notifications on this ticket. I'll try to look at your patch over the next few days.

#8 @boonebgorges
8 years ago

Bump. wpmuguru, if you get a chance in the next week or so, please have a look to see if this'll do what you need it to do. If so, I'm going to go ahead and commit it.

#9 @wpmuguru
8 years ago

Sorry Boone, I completely forgot :/ I put it on my list for today.

#10 @wpmuguru
8 years ago

That works okay except something need to be done to switch from sitemeta to the options table. ex.

https://buddypress.trac.wordpress.org/browser/trunk/bp-core/admin/bp-core-admin.php#L57

If you update the pages on a sub site then the main one (and every other site) is thrown off.

Thoughts on a get_bp_option() and update_bp_option() which has the switching logic built in?

#11 @boonebgorges
8 years ago

Yes, the options thing is one that I knew existed but was avoiding. It deserves a different ticket #3261

I'll run another set of tests on this patch when I'm back from vacation, and take it from there. Thanks for having a look, wpmuguru.

#12 @boonebgorges
8 years ago

3219.2.patch modifies the URLs in the admin notices, based on bp_core_do_network_admin().

@boonebgorges
8 years ago

#13 @wpmuguru
8 years ago

I'll try to give it a run later today.

#14 @wpmuguru
8 years ago

That patch worked for me. I did notice that the dashboard menu item was gone though. When I remove the patch the dashboard menu item returns.

#15 @boonebgorges
8 years ago

Can you be more specific? The main thing the patch does is *move* the menu item. If BP_ENABLE_MULTIBLOG is enabled, it moves it to the Site Admin dashboard rather than the Network Admin dashboard.

#17 follow-up: @boonebgorges
8 years ago

Ah, I see. I'll move it to 3. Someone should really write proper menu building logic for WP so that this kind of stuff doesn't happen ;)

#18 in reply to: ↑ 17 @wpmuguru
8 years ago

Replying to boonebgorges:

Ah, I see. I'll move it to 3. Someone should really write proper menu building logic for WP so that this kind of stuff doesn't happen ;)

It's on my list to put in a ticket/patch to WP trac for that. All 3 of the dashboard menu items should occupy the same menu slot (imo anyway).

#20 @boonebgorges
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Thanks, wpmuguru. That'll at least solve the immediate inconsistency. Better menu logic will take a more significant patch!

In r4482 I implement the do_network_admin stuff. That should fix this ticket. See #3261 for the related topic of how settings data is stored when BP_ENABLE_MULTIBLOG is set.

Note: See TracTickets for help on using tickets.