[FIX] web: issue with field named "length"

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) <aab@odoo.com>
Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>

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

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: '<form><field name="foo"/><field name="length"/></form>',
+        });
+
+        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: `<form string="Partners">
+                    <field name="p">
+                        <tree>
+                            <field name="display_name"/>
+                        </tree>
+                        <form string="Partners">
+                            <field name="length"/>
+                            <field name="display_name"/>
+                            <field name="foo" attrs="{\'readonly\': [[\'display_name\', \'=\', \'readonly\']]}"/>
+                        </form>
+                    </field>
+                </form>`,
+            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();
+    });
+
 });
 
 });