Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autosave drafts after delay #1945

Merged
merged 3 commits into from
Jul 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions editor/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,29 @@ export function mergeBlocks( blockA, blockB ) {
};
}

/**
* Returns an action object used in signalling that the post should autosave.
*
* @return {Object} Action object
*/
export function autosave() {
return {
type: 'AUTOSAVE',
};
}

/**
* Returns an action object used in signalling that the post should be queued
* for autosave after a delay.
*
* @return {Object} Action object
*/
export function queueAutosave() {
return {
type: 'QUEUE_AUTOSAVE',
};
}

/**
* Returns an action object used in signalling that the blocks
* corresponding to the specified UID set are to be removed.
Expand Down
51 changes: 49 additions & 2 deletions editor/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import { BEGIN, COMMIT, REVERT } from 'redux-optimist';
import { get, uniqueId } from 'lodash';
import { get, uniqueId, debounce } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -14,12 +14,25 @@ import { __ } from 'i18n';
* Internal dependencies
*/
import { getGutenbergURL, getWPAdminURL } from './utils/url';
import { focusBlock, replaceBlocks, createSuccessNotice, createErrorNotice } from './actions';
import {
focusBlock,
replaceBlocks,
createSuccessNotice,
createErrorNotice,
autosave,
queueAutosave,
savePost,
editPost,
} from './actions';
import {
getCurrentPost,
getCurrentPostType,
getBlocks,
getPostEdits,
isCurrentPostPublished,
isEditedPostDirty,
isEditedPostNew,
isEditedPostSaveable,
} from './selectors';

export default {
Expand Down Expand Up @@ -207,4 +220,38 @@ export default {
]
) );
},
AUTOSAVE( action, store ) {
const { getState, dispatch } = store;
const state = getState();
if ( ! isEditedPostSaveable( state ) || ! isEditedPostDirty( state ) ) {
return;
}

if ( isCurrentPostPublished( state ) ) {
// TODO: Publish autosave.
// - Autosaves are created as revisions for published posts, but
// the necessary REST API behavior does not yet exist
// - May need to check for whether the status of the edited post
// has changed from the saved copy (i.e. published -> pending)
return;
}

// Change status from auto-draft to draft
if ( isEditedPostNew( state ) ) {
dispatch( editPost( { status: 'draft' } ) );
}

dispatch( savePost() );
},
QUEUE_AUTOSAVE: debounce( ( action, store ) => {
store.dispatch( autosave() );
}, 10000 ),
UPDATE_BLOCK_ATTRIBUTES: () => queueAutosave(),
INSERT_BLOCKS: () => queueAutosave(),
MOVE_BLOCKS_DOWN: () => queueAutosave(),
MOVE_BLOCKS_UP: () => queueAutosave(),
REPLACE_BLOCKS: () => queueAutosave(),
REMOVE_BLOCKS: () => queueAutosave(),
EDIT_POST: () => queueAutosave(),
MARK_DIRTY: () => queueAutosave(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need all these effects? I suspect dirtiness? Can't we use an internal somewhere instead?

Copy link
Member Author

@aduth aduth Jul 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need all these effects? I suspect dirtiness? Can't we use an internal somewhere instead?

Yeah, all the potentially dirty-ing effects, see also: reducer for editor.dirty.

As remarked in #1967, I'd like to look to reimplementing dirtiness as a diff against the original saved post, in which case we could reimplement this as a subscribe on the store (i.e. if ( currentDirty !== lastDirty ) queueAutosave()).

};
4 changes: 2 additions & 2 deletions editor/header/tools/publish-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { Button } from 'components';
import { editPost, savePost } from '../../actions';
import {
isSavingPost,
isEditedPostPublished,
isCurrentPostPublished,
isEditedPostBeingScheduled,
getEditedPostVisibility,
isEditedPostSaveable,
Expand Down Expand Up @@ -75,7 +75,7 @@ function PublishButton( {
export default connect(
( state ) => ( {
isSaving: isSavingPost( state ),
isPublished: isEditedPostPublished( state ),
isPublished: isCurrentPostPublished( state ),
isBeingScheduled: isEditedPostBeingScheduled( state ),
visibility: getEditedPostVisibility( state ),
isSaveable: isEditedPostSaveable( state ),
Expand Down
4 changes: 2 additions & 2 deletions editor/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,12 @@ export function getEditedPostVisibility( state ) {
}

/**
* Return true if the post being edited has already been published.
* Return true if the current post has already been published.
*
* @param {Object} state Global application state
* @return {Boolean} Whether the post has been published
*/
export function isEditedPostPublished( state ) {
export function isCurrentPostPublished( state ) {
const post = getCurrentPost( state );

return [ 'publish', 'private' ].indexOf( post.status ) !== -1 ||
Expand Down
4 changes: 2 additions & 2 deletions editor/sidebar/post-status/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import PostSticky from '../post-sticky';
import {
getEditedPostAttribute,
getSuggestedPostFormat,
isEditedPostPublished,
isCurrentPostPublished,
} from '../../selectors';
import { editPost } from '../../actions';

Expand Down Expand Up @@ -74,7 +74,7 @@ export default connect(
( state ) => ( {
status: getEditedPostAttribute( state, 'status' ),
suggestedFormat: getSuggestedPostFormat( state ),
isPublished: isEditedPostPublished( state ),
isPublished: isCurrentPostPublished( state ),
} ),
( dispatch ) => {
return {
Expand Down
66 changes: 65 additions & 1 deletion editor/test/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ import { getBlockTypes, unregisterBlockType, registerBlockType, createBlock } fr
/**
* Internal dependencies
*/
import { mergeBlocks, focusBlock, replaceBlocks } from '../actions';
import { mergeBlocks, focusBlock, replaceBlocks, editPost, savePost } from '../actions';
import effects from '../effects';
import * as selectors from '../selectors';

jest.mock( '../selectors' );

describe( 'effects', () => {
const defaultBlockSettings = { save: noop };

beforeEach( () => jest.resetAllMocks() );

describe( '.MERGE_BLOCKS', () => {
const handler = effects.MERGE_BLOCKS;

Expand Down Expand Up @@ -145,4 +150,63 @@ describe( 'effects', () => {
} ] ) );
} );
} );

describe( '.AUTOSAVE', () => {
const handler = effects.AUTOSAVE;
const dispatch = jest.fn();
const store = { getState: () => {}, dispatch };

it( 'should do nothing for unsaveable', () => {
selectors.isEditedPostSaveable.mockReturnValue( false );
selectors.isEditedPostDirty.mockReturnValue( true );
selectors.isCurrentPostPublished.mockReturnValue( false );
selectors.isEditedPostNew.mockReturnValue( true );

expect( dispatch ).not.toHaveBeenCalled();
} );

it( 'should do nothing for clean', () => {
selectors.isEditedPostSaveable.mockReturnValue( true );
selectors.isEditedPostDirty.mockReturnValue( false );
selectors.isCurrentPostPublished.mockReturnValue( false );
selectors.isEditedPostNew.mockReturnValue( true );

expect( dispatch ).not.toHaveBeenCalled();
} );

it( 'should return autosave action for saveable, dirty, published post', () => {
selectors.isEditedPostSaveable.mockReturnValue( true );
selectors.isEditedPostDirty.mockReturnValue( true );
selectors.isCurrentPostPublished.mockReturnValue( true );
selectors.isEditedPostNew.mockReturnValue( true );

// TODO: Publish autosave
expect( dispatch ).not.toHaveBeenCalled();
} );

it( 'should set auto-draft to draft before save', () => {
selectors.isEditedPostSaveable.mockReturnValue( true );
selectors.isEditedPostDirty.mockReturnValue( true );
selectors.isCurrentPostPublished.mockReturnValue( false );
selectors.isEditedPostNew.mockReturnValue( true );

handler( {}, store );

expect( dispatch ).toHaveBeenCalledTimes( 2 );
expect( dispatch ).toHaveBeenCalledWith( editPost( { status: 'draft' } ) );
expect( dispatch ).toHaveBeenCalledWith( savePost() );
} );

it( 'should return update action for saveable, dirty draft', () => {
selectors.isEditedPostSaveable.mockReturnValue( true );
selectors.isEditedPostDirty.mockReturnValue( true );
selectors.isCurrentPostPublished.mockReturnValue( false );
selectors.isEditedPostNew.mockReturnValue( false );

handler( {}, store );

expect( dispatch ).toHaveBeenCalledTimes( 1 );
expect( dispatch ).toHaveBeenCalledWith( savePost() );
} );
} );
} );
12 changes: 6 additions & 6 deletions editor/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
getDocumentTitle,
getEditedPostExcerpt,
getEditedPostVisibility,
isEditedPostPublished,
isCurrentPostPublished,
isEditedPostPublishable,
isEditedPostSaveable,
isEditedPostBeingScheduled,
Expand Down Expand Up @@ -490,15 +490,15 @@ describe( 'selectors', () => {
} );
} );

describe( 'isEditedPostPublished', () => {
describe( 'isCurrentPostPublished', () => {
it( 'should return true for public posts', () => {
const state = {
currentPost: {
status: 'publish',
},
};

expect( isEditedPostPublished( state ) ).toBe( true );
expect( isCurrentPostPublished( state ) ).toBe( true );
} );

it( 'should return true for private posts', () => {
Expand All @@ -508,7 +508,7 @@ describe( 'selectors', () => {
},
};

expect( isEditedPostPublished( state ) ).toBe( true );
expect( isCurrentPostPublished( state ) ).toBe( true );
} );

it( 'should return false for draft posts', () => {
Expand All @@ -518,7 +518,7 @@ describe( 'selectors', () => {
},
};

expect( isEditedPostPublished( state ) ).toBe( false );
expect( isCurrentPostPublished( state ) ).toBe( false );
} );

it( 'should return true for old scheduled posts', () => {
Expand All @@ -529,7 +529,7 @@ describe( 'selectors', () => {
},
};

expect( isEditedPostPublished( state ) ).toBe( true );
expect( isCurrentPostPublished( state ) ).toBe( true );
} );
} );

Expand Down