[FIX] web: BasicModel: don't write on readonly fields

Ultimately, it seems that sending the value of readonly fields
when saving is not such a good idea. Suppose that there is a
computed field displayed as readonly in a form view. This field is
computed from other records, e.g. from a one2many field which is
editable in the form view. Finally, the inverse function of the
computed field creates the one2many records. If the user adds some
records to the one2many, then saves, the readonly computed field's
value is sent to the server as well, and thus the inverse function
is executed, which isn't what we want.

This commit essentially reverts 0494d61274, except that we now take
into account the readonly modifier to determine whether or not a
value is sent to the server, and not only the readonly attribute of
the field (which can be seen as a default). Before rev. 0494d61274,
we made a difference between write and create RPCs, for an unclear
reason. We don't do that anymore: readonly fields are never sent to
the server (same behavior as before the new views).

2 files changed, 156 insertions, 0 deletions

diff --git a/addons/web/static/tests/views/basic_model_tests.js b/addons/web/static/tests/views/basic_model_tests.js
index fefc7231e5b..ca2f6f37f3e 100644
--- a/addons/web/static/tests/views/basic_model_tests.js
+++ b/addons/web/static/tests/views/basic_model_tests.js
@@ -572,6 +572,20 @@ QUnit.module('Views', {
         this.params.fieldNames = ['product_id', 'category', 'product_ids'];
         this.params.res_id = undefined;
         this.params.type = 'record';
+        this.params.fieldsInfo = {
+            form: {
+                category: {},
+                product_id: {},
+                product_ids: {
+                    fieldsInfo: {
+                        default: { name: {} },
+                    },
+                    relatedFields: this.data.product.fields,
+                    viewType: 'default',
+                },
+            },
+        };
+        this.params.viewType = 'form';
 
         var model = createModel({
             Model: BasicModel,
@@ -1015,6 +1029,19 @@ QUnit.module('Views', {
         this.params.fieldNames = ['total', 'product_ids'];
         this.params.res_id = undefined;
         this.params.type = 'record';
+        this.params.fieldsInfo = {
+            form: {
+                product_ids: {
+                    fieldsInfo: {
+                        default: { name: {} },
+                    },
+                    relatedFields: this.data.product.fields,
+                    viewType: 'default',
+                },
+                total: {},
+            },
+        };
+        this.params.viewType = 'form';
 
         var o2mRecordParams = {
             fields: this.data.product.fields,
@@ -1277,6 +1304,63 @@ QUnit.module('Views', {
         model.destroy();
     });
 
+    QUnit.test('dont write on readonly fields (write and create)', function (assert) {
+        assert.expect(6);
+
+        this.params.fieldNames = ['foo', 'bar'];
+        this.data.partner.onchanges.foo = function (obj) {
+            obj.bar = obj.foo.length;
+        };
+        this.data.partner.fields.bar.readonly = true;
+
+        var model = createModel({
+            Model: BasicModel,
+            data: this.data,
+            mockRPC: function (route, args) {
+                if (args.method === 'write') {
+                    assert.deepEqual(args.args[1], {foo: "verylongstring"},
+                        "should only save foo field");
+                }
+                if (args.method === 'create') {
+                    assert.deepEqual(args.args[0], {foo: "anotherverylongstring"},
+                        "should only save foo field");
+                }
+                return this._super(route, args);
+            },
+        });
+
+        model.load(this.params).then(function (resultID) {
+            var record = model.get(resultID);
+            assert.strictEqual(record.data.bar, 2,
+                "should be initialized with correct value");
+
+            model.notifyChanges(resultID, {foo: "verylongstring"});
+
+            record = model.get(resultID);
+            assert.strictEqual(record.data.bar, 14,
+                "should be changed with correct value");
+
+            model.save(resultID);
+        });
+
+        // start again, but with a new record
+        delete this.params.res_id;
+        model.load(this.params).then(function (resultID) {
+            var record = model.get(resultID);
+            assert.strictEqual(record.data.bar, false,
+                "should be initialized with correct value");
+
+            model.notifyChanges(resultID, {foo: "anotherverylongstring"});
+
+            record = model.get(resultID);
+            assert.strictEqual(record.data.bar, 21,
+                "should be changed with correct value");
+
+            model.save(resultID);
+        });
+        model.destroy();
+    });
+
     QUnit.test('default_get with one2many values', function (assert) {
         assert.expect(1);
 
@@ -1297,6 +1381,18 @@ QUnit.module('Views', {
             fields: this.data.partner.fields,
             modelName: 'partner',
             type: 'record',
+            fieldsInfo: {
+                form: {
+                    product_ids: {
+                        fieldsInfo: {
+                            default: { name: {} },
+                        },
+                        relatedFields: this.data.product.fields,
+                        viewType: 'default',
+                    },
+                },
+            },
+            viewType: 'form',
         };
         model.load(params).then(function (resultID) {
             assert.strictEqual(typeof resultID, 'string', "result should be a valid id");
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index 23986635b88..d9fbfe2f3a4 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -4979,5 +4979,65 @@ QUnit.module('Views', {
 
         form.destroy();
     });
+
+    QUnit.test('readonly fields are not sent when saving', function (assert) {
+        assert.expect(5);
+
+        // define an onchange on display_name to check that the value of readonly
+        // fields is correctly sent for onchanges
+        this.data.partner.onchanges = {
+            display_name: function () {},
+        };
+        var checkOnchange = false;
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<field name="p">' +
+                        '<tree>' +
+                            '<field name="display_name"/>' +
+                        '</tree>' +
+                        '<form string="Partners">' +
+                            '<field name="display_name"/>' +
+                            '<field name="foo" attrs="{\'readonly\': [[\'display_name\', \'=\', \'readonly\']]}"/>' +
+                        '</form>' +
+                    '</field>' +
+                '</form>',
+            mockRPC: function (route, args) {
+                if (checkOnchange && args.method === 'onchange') {
+                    assert.strictEqual(args.args[1].foo, 'foo value',
+                        "readonly fields value should be sent for onchanges");
+                }
+                if (args.method === 'create') {
+                    assert.deepEqual(args.args[0], {
+                        p: [[0, false, {display_name: 'readonly'}]]
+                    }, "should not have sent the value of the readonly field");
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        form.$('.o_field_x2many_list_row_add a').click();
+        assert.strictEqual($('.modal input.o_field_widget[name=foo]').length, 1,
+            'foo should be editable');
+        checkOnchange = true;
+        $('.modal .o_field_widget[name=foo]').val('foo value').trigger('input');
+        $('.modal .o_field_widget[name=display_name]').val('readonly').trigger('input');
+        checkOnchange = false;
+        assert.strictEqual($('.modal span.o_field_widget[name=foo]').length, 1,
+            'foo should be readonly');
+        $('.modal .modal-footer .btn-primary').click(); // close the modal
+
+        form.$('.o_data_row').click(); // re-open previous record
+        assert.strictEqual($('.modal .o_field_widget[name=foo]').text(), 'foo value',
+            "the edited value should have been kept");
+        $('.modal .modal-footer .btn-primary').click(); // close the modal
+
+        form.$buttons.find('.o_form_button_save').click(); // save the record
+
+        form.destroy();
+    });
 });
 });