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

Post Settings: Refactor revisions handling to avoid performance issues #3233

Merged
merged 1 commit into from
Oct 31, 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
21 changes: 21 additions & 0 deletions editor/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,27 @@ export function getCurrentPostId( state ) {
return getCurrentPost( state ).id || null;
}

/**
* Returns the number of revisions of the post currently being edited.
*
* @param {Object} state Global application state
* @return {Number} Number of revisions
*/
export function getCurrentPostRevisionsCount( state ) {
return get( getCurrentPost( state ), 'revisions.count', 0 );
}

/**
* Returns the last revision ID of the post currently being edited,
* or null if the post has no revisions.
*
* @param {Object} state Global application state
* @return {?Number} ID of the last revision
*/
export function getCurrentPostLastRevisionId( state ) {
return get( getCurrentPost( state ), 'revisions.last_id', null );
}

/**
* Returns any post values which have been changed in the editor but not yet
* been saved.
Expand Down
40 changes: 13 additions & 27 deletions editor/sidebar/last-revision/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,65 +2,51 @@
* External dependencies
*/
import { connect } from 'react-redux';
import { flowRight, first } from 'lodash';

/**
* WordPress dependencies
*/
import { sprintf, _n } from '@wordpress/i18n';
import { IconButton, PanelBody, withAPIData } from '@wordpress/components';
import { IconButton, PanelBody } from '@wordpress/components';

/**
* Internal dependencies
*/
import './style.scss';
import {
isEditedPostNew,
getCurrentPostId,
getCurrentPostType,
isSavingPost,
getCurrentPostLastRevisionId,
getCurrentPostRevisionsCount,
} from '../../selectors';
import { getWPAdminURL } from '../../utils/url';

function LastRevision( { revisions } ) {
const lastRevision = first( revisions.data );
if ( ! lastRevision ) {
function LastRevision( { lastRevisionId, revisionsCount } ) {
if ( ! lastRevisionId ) {
return null;
}

return (
<PanelBody>
<IconButton
href={ getWPAdminURL( 'revision.php', { revision: lastRevision.id } ) }
href={ getWPAdminURL( 'revision.php', { revision: lastRevisionId } ) }
className="editor-last-revision__title"
icon="backup"
>
{
sprintf(
_n( '%d Revision', '%d Revisions', revisions.data.length ),
revisions.data.length
_n( '%d Revision', '%d Revisions', revisionsCount ),
revisionsCount
)
}
</IconButton>
</PanelBody>
);
}

export default flowRight(
connect(
( state ) => {
return {
isNew: isEditedPostNew( state ),
postId: getCurrentPostId( state ),
postType: getCurrentPostType( state ),
isSaving: isSavingPost( state ),
};
}
),
withAPIData( ( props, { type } ) => {
const { postType, postId } = props;
export default connect(
( state ) => {
return {
revisions: `/wp/v2/${ type( postType ) }/${ postId }/revisions`,
lastRevisionId: getCurrentPostLastRevisionId( state ),
revisionsCount: getCurrentPostRevisionsCount( state ),
};
} )
}
)( LastRevision );
46 changes: 46 additions & 0 deletions editor/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
isCleanNewPost,
getCurrentPost,
getCurrentPostId,
getCurrentPostLastRevisionId,
getCurrentPostRevisionsCount,
getCurrentPostType,
getPostEdits,
getEditedPostTitle,
Expand Down Expand Up @@ -875,6 +877,50 @@ describe( 'selectors', () => {
} );
} );

describe( 'getCurrentPostLastRevisionId', () => {
it( 'should return null if the post has not yet been saved', () => {
const state = {
currentPost: {},
};

expect( getCurrentPostLastRevisionId( state ) ).toBeNull();
} );

it( 'should return the last revision ID', () => {
const state = {
currentPost: {
revisions: {
last_id: 123,
},
},
};

expect( getCurrentPostLastRevisionId( state ) ).toBe( 123 );
} );
} );

describe( 'getCurrentPostRevisionsCount', () => {
it( 'should return 0 if the post has no revisions', () => {
const state = {
currentPost: {},
};

expect( getCurrentPostRevisionsCount( state ) ).toBe( 0 );
} );

it( 'should return the number of revisions', () => {
const state = {
currentPost: {
revisions: {
count: 5,
},
},
};

expect( getCurrentPostRevisionsCount( state ) ).toBe( 5 );
} );
} );

describe( 'getCurrentPostType', () => {
it( 'should return the post type', () => {
const state = {
Expand Down
10 changes: 0 additions & 10 deletions lib/client-assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -462,12 +462,6 @@ function gutenberg_extend_wp_api_backbone_client() {
return model.prototype.route && route === model.prototype.route.index;
} );
};
wp.api.getPostTypeRevisionsCollection = function( postType ) {
var route = '/' + wpApiSettings.versionString + this.postTypeRestBaseMapping[ postType ] + '/(?P<parent>[\\\\d]+)/revisions';
return _.find( wp.api.collections, function( model ) {
return model.prototype.route && route === model.prototype.route.index;
} );
};
wp.api.getTaxonomyModel = function( taxonomy ) {
var route = '/' + wpApiSettings.versionString + this.taxonomyRestBaseMapping[ taxonomy ] + '/(?P<id>[\\\\d]+)';
return _.find( wp.api.models, function( model ) {
Expand Down Expand Up @@ -703,10 +697,6 @@ function gutenberg_editor_scripts_and_styles( $hook ) {
gutenberg_get_rest_link( $post_to_edit, 'about', 'edit' ),
);

if ( ! $is_new_post ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Preloading all revisions and exposing them in the Script tag was the main root cause of performance issues reported in #3167. Kudos to @pento for finding out 👍

Copy link
Member

Choose a reason for hiding this comment

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

But why?

Copy link
Member Author

Choose a reason for hiding this comment

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

See my description:

This code preloads API response for revisions endpoint, which fetches all existing post revisions and exposes them in the <script /> tag. When you have let's say 50 revisions, all of them are loaded from the database and exposed in the HTML code leading to the big size of the HTML and increases processing time to process all objects.

And more technical comment from @pento:

Loading all the revisions was running pretty much all of the code that would be run if the posts were being loaded to display on the front end. ‘the_content’ was the biggest problem, but everything added up.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, definitely overlooked this 👍

$preload_paths[] = gutenberg_get_rest_link( $post_to_edit, 'version-history' );
}

$preload_data = array_reduce(
$preload_paths,
'gutenberg_preload_api_request',
Expand Down
40 changes: 40 additions & 0 deletions lib/register.php
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,46 @@ function gutenberg_register_rest_routes() {
add_action( 'rest_api_init', 'gutenberg_register_rest_routes' );


/**
* Gets revisions details for the selected post.
*
* @since 1.6.0
*
* @param array $post The post object from the response.
* @return array|null Revisions details or null when no revisions present.
*/
function gutenberg_get_post_revisions( $post ) {
$revisions = wp_get_post_revisions( $post['id'] );
$revisions_count = count( $revisions );
if ( 0 === $revisions_count ) {
return null;
}

$last_revision = array_shift( $revisions );

return array(
'count' => $revisions_count,
'last_id' => $last_revision->ID,
);
}

/**
* Adds the custom field `revisions` to the REST API response of post.
*
* TODO: This is a temporary solution. Next step would be to find a solution that is limited to the editor.
*
* @since 1.6.0
*/
function gutenberg_register_rest_api_post_revisions() {
register_rest_field( get_post_types( '', 'names' ),
'revisions',
array(
'get_callback' => 'gutenberg_get_post_revisions',
)
);
}
add_action( 'rest_api_init', 'gutenberg_register_rest_api_post_revisions' );

/**
* Injects a hidden input in the edit form to propagate the information that classic editor is selected.
*
Expand Down