PLIP 240 (Improve locking configurability) has been implemented.  You
can get the review buildout from:
https://svn.plone.org/svn/plone/review/plip240-locking-improvements/

I'm going to paste the contents of
https://svn.plone.org/svn/plone/review/plip240-locking-improvements/PLIP_240_README.txt
below for ease of any discussion.  Please take careful note of the
"Other" section below.

Thanks!

Andrew (sending on behalf of David)


PLIP 240: Improve locking configurability
=========================================

This review buildout presents the changes that have been made in order
to implement PLIP 240.  The goal of this PLIP is to give more flexibility
in configuring Plone's lock-on-edit behavior.

One of the original aims of this PLIP was to reduce the pain that has been
occurring in the Plone 3.x series when items get inadvertently locked and
then accidentally left in that state.  A late-October fix in Archetypes
(http://dev.plone.org/archetypes/changeset/10163, which made it into Plone
3.2) already helped greatly with this by making sure that the
unlockOnFormUnload.js script also runs for inline edits.  However, that
script doesn't work in Webkit-based browsers, nor in case of things like
browser crashes.  So we have gone ahead with the plan to change the default
lock timeout to 5 minutes and keep it alive via a periodic AJAX call, as
well as adding a global lock-on-edit on/off switch.  The unlockOnFormUnload.js
also continues its old behavior, as it is still useful when it works.


Summary of changes
------------------

 * plone.locking:

   - Changed the default timeout for through-the-web locks to 10 minutes.
   - Added a refresh_lock method to the ILockable interface and to the default
     implementation, TTWLockable
   - Added a publishable refresh_lock method to the
plone_lock_operations browser
     view
   - Made the lock method from ITTWLockable check the lock_on_ttw_edit setting

 * Plone:

   - Added lock_on_ttw_edit property to site_properties, with a
default value of True
     to match the behavior of Plone <3.3
   - Adjusted unlockOnFormUnload.js to also call the refresh_lock
method every 5 minutes
     (for forms with the enableLockProtection class only)
   - Make sure the 'removeLockProtection' KSS action fires when someone cancels
     inline editing, to stop calling the periodic refresh_lock

 * plone.app.controlpanel:

   - Added 'Enable locking' setting to the site configlet, which adjusts the
     lock_on_ttw_edit property
   - Changed the SiteControlPanelAdapter to also adapt ILockSettings
from plone.locking
     so that plone.locking can look up the value of the setting
without introducing
     a hard dependency of plone.locking on other parts of Plone.
(This does introduce
     a dependency of plone.app.controlpanel on plone.locking, but that
seems less bad.)

 * plone.app.kss:

   - Fixed the broken implementation of the 'removeLockProtection' KSS action


Summary of test assertions
--------------------------

 X default timeout is 10 minutes for TTW
 X site creation adds 'lock_on_ttw_edit' to site properties (value: True)
 X - (and migration)
 X plone.locking.lockable.TTWLockable has a refresh_lock method that
updates the lock time
 X there is a configlet which can edit the lock_on_ttw_edit property

Not tested yet but will be before merging:

 - the lock_on_ttw_edit property is actually taken into account by content
   (even if old locks remain from before the property was turned off)

Tested manually (because I wasn't sure how to automate it):

 - The periodic AJAX call from unlockOnFormUnload.js actually updates the lock
   expiration time every five minutes.  (This is easiest to test by changing
   the time to 5 seconds and watching the Firebug console.)
 - Locking, and the auto-refresh behavior, is correctly
activated/deactivated when
   inline editing begins/ends.

I only had time to test on Firefox and Safari, so if the reviewers can
check other
browsers I'd appreciate it.


Needed documentation changes
----------------------------

 * Documentation of the locking feature should note the new setting in the
   site configlet for enabling/disabling locking.

 * Any existing documentation of lock timeouts should be adjusted
   to note the new behavior.


Needed translations
-------------------

 - title and description for the new schema field in
plone.app.controlpanel's site.py


Backwards compatibility
-----------------------

 * Custom ILockable adapters should continue to operate without error, though
   the lock refresh will not actually take place unless they are adjusted to
   implement the new refresh_lock method of the ILockable interface.

 * Any custom monkey patches to plone.locking.lockable.MAXTIMEOUT to change
   the timeout value will break.

 * Any customizations of the unlockOnFormUnload.js script will have to
be updated.


Other
-----

There is a weird edge case in the following scenario:
 1. Manager turns off locking.
 2. User A starts editing a document
 3. Manager turns locking back on
 4. User A clicks the edit tab again
 5. User B views the document

The document should show as locked to User B, but it doesn't.  We
didn't have time to
track this down or compare to current Plone behavior, but will do so
before merging
this PLIP.

However, it should be noted that if the user in step 4 above hits view and then
the edit tab (rather than just hitting edit an additional time) the
locking viewlet behaves
as expected (e.g. the item is listed as locked) for user B in step 5.
All initial analysis
of our added code from the debugger behaved as expected, so more
investigation is
needed on our part to determine if this was introduced by us.


Credits
-------

* David Glick
* Andrew Burkhalter

_______________________________________________
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team

Reply via email to