From 01520b4726e84c71a6ead4ab1f3e2983919889e7 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 4 Jun 2018 16:10:45 -0400 Subject: [PATCH 01/11] State: Trigger autosave as standard save for draft by current user --- editor/store/effects.js | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/editor/store/effects.js b/editor/store/effects.js index 5d98bb9821a9b1..864e06db5141df 100644 --- a/editor/store/effects.js +++ b/editor/store/effects.js @@ -92,11 +92,34 @@ export function removeProvisionalBlock( action, store ) { } } +/** + * Returns true if a requested autosave should occur as a standard save. This + * is true if the saved post is a draft or auto-draft, and if the request is + * is issued on behalf of the post author. + * + * @param {Object} post Post object + * + * @return {boolean} Whether autosave request should be issued as full save. + */ +function isAutosaveAsDraft( post ) { + // TODO: Improve retrieval of current user ID to not rely on this global. + const uid = Number( window.userSettings.uid ); + + return ( + uid === post.author && + includes( [ 'draft', 'auto-draft' ], post.status ) + ); +} + export default { REQUEST_POST_UPDATE( action, store ) { const { dispatch, getState } = store; const state = getState(); - const isAutosave = action.options && action.options.autosave; + const post = getCurrentPost( state ); + const isAutosave = ( + get( action.options, [ 'autosave' ], false ) && + ! isAutosaveAsDraft( post ) + ); // Prevent save if not saveable. const isSaveable = isAutosave ? isEditedPostAutosaveable : isEditedPostSaveable; @@ -104,7 +127,6 @@ export default { return; } - const post = getCurrentPost( state ); const edits = getPostEdits( state ); let toSend = { ...edits, From d817ec1c8b0c092dfe21ca1176c1c8145526d453 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 4 Jun 2018 17:04:04 -0400 Subject: [PATCH 02/11] Testing: Update E2E tests for expected autosave conditions --- test/e2e/specs/change-detection.test.js | 37 +++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/test/e2e/specs/change-detection.test.js b/test/e2e/specs/change-detection.test.js index 944346629f60be..20a4ed7387eca1 100644 --- a/test/e2e/specs/change-detection.test.js +++ b/test/e2e/specs/change-detection.test.js @@ -76,14 +76,45 @@ describe( 'Change detection', () => { it( 'Should autosave post', async () => { await page.type( '.editor-post-title__input', 'Hello World' ); - // Force autosave to occur immediately. + // Force autosave to occur immediately. It will occur as a normal save. 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-saving' ), page.waitForSelector( '.editor-post-saved-state.is-saved' ), ] ); - // Still dirty after an autosave. + // Autosave draft as same user should do full save, i.e. not dirty. + await assertIsDirty( false ); + } ); + + it( 'Should prompt to confirm unsaved changes for autosaved published post', async () => { + await page.type( '.editor-post-title__input', 'Hello World' ); + + await Promise.all( [ + page.waitForSelector( '.editor-post-publish-button' ), + page.click( '.editor-post-publish-panel__toggle' ), + ] ); + + await Promise.all( [ + page.waitForSelector( '.editor-post-publish-panel__header-published' ), + page.click( '.editor-post-publish-button' ), + ] ); + + // Close publish panel. + await Promise.all( [ + page.waitForFunction( () => ! document.querySelector( '.editor-post-publish-panel' ) ), + page.click( '.editor-post-publish-panel__header button' ), + ] ); + + // Should be dirty after autosave change of published post. + await page.type( '.editor-post-title__input', '!' ); + + await Promise.all( [ + page.waitForSelector( '.editor-post-publish-button.is-busy' ), + page.evaluate( () => window.wp.data.dispatch( 'core/editor' ).autosave() ), + ] ); + await page.waitForSelector( '.editor-post-publish-button:not( .is-busy )' ); + await assertIsDirty( true ); } ); From 08afec7b38a169de490057331f67356d81044f60 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 5 Jun 2018 11:35:12 -0400 Subject: [PATCH 03/11] Testing: Wait for animation to avoid cursor chasing button --- test/e2e/specs/change-detection.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/specs/change-detection.test.js b/test/e2e/specs/change-detection.test.js index 20a4ed7387eca1..fd7e696244ef42 100644 --- a/test/e2e/specs/change-detection.test.js +++ b/test/e2e/specs/change-detection.test.js @@ -95,6 +95,11 @@ describe( 'Change detection', () => { page.click( '.editor-post-publish-panel__toggle' ), ] ); + // Disable reason: Wait for animation to complete to avoid click + // occuring at wrong point. + // eslint-disable-next-line no-restricted-syntax + await page.waitFor( 100 ); + await Promise.all( [ page.waitForSelector( '.editor-post-publish-panel__header-published' ), page.click( '.editor-post-publish-button' ), @@ -111,9 +116,9 @@ describe( 'Change detection', () => { await Promise.all( [ page.waitForSelector( '.editor-post-publish-button.is-busy' ), + page.waitForSelector( '.editor-post-publish-button:not( .is-busy )' ), page.evaluate( () => window.wp.data.dispatch( 'core/editor' ).autosave() ), ] ); - await page.waitForSelector( '.editor-post-publish-button:not( .is-busy )' ); await assertIsDirty( true ); } ); From da5d327d2ba25ba62fbf07697d70e4fc775901bf Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 5 Jun 2018 11:37:11 -0400 Subject: [PATCH 04/11] State: Distinguish save on effective autosave Change detection reset for known-to-be-draft, pick from non-effective autosave for resetting post --- editor/store/effects.js | 52 ++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/editor/store/effects.js b/editor/store/effects.js index 864e06db5141df..409784ec1ae661 100644 --- a/editor/store/effects.js +++ b/editor/store/effects.js @@ -116,10 +116,7 @@ export default { const { dispatch, getState } = store; const state = getState(); const post = getCurrentPost( state ); - const isAutosave = ( - get( action.options, [ 'autosave' ], false ) && - ! isAutosaveAsDraft( post ) - ); + const isAutosave = get( action.options, [ 'autosave' ], false ); // Prevent save if not saveable. const isSaveable = isAutosave ? isEditedPostAutosaveable : isEditedPostSaveable; @@ -158,12 +155,6 @@ export default { data: toSend, } ); } else { - dispatch( { - type: 'UPDATE_POST', - edits: toSend, - optimist: { id: POST_UPDATE_TRANSACTION_ID }, - } ); - dispatch( removeNotice( SAVE_POST_NOTICE_ID ) ); dispatch( removeNotice( AUTOSAVE_POST_NOTICE_ID ) ); @@ -174,9 +165,48 @@ export default { } ); } + // An autosave may be processed by the server as an update to the post, + // in which case it should be treated the same as a regular save so far + // as updating canonical post values and resetting change detection. + const isEffectiveAutosave = isAutosave && ! isAutosaveAsDraft( post ); + if ( ! isEffectiveAutosave ) { + dispatch( { + type: 'UPDATE_POST', + edits: toSend, + optimist: { id: POST_UPDATE_TRANSACTION_ID }, + } ); + } + request.then( ( newPost ) => { - const reset = isAutosave ? resetAutosave : resetPost; + const reset = isEffectiveAutosave ? resetAutosave : resetPost; + + // Autosave response value shape is not the same as the post + // endpoint, so even if the server processes it as a regular + // save, it can't be received into state directly. Instead, + // merge to known saved post with updated common keys. + if ( isAutosave && ! isEffectiveAutosave ) { + newPost = { + ...post, + ...pick( newPost, [ + // Autosave content fields. + 'title', + 'content', + 'excerpt', + + // UI considers save to have occurred if modified + // date of post changes (e.g. PostPreviewButton). + // + // TODO: Consider more formalized pattern for + // identifying a save as having completed. + 'date', + 'date_gmt', + 'modified', + 'modified_gmt', + ] ), + }; + } + dispatch( reset( newPost ) ); dispatch( { From 8f82de82869f8a1be2fbddfdebf53677cf3f6093 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 5 Jun 2018 11:51:55 -0400 Subject: [PATCH 05/11] Testing: Ensure autosave is identified as autosave Passed previously because when autosaving, class is assigned with _both_ `is-saving` and `is-autosaving`. The superset of classes is what what should be expected. --- test/e2e/specs/change-detection.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/specs/change-detection.test.js b/test/e2e/specs/change-detection.test.js index fd7e696244ef42..2b04769cb81c44 100644 --- a/test/e2e/specs/change-detection.test.js +++ b/test/e2e/specs/change-detection.test.js @@ -76,10 +76,10 @@ describe( 'Change detection', () => { it( 'Should autosave post', async () => { await page.type( '.editor-post-title__input', 'Hello World' ); - // Force autosave to occur immediately. It will occur as a normal save. + // Force autosave to occur immediately. await Promise.all( [ page.evaluate( () => window.wp.data.dispatch( 'core/editor' ).autosave() ), - page.waitForSelector( '.editor-post-saved-state.is-saving' ), + page.waitForSelector( '.editor-post-saved-state.is-autosaving' ), page.waitForSelector( '.editor-post-saved-state.is-saved' ), ] ); From df6f471c69470beb26b7db66183f43cd0291c674 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 5 Jun 2018 15:39:43 -0400 Subject: [PATCH 06/11] State: Update post in autosave as assumed full save --- editor/store/actions.js | 15 ++++++ editor/store/effects.js | 116 ++++++++++++++++++++-------------------- editor/store/reducer.js | 6 +-- 3 files changed, 75 insertions(+), 62 deletions(-) diff --git a/editor/store/actions.js b/editor/store/actions.js index 65619fb8b54adc..272438254f5c76 100644 --- a/editor/store/actions.js +++ b/editor/store/actions.js @@ -59,6 +59,21 @@ export function resetAutosave( post ) { }; } +/** + * Returns an action object used in signalling that a patch of updates for the + * latest version of the post have been received. + * + * @param {Object} edits Updated post fields. + * + * @return {Object} Action object. + */ +export function updatePost( edits ) { + return { + type: 'UPDATE_POST', + edits, + }; +} + /** * Returns an action object used to setup the editor state when first opening an editor. * diff --git a/editor/store/effects.js b/editor/store/effects.js index 409784ec1ae661..833f818a56fdbf 100644 --- a/editor/store/effects.js +++ b/editor/store/effects.js @@ -29,6 +29,7 @@ import { setupEditorState, resetAutosave, resetPost, + updatePost, receiveBlocks, receiveSharedBlocks, replaceBlock, @@ -92,25 +93,6 @@ export function removeProvisionalBlock( action, store ) { } } -/** - * Returns true if a requested autosave should occur as a standard save. This - * is true if the saved post is a draft or auto-draft, and if the request is - * is issued on behalf of the post author. - * - * @param {Object} post Post object - * - * @return {boolean} Whether autosave request should be issued as full save. - */ -function isAutosaveAsDraft( post ) { - // TODO: Improve retrieval of current user ID to not rely on this global. - const uid = Number( window.userSettings.uid ); - - return ( - uid === post.author && - includes( [ 'draft', 'auto-draft' ], post.status ) - ); -} - export default { REQUEST_POST_UPDATE( action, store ) { const { dispatch, getState } = store; @@ -138,6 +120,14 @@ export default { isAutosave, } ); + // Optimistically apply updates under the assumption that the post + // will be updated. See below logic in success resolution for revert + // if the autosave is applied as a revision. + dispatch( { + ...updatePost( toSend ), + optimist: { id: POST_UPDATE_TRANSACTION_ID }, + } ); + let request; if ( isAutosave ) { // Ensure autosaves contain all expected fields, using autosave or @@ -165,55 +155,63 @@ export default { } ); } - // An autosave may be processed by the server as an update to the post, - // in which case it should be treated the same as a regular save so far - // as updating canonical post values and resetting change detection. - const isEffectiveAutosave = isAutosave && ! isAutosaveAsDraft( post ); - if ( ! isEffectiveAutosave ) { - dispatch( { - type: 'UPDATE_POST', - edits: toSend, - optimist: { id: POST_UPDATE_TRANSACTION_ID }, - } ); - } - request.then( ( newPost ) => { - const reset = isEffectiveAutosave ? resetAutosave : resetPost; - - // Autosave response value shape is not the same as the post - // endpoint, so even if the server processes it as a regular - // save, it can't be received into state directly. Instead, - // merge to known saved post with updated common keys. - if ( isAutosave && ! isEffectiveAutosave ) { - newPost = { - ...post, - ...pick( newPost, [ - // Autosave content fields. - 'title', - 'content', - 'excerpt', - - // UI considers save to have occurred if modified - // date of post changes (e.g. PostPreviewButton). - // - // TODO: Consider more formalized pattern for - // identifying a save as having completed. - 'date', - 'date_gmt', - 'modified', - 'modified_gmt', - ] ), - }; + // An autosave may be processed by the server as a regular save + // when its update is requested by the author and the post was + // draft or auto-draft. + const isRevision = newPost.id !== post.id; + + // Thus, the following behaviors occur: + // + // - If it was a revision, it is treated as latest autosave + // and updates optimistically applied are reverted. + // - If it was an autosave but not revision under the above + // noted conditions, cherry-pick updated properties since + // the received revision entity shares some but not all + // properties of a post. + // - Otherwise, it was a full save and the received entity + // can be considered the new reset post. + let updateAction; + if ( isRevision ) { + updateAction = resetAutosave( newPost ); + } else if ( isAutosave ) { + const revisionEdits = pick( newPost, [ + // Autosave content fields. + 'title', + 'content', + 'excerpt', + + // UI considers save to have occurred if modified date + // of post changes (e.g. PostPreviewButton). + // + // TODO: Consider formalized pattern for identifying a + // save as having completed. + 'date', + 'date_gmt', + 'modified', + 'modified_gmt', + ] ); + + updateAction = updatePost( revisionEdits ); + } else { + updateAction = resetPost( newPost ); } - dispatch( reset( newPost ) ); + dispatch( updateAction ); dispatch( { type: 'REQUEST_POST_UPDATE_SUCCESS', previousPost: post, post: newPost, - optimist: { type: COMMIT, id: POST_UPDATE_TRANSACTION_ID }, + optimist: { + // Note: REVERT is not a failure case here. Rather, it + // is simply reversing the assumption that the updates + // were applied to the post proper, such that the post + // treated as having unsaved changes. + type: isRevision ? REVERT : COMMIT, + id: POST_UPDATE_TRANSACTION_ID, + }, isAutosave, } ); }, diff --git a/editor/store/reducer.js b/editor/store/reducer.js index ec3558e5501c79..1884a72ef5c443 100644 --- a/editor/store/reducer.js +++ b/editor/store/reducer.js @@ -217,15 +217,15 @@ export const editor = flow( [ // Track undo history, starting at editor initialization. withHistory( { resetTypes: [ 'SETUP_EDITOR_STATE' ], - ignoreTypes: [ 'RECEIVE_BLOCKS' ], + ignoreTypes: [ 'RECEIVE_BLOCKS', 'RESET_POST', 'UPDATE_POST' ], shouldOverwriteState, } ), // Track whether changes exist, resetting at each post save. Relies on // editor initialization firing post reset as an effect. withChangeDetection( { - resetTypes: [ 'SETUP_EDITOR_STATE', 'UPDATE_POST' ], - ignoreTypes: [ 'RECEIVE_BLOCKS', 'RESET_POST' ], + resetTypes: [ 'SETUP_EDITOR_STATE', 'REQUEST_POST_UPDATE_START' ], + ignoreTypes: [ 'RECEIVE_BLOCKS', 'RESET_POST', 'UPDATE_POST' ], } ), ] )( { edits( state = {}, action ) { From 5e9e1cac4a0b2eabf08c87af7b08f906ea4b60b8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 11 Jun 2018 11:43:30 -0400 Subject: [PATCH 07/11] Preview: Enable published posts preview (#7189) * REST API: Move preview_link to autosaves controller * Post Preview: Redirect to preview by autosave property availability * Preview Button: Use preview link as href if available * Preview Button: Move window bind to after write Was not reliably triggered when redirect occurs by setPreviewWindowLink, possibly because document.open triggered by document.write erased window handlers. * Preview Button: Always trigger autosave when dirty * Preview Button: Consider preview ready by change in value * State: Set default value for savePost options Allows use in handlers to assume options present * Store: Clarify preview link with query parameter * REST API: Set preview link by parent only if autosave * Store: Invalid autosave preview link on save intent Preview link is no longer valid until autosave completes, by which UI can infer availability of preview as being ready to view. * Testing: Add preview E2E tests * Testing: Fix copypasta * Preview: Use correct location property for URL comparison --- .../components/post-preview-button/index.js | 59 ++++--- .../post-preview-button/test/index.js | 68 ++++++-- editor/store/actions.js | 2 +- editor/store/effects.js | 43 +---- editor/store/reducer.js | 14 +- editor/store/selectors.js | 43 ++++- editor/store/test/actions.js | 8 + editor/store/test/reducer.js | 2 + editor/store/test/selectors.js | 79 +++++++++ lib/class-wp-rest-autosaves-controller.php | 30 +++- lib/rest-api.php | 39 ----- phpunit/class-gutenberg-rest-api-test.php | 15 -- .../class-rest-autosaves-controller-test.php | 3 +- test/e2e/specs/preview.test.js | 158 ++++++++++++++++++ test/e2e/specs/publishing.test.js | 14 +- test/e2e/support/utils.js | 24 ++- 16 files changed, 448 insertions(+), 153 deletions(-) create mode 100644 test/e2e/specs/preview.test.js diff --git a/editor/components/post-preview-button/index.js b/editor/components/post-preview-button/index.js index acb2ae41a3fac2..f9037ae64f8108 100644 --- a/editor/components/post-preview-button/index.js +++ b/editor/components/post-preview-button/index.js @@ -17,26 +17,34 @@ export class PostPreviewButton extends Component { super( ...arguments ); this.saveForPreview = this.saveForPreview.bind( this ); - - this.state = { - isAwaitingSave: false, - }; } - componentWillReceiveProps( nextProps ) { - const { modified, link } = nextProps; - const { isAwaitingSave } = this.state; - const hasFinishedSaving = ( - isAwaitingSave && - modified !== this.props.modified - ); + componentDidUpdate( prevProps ) { + const { previewLink } = this.props; - if ( hasFinishedSaving && this.previewWindow ) { - this.previewWindow.location = link; - this.setState( { isAwaitingSave: false } ); + // This relies on the window being responsible to unset itself when + // navigation occurs or a new preview window is opened, to avoid + // unintentional forceful redirects. + if ( previewLink && previewLink !== prevProps.previewLink ) { + this.setPreviewWindowLink( previewLink ); } } + /** + * Sets the preview window's location to the given URL, if a preview window + * exists and is not already at that location. + * + * @param {string} url URL to assign as preview window location. + */ + setPreviewWindowLink( url ) { + const { previewWindow } = this; + if ( ! previewWindow || previewWindow.location.href === url ) { + return; + } + + previewWindow.location = url; + } + getWindowTarget() { const { postId } = this.props; return `wp-preview-${ postId }`; @@ -44,6 +52,7 @@ export class PostPreviewButton extends Component { saveForPreview( event ) { const { isDirty, isNew } = this.props; + // Let default link behavior occur if no changes to saved post if ( ! isDirty && ! isNew ) { return; @@ -51,9 +60,6 @@ export class PostPreviewButton extends Component { // Save post prior to opening window this.props.autosave(); - this.setState( { - isAwaitingSave: true, - } ); // Open a popup, BUT: Set it to a blank page until save completes. This // is necessary because popups can only be opened in response to user @@ -64,10 +70,6 @@ export class PostPreviewButton extends Component { this.getWindowTarget() ); - // When popup is closed, delete reference to avoid later assignment of - // location in a post update. - this.previewWindow.onbeforeunload = () => delete this.previewWindow; - const markup = `

Please wait…

@@ -93,16 +95,20 @@ export class PostPreviewButton extends Component { this.previewWindow.document.write( markup ); this.previewWindow.document.close(); + + // When popup is closed or redirected by setPreviewWindowLink, delete + // reference to avoid later assignment of location in a post update. + this.previewWindow.onbeforeunload = () => delete this.previewWindow; } render() { - const { link, isSaveable } = this.props; + const { currentPostLink, isSaveable } = this.props; return (