WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#20982 closed defect (bug) (fixed)

Restoring post revisions does not update _edit_last

Reported by: redpixelstudios Owned by: ocean90
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.3.2
Component: Revisions Keywords: has-patch
Focuses: Cc:

Description

To recreate the problem:

Prerequisites

  • A WordPress install with 2 or more users.
  • (NOTE: In my particular case, user a was an Author while user b was an Admin. I am doubtful this matters, but I figured I would mention it just in case.)

Algorithm

  • Create a new post as user a.
  • Modify the post with some minor changes as user b.
  • As user a again, view the post. Go to revisions and restore the first post you made.
  • View the site and note that while the content has been updated to display the original post, the last modified author reads user b.

Attachments (7)

20982-patch01.diff (470 bytes) - added by redpixelstudios 5 years ago.
20982-patch01.2.diff (470 bytes) - added by adamsilverstein 5 years ago.
unit test for #20982
20982.ut.diff (1.3 KB) - added by adamsilverstein 5 years ago.
correction, this is unit test for #20982
20982.diff (549 bytes) - added by adamsilverstein 4 years ago.
updated against trunk, code moved to new location
20982.2.diff (509 bytes) - added by adamsilverstein 4 years ago.
updated against current trunk
20982.patch (1.1 KB) - added by ocean90 4 years ago.
20982.fix-unit-test.patch (733 bytes) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (24)

#1 @redpixelstudios
5 years ago

  • Keywords has-patch added

#2 @westi
5 years ago

  • Keywords revisions-3.6 added

Marking as a candidate for resolving as part of the work on revisions in 3.6

#3 @westi
5 years ago

  • Keywords revisions-3.6 removed
  • Milestone changed from Awaiting Review to 3.6

Addressing #16215 probably fixes this, if not we should fix this too.

#4 @adamsilverstein
5 years ago

  • Cc adamsilverstein@… added

westi: i agree #16215 fixes this. lines up with attached patch.

however, i was unable to reproduce bug anyway, either with or without the patch from #16215.

i don't understand the ticket author's last step in reproducing the bug "View the site and note that while the content has been updated to display the original post, the last modified author reads user b." - does the author mean to check the live site, if so my default setup doesn't show a 'last modified author', where is that? if they are talking about the admin revisions list, this looks correct now, shows current user on current revision after restoring any past revision, which seems like correct behavior.

suggest closing this ticket.

#5 @ethitter
5 years ago

  • Cc erick@… added

#6 follow-up: @ethitter
5 years ago

Confirmed that the bug exists, and that the 16215.4.diff patch addresses it.

One point of clarity: the value of _edit_last will be set not to the ID of the author of the restored revision, but to the ID of the user that restored the revision. This approach seems appropriate, but I wanted to note it here for others to consider.

@adamsilverstein
5 years ago

unit test for #20982

@adamsilverstein
5 years ago

correction, this is unit test for #20982

#7 @westi
5 years ago

In 1210/tests:

Revisions: Add a test case which checks the _edit_last postmeta value after a revision restore

Also validates some of the things to be fixed for the off-by-one issues with authorship.
See: #20982, #16215
Props adamsilverstein for the original patch.

#8 in reply to: ↑ 6 @westi
5 years ago

I've added Adams test case with some tweaks above.

Replying to ethitter:

Confirmed that the bug exists, and that the 16215.4.diff patch addresses it.

One point of clarity: the value of _edit_last will be set not to the ID of the author of the restored revision, but to the ID of the user that restored the revision. This approach seems appropriate, but I wanted to note it here for others to consider.

That sounds right, at the moment the test is wrong for this - I left a note in the test case because I'm not 100% sure it is right at the moment but wanted to get it into the repo so we could iterate on the tests.

#9 @adamsilverstein
4 years ago

based on the new approach in #16215 this patch is still needed.

@adamsilverstein
4 years ago

updated against trunk, code moved to new location

#10 @adamsilverstein
4 years ago

20982.diff​

  • stores the current user in _edit_last post meta when restoring revision

#11 @adamsilverstein
4 years ago

fix now rolled into patch on #23497 for testing

@adamsilverstein
4 years ago

updated against current trunk

#12 @adamsilverstein
4 years ago

  • Keywords dev-feedback added

@ocean90
4 years ago

#13 follow-up: @ocean90
4 years ago

  • Keywords dev-feedback removed

20982.patch moves the line into wp_restore_post_revision() because it's also called in wp_restoreRevision().

#14 in reply to: ↑ 13 ; follow-up: @adamsilverstein
4 years ago

Replying to ocean90:

20982.patch moves the line into wp_restore_post_revision() because it's also called in wp_restoreRevision().

looks like a good spot for this...

not sure ! $post_id part needed since is_wp_error( $post_id ) will return false anyway if ! $post_id , right?

#15 in reply to: ↑ 14 ; follow-up: @ocean90
4 years ago

Replying to adamsilverstein:

not sure ! $post_id part needed since is_wp_error( $post_id ) will return false anyway if ! $post_id , right?

No, see trunk/wp-includes/class-wp-error.php:19712#L206.

#16 in reply to: ↑ 15 @adamsilverstein
4 years ago

Replying to ocean90:

Replying to adamsilverstein:

not sure ! $post_id part needed since is_wp_error( $post_id ) will return false anyway if ! $post_id , right?

No, see trunk/wp-includes/class-wp-error.php:19712#L206.

ah, i see.

#17 @ocean90
4 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 24178:

Revisions: Update _edit_last when restoring a post.

props redpixelstudios. fixes #20982.

Note: See TracTickets for help on using tickets.