Skip to content

Commit

Permalink
[FIX] web: save changes when readonly if force_save is set
Browse files Browse the repository at this point in the history
In previous versions, readonly used to prevent sending the values
of the fields to the server. However, it was not working properly
for fields which have their own sub-arch in the view, as the value
used to be sent regardless of the readonly value.

In the new views, the readonly modifier is never overlooked, which
created a bug in the stock_picking_return wizard which was not
able to return products received from a po anymore. Basically the
value of the product_id field was not sent so the create crashed.

This commit introduces a new "force_save" modifier which is used
to express that we want to save the value of the field even though
it is readonly from a user interface point of view.
  • Loading branch information
dmo-odoo committed Jul 28, 2017
1 parent 4e12edc commit 3b3f6f0
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 7 deletions.
2 changes: 1 addition & 1 deletion addons/stock/wizard/stock_picking_return_views.xml
Expand Up @@ -18,7 +18,7 @@
<group>
<field name="product_return_moves" nolabel="1">
<tree editable="top" create="0">
<field name="product_id" readonly="1"/>
<field name="product_id" readonly="1" force_save="1"/>
<field name="quantity"/>
</tree>
</field>
Expand Down
13 changes: 7 additions & 6 deletions addons/web/static/src/js/views/basic/basic_model.js
Expand Up @@ -2203,7 +2203,7 @@ var BasicModel = AbstractModel.extend({
// remove readonly fields from the list of changes
if (!withReadonly && fieldName in changes || fieldName in commands) {
var editionViewType = record._editionViewType[fieldName] || viewType;
if (this._isFieldReadonly(record, fieldName, editionViewType)) {
if (this._isFieldProtected(record, fieldName, editionViewType)) {
delete changes[fieldName];
continue;
}
Expand Down Expand Up @@ -2562,9 +2562,10 @@ var BasicModel = AbstractModel.extend({
return context;
},
/**
* Returns true if the field is readonly (checking first in the modifiers,
* and if there is no readonly modifier, checking the readonly attribute of
* the field).
* Returns true if the field is protected against changes, looking for a
* readonly modifier unless there is a force_save modifier (checking first
* in the modifiers, and if there is no readonly modifier, checking the
* readonly attribute of the field).
*
* @private
* @param {Object} record an element from the localData
Expand All @@ -2573,12 +2574,12 @@ var BasicModel = AbstractModel.extend({
* main viewType from the record
* @returns {boolean}
*/
_isFieldReadonly: function (record, fieldName, viewType) {
_isFieldProtected: function (record, fieldName, viewType) {
var fieldInfo = record.fieldsInfo[viewType || record.viewType][fieldName];
if (fieldInfo) {
var rawModifiers = JSON.parse(fieldInfo.modifiers || "{}");
var modifiers = this._evalModifiers(record, rawModifiers);
return modifiers.readonly;
return modifiers.readonly && !fieldInfo.force_save;
} else {
return false;
}
Expand Down
65 changes: 65 additions & 0 deletions addons/web/static/tests/views/basic_model_tests.js
Expand Up @@ -1475,6 +1475,71 @@ QUnit.module('Views', {
model.destroy();
});

QUnit.test('dont write on readonly fields unless save attribute is set', function (assert) {
assert.expect(6);

this.params.fieldNames = ['foo', 'bar'];
this.data.partner.onchanges.foo = function (obj) {
obj.bar = obj.foo.length;
};
this.params.fieldsInfo = {
default: {
foo: {},
bar: {
modifiers:"{\"readonly\": true}",
force_save: true,
},
}
};

var model = createModel({
Model: BasicModel,
data: this.data,
mockRPC: function (route, args) {
if (args.method === 'write') {
assert.deepEqual(args.args[1], {bar: 14, foo: "verylongstring"},
"should only save foo field");
}
if (args.method === 'create') {
assert.deepEqual(args.args[0], {bar: 21, 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, 0,
"should be initialized with correct value (0 as integer)");

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);

Expand Down
1 change: 1 addition & 0 deletions odoo/addons/base/rng/view.rng
Expand Up @@ -556,6 +556,7 @@
<rng:optional><rng:attribute name="default_get"/></rng:optional>
<rng:optional><rng:attribute name="required"/></rng:optional>
<rng:optional><rng:attribute name="readonly"/></rng:optional>
<rng:optional><rng:attribute name="force_save"/></rng:optional>
<rng:optional><rng:attribute name="view_mode"/></rng:optional>
<rng:optional><rng:attribute name="widget"/></rng:optional>
<rng:optional><rng:attribute name="context"/></rng:optional>
Expand Down

0 comments on commit 3b3f6f0

Please sign in to comment.