Default theme content - problem with slugs

[pavelevap]

I installed latest RC1-39355, activated Twenty Seventeen and went to Customizer.

There were 5 pages created, but with number in slug, for example contact-2, blog-2, etc. It is a fresh install and I did not have any pages created before. For every created page I have 3 records in database:

- page: auto-draft > a-homepage-section
- page: publish > a-homepage-section-2
- revision: inherit > 20-revision-v1

I do not think it is necessary? When default page during WP installation is created, there is only one publish record in database.

[lukecavanagh]

@pavelevap

Created the starter content just fine, from the customizer on local dev site which was a fresh install. No duplicate slug issue found.

[westonruter]

I can replicate this issue. It happens if you open the customizer, close it, and then open it again so that the starter content gets imported a second time. When you do that, you currently get a posts table that looks like:

+----+--------------------+--------------------+---------------------+-------------+
| ID | post_title         | post_name          | post_date           | post_status |
+----+--------------------+--------------------+---------------------+-------------+
| 19 | A homepage section | a-homepage-section | 2016-11-28 23:08:28 | auto-draft  |
| 18 | Blog               | blog               | 2016-11-28 23:08:28 | auto-draft  |
| 17 | Contact            | contact            | 2016-11-28 23:08:28 | auto-draft  |
| 16 | About              | about              | 2016-11-28 23:08:28 | auto-draft  |
| 15 | Home               | home               | 2016-11-28 23:08:28 | auto-draft  |
| 10 | A homepage section | a-homepage-section | 2016-11-28 23:08:16 | auto-draft  |
| 9  | Blog               | blog               | 2016-11-28 23:08:16 | auto-draft  |
| 8  | Contact            | contact            | 2016-11-28 23:08:16 | auto-draft  |
| 7  | About              | about              | 2016-11-28 23:08:16 | auto-draft  |
| 6  | Home               | home               | 2016-11-28 23:08:16 | auto-draft  |
+----+--------------------+--------------------+---------------------+-------------+

There are two fixes we can consider here, which aren't mutually exclusive:

For one, the logic for wp_unique_post_slug() can be changed to explicitly ignore auto-draft posts when it is doing the lookup to see if a slug is already being used. This seems like a no-brainer: 38928.2.diff​

The second fix would be specifically to account for when we allow starter content to be applied on existing sites. Namely, when we do our lookup to see if there are existing auto-draft posts in the current customized state that have the given slugs… we could expand this to see if there are any non-auto-draft posts _outside_ the customized state which already have those slugs, and then re-use them instead of inserting new auto-drafts.

[westonruter]

And actually I think wp_unique_post_slug() should include the post_type in the query since it is fine for posts of different types to have the same slug, right?

[westonruter]

Nevermind, I see for non-attachment posts, it is already explicitly checking the post_type as well. See 38928.2.diff​ for update.

[westonruter]

@helen 38928.3.diff​ includes the logic for re-using non-auto-draft posts instead of adding new auto-drafts for the starter content.

[pavelevap]

Great, I do not remember that I returned back to Customizer. I installed RC version without Twenty Seventeen (so Twenty Sixteen was activated by default), then added Twenty Seventeen and activated it manually. Then looked at the site and then went to Customizer to set up homepage. But I do not think that I returned to Customizer (I will test it once again).

And also attachments were duplicated and pictures imported twice (with all related sizes).

[westonruter]

@pavelevap Can you confirm the patch here improves the expected experience?

[westonruter]

Adding unit tests for wp_unique_post_slug() changes: ​https://github.com/xwp/wordpress-develop/pull/210

[westonruter]

Δ ​https://github.com/xwp/wordpress-develop/pull/210/commits/eaa15694325e82a2af00d40717a49f6fff2ce6d6

[westonruter]

Δ ​https://github.com/xwp/wordpress-develop/pull/210/commits/0372007634753eaf12710f3c4e3dcbecfd5ab428

[westonruter]

In writing more unit tests, as always happens, I found some gaps in starter content. Namely, it was not possible to set starter content for custom logo, background, or header. The latest patch ensures these are supported via unit tests and also it adds tests for this ticket's original issue, which was re-using non-auto-draft posts and attachments that have matching slugs.

[helen]

Patch looks good to me, also fixes a pretty odd bug I happened to come across just before I tested this patch, where already-published pages were getting their slugs appended when I went to edit them.

The one concern I have (that doesn't block commit, but needs discussion) is what then is the difference between default header and background images vs. ones provided in starter content. There's the different in sideloading, but is there one that should be preferred / recommended over the other?

[westonruter]

In 39411:

Customize: Reuse existing non-auto-draft posts and existing auto-draft posts in the customized state with matching slugs when applying starter content.

• Updates wp_unique_post_slug() to ignore auto-draft posts. Prevents publishing multiple posts that have the same slugs from starter content.
• Fixes fatal error when attempting to save an header_image setting from a non-admin context.
• Fixes substituting attachment symbols in options and theme mods.
• Fixes applying starter content for header images and background images.

See #38114.
Fixes #38928.

[westonruter]

In 39412:

Customize: Reuse existing non-auto-draft posts and existing auto-draft posts in the customized state with matching slugs when applying starter content.

• Updates wp_unique_post_slug() to ignore auto-draft posts. Prevents publishing multiple posts that have the same slugs from starter content.
• Fixes fatal error when attempting to save an header_image setting from a non-admin context.
• Fixes substituting attachment symbols in options and theme mods.
• Fixes applying starter content for header images and background images.

Merges [39411] to 4.7 branch.
See #38114.
Fixes #38928 for 4.7.

[westonruter]

In 39506:

Customize: Defer populating post_name for auto-draft posts in customized state until posts are published.

The ultimate post_name is stored in postmeta until the post is published. The get_page_by_path() function does not exclude auto-draft posts. Revert changes to wp_unique_post_slug() from [39411] which excluded auto-draft posts.

Props westonruter, dlh for testing, helen for testing.
See #38114, #38928.
Fixes #39078.