Skip to content
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.

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

Fix the parent menu item field in REST API responses #34835

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Sep 15, 2021

Description

It looks like there was a small regression in #34673 which caused menu items to return a parent of 0. The parent field was switched to using the post_parent instead of the menu_item_parent field. Switching it back seems to fix things.

I'm not sure exactly what the difference between the two is. It looks like there's a little difference in how wp_setup_nav_menu_item handles this value - https://github.com/WordPress/wordpress-develop/blob/c3ef52ded7bb480899adfd2f69d9e3fddd210615/src/wp-includes/nav-menu.php#L819

I'll add some tests for this once #34765 has been tackled.

How has this been tested?

  1. Create a nested menu
  2. Save it
  3. Reload

Expected: the menu should have the same structure as when it was saved.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@talldan talldan added REST API Interaction Related to REST API [Type] Regression Related to a regression in the latest release [Feature] Navigation Screen labels Sep 15, 2021
@talldan talldan self-assigned this Sep 15, 2021
@@ -671,7 +671,7 @@ public function prepare_item_for_response( $post, $request ) {

if ( rest_is_field_included( 'parent', $fields ) ) {
// Same as post_parent, expose as integer.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update this inline comment as well? To avoid the same mistake in the future.

Based on this property definition, menu_item_parent is what we need in this case.

https://github.com/WordPress/wordpress-develop/blob/f44a99529758e9a5ada443347c5febb5b8a67b6f/src/wp-includes/nav-menu.php#L797-L800

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's saying to treat the type of menu_item_parent as an int, the same as post_parent, so seems correct, but maybe a bit ambiguous.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works as expected.

@talldan talldan merged commit 79be738 into trunk Sep 15, 2021
@talldan talldan deleted the fix/menu-item-parent-rest-api branch September 15, 2021 08:45
@github-actions github-actions bot added this to the Gutenberg 11.6 milestone Sep 15, 2021
fullofcaffeine added a commit that referenced this pull request Sep 16, 2021
* trunk: (74 commits)
  Update docs for ClipboardButton component (#34711)
  Post Title Block: add typography formatting options (#31623)
  Bump plugin version to 11.5.0
  Navigation Screen: Adjust header toolbar icon styles (#34833)
  Fix the parent menu item field in REST API responses (#34835)
  Rewrite FocusableIframe as hook API (#26753)
  Create Block: Remove wp-cli callout since not recommended and outdated (#34821)
  Global Styles: Fix dimensions panel default controls display (#34828)
  [RNMobile][Embed block] Enable embed preview for Instagram and Vimeo providers (#34563)
  Increase Link UI search results to 10 on Nav Editor screen (#34808)
  Prevent welcome guide overflow x scroll (#34713)
  Enable open on click for Page List inside Navigation. (#34675)
  [RNMobile] [Embed block] -  Unavailable preview fallback bottom sheet title update  (#34674)
  Add missing field _invalid in menu item REST API (#34670)
  Fix Dropdown/DropdownMenu toggle closing in all UAs (#31170)
  Navigation submenu block: replace global shortcut event handlers with local ones (#34812)
  Navigation Screen: Consolidate menu name and switcher (#34786)
  Remove parent and position validation from menu item REST API endpoint (#34672)
  Clean theme data when switching themes in the customizer (#34540)
  Components: add reset timeout to ColorPicker's copy functionality. (#34601)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REST API Interaction Related to REST API [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants