React to any errors coming up in gutenberg_migrate_menu_to_navigation_post#36461
Merged
React to any errors coming up in gutenberg_migrate_menu_to_navigation_post#36461
Conversation
adamziel
requested review from
draganescu,
getdave,
noisysocks,
spacedmonkey and
talldan
getdave
approved these changes
Nov 13, 2021
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
👍 Thanks for handling this. Looks good.
I left a suggestion around comments and also a query.
| } | ||
|
|
||
| add_action( 'switch_theme', 'gutenberg_migrate_menu_to_navigation_post', 200, 3 ); | ||
| add_action( 'switch_theme', 'gutenberg_migrate_menu_to_navigation_post', 99, 3 ); |
Contributor
There was a problem hiding this comment.
Why do we update the ordering here?
Contributor
Author
There was a problem hiding this comment.
Because of this change: https://github.com/WordPress/wordpress-develop/pull/1865/files#diff-4112a9a0bb18713bc2fc7868ceffe4403d765722f605e42b313ef2cdc218177fR685
I learned it's more typical to use priority at most 99 in core and that's how it was backported.
Co-authored-by: Dave Smith <getdavemail@gmail.com>
noisysocks
pushed a commit
that referenced
this pull request
Nov 15, 2021
…_post (#36461) * React to any errors coming up in gutenberg_migrate_menu_to_navigation_post * Lint * Update lib/navigation.php Co-authored-by: Dave Smith <getdavemail@gmail.com> Co-authored-by: Dave Smith <getdavemail@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As @spacedmonkey mentioned in a review of the navigation areas PR, migrating menu locations to navigation areas on theme switch,
wp_insert_postmay fail and return WP_Error instance. This must be handled somehow, and I propose to just short-circuit the entire migration. There isn't a fully functional mapping available, so I'd rather abort than set a half-baked one.