Alternative fix: set persisted menu id when no menus or missing menu (#…

…32313) * Avoid unwanted requests for menus which cannot exist 0 is a valid menuID so we had requests going out to `/menus/0` which always 404’d. By using `null` as the default we can reset a selected menu ID and ensure that: 1. No requests go out. 2. We’re clearer that we have no menu set. * Track resolution state of requested menu * Reset the selected menu ID if the requested menu returns empty dataset * Update tests * Mock pages endpoint response
WordPress · Jun 3, 2021 · ca4cadf · ca4cadf
1 parent 7e360e4
commit ca4cadf
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 9 deletions.
diff --git a/packages/e2e-tests/specs/experiments/navigation-editor.test.js b/packages/e2e-tests/specs/experiments/navigation-editor.test.js
@@ -83,6 +83,11 @@ const REST_SEARCH_ROUTES = [
 	`rest_route=${ encodeURIComponent( '/wp/v2/search' ) }`,
 ];
 
+const REST_PAGES_ROUTES = [
+	'/wp/v2/pages',
+	`rest_route=${ encodeURIComponent( '/wp/v2/pages' ) }`,
+];
+
 /**
  * Determines if a given URL matches any of a given collection of
  * routes (expressed as substrings).
@@ -135,6 +140,10 @@ function getSearchMocks( responsesByMethod ) {
 	return getEndpointMocks( REST_SEARCH_ROUTES, responsesByMethod );
 }
 
+function getPagesMocks( responsesByMethod ) {
+	return getEndpointMocks( REST_PAGES_ROUTES, responsesByMethod );
+}
+
 async function visitNavigationEditor() {
 	const query = addQueryArgs( '', {
 		page: 'gutenberg-navigation',
@@ -184,6 +193,7 @@ describe( 'Navigation editor', () => {
 				POST: menuPostResponse,
 			} ),
 			...getMenuItemMocks( { GET: [] } ),
+			...getPagesMocks( { GET: [ {} ] } ), // mock a single page
 		] );
 
 		await page.keyboard.type( 'Main Menu' );

diff --git a/packages/edit-navigation/src/hooks/use-menu-entity.js b/packages/edit-navigation/src/hooks/use-menu-entity.js
@@ -12,16 +12,28 @@ export default function useMenuEntity( menuId ) {
 	const { editEntityRecord } = useDispatch( coreStore );
 
 	const menuEntityData = [ MENU_KIND, MENU_POST_TYPE, menuId ];
-	const editedMenu = useSelect(
-		( select ) =>
-			menuId &&
-			select( coreStore ).getEditedEntityRecord( ...menuEntityData ),
+	const { editedMenu, hasLoadedEditedMenu } = useSelect(
+		( select ) => {
+			return {
+				editedMenu:
+					menuId &&
+					select( coreStore ).getEditedEntityRecord(
+						...menuEntityData
+					),
+				hasLoadedEditedMenu: select(
+					coreStore
+				).hasFinishedResolution( 'getEditedEntityRecord', [
+					...menuEntityData,
+				] ),
+			};
+		},
 		[ menuId ]
 	);
 
 	return {
 		editedMenu,
 		menuEntityData,
 		editMenuEntityRecord: editEntityRecord,
+		hasLoadedEditedMenu,
 	};
 }
diff --git a/packages/edit-navigation/src/hooks/use-navigation-editor.js b/packages/edit-navigation/src/hooks/use-navigation-editor.js
@@ -12,6 +12,7 @@ import { store as coreStore } from '@wordpress/core-data';
  */
 import { store as editNavigationStore } from '../store';
 import { useSelectedMenuId } from './index';
+import useMenuEntity from './use-menu-entity';
 
 const getMenusData = ( select ) => {
 	const selectors = select( 'core' );
@@ -29,8 +30,19 @@ export default function useNavigationEditor() {
 	const [ hasFinishedInitialLoad, setHasFinishedInitialLoad ] = useState(
 		false
 	);
+	const { editedMenu, hasLoadedEditedMenu } = useMenuEntity( selectedMenuId );
 	const { menus, hasLoadedMenus } = useSelect( getMenusData, [] );
 
+	/**
+	 * If the Menu being edited has been requested from API and it has
+	 * no values then it has been deleted so reset the selected menu ID.
+	 */
+	useEffect( () => {
+		if ( hasLoadedEditedMenu && ! Object.keys( editedMenu )?.length ) {
+			setSelectedMenuId( null );
+		}
+	}, [ hasLoadedEditedMenu, editedMenu ] );
+
 	const { createErrorNotice, createInfoNotice } = useDispatch( noticesStore );
 	const isMenuBeingDeleted = useSelect(
 		( select ) =>
@@ -67,7 +79,7 @@ export default function useNavigationEditor() {
 			force: true,
 		} );
 		if ( didDeleteMenu ) {
-			setSelectedMenuId( 0 );
+			setSelectedMenuId( null );
 			createInfoNotice(
 				sprintf(
 					// translators: %s: the name of a menu.

diff --git a/packages/edit-navigation/src/store/reducer.js b/packages/edit-navigation/src/store/reducer.js
@@ -89,7 +89,7 @@ export function processingQueue( state, action ) {
  *
  * @return {Object} Updated state.
  */
-export function selectedMenuId( state = 0, action ) {
+export function selectedMenuId( state = null, action ) {
 	switch ( action.type ) {
 		case 'SET_SELECTED_MENU_ID':
 			return action.menuId;

diff --git a/packages/edit-navigation/src/store/selectors.js b/packages/edit-navigation/src/store/selectors.js
@@ -23,7 +23,7 @@ import { buildNavigationPostId } from './utils';
  * @return {number} The selected menu ID.
  */
 export function getSelectedMenuId( state ) {
-	return state.selectedMenuId ?? 0;
+	return state.selectedMenuId ?? null;
 }
 
 /**

diff --git a/packages/edit-navigation/src/store/test/reducer.js b/packages/edit-navigation/src/store/test/reducer.js
@@ -179,7 +179,7 @@ describe( 'processingQueue', () => {
 
 describe( 'selectedMenuId', () => {
 	it( 'should apply default state', () => {
-		expect( selectedMenuId( undefined, {} ) ).toEqual( 0 );
+		expect( selectedMenuId( undefined, {} ) ).toEqual( null );
 	} );
 
 	it( 'should update when a new menu is selected', () => {

diff --git a/packages/edit-navigation/src/store/test/selectors.js b/packages/edit-navigation/src/store/test/selectors.js
@@ -135,7 +135,7 @@ describe( 'getMenuItemForClientId', () => {
 describe( 'getSelectedMenuId', () => {
 	it( 'returns default selected menu ID (zero)', () => {
 		const state = {};
-		expect( getSelectedMenuId( state ) ).toBe( 0 );
+		expect( getSelectedMenuId( state ) ).toBe( null );
 	} );
 
 	it( 'returns selected menu ID', () => {