[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 adamsilverstein):

 Replying to [comment:27 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.


 i don't think we need to worry about upgrade/downgrade/upgrade you
 mentioned. i'm not even sure we need to upgrade the old db entries, i
 think just showing them correctly and storing them correctly from now on
 might suffice.

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