XrmToolBox icon indicating copy to clipboard operation
XrmToolBox copied to clipboard

Application wide action history with undo / redo

Open shytikov opened this issue 10 years ago • 25 comments

There is nice functionality in FetchXML Builder to undo steps: https://github.com/Cinteros/FetchXMLBuilder/issues/76

It would be so cool if something similar will appear, for example in Web Resources Manager.

Would be totally awesome, if this functionality would be global for XTB, so plugin developers should not worry that much about implementing their own vision of undo / redo.

It would be epic, if XTB will expose easy to use History API, so plugin developer could add user action in the queue easily, and revert this action, if needed.

Could we consider this generic approach?

shytikov avatar Sep 08 '15 07:09 shytikov

How is this type of functionality implemented in the FetchXmlBuilder? I can understand having a stack of actions and poping off the actions but I don't know how you'd be able to automatically undo the action

daryllabar avatar Sep 08 '15 10:09 daryllabar

FXB simply stores "snapshots" of the complete fetchxml after every change. Undoing restores xml to one step back in history, redoing goes forward. Redo-possibility is removed after each manual change.

There are quite simple methods to StoreSnapshot and RestoreSnapshot including keeping track of current position in the history.

It might be quite easy to extract this to generic functionality, by allowing any object to be stored, and exposing a few methods like the above mentioned to the plugins.

If, when and where this might be useful would of course be individual to each plugin, and it would require som redesign of the XTB UI to allow for more plugin generic functionality. This by the way, would also be part of the request in #221.

Personally I am not sure the request is feasible, but if more plugins could benefit from this I would certainly re-implement the undo/redo in FXB to use this!

rappen avatar Sep 08 '15 10:09 rappen

It works in some scenarios, but not others. It's also something that I'm not sure how the XTB could help with besides maybe providing a common UI if the Plugin supports the features. None of my plugins would. The Early Bound Generator has nothing to undo and the Attribute Manager migrates data, and there is no safe way to do that.

daryllabar avatar Sep 08 '15 12:09 daryllabar

+1 for @daryllabar The only common way to handle that would be to record all calls to IOrganizationService (and saving data before that call is made to be able to reapply) but even that won't work for some scenarios

MscrmTools avatar Sep 14 '15 08:09 MscrmTools

I definitely agree this is a low prio nice-to-have. But to clarify - XTB should not be made responsible for the actual "undoing", but as per suggestion provide the UI and generic history list, for each plugin to implement actual code behind the operations.

rappen avatar Sep 14 '15 10:09 rappen

I thought about it yesterday and maybe found a solution: What about an interface that provides an event (SendSnapshot) and a method (RestoreSnapshot)

SendSnapshot takes an eventargs that stores pointer to the plugin, a message ("updated webresource XYZ", for example) and an object handled by the plugin developer (can be anything) that stores a state before an user operation.

RestoreSnaphost have one argument: the object sent during the SendSnapshot event.

XrmToolbox would also display a toolbar button to display/hide a dialog that shows snapshot for the currently displayed plugin

Make sense?

MscrmTools avatar Jun 12 '16 09:06 MscrmTools

Here is an example of implementation

image

When clicking the WhoAmI button, I send a snapshot with only the current datetime as plugin state it is added in the snapshot dialog If I click on an item then on "Restore..." button, I send a message to the plugin which displays the information

I would need to try implementing in a real plugin to see if it fits all needs

Any thoughts? @daryllabar @shytikov @rappen

MscrmTools avatar Jun 13 '16 11:06 MscrmTools

Couple Questions...

  1. Should the Snapshot be serializable? That way you could ensure that the snapshot stays unchanged... At the same time, I'm not sure if some of the CRM Data is serializable...
  2. According to the menu, it looks like you can choose any snapshot... would the system roll back each snapshot in between? Or is this less of an undo, more of a strict SnapShot type of situation?

daryllabar avatar Jun 13 '16 12:06 daryllabar

  1. Not currently, I'm just storing the Object passed by the plugin but I understand your point. I have no strong experience with serialization but I guess the answer to your question is "Yes"
  2. still currently, you can choose any snapshot and this is the plugin that should handle restore correctly. Maybe the plugin could specified how it wants to handle restore (some Enum property that allows "One snapshot" or "All snapshots until selected one")

As I said, I do not have real implementation yet, so I still have to figure out what scenario are possible

MscrmTools avatar Jun 13 '16 12:06 MscrmTools

The Xrm Entities and Request objects do not implement ISerializable, just the DotNetDataContract. I'm assuming you'll need to go that route, rather than ISerializable.

Before we(mostly just @MscrmTools currently), spend too much time working on this, I think we should have a couple well thought out scenarios that we apply to this enhancement.

The only thing I can think of using this for in my plugins, would be to undo setting changes... Do we have other scenarios we would like to apply this to?

daryllabar avatar Jun 13 '16 12:06 daryllabar

I could have plenty of them like editing the sitemap, editing web resources, editing entities icons, etc... But I'm not sure to cover all scenario. I guess I will mabye create a new branch to play with this feature

MscrmTools avatar Jun 13 '16 12:06 MscrmTools

I could also see a couple of useful scenarios, but those are mostly related to things done within the plugin, and not regarding CRUD actions to CRM. For example in the ViewDesigner - undo/redo manual design changes, but perhaps no undo after view design has been sent to CRM. As the current implementation in FXB stores snapshots of current fetch and not a delta, undoing several "versions" back would not be a problem. I think it would be far more complicated (for consuming plugins) to implement delta history, where you have to roll back one change at a time to get a proper undo of several version. I just want to verify - does the design include redo? Basically - I could live with the proposed design :)

rappen avatar Jun 14 '16 11:06 rappen

Sample of a very simple, but "does the job" class: https://github.com/Innofactor/FetchXMLBuilder/blob/master/FetchXmlBuilder/HistoryManager.cs

Sample of internal methods in FXB using the above class: https://github.com/Innofactor/FetchXMLBuilder/blob/master/FetchXmlBuilder/FetchXmlBuilder.cs#L2089-L2112

Sample of pushing a "change" to the history: https://github.com/Innofactor/FetchXMLBuilder/blob/master/FetchXmlBuilder/FetchXmlBuilder.cs#L341

Sample of poping a state using undo or redo button: https://github.com/Innofactor/FetchXMLBuilder/blob/master/FetchXmlBuilder/FetchXmlBuilder.cs#L527-L535

rappen avatar Jun 14 '16 11:06 rappen

I already have a working code (the one used for the screenshot) and I basically store what the plugin wants to store and pass it back data when the user select an action and click on "Restore" button. What remains to be decided/coded:

  • store unalterable data. @daryllabar suggested to serialize data but I don't know yet if it is possible to handle any kind of serialization without knowing types first. At least, I could replace object by string and let plugin serialize and deserialize data itself
  • store some kind of snapshot data like "is it possible to restore any snapshot or only the last one?" or "is it possible to select multiple snapshots to restore", etc.

Anyway, I will update the DEV branch with my changes so that you can see it and play with it

MscrmTools avatar Jun 14 '16 11:06 MscrmTools

The DEV branch is up to date, waiting for your feedback

MscrmTools avatar Jun 14 '16 12:06 MscrmTools

I'm leaning towards not forcing the data to be serializable. It could be extremely costly. And it if is a problem for a particular plugin, then could clone/serialize themselves...

daryllabar avatar Jun 14 '16 13:06 daryllabar

ok, so leave the data as object but let the plugin handle the job itself so no XrmToolBox responsability is involved

MscrmTools avatar Jun 14 '16 13:06 MscrmTools

I think the parameter should be of type Snapshot, not object. Right? https://github.com/MscrmTools/XrmToolBox/blob/Dev/XrmToolBox.Extensibility/Interfaces/ISnapshotable.cs#L8

rappen avatar Jun 14 '16 14:06 rappen

Yes, you are right! my mistake

MscrmTools avatar Jun 14 '16 14:06 MscrmTools

Other than that - it works fine, technically. I still think an Undo and a Redo button is more intuitive. Having the "stack" of events might be good for an overview and the possibility to really trace what has happened, but in most cases Undo/Redo does the job. Having that possibility would definitely be more important than this list of snapshots, from my perspective.

If the list is kept, i think the items should be sorted the other way, so latest "version" is at the top and the further you look down the list, the further back in time you get.

The interface (or the tiny form) should also have a possibility to clear the history.

rappen avatar Jun 14 '16 14:06 rappen

I'm not sure about the Redo feature. What is its purpose? History should enable restoring state after mistakes. So, why would you like to reapply something wrong?

MscrmTools avatar Jun 14 '16 14:06 MscrmTools

Probably for the same reason it is available in all Office apps etc... Try something, test it, undo it, compare results, redo it, be happy...

rappen avatar Jun 14 '16 14:06 rappen

ok, so where these buttons should be located? XTB toolbar is quite crowdy yet

MscrmTools avatar Jun 14 '16 14:06 MscrmTools

Full respect for the tight toolbar.... Could the one you created be a flyout (or whatever they are called) with sub items:

  • Undo
  • Redo
  • History... (opens the tiny form)

Undo and Redo should be enabled based on the existing stack and the current "position" in the stack. Having this approach should also disable Redo as soon as another snapshot is placed in the stack. Existing code in my example on link above in this thread :)

rappen avatar Jun 14 '16 14:06 rappen

ok, I will work on this, thank you for the discussion

MscrmTools avatar Jun 14 '16 14:06 MscrmTools