Skip to:
Content

Opened 3 years ago

Closed 3 years ago

#3422 closed defect (bug) (fixed)

Components registered via bp_core_add_root_component() produce zillions of pages when original page is deleted

Reported by: boonebgorges Owned by:
Milestone: 1.5 Priority: blocker
Severity: blocker Version: 1.5
Component: Core Keywords: 2nd-opinion
Cc:

Description

Steps to reproduce:

  • In a plugin, register a new component in the following way:
function bbgtest_setup_root_component() {
	bp_core_add_root_component( 'boone_is_cool' );
}
add_action( 'bp_setup_root_components', 'bbgtest_setup_root_component' );
  • Load the front end of BP. A page 'Boone_is_cool' will be auto-created.
  • At Dashboard > Pages, send the page Boone_is_cool to the Trash
  • Load the front end of BP. A new page will be created
  • Load the front end of BP again. Two more pages will be created (ad infinitum, each time you reload)

The problem is this. When a component is added with bp_core_add_root_component(), BP checks to see whether a corresponding item can be found in $bp->pages. If no page is found, it is created using the post_name of the $slug passed to bp_core_add_root_component() ('boone_is_cool', in our example). When you Trash the page, the post_status is set to 'trash', which means that it's no longer picked up by the query in bp_core_get_page_names(). As a result, it's not loaded into $bp->pages (so far, this seems logical enough). However, when you reload BP, it tries to create a new page with the slug 'boone_is_cool', but WP prevents this with wp_unique_post_slug(). As a result, pages with names like 'boone_is_cool-2' etc are created and added to $bp->pages.

One quick-and-dirty solution is to modify the query in bp_core_get_page_names() so that it gets all posts, not just those with post_status = trash. The downside of this approach is that you wouldn't have any obvious way of knowing, by looking at $bp->pages, when a page had been deleted (and, by extension, the component had been turned off).

Another option, which I think is better, is to hook into the WP page delete action, and remove the page from the bp-pages option. That should have the effect of having the newly-generated page be properly added to the bp-pages array on the next page load.

The situation is quite confusing indeed, and needs some more thought. Hopefully someone else can pitch in with some thoughts.

Attachments (1)

3422.01.patch (1.3 KB) - added by boonebgorges 3 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 DJPaul3 years ago

I too like the idea of your second approach. Also need to check if renaming the pages causes this issue, too.

comment:2 modemlooper3 years ago

when you deactivate a plugin the page is still there and it has an error:

Some of your WordPress pages are linked to BuddyPress Components that are disabled: Achievements. [Repair]

When you click repair it does nothing. Bad UX if it does nothing. This happened with Achievements and a few of my other plugins.

should the page be trashed on plugin deactivation?

When you trash a page that is attached to a component slug creation it creates the loop hell. Just tested on 4 different plugins and it's the same thing.

steps to recreate bug:

add a plugin and activate, it creates the correct page and all is fine.
trash page for the plugin and it will then go into page creation loop hell.

Last edited 3 years ago by modemlooper (previous) (diff)

comment:3 modemlooper3 years ago

Also, when you trash a page it really needs to delete it forever because when you then need to add another page, lets say you trash members by accident and need to recreate it you are then not getting /members/ you are getting /members-20/

This is going to confuse non technical people and it will break all links.

Example: http://demo.buddiphone.com/members-2/admin/

now click the admin username on an activity stream item.

comment:4 boonebgorges3 years ago

Behavior of trashed WP pages is beyond the scope of what we can fix in BuddyPress.

If we implement a fix that wipes the page from bp-pages when trashing a WP page, it should fix part of the issue. But because things like activity links are stored in the DB as hardcoded links, there is really nothing we can do about it, aside from looping through the entire database to correct every hardcoded link. I'm not opposed to that, but it's an additional enhancement.

comment:5 boonebgorges3 years ago

  • Priority changed from normal to blocker
  • Severity changed from normal to blocker

I've been doing some more investigation, and it looks like the problem will likely be more far-ranging than on installations where pages have been trashed. On a clean installation of BP 1.2.9, I installed Achievements; I then upgraded to 1.5 trunk, and experienced the problem.

Changing status to blocker.

comment:6 boonebgorges3 years ago

I've tracked down at least part of the problem, at least with upgrades. Here's what happens:

  • When BP is first loaded after the upgrade (but before the wizard has been completed), the option 'bp-pages' is empty.
  • During that first page load, bp_core_add_root_component() is called by the plugin, which in turn attempts to call up the list of bp-pages, via bp_core_get_page_names() and bp_core_get_page_meta(). In these cases, it will end up being empty.
  • When bp_core_get_page_meta() then attempts to build a stdClass with the page data, it ends up creating an array with a null value and key. bp_core_add_root_component() then adds an item to this array, which creates invalid sql in bp_core_get_page_names(), and generally screws everything up.

Checking to make sure that there are no invalid keys in the $page_ids array in bp_core_get_page_meta() seems to fix at least this part of the problem. See 3422.01.patch.

boonebgorges3 years ago

comment:7 enderandrew3 years ago

I can confirm this behavior. I tried installing the Buddypress Privacy plugin, which calls bp_core_add_root_component() and in turn, created pages for Welcome, Maintenance, Privacy and Privacy-Profile. I didn't want those pages at the top of my nav menu, and I already have comparable pages for those functions already. I deleted those 4 pages, and then it kept creating tons of those pages with each new page load. I also saw the message:

Some of your WordPress pages are linked to BuddyPress Components that are disabled: Welcome, Maintenance, Privacy, Privacy-Profile. [Repair]

But Repair didn't seem to do anything.

comment:8 follow-up: boonebgorges3 years ago

enderandrew's comments underscore the need for better messaging and documentation. Because of bp-default's integration with WP's native menu system, the proper way to remove an item from the top-level nav will be to create a custom nav menu, NOT to delete it. We might want to (re)consider a meta box on the WP page edit screen that warns users not to delete, or something like that.

comment:9 in reply to: ↑ 8 enderandrew3 years ago

Replying to boonebgorges:

enderandrew's comments underscore the need for better messaging and documentation. Because of bp-default's integration with WP's native menu system, the proper way to remove an item from the top-level nav will be to create a custom nav menu, NOT to delete it. We might want to (re)consider a meta box on the WP page edit screen that warns users not to delete, or something like that.

Would it be possible when BP creates the pages, that they create them as hidden pages that don't show up in the nav menu?

To be honest, I'm not aware of any means of keeping a page from showing up in the nav menu unless you replace it with a custom menu.

comment:10 boonebgorges3 years ago

Would it be possible when BP creates the pages, that they create them as hidden pages that don't show up in the nav menu?

I don't think we want that. Having the pages show up in the main nav is a feature, not a bug. It will create some annoyance for people upgrading from BP 1.2.9 (where they may have hardcoded their menus), but it's much better in the long run for people to be able to use WP's custom menus. And for most people, pages-as-menus will continue to be a good option.

comment:11 boonebgorges3 years ago

I've opened a separate ticket, with a patch, for the larger issue of integrating bp-pages and third-party components: #3428. Please please give feedback!

comment:12 boonebgorges3 years ago

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

Changeset r4920 disabled the automatic page creation that was causing this bug. Closing as fixed.

Note: See TracTickets for help on using tickets.