Remove parent and position validation from menu item REST API endpoint #34672
Conversation
|
Thanks for working on this. Looks like just the PHP Unit tests to update and this should be good to go. |
| public function test_create_item_change_position() { | ||
| wp_set_current_user( self::$admin_id ); | ||
| $new_menu_id = wp_create_nav_menu( rand_str() ); | ||
| for ( $i = 1; $i < 5; $i ++ ) { | ||
| $request = new WP_REST_Request( 'POST', '/__experimental/menu-items' ); | ||
| $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); | ||
| $params = $this->set_menu_item_data( | ||
| array( | ||
| 'menus' => $new_menu_id, | ||
| ) | ||
| ); | ||
| $request->set_body_params( $params ); | ||
| $response = rest_get_server()->dispatch( $request ); | ||
| $this->check_create_menu_item_response( $response ); | ||
| $data = $response->get_data(); | ||
| $this->assertEquals( $data['menu_order'], $i ); | ||
| } | ||
| } |
talldan
Sep 10, 2021
Contributor
Should this test be removed? It doesn't seem to trigger validation.
Should this test be removed? It doesn't seem to trigger validation.
spacedmonkey
Sep 10, 2021
Author
Member
It failed.
It failed.
adamziel
Sep 10, 2021
•
Contributor
It failed because it resulted in the following sequence of positions: 0, 2, 3, 4, 5. According to https://core.trac.wordpress.org/ticket/28140, menu position 0 is invalid and breaks things so it would be good to get it fixed. I proposed an adjustment in 4e97c84 (feel free to roll it back) – what do you think @spacedmonkey ?
It failed because it resulted in the following sequence of positions: 0, 2, 3, 4, 5. According to https://core.trac.wordpress.org/ticket/28140, menu position 0 is invalid and breaks things so it would be good to get it fixed. I proposed an adjustment in 4e97c84 (feel free to roll it back) – what do you think @spacedmonkey ?
spacedmonkey
Sep 10, 2021
Author
Member
I think it might be better change the schema to make the min 1 and default 1.
gutenberg/lib/class-wp-rest-menu-items-controller.php
Lines 958 to 964
in
8e4ebfa
I think it might be better change the schema to make the min 1 and default 1.
gutenberg/lib/class-wp-rest-menu-items-controller.php
Lines 958 to 964 in 8e4ebfa
spacedmonkey
Sep 10, 2021
Author
Member
I would also think about making the position a required field.
I would also think about making the position a required field.
adamziel
Sep 10, 2021
Contributor
Nice! Yes, let's do that instead
Nice! Yes, let's do that instead
talldan
changed the title
|
Renamed the PR to be a little clearer as the title is what appears in the Gutenberg changelog. |
|
Please do not merge until this is resolved - #34672 (comment) |
Description
As noted in #25093 (comment), we need to remove this validation, so that we can make batch requests work. This PR is simple, remove validation for parent and position.
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).Fixes #25093