From 5f8dea490acd1812cb7d920ef793cf414014e296 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Mon, 17 Jul 2017 15:50:43 +0200
Subject: [PATCH] [FIX] web: properly display id in some cases

When a one2many is set through an onchange, the web client did not set
the id field properly.  Because of that, it could not be displayed in a
list. This is due to the fact that the id is given in the command like this:

[1, 53, {...}]

Here, the command is 1 (update), then 53 (id), then some values, which
do not have the id. So, in this specific case, we assumed that all the
new values were in the object, even though we properly set the res_id
property.

We fix this issue at the root: in the _makeDataPoint method, we make
sure that no datapoint can be created with a res_id and without the
corresponding id key.

Note that this issue was not found in a standard installation of Odoo
(but it may be present somewhere), but in some custom code.  It is rare
to actually use the id field in a view, but in this case, we had a
many2many in kanban view, and the id was used to format the url of an
image.
---
 .../static/src/js/views/basic/basic_model.js  | 10 +++++--
 .../tests/fields/relational_fields_tests.js   | 28 +++++++++++++++++++
 2 files changed, 36 insertions(+), 2 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 9613d5fb8be..c95f829af71 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -2656,8 +2656,14 @@ var BasicModel = AbstractModel.extend({
         var type = params.type || ('domain' in params && 'list') || 'record';
         var res_id, value;
         var res_ids = params.res_ids || [];
+        var data = params.data || (type === 'record' ? {} : []);
         if (type === 'record') {
-            res_id = params.res_id || (params.data && params.data.id) || _.uniqueId('virtual_');
+            res_id = params.res_id || (params.data && params.data.id);
+            if (res_id) {
+                data.id = res_id;
+            } else {
+                res_id = _.uniqueId('virtual_');
+            }
         } else {
             var isValueArray = params.value instanceof Array;
             res_id = isValueArray ? params.value[0] : undefined;
@@ -2675,7 +2681,7 @@ var BasicModel = AbstractModel.extend({
             aggregateValues: params.aggregateValues || {},
             context: params.context || {},
             count: params.count || res_ids.length,
-            data: params.data || (type === 'record' ? {} : []),
+            data: data,
             domain: params.domain || [],
             fields: fields,
             fieldsInfo: params.fieldsInfo,
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index aacbb744252..026026906c4 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -3652,6 +3652,34 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('id in one2many obtained in onchange is properly set', function (assert) {
+        assert.expect(1);
+
+        this.data.partner.onchanges.turtles = function (obj) {
+            obj.turtles = [
+                [5],
+                [1, 3, {turtle_foo: "kawa"}]
+            ];
+        };
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<field name="turtles">' +
+                        '<tree>' +
+                            '<field name="id"/>' +
+                            '<field name="turtle_foo"/>' +
+                        '</tree>' +
+                    '</field>' +
+                '</form>',
+        });
+
+        assert.strictEqual(form.$('tr.o_data_row').text(), '3kawa',
+            "should have properly displayed id and foo field");
+        form.destroy();
+    });
+
     QUnit.test('sub form view with a required field', function (assert) {
         assert.expect(2);
         this.data.partner.fields.foo.required = true;