This repository has been archived by the owner. It is now read-only.

Add the edit lock to the post objects #2225

Open
joehoyle opened this Issue Feb 3, 2016 · 11 comments

Comments

Projects
None yet
7 participants
@joehoyle
Contributor

joehoyle commented Feb 3, 2016

In context=edit it would be useful to know if the post is currently being edited already elsewhere. This is know as the Edit Lock in WordPress. I suggested we add a locked attribute to the post in the context=edit case. That way clients can update this field to add a lock, or not allow editing of it if there is already a lock on it.

@WP-API/amigos thoughts?

@joehoyle joehoyle added this to the 2.0 milestone Feb 3, 2016

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Feb 3, 2016

Member

That way clients can update this field to add a lock, or not allow editing of it if there is already a lock on it.

If there's already a lock on it, how would a client know whether it's their request that locked it?

Also, would the lock be discarded after some period of time? Or, how would you know whether it was safe to ignore the lock?

Member

danielbachhuber commented Feb 3, 2016

That way clients can update this field to add a lock, or not allow editing of it if there is already a lock on it.

If there's already a lock on it, how would a client know whether it's their request that locked it?

Also, would the lock be discarded after some period of time? Or, how would you know whether it was safe to ignore the lock?

@joehoyle

This comment has been minimized.

Show comment
Hide comment
@joehoyle

joehoyle Feb 3, 2016

Contributor

The lock expiring is already built into wordpress, for who locked it, I'll look into how core handles it. I think it's probably got the user id in the meta.

Contributor

joehoyle commented Feb 3, 2016

The lock expiring is already built into wordpress, for who locked it, I'll look into how core handles it. I think it's probably got the user id in the meta.

@beatelite

This comment has been minimized.

Show comment
Hide comment
@beatelite

beatelite Feb 18, 2016

So glad this is being discussed. I just ran into this as an issue. It would also be cool if we could see who's editing it, if possible.

beatelite commented Feb 18, 2016

So glad this is being discussed. I just ran into this as an issue. It would also be cool if we could see who's editing it, if possible.

@aaronjorbin

This comment has been minimized.

Show comment
Hide comment
@aaronjorbin

aaronjorbin Feb 18, 2016

Contributor

This might be a great opportunity to use the infrastructure that was merged into core already and migrate the existing heartbeat check over to the REST API

Contributor

aaronjorbin commented Feb 18, 2016

This might be a great opportunity to use the infrastructure that was merged into core already and migrate the existing heartbeat check over to the REST API

@kadamwhite

This comment has been minimized.

Show comment
Hide comment
@kadamwhite

kadamwhite Sep 27, 2016

Contributor

The two options that have been proposed so far:

  1. We add a locked attribute to the post in the context=edit case. That way clients can update this field to add a lock, or not allow editing of it if there is already a lock on it.
  2. We ignore edit locking in the REST API until a better solution is proposed.
Contributor

kadamwhite commented Sep 27, 2016

The two options that have been proposed so far:

  1. We add a locked attribute to the post in the context=edit case. That way clients can update this field to add a lock, or not allow editing of it if there is already a lock on it.
  2. We ignore edit locking in the REST API until a better solution is proposed.
@aaronjorbin

This comment has been minimized.

Show comment
Hide comment
@aaronjorbin

aaronjorbin Sep 27, 2016

Contributor

@getsource as one of the original authors of the Heartbeat, I would love to know your thoughts here.

Contributor

aaronjorbin commented Sep 27, 2016

@getsource as one of the original authors of the Heartbeat, I would love to know your thoughts here.

@getsource

This comment has been minimized.

Show comment
Hide comment
@getsource

getsource Sep 28, 2016

I'm thinking on what the best solution is, but in the meantime, @joehoyle is correct -- both the user ID and the time when the post was locked get saved in postmeta:
https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/post.php#L1503

I'd love if we can move to using the REST API for Heartbeat, but we'd probably want to move all of the functions over (or at least the primary Heartbeat endpoint), rather than just post locking on its own, since the data is batched for performance reasons.

getsource commented Sep 28, 2016

I'm thinking on what the best solution is, but in the meantime, @joehoyle is correct -- both the user ID and the time when the post was locked get saved in postmeta:
https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/post.php#L1503

I'd love if we can move to using the REST API for Heartbeat, but we'd probably want to move all of the functions over (or at least the primary Heartbeat endpoint), rather than just post locking on its own, since the data is batched for performance reasons.

@rmccue

This comment has been minimized.

Show comment
Hide comment
@rmccue

rmccue Sep 28, 2016

Member

FWIW, I don't think Heartbeat itself is a good candidate for the REST API; it's not a good conceptual fit, and provides a totally different API of its own. In any case, that's really a different discussion; really, all we need is a way of exposing the lock, if we want to expose it at all.

Member

rmccue commented Sep 28, 2016

FWIW, I don't think Heartbeat itself is a good candidate for the REST API; it's not a good conceptual fit, and provides a totally different API of its own. In any case, that's really a different discussion; really, all we need is a way of exposing the lock, if we want to expose it at all.

@kadamwhite

This comment has been minimized.

Show comment
Hide comment
@kadamwhite

kadamwhite Sep 28, 2016

Contributor

Per discussion in Dev Chat on 2016-09-28 this is not a feature which needs to be addressed in 4.7; bumping to the 2.1 milestone. There is some great discussion started in the linked slack chat, which will be continued & expanded as we enter the next dev cycle.

Contributor

kadamwhite commented Sep 28, 2016

Per discussion in Dev Chat on 2016-09-28 this is not a feature which needs to be addressed in 4.7; bumping to the 2.1 milestone. There is some great discussion started in the linked slack chat, which will be continued & expanded as we enter the next dev cycle.

@kadamwhite kadamwhite modified the milestones: 2.1, 2.0 Sep 28, 2016

@getsource

This comment has been minimized.

Show comment
Hide comment
@getsource

getsource Sep 28, 2016

Completely agreed that this does not need to happen for 4.7. Longer term, I agree it'd be helpful to expose the lock, and it's a separate discussion from whether Heartbeat would/wouldn't take advantage of the REST API.

getsource commented Sep 28, 2016

Completely agreed that this does not need to happen for 4.7. Longer term, I agree it'd be helpful to expose the lock, and it's a separate discussion from whether Heartbeat would/wouldn't take advantage of the REST API.

@rmccue

This comment has been minimized.

Show comment
Hide comment
@rmccue

rmccue Sep 28, 2016

Member

Just as a note to the future, some quick thoughts on how we could do locking: edit locks are probably best implemented via a custom action, something like /posts/%d/lock as a write-only endpoint. This would set a lock key on the post, which would contain when the lock started, when it expires, and who owns it (this field would be nullable; if updated with a null value, this would clear the lock).

Member

rmccue commented Sep 28, 2016

Just as a note to the future, some quick thoughts on how we could do locking: edit locks are probably best implemented via a custom action, something like /posts/%d/lock as a write-only endpoint. This would set a lock key on the post, which would contain when the lock started, when it expires, and who owns it (this field would be nullable; if updated with a null value, this would clear the lock).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.