From ae4501da9c997fcc86fb2ce0e032cd558cc1534e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 9 Nov 2017 09:40:54 +0100 Subject: [PATCH] [FIX] web: do not lose onchange info in some rare cases Here is a situation where we had a problem: - a form view with a one2many field, which has no inline views - the (non inline) list view has a field A, and is not editable - the (non inline) sub form view has fields A and B, with an onchange on B which modifies A In that case, the user could do this: - go to edit mode - click on 'add' a new one2many line - change the value of B in the form view, this changes the value of A - click on save to close the modal form view - click on the new o2m record to reopen the modal form view - rechange B => the onchange does not work The explanation is that when we reopen the modal, we update the known fields information, but we had to perform a fieldviewget to fetch the list view, so we have a full knowledge of the fields. However, the code did not update the fields info (because it uses _.default), which means that the onchange information contained in the form view is lost. Note: the test system had to be adapted to more closely simulate what actually happens. In particular, the onchange flag is no longer added by the mock server, since it should be done by the data manager, like 'real' code. This change broke the basic model tests, which had to be modified accordingly, by setting manually the onchange flag. --- .../static/src/js/views/basic/basic_model.js | 4 +- .../tests/fields/relational_fields_tests.js | 40 +++++++++++++++++++ .../web/static/tests/helpers/mock_server.js | 5 +-- .../static/tests/views/basic_model_tests.js | 8 ++++ 4 files changed, 51 insertions(+), 6 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 445967287bf..7e702e30862 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -949,8 +949,8 @@ var BasicModel = AbstractModel.extend({ */ addFieldsInfo: function (recordID, viewInfo) { var record = this.localData[recordID]; - record.fields = _.defaults(record.fields, viewInfo.fields); - record.fieldsInfo = _.defaults(record.fieldsInfo, viewInfo.fieldsInfo); + record.fields = _.extend({}, record.fields, viewInfo.fields); + record.fieldsInfo = _.extend({}, record.fieldsInfo, viewInfo.fieldsInfo); }, /** * Manually sets a resource as dirty. This is used to notify that a field diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index 8f789c88647..8a67f6eec05 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -135,6 +135,7 @@ QUnit.module('relational_fields', { partner_ids: [], turtle_ref: 'product,37', }], + onchanges: {}, }, user: { fields: { @@ -2677,6 +2678,45 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('edition of one2many field, with onchange and not inline sub view', function (assert) { + assert.expect(2); + + this.data.turtle.onchanges.turtle_int = function (obj) { + obj.turtle_foo = String(obj.turtle_int); + }; + this.data.partner.onchanges.turtles = function () {}; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '', + archs: { + 'turtle,false,list': '', + 'turtle,false,form': '
', + }, + mockRPC: function (route, args) { + return this._super.apply(this, arguments); + }, + res_id: 1, + }); + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_field_x2many_list_row_add a').click(); + $('input[name="turtle_int"]').val('5').trigger('input'); + $('.modal-footer button.btn-primary').first().click(); + assert.strictEqual(form.$('tbody tr:eq(1) td.o_data_cell').text(), '5', + 'should display 5 in the foo field'); + form.$('tbody tr:eq(1) td.o_data_cell').click(); + + $('input[name="turtle_int"]').val('3').trigger('input'); + $('.modal-footer button.btn-primary').first().click(); + assert.strictEqual(form.$('tbody tr:eq(1) td.o_data_cell').text(), '3', + 'should now display 3 in the foo field'); + form.destroy(); + }); + QUnit.test('sorting one2many fields', function (assert) { assert.expect(4); diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 76391e4cb7a..f78de2f5c0b 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -32,9 +32,6 @@ var MockServer = Class.extend({ if (!('name' in model.fields)) { model.fields.name = {string: "Name", type: "char", default: "name"}; } - for (var fieldName in model.onchanges) { - model.fields[fieldName].onChange = "1"; - } model.records = model.records || []; for (var i = 0; i < model.records.length; i++) { @@ -282,7 +279,7 @@ var MockServer = Class.extend({ // add onchanges if (name in onchanges) { - field.onChange="1"; + node.attrs.on_change="1"; } }); return { diff --git a/addons/web/static/tests/views/basic_model_tests.js b/addons/web/static/tests/views/basic_model_tests.js index b2a00f1ca09..8de3e3c91a2 100644 --- a/addons/web/static/tests/views/basic_model_tests.js +++ b/addons/web/static/tests/views/basic_model_tests.js @@ -203,6 +203,7 @@ QUnit.module('Views', { QUnit.test('basic onchange', function (assert) { assert.expect(5); + this.data.partner.fields.foo.onChange = true; this.data.partner.onchanges.foo = function (obj) { obj.bar = obj.foo.length; }; @@ -240,6 +241,7 @@ QUnit.module('Views', { QUnit.test('onchange with a many2one', function (assert) { assert.expect(5); + this.data.partner.fields.product_id.onChange = true; this.data.partner.onchanges.product_id = function (obj) { if (obj.product_id === 37) { obj.foo = "space lollipop"; @@ -280,6 +282,7 @@ QUnit.module('Views', { QUnit.test('onchange on a one2many not in view (fieldNames)', function (assert) { assert.expect(6); + this.data.partner.fields.foo.onChange = true; this.data.partner.onchanges.foo = function (obj) { obj.bar = obj.foo.length; obj.product_ids = []; @@ -425,6 +428,7 @@ QUnit.module('Views', { QUnit.test('onchange on a char with an unchanged many2one', function (assert) { assert.expect(2); + this.data.partner.fields.foo.onChange = true; this.data.partner.onchanges.foo = function (obj) { obj.foo = obj.foo + " alligator"; }; @@ -453,6 +457,7 @@ QUnit.module('Views', { QUnit.test('onchange on a char with another many2one not set to a value', function (assert) { assert.expect(2); this.data.partner.records[0].product_id = false; + this.data.partner.fields.foo.onChange = true; this.data.partner.onchanges.foo = function (obj) { obj.foo = obj.foo + " alligator"; }; @@ -1153,6 +1158,7 @@ QUnit.module('Views', { assert.expect(4); this.data.partner.fields.total.default = 50; + this.data.partner.fields.product_ids.onChange = true; this.data.partner.onchanges.product_ids = function (obj) { obj.total += 100; }; @@ -1497,6 +1503,7 @@ QUnit.module('Views', { assert.expect(6); this.params.fieldNames = ['foo', 'bar']; + this.data.partner.fields.foo.onChange = true; this.data.partner.onchanges.foo = function (obj) { obj.bar = obj.foo.length; }; @@ -1561,6 +1568,7 @@ QUnit.module('Views', { assert.expect(6); this.params.fieldNames = ['foo', 'bar']; + this.data.partner.fields.foo.onChange = true; this.data.partner.onchanges.foo = function (obj) { obj.bar = obj.foo.length; };