[FIX] web,mail: only send fields in view for onchanges

The field 'display_name' is automatically added to the list of
fields to fetch when fetching a record. Before this rev., its
value was also sent alongside the fields in the view for onchange
RPCs. It shouldn't: only the fields in the view should be sent.

It produced a weird bug in the res.partner form view: open a
contact (company), add a sub-contact, save -> the display_name of
the contact was then '' (and 'Contacts' was displayed in the
breadcrumbs instead of the display_name of the opened contact).
This is because 'display_name' was sent to the onchange, which
returned '' as value (because of another bug python side). So as
it's value changed, when saving, the new display_name was sent to
the server (even if this field was actually readonly).

Before this rev., we also always fetched 'display_name' when
fetching list of records (read or search_read). This wasn't a bug
per se, but it might be a computed field, and it isn't necessary,
so we stop fetching it in that case if it isn't in the view (as
it was done in saas-15, before the new views).

4 files changed, 103 insertions, 12 deletions

diff --git a/addons/web/static/src/js/services/data_manager.js b/addons/web/static/src/js/services/data_manager.js
index 999a9d8df01..77857ef2d31 100644
--- a/addons/web/static/src/js/services/data_manager.js
+++ b/addons/web/static/src/js/services/data_manager.js
@@ -398,11 +398,6 @@ return core.Class.extend({
      */
     _processFieldsView: function (viewInfo) {
         var viewFields = this._processFields(viewInfo.type, viewInfo.arch, viewInfo.fields);
-        // by default fetch display_name and id
-        if (!viewInfo.fields.display_name) {
-            viewInfo.fields.display_name = {type: 'char'};
-            viewFields.display_name = {};
-        }
         viewInfo.fieldsInfo = {};
         viewInfo.fieldsInfo[viewInfo.type] = viewFields;
         utils.deepFreeze(viewInfo.fields);
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 d54f609f002..aa0aab7aa64 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -1629,8 +1629,8 @@ var BasicModel = AbstractModel.extend({
      */
     _fetchRecord: function (record, options) {
         var self = this;
-        var fieldNames = options && options.fieldNames ||
-                         _.uniq(record.getFieldNames().concat(['display_name']));
+        var fieldNames = options && options.fieldNames || record.getFieldNames();
+        fieldNames = _.uniq(fieldNames.concat(['display_name']));
         return this._rpc({
                 model: record.model,
                 method: 'read',
@@ -2337,6 +2337,16 @@ var BasicModel = AbstractModel.extend({
     _generateOnChangeData: function (record) {
         var commands = this._generateX2ManyCommands(record, {withReadonly: true});
         var data = _.extend(this.get(record.id, {raw: true}).data, commands);
+        // 'display_name' is automatically added to the list of fields to fetch,
+        // when fetching a record, even if it doesn't appear in the view. However,
+        // only the fields in the view must be passed to the onchange RPC, so we
+        // remove it from the data sent by RPC if it isn't in the view.
+        var hasDisplayName = _.some(record.fieldsInfo, function (fieldsInfo) {
+            return 'display_name' in fieldsInfo;
+        });
+        if (!hasDisplayName) {
+            delete data.display_name;
+        }
 
         // one2many records have a parentID
         if (record.parentID) {
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index a2504af38bb..92a24066408 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -2521,6 +2521,7 @@ QUnit.module('relational_fields', {
             arch:'<form string="Partners">' +
                     '<field name="p">' +
                         '<kanban create="0">' +
+                            '<field name="display_name"/>' +
                             '<templates>' +
                                 '<t t-name="kanban-box">' +
                                     '<div class="oe_kanban_global_click">' +
@@ -3499,7 +3500,7 @@ QUnit.module('relational_fields', {
                 rpcCount++;
                 if (args.method === 'write') {
                     assert.deepEqual(args.args[1].p, [[0, false, {
-                        display_name: false, int_field: 123, product_id: 41
+                        int_field: 123, product_id: 41
                     }]]);
                 }
                 return this._super(route, args);
@@ -3736,7 +3737,7 @@ QUnit.module('relational_fields', {
                 if (args.method === 'default_get') {
                     var expected = counter === 0 ?
                         [[4, 2, false]] :
-                        [[4, 2, false], [0, false, {display_name: false, turtle_foo: 'hammer'}]];
+                        [[4, 2, false], [0, false, {turtle_foo: 'hammer'}]];
                     assert.deepEqual(args.kwargs.context.turtles, expected,
                         "should have properly evaluated turtles key in context");
                     counter++;
@@ -5544,6 +5545,7 @@ QUnit.module('relational_fields', {
             arch:'<form string="Partners">' +
                     '<field name="timmy">' +
                         '<kanban create="0">' +
+                            '<field name="display_name"/>' +
                             '<templates>' +
                                 '<t t-name="kanban-box">' +
                                     '<div class="oe_kanban_global_click">' +
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index 15147c22d1d..6bd2b4ca7b5 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -1134,7 +1134,7 @@ QUnit.module('Views', {
             mockRPC: function (route, args) {
                 if (args.method === "onchange") {
                     assert.deepEqual(args.args[3],
-                        {"foo": "1", "p": "", "p.bar": "", "p.display_name": "", "p.product_id": "", "timmy": "", "timmy.name": ""},
+                        {"foo": "1", "p": "", "p.bar": "", "p.product_id": "", "timmy": "", "timmy.name": ""},
                         "should send only the fields used in the views");
                 }
                 return this._super(route, args);
@@ -1282,7 +1282,6 @@ QUnit.module('Views', {
                 if (args.method === 'onchange') {
                     assert.deepEqual(args.args[3], {
                         p: '',
-                        'p.display_name': '',
                         'p.foo': '1'
                     }, "onchangeSpec should be correct (with sub fields)");
                 }
@@ -2827,6 +2826,92 @@ QUnit.module('Views', {
         form.destroy();
     });
 
+    QUnit.test('display_name not sent for onchanges if not in view', function (assert) {
+        assert.expect(7);
+
+        this.data.partner.records[0].timmy = [12];
+        this.data.partner.onchanges = {
+            foo: function () {},
+        };
+        this.data.partner_type.onchanges = {
+            name: function () {},
+        };
+        var readInModal = false;
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<group>' +
+                        '<field name="foo"/>' +
+                        '<field name="timmy">' +
+                            '<tree>' +
+                                '<field name="name"/>' +
+                            '</tree>' +
+                            '<form>' +
+                                '<field name="name"/>' +
+                                '<field name="color"/>' +
+                            '</form>' +
+                        '</field>' +
+                    '</group>' +
+                '</form>',
+            mockRPC: function (route, args) {
+                if (args.method === 'read' && args.model === 'partner') {
+                    assert.deepEqual(args.args[1], ['foo', 'timmy', 'display_name'],
+                        "should read display_name even if not in the view");
+                }
+                if (args.method === 'read' && args.model === 'partner_type') {
+                    if (!readInModal) {
+                        assert.deepEqual(args.args[1], ['name'],
+                            "should not read display_name for records in the list");
+                    } else {
+                        assert.deepEqual(args.args[1], ['name', 'color', 'display_name'],
+                            "should read display_name when opening the subrecord");
+                    }
+                }
+                if (args.method === 'onchange' && args.model === 'partner') {
+                    assert.deepEqual(args.args[1], {
+                        id: 1,
+                        foo: 'coucou',
+                        timmy: [[6, false, [12]]],
+                    }, "should only send the value of fields in the view (+ id)");
+                    assert.deepEqual(args.args[3], {
+                        foo: '1',
+                        timmy: '',
+                        'timmy.name': '1',
+                        'timmy.color': '',
+                    }, "only the fields in the view should be in the onchange spec");
+                }
+                if (args.method === 'onchange' && args.model === 'partner_type') {
+                    assert.deepEqual(args.args[1], {
+                        id: 12,
+                        name: 'new name',
+                        color: 2,
+                    }, "should only send the value of fields in the view (+ id)");
+                    assert.deepEqual(args.args[3], {
+                        name: '1',
+                        color: '',
+                    }, "only the fields in the view should be in the onchange spec");
+                }
+                return this._super.apply(this, arguments);
+            },
+            res_id: 1,
+            viewOptions: {
+                mode: 'edit',
+            },
+        });
+
+        // trigger the onchange
+        form.$('.o_field_widget[name=foo]').val("coucou").trigger('input');
+
+        // open a subrecord and trigger an onchange
+        readInModal = true;
+        form.$('.o_data_row .o_data_cell:first').click();
+        $('.modal .o_field_widget[name=name]').val("new name").trigger('input');
+
+        form.destroy();
+    });
+
     QUnit.test('onchanges on date(time) fields', function (assert) {
         assert.expect(6);
 
@@ -3725,7 +3810,6 @@ QUnit.module('Views', {
                 if (args.method === 'create') {
                     var command = args.args[0].p;
                     assert.deepEqual(command, [[0, false, {
-                        display_name: false,
                         timmy: [[6, false, [12]]],
                     }]], "the default partner_type_id should be equal to 12");
                 }