From 635572b49d5b70a32b717cf2d52da49b10aac28e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Fri, 20 Oct 2017 15:29:10 +0200 Subject: [FIX] web: properly handle failures in onchanges With the new views, we mainly focused on making them work. But we also need to be able to handle unusual cases, such as some RPC failing. An example of such a problem was the following: - a form view with a one2many with an onchange - edit mode, change a field in the one2many, click elsewhere - the onchange triggers, calls the server - it may happen that the server fails with a ValidationError - the onchange deferred fails, the line in the one2many is restored to the value known by the list renderer, which is the previous value (but the new value was applied to the model) - we now have a 'corrupted' state: clicking on save will send the invalid value, even though the UI displays the previous one. We solve this problem in this commit by making sure that the onchange deferred returned by the _performOnchange always succeed (or stays pending). If the rpc fails, the deferred will succeed with an empty dictionary. This means that the list renderer will be updated with the value from the model. Note that it may not be intuitive that a failure in the onchange is not seen as an error from the perspective of the _performOnChange, but I think that it still makes sense: - the validation/network errors are not really a concern of the UI. The UI just display the values entered by the user. - if an onchange fails (for example, because of a coding error), we do not really want to prevent the user from working. Also, when the onchange is performed, the initial change was already applied to the local data. Rollbacking them would be a more difficult task. Note that we also improve the mock server in this commit, to allow mocking failures. --- addons/web/static/src/js/views/basic/basic_model.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 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 b4a0b999453..717dcc9f6af 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -3283,11 +3283,17 @@ var BasicModel = AbstractModel.extend({ * This method is quite important: it is supposed to perform the /onchange * rpc and apply the result. * + * The changes that triggered the onchange are assumed to have already been + * applied to the record. + * * @param {Object} record * @param {string[]} fields changed fields * @param {string} [viewType] current viewType. If not set, we will assume * main viewType from the record - * @returns {Deferred} + * @returns {Deferred} Note: this deferred cannot fail. It is either in the + * 'success' or 'pending' state. If the onchange rpc fails, it will be + * resolved with an empty dictionary. Note that the initial change was + * already applied. */ _performOnChange: function (record, fields, viewType) { var self = this; @@ -3307,7 +3313,9 @@ var BasicModel = AbstractModel.extend({ var context = this._getContext(record, options); var currentData = this._generateOnChangeData(record, {changesOnly: false}); - return self._rpc({ + var def = $.Deferred(); + + self._rpc({ model: record.model, method: 'onchange', args: [idList, currentData, fields, onchangeSpec, context], @@ -3332,9 +3340,12 @@ var BasicModel = AbstractModel.extend({ record._domains = _.extend(record._domains, result.domain); } return self._applyOnChange(result.value, record).then(function () { - return result; + def.resolve(result); }); + }).fail(function () { + def.resolve({}); }); + return def; }, /** * Once a record is created and some data has been fetched, we need to do -- cgit v1.2.3