[FIX] web: handle 'required' attribute in x2many+onchange

Before doing an onchange in a x2many, the basic model tries to evaluate
if the line is valid or not.  In that computation, we used the
field.required property instead of using the required modifier.  This
commit had to move the modifier handling from the BasicRenderer to the
BasicModel, in order to use it in the _isX2ManyValid method.

2 files changed, 46 insertions, 29 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 9d4d29b7635..85150061075 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -27,7 +27,6 @@ odoo.define('web.BasicModel', function (require) {
  *      fieldsInfo: {Object},
  *      getContext: {function},
  *      getDomain: {function},
- *      getEvalContext: {function},
  *      getFieldNames: {function},
  *      groupedBy: {string[]},
  *      id: {integer},
@@ -346,11 +345,11 @@ var BasicModel = AbstractModel.extend({
                 count: element.count,
                 data: data,
                 domain: element.domain.slice(0),
+                evalModifiers: element.evalModifiers,
                 fields: element.fields,
                 fieldsInfo: element.fieldsInfo,
                 getContext: element.getContext,
                 getDomain: element.getDomain,
-                getEvalContext: element.getEvalContext,
                 getFieldNames: element.getFieldNames,
                 id: element.id,
                 limit: element.limit,
@@ -400,7 +399,6 @@ var BasicModel = AbstractModel.extend({
             fields: element.fields,
             getContext: element.getContext,
             getDomain: element.getDomain,
-            getEvalContext: element.getEvalContext,
             getFieldNames: element.getFieldNames,
             groupedBy: element.groupedBy,
             id: element.id,
@@ -1272,6 +1270,37 @@ var BasicModel = AbstractModel.extend({
         return hasOnchange ? specs : false;
     },
     /**
+     * Evaluate modifiers
+     *
+     * @private
+     * @param {Object} element a valid element object, which will serve as eval
+     *   context.
+     * @param {Object} modifiers
+     * @returns {Object}
+     */
+    _evalModifiers: function (element, modifiers) {
+        var result = {};
+        var self = this;
+        var evalContext;
+        function evalModifier (mod) {
+            if (mod === undefined || mod === false || mod === true) {
+                return !!mod;
+            }
+            evalContext = evalContext || self._getEvalContext(element);
+            return new Domain(mod, evalContext).compute(evalContext);
+        }
+        if ('invisible' in modifiers) {
+            result.invisible = evalModifier(modifiers.invisible);
+        }
+        if ('readonly' in modifiers) {
+            result.readonly = evalModifier(modifiers.readonly);
+        }
+        if ('required' in modifiers) {
+            result.required = evalModifier(modifiers.required);
+        }
+        return result;
+    },
+    /**
      * Fetch all name_gets for the many2ones in a group
      *
      * @param {Object} group a valid resource object
@@ -2168,11 +2197,17 @@ var BasicModel = AbstractModel.extend({
     _isX2ManyValid: function (id) {
         var self = this;
         var isValid = true;
-        var element = this.get(id, {raw: true});
-        _.each(element.getFieldNames(), function (fieldName) {
-            var field = element.fields[fieldName];
-            _.each(element.data, function (rec) {
-                if (field.required && !self._isFieldSet(rec.data[fieldName], field.type)) {
+        var element = this.localData[id];
+        _.each(element._changes || element.data, function (recordID) {
+            var recordData = self.get(recordID, {raw: true}).data;
+            var record = self.localData[recordID];
+            _.each(element.getFieldNames(), function (fieldName) {
+                var field = element.fields[fieldName];
+                var fieldInfo = element.fieldsInfo[element.viewType][fieldName];
+                var rawModifiers = JSON.parse(fieldInfo.modifiers || "{}");
+                var modifiers = self._evalModifiers(record, rawModifiers);
+                var required = 'required' in modifiers ? modifiers.required : field.required;
+                if (required && !self._isFieldSet(recordData[fieldName], field.type)) {
                     isValid = false;
                 }
             });
@@ -2260,9 +2295,9 @@ var BasicModel = AbstractModel.extend({
             viewType: params.viewType,
         };
 
+        dataPoint.evalModifiers = this._evalModifiers.bind(this, dataPoint);
         dataPoint.getContext = this._getContext.bind(this, dataPoint);
         dataPoint.getDomain = this._getDomain.bind(this, dataPoint);
-        dataPoint.getEvalContext = this._getEvalContext.bind(this, dataPoint);
         dataPoint.getFieldNames = this._getFieldNames.bind(this, dataPoint);
 
         this.localData[dataPoint.id] = dataPoint;
diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js
index 907ea7ddc07..2e36a4cac55 100644
--- a/addons/web/static/src/js/views/basic/basic_renderer.js
+++ b/addons/web/static/src/js/views/basic/basic_renderer.js
@@ -269,22 +269,6 @@ var BasicRenderer = AbstractRenderer.extend({
         return widget.isValid() && (widget.isSet() || !modifiers.required);
     },
     /**
-     * Updates the modifiers evaluation associated to a given modifiers data and
-     * a given record. This only updates the modifiers values. To see associated
-     * DOM updates: @see _updateAllModifiers @see _applyModifiers.
-     *
-     * @private
-     * @param {Object} modifiersData
-     * @param {Object} record
-     */
-    _computeModifiers: function (modifiersData, record) {
-        var evalContext = record.evalContext;
-        modifiersData.evaluatedModifiers[record.id]
-            = _.mapObject(modifiersData.modifiers, function (modifier) {
-                return new Domain(modifier, evalContext).compute(evalContext);
-            });
-    },
-    /**
      * Destroys a given widget associated to the given record and removes it
      * from internal referencing.
      *
@@ -342,7 +326,6 @@ var BasicRenderer = AbstractRenderer.extend({
      *   record. This allows nodes that will produce an AbstractField instance
      *   to have their modifiers registered before this field creation as we
      *   need the readonly modifier to be able to instantiate the AbstractField.
-     *   (@see _computeModifiers).
      *
      * - On additional registrations, if the node was already registered but the
      *   record is different, we evaluate the modifiers for this record and
@@ -399,7 +382,7 @@ var BasicRenderer = AbstractRenderer.extend({
 
         // Evaluate if necessary
         if (!modifiersData.evaluatedModifiers[record.id]) {
-            this._computeModifiers(modifiersData, record);
+            modifiersData.evaluatedModifiers[record.id] = record.evalModifiers(modifiersData.modifiers);
         }
 
         // Element might not be given yet (a second call to the function can
@@ -569,7 +552,6 @@ var BasicRenderer = AbstractRenderer.extend({
      * 2) Updates the rendering of the view elements associated to the given
      *    record to match the new modifiers.
      *
-     * @see _computeModifiers
      * @see _applyModifiers
      *
      * @private
@@ -582,7 +564,7 @@ var BasicRenderer = AbstractRenderer.extend({
         var defs = [];
         this.defs = defs; // Potentially filled by widget rerendering
         _.each(this.allModifiersData, function (modifiersData) {
-            self._computeModifiers(modifiersData, record);
+            modifiersData.evaluatedModifiers[record.id] = record.evalModifiers(modifiersData.modifiers);
             self._applyModifiers(modifiersData, record);
         });
         delete this.defs;