From 8889a5db11c7a3763c77b11d54436efd963f4325 Mon Sep 17 00:00:00 2001
From: Daniel Richards <daniel.richards@automattic.com>
Date: Mon, 8 Feb 2021 09:33:10 +0800
Subject: [PATCH] Navigation screen: Fix failing request for menu items
 (#28764)

* Avoid making a request for menu items before menus have been fetched

* Add unit test

* Improve handling of pending state for editor

* Also disable save button when there is no navigation post
---
 .../src/components/header/index.js                |  2 +-
 .../src/components/layout/index.js                |  7 ++++---
 .../components/layout/use-navigation-editor.js    | 15 +++++++++++----
 .../src/components/toolbar/save-button.js         |  1 +
 packages/edit-navigation/src/store/resolvers.js   |  4 ++++
 .../edit-navigation/src/store/test/resolvers.js   |  6 ++++++
 6 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/packages/edit-navigation/src/components/header/index.js b/packages/edit-navigation/src/components/header/index.js
index 4e27a3a24778c..c930693a9ad4a 100644
--- a/packages/edit-navigation/src/components/header/index.js
+++ b/packages/edit-navigation/src/components/header/index.js
@@ -61,7 +61,7 @@ export default function Header( {
 						showTooltip: false,
 						children: __( 'Select menu' ),
 						isTertiary: true,
-						disabled: ! menus,
+						disabled: ! menus?.length,
 						__experimentalIsFocusable: true,
 					} }
 					popoverProps={ {
diff --git a/packages/edit-navigation/src/components/layout/index.js b/packages/edit-navigation/src/components/layout/index.js
index 37ce18de7580b..357fc400d073a 100644
--- a/packages/edit-navigation/src/components/layout/index.js
+++ b/packages/edit-navigation/src/components/layout/index.js
@@ -37,6 +37,7 @@ export default function Layout( { blockEditorSettings } ) {
 		navigationPost,
 		selectMenu,
 		deleteMenu,
+		hasLoadedMenus,
 	} = useNavigationEditor();
 
 	const [ blocks, onInput, onChange ] = useNavigationBlockEditor(
@@ -56,7 +57,7 @@ export default function Layout( { blockEditorSettings } ) {
 
 					<div className="edit-navigation-layout">
 						<Header
-							isPending={ ! navigationPost }
+							isPending={ ! hasLoadedMenus }
 							menus={ menus }
 							selectedMenuId={ selectedMenuId }
 							onSelectMenu={ selectMenu }
@@ -78,11 +79,11 @@ export default function Layout( { blockEditorSettings } ) {
 								saveBlocks={ savePost }
 							/>
 							<Toolbar
-								isPending={ ! navigationPost }
+								isPending={ ! hasLoadedMenus }
 								navigationPost={ navigationPost }
 							/>
 							<Editor
-								isPending={ ! navigationPost }
+								isPending={ ! hasLoadedMenus }
 								blocks={ blocks }
 							/>
 							<InspectorAdditions
diff --git a/packages/edit-navigation/src/components/layout/use-navigation-editor.js b/packages/edit-navigation/src/components/layout/use-navigation-editor.js
index bdc615828de3b..2606d7a2cb3e6 100644
--- a/packages/edit-navigation/src/components/layout/use-navigation-editor.js
+++ b/packages/edit-navigation/src/components/layout/use-navigation-editor.js
@@ -10,10 +10,16 @@ import { useState, useEffect } from '@wordpress/element';
 import { store as editNavigationStore } from '../../store';
 
 export default function useNavigationEditor() {
-	const menus = useSelect(
-		( select ) => select( 'core' ).getMenus( { per_page: -1 } ),
-		[]
-	);
+	const { menus, hasLoadedMenus } = useSelect( ( select ) => {
+		const selectors = select( 'core' );
+		const params = { per_page: -1 };
+		return {
+			menus: selectors.getMenus( params ),
+			hasLoadedMenus: selectors.hasFinishedResolution( 'getMenus', [
+				params,
+			] ),
+		};
+	}, [] );
 
 	const [ selectedMenuId, setSelectedMenuId ] = useState( null );
 
@@ -52,5 +58,6 @@ export default function useNavigationEditor() {
 		navigationPost,
 		selectMenu,
 		deleteMenu,
+		hasLoadedMenus,
 	};
 }
diff --git a/packages/edit-navigation/src/components/toolbar/save-button.js b/packages/edit-navigation/src/components/toolbar/save-button.js
index 92132f47198d6..4d526ebbe40ed 100644
--- a/packages/edit-navigation/src/components/toolbar/save-button.js
+++ b/packages/edit-navigation/src/components/toolbar/save-button.js
@@ -20,6 +20,7 @@ export default function SaveButton( { navigationPost } ) {
 			onClick={ () => {
 				saveNavigationPost( navigationPost );
 			} }
+			disabled={ ! navigationPost }
 		>
 			{ __( 'Save' ) }
 		</Button>
diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js
index af3b59a02b8ac..a7b8bb5ca67ed 100644
--- a/packages/edit-navigation/src/store/resolvers.js
+++ b/packages/edit-navigation/src/store/resolvers.js
@@ -25,6 +25,10 @@ import { KIND, POST_TYPE, buildNavigationPostId } from './utils';
  * @return {void}
  */
 export function* getNavigationPostForMenu( menuId ) {
+	if ( ! menuId ) {
+		return;
+	}
+
 	const stubPost = createStubPost( menuId );
 	// Persist an empty post to warm up the state
 	yield persistPost( stubPost );
diff --git a/packages/edit-navigation/src/store/test/resolvers.js b/packages/edit-navigation/src/store/test/resolvers.js
index 984be802bb4bd..c056222e1595c 100644
--- a/packages/edit-navigation/src/store/test/resolvers.js
+++ b/packages/edit-navigation/src/store/test/resolvers.js
@@ -24,6 +24,12 @@ jest.mock( '@wordpress/blocks', () => {
 } );
 
 describe( 'getNavigationPostForMenu', () => {
+	it( 'returns early when a menuId is not provided', () => {
+		const generator = getNavigationPostForMenu( null );
+		expect( generator.next().value ).toBeUndefined();
+		expect( generator.next().done ).toBe( true );
+	} );
+
 	it( 'gets navigation post for menu id', () => {
 		const menuId = 123;