[FIX] web, purchase_product_matrix: can open twice the matrix

Let's assume the following scenario with 'purchase_product_matrix'
module installed:
 - create a new Purchase Order
 - add a line in the one2many
 - select "Customizable desk" as product
 - [the desk matrix opens]
 - close the matrix
 - select "Conference chair" instead
 - [the desk matrix opens, whereas it should be the chair one]

It didn't work because of a small bug in the FieldOne2Many. This
field is configured to be reset when any other field in the view
changes. The product configurator feature relies on that. However,
when another field changes, the One2Many didn't update its internal
state with the new record (and it skipped the rendering, which is
fine). The matrix product configurator thus read an obsolete value
in the internal of the one2many.

Since the internal state is now always up-to-date, we can directly
read there the grid information, instead of looking inside other
fields that have been updated (attempt done in [1])

It would be nice to rethink the whole product configurator stuff
in master, to make it more robust, maybe when converting it to owl?

[1] https://github.com/odoo/odoo/commit/21bcafc053be7f80eed7a684fee2dc1e3caaafa5

opw~2421798

closes odoo/odoo#64552

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

4 files changed, 131 insertions, 31 deletions

diff --git a/addons/product_matrix/static/src/js/section_and_note_widget.js b/addons/product_matrix/static/src/js/section_and_note_widget.js
index 3cdddf0f596..650efbf22af 100644
--- a/addons/product_matrix/static/src/js/section_and_note_widget.js
+++ b/addons/product_matrix/static/src/js/section_and_note_widget.js
@@ -71,17 +71,8 @@ SectionAndNoteFieldOne2Many.include({
                 grid_product_tmpl_id: {id: productTemplateId}
             },
             viewType: 'form',
-            onSuccess: function (result) {
-                // result = list of widgets
-                // find one of the SO widget
-                // (not so lines because the grid values are computed on the SO)
-                // and get the grid information from its recordData.
-                var gridInfo = result.find(
-                    r => r.recordData.grid && r.recordData.grid.includes("header")
-                ).recordData.grid;
-                // includes "header" is necessary to ensure the correct data is taken
-                // because the grid data isn't correctly updated in all widgets
-                // when in an new record environment.
+            onSuccess: function () {
+                const gridInfo = self.recordData.grid;
                 self._openMatrixConfigurator(gridInfo, productTemplateId, editedCellAttributes);
             }
         });
diff --git a/addons/purchase_product_matrix/static/tests/section_and_note_widget_tests.js b/addons/purchase_product_matrix/static/tests/section_and_note_widget_tests.js
index 0d60fc62a58..de40d33641f 100644
--- a/addons/purchase_product_matrix/static/tests/section_and_note_widget_tests.js
+++ b/addons/purchase_product_matrix/static/tests/section_and_note_widget_tests.js
@@ -5,6 +5,21 @@ var FormView = require('web.FormView');
 var testUtils = require('web.test_utils');
 var createView = testUtils.createView;
 
+function getGrid(product) {
+    return JSON.stringify({
+        header: [{name: product.name}, {name: "M"}, {name: "L"}],
+        matrix: [[
+            {name: "Men"},
+            {ptav_ids: [10, 13], qty: 0, is_possible_combination: true},
+            {ptav_ids: [11, 13], qty: 0, is_possible_combination: true},
+        ], [
+            {name: "Women"},
+            {ptav_ids: [10, 14], qty: 0, is_possible_combination: true},
+            {ptav_ids: [11, 14], qty: 0, is_possible_combination: true},
+        ]],
+    });
+}
+
 QUnit.module('section_and_note: purchase_product_matrix', {
     beforeEach: function () {
         this.data = {
@@ -17,6 +32,16 @@ QUnit.module('section_and_note: purchase_product_matrix', {
                         relation_field: 'order_id',
                     },
                     grid: {string: "Grid", type: 'char'},
+                    grid_product_tmpl_id: {string: "Grid Product", type: 'many2one', relation: 'product'},
+                },
+                onchanges: {
+                    grid_product_tmpl_id: (obj) => {
+                        const product = this.data.product.records.find((p) => {
+                            return p.id === obj.grid_product_tmpl_id;
+                        });
+                        obj.grid = product ? getGrid(product) : false;
+                    },
+                    grid: () => {},
                 },
             },
             order_line: {
@@ -34,36 +59,18 @@ QUnit.module('section_and_note: purchase_product_matrix', {
                 ],
             },
         };
-
-        this.grid = JSON.stringify({
-            header: [{name: "My Company Tshirt (GRID)"}, {name: "M"}, {name: "L"}],
-            matrix: [[
-                {name: "Men"},
-                {ptav_ids: [10, 13], qty: 0, is_possible_combination: true},
-                {ptav_ids: [11, 13], qty: 0, is_possible_combination: true},
-            ], [
-                {name: "Women"},
-                {ptav_ids: [10, 14], qty: 0, is_possible_combination: true},
-                {ptav_ids: [11, 14], qty: 0, is_possible_combination: true},
-            ]],
-        });
     },
 }, function () {
     QUnit.test('can configure a product with the matrix', async function (assert) {
         assert.expect(4);
 
-        this.data.purchase_order.onchanges = {
-            order_line_ids: obj => {
-                obj.grid = this.grid;
-            },
-            grid: () => {},
-        };
         var form = await createView({
             View: FormView,
             model: 'purchase_order',
             data: this.data,
             arch: `<form>
                     <field name="grid" invisible="1"/>
+                    <field name="grid_product_tmpl_id" invisible="1"/>
                     <field name="order_line_ids" widget="section_and_note_one2many">
                         <tree editable="bottom">
                             <field name="product_template_id" widget="matrix_configurator"/>
@@ -93,7 +100,70 @@ QUnit.module('section_and_note: purchase_product_matrix', {
         assert.containsOnce(document.body, '.modal .o_product_variant_matrix');
         const $matrix = $('.modal .o_product_variant_matrix');
         assert.strictEqual($matrix.text().replace(/[\n\r\s\u00a0]+/g, ' '),
-            ' My Company Tshirt (GRID) M L Men Women ');
+            ' A configurable product M L Men Women ');
+
+        // select 2 M-Men and 3 L-Women
+        await testUtils.fields.editInput($matrix.find('.o_matrix_input[ptav_ids="10,13"]'), '2');
+        await testUtils.fields.editInput($matrix.find('.o_matrix_input[ptav_ids="11,14"]'), '3');
+        await testUtils.dom.click($('.modal .modal-footer .btn-primary'));
+
+        form.destroy();
+    });
+
+    QUnit.test('can open the matrix twice with 2 different products', async function (assert) {
+        assert.expect(5);
+
+        this.data.product.records.push({ id: 101, name: "Product A" });
+        this.data.product.records.push({ id: 102, name: "Product B" });
+
+        const form = await createView({
+            View: FormView,
+            model: 'purchase_order',
+            data: this.data,
+            arch: `<form>
+                    <field name="grid" invisible="1"/>
+                    <field name="grid_product_tmpl_id" invisible="1"/>
+                    <field name="order_line_ids" widget="section_and_note_one2many">
+                        <tree editable="bottom">
+                            <field name="product_template_id" widget="matrix_configurator"/>
+                        </tree>
+                    </field>
+                </form>`,
+            mockRPC: function (route, args) {
+                if (args.method === 'onchange' && args.args[2] === 'grid') {
+                    // should trigger an onchange on the grid field and let the
+                    // business logic create rows according to the matrix content
+                    assert.deepEqual(args.args[1].grid, JSON.stringify({
+                        changes: [{qty: 2, ptav_ids: [10, 13]}, {qty: 3, ptav_ids: [11, 14]}],
+                        product_template_id: 102,
+                    }));
+                }
+                if (args.method === 'get_single_product_variant') {
+                    return Promise.resolve({mode: 'matrix'});
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        // open the matrix with "Product A" and close it
+        await testUtils.dom.click('.o_field_x2many_list_row_add a');
+        await testUtils.fields.many2one.searchAndClickItem("product_template_id", {item: 'Product A'});
+
+        assert.containsOnce(document.body, '.modal .o_product_variant_matrix');
+        let $matrix = $('.modal .o_product_variant_matrix');
+        assert.strictEqual($matrix.text().replace(/[\n\r\s\u00a0]+/g, ' '),
+            ' Product A M L Men Women ');
+
+        await testUtils.dom.click($('.modal .modal-footer .btn-secondary')); // close
+
+        // re-open the matrix with "Product B"
+        await testUtils.dom.click('.o_field_x2many_list_row_add a');
+        await testUtils.fields.many2one.searchAndClickItem("product_template_id", {item: 'Product B'});
+
+        assert.containsOnce(document.body, '.modal .o_product_variant_matrix');
+        $matrix = $('.modal .o_product_variant_matrix');
+        assert.strictEqual($matrix.text().replace(/[\n\r\s\u00a0]+/g, ' '),
+            ' Product B M L Men Women ');
 
         // select 2 M-Men and 3 L-Women
         await testUtils.fields.editInput($matrix.find('.o_matrix_input[ptav_ids="10,13"]'), '2');
diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index b97cecdd49e..55ed1ecb731 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -1192,6 +1192,7 @@ var FieldX2Many = AbstractField.extend(WidgetAdapterMixin, {
         if (!fieldChanged && !actionsChanged) {
             var newEval = this._evalColumnInvisibleFields();
             if (_.isEqual(this.currentColInvisibleFields, newEval)) {
+                this._reset(record, ev); // update the internal state, but do not re-render
                 return Promise.resolve();
             }
         } else if (ev && ev.target === this && ev.data.changes && this.view.arch.tag === 'tree') {
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 f8ff8cba1eb..bb64e3f0775 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
@@ -16,6 +16,7 @@ var fieldUtils = require('web.field_utils');
 
 const cpHelpers = testUtils.controlPanel;
 var createView = testUtils.createView;
+const { FieldOne2Many } = relationalFields;
 
 QUnit.module('fields', {}, function () {
 
@@ -9513,6 +9514,43 @@ QUnit.module('fields', {}, function () {
 
             assert.verifySteps(["willUnmount"]);
         });
+
+        QUnit.test('one2many: internal state is updated after another field changes', async function (assert) {
+            // The FieldOne2Many is configured such that it is reset at any field change.
+            // The MatrixProductConfigurator feature relies on that, and requires that its
+            // internal state is correctly updated. This white-box test artificially checks that.
+            assert.expect(2);
+
+            let o2m;
+            testUtils.patch(FieldOne2Many, {
+                init() {
+                    this._super(...arguments);
+                    o2m = this;
+                },
+            });
+
+            const form = await createView({
+                View: FormView,
+                model: 'partner',
+                data: this.data,
+                arch: `
+                    <form>
+                        <field name="display_name"/>
+                        <field name="p">
+                            <tree><field name="display_name"/></tree>
+                        </field>
+                    </form>`,
+            });
+
+            assert.strictEqual(o2m.recordData.display_name, false);
+
+            await testUtils.fields.editInput(form.$('.o_field_widget[name=display_name]'), 'val');
+
+            assert.strictEqual(o2m.recordData.display_name, "val");
+
+            form.destroy();
+            testUtils.unpatch(FieldOne2Many);
+        });
     });
 });
 });