Editor: Incorrect error handling when converting classic to block menus

[dlh]

Originally filed as ​https://github.com/WordPress/gutenberg/issues/52481:

Some errors that can occur when WP_Navigation_Fallback::create_classic_menu_fallback() calls WP_Classic_To_Block_Menu_Converter::convert() aren't handled as accurately as they could be.

1. ::convert() can return a WP_Error object, but this error object won't be detected by the empty() check in ::create_classic_menu_fallback(). The WP_Error return type is also missing from the docs.
2. ::convert() returns an array instead of a string when no menu items are returned by wp_get_nav_menu_items().

See here: ​https://github.com/WordPress/wordpress-develop/blob/be1cb4583abc8d0870506dfeec9aad7f6cf82c7e/src/wp-includes/class-wp-navigation-fallback.php#L105-L110

and here: ​https://github.com/WordPress/wordpress-develop/blob/be1cb4583abc8d0870506dfeec9aad7f6cf82c7e/src/wp-includes/class-wp-classic-to-block-menu-converter.php#L17-L56

Thank you for this PR and catching those Issues. Looks like the unit tests will need updating to conform to the changes. Do you think you will be able (have capacity) to do that?

Also cc'ing @spacedmonkey as he may need to oversee this being merged into Core.

This will also resolve ​https://github.com/WordPress/gutenberg/issues/52481

Yep, it's on my list to update in the next week or so, I hope.

@getdave Test updated!

[get_dave]

This PR seems to be ready to land. Are any core contributors available to help merge?

[hellofromTonya]

Pulling this ticket and patch into 6.3.1.

Code was introduced in 6.3.0 via [56052] / #58557.

[hellofromTonya]

​The patch is ready. Prepping the trunk commit.

[hellofromTonya]

In 56422:

Editor: Fix error handling of converting classic to block menus.

Fixes the error handling for when WP_Classic_To_Block_Menu_Converter::convert() returns an instance of WP_Error. WP_Navigation_Fallback::create_classic_menu_fallback() now checks for is_wp_error() and if true, returns the error. And the @return type is updated to string|WP_Error.

Also includes a fix in the return type in WP_Classic_To_Block_Menu_Converter::convert() to return an empty string instead of an array instead, i.e. when bailing out for no menu items returned by wp_get_nav_menu_items(). The return type is clearly documented as a string.

Follow-up to [56052].

Props dlh, get_dave, antonvlasenko, hellofromTonya.
Fixes #58823.

[hellofromTonya]

Reopening for backport to 6.3.x consideration.

Pinging @azaozz.

Committed via https://core.trac.wordpress.org/changeset/56422. Thank you everyone for your contributions!

[hellofromTonya]

@azaozz I moved this fix into 6.3.1 as it was introduced in 6.3.0. It might not be severe enough to be bundled into the .1 minor and could likely wait until .2 minor. Take a look and see what you think.

[azaozz]

Replying to hellofromTonya:

Thanks for the ping! Looking at [56422] it seems simple and straightforward enough to add to 6.3.1. The only thing I'm a bit unsure is whether it affects a lot of people, i.e. if it is a higher priority fix.

[audrasjb]

I don't think it's a high priority issue, but given the minimal scope of the patch I think we can safely include this bugfix in 6.3.1.

[hellofromTonya]

Removing myself as owner, just in case I'm not available to do the backport to the 6.3-branch, once approved for backport.

[audrasjb]

As per today's bug scrub, marking this as reviewed.

[audrasjb]

Self assigning for backport

[audrasjb]

WP 6.3.1 is going to be released in the next few days, so let's move this ticket to 6.3.2 to give it more time to be committed and backported.

[audrasjb]

Reassigning to milestone 6.3.1.

[audrasjb]

In 56426:

Editor: Fix error handling of converting classic to block menus.

Fixes the error handling for when WP_Classic_To_Block_Menu_Converter::convert() returns an instance of WP_Error.
WP_Navigation_Fallback::create_classic_menu_fallback() now checks for is_wp_error() and if true, returns the error. And the @return type is updated to
string|WP_Error.

Also includes a fix in the return type in WP_Classic_To_Block_Menu_Converter::convert() to return an empty string instead of an array instead, i.e. when bailing
out for no menu items returned by wp_get_nav_menu_items(). The return type is clearly documented as a string.

Follow-up to [56052].

Props dlh, get_dave, antonvlasenko, hellofromTonya.
Reviewed by azaozz, audrasjb.
Merges [56422] to the 6.3 branch.
Fixes #58823.

--Cette ligne, et les suivantes ci-dessous, seront

ignorées--

_M .
M src/wp-includes/class-wp-classic-to-block-menu-converter.php
M src/wp-includes/class-wp-navigation-fallback.php
M tests/phpunit/tests/editor/classic-to-block-menu-converter.php