[FIX] web: revert to previous state when onchange fails

When an onchange fails (for example, because the onchange python code
raised a ValidationError, it is convenient to revert the form view to a
valid state, which is the previous state.

To do that, we keep in the _applyChange function the initial state, and
restore it in the case the onchange fails.

1 files changed, 23 insertions, 18 deletions

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 2cf96815b5e..27a1e12ecfe 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -1109,6 +1109,10 @@ var BasicModel = AbstractModel.extend({
         if (!options.doNotSetDirty) {
             record._isDirty = true;
         }
+        var initialData = {};
+        this._visitChildren(record, function (elem) {
+            initialData[elem.id] = $.extend(true, {}, _.pick(elem, 'data', '_changes'));
+        });
 
         // apply changes to local data
         for (var fieldName in changes) {
@@ -1137,12 +1141,18 @@ var BasicModel = AbstractModel.extend({
                     }
                 }
             }
-            var onchangeDef;
+            var onchangeDef = $.Deferred();
             if (onChangeFields.length) {
-                onchangeDef = self._performOnChange(record, onChangeFields, options.viewType).then(function (result) {
-                    delete record._warning;
-                    return _.keys(changes).concat(Object.keys(result && result.value || {}));
-                });
+                self._performOnChange(record, onChangeFields, options.viewType)
+                    .then(function (result) {
+                        delete record._warning;
+                        onchangeDef.resolve(_.keys(changes).concat(Object.keys(result && result.value || {})));
+                    }).fail(function () {
+                        self._visitChildren(record, function (elem) {
+                            _.extend(elem, initialData[elem.id]);
+                        });
+                        onchangeDef.resolve({});
+                    });
             } else {
                 onchangeDef = $.Deferred().resolve(_.keys(changes));
             }
@@ -3232,15 +3242,18 @@ var BasicModel = AbstractModel.extend({
                 });
                 return $.when.apply($, defs)
                     .then(function () {
-                        return self._performOnChange(record, fields_key).then(function () {
+                        var def = $.Deferred();
+                        self._performOnChange(record, fields_key).always(function () {
                             if (record._warning) {
                                 if (params.allowWarning) {
                                     delete record._warning;
                                 } else {
-                                    return $.Deferred().reject();
+                                    def.reject();
                                 }
                             }
+                            def.resolve();
                         });
+                        return def;
                     })
                     .then(function () {
                         return self._fetchRelationalData(record);
@@ -3307,10 +3320,7 @@ var BasicModel = AbstractModel.extend({
      * @param {string[]} fields changed fields
      * @param {string} [viewType] current viewType. If not set, we will assume
      *   main viewType from the record
-     * @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.
+     * @returns {Deferred}
      */
     _performOnChange: function (record, fields, viewType) {
         var self = this;
@@ -3330,9 +3340,7 @@ var BasicModel = AbstractModel.extend({
         var context = this._getContext(record, options);
         var currentData = this._generateOnChangeData(record, {changesOnly: false});
 
-        var def = $.Deferred();
-
-        self._rpc({
+        return self._rpc({
                 model: record.model,
                 method: 'onchange',
                 args: [idList, currentData, fields, onchangeSpec, context],
@@ -3357,12 +3365,9 @@ var BasicModel = AbstractModel.extend({
                     record._domains = _.extend(record._domains, result.domain);
                 }
                 return self._applyOnChange(result.value, record).then(function () {
-                    def.resolve(result);
+                    return result;
                 });
-            }).fail(function () {
-                def.resolve({});
             });
-        return def;
     },
     /**
      * Once a record is created and some data has been fetched, we need to do