Implement Post Locking MVP

[swissspidy]

Feature Description

The story editor does not support editing a story at the same time by multiple authors. In order to avoid collisions, only one author should be able to edit a story at a time.

WordPress has this useful feature called Post Locking forr that.

As soon as you start editing a post/story, it will be marked as "locked". Anyone else trying to edit the story after you will get a warning like this:

Context

• [DRAFT] Product brief

Alternatives Considered

Additional Context

Original implementation in Gutenberg:
WordPress/gutenberg#4217
WordPress/gutenberg#4331
WordPress/gutenberg#11781

Screenshots:

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance Criteria

• There's a new experiment / feature flag for post locking
• Post lock status is displayed in the dashboard
• Post locking mechanism works in the editor the same way as in the regular post editor (i.e. feature parity)
  • Dialog if someone is already editing
  • Allow taking over editing
  • Dialog if someone took over editing from you, autosave story in that case
  • Set lock when opening editor, remove lock upon closing
• There is a storybook story for the new dialog
• Dialogs use existing styles from editor, not WordPress styling
• Since there is no UX defined yet for these interactions, make your best guesses. It doesn't have to be pixel-perfect
• Any new code has reasonable code coverage
  • PHP unit tests for new PHP parts
  • E2E tests for the main interaction
    Discovery around feasibility of e2e tests is enough depending on complexity, perhaps laying some groundwork (e.g. write mock plugin). We could always add these tests later.
• Any new components have been tested with a11y and i18n best practices in mind
• Ensure show_post_locked_dialog filter is applied/respeted (as used in e.g. wp-admin/edit-form-blocks.php)

Implementation Brief

• • Editor: Pass necessary post locking information in Story_Post_Type.php so it's available to editor (e.g. lock status)
• • Hook into Heartbeat API to send & retrieve post lock information.
  • Use addAction from @wordpress/data instead of jQuery

Considerations:

• Post locking/unlocking state in the editor could be handled in story context provider (is locked, is taken over, release lock, etc.)

[bmattb]

might need a seperate ticket to update the story list to indicate that there is a edit in progress similar to how wp currently does it.

[spacedmonkey]

It is also worth noting that a locked post appears in the list page as above. If we implement locking, then will likely need to implement this message in dashboard as well. Part of the point of the lock is so you can see if before you into it.

If it is implement in the dashboard, WordPress heartbeat api will need to be used to refresh the locks every 60 seconds, how it works on the lst page.

You think that this functionality is part locking @o-fernandez ?

[allan23]

Implementation Proposal

Initial Edit Story Page Load

When the edit story page is accessed by the end-user, the story (post) will be checked whether or not it is locked by another user. The results of this check are then localized into the JS config object (webStoriesEditorSettings.config.lockStatus) for use by the React App.

Example:

/**
 * Determine if story is locked by another user.
 *
 * @param int $story_id The Post ID of the Web Story.
 *
 * @return array
 */
function get_lock_status( int $story_id ) {
	// Check if locked by current user.
	$current_user_id       = get_current_user_id();
	$post_locked           = wp_check_post_lock( $story_id );
	$lock_status['locked'] = false;

	if ( false === $post_locked ) { // Post is unlocked.
		// Lock it.
		wp_set_post_lock( $story_id );
	} else { // See who is locking it.
		$user         = get_userdata( $post_locked );
		$user_details = array(
			'name' => $user->display_name,
		);
		$avatar       = get_avatar( $user->ID, 64 );
		if ( $avatar ) {
			if ( preg_match( "|src='([^']+)'|", $avatar, $matches ) ) {
				$user_details['avatar'] = $matches[1];
			}
		}

		$lock_status['locked']      = true;
		$lock_status['userDetails'] = $user_details;
	}

	return $lock_status;
}

Heartbeat API Usage

Utilizing the Heartbeat API, we can send the storyId and run another post lock check server-side. If the post is not already locked by another user it will be locked by the current user.

Example (jQuery will not be used in actual implementation):

jQuery(document).on('heartbeat-send', function (event, data) {
  // Add additional data to Heartbeat data.
  data.storyId = window.webStoriesEditorSettings.config.storyId;
});

Example of server-side handling:

add_filter( 'heartbeat_received', function ( $response, $data, $screen ) {
	if ( 'web-story' === $screen && ! empty( $data['storyId'] ) ) {
		$lock_status            = [];
		$story_id               = absint( $data['storyId'] );
		$response['lockStatus'] = get_lock_status( $story_id );
	}

	return $response;
}, 10, 3 );

The lockStatus object will be replaced with the latest data from the Heartbeat response.

Example (jQuery will not be used in actual implementation):

jQuery(document).on('heartbeat-tick', function (event, data) {
  if (!data.lockStatus) {
    return;
  }
  // Cache current lock status.
  window.webStoriesEditorSettings.config.lockStatus = data.lockStatus;
});

[spacedmonkey]

I have two questions here.

I am wondering here, if we even need to use heartbeat library at all here. For some so simple, could we have use a our own timer ( like setTimeout) to call check lock every 60 seconds? What benefit does the heart beat add here?

How does a story get unlocked? If a user navigates away from the story, how is a unlock story unlocked?

I am wondering, if we could do something simpler.

Create a new REST API with three endpoints.

/web-stories/v1/post-lock

Get endpoint

GET - /web-stories/v1/post-lock/<id>
Get the post lock, similar to get_lock_status function listed above.

POST - /web-stories/v1/post-lock/<id>
Set post lock use wp_set_post_lock

DELETE - /web-stories/v1/post-lock/<id>
Removes post lock ( may not be required ).

With these endpoints, we could call a GET request, early in the editor bootstrap. If there is a lock is setup, use that data to render the locked dialog.

If not lock is set, call the POST endpoint, to set lock, if one is not set already.
Set timer, to call POST endpoint, to refresh post lock every 60 seconds.

If user navigates away from editor, call remove lock DELETE endpoint.

That seems a little simpler to me and doesn't require loading jquery and heartbeat on the page.

I will admit to not knowing how heartbeat really works and if we need it here.

[swissspidy]

Thanks both for your comments! Certainly a start, although it sounds like we need to reach some consensus on some of the more fundamental parts of the implementation.

Feel free to kick off some discussion on Slack or some meeting for this.

Also, we need to keep the ACs in mind. There's more to this than just reading and writing a lock.

When the edit story page is accessed by the end-user, the story (post) will be checked whether or not it is locked by another user. The results of this check are then localized into the JS config object (webStoriesEditorSettings.config.lockStatus) for use by the React App.

window.webStoriesEditorSettings.* is to be considered read only. It is used for passing initial data to the application, but should not be updated by the app.

function get_lock_status( int $story_id ) { ... }

FWIW this can be simplified a bit. No need for preg_match stuff. get_avatar_url() is a thing. Check wp-admin/edit-form-blocks.php for how it can be done.

The lockStatus object will be replaced with the latest data from the Heartbeat response.

Here's what I suggested in the ticket description originally:

Post locking/unlocking state in the editor could be handled in story context provider (is locked, is taken over, release lock, etc.)

Example of server-side handling:

Don't think we need that part. wp_refresh_post_lock already runs on heartbeat_received

I am wondering here, if we even need to use heartbeat library at all here. For some so simple, could we have use our own timer ( like setTimeout) to call check lock every 60 seconds? What benefit does the heart beat add here?

The Heartbeat API can also be used by other plugins that might modify data that is sent and received, and the heartbeat interval can be changed by users as well. Using this existing platform would ensure greater compatibility.

We can of course also try doing this directly, but for a first MVP solution it seems safest to just use this pre-existing solution

How does a story get unlocked? If a user navigates away from the story, how is a unlock story unlocked?

One needs to hook into the beforeunload event to remove the lock.

I am wondering, if we could do something simpler.
Create a new REST API with three endpoints.

Not sure I would call that simpler per se :-) This significantly increases complexity & required estimate.

Not to mention that it doesn't cover all aspects:

• Check lock
• Refresh lock
• Acquire lock (take over post)
• Release lock

Not even in WordPress core was there agreement on what such a REST API could look like.

If we can come up with a sound API design for a next iteration of this, then great.

That seems a little simpler to me and doesn't require loading jquery and heartbeat on the page.

Nobody suggests using jQuery. To listen to Heartbeat events, @wordpress/data can be utilized.

Aside: this requirement of using @wordpress/data for Heartbeat is the only thing that bugs me about Heartbeat, really. But again, for this MVP solution I think it's acceptable.

[allan23]

After meeting with @spacedmonkey and discussing options, I feel that we should revisit the REST API endpoints approach instead of using the Heartbeat API.

I say this because the Heartbeat API uses admin-ajax and has been known to be a performance bottleneck for some hosting providers. While my initial intuition has been to do things the "WordPress way", I feel like the Heartbeat API is likely going to be refactored in core to utilize the WP REST API instead. Following this approach and writing our own post locking endpoints allows us to have better control of what gets processed server-side.

It also opens the door for expanding the plugin outside of the WordPress space by not making it too WordPress dependent.

[swissspidy]

If you both are confident that it makes sense, we can certainly give it a try.

But this would need a clear proposal for the REST API design (needs to cover all use cases, as I already mentioned), and then a design brief brief for the actual implementation in the editor leveraging said API.

[spacedmonkey]

Doing a little research these seem to be the functions related post locking

wp_check_post_lock
wp_set_post_lock
wp_refresh_post_lock
wp_check_locked_posts
wp_ajax_wp_remove_post_lock

With a crud api, I think we could cover these use cases.

• Check lock - GET
• Refresh lock - POST
• Acquire lock (take over post) - POST
• Release lock - DELETE

@allan23 Can you draw up a REST API design document ( in google docs ), with endpoints, params and responses. Then write up how you plan to map core functionality like refreshes to the endpoints. From there, we should able to spot things missing / not yet mapped.

We have two options with post locking. If can either be part of the existing post type endpoint, /web-stories/v1/web-stoires/<id> and use register_meta to expose this field or a custom endpoint. I believe a custom endpoint is cleaner, but I can be talked about of it.

I also believe that this endpoint, should using embedding. So a get endpoint could look like this.
/web-stories/v1/post-lock/233?_embed=user

"_links": {
    "author": [
      {
        "embeddable": true,
        "href": "https://www.spacedmonkey.com/wp-json/wp/v2/users/1"
      }
    ],
}

Meaning you don't have to pick and choose fields for the user object, just embed it.

[spacedmonkey]

Regarding preview links, you might want to take a look at this PR - #6037

[swissspidy]

I already linked to it above, but in case you've missed it: https://core.trac.wordpress.org/ticket/44862

[spacedmonkey]

This doesn't seem that different from what I suggested.

Check Lock
GET wp/v2/{base}/{id}/lock
User ID
Acquired
Expires
Aquire/Takeover a Lock
POST wp/v2/{base}/{id}/lock
Update a lock
Heartbeat implementation rejects this if the lock’s user has changed.
PUT wp/v2/{base}/{id}/lock
WP_Error 409 if user ID of current lock !== current user
Release a lock
DELETE wp/v2/{base}/{id}/lock

The big difference is the base is different, but I like basing the web-stories endpoint. So it would result in

GET - /web-stories/v1/web-story/<id>/lock
POST - /web-stories/v1/web-story/<id>/lock
DELETE - /web-stories/v1/web-story/<id>/lock

It would mean just need endpoints and not a new router, which is pretty neat.

[allan23]

Implementation Brief

Initial Edit Story Page Load

When the edit story page is accessed by the end-user, the story (post) will be checked whether or not it is locked by another user. The results of this check are then localized into the JS config object (webStoriesEditorSettings.config.lockStatus) for use by the React App.

WP REST API Usage

Utilizing a custom endpoint (/web-stories/v1/post-lock/<id>/) periodically, we can refresh and retrieve the post lock status every 60 seconds. This custom endpoint uses similar functionality as the HeartBeat API without the added load associated with admin-ajax.

React Component

The PostLock React component will query the custom endpoint and update the display accordingly. If the post is locked by another user, a Dialog component will appear showing who has control of the post/story.

Additional actions, similar to WordPress core, such as Go Back, Preview, and Take Over are to be added.

[swissspidy]

Thanks @allan23! This, together with the draft at #6314, makes it more clear which route this can take.
We still need to flesh out the Take Over part, which seems critical.

[spacedmonkey]

@swissspidy Do you have any recommendations on how to delete the lock on existing editor.

Set lock when opening editor, remove lock upon closing

Using usePreventWindowUnload didn't work, as it meant that everytime you exist the editor, it shows you this message.

This is not an expected behaviour.

The only time we really know a user is navigating away the editor is when the press this button.

Seems a little pointless to delete the lock at this point.

[swissspidy]

@swissspidy Do you have any recommendations on how to delete the lock on existing editor.

Set lock when opening editor, remove lock upon closing

Using usePreventWindowUnload didn't work, as it meant that everytime you exist the editor, it shows you this message.

This is not an expected behaviour.

The only time we really know a user is navigating away the editor is when the press this button.

Seems a little pointless to delete the lock at this point.

useEffect(() => {
  function releasePostLock() {
    // ...
  }

  window.addEventListener('beforeunload', releasePostLock);

  return () => {
    window.removeEventListener('beforeunload', releasePostLock);
  };
});

[spacedmonkey]

  useEffect(() => {
    async function releasePostLock() {
      if (enablePostLocking && postLockEnabled) {
        await deleteStoryLockById(storyId);
      }
    }

    window.addEventListener('beforeunload', releasePostLock);

    return () => {
      window.removeEventListener('beforeunload', releasePostLock);
    };
  }, [deleteStoryLockById, storyId, enablePostLocking, postLockEnabled]);

Do the issue with this is, that we trigger an async call to the REST API. But the issue is, if we dont want around for the reason / even wait for the request to be made, the page navigates away before the request is made. If the request is not made, then the lock is deleted. I am not sure that is this possible using beforeunload. Am I missing something?

[spacedmonkey]

If you want to see the code in action. https://github.com/google/web-stories-wp/tree/try/post-locking-with-delete

[maxyinger]

Maybe I'm missing some context, but this sounds like we may want to wait for some async action to occur before an action that results in an update to the component tree. Maybe something like:

const releasePostLock = useCallback(() => {
  ...
  someAsyncAction().then(componentTreeUpdateAction)
}, [])
...
onClick={releasePostLock}

Also looking into the implementation a bit, might be nice to set some safeguards to prevent state updates from happening in the scenario that a component has unmounted before a promise has resolved

[spacedmonkey]

Maybe I'm missing some context, but this sounds like we may want to wait for some async action to occur before an action that results in an update to the component tree. Maybe something like:

The issue here is about removing the lock when the user navigates away. The releasePostLock is not called unless you navigation from away from the editor. At this point, all we care about it that request is fired and completed and not about the rest of the app.

[swissspidy]

  useEffect(() => {
    async function releasePostLock() {
      if (enablePostLocking && postLockEnabled) {
        await deleteStoryLockById(storyId);
      }
    }

    window.addEventListener('beforeunload', releasePostLock);

    return () => {
      window.removeEventListener('beforeunload', releasePostLock);
    };
  }, [deleteStoryLockById, storyId, enablePostLocking, postLockEnabled]);

Do the issue with this is, that we trigger an async call to the REST API. But the issue is, if we dont want around for the reason / even wait for the request to be made, the page navigates away before the request is made. If the request is not made, then the lock is deleted. I am not sure that is this possible using beforeunload. Am I missing something?

Oh, then you'd want navigator.sendBeacon instead of a regular fetch().

See https://github.com/WordPress/gutenberg/blob/a42aeea340acf58fcb3e91da0e1c77b1cddf4986/packages/editor/src/components/post-locked-modal/index.js#L106-L127 for comparison (ignore the window.XMLHttpRequest part)

[spacedmonkey]

Moving back to in progress, as need to add tests.

[spacedmonkey]

I posted on WordPress core trac ticket, with the offer to port this change to core.