From 6fa254e87f13859a1eccfe19851a0d221c47fea2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 20 Jun 2018 10:43:11 -0400 Subject: [PATCH] State: Partially pick saved fields for autosave Must dirty artificially once save completes if there were edits not included in the payload. --- editor/store/effects.js | 34 ++++++++++++++++++++++--- editor/store/reducer.js | 3 +++ editor/store/test/effects.js | 34 +++++++++++++++++++++---- test/e2e/specs/change-detection.test.js | 24 ++++++++++++++++- test/e2e/support/utils.js | 16 ++++++++++++ 5 files changed, 102 insertions(+), 9 deletions(-) diff --git a/editor/store/effects.js b/editor/store/effects.js index 41d6148cc7fa8..75da4df7821d2 100644 --- a/editor/store/effects.js +++ b/editor/store/effects.js @@ -65,6 +65,7 @@ import { getTemplate, getTemplateLock, getAutosave, + isEditedPostNew, POST_UPDATE_TRANSACTION_ID, } from './selectors'; @@ -106,7 +107,26 @@ export default { return; } - const edits = getPostEdits( state ); + let edits = getPostEdits( state ); + if ( isAutosave ) { + edits = pick( edits, [ 'title', 'content', 'excerpt' ] ); + } + + // New posts (with auto-draft status) must be explicitly assigned draft + // status if there is not already a status assigned in edits (publish). + // Otherwise, they are wrongly left as auto-draft. Status is not always + // respected for autosaves, so it cannot simply be included in the pick + // above. This behavior relies on an assumption that an auto-draft post + // would never be saved by anyone other than the owner of the post, per + // logic within autosaves REST controller to save status field only for + // draft/auto-draft by current user. + // + // See: https://core.trac.wordpress.org/ticket/43316#comment:88 + // See: https://core.trac.wordpress.org/ticket/43316#comment:89 + if ( isEditedPostNew( state ) ) { + edits = { status: 'draft', ...edits }; + } + let toSend = { ...edits, content: getEditedPostContent( state ), @@ -198,7 +218,16 @@ export default { }, REQUEST_POST_UPDATE_SUCCESS( action, store ) { const { previousPost, post, isAutosave } = action; - const { dispatch } = store; + const { dispatch, getState } = store; + + // TEMPORARY: If edits remain after a save completes, the user must be + // prompted about unsaved changes. This should be refactored as part of + // the `isEditedPostDirty` selector instead. + // + // See: https://github.com/WordPress/gutenberg/issues/7409 + if ( Object.keys( getPostEdits( getState() ) ).length ) { + dispatch( { type: 'DIRTY_ARTIFICIALLY' } ); + } // Autosaves are neither shown a notice nor redirected. if ( isAutosave ) { @@ -390,7 +419,6 @@ export default { const edits = {}; if ( post.status === 'auto-draft' ) { edits.title = post.title.raw; - edits.status = 'draft'; } // Check the auto-save status diff --git a/editor/store/reducer.js b/editor/store/reducer.js index f0a960321a8e3..ee29c384ef13a 100644 --- a/editor/store/reducer.js +++ b/editor/store/reducer.js @@ -254,6 +254,9 @@ export const editor = flow( [ return state; + case 'DIRTY_ARTIFICIALLY': + return { ...state }; + case 'UPDATE_POST': case 'RESET_POST': const getCanonicalValue = action.type === 'UPDATE_POST' ? diff --git a/editor/store/test/effects.js b/editor/store/test/effects.js index b585ba079771b..8e135cb7c566b 100644 --- a/editor/store/test/effects.js +++ b/editor/store/test/effects.js @@ -34,6 +34,7 @@ import { convertBlockToStatic, convertBlockToShared, setTemplateValidity, + editPost, } from '../actions'; import effects, { removeProvisionalBlock, @@ -260,6 +261,16 @@ describe( 'effects', () => { describe( '.REQUEST_POST_UPDATE_SUCCESS', () => { const handler = effects.REQUEST_POST_UPDATE_SUCCESS; + function createGetState( hasLingeringEdits = false ) { + let state = reducer( undefined, {} ); + if ( hasLingeringEdits ) { + state = reducer( state, editPost( { edited: true } ) ); + } + + const getState = () => state; + return getState; + } + const defaultPost = { id: 1, title: { @@ -280,7 +291,7 @@ describe( 'effects', () => { it( 'should dispatch notices when publishing or scheduling a post', () => { const dispatch = jest.fn(); - const store = { dispatch }; + const store = { dispatch, getState: createGetState() }; const previousPost = getDraftPost(); const post = getPublishedPost(); @@ -302,7 +313,7 @@ describe( 'effects', () => { it( 'should dispatch notices when reverting a published post to a draft', () => { const dispatch = jest.fn(); - const store = { dispatch }; + const store = { dispatch, getState: createGetState() }; const previousPost = getPublishedPost(); const post = getDraftPost(); @@ -328,7 +339,7 @@ describe( 'effects', () => { it( 'should dispatch notices when just updating a published post again', () => { const dispatch = jest.fn(); - const store = { dispatch }; + const store = { dispatch, getState: createGetState() }; const previousPost = getPublishedPost(); const post = getPublishedPost(); @@ -350,7 +361,7 @@ describe( 'effects', () => { it( 'should do nothing if the updated post was autosaved', () => { const dispatch = jest.fn(); - const store = { dispatch }; + const store = { dispatch, getState: createGetState() }; const previousPost = getPublishedPost(); const post = { ...getPublishedPost(), id: defaultPost.id + 1 }; @@ -359,6 +370,19 @@ describe( 'effects', () => { expect( dispatch ).toHaveBeenCalledTimes( 0 ); } ); + + it( 'should dispatch dirtying action if edits linger after autosave', () => { + const dispatch = jest.fn(); + const store = { dispatch, getState: createGetState( true ) }; + + const previousPost = getPublishedPost(); + const post = { ...getPublishedPost(), id: defaultPost.id + 1 }; + + handler( { post, previousPost, isAutosave: true }, store ); + + expect( dispatch ).toHaveBeenCalledTimes( 1 ); + expect( dispatch ).toHaveBeenCalledWith( { type: 'DIRTY_ARTIFICIALLY' } ); + } ); } ); describe( '.REQUEST_POST_UPDATE_FAILURE', () => { @@ -531,7 +555,7 @@ describe( 'effects', () => { expect( result ).toEqual( [ setTemplateValidity( true ), - setupEditorState( post, [], { title: 'A History of Pork', status: 'draft' } ), + setupEditorState( post, [], { title: 'A History of Pork' } ), ] ); } ); } ); diff --git a/test/e2e/specs/change-detection.test.js b/test/e2e/specs/change-detection.test.js index 2b04769cb81c4..2ab2bcbc1a416 100644 --- a/test/e2e/specs/change-detection.test.js +++ b/test/e2e/specs/change-detection.test.js @@ -2,7 +2,12 @@ * Internal dependencies */ import '../support/bootstrap'; -import { newPost, newDesktopBrowserPage, pressWithModifier } from '../support/utils'; +import { + newPost, + newDesktopBrowserPage, + pressWithModifier, + ensureSidebarOpened, +} from '../support/utils'; describe( 'Change detection', () => { let handleInterceptedRequest, hadInterceptedSave; @@ -87,6 +92,23 @@ describe( 'Change detection', () => { await assertIsDirty( false ); } ); + it( 'Should prompt to confirm unsaved changes for autosaved draft for non-content fields', async () => { + await page.type( '.editor-post-title__input', 'Hello World' ); + + // Toggle post as sticky (not persisted for autosave). + await ensureSidebarOpened(); + await page.click( '[id^="post-sticky-toggle-"]' ); + + // Force autosave to occur immediately. + await Promise.all( [ + page.evaluate( () => window.wp.data.dispatch( 'core/editor' ).autosave() ), + page.waitForSelector( '.editor-post-saved-state.is-autosaving' ), + page.waitForSelector( '.editor-post-saved-state.is-saved' ), + ] ); + + await assertIsDirty( true ); + } ); + it( 'Should prompt to confirm unsaved changes for autosaved published post', async () => { await page.type( '.editor-post-title__input', 'Hello World' ); diff --git a/test/e2e/support/utils.js b/test/e2e/support/utils.js index 4585a2580758b..40c2f756b708d 100644 --- a/test/e2e/support/utils.js +++ b/test/e2e/support/utils.js @@ -112,6 +112,22 @@ export async function getHTMLFromCodeEditor() { return textEditorContent; } +/** + * Verifies that the edit post sidebar is opened, and if it is not, opens it. + * + * @return {Promise} Promise resolving once the edit post sidebar is opened. + */ +export async function ensureSidebarOpened() { + // This try/catch flow relies on the fact that `page.$eval` throws an error + // if the element matching the given selector does not exist. Thus, if an + // error is thrown, it can be inferred that the sidebar is not opened. + try { + return page.$eval( '.edit-post-sidebar', () => {} ); + } catch ( error ) { + return page.click( '.edit-post-header__settings [aria-label="Settings"]' ); + } +} + /** * Opens the inserter, searches for the given term, then selects the first * result that appears.