WordPress.org

Make WordPress Core

Changes between Initial Version and Version 1 of Ticket #16215, comment 28


Ignore:
Timestamp:
02/03/13 03:30:01 (17 months ago)
Author:
nacin
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #16215, comment 28

    initial v1  
    11Replying to [comment:27 mdawaffe]: 
    2 > attachment:16215.4.diff 
    3 > * Stores revision_version wp_posts.post_name.  Revisions already completely takes over that field, so we may as well use it. 
    4 > * Updates _edit_last post meta during wp_insert_post() so that meta value is always correct. 
    5 > * For new revisions, uses _edit_last post meta as the revision author (the user who edited the post to that state). 
    6 > * For old revisions, fixes the post_author on the fly during display using an expanded `get_the_modified_author`. 
    7 >  * 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. 
    8 >  * The post_author as corrected by `get_the_modified_author` is not stored in the post object to avoid cache pollution. 
    9 >  * The post_author as corrected by `get_the_modified_author` is not stored in the DB. 
    10 >  * 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 
    11 >  * This addition to `get_the_modified_author` could be made a filter. 
    12 > * Includes a new `_wp_upgrade_revisions_of_post()` to upgrade a post's revisions.  Currently uncalled. 
    13 >  * Goes through each revision from most recent to oldest and determines if and how the revision needs to be upgraded. 
    14 >  * 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. 
    15 >  
    16 > ---- 
    17 >  
    18 > Here's how I tested: 
    19 > 1. New blog with 3 users running unpatched trunk. 
    20 > 2. Create a new post as user_id=1. 
    21 > 3. Edit the post as user_id=2. 
    22 > 4. Edit the post as user_id=3. 
    23 > 5. Edit the post as user_id=1. 
    24 > 6. Edit the post as user_id=2. 
    25 > 7. Edit the post as user_id=3. 
    26 > 8. View the revisions for that post.  Notice how they are off by one. 
    27 > 9. Apply the patch. 
    28 > 10. View the revisions for that post.  Notice how they are correct. 
    29 > 11. Edit the post as user_id=1. 
    30 > 12. Edit the post as user_id=2. 
    31 > 13. Edit the post as user_id=3. 
    32 > 14. View the revisions for that post.  Notice how they are correct. 
    33 > 15. Revert the patch. 
    34 > 16. View the revisions for that post.  Notice how the revisions created before the patch was applied are off by one. 
    35 > 17. Edit the post as user_id=1. 
    36 > 18. Edit the post as user_id=2. 
    37 > 19. Edit the post as user_id=3. 
    38 > 20. Apply the patch. 
    39 > 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). 
    40 > 22. Run the upgrade function on that post. 
    41 > 23. View the revisions for that post.  Should be the same as in step 21. 
    42 >  
    43 > ---- 
    44 >  
    45 > In the above testing procedure, we end up with revisions having something like the following revision_versions going from oldest to newest: 
    46 >  0,0,0,0,0,0,1,1,1,0,0,0 
    47 >  
    48 > After the upgrade function is run, we end up with: 
    49 >  1,1,1,1,1,1,1,1,1,0,1,1 
    50 >  
    51 > That revision on the boundary between 1 → 0 is the revision for which we have to guess the author. 
    52 >  
    53 > Those guessed authors should be rare.  They only occur when the site is upgraded to include this patch, then downgraded, then upgraded again. 
    54 >  
    55 > 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. 
    56 >  
    57 > ---- 
    58 >  
    59 > I'm not sure when to run the upgrade function.  On post edit? On revision listing? Asynchronously on post edit via WP_Cron? 
    60 >  
    61 > 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`. 
    62 >  
    63 > ---- 
    64 >  
    65 > The patch is probably missing some `post_type_supports( $post->post_type, 'revisions' )` and/or `WP_POST_REVISIONS` checks. 
    66 >  
    67 > ---- 
    68 >  
    69 > 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. 
    70  
    712 
    723i 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.