ether / etherpad-lite

Etherpad: A modern really-real-time collaborative document editor.
http://docs.etherpad.org/
Apache License 2.0
16.02k stars 2.79k forks source link

Add option to restore revisions #1791

Closed osamak closed 9 years ago

osamak commented 11 years ago

It would be a very important addition to the revision system to add the ability to restore previous revisions. It was mentioned in issue #229, but the ticket was closed.

[Explanatory note: if a restoration takes place it should happen as a document change and not as a reset back to rev x.]

JohnMcLear commented 11 years ago

This should be a plugin only the site admin should be able to restore to a previous revision OR if a restoration takes place it should happen as a document change and NOT a rewrite back to rev 0.

It should be relatively easy to write btw, the functionality to view contents at a specific rev exists already, it's just a case of adding a button and a function to import the document state at a given revision (note you need to keep full doc fidelity IE authors and previous rev states). Also note doing this process needs to tell the clients it happened (like an import) so that modifications are kept sane.

nemobis commented 10 years ago

Sorry, I don't get it: why only an admin? I'm a wiki user like osamak, so perhaps it's our own bias, but there is little purpose in having history if you can't bring the document back to a previous version. If I remember correctly the original etherpad had this feature, I used to restore saved/featured revisions from the timeslider.

JohnMcLear commented 10 years ago

If anyone could restore a version you would destroy the fidelity of a pad.

You could "fork" the pad at a specific revision, creating a new PadID.

Etherpad never forgives, Etherpad never forgets.

If you want this functionality I'd suggest referring to the ep_copypad plugin and just modify that, it's a tiny job to create the functionality you want btw.

nemobis commented 10 years ago

"If anyone could restore a version you would destroy the fidelity of a pad": can you elaborate this? I don't get it. I can blank a pad, nobody can easily restore it as it was before I blanked it; is this fidelity?

JohnMcLear commented 10 years ago

Ugh, I thought this was kinda obvious..

Revision 1 = Hello world Revision 2 = Hello world again Revision 3 = Oh my *RESTORED VERSION = Revision 4 = Hello world

Here you restored to state 1 but it's actually Revision 4, we never remove Revision 1, 2 or 3

In Etherpad at current if you blank a pad the previous revision changes are still there as previous document Revisions and we also keep individual changesets.

If you restore a pad the way you want then fidelity is destroyed and the pad can not be restored to Revision 2 or 3. I strongly recommend against doing this as it means you lose pad history and allows for document distortion. Implement a forking mechanism (using ep_copypad).

Alternatively go a head with what you want, but write a plug-in to do and be aware you are destroying pad history integrity.

I hope that clears it up for you.

marcelklehr commented 10 years ago

So, it's a valid concern -- ... that I'd put into a plugin.

JohnMcLear commented 10 years ago

Again to emphasize, afaik this functionality exists in ep_copypad. https://github.com/redhog/ep_copypad

Feel free to check :)

nemobis commented 10 years ago

Nope, I don't want to fork pads.

"Here you restored to state 1 but it's actually Revision 4, we never remove Revision 1, 2 or 3": yes, so what's the problem?

marcelklehr commented 10 years ago

as I understand it, john pointed out that it would be naive to expose history rewriting functionality to all users. However, taking the contents of any revision and creating a new revision on top of the pad's history could be allowed for anyone.

That's how I understand John's first comment. However, it seems nemobis thought, john meant that only admins should be allowed to both rewrite the pad's history and restore a specific revision. Which is definitely useless.

So, no problem at all. (Except that noone wants to fork a pad... ;) )

JohnMcLear commented 10 years ago

Yep exactly that.

nemobis commented 10 years ago

Wonderful. Nobody asked history rewriting here but it seems it wasn't clear enough; if someone with the power to edit the bug summary clarified this, it would be helpful.

bee-keeper commented 9 years ago

"However, taking the contents of any revision and creating a new revision on top of the pad's history could be allowed for anyone".

Does this functionality exist anywhere? I don't see why a user shouldn't be able to do this. Also, forking a pad doesn't make sense when all you need to do is create a new revision.

JohnMcLear commented 9 years ago

It shouldn't exist because in theory you could destroy a pads linear state. ep_copy_pad is the closest thing afaik

bee-keeper commented 9 years ago

Isn't that like having a revision system which doesn't allow you to actually revert?

In this case I'm not sure I see the point in letting users save revisions - can they do anything with them or are they simply lulled into thinking they can go back to some previous state?

(By revert I mean to create a new revision from a previous revision.)

JohnMcLear commented 9 years ago

I think you are missing the point here.. If you want it, make it as a plugin, it's up to you how your revision state is managed. I'm simply explaining why that functionality is not in core.

bee-keeper commented 9 years ago

No, I'm happy to make this as a plugin, in fact I probably will have to now :)

I'm interested in exactly what you mean by 'because in theory you could destroy a pads linear state' - how in theory could you do this by creating a new version from a previous revision?

Thanks for taking to the time to answer by the way, I'm just trying to scope out the problem here.

marcelklehr commented 9 years ago

From a vandalism point-of-view, I think non-destructive reverting can cause the same level of damage as importing rubbish, or deleting the entire pad content by hand -- thus I think it's relatively uncritical to implement it in core since those two methods to vandalism are way simpler.

Conceptually, it should be similar to import and setText, I think, so I don't see a problem there. From a UX perspective we might want to consider adding some marker that shows the reverting edit or sth.

pdinc-oss commented 9 years ago

I started naively by

exportHtml.getPadHTML(pad, rev, function(err, html)
{
  importHtml.setPadHTML(pad, html, callback);
  padMessageHandler.updatePadClients(pad, callback);
}

but I quickly discovered the import an export looses fidelity.

Now I am looking to call appendRevision(aChangeset, author), but I need to make a changeset from a getInternalRevisionAText

Any suggestions?

pdinc-oss commented 9 years ago

Why does this happen only on every 100 revisions?

if(newRev % 100 == 0)
{
  newRevData.meta.atext = this.atext;
}
marcelklehr commented 9 years ago

ImportHtml uses setText("") (essentially creates a blank sheet of paper) before appending the new content as a revision. What's the problem with that?

The atext property is actually only set for performance reasons. In order to save memory, only the changes for each revision are stored. In order to see the full contents of the revision you need to apply the previous changesets on top of each other, which is what getInternalRevisionAText does.

pdinc-oss commented 9 years ago

There is no problem with that, it is just that the formatting gets lost.

pdinc-oss commented 9 years ago

I have been looking at the Changeset Builder, but I am not sure how I should be building it from an existing atext and attributes. I was hoping there was a Changeset.from(v1,v2) function.

pdinc-oss commented 9 years ago

A side note: Ctrl-A, Ctrl-C from the time slider then Ctrl-A, Ctrl-V works for my simple test document.

marcelklehr commented 9 years ago

Yea, I think stuff like lists are causing problems. Nope there's no diff yet. It shouldn't be necessary, too.

Atext looks like this {text: 'fooo', attribs: '|5*0+1|etc.pp.'}, where attribs is actually a list of insert ops. Now, you should be able to string together a changeset like this:

cs = "Z:"+"1"+">"+changeset.numToString(atext.text.length)+""+atext.attribs+"$"atext.text

Where Z is nothing. 1 is the old length, > indicates whether the document shrank or grew, atext.attribs is the set of insert operations and everything after $ are the characters being inserted.

Since atext.text.length might cause problems, you can also iterate over the ops and use a builder to create a changeset as seen in ImportHtml.js

marcelklehr commented 9 years ago

It might be a good idea to refactor and take that part out of ImportHtml and put it into a new Pad method, like Pad#setAtext

pdinc-oss commented 9 years ago

essentially any change set can start from nothing (Z:0) and clobber the cumulative changes.

pdinc-oss commented 9 years ago

I will put it in Pad then, I was adding restore(padID,rev) in the API.

marcelklehr commented 9 years ago

An API endpoint would definitely be nice, of course.

Re Z: What I meant is, it has no meaning, it's more like "this is a changeset". Every changeset starts with "Z".

nemobis commented 9 years ago

So, to followup, what revert features are available now to users and since what release? The commit seems to stay admin powers are required, which would be useless for etherpad.wikimedia.org.

dcht00 commented 8 years ago

For anyone just looking to simply restore contents of a pad - see "ep_cristo_restore_revision" plugin. Install via /admin.

When installed, you get a button in Timeslider, or you can also restore with a link, for example: http://sometherpad.org/restore/[pad_name]/[revision_no]

lstandish commented 3 years ago

I'm a new Etherpad user, and I see a great need to be able to set the current pad content to what was in some past revision. As pointed out in this discussion, this is not "reversion," history is not lost. Without this, I think the version history, including all the cool functionality of the "timeslider," is pretty much wasted.

I'm at the current version of etherpad-lite as-of this writing (develop branch), and I installed ep_cristo_restore_revision via /admin. I don't get any button in Timeslider, and restore with a link hangs. I hope one of the developers will look at this. If I can provide any debug information, I'd be glad to.

JohnMcLear commented 3 years ago

@lstandish please post on plugin GitHub repository not here.

dcht00 commented 2 years ago

You can since version 1.2.13, if you have the key, do it via an API:

$ curl "https://example.org:9001/api/1.2.13/restoreRevision?apikey=SECRET&padID=yourpad&rev=1337"

See the API documentation for more info.