Add support for options page locking (settings API concurrency)

[westonruter]

If two users access an options page at the same time, the user who saves last will have their settings stick, while the user who saves first will have their settings lost. This is because options pages lack any access concurrency checking.

In #31436 we propose to handle conflicts in concurrent Customizer sessions by adding locking to individual controls so that users can't override each others' values. The same functionality is needed for options pages generated generated via the Settings API. At the most basic level, locking could be implemented to block a user from accessing an options page if someone else is already there, presenting the user with the same takeover screen as shown for post locking.

A more sophisticated approach would be to allow multiple users to edit an options page at once, either by syncing values across the sessions via Heartbeat, or by disabling a field entirely for other users when one user starts editing it. In this way, the disabled field would not be included when the other users submit the form. This may not work in all cases, however.

Another approach, less sophisticated and not using Heartbeat at all, would be to introduce a persistent counter for saves to a given admin page. This would serve as a revision number, and the revision could be included with each settings form. If the submitted form has a setting number that is not the same as what is currently stored, then this would indicate there is a potential conflict and the user could be presented with a screen to resolve any differences.

[MikeNGarrett]

We talked through this proposal this morning. The ideas of merging settings changes seems like a much larger step forward than making the locking mechanism more generic.

I think the best initial approach would be to make the locking mechanism available to all developers where someone could specify a unique string to "lock" and a checking mechanism.

As an example of this functionality, we could implement this for all options pages (with exceptions for edit.php, edit-tags.php, etc ).

To me, that seems like enough of a jump for a future release. In the future we can look at handling different sections of a page or merging different values from 2 different people editing the page.

[daxelrod]

Generic edit locking functions for wp-admin

[daxelrod]

Attached a generic edit locking concept for core. Roughly extends the API for post_locks and could serve as a base for settings page or customizer locking. The edit locks leverage *meta tables for comments, posts, and users; sitemeta table (type = 'site') for multisite; and options for everything else. The $type and $id parameters are purposefully not limited as to allow developers edit lock capabilities on their own settings pages.

Basic API:

wp_get_edit_lock( $type, $id ); // Retrieves the edit lock if it exists
wp_check_edit_lock( $type, $id ); // Checks if the object is being edited by another user
wp_set_edit_lock( $type, $id, $time, $user_id ); // Mark an object as being edited.

Introduced some helper functions for consistent handling of locks:

wp_read_edit_lock( $lock ); // Splits apart edit lock into time and user_id
wp_create_edit_lock( $time, $user_id ); // Packs time and user_id into an edit lock string

Introduced 2 new filters:

add_filter('allow_edit_lock', function ( $allow, $type, $id ) {}, 10, 3); // Control over if edit locks are allowed
add_filter('wp_edit_' . $type . '_lock_window', function ( $time_window ) {}); // Control over duration of edit lock for a particular type.

It has full compatibility with existing post_locks and those existing functions now proxy the new edit_lock API.

If this is desirable for core, the next step would be to make a generic version of the locking UI (This XXX is currently locked by YYY) so visual feedback for edit locks can be applied in other contexts (i.e. settings pages, customizer, etc.). We have set up a small demo where taxonomy terms can be edit locked as a proof of concept if anyone is interested. Any input appreciated.

[MikeNGarrett]

It would be awesome to get someone to test this out.

Attaching appropriate tags.

[azaozz]

There are several things that need to be considered:

• The removal of locks (AJAX on unload) doesn't work well in all browsers and in all circumstances. Sometimes the AJAX request doesn't fire, sometimes the browser aborts it. We need to look at improving it/making it more reliable.
• To be able to do "take over", we may need to save the current user's changes somehow. Not all settings screens will need this but there are several where we wouldn't want to discard unsaved changes/loose the current user's data.

One way this may work is by storing all form fields in session storage and restore on page reload. Another option is to show an "attempted take over" dialog with a cancel button instead of locking the screen.

edit_lock.patch is a good start, but we will need to implement the above too. Also, not sure we need variable lock timeouts. That complicates things without adding any benefits. If we manage to fix lock removal, a 5 min lock timeout would be good.

[daxelrod]

The removal of locks (AJAX on unload) doesn't work well in all browsers and in all circumstances. Sometimes the AJAX request doesn't fire, sometimes the browser aborts it. We need to look at improving it/making it more reliable.

It seems quite reliable in modern browsers, and in extreme cases such as force quit, in my opinion the lock timeout provides an adequate fallback.

To be able to do "take over", we may need to save the current user's changes somehow. Not all settings screens will need this but there are several where we wouldn't want to discard unsaved changes/loose the current user's data.

I agree the edit lock API should provide functions for this. Local storage seems like a logical choice to implement, but what/how data is saved and how the user would go about restoring it are UX questions.

One way this may work is by storing all form fields in session storage and restore on page reload. Another option is to show an "attempted take over" dialog with a cancel button instead of locking the screen.

I'm not sure I would want to go down this route of enabling ping-ponging back and forth.

[danielbachhuber]

Created #34173 for a more specific conversation about locking UX on term edit pages.

[johnbillion]

I'd like to look into this for 4.8.

[pbiron]

Related: #44900

[fawadinho]

Any updates on this? We have decided to make use of this library for WP: ​https://github.com/soulseekah/wp-lock
It does the job as being generic and easy to use in all types of scenarios

[callumbw95]

Hi All,

Just wondering if this is still relevant within the scope of WordPress today? ​Phase 3 looks to move away from locking posts, and instead move towards more collaborative post / page creation through data views. I know this ticket is talking more around the settings / options pages, but I imagine this might also be going the same way too in the future?

[westonruter]

@callumbw95 Thanks for reminding me of this decade-old ticket! It definitely seems relevant for Phase 3 (Collaboration).

I think the open question I have for this ticket is how often it is an issue. In practice, most setting screens (in core) are likely updated rarely. I'd love to hear anecdotes from users who have actually had the problem of two users overwriting their pending changes. Granted, plugins may have settings screens which are updated much more commonly, even on a daily basis, having some locking mechanism may make sense.

At the very least, for collaborative editing I should think that there should be a way to have a presence indicator shown on the various admin screens. In this way, if I land on the Reading screen and yet I see that Callum Bridgford-Whittick is shown as also being there, then I should probably ping you to collaborate and not make any change. So I think the awareness of presence is the first problem to solve.

Going beyond this, maybe an active presence from another user could cause a screen to be locked with an overlay. But I can imagine this being problematic for screens like those with list tables on which users could make discrete changes to individual items without there being a conflict. Granted these are not "options" screens, but consider the case of two users both being on the Comments screen to moderate pending comments. It could be that the two users are collaborating so that one starts moderating comments list from the top and the other from the bottom to then meet in the middle. This issue could also manifest on non-core options screens that have option toggles that apply their updates live without having to submit the form.

Then again, if two users are allowed on the same screen at the same time, there's then the issue of the state of the pages diverging, so the two users aren't sure what has been touched by the other user or not. To address this, the ideal would be for the page states to sync across the user sessions, but this quickly adds to the complexity (and may not even be possible for custom options screens using their own ad hoc JS).

I think I've argued myself back in favor of having a screen locking overlay when there is another user present, for options screens specifically. For other screens, like those using data views, more sophisticated collaboration could be used such as synchronizing state across user sessions.

I don't think it makes sense to try to implement any kind of synchronization of form fields on an options screen for concurrent users. This will be unreliable. A simple lock screen overlay should be quite sufficient. Anything beyond this should be plugin territory.

[callumbw95]

Hey @westonruter,

Thanks for your reply here, and I agree with you in regards to just locking the options screen as it does feel a bit overkill to make these pages collaborative. I need to take another look into the previous patch, but I suspect that this may not work fully in the current release, and will do some testing to check it out. However as you have also said, it does feel pretty edge case, and I am not sure how many people are running in to this issue on a regular basis?