From ca4cadf63e28998e463615eab269b5b56cc26bd8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Jun 2021 12:55:56 +0100 Subject: [PATCH] Alternative fix: set persisted menu id when no menus or missing menu (#32313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../experiments/navigation-editor.test.js | 10 ++++++++++ .../src/hooks/use-menu-entity.js | 20 +++++++++++++++---- .../src/hooks/use-navigation-editor.js | 14 ++++++++++++- packages/edit-navigation/src/store/reducer.js | 2 +- .../edit-navigation/src/store/selectors.js | 2 +- .../edit-navigation/src/store/test/reducer.js | 2 +- .../src/store/test/selectors.js | 2 +- 7 files changed, 43 insertions(+), 9 deletions(-) diff --git a/packages/e2e-tests/specs/experiments/navigation-editor.test.js b/packages/e2e-tests/specs/experiments/navigation-editor.test.js index 9e9d0499387263..15e8b02c3492b9 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' ); diff --git a/packages/edit-navigation/src/hooks/use-menu-entity.js b/packages/edit-navigation/src/hooks/use-menu-entity.js index 7d3946e201a06a..ff0f011ac1a341 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, }; } diff --git a/packages/edit-navigation/src/hooks/use-navigation-editor.js b/packages/edit-navigation/src/hooks/use-navigation-editor.js index cec78cf3a59dc2..87c18ae51762c9 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. diff --git a/packages/edit-navigation/src/store/reducer.js b/packages/edit-navigation/src/store/reducer.js index 8511ccd03d0496..cab46d584cf85f 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 764c7effc7c69d..ed9b947f62b495 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; } /** diff --git a/packages/edit-navigation/src/store/test/reducer.js b/packages/edit-navigation/src/store/test/reducer.js index d987bedf6f1e88..b4758643cca558 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 50eae47e959fd7..fd70203c47f315 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', () => {