Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OX-475] Versioned tracings / Reset button #51

Closed
tmbo opened this issue Aug 27, 2013 · 14 comments · Fixed by #3194
Closed

[OX-475] Versioned tracings / Reset button #51

tmbo opened this issue Aug 27, 2013 · 14 comments · Fixed by #3194
Assignees
Milestone

Comments

@tmbo
Copy link
Member

tmbo commented Aug 27, 2013

[reporter="autoreporter", created="Tue, 5 Mar 2013 21:44:14 +0100"]

I just wish there was an undo button for a node that has been accidentally deleted or for other such mistakes!

Reported by: Raksha Ravikumar ([email protected])

Log Time

@tmbo
Copy link
Member Author

tmbo commented Aug 27, 2013

[author="tmbo", created="Wed, 6 Mar 2013 00:58:10 +0100"]

At an earlier stage of the project (august if I remember correctly) we had a discussion about this feature and decided that a back feature is not necessary. Should we reconsider this decision?

Daniel Werner, Georg Wiese: How much effort would a general back feature cost?

@tmbo
Copy link
Member Author

tmbo commented Aug 27, 2013

[author="", created="Wed, 6 Mar 2013 01:02:10 +0100"]

My understanding at that point was that a general undo feature is hard to implement. Also it's not reallu nexessary because there are few operations that can destroy a lot of work.
Best
K

@tmbo
Copy link
Member Author

tmbo commented Aug 27, 2013

[author="georg", created="Wed, 6 Mar 2013 12:22:21 +0100"]

My guess is that for those features it would be hard and for non-critical operations it would be easy. So, if we are going to implement undo, it would probably make sense to implement a general undo, just to avoid confusions.

I've no experience with implementing undo, but it shouldn't be too hard. Maybe a day of work during the upcoming session, as a rough approximation.

@tmbo
Copy link
Member Author

tmbo commented Aug 27, 2013

[author="", created="Wed, 6 Mar 2013 13:41:04 +0100"]

I'm sorry - I still think we shouldn't do it. There are more important challenges ahead and we're still struggling to polish current Oxalis to a level where it feels finished.
To reiterate my point: In (e.g.) Word, you can press Ctrl A and then press C instead of Ctrl C, deleting everything. We don't have anything that would be nearly as destructive as that in Oxalis.
On the other hand: Tom, do you think it would be possible to not discard old saves on the server (for a while)?
If then someone calls crying that they just deleted 5 hours worth of skeleton, we could look for an earlier save.

Best,
Kevin

@tmbo
Copy link
Member Author

tmbo commented Aug 27, 2013

[author="tmbo", created="Sat, 6 Apr 2013 21:10:41 +0200"]

Currently there is only one version for each tracing. Therefore it is not possible to go back in time on the server. It is not to hard to implement this, but it would take some time (implement version saving and make the versions accessible in the front end).

@tmbo
Copy link
Member Author

tmbo commented Aug 27, 2013

[author="", created="Sun, 7 Apr 2013 00:31:28 +0200"]

I think it would be neat if this could be included in the Reset button.
So it could be: Reset to beginning, reset to one of these versions: list

Best,
Kevin

@ghost ghost assigned tmbo Aug 27, 2013
@tmbo tmbo removed their assignment Dec 11, 2015
@jfrohnhofen jfrohnhofen self-assigned this Apr 24, 2017
@fm3
Copy link
Member

fm3 commented Aug 1, 2017

I propose the backend supporting an updateAction in the format

{
  "name": "revertToVersion",
  "value": {
    "sourceVersion": 7
  }
}

which will create a new version just like any other updateAction, but with the content copied from the sourceVersion.
This way the frontend-undo does not have to send tons of individual updateActions. Redo could also be implemented by “reverting” to the version prior to the undo.
Any thoughts on this @daniel-wer @jfrohnhofen ?

@jfrohnhofen
Copy link
Contributor

I would certainly like the backend to provide this, since it allows users to revert to versions prior to their current session.
One (not necessarily related) problem that occurred to me: When users want to revert to a previous version, how do they know which version to revert to? Is it enough to display the current version somewhere or should we allow users to take named snapshots? If we used timestamps as versions, would that resolve the problem? And is that practical?

@hotzenklotz
Copy link
Member

I can think of two ways for versioning:

  1. Explicit user actions: Tagging / named snapshots
  2. Implicit detection by the server: Large scale actions (removal of X nodes in Y seconds, etc) could be detected and auto-tagged.

@daniel-wer
Copy link
Member

Just to make this clear and have everyone on the same page, Undo (Ctrl+Z) and Tracing-Version support are two "different" things to me.
If I understand correctly there will be two phases in which we'll add Undo support:

The first will be implemented as part of #1925, will be frontend-only and will only support to undo the last x changes made in the current session of the user. (This will likely cause performance problems when reverting the deletion of a very large tree, as all the nodes need to be recreated and sent to the server as update actions)

The second phase which can be implemented after #1900 lands in master and every user action results in a separate tracing version. Then we can introduce the new update action @fm3 mentioned (revertToVersion) to avoid sending lots of update actions when reverting a tree deletion.

The Tracing-Version support should be separate from the Undo (Ctrl+Z) functionality to avoid confusion imo. It could be implemented as a sort of "Tracing History" kind of view with support to revert to an earlier version.

Do you agree with this?

@hotzenklotz
Copy link
Member

I agree. 👍

@hotzenklotz
Copy link
Member

@daniel-wer Please coordinate the frontend support with @jfrohnhofen and @fm3. Thank you

Frontend ToDos

  • Automatic detection of large changes. Perhaps use a new updateAction to inform the server about worthy candidates for reversion. e.g. splits, merges, tree delete, tree adds etc
  • Create a UI for selecting the revision for a tracings. Probably some sort of list with suggested version candidates (see above) and an input to manually jump to a version.
  • Integrate the UI either in the admin menu or as a modal directly in the tracing view

Other

  • Coordinate on discuss to find more use cases and user preferences.

@jfrohnhofen
Copy link
Contributor

One central question is, whether these snapshots or labels should be part of the tracing (i.e. versioned themselves). Say you have two snapshots (A at v100 and B at v200) and are currently at v300. Now you revert to A. What will happen behind the scenes is that the server creates a new version v301, that resets the tracing to v100. But no data is lost. If you continue tracing, new versions v302 etc. will be created.
Should one be able to revert to B now (a snapshot that does not even appear in the current tracing history)? This is easily possible for the server. You could even (provided you create enough snapshots) create two completely different tracings in one and work on both concurrently.
But I am ot sure, we want to expose that possibility to the user.

@fm3 fm3 removed their assignment Feb 20, 2018
@jfrohnhofen jfrohnhofen removed their assignment Feb 28, 2018
@philippotto
Copy link
Member

I understand that the need for this feature is greatly reduced due to the front-end side undo functionality, but I still think, that this feature would be sensible to implement. All the groundwork has already been done, as far as I understand. The feature would probably give users a greater confidence level and lower the need to download / re-upload NMLs. In general, it could also give us an edge over competitive tools.

@daniel-wer @fm3 What do you think? Should we tackle this in the next sprint? Is the to do list defined above still valid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants