From 931f934367f931407e0c7f92a6e3d1b44171ce6d Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 11 Mar 2021 10:05:57 +0800 Subject: [PATCH 1/2] Fix navigation editor saving --- packages/edit-navigation/src/store/actions.js | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/packages/edit-navigation/src/store/actions.js b/packages/edit-navigation/src/store/actions.js index 90dcf5aa19227..aa3e91949465a 100644 --- a/packages/edit-navigation/src/store/actions.js +++ b/packages/edit-navigation/src/store/actions.js @@ -7,7 +7,7 @@ import { v4 as uuid } from 'uuid'; /** * WordPress dependencies */ -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; import { store as noticesStore } from '@wordpress/notices'; /** @@ -17,6 +17,7 @@ import { getMenuItemToClientIdMapping, resolveMenuItems, dispatch, + select, apiFetch, } from './controls'; import { @@ -89,7 +90,7 @@ export const saveNavigationPost = serializeProcessing( function* ( post ) { try { // Save edits to the menu, like the menu name. - const menuResponse = yield dispatch( + yield dispatch( 'core', 'saveEditedEntityRecord', 'root', @@ -97,6 +98,18 @@ export const saveNavigationPost = serializeProcessing( function* ( post ) { menuId ); + const error = yield select( + 'core', + 'getLastEntitySaveError', + 'root', + 'menu', + menuId + ); + + if ( error ) { + throw new Error( error.message ); + } + // Save blocks as menu items. const batchSaveResponse = yield* batchSave( menuId, @@ -104,8 +117,8 @@ export const saveNavigationPost = serializeProcessing( function* ( post ) { post.blocks[ 0 ] ); - if ( ! batchSaveResponse.success || ! menuResponse ) { - throw new Error(); + if ( ! batchSaveResponse.success ) { + throw new Error( batchSaveResponse.data.message ); } yield dispatch( @@ -116,15 +129,17 @@ export const saveNavigationPost = serializeProcessing( function* ( post ) { type: 'snackbar', } ); - } catch ( e ) { - yield dispatch( - noticesStore, - 'createErrorNotice', - __( 'There was an error.' ), - { - type: 'snackbar', - } - ); + } catch ( saveError ) { + const errorMessage = saveError + ? sprintf( + /* translators: %s: The text of an error message (potentially untranslated). */ + __( "Unable to save: '%s'" ), + saveError + ) + : __( 'Unable to save: An error ocurred.' ); + yield dispatch( noticesStore, 'createErrorNotice', errorMessage, { + type: 'snackbar', + } ); } } ); From 6331af3f0c525824fd504d631dddeada4c7a4926 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 11 Mar 2021 11:15:13 +0800 Subject: [PATCH 2/2] Update error message and tests --- packages/edit-navigation/src/store/actions.js | 2 +- .../edit-navigation/src/store/test/actions.js | 141 +++++++++++++++++- 2 files changed, 135 insertions(+), 8 deletions(-) diff --git a/packages/edit-navigation/src/store/actions.js b/packages/edit-navigation/src/store/actions.js index aa3e91949465a..de4ec3e1691c1 100644 --- a/packages/edit-navigation/src/store/actions.js +++ b/packages/edit-navigation/src/store/actions.js @@ -134,7 +134,7 @@ export const saveNavigationPost = serializeProcessing( function* ( post ) { ? sprintf( /* translators: %s: The text of an error message (potentially untranslated). */ __( "Unable to save: '%s'" ), - saveError + saveError.message ) : __( 'Unable to save: An error ocurred.' ); yield dispatch( noticesStore, 'createErrorNotice', errorMessage, { diff --git a/packages/edit-navigation/src/store/test/actions.js b/packages/edit-navigation/src/store/test/actions.js index 9f24671057fea..d3e1bbf4a8ab5 100644 --- a/packages/edit-navigation/src/store/test/actions.js +++ b/packages/edit-navigation/src/store/test/actions.js @@ -12,6 +12,7 @@ import { resolveMenuItems, getMenuItemToClientIdMapping, dispatch, + select, apiFetch, } from '../controls'; import { menuItemsQuery, computeCustomizedAttribute } from '../utils'; @@ -24,7 +25,7 @@ jest.mock( '../utils', () => { } ); describe( 'createMissingMenuItems', () => { - it( 'create missing menu for navigation block', () => { + it( 'creates a missing menu for navigation block', () => { const post = { id: 'navigation-post-1', slug: 'navigation-post-1', @@ -100,7 +101,7 @@ describe( 'createMissingMenuItems', () => { expect( action.next( [] ).done ).toBe( true ); } ); - it( 'create missing menu for navigation link block', () => { + it( 'creates a missing menu for navigation link block', () => { const post = { id: 'navigation-post-1', slug: 'navigation-post-1', @@ -227,7 +228,7 @@ describe( 'createMissingMenuItems', () => { } ); describe( 'saveNavigationPost', () => { - it( 'should convert all the blocks into menu items and batch save them at once', () => { + it( 'converts all the blocks into menu items and batch save them at once', () => { const post = { id: 'navigation-post-1', slug: 'navigation-post-1', @@ -326,8 +327,11 @@ describe( 'saveNavigationPost', () => { expect( action.next( mapping ).value ).toEqual( dispatch( 'core', 'saveEditedEntityRecord', 'root', 'menu', 1 ) ); - expect( action.next( { id: 1 } ).value ).toEqual( + select( 'core', 'getLastEntitySaveError', 'root', 'menu', 1 ) + ); + + expect( action.next().value ).toEqual( apiFetch( { path: '/__experimental/customizer-nonces/get-save-nonce', } ) @@ -361,7 +365,7 @@ describe( 'saveNavigationPost', () => { ); } ); - it( 'should handle error and show error notifications', () => { + it( 'handles an error from the batch API and show error notifications', () => { const post = { id: 'navigation-post-1', slug: 'navigation-post-1', @@ -461,6 +465,10 @@ describe( 'saveNavigationPost', () => { dispatch( 'core', 'saveEditedEntityRecord', 'root', 'menu', 1 ) ); + expect( action.next( { id: 1 } ).value ).toEqual( + select( 'core', 'getLastEntitySaveError', 'root', 'menu', 1 ) + ); + expect( action.next().value ).toEqual( apiFetch( { path: '/__experimental/customizer-nonces/get-save-nonce', @@ -483,11 +491,130 @@ describe( 'saveNavigationPost', () => { ) ); - expect( action.next( { success: false } ).value ).toEqual( + expect( + action.next( { success: false, data: { message: 'Test Message' } } ) + .value + ).toEqual( + dispatch( + noticesStore, + 'createErrorNotice', + __( "Unable to save: 'Test Message'" ), + { + type: 'snackbar', + } + ) + ); + } ); + + it( 'handles an error from the entity and show error notifications', () => { + const post = { + id: 'navigation-post-1', + slug: 'navigation-post-1', + type: 'page', + meta: { + menuId: 1, + }, + blocks: [ + { + attributes: { showSubmenuIcon: true }, + clientId: 'navigation-block-client-id', + innerBlocks: [ + { + attributes: { + label: 'wp.org', + opensInNewTab: false, + url: 'http://wp.org', + className: '', + rel: '', + description: '', + title: '', + }, + clientId: 'navigation-link-block-client-id-1', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + { + attributes: { + label: 'wp.com', + opensInNewTab: false, + url: 'http://wp.com', + className: '', + rel: '', + description: '', + title: '', + }, + clientId: 'navigation-link-block-client-id-2', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + ], + isValid: true, + name: 'core/navigation', + }, + ], + }; + + const menuItems = [ + { + id: 100, + title: { + raw: 'wp.com', + rendered: 'wp.com', + }, + url: 'http://wp.com', + menu_order: 1, + menus: [ 1 ], + classes: [], + xfn: [], + description: '', + attr_title: '', + }, + { + id: 101, + title: { + raw: 'wp.org', + rendered: 'wp.org', + }, + url: 'http://wp.org', + menu_order: 2, + menus: [ 1 ], + classes: [], + xfn: [], + description: '', + attr_title: '', + }, + ]; + + const mapping = { + 100: 'navigation-link-block-client-id-1', + 101: 'navigation-link-block-client-id-2', + }; + + const action = saveNavigationPost( post ); + + expect( action.next().value ).toEqual( + resolveMenuItems( post.meta.menuId ) + ); + + expect( action.next( menuItems ).value ).toEqual( + getMenuItemToClientIdMapping( post.id ) + ); + + expect( action.next( mapping ).value ).toEqual( + dispatch( 'core', 'saveEditedEntityRecord', 'root', 'menu', 1 ) + ); + + expect( action.next().value ).toEqual( + select( 'core', 'getLastEntitySaveError', 'root', 'menu', 1 ) + ); + + expect( action.next( { message: 'Test Message 2' } ).value ).toEqual( dispatch( noticesStore, 'createErrorNotice', - __( 'There was an error.' ), + __( "Unable to save: 'Test Message 2'" ), { type: 'snackbar', }