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 'menu_exists' response status code #34888

Merged
merged 1 commit into from Sep 17, 2021
Merged

Fix 'menu_exists' response status code #34888

merged 1 commit into from Sep 17, 2021

Conversation

@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Sep 16, 2021

Description

Alterative version of #34868

Menu REST API was returning response code 500 for menu already exists error. PR adjusts the check to set the status code correctly.

How has this been tested?

  1. Go to the experimental Navigation screen.
  2. Try to create a new menu with the name that already exists.
wp.data.dispatch('core').saveMenu( { name: 'Menu' } );
  1. Check the error in DevTools' Network tab.
  2. Status code should be 400.

Screenshots

CleanShot 2021-09-16 at 14 02 24

Types of changes

Bugfix

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).
$term->add_data(
array(
'status' => 400,
'term_id' => $term_id,
'term_id' => $existing_term->term_id,

This comment has been minimized.

@Mamaduka

Mamaduka Sep 16, 2021
Member

Suggested change
'term_id' => $existing_term->term_id,
'menu_id' => $existing_term->term_id,

Nit: Maybe we should use menu_id here.

This comment has been minimized.

@spacedmonkey

spacedmonkey Sep 16, 2021
Author Member

I am trying to keep the response the same as this.

)
);
} else {
$term->add_data( array( 'status' => 400 ) );

This comment has been minimized.

@Mamaduka

Mamaduka Sep 16, 2021
Member

This is great 🌟

Copy link
Member

@Mamaduka Mamaduka left a comment

Tested and works as expected 👍

I only left one minor suggestion.

@Mamaduka Mamaduka merged commit 4f3d93e into trunk Sep 17, 2021
20 checks passed
20 checks passed
@github-actions
Compute current and next stable release branches
Details
@github-actions
Checks (12)
Details
@github-actions
Admin - 1
Details
@github-actions
Run performance tests
Details
@github-actions
pull-request-automation (14)
Details
@github-actions
pull-request-automation (14)
Details
@github-actions
test (gutenberg-editor-initial-html, 14)
Details
@github-actions
test (12.2, gutenberg-editor-initial-html, 14)
Details
@github-actions
JavaScript (12)
Details
@github-actions
Checks (14)
Details
@github-actions
Admin - 2
Details
@github-actions
JavaScript (14)
Details
@github-actions
Admin - 3
Details
@github-actions
Admin - 4
Details
@github-actions
Bump version
Details
@github-actions
Build Release Artifact
Details
@github-actions
Mobile
Details
@github-actions
Create Release Draft and Attach Asset
Details
@Mamaduka Mamaduka deleted the fix/menu_exists_error branch Sep 17, 2021
@github-actions github-actions bot added this to the Gutenberg 11.6 milestone Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants