Skip to content

Commit

Permalink
Introduce a dedicated autosaves endpoint for handling autosave behavi…
Browse files Browse the repository at this point in the history
…or (#6257)

* autosaves with rest controller, first pass

* use newer title call

* autosave: compare raw attributes for change check

* Clean up autosave effects

* pull isAutosaving from props in PostSavedState

* from .14

* effects, improve toggle autosave timing, enable autosaving for published posts

* Disable ‘save draft’ after draft is autosaved

* revert post saved state changes, keep save button active regardless of autosave status

* remove unrelated showAutosaveAlert

* fixes for eslint

* leverage withChangeDetection for autosaves

* fixes for php slint

* rename WP_REST_Autosaves_Controller  -> GB_REST_Autosaves_Controller to avoid conflict with core class

* Add a docblock for savePost action

* check state.autosave exists before using

* Fix autosave tests

* Revert "leverage withChangeDetection for autosaves"

This reverts commit b189a3c.

# Conflicts:
#	editor/store/selectors.js

* rename rest controler file to match class passing phplint

* set up isAutosaving

* rename controller WP_REST_Autosaves_Controller and only load if class doesn’t exist

* Cleanup doc blocks

* rename revision_controller -> revisions_controller

* clean up isAutosaving reducer, removing toggleAutosave action

* fixes for php-lint

* fix test: button stays in saved state when post not dirty

* restore state check

* Properly instantiate the autosaves controller

* ensure isAutosave returns a boolean

* Add PHP Unit tests covering WP_REST_Autosaves_Controller functionality

* Avoid duplicate route resgistration

* Rename/fix up unit tests for rest controller

* fixes for phpcs

* document autosave as a parameter property for options

* remove redundant check for isEditedPostNew

* rename currentlyAutosaving -> isAutosaving

* move autosave reducer outside editor

* set the default autosave state to null

* remove unneded object check that confused logic

* fix for eslint

* ensure href set before using in gutenberg_add_target_schema_to_links

* include the autosaves controller

* docblock object -> boolean

* document isAutosaving reducer

* document autosave reducer

* skip destructuring post

* eliminate unneeded conditional

* rename autosavable -> autosaveable (with an e)

* Simplify class check

* fixes for phpcs

* savePost on CMD-S

* correctly return autosave from selector

* correct state autosave usage

* fix docblock

* add test: should stop autosave timer when the autosave is up to date

* correct test naming: should stop autosave timer when the autosave is up to date

* Add an e2e test for post autosaving

* whitespace

* respect AUTOSAVE_INTERVAL for autosave timing

* reset saving state on `RESET_AUTOSAVE`

* add mu plugin to lower the autosave interval for testing

* docs spacing

* Add JSDoc for hasAutosave

* eslint: missing trailing comma

* Add tests for the hasAutosave selector

* remove unused isAutosave in REQUEST_POST_UPDATE_SUCCESS

* adjust initial state for autosave test

* only pass autosaveInterval

* Testing: Trigger programmatic autosave

* revert 2012003

* remove unused `className`

* use isAutosaving

* Drop autosave interval back to 10 seconds, because 60 is really slow

* clarify use of isEditedPostSaveable in effects/AUTOSAVE

* Prevent autosaves if an autosave is in progress.

* Add isEditedPostSaveable logic to isPostAutosaveable

* autosave monitor tests - use isAutosaveable

* fix tests for autosave monitor

* Autosave: Cancel scheduled autosave when autosaving starts

* Autosave: Style: Use property shorthand for autosaveInterval assignment

* Revert "revert 2012003"

This reverts commit bd29ca0.

* State: Rename `isPostAutosaveable` to `isEditedPostAutosaveable`

Consistency with `isEditedPostSaveable`. If we want to simplify, we must simplify both.

* State: Implement `isEditedPostAutosaveable` as composing `isAutosavingPost`

In case implementation details of detecting whether an autosave is in progress ever change.

* State: Implement autosave action as action creator

* State: Improve isAutosavingPost selector JSDoc

Newline between param, return. Full sentence stops. Grammatical consistency of "Returns ..." with other selectors.

* State: Revert doAutosave action creator name to autosave

If "save" is a verb, why not is "autosave" ?

* Testing: Avoid potential race conditions on Cmd+S save fail

Not guaranteed that notice will not already have been displayed by time Cmd+S press resolves.

* State: Prevent save request if post is not saveable

e.g. New post without content. Previously handled in `AUTOSAVE` handler since keyboard shortcuts executed `autosave` action dispatch.

* State: Implement autosaving as superset of saving condition

Simultaneous saves and autosaves should not be allowed, since it opens to race conditions where an explicit save _after_ and autosave occurs could be interpreted by the server _before_ the autosave.

* State: Consider save-in-progress as unsaveable

Move UI restriction to state consideration, evaluated as part of savePost effect handler.

* Testing: Evaluate autosave completion by class presence

Also eliminate possible race condition on autosave effecting selector prior to resolved evaluation.

* Keyboard Shortcuts: Disallow non-dirty save requests

UI restriction previously accounted for by fact that `AUTOSAVE` handler had aborted when post not dirty. This will need to be a future iteration on `savePost` effect.

* State: Track autosave object as subset of received properties

Only those used in isEditedPostAutosaveable implementation

* State: Remove redundant condition from autosaveable

Since already checking isEditedPostSaveable, which returns false if save (including autosave) is in progress

* State: Simplify isEditedPostAutosaveable as Array#some of fields

Consolidate to getEditedPostAttribute as canonical source of fields values

* State: Deprecate getEditedPostExcerpt selector

Consolidate to getEditedPostAttribute. Unlike content, there is no special behavior for excerpt. Even if there were, getEditedPostAttribute should be the entry point, which like content, would only then defer its implementation to getEditedPostExcerpt.

* Remove tests originally removed in bdcbf71

These ended up on the wrong side of the merge conflict 0bec4ea

* Fix linting
  • Loading branch information
Adam Silverstein authored and danielbachhuber committed May 31, 2018
1 parent eb4fc18 commit 5a6c24e
Show file tree
Hide file tree
Showing 19 changed files with 1,360 additions and 171 deletions.
25 changes: 17 additions & 8 deletions editor/components/autosave-monitor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import { withSelect, withDispatch } from '@wordpress/data';

export class AutosaveMonitor extends Component {
componentDidUpdate( prevProps ) {
const { isDirty, isSaveable } = this.props;
if ( prevProps.isDirty !== isDirty ||
prevProps.isSaveable !== isSaveable ) {
this.toggleTimer( isDirty && isSaveable );
const { isDirty, isAutosaveable } = this.props;

if (
prevProps.isDirty !== isDirty ||
prevProps.isAutosaveable !== isAutosaveable
) {
this.toggleTimer( isDirty && isAutosaveable );
}
}

Expand All @@ -19,11 +22,11 @@ export class AutosaveMonitor extends Component {

toggleTimer( isPendingSave ) {
clearTimeout( this.pendingSave );

const { autosaveInterval } = this.props;
if ( isPendingSave ) {
this.pendingSave = setTimeout(
() => this.props.autosave(),
10000
autosaveInterval * 1000
);
}
}
Expand All @@ -35,10 +38,16 @@ export class AutosaveMonitor extends Component {

export default compose( [
withSelect( ( select ) => {
const { isEditedPostDirty, isEditedPostSaveable } = select( 'core/editor' );
const {
isEditedPostDirty,
isEditedPostAutosaveable,
getEditorSettings,
} = select( 'core/editor' );
const { autosaveInterval } = getEditorSettings();
return {
isDirty: isEditedPostDirty(),
isSaveable: isEditedPostSaveable(),
isAutosaveable: isEditedPostAutosaveable(),
autosaveInterval,
};
} ),
withDispatch( ( dispatch ) => ( {
Expand Down
16 changes: 11 additions & 5 deletions editor/components/autosave-monitor/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@ describe( 'AutosaveMonitor', () => {

describe( '#componentDidUpdate()', () => {
it( 'should start autosave timer when having become dirty and saveable', () => {
wrapper.setProps( { isDirty: true, isSaveable: true } );
wrapper.setProps( { isDirty: true, isAutosaveable: true } );

expect( toggleTimer ).toHaveBeenCalledWith( true );
} );

it( 'should stop autosave timer when having become dirty but not saveable', () => {
wrapper.setProps( { isDirty: true, isSaveable: false } );
it( 'should stop autosave timer when the autosave is up to date', () => {
wrapper.setProps( { isDirty: true, isAutosaveable: false } );

expect( toggleTimer ).toHaveBeenCalledWith( false );
} );

it( 'should stop autosave timer when having become dirty but not autosaveable', () => {
wrapper.setProps( { isDirty: true, isAutosaveable: false } );

expect( toggleTimer ).toHaveBeenCalledWith( false );
} );
Expand All @@ -42,10 +48,10 @@ describe( 'AutosaveMonitor', () => {
expect( toggleTimer ).toHaveBeenCalledWith( false );
} );

it( 'should stop autosave timer when having become not saveable', () => {
it( 'should stop autosave timer when having become not autosaveable', () => {
wrapper.setProps( { isDirty: true } );
toggleTimer.mockClear();
wrapper.setProps( { isSaveable: false } );
wrapper.setProps( { isAutosaveable: false } );

expect( toggleTimer ).toHaveBeenCalledWith( false );
} );
Expand Down
20 changes: 17 additions & 3 deletions editor/components/editor-global-keyboard-shortcuts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export default compose( [
getMultiSelectedBlockUids,
hasMultiSelection,
getEditorSettings,
isEditedPostDirty,
} = select( 'core/editor' );
const { templateLock } = getEditorSettings();

Expand All @@ -107,25 +108,38 @@ export default compose( [
multiSelectedBlockUids: getMultiSelectedBlockUids(),
hasMultiSelection: hasMultiSelection(),
isLocked: !! templateLock,
isDirty: isEditedPostDirty(),
};
} ),
withDispatch( ( dispatch ) => {
withDispatch( ( dispatch, ownProps ) => {
const {
clearSelectedBlock,
multiSelect,
redo,
undo,
removeBlocks,
autosave,
savePost,
} = dispatch( 'core/editor' );

return {
onSave() {
// TODO: This should be handled in the `savePost` effect in
// considering `isSaveable`. See note on `isEditedPostSaveable`
// selector about dirtiness and meta-boxes. When removing, also
// remember to remove `isDirty` prop passing from `withSelect`.
//
// See: `isEditedPostSaveable`
if ( ! ownProps.isDirty ) {
return;
}

savePost();
},
clearSelectedBlock,
onMultiSelect: multiSelect,
onRedo: redo,
onUndo: undo,
onRemove: removeBlocks,
onSave: autosave,
};
} ),
] )( EditorGlobalKeyboardShortcuts );
2 changes: 1 addition & 1 deletion editor/components/post-publish-button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function PostPublishButton( {
onSubmit = noop,
forceIsSaving,
} ) {
const isButtonEnabled = ! isSaving && isPublishable && isSaveable;
const isButtonEnabled = isPublishable && isSaveable;

let publishStatus;
if ( ! hasPublishAction ) {
Expand Down
20 changes: 17 additions & 3 deletions editor/components/post-saved-state/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -38,13 +43,20 @@ export class PostSavedState extends Component {
}

render() {
const { isNew, isPublished, isDirty, isSaving, isSaveable, onSave } = this.props;
const { isNew, isPublished, isDirty, isSaving, isSaveable, onSave, isAutosaving } = this.props;
const { forceSavedMessage } = this.state;
if ( isSaving ) {
// TODO: Classes generation should be common across all return
// paths of this function, including proper naming convention for
// the "Save Draft" button.
const classes = classnames( 'editor-post-saved-state', 'is-saving', {
'is-autosaving': isAutosaving,
} );

return (
<span className="editor-post-saved-state is-saving">
<span className={ classes }>
<Dashicon icon="cloud" />
{ __( 'Saving' ) }
{ isAutosaving ? __( 'Autosaving' ) : __( 'Saving' ) }
</span>
);
}
Expand Down Expand Up @@ -88,6 +100,7 @@ export default compose( [
isSavingPost,
isEditedPostSaveable,
getCurrentPost,
isAutosavingPost,
} = select( 'core/editor' );
return {
post: getCurrentPost(),
Expand All @@ -96,6 +109,7 @@ export default compose( [
isDirty: forceIsDirty || isEditedPostDirty(),
isSaving: forceIsSaving || isSavingPost(),
isSaveable: isEditedPostSaveable(),
isAutosaving: isAutosavingPost(),
};
} ),
withDispatch( ( dispatch ) => ( {
Expand Down
30 changes: 26 additions & 4 deletions editor/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ export function resetPost( post ) {
};
}

/**
* Returns an action object used in signalling that the latest autosave of the
* post has been received, by initialization or autosave.
*
* @param {Object} post Autosave post object.
*
* @return {Object} Action object.
*/
export function resetAutosave( post ) {
return {
type: 'RESET_AUTOSAVE',
post,
};
}

/**
* Returns an action object used to setup the editor state when first opening an editor.
*
Expand Down Expand Up @@ -351,9 +366,18 @@ export function editPost( edits ) {
};
}

export function savePost() {
/**
* Returns an action object to save the post.
*
* @param {Object} options Options for the save.
* @param {boolean} options.autosave Perform an autosave if true.
*
* @return {Object} Action object.
*/
export function savePost( options ) {
return {
type: 'REQUEST_POST_UPDATE',
options,
};
}

Expand Down Expand Up @@ -392,9 +416,7 @@ export function mergeBlocks( blockAUid, blockBUid ) {
* @return {Object} Action object.
*/
export function autosave() {
return {
type: 'AUTOSAVE',
};
return savePost( { autosave: true } );
}

/**
Expand Down
Loading

0 comments on commit 5a6c24e

Please sign in to comment.