From 0cf7b8fd6f80c643fb53a980cf551990ca8070da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 3 Mar 2021 10:02:33 +0000 Subject: [PATCH] [FIX] web: issue with field named "length" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The underscore (_) library has a bug in which the _.each method does not work with object which contains a "length" property. This is because it does look for that key and if it is a number, it will assume that it is an array with that length value. Nicely done... If that length value is set to 0, then it will just do nothing, since it thinks that it is dealing with an empty array. Note that if the value is set to an object, _.each is smart enough to notice that it cannot be an array, and will do the correct thing in this case. Usually, our _.each calls are safe, since we usually iterate on arrays, or on object with safe keys, or on object with values that cannot be a number. But there was 2 unsafe calls in basic_model, which leads to strange bugs: some code is skipped, and the form view is then confused. The motivation for this fix is the fact that onchanges are not applied at all, if there is a length field set to 0. To fix this, we can just avoid using _.each. Note to every Odoo JS developers reading this: new Odoo code should avoid using the _ and $ libraries, because we do not really need them, and we want to keep our dependencies to the strict mininum. OPW: #2465808 closes odoo/odoo#66126 closes odoo/odoo#67172 X-original-commit: 8dbd1efef899fb637acca3c318ae99cb23838b8f Signed-off-by: Aaron Bohy (aab) Signed-off-by: Géry Debongnie (ged) --- .../static/src/js/views/basic/basic_model.js | 11 +-- addons/web/static/tests/views/form_tests.js | 72 +++++++++++++++++++ 2 files changed, 78 insertions(+), 5 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 d90c7ff5892..cfeb1e8c305 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1122,9 +1122,9 @@ var BasicModel = AbstractModel.extend({ // save the viewType of edition, so that the correct readonly modifiers // can be evaluated when the record will be saved - _.each((record._changes || {}), function (value, fieldName) { + for (let fieldName in (record._changes || {})) { record._editionViewType[fieldName] = options.viewType; - }); + } } var shouldReload = 'reload' in options ? options.reload : true; var method = self.isNew(recordID) ? 'create' : 'write'; @@ -1695,7 +1695,8 @@ var BasicModel = AbstractModel.extend({ const viewType = options.viewType || record.viewType; record._changes = record._changes || {}; - _.each(values, function (val, name) { + for (let name in (values || {})) { + const val = values[name]; var field = record.fields[name]; if (!field) { // this field is unknown so we can't process it for now (it is not @@ -1714,7 +1715,7 @@ var BasicModel = AbstractModel.extend({ // if (options.firstOnChange) { // record._changes[name] = val; // } - return; + continue; } if (record._rawChanges[name]) { // if previous _rawChanges exists, clear them since the field is now knwon @@ -1876,7 +1877,7 @@ var BasicModel = AbstractModel.extend({ record._changes[name] = newValue; } } - }); + } return Promise.all(defs); // inner function that adds a record (based on its res_id) to a list diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index b3685b84128..c7152ae7c8a 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/addons/web/static/tests/views/form_tests.js @@ -11017,6 +11017,78 @@ QUnit.module('Views', { delete fieldRegistry.map.customwidget; }); + + QUnit.test('field "length" with value 0: can apply onchange', async function (assert) { + assert.expect(1); + + this.data.partner.fields.length = {string: 'Length', type: 'float', default: 0 }; + this.data.partner.fields.foo.default = "foo default"; + + const form = await createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
', + }); + + assert.strictEqual(form.$('input[name=foo]').val(), "foo default", + "should contain input with initial value"); + + form.destroy(); + }); + + QUnit.test('field "length" with value 0: readonly fields are not sent when saving', async function (assert) { + assert.expect(3); + + this.data.partner.fields.length = {string: 'Length', type: 'float', default: 0 }; + this.data.partner.fields.foo.default = "foo default"; + + // define an onchange on display_name to check that the value of readonly + // fields is correctly sent for onchanges + this.data.partner.onchanges = { + display_name: function () {}, + p: function () {}, + }; + + const form = await createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: `
+ + + + + + + + + + + `, + mockRPC: function (route, args) { + if (args.method === 'create') { + assert.deepEqual(args.args[0], { + p: [[0, args.args[0].p[0][1], {length: 0, display_name: 'readonly'}]] + }, "should not have sent the value of the readonly field"); + } + return this._super.apply(this, arguments); + }, + }); + + await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + assert.containsOnce(document.body, '.modal input.o_field_widget[name=foo]', + 'foo should be editable'); + await testUtils.fields.editInput($('.modal .o_field_widget[name=foo]'), 'foo value'); + await testUtils.fields.editInput($('.modal .o_field_widget[name=display_name]'), 'readonly'); + assert.containsOnce(document.body, '.modal span.o_field_widget[name=foo]', + 'foo should be readonly'); + await testUtils.dom.clickFirst($('.modal-footer .btn-primary')); + + await testUtils.form.clickSave(form); // save the record + form.destroy(); + }); + }); });