Lock is not set to correct user until there is a save

[lancehudson]

When a user opens a post for editing the editor calls
wp_check_post_lock which returns false if it has been AUTOSAVE_INTERVAL * 2 since the lock was set and the user is not equal to _edit_last.

The problem is _edit_last is not the person who has the lock, until an autosave or save is triggered.

Originally the wp_set_post_lock set the _edit_last to the user setting the lock, #6043 and [7103]. But in ticket #12763 this was removed because _edit_last is the last modified by author.

wp_set_post_lock needs to set the lock user to a new post meta (_edit_lock_user ?) and wp_check_post_lock needs to be changed to reference this variable.
_admin_notice_post_locked also needs to be updated to match.

[lancehudson]

After more research it appears that _edit_last is not updated util the post is updated which is good, but it means the lock it set but not by the correct user until they save once.

[jane]

I hate this bug.

[westi]

Current patch appears to just paper round the cracks by changing things.

Need to work out how to reliably reproduce this and investigate - possibly need to clear _edit_last when we set a new lock.

[jane]

So we need a new patch? Adding tag so it will show up in reports.

[nacin]

I took a guess that wp_set_post_lock() was being called before wp_check_post_lock(), but I've been tailing a log for a bit now and can't reproduce.

We need a way to reproduce this issue. I think wp_set_post_lock() could probably clear the other meta key, and that would at least fix the symptoms.

[lancehudson]

If we set or clear _edit_last when doing wp_set_post_lock() won't that make the last modified author be set to someone who hasn't saved anything?

In the patch I wrote I moved the locking to separate meta tags because locking and who last editing are different things.

A stab at repro (I can't test this right now)
Setup WordPress with 3 users:
Login with one and edit a page.
Wait for that lock to expire then, open edit the page logged in as the other user, (don't make any changes).
Then before that lock expires, open edit the page logged in as the third user.
Which user does it say has the lock? It should be user #2 but it will be #1.

[duck_]

You can also reproduce with two users with essentially the same steps. Update and save a post with the first user and logout. Wait until the lock expires and visit the edit post page with the second user without saving (to bump the lock) and then refresh the page. You can also bump the lock time and then be blocked from bulk and quick editing that post.

We don't update _edit_last on every lock because of #12763.

We can't clear _edit_last on every lock because that means that _edit_last is cleared every save. This will making it useless for get_the_modified_author (will not display anything) and will stop wp_check_post_lock from working as desired (i.e. returning an empty string instead of the ID of the last user) which in turn stops all current core code using wp_check_post_lock to stop functioning properly.

The two 'solutions' I can think of: either we have a separate way to store the user on lock as suggested by lancehudson (or combine the ID into _edit_lock somehow) or don't show the name of the user currently holding a lock.

[lancehudson]

Thank you duck_, I think thats how I originally found it. I hit refresh and then couldn't edit. (My WordPress install is experimenting with pessimistic locking.)

If we don't display who has the lock, won't that mean that wp_check_post_lock can't tell who has the lock? then the core code would also stop working.

[duck_]

Replying to lancehudson:

If we don't display who has the lock, won't that mean that wp_check_post_lock can't tell who has the lock? then the core code would also stop working.

Yep, that doesn't work. We'd just have the same situation as we have now, but it will say "Somebody else is currently editing this item" even though nobody else is. I think I wrote that 'solution' when I was thinking of the problem in terms of your three users example when somebody else did actually have the lock but it was just displaying the wrong name.

[cyberhobo]

It seems to me like the current patch or something very similar will have to be done. I'm listening to find out how a plugin that edits posts should go about reliably setting the lock.

[lancehudson]

Does this need a patch anymore? if so what ideas do we have, I will be happy to write a patch that does anything we can think of.

I personally think that there are 2 things we are trying to store in meta tags, the last author and who has a lock. They aren't related and so cant be stored together. Short of making them two separate meta tags I don't have any ideas.

[scribu]

That sounds good.

[duck_]

Replying to duck_:

(or combine the ID into _edit_lock somehow)

15130.add-id-to-lock.diff does this. Note not completely ready since you will get a warning the first time you visit a post with the old style lock or a post without a lock. Just a quick proof-of-concept type patch.

[mdawaffe]

Replying to duck_:

15130.add-id-to-lock.diff does this. Note not completely ready since you will get a warning the first time you visit a post with the old style lock or a post without a lock. Just a quick proof-of-concept type patch.

+1 to storing the user_id and the lock time in the same key.

If you store "$time:$user_id" (instead of the other way around as 15130.add-id-to-lock.2.diff does), it's easier to handle the old style lock case, just pull _edit_last if there's no $user_id after the list().

With that patch, I don't get a warning when visiting a post without a lock (which is good). Why do you say I will?

Tangentially, I'd also like to see the wp_set_post_lock() call moved to below the edit-form-advanced.php include in wp-admin/post.php. That way, the only DB write for that page is after all of the DB read queries, which is a performance optimization for sites running HyperDB and/or some caching plugins. (That's lower priority than getting the lock to stop lying: probably worth a new ticket.)

[duck_]

Replying to mdawaffe:

With that patch, I don't get a warning when visiting a post without a lock (which is good). Why do you say I will?

The first patch would have, take two works for no lock (checks for false before exploding) but still not properly for the old style locks.

15130.add-id-to-lock.3.diff attached. Does isset check after explode instead of just list (stops a notice), grabs _edit_last meta if missing the user id in _edit_lock.

[automattor]

(In [16901]) Add user id to lock. Props duck_. see #15130