WordPress.org

Make WordPress Core

Opened 10 months ago

Last modified 11 days ago

#39254 accepted defect (bug)

When in Customizer Preview, starter content posts are not displayed in the loop

Reported by: tiagonoronha Owned by: westonruter
Milestone: 4.9.1 Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: needs-unit-tests needs-testing reporter-feedback needs-patch
Focuses: administration, template Cc:

Description

As discussed in Slack, posts in Starter Content aren't appearing in the posts list.

Being able to display the bundled content, specifically posts, is key for an improved user experience when configuring a new theme. Posts give structure to the theme, allowing users to see exactly how the theme will look with content.

This would also allow theme developers to completely match the content bundled with the theme to the content on the theme’s demo site. This helps address the popular complaint amongst WordPress users that a newly installed theme looks nothing like the demo.

Change History (35)

#1 @westonruter
10 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.8
  • Version changed from trunk to 4.7

@tiagonoronha you're absolutely right. The underlying issue is that the starter content posts have the auto-draft status and so they are naturally excluded from being included in WP_Query loops that predominantly query for posts with the publish status. The fix for this has been researched and implemented in the Customize Posts plugin. However, it was not included in the core merge partially because the primary post type in focus for starter content was page and its placement in nav menus, for which the auto-draft status doesn't pose a problem. The solution developed in Customize Posts is also somewhat “out there” although so far it seems to work flawlessly and allows changes to posts to be previewed anywhere they appear.

The solution can be seen in the pull request, and specifically the method \WP_Customize_Posts_Preview::filter_posts_request_to_inject_customized_state() which is added as a filter for posts_request: https://github.com/xwp/wp-customize-posts/blob/ccf7d89/php/class-wp-customize-posts-preview.php#L584-L744

Note that Customize Posts also does filtering for postmeta SQL to ensure that meta queries will also work seamlessly. This wouldn't have to be part of the scope for just adding support for starter content since postmeta is not part of the scope, while previewing changes to posts and postmeta are the scope for the Customize Posts plugin.

When the solution in Customize Posts was being developed, I did do a sanity check with @pento on the approach, and he couldn't see a fundamental problem with it. Two of the primary concerns I have with the approach is when plugins have added custom fields to the wp_posts table, as currently this would cause problems with the UNIONs. Another concern with the approach is plugins which offload queries to an index like Elasticsearch: the plugin is currently attempting to prevent such offloading by supplying 'es' => false, but in reality the query indexing plugins should probably be looking at the customized state and selectively turning off their indexed queries when detecting that there is post/page data in the customized state.

I'm milestoning this for 4.8 because I think that generally themes will be using pages instead of posts among their starter content, and also because the scope of the fix is somewhat large. Perhaps the resolution of this defect would include a merge of the key infrastructure pieces of Customize Posts, including the WP_Customize_Post_Setting and WP_Customize_Postmeta_Setting classes. The UI as found in Customize Posts would naturally need complete UX review and overhaul prior to merge, where the UI could end up in the form of frontend editing.

#2 @westonruter
10 months ago

  • Milestone changed from 4.8 to 4.7.1

Thinking about this a bit more, there may be a solution that could fix the issue specifically for page/post stubs added via starter content. A simpler solution than the one for Customize Posts could be implemented because the customizer as of 4.7 doesn't allow you to preview any changes to the stubbed posts or their postmeta, and so we don't need to worry about injecting the customized state into the queries. In short, what we'd need to do filter posts_where to amend the conditions for the post_status checks to also include:

"OR {$wpdb->posts}.ID IN ( " . join( ',', $wp_customize->get_setting('nav_menus_created_posts')->value() ) . " )"

so that these posts will match queries for publish posts even though they have the auto-draft status. With this done, the posts should then be included in the results. And then the publish status should be set on each of the queried posts via the posts_results filter and also supplied via the get_post_status filter so that these auto-draft posts won't get filtered out of the resulting posts array.

The code required to do this would be small and I think could be in scope for a point release, such as 4.7.1.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


10 months ago

#4 @westonruter
10 months ago

  • Milestone changed from 4.7.1 to 4.7.2

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 months ago

#6 @westonruter
8 months ago

  • Milestone changed from 4.7.3 to 4.7.4

This ticket was mentioned in Slack in #core-customize by dlh. View the logs.


8 months ago

#8 @swissspidy
7 months ago

@westonruter Your outlined approach sounds reasonable. Are you happy to own this ticket to get it into 4.7.4?

#9 @westonruter
7 months ago

  • Keywords needs-unit-tests added

I'm not confident that the approach in comment 2 will fully work as expected. The approach is prone to starter content posts leaking into queries unexpectedly. I think a better approach to explore may be:

  • Modify queries for publish posts to also ask for auto-draft posts. This could be done at the pre_get_posts action; or, if no post_status was specified, make sure that auto-draft post status is marked as protected and private so it will be allowed (this should be handled already via \WP_Customize_Nav_Menus::make_auto_draft_status_previewable()).
  • Add a posts_where clause which does AND ( post_status != 'auto-draft' OR ID IN ( " . join( ',', $wp_customize->get_setting('nav_menus_created_posts')->value() ) . " )).
  • Then, as noted in the comment, add a posts_results filter which loops over all posts and forces the post objects to have: if ( 'auto-draft' === $post->post_status ) { $post->post_status = 'publish'; } and to also do the same for the get_post_status filter.

I'm not sure this ticket is particularly important so I'm not sure I can prioritize for 4.7.4.

#10 @westonruter
7 months ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to westonruter
  • Status changed from new to accepted

On second thought, since this problem is already in my thoughts, I put together a patch in the form of a plugin for testing.

@tiagonoronha would you please give this a try: https://gist.github.com/westonruter/5e8fc5b2a972392d85af4d7befcbb8da

#11 @westonruter
7 months ago

  • Keywords needs-testing added

#12 @tiagonoronha
7 months ago

@westonruter I tested with Twenty Seventeen and the posts I added showed up in the main loop, as expected.

However, in the front page panel for "blog", there were no posts there.

#13 @westonruter
7 months ago

@tiagonoronha you're absolutely right. Not sure how I missed that. I just pushed up another change that both fixes and simplifies the logic for including the starter content posts in queries for posts that have a publish post status. See https://gist.github.com/westonruter/5e8fc5b2a972392d85af4d7befcbb8da

Could you give it another test?

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


7 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


6 months ago

#16 @swissspidy
6 months ago

  • Milestone changed from 4.7.4 to 4.7.5

#17 @FlorianBrinkmann
6 months ago

Would be great if this could be fixed, I am currently working on a theme which can display a grid of posts, and it would be cool to provide an example via starter content for this :)

I installed the plugin from https://gist.github.com/westonruter/5e8fc5b2a972392d85af4d7befcbb8da and tried it. It seems to work now for the front page section of Twenty Seventeen (displays the post which I added via starter content), but the starter content post is displayed multiple times on the blog page.

Last edited 6 months ago by FlorianBrinkmann (previous) (diff)

This ticket was mentioned in Slack in #core by desrosj. View the logs.


6 months ago

#19 @desrosj
6 months ago

  • Milestone changed from 4.7.5 to 4.8

#20 @jbpaul17
6 months ago

  • Keywords reporter-feedback added
  • Milestone changed from 4.8 to 4.7.5

#21 @jbpaul17
6 months ago

  • Milestone changed from 4.7.5 to 4.8

eek, doubled up on edits there in Trac, moving back to 4.8, sorry!

#22 @FlorianBrinkmann
6 months ago

Something I noticed today, maybe it helps: Every full reload (via F5 or reload button, not reload of the preview) of the customizer creates a new instance of the starter content post, which is then displayed as a new post (besides the other instances of this post) in the blog view. But the front page section only shows the post once.

#23 @westonruter
6 months ago

@FlorianBrinkmann please share the starter content array that you're using to test. It may be that you are getting duplicates because of not providing post_name.

#24 @FlorianBrinkmann
5 months ago

@westonruter here is the complete starter content part: https://gist.github.com/FlorianBrinkmann/4356ffbb8badf9604cdfbb88f09ef2df
The entry for the post looks like that (I tried post_name instead post_title but this does not seem to work):

'snowy-landscape' => [
        'post_type'  => 'post',
        'post_title' => 'Snowy Landscape',
],

I think I found the issue. The problem only appears if I use the customizer without a &changeset_uuid= in the URL (for example if I switched the theme via »Design« › »Themes« and not in the customizer). If the changeset exists, I do not get the duplicates.

#25 @westonruter
5 months ago

@FlorianBrinkmann I didn't mean post_name instead of post_title. I meant make sure that there is both a post_title and a post_name for each item. If you supply both, does that fix the problem?

#26 @FlorianBrinkmann
5 months ago

@westonruter ah okay, misunderstood that. I tried it now with the following array, but I get the same result.

'snowy-landscape' => [
        'post_type'  => 'post',
        'post_name'  => 'snowy-landscape',
        'post_title' => 'Snowy Landscape',
],

I looked into the wp-includes/class-wp-customize-manager.php and its import_theme_starter_content() function. For some reason, the $existing_starter_content_posts array is empty at the end of the function. I get the post name »snowy-landscape« from the $post_name = $existing_post->post_name (https://core.trac.wordpress.org/browser/tags/4.7.4/src/wp-includes/class-wp-customize-manager.php#L1016) but the array is not filled with data. So the check for existing auto draft posts seems not to work properly?

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#28 @jbpaul17
5 months ago

  • Milestone changed from 4.8 to 4.8.1

Punting to 4.8.1 per discussion in today's 4.8 rc1 bug scrub in #core.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

#31 @westonruter
3 months ago

  • Milestone changed from 4.8.1 to 4.9

#32 @bduclos
3 weeks ago

Is it still planned for 4.9?

#33 @westonruter
3 weeks ago

  • Keywords needs-patch added; has-patch removed

@bduclos this is marked as a defect so our attention will shift to it once we get Beta1 out the door. Right now focused on new features and enhancements. I still want to get this into 4.9, yes.

This ticket was mentioned in Slack in #core by melchoyce. View the logs.


2 weeks ago

#35 @westonruter
11 days ago

  • Milestone changed from 4.9 to 4.9.1

Punting this to the next minor release. There is already a ton of changes in 4.9 and adding this as well is feeling overwhelming.

Note: See TracTickets for help on using tickets.