Opened 6 years ago
Closed 6 years ago
#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: |
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
Attachments (3)
Change History (23)
@
6 years 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
@
6 years 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
@
6 years 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
@
6 years 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
@
6 years 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.
One could also try with some extra posts in the database, e.g.:
self::factory()->post->create_many( 1000 );
I also skimmed through wp_count_posts()
, but it didn't seem usable here. Just imagine if WP_Query
would support a count attribute :-)
#6
@
6 years ago
I believe this should be predecessor-version
not predecessor
, https://tools.ietf.org/html/rfc5829#section-3.5
#7
@
6 years 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
@
6 years 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
@
6 years 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 inpredecessor
to avoid needing to parse a URL. - Adds unit tests.
#10
@
6 years ago
In 44321.3.diff
:
- Renames
predecessor
topredecessor-version
per https://core.trac.wordpress.org/ticket/44321#comment:6
#11
@
6 years 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.
6 years ago
This ticket was mentioned in Slack in #core by joshuawold. View the logs.
6 years ago
#14
@
6 years 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:
↓ 16
@
6 years 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
@
6 years 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!
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:This means that
count: ##
would be appropriate for addition to both theversion-history
andreplies
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?