[FIX] web: nested one2manys, onchange and no command

Let's assume the following situation. We have a form view with a
one2many field A displayed as a list. In the list, there is a
one2many field B. B can't be edited, its value is computed by
an onchange. By default, it contains a single record (i.e. the
first value returned by the onchange is [[5], [0, 0, {...}]]).
When another field (say C) changes, B's value is re-computed to
[[5]]. Moreover, there is an onchange on A.

In this form view, let's assume the following scenario. Create a
new record and add a line to A. In this new line, B already
contains a record. Change C. This triggers an onchange that
returns [[5]], and B is now empty. It triggers a second onchange,
on the main record (as field A changed).

Before this commit, in this second onchange, B's value wasn't sent
among the other values of the new line.

The spec says that for onchanges, we must send all data, not only
what has really changed. From that perspective, the above scenario
highlights an issue.

That issue had two root causes. First, commit [1] wrongly fixed
another issue, and as a consequence, when building what to send
for the onchange, we didn't generate the values for fields that
hadn't changed inside an x2many (for added subrecords at least).
This commit reverts the fix of [1], and fixes it differently by
only sending a command 1 (update) after a command 4 (link to)
when the record is dirty (i.e. when it has been modified). See
[1] for context and details.

Second, the code that generates the values to send to onchanges is
the same as the one that generates the values to save records
(write or create). However, when saving, we only send what has
really changed. The values are at some point processed to remove
empty command lists from the list of changes (as it means that
nothing changed). However, here we ignored the flag that stated
whether we want all field values or just what has changed. This
commit takes the flag into account before removing the field's
value.

[1] https://github.com/odoo/odoo/commit/3e3a244e1afc4d74920a6302a14fa2590e8b6648

Issue reported in task~2352524
  Model: account.move
  One2Many (A): account.move.lines
  Nested computed One2Many (B): tax_detail_ids
  Field triggering the onchange (C): tax_ids

closes odoo/odoo#67679

Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>
Co-authored-by: Géry Debongnie <ged@odoo.com>

2 files changed, 77 insertions, 1 deletions

diff --git a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
index f4d9fba9eec..622eece2782 100644
--- a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
+++ b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
@@ -9693,6 +9693,82 @@ QUnit.module('fields', {}, function () {
             form.destroy();
             testUtils.unpatch(FieldOne2Many);
         });
+
+        QUnit.test('nested one2many, onchange, no command value', async function (assert) {
+            // This test ensures that we always send all values to onchange rpcs for nested
+            // one2manys, even if some field hasn't changed. In this particular test case,
+            // a first onchange returns a value for the inner one2many, and a second onchange
+            // removes it, thus restoring the field to its initial empty value. From this point,
+            // the nested one2many value must still be sent to onchange rpcs (on the main record),
+            // as it might be used to compute other fields (so the fact that the nested o2m is empty
+            // must be explicit).
+            assert.expect(3);
+
+            this.data.turtle.fields.o2m = {
+                string: "o2m", type: "one2many", relation: 'partner', relation_field: 'trululu',
+            };
+            this.data.turtle.fields.turtle_bar.default = true;
+            this.data.partner.onchanges.turtles = function (obj) {};
+            this.data.turtle.onchanges.turtle_bar = function (obj) {
+                if (obj.turtle_bar) {
+                    obj.o2m = [[5], [0, false, { display_name: "default" }]];
+                } else {
+                    obj.o2m = [[5]];
+                }
+            };
+
+            let step = 1;
+            const form = await createView({
+                View: FormView,
+                model: 'partner',
+                data: this.data,
+                arch: `<form>
+                        <field name="turtles">
+                            <tree editable="bottom">
+                                <field name="o2m"/>
+                                <field name="turtle_bar"/>
+                            </tree>
+                        </field>
+                    </form>`,
+                async mockRPC(route, args) {
+                    if (step === 3 && args.method === 'onchange' && args.model === 'partner') {
+                        assert.deepEqual(args.args[1].turtles[0][2], {
+                            turtle_bar: false,
+                            o2m: [], // we must send a value for this field
+                        });
+                    }
+                    const result = await this._super(...arguments);
+                    if (args.model === 'turtle') {
+                        // sanity checks; this is what the onchanges on turtle must return
+                        if (step === 2) {
+                            assert.deepEqual(result.value, {
+                                o2m: [[5], [0, false, { display_name: "default" }]],
+                                turtle_bar: true,
+                            });
+                        }
+                        if (step === 3) {
+                            assert.deepEqual(result.value, {
+                                o2m: [[5]],
+                            });
+                        }
+                    }
+                    return result;
+                },
+            });
+
+            step = 2;
+            await testUtils.dom.click(form.$('.o_field_x2many_list .o_field_x2many_list_row_add a'));
+            // use of owlCompatibilityNextTick because we have an x2many field with a boolean field
+            // (written in owl), so when we add a line, we sequentially render the list itself
+            // (including the boolean field), so we have to wait for the next animation frame, and
+            // then we render the control panel (also in owl), so we have to wait again for the
+            // next animation frame
+            await testUtils.owlCompatibilityNextTick();
+            step = 3;
+            await testUtils.dom.click(form.$('.o_data_row .o_field_boolean input'));
+
+            form.destroy();
+        });
     });
 });
 });
diff --git a/addons/web/static/tests/views/basic_model_tests.js b/addons/web/static/tests/views/basic_model_tests.js
index dd802790210..a97fb0ddd1c 100644
--- a/addons/web/static/tests/views/basic_model_tests.js
+++ b/addons/web/static/tests/views/basic_model_tests.js
@@ -1145,7 +1145,7 @@ odoo.define('web.basic_model_tests', function (require) {
                 mockRPC: function (route, args) {
                     if (args.method === 'create') {
                         // has to be done before the call to _super
-                        assert.notOk('product_ids' in args.args[0], "should not have any value");
+                        assert.deepEqual(args.args[0].product_ids, [], "should not have any command");
                         assert.notOk('category' in args.args[0], "should not have other fields");
 
                         assert.strictEqual(args.kwargs.context.active_field, 2,