From 91d1fd870123e9a56d64e3482e2ead1536230ab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 3 May 2017 15:18:08 +0200 Subject: [PATCH] [FIX] web: prevent double onchange in some cases When we unselect a line in a one2many list, we ask for all fields to commit their changes (because many fields are debounced, so we may not know their current last value). This is good, except that it could trigger an extra field change event, which may in turn trigger a second onchange. We solve this issue by improving the _setValue method and delegating the comparison to a new _isSameValue method. It is particularly relevant for field types such as integer, which are not stored as a string. --- .../static/src/js/fields/abstract_field.js | 19 ++++- .../web/static/src/js/fields/basic_fields.js | 18 ++++- .../tests/fields/relational_fields_tests.js | 80 +++++++++++++++++++ 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js index 757e917dd36..9f977d41392 100644 --- a/addons/web/static/src/js/fields/abstract_field.js +++ b/addons/web/static/src/js/fields/abstract_field.js @@ -259,6 +259,20 @@ var AbstractField = Widget.extend({ var options = _.extend({}, this.nodeOptions, { data: this.recordData }); return field_utils.format[this.field.type](value, this.field, options); }, + /** + * This method check if a value is the same as the current value of the + * field. For example, a fieldDate widget might want to use the moment + * specific value isSame instead of ===. + * + * This method is used by the _setValue method. + * + * @private + * @param {any} value + * @returns {boolean} + */ + _isSameValue: function (value) { + return this.value === value; + }, /** * convert a string representation to a valid value, depending on the field * type. @@ -330,7 +344,7 @@ var AbstractField = Widget.extend({ _setValue: function (value) { // we try to avoid doing useless work, if the value given has not // changed. Note that we compare the unparsed values. - if (this.lastSetValue === value || (value !== false && value === this.value)) { + if (this.lastSetValue === value) { return; } this.lastSetValue = value; @@ -341,6 +355,9 @@ var AbstractField = Widget.extend({ this._isValid = false; return; } + if (this._isSameValue(value)) { + return; + } var changes = {}; changes[this.name] = value; this.trigger_up('field_changed', { diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index d1569797731..dd87d893cc0 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -264,7 +264,7 @@ var FieldDate = InputField.extend({ if (this.mode === 'edit') { this.datewidget = this._makeDatePicker(); this.datewidget.on('datetime_changed', this, function () { - var value = this._getDateWidgetValue(); + var value = this._getValue(); if ((!value && this.value) || (value && !value.isSame(this.value))) { this._setValue(value); } @@ -295,10 +295,20 @@ var FieldDate = InputField.extend({ * * @private */ - _getDateWidgetValue: function () { + _getValue: function () { return this.datewidget.get_value(); }, - + /** + * @override + * @private + * @param {Moment|false} value + */ + _isSameValue: function (value) { + if (value === false) { + return this.value === false; + } + return value.isSame(this.value); + }, /** * Instantiates a new DateWidget datepicker. * @@ -343,7 +353,7 @@ var FieldDateTime = FieldDate.extend({ * * @private */ - _getDateWidgetValue: function () { + _getValue: function () { var value = this.datewidget.get_value(); return value && value.add(-this.getSession().tzOffset, 'minutes'); }, diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index 4e455b290fa..d6ea4a946be 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -1614,6 +1614,86 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('one2many and onchange (with integer)', function (assert) { + assert.expect(4); + + this.data.turtle.onchanges = { + turtle_int: function (obj) {} + }; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch:'
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + mockRPC: function (route, args) { + assert.step(args.method); + return this._super.apply(this, arguments); + }, + }); + form.$buttons.find('.o_form_button_edit').click(); + + form.$('td:contains(9)').click(); + form.$('td input[name="turtle_int"]').val("3").trigger('input'); + + // the 'change' event is triggered on the input when we focus somewhere + // else, for example by clicking in the body. However, if we try to + // programmatically click in the body, it does not trigger a change + // event, so we simply trigger it directly instead. + form.$('td input[name="turtle_int"]').trigger('change'); + + assert.verifySteps(['read', 'read', 'onchange']); + form.destroy(); + }); + + QUnit.test('one2many and onchange (with date)', function (assert) { + assert.expect(6); + + this.data.partner.onchanges = { + date: function (obj) {} + }; + this.data.partner.records[0].p = [2]; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch:'
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + mockRPC: function (route, args) { + assert.step(args.method); + return this._super.apply(this, arguments); + }, + }); + form.$buttons.find('.o_form_button_edit').click(); + + form.$('td:contains(01/25/2017)').click(); + form.$('.o_datepicker_input').click(); + form.$('.bootstrap-datetimepicker-widget .picker-switch').first().click(); // Month selection + form.$('.bootstrap-datetimepicker-widget .picker-switch').first().click(); // Year selection + form.$('.bootstrap-datetimepicker-widget .year:contains(2017)').click(); + form.$('.bootstrap-datetimepicker-widget .month').eq(1).click(); // February + form.$('.day:contains(22)').click(); // select the 22 February + + form.$buttons.find('.o_form_button_save').click(); + + assert.verifySteps(['read', 'read', 'onchange', 'write', 'read']); + form.destroy(); + }); + QUnit.test('one2many list (editable): readonly domain is evaluated', function (assert) { assert.expect(2);