[FIX] web: readonly fields may be edited

The readonly attribute of a field in its description can be
overriden in the views (e.g. with modifiers). So readonly fields
may actually be edited from views in which there is a modifiers
on their node overriding the default value.

Before this rev., readonly fields couldn't be saved, even if they
could be edited from views (with such modifiers).

For instance, it wasn't possible to change the value of the
partner_id field in a draft sale_order.

addons/web/static/src/js/views/basic/basic_model.js

@@ -668,15 +668,6 @@ var BasicModel = AbstractModel.extend({
             // make sure we don't write an undefined id
             delete changes.id;
 
-            // remove readonly fields from the list of changes
-            if (method === 'write') {
-                for (var fieldName in changes) {
-                    if (record.fields[fieldName].readonly) {
-                        delete changes[fieldName];
-                    }
-                }
-            }
-
             // in the case of a write, only perform the RPC if there are changes to save
             if (method === 'create' || Object.keys(changes).length) {
                 var args = method === 'write' ? [[record.data.id], changes] : [changes];

addons/web/static/tests/views/basic_model_tests.js

@@ -988,65 +988,6 @@ QUnit.module('Views', {
         model.destroy();
     });
 
-    QUnit.test('dont write on readonly fields (except if 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",
-                        bar: 21,
-                    }, "should also save bar field in create mode");
-                }
-                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);
 

addons/web/static/tests/views/form_tests.js

@@ -2788,5 +2788,49 @@ QUnit.module('Views', {
         });
     });
 
+    QUnit.test('readonly fields with modifiers may be saved', function (assert) {
+        // the readonly property on the field description only applies on view,
+        // this is not a DB constraint. It should be seen as a default value,
+        // that may be overriden in views, for example with modifiers. So
+        // basically, a field defined as readonly may be edited.
+        assert.expect(3);
+
+        this.data.partner.fields.foo.readonly = true;
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<sheet>' +
+                        '<field name="foo" attrs="{\'readonly\': [(\'bar\',\'=\',False)]}"/>' +
+                        '<field name="bar"/>' +
+                    '</sheet>' +
+                '</form>',
+            res_id: 1,
+            mockRPC: function (route, args) {
+                if (args.method === 'write') {
+                    assert.deepEqual(args.args[1], {foo: 'New foo value'},
+                        "the new value should be saved");
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        // bar being set to true, foo shouldn't be readonly and thus its value
+        // could be saved, even if in its field description it is readonly
+        form.$buttons.find('.o_form_button_edit').click();
+
+        assert.strictEqual(form.$('.o_form_input').length, 1,
+            "foo field should be editable");
+        form.$('.o_form_input').val('New foo value').trigger('input');
+
+        form.$buttons.find('.o_form_button_save').click();
+
+        assert.strictEqual(form.$('.o_form_field[name=foo]').text(), 'New foo value',
+            "new value for foo field should have been saved");
+
+        form.destroy();
+    });
+
 });
 });