From 9e97ad8dce4525d6a132b8f8353b5c8fa87caba2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Tue, 7 Nov 2017 10:53:04 +0100 Subject: [FIX] web: prevent useless discard dialog from showing up Here is a scenario that could cause an issue: - open a form view with a many2one - click on edit button - click on small external button for many2one. this opens a modal form view - click on save in the modal (without changing anything) - click on save in the main record - exit form view. this opens a discard dialog There were two issues here: - saving the modal dialog automatically marks the many2one field as changed. This was necessary to force reloading the data, because editing a sub value in the modal form view could have changed the display_name of the manyone field. However, this is not necessary when no change was done. - when saving a record, the _isDirty flag was reset to false only if an actual rpc was done. However, it may happen that the flag is set to dirty (for example, when modifying a value inside a many2one), but the main record has no changed fields. In this commit, we also remove the on_save handler in the formviewdialog. This is a small refactoring in a stable version, but no code currently use it, and I believe that it will make the code much easier to maintain (the previous code was really awkward), so I think that the tradeoff is acceptable. --- addons/web/static/src/js/views/basic/basic_model.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index 27a1e12ecfe..445967287bf 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -896,6 +896,9 @@ var BasicModel = AbstractModel.extend({ return def.resolve(changedFields); } + def.then(function () { + record._isDirty = false; + }); // in the case of a write, only perform the RPC if there are changes to save if (method === 'create' || changedFields.length) { var args = method === 'write' ? [[record.data.id], changes] : [changes]; @@ -917,7 +920,6 @@ var BasicModel = AbstractModel.extend({ // Erase changes as they have been applied record._changes = {}; - record._isDirty = false; // Update the data directly or reload them if (shouldReload) { -- cgit v1.2.3 From ae4501da9c997fcc86fb2ce0e032cd558cc1534e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 9 Nov 2017 09:40:54 +0100 Subject: [FIX] web: do not lose onchange info in some rare cases Here is a situation where we had a problem: - a form view with a one2many field, which has no inline views - the (non inline) list view has a field A, and is not editable - the (non inline) sub form view has fields A and B, with an onchange on B which modifies A In that case, the user could do this: - go to edit mode - click on 'add' a new one2many line - change the value of B in the form view, this changes the value of A - click on save to close the modal form view - click on the new o2m record to reopen the modal form view - rechange B => the onchange does not work The explanation is that when we reopen the modal, we update the known fields information, but we had to perform a fieldviewget to fetch the list view, so we have a full knowledge of the fields. However, the code did not update the fields info (because it uses _.default), which means that the onchange information contained in the form view is lost. Note: the test system had to be adapted to more closely simulate what actually happens. In particular, the onchange flag is no longer added by the mock server, since it should be done by the data manager, like 'real' code. This change broke the basic model tests, which had to be modified accordingly, by setting manually the onchange flag. --- addons/web/static/src/js/views/basic/basic_model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index 445967287bf..7e702e30862 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -949,8 +949,8 @@ var BasicModel = AbstractModel.extend({ */ addFieldsInfo: function (recordID, viewInfo) { var record = this.localData[recordID]; - record.fields = _.defaults(record.fields, viewInfo.fields); - record.fieldsInfo = _.defaults(record.fieldsInfo, viewInfo.fieldsInfo); + record.fields = _.extend({}, record.fields, viewInfo.fields); + record.fieldsInfo = _.extend({}, record.fieldsInfo, viewInfo.fieldsInfo); }, /** * Manually sets a resource as dirty. This is used to notify that a field -- cgit v1.2.3