From a71f22370217d2da6ad13bd29c2fa412a40ebe4c Mon Sep 17 00:00:00 2001 From: Adrien Dieudonne Date: Tue, 25 Jul 2017 11:34:11 +0200 Subject: [PATCH] [FIX] web: InputField: don't re-render if dirty Let's assume that we have a form view with a one2many field displayed as a list containing two fields A and B (an input field, e.g. a char field). An onchange on the one2many is triggered as soon as A changes. This triggers a reset of the one2many which thus redraws the corresponding line by resetting its widgets. A problem occured when the user updated A, and directly (before the onchange returned) updated the input field B, as in this case the new value in B was erased when the onchanged returned, as the widget was reset with its former value (or with the value returned by the onchange, if any). Note that the model was aware of the change on B, so the model and the UI were actually desynchronized. This was quite hard to reproduce in practice, by hand, but it occured in the tour testing 'the flow', in the form view of account.bank_statement, with fields partner_id and amount of the line_ids one2many. With this rev., we don't re-render the input fields if there are pending changes that haven't been acknowledged by the model. --- .../web/static/src/js/fields/basic_fields.js | 33 ++++- .../static/tests/fields/basic_fields_tests.js | 116 ++++++++++++++++++ 2 files changed, 146 insertions(+), 3 deletions(-) diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index ecf8661a384..0cb08b55d02 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -151,6 +151,9 @@ var DebouncedField = AbstractField.extend({ }); var InputField = DebouncedField.extend({ + custom_events: _.extend({}, DebouncedField.prototype.custom_events, { + field_changed: '_onFieldChanged', + }), events: _.extend({}, DebouncedField.prototype.events, { 'input': '_onInput', 'change': '_onChange', @@ -168,6 +171,12 @@ var InputField = DebouncedField.extend({ if (this.mode === 'edit') { this.tagName = 'input'; } + // We need to know if the widget is dirty (i.e. if the user has changed + // the value, and those changes haven't been acknowledged yet by the + // environment), to prevent erasing that new value on a reset (e.g. + // coming by an onchange on another field) + this.isDirty = false; + this.lastChangeEvent = undefined; }, //-------------------------------------------------------------------------- @@ -183,14 +192,21 @@ var InputField = DebouncedField.extend({ return this.$input || $(); }, /** - * Do not re-render this field if it was the origin of the onchange call. - * FIXME: make the onchange work on itself without disturbing the user typing + * Re-renders the widget if it isn't dirty. The widget is dirty if the user + * changed the value, and that change hasn't been acknowledged yet by the + * environment. For example, another field with an onchange has been updated + * and this field is updated before the onchange returns. Two '_setValue' + * are done (this is sequential), the first one returns and this widget is + * reset. However, it has pending changes, so we don't re-render. * * @override */ reset: function (record, event) { this._reset(record, event); - if (event && event.target === this) { + if (!event || event === this.lastChangeEvent) { + this.isDirty = false; + } + if (this.isDirty || (event && event.target === this)) { return $.when(); } else { return this._render(); @@ -265,6 +281,16 @@ var InputField = DebouncedField.extend({ _onChange: function () { this._doAction(); }, + /** + * Listens to events 'field_changed' to keep track of the last event that + * has been trigerred. This allows to detect that all changes have been + * acknowledged by the environment. + * + * @param {OdooEvent} event 'field_changed' event + */ + _onFieldChanged: function (event) { + this.lastChangeEvent = event; + }, /** * Called when the user is typing text -> By default this only calls a * debounced method to notify the outside world of the changes. @@ -273,6 +299,7 @@ var InputField = DebouncedField.extend({ * @private */ _onInput: function () { + this.isDirty = true; this._doDebouncedAction(); }, /** diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js index c732fe35ce6..8f1c04b6bb8 100644 --- a/addons/web/static/tests/fields/basic_fields_tests.js +++ b/addons/web/static/tests/fields/basic_fields_tests.js @@ -831,6 +831,122 @@ QUnit.module('basic_fields', { form.destroy(); }); + QUnit.test('input field: change value before pending onchange returns', function (assert) { + assert.expect(3); + + this.data.partner.onchanges = { + product_id: function () {}, + }; + + var def; + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + mockRPC: function (route, args) { + var result = this._super.apply(this, arguments); + if (args.method === "onchange") { + return $.when(def).then(function () { + return result; + }); + } else { + return result; + } + }, + viewOptions: { + mode: 'edit', + }, + }); + + form.$('.o_field_x2many_list_row_add a').click(); + assert.strictEqual(form.$('input[name="foo"]').val(), 'My little Foo Value', + 'should contain the default value'); + + def = $.Deferred(); + form.$('.o_field_many2one input').click(); + var $dropdown = form.$('.o_field_many2one input').autocomplete('widget'); + $dropdown.find('li:first()').click(); + + // set foo before onchange + form.$('input[name="foo"]').val("tralala").trigger('input'); + assert.strictEqual(form.$('input[name="foo"]').val(), 'tralala', + 'input should contain tralala'); + + // complete the onchange + def.resolve(); + assert.strictEqual(form.$('input[name="foo"]').val(), 'tralala', + 'input should contain the same value as before onchange'); + + form.destroy(); + }); + + QUnit.test('input field: change value before pending onchange renaming', function (assert) { + assert.expect(3); + + this.data.partner.onchanges = { + product_id: function (obj) { + obj.foo = 'on change value'; + }, + }; + + var def = $.Deferred(); + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + mockRPC: function (route, args) { + var result = this._super.apply(this, arguments); + if (args.method === "onchange") { + return $.when(def).then(function () { + return result; + }); + } else { + return result; + } + }, + viewOptions: { + mode: 'edit', + }, + }); + + assert.strictEqual(form.$('input[name="foo"]').val(), 'yop', + 'should contain the correct value'); + + form.$('.o_field_many2one input').click(); + var $dropdown = form.$('.o_field_many2one input').autocomplete('widget'); + $dropdown.find('li:first()').click(); + + // set foo before onchange + form.$('input[name="foo"]').val("tralala").trigger('input'); + assert.strictEqual(form.$('input[name="foo"]').val(), 'tralala', + 'input should contain tralala'); + + // complete the onchange + def.resolve(); + assert.strictEqual(form.$('input[name="foo"]').val(), 'tralala', + 'input should contain the same value as before onchange'); + + form.destroy(); + }); + QUnit.module('UrlWidget'); QUnit.test('url widget in form view', function (assert) {