From bc89bb79d2eac7db2861127bc41dcaf131568d4b Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Wed, 5 Apr 2017 11:16:37 +0200
Subject: [PATCH] [FIX] web: BasicModel: ignore unknown fields after onchange

It may happen that an onchange returns values for unknown fields in
the view. In that case, the corresponding values need to be ignored
as the postprocessing base on the field type can't be done, which
may lead to errors later on (e.g. when saving).

For example: one2many list in a form view which doesn't display
a many2one field that exists on the related model. If an onchange
on the main form view updates the value of the many2one inside the
one2many, it will crash later on when the user will try to save
(because the value [id, display_name] will be sent to the server).
---
 .../static/src/js/views/basic/basic_model.js  | 15 +++--
 addons/web/static/test/views/form_tests.js    | 66 +++++++++++++++++++
 2 files changed, 75 insertions(+), 6 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 304f7e475f7..47d5cee5c9c 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -942,10 +942,16 @@ var BasicModel = AbstractModel.extend({
                                     params.res_id = command[1];
                                 }
                                 rec = self._makeDataPoint(params);
-                                var data = command[2];
-                                _.each(Object.keys(data), function (name) {
+                                var data = {};
+                                _.each(Object.keys(command[2]), function (name) {
                                     var field = rec.fields[name];
-                                    if (field && field.type === 'many2one') {
+                                    if (!field) {
+                                        // ignore if this field is unknown as we don't
+                                        // know if it requires a special handling
+                                        return;
+                                    }
+                                    data[name] = command[2][name];
+                                    if (field.type === 'many2one') {
                                         var r = self._makeDataPoint({
                                             modelName: field.relation,
                                             data: {
@@ -957,9 +963,6 @@ var BasicModel = AbstractModel.extend({
                                     }
                                 });
                                 rec._changes = data;
-                                for (var f in rec._changes) {
-                                    rec.data[f] = null;
-                                }
                                 list._changes.push(rec.id);
                             }
                             if (command[0] === 5) {
diff --git a/addons/web/static/test/views/form_tests.js b/addons/web/static/test/views/form_tests.js
index 06616a407d6..a202ba93995 100644
--- a/addons/web/static/test/views/form_tests.js
+++ b/addons/web/static/test/views/form_tests.js
@@ -2275,6 +2275,72 @@ QUnit.module('Views', {
         form.destroy();
     });
 
+    QUnit.test('onchanges on unknown fields of o2m are ignored', function (assert) {
+        // many2one fields need to be postprocessed (the onchange returns [id,
+        // display_name]), but if we don't know the field, we can't know it's a
+        // many2one, so it isn't ignored, its value is an array instead of a
+        // dataPoint id, which may cause errors later (e.g. when saving).
+        assert.expect(2);
+
+        this.data.partner.records[1].p = [4];
+        this.data.partner.onchanges = {
+            foo: function () {},
+        };
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form>' +
+                    '<group>' +
+                        '<field name="foo"/>' +
+                        '<field name="int_field"/>' +
+                        '<field name="p">' +
+                            '<tree>' +
+                                '<field name="foo"/>' +
+                                '<field name="bar"/>' +
+                            '</tree>' +
+                            '<form>' +
+                                '<field name="foo"/>' +
+                                '<field name="product_id"/>' +
+                            '</form>' +
+                        '</field>' +
+                    '</group>' +
+                '</form>',
+            res_id: 2,
+            mockRPC: function (route, args) {
+                if (args.method === 'onchange') {
+                    return $.when({
+                        value: {
+                            p: [
+                                [5],
+                                [1, 4, {
+                                    foo: 'foo changed',
+                                    product_id: [37, "xphone"],
+                                }]
+                            ],
+                        },
+                    });
+                }
+                if (args.method === 'write') {
+                    assert.deepEqual(args.args[1].p, [[1, 4, {
+                        foo: 'foo changed',
+                    }]], "should only write value of known fields");
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        form.$buttons.find('.o_form_button_edit').click();
+        form.$('.o_form_input:first').val('trigger an onchange').trigger('input');
+
+        assert.strictEqual(form.$('.o_data_row td:first').text(), 'foo changed',
+            "onchange should have been correctly applied on field in o2m list");
+
+        form.$buttons.find('.o_form_button_save').click();
+
+        form.destroy();
+    });
+
     QUnit.test('navigation with tab key in form view', function (assert) {
         assert.expect(2);