Opened 8 years ago
Last modified 5 years 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: | Future Release | 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 (38)
#1
@
8 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 4.8
- Version changed from trunk to 4.7
#2
@
8 years 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.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-customize by dlh. View the logs.
8 years ago
#8
@
8 years ago
@westonruter Your outlined approach sounds reasonable. Are you happy to own this ticket to get it into 4.7.4?
#9
@
8 years 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 forauto-draft
posts. This could be done at thepre_get_posts
action; or, if nopost_status
was specified, make sure thatauto-draft
post status is marked asprotected
andprivate
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 doesAND ( 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 theget_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
@
8 years 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
#12
@
8 years 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
@
8 years 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.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#17
@
8 years 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.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
#21
@
8 years 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
@
8 years 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
@
8 years 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
@
8 years 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
@
8 years 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
@
8 years 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.
8 years ago
#28
@
8 years 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.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#33
@
7 years 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.
7 years ago
#35
@
7 years 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.
@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 inWP_Query
loops that predominantly query for posts with thepublish
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 waspage
and its placement in nav menus, for which theauto-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 forposts_request
: https://github.com/xwp/wp-customize-posts/blob/ccf7d89/php/class-wp-customize-posts-preview.php#L584-L744Note 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 theUNION
s. 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
andWP_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.