Locking support

[avoinea]

See #1522 and https://github.com/plone/plone.restapi/pull/1182

[tiberiuichim]

What about building the toasts on top of https://github.com/plone/volto/pull/2581 ?

[avoinea]

It depends on which one gets in first :smile:

[tisto]

@avoinea thanks for working on this! I gave it a shot and enabled locking in the config.js in Volto. I manually checked out the plone.restapi branch for locking support.

Then I created a second user "timo" and did the following:

1. create page "my page" as user "admin" and open it in edit mode
2. login as user "timo" and edit "my page"
3. -> no warning message pops up

In addition, the following happens:

1. create page "my page" as user "admin" and open it in edit mode
2. login as user "timo" and edit "my page"
3. leave page
4. edit "my page" as "admin
5. -> I see a message that the file is locked and can not be edited (even though the current user is the creator)

and the following:

1. create a few pages as "admin" and "timo" to try out the feature
2. try to delete (bulk actions) all files as user "admin" in the folder_contents view
3. -> clicking on the delete button has no effect at all. backend throws a 423 HTTP error that does not show up in the frontend

I would recommend that we check out the p.restapi branch in this Volto branch and enable locking by default. Then we should write a set of Cypress tests to make sure things work as expected. Those tests would also help us to be on the same page when we talk about the feature and what exactly we expect the system to do.

I only did a quick test, because officially I am on holiday and should stay away from my computer, maybe I missed something...

[avoinea]

@tisto Thank you for testing the detailed feedback.

I did all my tests and dev by using 2 Plone users. I will check what's wrong with Zope admin user.

Good catch with the missing unlock trigger on window.unload

[tisto]

@avoinea for further discussion I drafted three user scenarios to described the functionality I would expect. It would be great if we could write Cypress tests for those three scenarios. Maybe my collegues @iFlameing @jackahl or @ThomasKindermann are willing to help with writing those scenarios.

Scenario: As editor, a page is locked for other users when I edit that page
Given a logged-in editor
 When I edit an existing page
 Then the page is locked for other users

Scenario: As editor, I can see when a page is currently edited by another user
Given a logged-in user
  and a page that is currently edited by another user
 When I view the page
 Then I see the edit button is greyed out
  and I see a message that this page is currently logged

Scenario: As editor, I can unlock a locked page
Given a logged-in site-administrator
  and a page that is currently edited by another user
 When I click the "unlock" button
 Then I can edit the page

[avoinea]

Scenario: As administrator, I can unlock a locked page Given a logged-in site-administrator and a page that is currently edited by another user When I click the "unlock" button Then I can edit the page

Currently, in Plone, any other editor can unlock a locked content. I don't see why we should limit it site admin, only.

[tisto]

@avoinea I amended the scenario accordingly.

[avoinea]

Cypress in-place

[avoinea]

@sneridagh @tisto Exactly what I was afraid by enabling it by default, guillotina tests :smile:

[sneridagh]

Guillotina tests passes it seems, right? What's the issue now? Also, we do need the setting or we just want it "always on"?

[avoinea]

Funky :thinking:

https://user-images.githubusercontent.com/323385/129888008-72424ee8-1f8c-4943-adae-4603d8e0c99a.mp4

[avoinea]

Also, we do need the setting or we just want it "always on"?

:thinking: Would be nice to have a smart check for locking support. Maybe we can look into the enabled behaviors on context.

[avoinea]

@sneridagh @tisto https://github.com/plone/plone.restapi/pull/1182#issuecomment-901036255

[avoinea]

I'll be offline until Monday 23 Aug and this PR needs some adjustments according to https://github.com/plone/plone.restapi/pull/1201

[avoinea]

@sneridagh @tisto I think this is ready and complete now.

[tisto]

Funky 🤔

Locking-support-multilingual-error.mp4

@avoinea is this issue with locking and multilingual fixed?

[avoinea]

@avoinea is this issue with locking and multilingual fixed?

@tisto It is.

Still, now it doesn't seem possible to edit a translation anymore.

[avoinea]

Still, now it doesn't seem possible to edit a translation anymore.

Hmm, It seems to be related with the way I was starting frontend/backend as the error was CORS related.

Maybe someone with a multilingual enabled could give it a try. @nzambello?!

[avoinea]

Mystery solved: We need to add also Lock-Token to CORS zcml allow_headers when backend is at the different URL than frontend:

cors-overrides.zcml

<configure
  xmlns="http://namespaces.zope.org/zope">
  <configure
    xmlns="http://namespaces.zope.org/zope"
    xmlns:plone="http://namespaces.plone.org/plone">
    <plone:CORSPolicy
      allow_origin="http://localhost:3000,http://127.0.0.1:3000"
      allow_methods="DELETE,GET,OPTIONS,PATCH,POST,PUT"
      allow_credentials="true"
      expose_headers="Content-Length"
      allow_headers="Accept,Authorization,Content-Type,Lock-Token"
      max_age="3600"
     />
  </configure>
</configure>

[tisto]

@avoinea uh. glad we could solve this. I guess we need to update our documentation, Volto integration packages, etc.

@sneridagh do we have a breaking release in the pipeline? This would allow us to put this into the breaking changes documentation (even though it is technically not a breaking change but a new feature).

[avoinea]

@tisto Yes, this https://github.com/plone/volto/pull/2594/commits/5b62b9b15657f5015f0df77f1697063d20eec55a should be added everywhere

[sneridagh]

@tisto yes, 14 should go in the next days. We can include it in there.