Issue 10480

Title
Closing tab on a new form reload another record after not saving and don't close the tab.
Priority
bug
Status
resolved
Nosy list
ced, mrichez, pokoli, reviewbot, roundup-bot, tbruyere, yangoon
Assigned to
mrichez
Keywords
review

Created on 2021-06-02.10:54:30 by mrichez, last changed 1 week ago by roundup-bot.

Messages

New changeset 1a549e8dda5c by Maxime Richez in branch '6.0':
Close tab when cancelling save on new record
https://hg.tryton.org/tryton/rev/1a549e8dda5c

New changeset e3a715ee875f by Maxime Richez in branch '5.8':
Close tab when cancelling save on new record
https://hg.tryton.org/tryton/rev/e3a715ee875f

New changeset c7477c90e6be by Maxime Richez in branch '5.0':
Close tab when cancelling save on new record
https://hg.tryton.org/tryton/rev/c7477c90e6be
New changeset b0b231e9ed9a by Maxime Richez in branch '6.0':
Close tab when cancelling save on new record
https://hg.tryton.org/sao/rev/b0b231e9ed9a

New changeset 279f4dca4fbe by Maxime Richez in branch '5.8':
Close tab when cancelling save on new record
https://hg.tryton.org/sao/rev/279f4dca4fbe

New changeset 86070e4180f1 by Maxime Richez in branch '5.0':
Close tab when cancelling save on new record
https://hg.tryton.org/sao/rev/86070e4180f1
New changeset 4df7e301f1df by Maxime Richez in branch 'default':
Close tab when cancelling save on new record
https://hg.tryton.org/tryton-env/rev/4df7e301f1df
New changeset 941b90300b84 by Maxime Richez in branch 'default':
Close tab when cancelling save on new record
https://hg.tryton.org/tryton/rev/941b90300b84
New changeset bea021af96d5 by Maxime Richez in branch 'default':
Close tab when cancelling save on new record
https://hg.tryton.org/sao/rev/bea021af96d5
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-06-15.09:37:09

Yes you must pass an argument so the _close_allowed can return a resolved or failed promise.

Author: [hidden] (mrichez)
Date: 2021-06-15.08:59:13

I'm not comfortable with javascript and the promises.
In the python script, when record_id is < 0, return is None. So in javascript it should return a jQuery.Deferred().reject() ?
Should i pass the value in the "reject" ? jQuery.Deferred().reject(true) ?

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-06-09.08:53:03

The solution of msg67928 can not be implemented in sao because Promise has only two states.
I guess we could resolve the promise with the value True or Null and test it in __close_allowed.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-06-02.23:24:01

I do not agree sig_close must return True for the tab being closed.

The problem is the special case of new record for which the user decides to not save. For this case we want to close the tab but we do not want to launch an action.
So we need a third returned value which could be None.
The only ambiguous case is the ko. I think we must return True if the current record is still the same and otherwise return None. Then the sig_close must return True if modified_save returns True or None. (sig_action does not need to be changed).

Author: [hidden] (mrichez)
Date: 2021-06-02.15:37:43

There's something not logical in the returned value in method "modified_save":

  • sig_close (https://hg.tryton.org/tryton/file/default/tryton/gui/window/form.py#l677) needs a True value for closing which is the value returned when i don't want to save a new record.

  • But action (https://hg.tryton.org/tryton/file/default/tryton/gui/window/form.py#l683) needs the opposite value to bypass the action

Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2021-06-02.15:05:20

Since issue9881 we should not execute a report nor a different record when the changes are discarded. We should do nothing in this cas.

Probably it is need to add a new parameter to the modified_save to skip the test for some actions (i.e: report, action, relate) and keep the current behaviour for other (i.e: close tab)

Author: [hidden] (mrichez)
Date: 2021-06-02.15:02:15

I checked in 5.7 : the behavior is the same as my patch when clicking on report on a new record that should not be saved, it loads another record. Don't know what should be done in this case because it was the original behavior.

Author: [hidden] (mrichez)
Date: 2021-06-02.14:41:00

Hum, too quick... It's not working when clicking on report

Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2021-06-02.14:33:05

Sounds like a reasonable solution but should be tested with a report or and action related

Author: [hidden] (mrichez)
Date: 2021-06-02.14:13:39

Could we just invert order of testing ?

https://hg.tryton.org/tryton/file/default/tryton/gui/window/form.py#l669

                if self.sig_reload(test_modified=False):
                    if self.screen.current_record:
                        return record_id == self.screen.current_record.id
                    elif record_id < 0:
                        return True

by

                if self.sig_reload(test_modified=False):
                    if record_id < 0:
                        return True
                    elif self.screen.current_record:
                        return record_id == self.screen.current_record.id
Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2021-06-02.12:20:55

I just tested it on 5.8 series and the problem is just with the new records.
For me the behaviour seems correct when the record is already saved.

Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2021-06-02.11:05:05

As discussed on the irc this is linked to issue9881.
We should keep the behaviour introduced on this issue but just for some actions.

The new, switch view, next, forward and close actions should be executed without comparing the ids while others should not be executed if the record does not save the changes.

Author: [hidden] (mrichez)
Date: 2021-06-02.10:54:30

Linked to https://hg.tryton.org/tryton/rev/5b4d41c301a3.

To reproduce (tryton >=5.8.3):

  • Open a new tab (sales for instance)
  • Create a new record
  • Close the tab
  • At the confirmation to save, click on "No"
  • Another record will be displayed in form view instead of closing tab.
History
Date User Action Args
2021-09-14 21:52:49roundup-botsetmessages: + msg70105
2021-09-14 21:52:46roundup-botsetkeyword: - backport
messages: + msg70104
2021-09-10 19:38:27cedsetkeyword: + backport
2021-09-10 19:38:12roundup-botsetmessages: + msg70000
2021-09-10 19:38:09roundup-botsetmessages: + msg69999
2021-09-10 19:38:07roundup-botsetmessages: + msg69998
nosy: + roundup-bot
status: testing -> resolved
2021-08-12 23:56:27reviewbotsetmessages: + msg69356
2021-07-05 14:30:04reviewbotsetmessages: + msg68715
2021-06-15 11:04:59reviewbotsetmessages: + msg68260
2021-06-15 09:37:09cedsetmessages: + msg68258

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