Skip to content

Commit

Permalink
State: Partially pick saved fields for autosave
Browse files Browse the repository at this point in the history
Must dirty artificially once save completes if there were edits not included in the payload.
  • Loading branch information
aduth committed Jun 20, 2018
1 parent 5e9e1ca commit 6fa254e
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 9 deletions.
34 changes: 31 additions & 3 deletions editor/store/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
getTemplate,
getTemplateLock,
getAutosave,
isEditedPostNew,
POST_UPDATE_TRANSACTION_ID,
} from './selectors';

Expand Down Expand Up @@ -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 ),
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions editor/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' ?
Expand Down
34 changes: 29 additions & 5 deletions editor/store/test/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
convertBlockToStatic,
convertBlockToShared,
setTemplateValidity,
editPost,
} from '../actions';
import effects, {
removeProvisionalBlock,
Expand Down Expand Up @@ -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: {
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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 };
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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' } ),
] );
} );
} );
Expand Down
24 changes: 23 additions & 1 deletion test/e2e/specs/change-detection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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' );

Expand Down
16 changes: 16 additions & 0 deletions test/e2e/support/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 6fa254e

Please sign in to comment.