[wp-trac] [WordPress Trac] #16215: Post Revision history displays the incorrect author

[WordPress Trac]

#16215: Post Revision history displays the incorrect author
--------------------------------------------------+-----------------------
 Reporter:  mdawaffe                              |       Owner:  westi
     Type:  defect (bug)                          |      Status:  accepted
 Priority:  normal                                |   Milestone:  3.6
Component:  Revisions                             |     Version:  2.6
 Severity:  normal                                |  Resolution:
 Keywords:  dev-feedback needs-testing has-patch  |
--------------------------------------------------+-----------------------

Comment (by mdawaffe):

 attachment:16215.4.diff
 * Stores revision_version wp_posts.post_name.  Revisions already
 completely takes over that field, so we may as well use it.
 * Updates _edit_last post meta during wp_insert_post() so that meta value
 is always correct.
 * For new revisions, uses _edit_last post meta as the revision author (the
 user who edited the post to that state).
 * For old revisions, fixes the post_author on the fly during display using
 an expanded `get_the_modified_author`.
  * If the correct author cannot be determined, it makes an essentially
 arbitrary guess) and appends a '?' to the end of the author's display
 name.
  * The post_author as corrected by `get_the_modified_author` is not stored
 in the post object to avoid cache pollution.
  * The post_author as corrected by `get_the_modified_author` is not stored
 in the DB.
  * As a side effect of using `get_the_modified_author` everywhere, the
 display of the "current revision" author in `wp_list_post_revisions()` is
 fixed
  * This addition to `get_the_modified_author` could be made a filter.
 * Includes a new `_wp_upgrade_revisions_of_post()` to upgrade a post's
 revisions.  Currently uncalled.
  * Goes through each revision from most recent to oldest and determines if
 and how the revision needs to be upgraded.
  * Uses a options table hack to get an exclusive lock on the upgrade
 routine for that post.  Calling this function multiple times in parallel
 would be racy.

 ----

 Here's how I tested:
 1. New blog with 3 users running unpatched trunk.
 2. Create a new post as user_id=1.
 3. Edit the post as user_id=2.
 4. Edit the post as user_id=3.
 5. Edit the post as user_id=1.
 6. Edit the post as user_id=2.
 7. Edit the post as user_id=3.
 8. View the revisions for that post.  Notice how they are off by one.
 9. Apply the patch.
 10. View the revisions for that post.  Notice how they are correct.
 11. Edit the post as user_id=1.
 12. Edit the post as user_id=2.
 13. Edit the post as user_id=3.
 14. View the revisions for that post.  Notice how they are correct.
 15. Revert the patch.
 16. View the revisions for that post.  Notice how the revisions created
 before the patch was applied are off by one.
 17. Edit the post as user_id=1.
 18. Edit the post as user_id=2.
 19. Edit the post as user_id=3.
 20. Apply the patch.
 21. View the revisions for that post.  Notice how all are correct except
 one: whenever there is a revision_version=1 revision chronologically
 followed by a revision_version=0 revision, we do not know what author
 created the revision_version=0 revision. Authorship for revision_version=0
 revisions is stored in the chronologically preceding revision.  Notice the
 question mark after that revision author's name.  It's a guess (a bad
 one).
 22. Run the upgrade function on that post.
 23. View the revisions for that post.  Should be the same as in step 21.

 ----

 In the above testing procedure, we end up with revisions having something
 like the following revision_versions going from oldest to newest:
  0,0,0,0,0,0,1,1,1,0,0,0

 After the upgrade function is run, we end up with:
  1,1,1,1,1,1,1,1,1,0,1,1

 That revision on the boundary between 1 → 0 is the revision for which
 we have to guess the author.

 Those guessed authors should be rare.  They only occur when the site is
 upgraded to include this patch, then downgraded, then upgraded again.

 In the normal wp-admin editor, a revision is created essentially every
 time a post is first created.  Without that initial revision, it's
 possible that for posts created pre-patch, `get_the_modified_author()`
 will have to guess at the author of the first revision. So created pre-
 patch with XML-RPC, for example, might have guessed first revision
 authorship.  I'm not sure: I haven't tested that.

 ----

 I'm not sure when to run the upgrade function.  On post edit? On revision
 listing? Asynchronously on post edit via WP_Cron?

 In theory we could let the data in the DB be incorrect for ever, and
 always correcty it dynamically on display or even in `WP_Post`.

 ----

 The patch is probably missing some `post_type_supports( $post->post_type,
 'revisions' )` and/or `WP_POST_REVISIONS` checks.

 ----

 I just discovered there's a bug in the upgrade script: the very first
 revision is never upgraded... I think that may be intentional :)  I'll
 look into it later.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/16215#comment:27>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software