From b5f6a3e34d9990273d809165ffa306bf36a4e3d2 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Wed, 8 Sep 2021 23:19:43 +0100 Subject: [PATCH 1/7] Remove validation --- lib/class-wp-rest-menu-items-controller.php | 46 ++------------------- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/lib/class-wp-rest-menu-items-controller.php b/lib/class-wp-rest-menu-items-controller.php index c84b71bc297c2..ddaaf102990d0 100644 --- a/lib/class-wp-rest-menu-items-controller.php +++ b/lib/class-wp-rest-menu-items-controller.php @@ -524,49 +524,9 @@ protected function prepare_item_for_database( $request ) { } } - // If menu id is set, valid the value of menu item position and parent id. - if ( ! empty( $prepared_nav_item['menu-id'] ) ) { - // Check if nav menu is valid. - if ( ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { - return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); - } - - // If menu item position is set to 0, insert as the last item in the existing menu. - $menu_items = wp_get_nav_menu_items( $prepared_nav_item['menu-id'], array( 'post_status' => 'publish,draft' ) ); - if ( 0 === (int) $prepared_nav_item['menu-item-position'] ) { - if ( $menu_items ) { - $last_item = $menu_items[ count( $menu_items ) - 1 ]; - if ( $last_item && isset( $last_item->menu_order ) ) { - $prepared_nav_item['menu-item-position'] = $last_item->menu_order + 1; - } else { - $prepared_nav_item['menu-item-position'] = count( $menu_items ) - 1; - } - array_push( $menu_items, $last_item ); - } else { - $prepared_nav_item['menu-item-position'] = 1; - } - } - - // Check if existing menu position is already in use by another menu item. - $menu_item_ids = array(); - foreach ( $menu_items as $menu_item ) { - $menu_item_ids[] = $menu_item->ID; - if ( $menu_item->ID !== (int) $menu_item_db_id ) { - if ( (int) $prepared_nav_item['menu-item-position'] === (int) $menu_item->menu_order ) { - return new WP_Error( 'invalid_menu_order', __( 'Invalid menu position.', 'gutenberg' ), array( 'status' => 400 ) ); - } - } - } - - // Check if valid parent id is valid nav menu item in menu. - if ( $prepared_nav_item['menu-item-parent-id'] ) { - if ( ! is_nav_menu_item( $prepared_nav_item['menu-item-parent-id'] ) ) { - return new WP_Error( 'invalid_menu_item_parent', __( 'Invalid menu item parent.', 'gutenberg' ), array( 'status' => 400 ) ); - } - if ( ! $menu_item_ids || ! in_array( $prepared_nav_item['menu-item-parent-id'], $menu_item_ids, true ) ) { - return new WP_Error( 'invalid_item_parent', __( 'Invalid menu item parent.', 'gutenberg' ), array( 'status' => 400 ) ); - } - } + // Check if nav menu is valid. + if ( ! empty( $prepared_nav_item['menu-id'] ) && ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { + return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); } foreach ( array( 'menu-item-object-id', 'menu-item-parent-id' ) as $key ) { From a9481ce96f3fa54c4c4defedb9e18059830e8897 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Thu, 9 Sep 2021 09:09:26 +0100 Subject: [PATCH 2/7] Remove tests --- ...ss-rest-nav-menu-items-controller-test.php | 90 ------------------- 1 file changed, 90 deletions(-) diff --git a/phpunit/class-rest-nav-menu-items-controller-test.php b/phpunit/class-rest-nav-menu-items-controller-test.php index 34dd004e162fe..172a354b23d2f 100644 --- a/phpunit/class-rest-nav-menu-items-controller-test.php +++ b/phpunit/class-rest-nav-menu-items-controller-test.php @@ -271,59 +271,6 @@ public function test_create_item_invalid_term() { $this->assertErrorResponse( 'rest_term_invalid_id', $response, 400 ); } - /** - * - */ - 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 ); - } - } - - /** - * - */ - public function test_create_item_invalid_position() { - wp_set_current_user( self::$admin_id ); - $new_menu_id = wp_create_nav_menu( rand_str() ); - $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( - 'menu_order' => 1, - 'menus' => $new_menu_id, - ) - ); - $request->set_body_params( $params ); - $response = rest_get_server()->dispatch( $request ); - $this->check_create_menu_item_response( $response ); - $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( - 'menu_order' => 1, - 'menus' => $new_menu_id, - ) - ); - $request->set_body_params( $params ); - $response = rest_get_server()->dispatch( $request ); - - $this->assertErrorResponse( 'invalid_menu_order', $response, 400 ); - } - /** * */ @@ -380,43 +327,6 @@ public function test_create_item_invalid_parent() { $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } - /** - * - */ - public function test_create_item_invalid_parent_menu_item() { - wp_set_current_user( self::$admin_id ); - $new_menu_id = wp_create_nav_menu( rand_str() ); - $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, - 'parent' => $this->menu_item_id, - ) - ); - $request->set_body_params( $params ); - $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'invalid_item_parent', $response, 400 ); - } - - /** - * - */ - public function test_create_item_invalid_parent_post() { - wp_set_current_user( self::$admin_id ); - $post_id = self::factory()->post->create(); - $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( - 'parent' => $post_id, - ) - ); - $request->set_body_params( $params ); - $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'invalid_menu_item_parent', $response, 400 ); - } - /** * */ From 4e97c84966d0bfaf72db360b7945fafb4006fb63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Fri, 10 Sep 2021 14:16:44 +0200 Subject: [PATCH 3/7] Validate if menu_order is set and >= 1 --- lib/class-wp-rest-menu-items-controller.php | 14 ++++- ...ss-rest-nav-menu-items-controller-test.php | 62 ++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/lib/class-wp-rest-menu-items-controller.php b/lib/class-wp-rest-menu-items-controller.php index ddaaf102990d0..636376fcdee36 100644 --- a/lib/class-wp-rest-menu-items-controller.php +++ b/lib/class-wp-rest-menu-items-controller.php @@ -524,7 +524,19 @@ protected function prepare_item_for_database( $request ) { } } - // Check if nav menu is valid. + // If menu id is set, validate the value of menu item position. + if ( ! empty( $prepared_nav_item['menu-id'] ) ) { + // Check if nav menu is valid. + if ( ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { + return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); + } + + // Check if menu item position is non-zero and positive. + if ( (int) $prepared_nav_item['menu-item-position'] < 1 ) { + return new WP_Error( 'invalid_menu_order', __( 'Invalid menu order.', 'gutenberg' ), array( 'status' => 400 ) ); + } + } + if ( ! empty( $prepared_nav_item['menu-id'] ) && ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); } diff --git a/phpunit/class-rest-nav-menu-items-controller-test.php b/phpunit/class-rest-nav-menu-items-controller-test.php index 172a354b23d2f..b793a23ef7204 100644 --- a/phpunit/class-rest-nav-menu-items-controller-test.php +++ b/phpunit/class-rest-nav-menu-items-controller-test.php @@ -271,6 +271,66 @@ public function test_create_item_invalid_term() { $this->assertErrorResponse( 'rest_term_invalid_id', $response, 400 ); } + /** + * + */ + public function test_create_item_change_position() { + wp_set_current_user( self::$admin_id ); + $new_menu_id = wp_create_nav_menu( rand_str() ); + $expected = array(); + $actual = array(); + 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( + 'menu_order' => $i, + '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(); + + $expected[] = $i; + $actual[] = $data['menu_order']; + } + $this->assertEquals( $actual, $expected ); + } + + /** + * + */ + public function test_menu_order_must_be_set() { + wp_set_current_user( self::$admin_id ); + $new_menu_id = wp_create_nav_menu( rand_str() ); + + $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( + 'menu_order' => 0, + 'menus' => $new_menu_id, + ) + ); + $request->set_body_params( $params ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'invalid_menu_order', $response, 400 ); + + $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( + 'menu_order' => 1, + 'menus' => $new_menu_id, + ) + ); + $request->set_body_params( $params ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 201, $response->get_status() ); + } + /** * */ @@ -818,7 +878,7 @@ protected function set_menu_item_data( $args = array() ) { $defaults = array( 'object_id' => 0, 'parent' => 0, - 'menu_order' => 0, + 'menu_order' => 1, 'menus' => $this->menu_id, 'type' => 'custom', 'title' => 'Custom Link Title', From 587f6bcf2799ea053a7e8603b44aaa5ee08fe643 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Sat, 11 Sep 2021 13:00:32 +0100 Subject: [PATCH 4/7] Feddback. --- lib/class-wp-rest-menu-items-controller.php | 23 ++++++++++--------- ...ss-rest-nav-menu-items-controller-test.php | 11 ++++++--- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/class-wp-rest-menu-items-controller.php b/lib/class-wp-rest-menu-items-controller.php index ddaaf102990d0..9c8fafbfb9f9c 100644 --- a/lib/class-wp-rest-menu-items-controller.php +++ b/lib/class-wp-rest-menu-items-controller.php @@ -410,7 +410,7 @@ protected function prepare_item_for_database( $request ) { 'menu-item-object-id' => 0, 'menu-item-object' => '', 'menu-item-parent-id' => 0, - 'menu-item-position' => 0, + 'menu-item-position' => 1, 'menu-item-type' => 'custom', 'menu-item-title' => '', 'menu-item-url' => '', @@ -524,11 +524,6 @@ protected function prepare_item_for_database( $request ) { } } - // Check if nav menu is valid. - if ( ! empty( $prepared_nav_item['menu-id'] ) && ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { - return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); - } - foreach ( array( 'menu-item-object-id', 'menu-item-parent-id' ) as $key ) { // Note we need to allow negative-integer IDs for previewed objects not inserted yet. $prepared_nav_item[ $key ] = (int) $prepared_nav_item[ $key ]; @@ -707,8 +702,13 @@ public function prepare_item_for_response( $post, $request ) { $base = ! empty( $taxonomy->rest_base ) ? $taxonomy->rest_base : $taxonomy->name; if ( in_array( $base, $fields, true ) ) { - $terms = get_the_terms( $post, $taxonomy->name ); - $data[ $base ] = $terms ? array_values( wp_list_pluck( $terms, 'term_id' ) ) : array(); + $terms = get_the_terms( $post, $taxonomy->name ); + $term_ids = $terms ? array_values( wp_list_pluck( $terms, 'term_id' ) ) : array(); + if ( 'nav_menu' === $taxonomy->name ) { + $data[ $base ] = $term_ids ? array_shift( $term_ids ) : 0; + } else { + $data[ $base ] = $term_ids; + } } } @@ -919,10 +919,11 @@ public function get_item_schema() { 'description' => __( 'The DB ID of the nav_menu_item that is this item\'s menu parent, if any, otherwise 0.', 'gutenberg' ), 'context' => array( 'view', 'edit', 'embed' ), 'type' => 'integer', - 'minimum' => 0, - 'default' => 0, + 'minimum' => 1, + 'default' => 1, ); - $schema['properties']['object'] = array( + + $schema['properties']['object'] = array( 'description' => __( 'The type of object originally represented, such as "category," "post", or "attachment."', 'gutenberg' ), 'context' => array( 'view', 'edit', 'embed' ), 'type' => 'string', diff --git a/phpunit/class-rest-nav-menu-items-controller-test.php b/phpunit/class-rest-nav-menu-items-controller-test.php index 172a354b23d2f..24949a4953329 100644 --- a/phpunit/class-rest-nav-menu-items-controller-test.php +++ b/phpunit/class-rest-nav-menu-items-controller-test.php @@ -740,8 +740,13 @@ protected function check_menu_item_data( $post, $data, $context, $links ) { $this->assertTrue( isset( $data[ $taxonomy->rest_base ] ) ); $terms = wp_get_object_terms( $post->ID, $taxonomy->name, array( 'fields' => 'ids' ) ); sort( $terms ); - sort( $data[ $taxonomy->rest_base ] ); - $this->assertEquals( $terms, $data[ $taxonomy->rest_base ] ); + if ( 'nav_menu' === $taxonomy->name ) { + $term_id = $terms ? array_shift( $terms ) : 0; + $this->assertEquals( $term_id, $data[ $taxonomy->rest_base ] ); + } else { + sort( $data[ $taxonomy->rest_base ] ); + $this->assertEquals( $terms, $data[ $taxonomy->rest_base ] ); + } } // test links. @@ -818,7 +823,7 @@ protected function set_menu_item_data( $args = array() ) { $defaults = array( 'object_id' => 0, 'parent' => 0, - 'menu_order' => 0, + 'menu_order' => 1, 'menus' => $this->menu_id, 'type' => 'custom', 'title' => 'Custom Link Title', From 2c950880a0b5ef1bb71eb42b37985527f63f72f5 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Sat, 11 Sep 2021 13:03:30 +0100 Subject: [PATCH 5/7] Remove validation again. --- lib/class-wp-rest-menu-items-controller.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/lib/class-wp-rest-menu-items-controller.php b/lib/class-wp-rest-menu-items-controller.php index d3ef6504359ce..9c8fafbfb9f9c 100644 --- a/lib/class-wp-rest-menu-items-controller.php +++ b/lib/class-wp-rest-menu-items-controller.php @@ -524,26 +524,6 @@ protected function prepare_item_for_database( $request ) { } } -<<<<<<< HEAD -======= - // If menu id is set, validate the value of menu item position. - if ( ! empty( $prepared_nav_item['menu-id'] ) ) { - // Check if nav menu is valid. - if ( ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { - return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); - } - - // Check if menu item position is non-zero and positive. - if ( (int) $prepared_nav_item['menu-item-position'] < 1 ) { - return new WP_Error( 'invalid_menu_order', __( 'Invalid menu order.', 'gutenberg' ), array( 'status' => 400 ) ); - } - } - - if ( ! empty( $prepared_nav_item['menu-id'] ) && ! is_nav_menu( $prepared_nav_item['menu-id'] ) ) { - return new WP_Error( 'invalid_menu_id', __( 'Invalid menu ID.', 'gutenberg' ), array( 'status' => 400 ) ); - } - ->>>>>>> 4e97c84966d0bfaf72db360b7945fafb4006fb63 foreach ( array( 'menu-item-object-id', 'menu-item-parent-id' ) as $key ) { // Note we need to allow negative-integer IDs for previewed objects not inserted yet. $prepared_nav_item[ $key ] = (int) $prepared_nav_item[ $key ]; From fa7671e3e4276086a6536b90b85e9ef6886bacc2 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Tue, 14 Sep 2021 01:08:31 +0100 Subject: [PATCH 6/7] Fix test. --- phpunit/class-rest-nav-menu-items-controller-test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit/class-rest-nav-menu-items-controller-test.php b/phpunit/class-rest-nav-menu-items-controller-test.php index 7910098ee7829..46c55d17653ec 100644 --- a/phpunit/class-rest-nav-menu-items-controller-test.php +++ b/phpunit/class-rest-nav-menu-items-controller-test.php @@ -316,7 +316,7 @@ public function test_menu_order_must_be_set() { ); $request->set_body_params( $params ); $response = rest_get_server()->dispatch( $request ); - $this->assertErrorResponse( 'invalid_menu_order', $response, 400 ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); $request = new WP_REST_Request( 'POST', '/__experimental/menu-items' ); $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); From 8f2ae78031c5a5c16a8905c40c77f067be3398f9 Mon Sep 17 00:00:00 2001 From: Jonny Harris Date: Tue, 14 Sep 2021 01:27:21 +0100 Subject: [PATCH 7/7] Menu order 1. --- packages/edit-navigation/src/store/actions.js | 2 +- packages/edit-navigation/src/store/test/actions.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/edit-navigation/src/store/actions.js b/packages/edit-navigation/src/store/actions.js index 587e4cf62e080..ea2897f63992f 100644 --- a/packages/edit-navigation/src/store/actions.js +++ b/packages/edit-navigation/src/store/actions.js @@ -65,7 +65,7 @@ export const createMissingMenuItems = serializeProcessing( function* ( post ) { data: { title: 'Placeholder', url: 'Placeholder', - menu_order: 0, + menu_order: 1, }, } ); diff --git a/packages/edit-navigation/src/store/test/actions.js b/packages/edit-navigation/src/store/test/actions.js index 0c2bf7c90f681..fc5ca7f5c4a65 100644 --- a/packages/edit-navigation/src/store/test/actions.js +++ b/packages/edit-navigation/src/store/test/actions.js @@ -77,7 +77,7 @@ describe( 'createMissingMenuItems', () => { data: { title: 'Placeholder', url: 'Placeholder', - menu_order: 0, + menu_order: 1, }, } ) ); @@ -200,7 +200,7 @@ describe( 'createMissingMenuItems', () => { data: { title: 'Placeholder', url: 'Placeholder', - menu_order: 0, + menu_order: 1, }, } ) );