[FIX] web: in x2manys, do not do onchange if invalid line

In general, with the new views, we execute immediately and recursively
all onchanges, even from a x2many in a form view.  It allows for nice
interactivity, for example, recomputing immediately a total or a tax in
a sale order when an order line changed.

However, we are used to treat x2manys in a slightly weird way: for
example, if you add a line in a one2many, then click elsewhere, most of
the time, it is validated.  But if the line is 'incomplete' (I mean, if
a required field is not set), the line will not be validated, but be
automatically discarded.  If it was dirty, a confirm dialog will open.

So, to keep in line with that semantic, we have decided that onchange
will just not be done until the line is 'valid'.  This will avoid
trouble when someone add a line, it triggers some onchange which changes
some values in the form, then click somewhere else, the line is
discarded, but the changed values persist.  Most of the time, it
probably won't be a problem, because another onchange will be triggered,
but it is some useless work anyway.

1 files changed, 33 insertions, 4 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 87c9f3c57bc..62898a338bd 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -825,7 +825,6 @@ var BasicModel = AbstractModel.extend({
     _applyChange: function (recordID, changes) {
         var self = this;
         var record = this.localData[recordID];
-        var onChangeFields = []; // the fields that have changed and that have an on_change
         var field;
         var defs = [];
         record._changes = record._changes || {};
@@ -840,12 +839,19 @@ var BasicModel = AbstractModel.extend({
             } else {
                 record._changes[fieldName] = changes[fieldName];
             }
-            if (field.onChange) {
-                onChangeFields.push(fieldName);
-            }
         }
 
         return $.when.apply($, defs).then(function () {
+            var onChangeFields = []; // the fields that have changed and that have an on_change
+            for (var fieldName in changes) {
+                field = record.fields[fieldName];
+                if (field.onChange) {
+                    var isX2Many = field.type === 'one2many' || field.type === 'many2many';
+                    if (!isX2Many || (self._isX2ManyValid(record._changes[fieldName] || record.data[fieldName]))) {
+                        onChangeFields.push(fieldName);
+                    }
+                }
+            }
             var onchangeDef;
             if (onChangeFields.length) {
                 onchangeDef = self._performOnChange(record, onChangeFields).then(function (result) {
@@ -1949,6 +1955,29 @@ var BasicModel = AbstractModel.extend({
         return Object.keys(fieldsInfo && fieldsInfo[element.viewType] || {});
     },
     /**
+     * return true if a list element is 'valid'. Such an element is valid if it
+     * has no sub record with an unset required field.
+     *
+     * This method is meant to be used to check if a x2many change will trigger
+     * an onchange.
+     *
+     * @param {string} id id for a local resource of type 'list'. This is
+     *   assumed to be a list element for a one2many
+     * @returns {boolean}
+     */
+    _isX2ManyValid: function (id) {
+        var isValid = true;
+        var element = this.get(id, {raw: true});
+        _.each(element.data, function (rec) {
+            for (var fieldName in rec.fields) {
+                if (rec.fields[fieldName].required && !rec.data[fieldName]) {
+                    isValid = false;
+                }
+            }
+        });
+        return isValid;
+    },
+    /**
      * Helper method for the load entry point.
      *
      * @see load