From 7d26d244a438a71d633922e043b18e488dbe269b Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 4 May 2017 10:35:55 +0200 Subject: [PATCH] [FIX] web: BasicModel: trigger onchange when x2m is valid When a record is added in an x2many field with an onchange, the onchange is only triggered once the new record is valid (i.e. when all its required fields have a correct value). However, there was an issue in the code checking the validity of a record, as it checked, for each required field, that its value wasn't falsy (!value). This is incorrect for several type of fields: - numerical: a value of 0 is correct - boolean: should be always considered as correct - x2m: their value is a dataPoint, so it is never falsy, what should be check is the number of records inside the dataPoint. This was an issue for example in the sale.order form view, as the onchange wasn't triggered when an order line was added (with a set product), so the total amount wasn't updated correctly. --- .../static/src/js/views/basic/basic_model.js | 27 ++++++++++++++++--- .../tests/fields/relational_fields_tests.js | 27 ++++++++++++++++--- 2 files changed, 47 insertions(+), 7 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 f7026f27129..42731bcf83b 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -2008,6 +2008,25 @@ var BasicModel = AbstractModel.extend({ var fieldsInfo = element.fieldsInfo; return Object.keys(fieldsInfo && fieldsInfo[element.viewType] || {}); }, + /** + * Returns true iff value is considered to be set for the given field's type. + * + * @private + * @param {any} value a value for the field + * @param {string} fieldType a type of field + * @returns {boolean} + */ + _isFieldSet: function (value, fieldType) { + switch (fieldType) { + case 'boolean': + return true; + case 'one2many': + case 'many2many': + return value.count > 0; + default: + return value !== false; + } + }, /** * return true if a list element is 'valid'. Such an element is valid if it * has no sub record with an unset required field. @@ -2020,11 +2039,13 @@ var BasicModel = AbstractModel.extend({ * @returns {boolean} */ _isX2ManyValid: function (id) { + var self = this; var isValid = true; var element = this.get(id, {raw: true}); - _.each(element.data, function (rec) { - _.each(rec.getFieldNames(), function (fieldName) { - if (rec.fields[fieldName].required && !rec.data[fieldName]) { + _.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)) { isValid = false; } }); diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index db81e19731c..4d4388b508d 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -3029,13 +3029,19 @@ QUnit.module('relational_fields', { form.destroy(); }); - QUnit.test('one2many list editable: trigger when row is valid', function (assert) { + QUnit.test('one2many list editable: trigger onchange when row is valid', function (assert) { // should omit require fields that aren't in the view as they (obviously) // have no value, when checking the validity of required fields - assert.expect(7); + // shouldn't consider numerical fields with value 0 as unset + assert.expect(11); this.data.turtle.fields.turtle_foo.required = true; this.data.turtle.fields.turtle_qux.required = true; // required field not in the view + this.data.turtle.fields.turtle_bar.required = true; // required boolean field with no default + delete this.data.turtle.fields.turtle_bar.default; + this.data.turtle.fields.turtle_int.required = true; // required int field (default 0) + this.data.turtle.fields.turtle_int.default = 0; + this.data.turtle.fields.partner_ids.required = true; // required many2many this.data.partner.onchanges = { turtles: function (obj) { obj.int_field = obj.turtles.length; @@ -3058,8 +3064,11 @@ QUnit.module('relational_fields', { }, archs: { 'turtle,false,list' : '' + + '' + + '' + '' + '' + + '' + '', }, res_id: 1, @@ -3068,13 +3077,23 @@ QUnit.module('relational_fields', { assert.strictEqual(form.$('.o_field_widget[name="int_field"]').val(), "0", "int_field should start with value 0"); + + // add a new row (which is invalid at first) form.$('.o_field_x2many_list_row_add a').click(); assert.strictEqual(form.$('.o_field_widget[name="int_field"]').val(), "0", - "int_field should still be 0 (no onchange should have been done yet"); - + "int_field should still be 0 (no onchange should have been done yet)"); assert.verifySteps(['read', 'default_get'], "no onchange should have been applied"); + // fill turtle_foo field form.$('.o_field_widget[name="turtle_foo"]').val("some text").trigger('input'); + assert.strictEqual(form.$('.o_field_widget[name="int_field"]').val(), "0", + "int_field should still be 0 (no onchange should have been done yet)"); + assert.verifySteps(['read', 'default_get'], "no onchange should have been applied"); + + // fill partner_ids field with a tag (all required fields will then be set) + var $m2mInput = form.$('.o_field_widget[name=partner_ids] input'); + $m2mInput.click(); + $m2mInput.autocomplete('widget').find('li:first()').click(); assert.strictEqual(form.$('.o_field_widget[name="int_field"]').val(), "1", "int_field should now be 1 (the onchange should have been done");