From bfee37a5ae6ea314834019eb2044aef23892294c Mon Sep 17 00:00:00 2001 From: epiqueras Date: Fri, 9 Aug 2019 14:26:04 -0400 Subject: [PATCH] Core Data: Handle more change detection and saving flows. --- .../src/components/block-list/block.js | 2 + packages/core-data/src/actions.js | 54 +++++++++++++------ packages/core-data/src/selectors.js | 7 ++- .../e2e-tests/specs/change-detection.test.js | 7 ++- packages/editor/src/store/actions.js | 13 ++++- packages/editor/src/store/selectors.js | 2 +- packages/editor/src/store/test/actions.js | 14 ++++- 7 files changed, 74 insertions(+), 25 deletions(-) diff --git a/packages/block-editor/src/components/block-list/block.js b/packages/block-editor/src/components/block-list/block.js index d95ce1f3a93813..2f719f3f79ad92 100644 --- a/packages/block-editor/src/components/block-list/block.js +++ b/packages/block-editor/src/components/block-list/block.js @@ -702,6 +702,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => { replaceBlocks, toggleSelection, setNavigationMode, + __unstableMarkLastChangeAsPersistent, } = dispatch( 'core/block-editor' ); return { @@ -755,6 +756,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => { } }, onReplace( blocks, indexToSelect ) { + __unstableMarkLastChangeAsPersistent(); replaceBlocks( [ ownProps.clientId ], blocks, indexToSelect ); }, onShiftSelection() { diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index 44bd397e616480..b50e7cbfa9c7c3 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -225,7 +225,11 @@ export function* saveEntityRecord( kind, name, record, - { isAutosave = false, getNoticeActionArgs } = { isAutosave: false } + { + isAutosave = false, + getSuccessNoticeActionArgs, + getFailureNoticeActionArgs, + } = { isAutosave: false } ) { const entities = yield getKindEntities( kind ); const entity = find( entities, { kind, name } ); @@ -236,14 +240,14 @@ export function* saveEntityRecord( const recordId = record[ entityIdKey ]; yield { type: 'SAVE_ENTITY_RECORD_START', kind, name, recordId, isAutosave }; + let persistedRecord; + if ( isAutosave || getSuccessNoticeActionArgs || getFailureNoticeActionArgs ) { + persistedRecord = yield select( 'getEntityRecord', kind, name, recordId ); + } let error; try { const path = `${ entity.baseURL }${ recordId ? '/' + recordId : '' }`; - let persistedRecord; let updatedRecord; - if ( isAutosave || getNoticeActionArgs ) { - persistedRecord = yield select( 'getEntityRecord', kind, name, recordId ); - } if ( isAutosave ) { const currentUser = yield select( 'getCurrentUser' ); @@ -270,7 +274,20 @@ export function* saveEntityRecord( method: 'POST', data, } ); - yield receiveAutosaves( persistedRecord.id, updatedRecord ); + // An autosave may be processed by the server as a regular save + // when its update is requested by the author and the post had + // draft or auto-draft status. + if ( persistedRecord.id === updatedRecord.id ) { + yield receiveEntityRecords( + kind, + name, + { ...persistedRecord, ...updatedRecord }, + undefined, + true + ); + } else { + yield receiveAutosaves( persistedRecord.id, updatedRecord ); + } } else { updatedRecord = yield apiFetch( { path, @@ -280,21 +297,28 @@ export function* saveEntityRecord( yield receiveEntityRecords( kind, name, updatedRecord, undefined, true ); } - if ( getNoticeActionArgs ) { + if ( getSuccessNoticeActionArgs ) { const postType = updatedRecord.type || persistedRecord.type; - const args = postType && getNoticeActionArgs( - persistedRecord, - updatedRecord, - yield select( 'getPostType', postType ) - ); - if ( args && args.length ) { - yield dispatch( - ...args + const args = + postType && + getSuccessNoticeActionArgs( + persistedRecord, + updatedRecord, + yield select( 'getPostType', postType ) ); + if ( args && args.length ) { + yield dispatch( ...args ); } } } catch ( _error ) { error = _error; + + if ( getFailureNoticeActionArgs ) { + const args = getFailureNoticeActionArgs( persistedRecord, record, error ); + if ( args && args.length ) { + yield dispatch( ...args ); + } + } } yield { type: 'SAVE_ENTITY_RECORD_FINISH', diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index ee689443510606..b5ca3250534d73 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -243,12 +243,11 @@ export function isAutosavingEntityRecord( state, kind, name, recordId ) { * @return {boolean} Whether the entity record is saving or not. */ export function isSavingEntityRecord( state, kind, name, recordId ) { - const { pending, isAutosave } = get( + return get( state.entities.data, - [ kind, name, 'saving', recordId ], - {} + [ kind, name, 'saving', recordId, 'pending' ], + false ); - return Boolean( pending && ! isAutosave ); } /** diff --git a/packages/e2e-tests/specs/change-detection.test.js b/packages/e2e-tests/specs/change-detection.test.js index 30895ce5590dea..8900df4b3ea0df 100644 --- a/packages/e2e-tests/specs/change-detection.test.js +++ b/packages/e2e-tests/specs/change-detection.test.js @@ -151,6 +151,7 @@ describe( 'Change detection', () => { it( 'Should prompt if content added without save', async () => { await clickBlockAppender(); + await page.keyboard.type( 'Paragraph' ); await assertIsDirty( true ); } ); @@ -223,9 +224,9 @@ describe( 'Change detection', () => { // Keyboard shortcut Ctrl+S save. await pressKeyWithModifier( 'primary', 'S' ); - await releaseSaveIntercept(); - await assertIsDirty( true ); + + await releaseSaveIntercept(); } ); it( 'Should prompt if changes made while save is in-flight', async () => { @@ -240,6 +241,7 @@ describe( 'Change detection', () => { await pressKeyWithModifier( 'primary', 'S' ); await page.type( '.editor-post-title__input', '!' ); + await page.waitForSelector( '.editor-post-save-draft' ); await releaseSaveIntercept(); @@ -279,6 +281,7 @@ describe( 'Change detection', () => { await pressKeyWithModifier( 'primary', 'S' ); await clickBlockAppender(); + await page.keyboard.type( 'Paragraph' ); // Allow save to complete. Disabling interception flushes pending. await Promise.all( [ diff --git a/packages/editor/src/store/actions.js b/packages/editor/src/store/actions.js index 95e4bf6c7aeb06..6c7a0be829e6ce 100644 --- a/packages/editor/src/store/actions.js +++ b/packages/editor/src/store/actions.js @@ -29,6 +29,7 @@ import { } from './constants'; import { getNotificationArgumentsForSaveSuccess, + getNotificationArgumentsForSaveFail, getNotificationArgumentsForTrashFail, } from './utils/notice-builder'; import { awaitNextStateChange, getRegistry } from './controls'; @@ -397,7 +398,7 @@ export function* savePost( options = {} ) { postId, { ...options, - getNoticeActionArgs: ( previousEntity, entity, type ) => { + getSuccessNoticeActionArgs: ( previousEntity, entity, type ) => { const args = getNotificationArgumentsForSaveSuccess( { previousPost: previousEntity, post: entity, @@ -408,6 +409,16 @@ export function* savePost( options = {} ) { return [ 'core/notices', 'createSuccessNotice', ...args ]; } }, + getFailureNoticeActionArgs: ( previousEntity, edits, error ) => { + const args = getNotificationArgumentsForSaveFail( { + post: previousEntity, + edits, + error, + } ); + if ( args && args.length ) { + return [ 'core/notices', 'createErrorNotice', ...args ]; + } + }, } ); yield __experimentalRequestPostUpdateFinish( options ); diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 003bd720861b99..10c2a0405ed6e8 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -713,7 +713,7 @@ export function isAutosavingPost( state ) { if ( ! isSavingPost( state ) ) { return false; } - return !! state.saving.options.isAutosave; + return !! get( state.saving, [ 'options', 'isAutosave' ] ); } /** diff --git a/packages/editor/src/store/test/actions.js b/packages/editor/src/store/test/actions.js index e6ebd5049aa9c8..f2940a0937c5d7 100644 --- a/packages/editor/src/store/test/actions.js +++ b/packages/editor/src/store/test/actions.js @@ -142,7 +142,11 @@ describe( 'Post generator actions', () => { () => true, () => { const { value } = fulfillment.next( currentPost().id ); - value.args[ 3 ] = { ...value.args[ 3 ], getNoticeActionArgs: 'getNoticeActionArgs' }; + value.args[ 3 ] = { + ...value.args[ 3 ], + getSuccessNoticeActionArgs: 'getSuccessNoticeActionArgs', + getFailureNoticeActionArgs: 'getFailureNoticeActionArgs', + }; expect( value ).toEqual( dispatch( 'core', @@ -150,7 +154,13 @@ describe( 'Post generator actions', () => { 'postType', currentPost().type, currentPost().id, - { isAutosave, getNoticeActionArgs: 'getNoticeActionArgs' } + { + isAutosave, + getSuccessNoticeActionArgs: + 'getSuccessNoticeActionArgs', + getFailureNoticeActionArgs: + 'getFailureNoticeActionArgs', + } ) ); },