Tryton - Issues

 

Issue3325

Title One2Many fields: Draft records are removed on cancel
Priority wish Status resolved
Superseder Nosy List ced, nicoe, reviewbot, roba, roundup-bot, yangoon
Type behavior Components sao, tryton
Assigned To nicoe Keywords review
Reviews 36451002, 41921002
View: 36451002, 41921002

Created on 2013-07-29.12:09:06 by roba, last changed by roundup-bot.

Messages
New changeset 2983e03abb72 by Nicolas ?vrard in branch 'default':
Reset record to its original state when discarding the popup window
http://hg.tryton.org/sao/rev/2983e03abb72
New changeset 455556b78d08 by Nicolas ?vrard in branch 'default':
Reset record to its original state when discarding the popup window
http://hg.tryton.org/tryton/rev/455556b78d08
review41921002 updated at https://codereview.tryton.org/41921002/#ps120001
review41921002 updated at https://codereview.tryton.org/41921002/#ps100001
review36451002 updated at https://codereview.tryton.org/36451002/#ps160001
review41921002 updated at https://codereview.tryton.org/41921002/#ps60001
review41921002 updated at https://codereview.tryton.org/41921002/#ps40001
review36451002 updated at https://codereview.tryton.org/36451002/#ps140001
review41921002 updated at https://codereview.tryton.org/41921002/#ps20001
msg39027 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2018-03-16.11:31:22
Patch for sao added.
New review41921002 at https://codereview.tryton.org/41921002/#ps1
review36451002 updated at https://codereview.tryton.org/36451002/#ps120001
review36451002 updated at https://codereview.tryton.org/36451002/#ps100001
review36451002 updated at https://codereview.tryton.org/36451002/#ps80001
review36451002 updated at https://codereview.tryton.org/36451002/#ps60001
review36451002 updated at https://codereview.tryton.org/36451002/#ps40001
msg34519 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-07-10.16:49:40
It needs a patch also for sao.
review36451002 updated at https://codereview.tryton.org/36451002/#ps20001
msg34515 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2017-07-10.14:54:42
After some thoughts on this subject with Cédric we realized that we could have a better behavior than discarding the record completely. It could be possible to restore the record to its state before the popup window has been opened.
msg18101 (view) Author: [hidden] (roba) (Tryton committer) Date: 2014-09-15.16:38:41
> And what does the cancel ?

I guess I see now what you mean. Currently it would do the same as the OK button... Well, it is actually more complicated than I thought.
msg18100 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2014-09-15.16:32:41
On 15 Sep 16:29, Robin Baumgartner wrote:
> I don't see the need for an undo stack. Just remove the delete button from the form and always show the cancel button instead. If you want to delete the entry there is a delete button on the list field.

And what does the cancel ?
msg18099 (view) Author: [hidden] (roba) (Tryton committer) Date: 2014-09-15.16:29:25
I don't see the need for an undo stack. Just remove the delete button from the form and always show the cancel button instead. If you want to delete the entry there is a delete button on the list field.

I did a short test with this kind of behavior and it feels a lot more intuitive.
msg18069 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2014-09-13.12:39:54
Patch is welcomed to implement undo stack.
msg18065 (view) Author: [hidden] (yangoon) Date: 2014-09-13.12:18:30
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
> 
> On 13 Sep 01:32, Mathias Behrle wrote:
> > I was pointed to this issue by ced from issue4147. In issue4147 I stumbled
> > upon this behavior, when using Esc to quit the dialog. So no button
> > involved, but only using the classic 'Esc'(ape) to just quit a window.
> > 
> > Is it really that complicated to just dismiss a dialog without any further
> > action? This would be for me the correct behavior of Esc.
> 
> This will be a misconception because the user can not see what will be
> the resul of the Esc because when you click on "new", you expect that
> Esc remove the new record. So the now, we always show via the button
> which action will be performed.

Here I don't understand. For me the user on pressing Esc can expect to find the application in the state before he opened the form/view. Nothing more, nothing less. I think, this is the regular and common behavior over all applications (and what the 'Esc' key was made for). That is, why it is named 'Escape': get me off without doing anything. Do you know of any applications, that are handling this in a different way?

> More over, if we don't delete in this case, it means we validate ("OK"
> button) and so to close the popup the record must be valid which will be
> awckward for Esc on non-valid record.

I take a complete different approach. Esc means 'quit without doing anything with the data on this form'. We could discuss, if the cancel button should behave also this way or if it should validate in the way: 'The form is modified. Closing will lose the data.' I would prefer nevertheless the first behavior (Esc and Cancel behaving the same way), because a user pressing this button actively says: 'I want to interrupt here without saving anything.'

The correct behavior would be for me to let the record just as is in the underlying view, where it will be validated anyway on saving. Losing data silently as in the current state is the worse case and leaves the user in an unclear situation.
 
> So all this disavantage are due to the fact that we only rely on the
> server database to cancel changes. This will could be avoided if there
> are some sort of undo management in the client but this is a really big
> task.

AFAIU records are saved into the database as soon as you hit Ok (Validate and write). Am I wrong here?
msg18061 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2014-09-13.09:20:27
On 13 Sep 01:32, Mathias Behrle wrote:
> I was pointed to this issue by ced from issue4147. In issue4147 I stumbled upon this behavior, when using Esc to quit the dialog. So no button involved, but only using the classic 'Esc'(ape) to just quit a window.
> 
> Is it really that complicated to just dismiss a dialog without any further action? This would be for me the correct behavior of Esc.

This will be a misconception because the user can not see what will be
the resul of the Esc because when you click on "new", you expect that
Esc remove the new record. So the now, we always show via the button
which action will be performed.
More over, if we don't delete in this case, it means we validate ("OK"
button) and so to close the popup the record must be valid which will be
awckward for Esc on non-valid record.

So all this disavantage are due to the fact that we only rely on the
server database to cancel changes. This will could be avoided if there
are some sort of undo management in the client but this is a really big
task.
msg18059 (view) Author: [hidden] (yangoon) Date: 2014-09-13.01:32:22
I still have a question, that is not really answered for me in this issue so far:

I was pointed to this issue by ced from issue4147. In issue4147 I stumbled upon this behavior, when using Esc to quit the dialog. So no button involved, but only using the classic 'Esc'(ape) to just quit a window.

Is it really that complicated to just dismiss a dialog without any further action? This would be for me the correct behavior of Esc.
msg14165 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2013-09-30.12:36:54
undo feature is more a wish than a feature because it is a so much big job.
msg13915 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2013-08-16.11:26:05
The patch about the button appearance has been commited in changeset b0cb42542ab0

The undo feature is still unresolved.
msg13856 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2013-07-29.14:29:26
On 29/07/13 13:57 +0200, Robin Baumgartner wrote:
> > close dialog is linked to the cancel button (and future remove button).
> > I can not see it linked to the validate button.
> 
> I think closing the dialog window should neither delete nor validate the record.
> It should just discard any changes made, maybe with some sort of confirmation
> dialog.

Patch is welcome.
msg13855 (view) Author: [hidden] (roba) (Tryton committer) Date: 2013-07-29.13:57:02
> close dialog is linked to the cancel button (and future remove button).
> I can not see it linked to the validate button.

I think closing the dialog window should neither delete nor validate the record.
It should just discard any changes made, maybe with some sort of confirmation
dialog.
msg13852 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2013-07-29.13:52:43
close dialog is linked to the cancel button (and future remove button).
I can not see it linked to the validate button.
msg13851 (view) Author: [hidden] (roba) (Tryton committer) Date: 2013-07-29.13:47:51
> This is the expected behavior but the buttons labels are not very good.

It does not feel right to me. If the button is labeled correctly I can accept
this, but closing the dialog window should definitely not remove the record.
msg13849 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2013-07-29.13:38:31
This is the expected behavior but the buttons labels are not very good.
This is improved by the review894002
msg13848 (view) Author: [hidden] (roba) (Tryton committer) Date: 2013-07-29.12:09:55
Forgot to mention: The error occurs with the Tryton client 2.8.
msg13847 (view) Author: [hidden] (roba) (Tryton committer) Date: 2013-07-29.12:09:05
Steps to reproduce the problem:
* Create a new record in a One2Many field
* Close the dialog
* Do NOT save
* Open the draft record a second time
* Click the cancel button or close the window
* The draft record is gone
History
Date User Action Args
2018-03-23 16:39:28roundup-botsetmessages: + msg39276
2018-03-23 16:38:47roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg39275
2018-03-23 15:54:57reviewbotsetmessages: + msg39269
2018-03-22 14:19:55reviewbotsetmessages: + msg39213
2018-03-22 13:54:52reviewbotsetmessages: + msg39212
2018-03-21 23:55:33reviewbotsetmessages: + msg39195
2018-03-21 19:21:34reviewbotsetmessages: + msg39193
2018-03-21 19:21:18reviewbotsetmessages: + msg39192
2018-03-21 09:49:12reviewbotsetmessages: + msg39154
2018-03-16 11:31:23nicoesetmessages: + msg39027

Showing 10 items. Show all history (warning: this could be VERY long)