From a40a8671d1083c089f7c9f25764f8144e9c45858 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 28 May 2021 12:45:05 +0100 Subject: [PATCH 1/5] Avoid unwanted requests for menus which cannot exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/edit-navigation/src/store/reducer.js | 2 +- packages/edit-navigation/src/store/selectors.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-navigation/src/store/reducer.js b/packages/edit-navigation/src/store/reducer.js index 8511ccd03d049..cab46d584cf85 100644 --- 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 index 764c7effc7c69..ed9b947f62b49 100644 --- 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; } /** From cca6f0450d7235ea95d4d7833f45ea2da2073c79 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 28 May 2021 12:45:23 +0100 Subject: [PATCH 2/5] Track resolution state of requested menu --- .../src/hooks/use-menu-entity.js | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/edit-navigation/src/hooks/use-menu-entity.js b/packages/edit-navigation/src/hooks/use-menu-entity.js index 7d3946e201a06..ff0f011ac1a34 100644 --- a/packages/edit-navigation/src/hooks/use-menu-entity.js +++ b/packages/edit-navigation/src/hooks/use-menu-entity.js @@ -12,10 +12,21 @@ 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 ] ); @@ -23,5 +34,6 @@ export default function useMenuEntity( menuId ) { editedMenu, menuEntityData, editMenuEntityRecord: editEntityRecord, + hasLoadedEditedMenu, }; } From d41efd738636ab11bb58950c223f80ea009cd90b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 28 May 2021 12:45:55 +0100 Subject: [PATCH 3/5] Reset the selected menu ID if the requested menu returns empty dataset --- .../src/hooks/use-navigation-editor.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/edit-navigation/src/hooks/use-navigation-editor.js b/packages/edit-navigation/src/hooks/use-navigation-editor.js index cec78cf3a59dc..87c18ae51762c 100644 --- 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. From f5010497dec8e8ea3461dcce02381949fe5f4efb Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 28 May 2021 14:12:57 +0100 Subject: [PATCH 4/5] Update tests --- packages/edit-navigation/src/store/test/reducer.js | 2 +- packages/edit-navigation/src/store/test/selectors.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-navigation/src/store/test/reducer.js b/packages/edit-navigation/src/store/test/reducer.js index d987bedf6f1e8..b4758643cca55 100644 --- 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 index 50eae47e959fd..fd70203c47f31 100644 --- 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', () => { From 76c565893b48572282d19c13557c2ff08cfd42d6 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 28 May 2021 14:24:16 +0100 Subject: [PATCH 5/5] Mock pages endpoint response --- .../specs/experiments/navigation-editor.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/e2e-tests/specs/experiments/navigation-editor.test.js b/packages/e2e-tests/specs/experiments/navigation-editor.test.js index 9e9d049938726..15e8b02c3492b 100644 --- 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' );