When removing a style through Style Editor and clicking on cancel the change is not undone

[rodrigoA]

Hi,

I was playing arround and found the following behavior:

1. Open Style Editor;
2. Select lets say American Amber Ale, or any other style;
3. Click on button remove;
4. Click on button Cancel;
5. Reopen the Style editor and try to find the American Amber Ale, or the style you've selected.

Result: It won't be available.

I know that after closing BrewTarget it asks if I want keep the changes etc, but if I regret deleting that style, is there a way back?

Is this a "as designed" behavior?

The cancel button aparrently only hides the dialog

void StyleEditor::clearAndClose()
{
   setVisible(false);
}

And the remove button directly deletes a style from DB

void StyleEditor::removeStyle()
{
   if( obsStyle )
      Database::instance().remove(obsStyle);

   setStyle(0);
}

I know almost nothing about sql, but isn't there a way to roolback the changes if the user hits the Cancel button?

[mikfire]

If we had the transactions defined, then yes. We could use a rollback. We don't have the transactions defined right now. I am trying, but it is slow and tedious work.

For the record, Database::remove mostly toggles the deleted flag to "true" on whatever you deleted. There are a few exceptions to that rule, but it holds for every BeerXML item. You could, if the delete was a really bad mistake, simply open the database and flip the flag back to false.

Yes. Cancelling the change should likely undo that, but it will take me some work to figure out how. It won't be enough to do just one. If you delete multiple elements from the interface, I will need to undelete all of them.

Perhaps it would be easier to just disable that delete button? If you want to delete things, use the trees?

[rodrigoA]

Oh, I see. Thank you for the explanation.
So, it is working as designed. Would you mind to reject this bug?

Thanks :)

[mikfire]

It may be working as designed, but if this confused or surprised you then
it confuses and surprises others. I would like some time to contemplate the
solution.

Until then, I would rather keep this open. It will help remind me that this
needs fixing.

Mik
On Feb 20, 2016 14:42, "rodrigo" [email protected] wrote:

Oh, I see. Thank you for the explanation.
So, it is working as designed. Would you mind to reject this bug?

Thanks :)

—
Reply to this email directly or view it on GitHub
#198 (comment)
.

[rocketman768]

To answer the question

I know almost nothing about sql, but isn't there a way to roolback the changes if the user hits the Cancel button?

No...at least not built into the language. SQLite allows rollbacks, but only basically for errors that happen in the middle of a transaction. If you want a higher-level rollback, you would have to implement it yourself. That's pretty much what I tried at one point with SetterCommand and SetterCommandStack in an attempt to have an "undo" button. It became clear trying to do this that capturing "high-level" things like adding an ingredient to a recipe (which requires a bunch of sql statements) into an "undoable" command would require lots of coding and careful design.

[theophae]

About the undo feature, do you think it is a good idea to keep a FIFO of the inverted database request so that you just have to execute this request at every undo action triggered ?
This meens that every time a DELETE or UPDATE request is prepared, we must prepare the opposite request, and when the command is applyed, the opposite one is pushed into the FIFO.

For example if the commande is to delete an ingredient from the database, we need to store the command that create that exacte same ingredient we've just removed.
In my mind, as Brewtarget is fully based on the database, it should work pretty well.

[mattiasmaahl]

I think this was resolved in #499, closing