Page mappings can create duplicate pages on reinstall

[johnjamesjacoby]

If page mappings are ever deleted and settings are saved, bp_core_add_page_mappings() will create duplicate pages. I believe our original intent with these mappings was to yield to existing pages if they exist.

[johnjamesjacoby]

Patched against bp-core-functions.php directly

[johnjamesjacoby]

In 8570:

When creating directory page mappings, check for and use pages that already exist with a matching slug. Fixes #5747.

[johnjamesjacoby]

In 8571:

Code formatting improvements to bp_core_add_page_mappings(). See #5747.

[boonebgorges]

The changes in r8570 will have the effect of hijacking any page with the slug 'groups'. IMO this edge case is a larger problem than the one we're trying to solve here (that duplicate pages can be created in the case of reinstallation). How about instead of get_page_by_path(), we check to see if the component has a page in bp_core_get_directory_page_ids(); if it does, and the page exists, use it; otherwise create a new one.

[DJPaul]

get_page_by_path() is also an uncached function, and while it might not make a huge difference here, I wanted to point it out.

[johnjamesjacoby]

Replying to boonebgorges:

The changes in r8570 will have the effect of hijacking any page with the slug 'groups'.

This was intentional.

IMO this edge case is a larger problem than the one we're trying to solve here (that duplicate pages can be created in the case of reinstallation).

How so?

When duplicate pages are created, they are also assigned as the directory page (likely with a -2 at the end.) Fixing this error in it's previous incarnation is a tedious and backwards process:

• Notice you have 3 duplicate Pages with -2 slugs
• Navigate to wp-admin
• Navigate to Pages
• Hover over each "View" link to figure out which ones are the duplicates
• Trash those Page(s)
• Navigate to Deleted Pages
• Delete those Page(s) completely (maybe without deleting other previously trashed pages you aren't ready to remove)
• Visit BuddyPress Settings
• Reassign the Page(s) to the originals
• Double check that each URL is correct

What happens after my changes, is BuddyPress (safely IMO) assumes these pages exist with the intent of being assigned. If that assumption is incorrect, the fix is much easier and (again, IMO) comes with some additional context and learning:

• Notice that your existing pages have been wrongly overridden (note that now you've learned what BuddyPress directories are and do.)
• Navigate to wp-admin
• Create New Page(s)
• Give them appropriate titles and Publish them
• Visit BuddyPress Settings
• Reassign the Page(s) to your new ones
• Double check (with more confidence than above because you just created those pages) that each URL is correct

What is currently in trunk:

• is up to 3 less steps (and possibly more per click, per page)
• Does not recreate duplicate site content unbeknownst to the user
• Reduces friction in learning how BuddyPress directories work when conflicts occur

How about instead of get_page_by_path(), we check to see if the component has a page in bp_core_get_directory_page_ids(); if it does, and the page exists, use it; otherwise create a new one.

When the pages exist, but the bp-pages option is deleted, duplicate pages will still be created.

Replying to DJPaul:

get_page_by_path() is also an uncached function, and while it might not make a huge difference here, I wanted to point it out.

Worth acknowledging, but it's the only way to query these pages by their respective slugs, and we'll inherit any improvements that may go into it later.

Ultimately, I'm willing to err on the side of a less confused user that already created pages than a more confused user that may need to learn how to edit/trash/delete pages they didn't already create.

[johnjamesjacoby]

If we don't agree with the approach that's currently in core, let's reconsider whether silently and automatically creating pages on activation is a good experience in the first place and/or improvements that can be made to whatever experience we settle on.

[boonebgorges]

I agree that it's annoying to fix the '-2' problem. But this situation should almost never arise for normal users.

The situtation I'm looking to avoid is one where an existing WP website has pages with page slugs 'groups', 'members', etc. These pages would have regular page content, indexed by Google, etc. They decide to install BuddyPress, and all of a sudden their content at 'groups' etc disappears from the front end of the site. Of course, we haven't deleted anything, but it still results in a confusing experience, and a poor first impression of BP.

We are dealing with the choice between two fairly unpleasant experiences, though each is definitely an edge case. My feeling is that someone who is deleting the 'bp-pages' key from the database in order to "reinstall" BuddyPress is going to be better prepared for a jarring experience than the new user who has his content hijacked.

[johnjamesjacoby]

Replying to boonebgorges:

The situtation I'm looking to avoid is one where an existing WP website has pages with page slugs 'groups', 'members', etc. These pages would have regular page content, indexed by Google, etc. They decide to install BuddyPress, and all of a sudden their content at 'groups' etc disappears from the front end of the site. Of course, we haven't deleted anything, but it still results in a confusing experience, and a poor first impression of BP.

I agree that claiming these existing pages isn't great, but I think it's better than creating domain.com/activity-2.

I agree that it's annoying to fix the '-2' problem. But this situation should almost never arise for normal users. We are dealing with the choice between two fairly unpleasant experiences, though each is definitely an edge case. My feeling is that someone who is deleting the 'bp-pages' key from the database in order to "reinstall" BuddyPress is going to be better prepared for a jarring experience than the new user who has his content hijacked.

I'm thinking less about the educated-enough admin poking around and deleting options than I am about the innocent user that is the victim of an incomplete data migration/restoration. Edge-case as it may be, we should aim for minimizing the damage we leave behind in our activation wake.

Tangentially, if/when we get rewrite rules working, we'll likely end up being behind the pages rewrite rules and losing in the event of a conflict, or at least wanting to yield to existing pages in a similar fashion to what you're describing now.

[boonebgorges]

I agree that claiming these existing pages isn't great, but I think it's better than creating domain.com/activity-2.

Really? You think it's better to hijack an existing page than to have an ugly 'activity-2' slug? Or do you mean it's better to to hijack an existing page than to have the reinstallation issue?

I'm still pretty unconvinced that anyone other than a developer messing around with the database is ever going to run into the 'activity-2' issue - I've never seen it in the wild myself - but the point is not important enough to argue anymore. I only reopened the ticket to make it clear that there may be consequences to the solution you chose. If you feel strongly about it, let's just make a note (maybe in the codex) that will advise folks on what to do if they do happen to find their content hijacked.