WordPress.org

Make WordPress Core

#44321 closed enhancement (fixed)

REST API: Expose revision count and last revision ID on Post response

Reported by: danielbachhuber Owned by: pento
Milestone: 4.9.8 Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch has-unit-tests commit fixed-major
Focuses: Cc:
PR Number:

Description

A Post in the Gutenberg needs access to:

  • Total number of revisions.
  • Last revision ID.

This data is important for revisions UI in the Post Settings section.

At the moment, the data is exposed in a pretty hacky manner:

"ping_status": "open",
"revisions": {
    "count": 2,
    "last_id": 3
},
"slug": "foo-bar-post",

Is there a better way of exposing this data? I don't have a strong opinion but maybe it'd be better on _links.

See https://github.com/WordPress/gutenberg/pull/3233/files#diff-9dfc2b70ad1c71918441461179054b0bR363

From https://github.com/WordPress/gutenberg/issues/3258

Attachments (3)

44321.1.diff (1.2 KB) - added by kadamwhite 17 months ago.
Add a count property to the version-history link specifying the number of available revisions, and a _link using the "predecessor" relation to specify the most recent revision.
44321.2.diff (3.3 KB) - added by danielbachhuber 16 months ago.
44321.3.diff (3.4 KB) - added by danielbachhuber 16 months ago.

Download all attachments as: .zip

Change History (23)

#1 @kadamwhite
17 months ago

Spoke to @rmccue about this a bit and here's my take: First, I would have expected that revision count would be handled similarly to comment count, as it's a similar type of question. Now, we don't seem to _expose_ comment count at all, sooo... maybe we should address that here, too :)

I agree that this feels like a task for _links. Looking at the web linking RFC, it's spec-compliant to add new key/value pairs to a link object:

Target attributes are a set of key/value pairs that describe the link or its target; for example, a media type hint. This specification does not attempt to coordinate their names or use, but does provide common target attributes for use in the Link HTTP header.

This means that count: ## would be appropriate for addition to both the version-history and replies properties.

For last_id, we could also jam that into the _link for the version history relation; OR we could specify it as a new link. I'm slightly biased towards the latter but the former is certainly lighter weight.

To validate an assumption: I haven't checked this, but I'm guessing the last revision ID refers to the parent revision of the published post, NOT a revision that is equal to the published post?

@kadamwhite
17 months ago

Add a count property to the version-history link specifying the number of available revisions, and a _link using the "predecessor" relation to specify the most recent revision.

#2 @kadamwhite
17 months ago

  • Keywords has-patch added; needs-patch removed

44321.1.diff executes the plan described above, by adding a count to the version-history link and adding a new link using the "predecessor" relation.

#3 @danielbachhuber
17 months ago

To validate an assumption: I haven't checked this, but I'm guessing the last revision ID refers to the parent revision of the published post, NOT a revision that is equal to the published post?

Yes. What you've implemented with the patch is the same as what's already in Gutenberg.

44321.1.diff looks good to me. My only concern is that wp_get_post_revisions() might be too heavy for our needs, but I'm not sure whether we have a reasonable alternative (aside from a direct SQL query).

#4 @kadamwhite
17 months ago

Agreed, but I'm not sure how best to practically validate that question about the weight of wp_get_post_revisions. I'd like to avoid a custom SQL query if possible, feels like overkill!

Tagging @adamsilverstein and @karmatosed as the listed Revisions component maintainers, any idea if there's a lighter-weight way to get this data?

#5 @birgire
17 months ago

One possible improvement that comes to mind is:

$revisions = wp_get_post_revisions( $post->ID, array( 
	'fields' => 'ids',
) );

instead of:

$revisions = wp_get_post_revisions( $post->ID );

since the patch in 44321.1.diff only needs the count and the ID.

Here's the result of few test runs:

100 revisions:
--------------
time: 0.011561
#qs: 2

time: 0.011199
#qs: 2

time: 0.011316
#qs: 2

200 revisions:
--------------
time: 0.020648
#qs: 2

time: 0.021641
#qs: 2

time: 0.021116
#qs: 2

100 revisions (fields=ids):
--------------
time: 0.001606
#qs: 1

time: 0.001734
#qs: 1

time: 0.001875
#qs: 1

200 revisions (fields=ids):
--------------
time: 0.002118
#qs: 1

time: 0.002110
#qs: 1

time: 0.002875
#qs: 1

where we test it with variations of:

global $wpdb;

$post_id           = self::factory()->post->create();
$revisions_count   = 100;
		
foreach( range( 1, $revisions_count ) as $rng ) {
	wp_update_post( array(
		'post_content' => $rng,
		'ID'           => $post_id,
	) );
}
		
$wpdb->timer_start();
$query_count = $wpdb->num_queries;

$revisions = wp_get_post_revisions( $post_id, array( 
	'fields'         => 'ids',
) );

printf( 'time: %f' . PHP_EOL, $wpdb->timer_stop() );
printf( '#qs: %d' . PHP_EOL, $wpdb->num_queries - $query_count );

So from this quick testing, for 100-200 revisions, it seems to reduce the number of queries from 2 to 1 and runs order of magnitude times faster.

Version 0, edited 17 months ago by birgire (next)

#6 @TimothyBlynJacobs
16 months ago

I believe this should be predecessor-version not predecessor, https://tools.ietf.org/html/rfc5829#section-3.5

#7 @danielbachhuber
16 months ago

+1 to using 'fields' => 'ids' for more performance.

We should also only expose this data when context=edit (which is a deficiency in the current Gutenberg implementation).

#8 @danielbachhuber
16 months ago

We should also only expose this data when context=edit (which is a deficiency in the current Gutenberg implementation).

Ah, this isn't possible because $request isn't passed through to prepare_links().

I suppose it's a non-issue to include the revision id in public data. In theory, this could be deduced by iterating ids on the revisions endpoint and looking at 401 vs 404 response headers. However, the revision count isn't public data. I don't see any issues with disclosing it, but something worth noting at the least.

#9 @danielbachhuber
16 months ago

  • Keywords has-unit-tests added; dev-feedback needs-unit-tests removed
  • Milestone changed from 4.9.8 to 4.9.7

In 44321.2.diff:

  • Uses 'fields' => 'ids' when fetching revisions to improve performance.
  • Includes id attribute in predecessor to avoid needing to parse a URL.
  • Adds unit tests.

#10 @danielbachhuber
16 months ago

In 44321.3.diff:

#11 @ocean90
16 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

This ticket was mentioned in Slack in #core by jon_bossenger. View the logs.


16 months ago

This ticket was mentioned in Slack in #core by joshuawold. View the logs.


15 months ago

#14 @pbiron
15 months ago

We discussed this during the 4.9.8 bug scrub today.

Is this ready to land in 4.9.8? Beta is scheduled for July 17 (next Tues). Any chance it will be ready?

#15 follow-up: @danielbachhuber
15 months ago

  • Keywords commit added
  • Owner set to pento
  • Status changed from new to assigned

@pbiron Yes, this is ready to land. It's in Gutenberg as of https://github.com/WordPress/gutenberg/pull/7495

#16 in reply to: ↑ 15 @pbiron
15 months ago

Replying to danielbachhuber:

@pbiron Yes, this is ready to land. It's in Gutenberg as of https://github.com/WordPress/gutenberg/pull/7495

Thanx for the quick response!

#17 @pento
15 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 43439:

REST API: Expose revision count and last revision ID on Post response

So that REST API clients can show appropriate UI for a post's revisions, it needs to know how many revisions the post has, and what the latest revision ID is.

Props kadamwhite, danielbachhuber, birgire, TimothyBlynJacobs.
Fixes #44321.

#18 @pento
15 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopened for backport.

#19 @pento
15 months ago

In 43441:

REST API: Update the test fixture generator following [43439]

As [43439] added data that contains object IDs, it can cause wp-api-generated.js to be unnecessarily regenerated.

Regenerating our list of fixtures that need normalising rectifies this.

See #44321.

#20 @pento
15 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43442:

REST API: Expose revision count and last revision ID on Post response

So that REST API clients can show appropriate UI for a post's revisions, it needs to know how many revisions the post has, and what the latest revision ID is.

Merge of [43439] and [43441] to the 4.9 branch.

Props kadamwhite, danielbachhuber, birgire, TimothyBlynJacobs, pento.
Fixes #44321.

Note: See TracTickets for help on using tickets.