From 13c885195d4e5be352fe4b92c5db04e331d8c3b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 2 Feb 2018 22:44:05 +0100
Subject: [PATCH] [FIX] web: make sure onchanges are properly triggered

The issue adressed by this fix was observed by a problem with a missing
onchange. The situation only happens when we have a one2many in a form
view with an onchange in a field in the list view, and the arch of the
list view is not inline.  In that case, the onchange was not properly
triggered.

There are two causes for this bug:
- the BasicModel does not exactly receive the same data when loading a
  record in a one2many with a non inline view.  More specifically, there
  is a confusion with the fields/viewFields in that case.  This is done
  because the postprocessing of the fields is slightly different when a
  sub view is not inline (not ideal situation, we should fix it btw)
- when it construct a subrecord, the BasicModel did not use the correct
  fields value, when there was a subview.  More specifically, it used
  the view.fields (which does not contain onchange informations) instead
  of the view.viewFields (which contains processed/view specific
  informations, including the onchange data.

This commit fixes the second issue, because this is clearly a bug, and
it actually fixes the problem.  The first issue is actually okay,
because we already have a difference of behaviour between the main view
and sub views (in the first case, we have the fieldget information, not
in the second case, so the processing is already a little bit different
anyway)

Notes:
- This bug was discovered only because we added a test in 11.0, which
  was forwardported and broke in master.  The breakage occured because
  of a recent refactoring in master which moved the arch processing
  inside the views and out of the data manager.
- It is good that we discovered this bug, but I want to say that it really
  sucks that forwardports are done, break the master/saas.11.2, then I
  have to fix it in emergency, and it completely break my own
  schedule...

wip

wip
---
 .../static/src/js/views/basic/basic_model.js  |  2 +-
 .../tests/fields/relational_fields_tests.js   | 37 +++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

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 d2dcaccb981..119adaacec4 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -2361,7 +2361,7 @@ var BasicModel = AbstractModel.extend({
                     count: ids.length,
                     context: record.context,
                     fieldsInfo: fieldsInfo,
-                    fields: view ? view.fields : fieldInfo.relatedFields,
+                    fields: view ? view.viewFields : fieldInfo.relatedFields,
                     limit: fieldInfo.limit,
                     modelName: field.relation,
                     res_ids: ids,
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index ef10a0aeb80..0a1fd9128e6 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -7834,6 +7834,43 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('onchange in a one2Many, but not in main record, with non inline view', function (assert) {
+        assert.expect(6);
+
+        this.data.partner.fields.sequence = {string: 'Sequence', type: 'integer'};
+        this.data.partner.records[0].sequence = 1;
+        this.data.partner.records[1].sequence = 2;
+        this.data.partner.onchanges = {sequence: function () {}};
+
+        this.data.partner_type.fields.partner_ids = {string: "Partner", type: "one2many", relation: 'partner'};
+        this.data.partner_type.records[0].partner_ids = [1,2];
+
+        var form = testUtils.createView({
+            View: FormView,
+            model: 'partner_type',
+            data: this.data,
+            arch: '<form><field name="partner_ids"/></form>',
+            archs: {
+                'partner,false,list': '<tree string="Vendors">' +
+                                            '<field name="sequence" widget="handle"/>' +
+                                            '<field name="display_name"/>' +
+                                      '</tree>',
+            },
+            res_id: 12,
+            mockRPC: function (route, args) {
+                assert.step(args.method);
+                return this._super.apply(this, arguments);
+            },
+            viewOptions: {mode: 'edit'},
+        });
+
+        // swap 2 lines in the one2many
+        testUtils.dragAndDrop(form.$('.ui-sortable-handle:eq(1)'), form.$('tbody tr').first(),
+                                {position: 'top'});
+        assert.verifySteps(['load_views', 'read', 'read', 'onchange', 'onchange']);
+        form.destroy();
+    });
+
     QUnit.module('FieldMany2Many');
 
     QUnit.test('many2many kanban: edition', function (assert) {