WordPress.org

Make WordPress Core

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


Ignore:
Timestamp:
02/03/2013 03:30:01 AM (5 years 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.