restore undo functionality

tkimnguyen commented 9 years ago

Since Plone 3 the undo_form portal_action has been made not visible and then removed entirely from Plone. Undo is an amazingly useful feature! It is a distinguishing feature of Plone as well. Yes it's not perfect but it was really really good.

This is a PLIP following from discussion in https://github.com/plone/Products.CMFPlone/issues/783

Here are some options

[ ] restoring the undo_form (requires rewriting) with extra information explaining what may or may not work
[ ] adding some JS to check boxes for all transactions following the one you want to undo (including the ones in previous paginations)
[ ] possibly implementing this as an add-on
[ ] ensuring that a user can't undo someone else's transactions unless they have the applicable permissions on those transactions
[ ] predicting better what can and cannot be undone
[ ] displaying more accurate success/failure of undo actions

hvelarde commented 9 years ago

+1

agitator commented 9 years ago

+1

rileydog commented 8 years ago

+1

djay commented 8 years ago

Maybe put in a risks section talking about UX issues relating to how to explain why you can't undo something and/or explaining what objects can be changed by an undo

thet commented 8 years ago

I would love an undo feature, but rolling tranactions back has some problems. For example In an active site, a user changes something and soon after that another user also changes something. Then the first users transaction cannot be rolled back without the second users transaction. So the undo must be disallowed for the first user, which can lead into frustration. It's even a bit dangerous for admins. But it would be great, if a good undo feature would become reality.

tkimnguyen commented 8 years ago

@thet I thought (and could be mistaken) that a non-Manager going to the undo_form saw only their own transactions, and could not undo anyone else's transactions. For admins, sometimes being able to undo, even if it affects others, is far better than not being able to undo.

djay commented 8 years ago

Another risk you need to add to the list. Viewing the Undo form has this nasty habbit of locking the DB. Not just the currently mounted one either. For large DB's it can take a long time render and you end up with sites going down. This has happened to us before. This should probably be fixed before we make it visible in the site setup.

tkimnguyen commented 8 years ago

Any long running operation should be performed on a dedicated ZEO client. This is true of reindexing, etc.

jensens commented 8 years ago

The DB/transaction based Undo Form can be an addon, but this way of undo is not stable enough and has its glitches, so I dont want to have it in core.

@tkimnguyen If you plan to make this a real plip, please use the template http://issuetemplate.com/#/plone/Products.CMFPlone/PLIP and turn this into a real PLIP so the FWT can discuss it - otherwise please close the issue.

tkimnguyen commented 8 years ago

@jensens I don't mind creating a PLIP with that template, but I don't know what to put in the "How is this going to be implemented" field, since it seems you're opposed to re-enabling this and I'm getting the feeling @vangheem is opposed as well. Undo has been such a lifesaver to me and many others. It may not be perfect, but it sure is better than not having it. Maybe someone else has a better idea on how to restore the feature or re-implement it, but I'm not that person.

hvelarde commented 8 years ago

I saw @seanupton mentioning zc.beforestorage on a thread on community and it seems to me it must be handy to implement this feature.

seanupton commented 8 years ago

@hvelarde I think beforestorage, plus some kind of current deletion log (just needs an event subscriber to write it), would be enough stored to have a user-facing (where user is Manager role, IMHO) interface to restore content as it was before deletion, provided it is still in the history stored (e.g. my pack keeps 7 days).

@tkimnguyen I suspect that relying on undo as it was before is a bit of a dead-end, especially on a site with just enough writes to be problematic if you want to roll-back a transaction.

I am happy to help PLIP and collaborate on implementing a means of doing this (my employer's customers need this anyway), provided others are interested in providing feedback on UX and security implications.

jensens commented 8 years ago

So I close this one - the framework team is happy to review an upcoming plip.

hvelarde commented 8 years ago

@seanupton could you please make that PLIP happen so we don't lose momentum?

seanupton commented 8 years ago

@hvelarde sure, though I think I need to think/talk about scope a bit more first. Undelete is just a subset of undo, and my thinking with beforestorage is that we could support both undelete and "overwrite existing with previous copy" (on one or more items deleted or modified in a transaction).

General assumptions:

There is some global log (stored as enumeration, mappings) persisted distinct from ZODB transaction log that keeps track of path/title pairs of objects modified or deleted.
If we are trying to solve more than just undelete, we likely need to log changes for IObjectModifiedEvent and IObjectRemovedEvent.
I do not think we can store the tid of the transaction in such a log, as it is computed during commit, so fetching the tid on restore will require some introspection of the log entry to indirectly get the tid. Hopefully this is feasible and robust.

General unresolved questions to my approach:

For whom and where is the UI for undelete and restore only visible? My thinking is site-global (control panel?) and Manager or Site Administrator roles (at site scope) only, for the moment. I see this as a "power tool". If content is deleted by an administrator for, say, regulatory purposes (and it must stay gone), we do not want just any user (possibly even original owner) from restoring in some cases.
What granularity for "before" a transaction do we care about? AFAICT persistent/TimeStamp.c implements granularity of one second, so we should look one second prior. I do not think this is perfect, but I think it is what we have to work with.
Can we enumerate known (not removed by pack) history for a single content

There are things to consider when overwriting current object with previous:

When a transaction modifies more than one object, can we select just some of those objects to restore, whether objects were deleted or objects were merely modified?
Do we have configuration knobs during for:
- Restore workflow history? [Yes/No, default Yes]
- Restore original ownership role [Yes/No, default No]
- Overwrite existing content at same path? [Yes/No, default No]
- This is tricky, safe thing to do is create a copy?

@tkimnguyen any thoughts on scope?

hvelarde commented 8 years ago

@seanupton I think that's exactly the kind of discussion we must have in the scope of the PLIP; copy and paste that on the implementation and risk parts of it.

IMO, we can just start with the undelete feature as that be useful in probably more than 80% of the time.

tkimnguyen commented 8 years ago

@seanupton supporting delete is a great start and probably the main use case for regular users, though some of the "trash" functionality @vangheem has implemented elsewhere might address that better than undo.

The other use cases that I need are for changes to site configuration, like the activation or deactivation of an add-on that damages the site.

plone / Products.CMFPlone