Skip to content

Menus: Fix wp_get_nav_menu_items() dropping items when the term count is stale or the query is not publish-only. #13513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MarcinDudekDev wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Menus: Fix wp_get_nav_menu_items() dropping items when the term count is stale or the query is not publish-only. #13513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/wp-includes/nav-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -738,11 +738,14 @@ function wp_get_nav_menu_items( $menu, $args = array() ) {
),
);
$args = wp_parse_args( $args, $defaults );
if ( $menu->count > 0 ) {
$items = get_posts( $args );
} else {
$items = array();
}

/*
* The term's count only reflects published menu items and can be stale,
* so it is not a safe proxy for whether any items match these args.
* get_posts() already returns an empty array for a genuinely empty menu,
* and the query is cached in the 'post-queries' group.
*/
$items = get_posts( $args );

$items = array_map( 'wp_setup_nav_menu_item', $items );

Expand Down
231 changes: 231 additions & 0 deletions tests/phpunit/tests/post/nav-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1392,4 +1392,235 @@ public function test_wp_update_nav_menu_item_with_post_date() {
$post = get_post( $menu_item_id );
$this->assertEqualsWithDelta( strtotime( gmdate( 'Y-m-d H:i:s' ) ), strtotime( $post->post_date ), 2, 'The dates should be equal' );
}

/**
* Adds a nav menu item via the public API.
*
* @param int $menu_id Menu term ID.
* @param array $args Overrides for wp_update_nav_menu_item() data.
* @return int Menu item post ID.
*/
protected function add_nav_menu_item( $menu_id, $args = array() ) {
$id = wp_update_nav_menu_item(
$menu_id,
0,
array_merge(
array(
'menu-item-type' => 'custom',
'menu-item-title' => 'Item',
'menu-item-url' => 'https://example.org/',
'menu-item-status' => 'publish',
),
$args
)
);
$this->assertNotWPError( $id );
return (int) $id;
}

/**
* Plucks db_id values from a wp_get_nav_menu_items() result.
*
* @param int|string|WP_Term $menu Menu.
* @param array $args Args.
* @return int[]
*/
protected function get_nav_menu_item_db_ids( $menu, $args = array() ) {
$items = wp_get_nav_menu_items( $menu, $args );
$this->assertIsArray( $items, 'wp_get_nav_menu_items() did not return an array.' );
return array_map( 'intval', wp_list_pluck( $items, 'db_id' ) );
}

/**
* Runs the same query wp_get_nav_menu_items() would run, directly through
* get_posts(), bypassing the menu term count.
*
* @param WP_Term $menu Menu term object.
* @param array $extra_args Args merged over the function's own defaults.
* @return int[] Post IDs the direct query returns.
*/
protected function direct_nav_menu_item_query_ids( $menu, $extra_args = array() ) {
$args = wp_parse_args(
$extra_args,
array(
'order' => 'ASC',
'orderby' => 'menu_order',
'post_type' => 'nav_menu_item',
'post_status' => 'publish',
'nopaging' => true,
'tax_query' => array(
array(
'taxonomy' => 'nav_menu',
'field' => 'term_taxonomy_id',
'terms' => $menu->term_taxonomy_id,
),
),
)
);
return array_map( 'intval', wp_list_pluck( get_posts( $args ), 'ID' ) );
}

/**
* Draft-only menu: the term count is 0 because only published items are
* counted, but a caller asking for 'draft' items must still get them.
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_draft_only_menu_draft_query() {
$d = $this->add_nav_menu_item( $this->menu_id, array( 'menu-item-status' => 'draft' ) );

$menu = wp_get_nav_menu_object( $this->menu_id );
$this->assertSame( 0, (int) $menu->count, 'Premise failed: draft item was counted.' );

// Premise: a direct identical query does return the item.
$this->assertSame( array( $d ), $this->direct_nav_menu_item_query_ids( $menu, array( 'post_status' => 'draft' ) ) );

$this->assertSame(
array( $d ),
$this->get_nav_menu_item_db_ids( $this->menu_id, array( 'post_status' => 'draft' ) ),
'Draft item in a draft-only menu was hidden by the $menu->count > 0 gate.'
);
}

/**
* Draft-only menu queried with post_status 'any'.
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_draft_only_menu_any_query() {
$d = $this->add_nav_menu_item( $this->menu_id, array( 'menu-item-status' => 'draft' ) );

$menu = wp_get_nav_menu_object( $this->menu_id );
$this->assertSame( 0, (int) $menu->count, 'Premise failed: draft item was counted.' );
$this->assertSame( array( $d ), $this->direct_nav_menu_item_query_ids( $menu, array( 'post_status' => 'any' ) ) );

$this->assertSame(
array( $d ),
$this->get_nav_menu_item_db_ids( $this->menu_id, array( 'post_status' => 'any' ) ),
'Draft item hidden from post_status=any by the $menu->count > 0 gate.'
);
}

/**
* Draft-only menu queried with 'publish,draft' — the same post_status core
* itself uses inside wp_update_nav_menu_item().
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_draft_only_menu_publish_draft_query() {
$d = $this->add_nav_menu_item( $this->menu_id, array( 'menu-item-status' => 'draft' ) );

$menu = wp_get_nav_menu_object( $this->menu_id );
$this->assertSame( 0, (int) $menu->count, 'Premise failed: draft item was counted.' );
$this->assertSame( array( $d ), $this->direct_nav_menu_item_query_ids( $menu, array( 'post_status' => 'publish,draft' ) ) );

$this->assertSame(
array( $d ),
$this->get_nav_menu_item_db_ids( $this->menu_id, array( 'post_status' => 'publish,draft' ) ),
'Draft item hidden from post_status=publish,draft by the $menu->count > 0 gate.'
);
}

/**
* Adding a second draft item to a draft-only menu must position it after
* the first. wp_update_nav_menu_item() computes the position from the
* publish,draft item list, which the count gate must not empty.
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_second_draft_item_position() {
$d1 = $this->add_nav_menu_item( $this->menu_id, array( 'menu-item-status' => 'draft' ) );
$d2 = $this->add_nav_menu_item( $this->menu_id, array( 'menu-item-status' => 'draft' ) );

$pos1 = (int) get_post( $d1 )->menu_order;
$pos2 = (int) get_post( $d2 )->menu_order;

$this->assertGreaterThan(
$pos1,
$pos2,
"Second draft item got menu_order {$pos2} <= first item's {$pos1} — the count gate hid the first item from wp_update_nav_menu_item()."
);
}

/**
* A published item attached while wp_defer_term_counting() is enabled
* leaves the term count stale at 0; the default publish query must still
* return the item.
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_deferred_term_counting() {
wp_defer_term_counting( true );
$item = $this->add_nav_menu_item( $this->menu_id );

$menu = wp_get_nav_menu_object( $this->menu_id );
$this->assertSame( 0, (int) $menu->count, 'Premise failed: count was not deferred.' );
$this->assertSame( array( $item ), $this->direct_nav_menu_item_query_ids( $menu ), 'Premise failed: direct query does not see the item.' );

$items = $this->get_nav_menu_item_db_ids( $this->menu_id );
wp_defer_term_counting( false );

$this->assertSame(
array( $item ),
$items,
'Published item attached during deferred term counting was hidden by the stale $menu->count gate.'
);
}

/**
* A caller holding a stale WP_Term snapshot (count fetched before the item
* was added) must still get the item: wp_get_nav_menu_object() returns a
* passed object verbatim, so a stale count must not short-circuit the
* query.
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_stale_term_object() {
$stale_obj = wp_get_nav_menu_object( $this->menu_id ); // count = 0 snapshot.
$this->assertSame( 0, (int) $stale_obj->count, 'Premise failed.' );

$a = $this->add_nav_menu_item( $this->menu_id ); // count is now 1 in DB.

// Sanity: a fresh fetch sees count 1 and returns the item.
$this->assertSame( array( $a ), $this->get_nav_menu_item_db_ids( $this->menu_id ) );

$items = wp_get_nav_menu_items( $stale_obj );
$this->assertIsArray( $items );
$this->assertSame(
array( $a ),
array_map( 'intval', wp_list_pluck( $items, 'db_id' ) ),
'A stale WP_Term snapshot hid a real published item.'
);
}

/**
* Trash-only menu: once the last published item is trashed the term count
* returns to 0, but a post_status 'trash' query must still see the
* trashed item.
*
* @ticket 66102
* @covers ::wp_get_nav_menu_items
*/
public function test_wp_get_nav_menu_items_trash_only_menu_trash_query() {
$a = $this->add_nav_menu_item( $this->menu_id );
$this->assertSame( array( $a ), $this->get_nav_menu_item_db_ids( $this->menu_id ) );

wp_trash_post( $a );

$menu = wp_get_nav_menu_object( $this->menu_id );
$this->assertSame( 0, (int) $menu->count, 'Premise failed: trashed item still counted.' );
$this->assertSame( array( $a ), $this->direct_nav_menu_item_query_ids( $menu, array( 'post_status' => 'trash' ) ) );

$this->assertSame(
array( $a ),
$this->get_nav_menu_item_db_ids( $this->menu_id, array( 'post_status' => 'trash' ) ),
'Trashed item hidden by the $menu->count > 0 gate.'
);
}
}
Loading