#33742 closed defect (bug) (fixed)
Menu Customizer: Adding draft items?
Reported by: | pavelevap | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Menus | Keywords: | has-unit-tests needs-patch |
Focuses: | administration | Cc: |
Description (last modified by )
I can add draft items into menu on classic menu page, but it is not possible with Customizer. I am not sure if it should be removed from admin page (items are not visually marked and mistake possible) or added to Customizer, but it should be consistent.
Originally mentioned in #33665.
Attachments (4)
Change History (27)
#5
@
8 years ago
- Milestone changed from Awaiting Review to 4.7
@welcher thanks for raising this. Interesting. It does seem like a glitch (hack?) that nav menu items for draft posts/pages can be added to a nav menu, and remain in the nav menu even when an unauthenticated user is viewing. I would have thought such nav menu items would have been removed on the frontend in the same way as _is_valid_nav_menu_item
is filtering ones that are _invalid
. I'm not sure if this is a defect or a feature. If a defect, then I think your patch is correct. If a feature, then draft
posts should be discoverable in the customizer as well. If a feature, this is closely related to #34923 since it involves adding nav menu items for auto-draft
posts. I'm adding this ticket to the 4.7 milestone for this reason.
(Something else to note about nav menu items: the wp_update_nav_menu_item()
function actually allows the nav_menu_item
post itself to be made a draft
(if you supply 'menu-item-status' => 'draft'
). See draft-nav-menu-items.png for how the nav menu admin page looks when there is a nav menu item with a draft
status (the blue notice and the word “(Pending)” after the nav menu item label). I've never ever seen this UI before today, although I have seen the reference to it in the code which I had to manually call to get this UI to appear. I don't think there is any UI to create draft nav menu items.)
@celloexpressions What are your thoughts on this? Is it a hidden feature to be able to add non-published posts to a nav menu? Or is it a defect?
#6
@
8 years ago
@westonruter It honestly feels like a bug to me. The query that is pulling the search results doesn't have 'post_status' => 'any'
set. In fact, it had no post_status
parameter set at all - which (unless I'm missing something ) should mean that only published posts are being queried.
I haven't investigated but could there be a pre_get_posts
happening that would change this or does having the s
param disregard the post_status
?
If we determine that this is a bug, I'm happy to remove my patch and mark it as a Good First Bug - feeling a little guilty about stealing a one-liner :)
#7
@
8 years ago
@welcher would you be willing to investigate and look into what is going on here? This definitely feels like a bug to me at first glance.
#8
@
8 years ago
- Keywords dev-feedback removed
I'm inclined to agree with @welcher that it is a bug to be able to add non-publish posts to a nav menu, and so 33742.diff is the way to go. But actually, the underlying issue here is that the following logic isn't getting applied to searches like it is for when wp_nav_menu_item_post_type_meta_box
runs:
<?php if ( isset( $box['args']->_default_query ) ) $args = array_merge($args, (array) $box['args']->_default_query );
So the fix I think is to make sure the same _default_query
is merged with the query vars when a search as done, just as when the list generated. See 33742.2.diff which does this.
#9
@
8 years ago
Interesting, but I do not think that it is a bug, but expected feature? See for example string pendingTitleTpl
with comment /* translators: %s: title of menu item in draft status */
This string will be no longer needed in this case? And what about users using this feature? It can be used for scheduled posts added to menu before publication or it does not work this way?
#10
@
8 years ago
@pavelevap that is speaking of the nav_menu_item
post itself, not the status of the underlying object
that is being referenced. See my comment above about “draft nav menu items”. There is no UI to create such draft nav menu items, but there is a UI to show them if they are in a draft (pending) status.
#11
@
8 years ago
Thank you, I did not realize that! So the only way to add draft item is publish some post, add it to menu and then unpublish it? It is only callback for unpublished items which were part of menu?
#12
@
8 years ago
@pavelevap no, if you add a nav menu which points to a published post, and then you unpublish that post, the nav menu item status does not change. The nav menu will continue to include that item, even though it points to a draft
post and unauthorized users would not be able to see it. That's probably a separate (yet related) issue to what we're discussing here: limiting non-published posts from being available to be added to a nav menu vs filtering out nav menu items from a nav menu when they point to a non-published post.
#13
@
8 years ago
I did some digging on this and it seems that when WP_Query
is run without a post_status
parameter in the admin, all post types that are available to the admin are queried:
class-wp-query
~ line 2345
if ( $this->is_admin ) { // Add protected states that should show in the admin all list. $admin_all_states = get_post_stati( array('protected' => true, 'show_in_admin_all_list' => true) ); foreach ( (array) $admin_all_states as $state ) { $where .= " OR {$this->db->posts}.post_status = '$state'"; } }
The above generates SQL like the following:
AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'future' OR wp_posts.post_status = 'draft' OR wp_posts.post_status = 'pending' OR wp_posts.post_status = 'private')
As such , the query is exhibiting expected behaviour - just the wrong behaviour for this use case.
At the end of the day, both patches are adding 'post_status' => 'publish'
to the query but @westonruter 's is a more standardized approach. I'd go with it.
I do wonder now however if this needs a filter to allow this behaviour? I can imagine someone out in the wild is using this bug as a feature.
At any rate we should get some unit tests on this too.
#16
@
8 years ago
- Keywords has-unit-tests added; needs-unit-tests dev-feedback removed
I've added a unit test to demonstrate that the current functionality in _wp_ajax_menu_quick_search
is indeed a bug. In 33742-tests.diff ] there are 4 posts created, 3 of which have post_statuses
that should keep them out of standard queries. In theory, only the publish
post status should be returned however all of them are.
@westonruter 's 33742.2.diff fixes the issue and the test passes. You can test by first applying 33742-tests.diff and then running the Core tests and then applying 33742.2.diff and re-running.
This ticket was mentioned in Slack in #core by welcher. View the logs.
8 years ago
#18
@
8 years ago
- Keywords 2nd-opinion removed
Sorry, just saw this. I think the ability to add drafts to menus is a bug; it probably initially made sense but can cause major UX issues, especially when getting to the front end site. Updating the admin to match the customizer result seems like the best action here.
#19
@
8 years ago
- Owner set to westonruter
- Resolution set to fixed
- Status changed from new to closed
In 38584:
#21
@
8 years ago
- Keywords needs-patch added; has-patch removed
- Resolution fixed deleted
- Status changed from closed to reopened
I'm afraid [38584] broke the quick search in the Menus screen (nav-menus.php
), where the search result should return a list with checkboxes and now returns instead a list of links:
trunk:
4.6
You can't add draft items to menus via the customizer. Attached is a patch to restrict the quick search to
'post_status' => true
in the classic menu page.