From ebd17217c4e7e9ff06d97028de64fb2fc44d6266 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 8 Jun 2017 15:37:04 +0200 Subject: [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). --- .../static/src/js/views/basic/basic_controller.js | 1 + .../web/static/src/js/views/basic/basic_model.js | 61 ++++++++++++++++++++-- addons/web/static/src/js/views/view_dialogs.js | 1 + 3 files changed, 58 insertions(+), 5 deletions(-) (limited to 'addons/web/static/src/js/views') diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js index 16a066db495..af4ca7d7a08 100644 --- a/addons/web/static/src/js/views/basic/basic_controller.js +++ b/addons/web/static/src/js/views/basic/basic_controller.js @@ -419,6 +419,7 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { var saveDef = this.model.save(recordID, { // Save then leave edit mode reload: options.reload, savePoint: options.savePoint, + viewType: options.viewType, }); if (!options.stayInEdit) { saveDef = saveDef.then(function (fieldNames) { 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 61c285d6581..20066b858f1 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -704,6 +704,8 @@ var BasicModel = AbstractModel.extend({ * @param {boolean} [options.savePoint=false] if true, the record will only * be 'locally' saved: its changes written in a _savePoint key that can * be restored later by call discardChanges with option rollback to true + * @param {string} [options.viewType] current viewType. If not set, we will + * assume main viewType from the record * @returns {Deferred} * Resolved with the list of field names (whose value has been modified) */ @@ -718,6 +720,11 @@ var BasicModel = AbstractModel.extend({ if (newValue instanceof Array) { rec._savePoint = newValue.slice(0); } else { + // save the viewType of edition, so that the correct readonly modifiers + // can be evaluated when the record will be saved + for (var fieldName in (rec._changes || {})) { + rec._editionViewType[fieldName] = options.viewType; + } rec._savePoint = _.extend({}, newValue); } }); @@ -728,7 +735,7 @@ var BasicModel = AbstractModel.extend({ // id never changes, and should not be written delete record._changes.id; } - var changes = self._generateChanges(record); + var changes = self._generateChanges(record, options.viewType); if (method === 'create') { var fieldNames = record.getFieldNames(); @@ -1869,12 +1876,26 @@ var BasicModel = AbstractModel.extend({ * * @private * @param {Object} record + * @param {string} [viewType] current viewType. If not set, we will assume + * main viewType from the record. Note that if an editionViewType is + * specified for a field, it will take the priority over the viewType arg. * @returns {Object} a map from changed fields to their new value */ - _generateChanges: function (record) { + _generateChanges: function (record, viewType) { + viewType = viewType || record.viewType; var changes = _.extend({}, record._changes); var commands = this._generateX2ManyCommands(record, true); for (var fieldName in record.fields) { + // remove readonly fields from the list of changes + if (fieldName in changes || fieldName in commands) { + var editionViewType = record._editionViewType[fieldName] || viewType; + if (this._isFieldReadonly(record, fieldName, editionViewType)) { + delete changes[fieldName]; + continue; + } + } + + // process relational fields and handle the null case var type = record.fields[fieldName].type; if (type === 'one2many' || type === 'many2many') { if (commands[fieldName].length) { // replace localId by commands @@ -2199,6 +2220,31 @@ 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). + * + * @private + * @param {Object} record an element from the localData + * @param {string} fieldName + * @param {string} [viewType] current viewType. If not set, we will assume + * main viewType from the record + * @returns {boolean} + */ + _isFieldReadonly: function (record, fieldName, viewType) { + var fieldInfo = record.fieldsInfo[viewType || record.viewType][fieldName]; + var modifiers; + if (fieldInfo) { + var rawModifiers = JSON.parse(fieldInfo.modifiers || "{}"); + modifiers = this._evalModifiers(record, rawModifiers); + } + if (modifiers && 'readonly' in modifiers) { + return modifiers.readonly; + } else { + return record.fields[fieldName].readonly; + } + }, /** * Returns true iff value is considered to be set for the given field's type. * @@ -2330,6 +2376,12 @@ var BasicModel = AbstractModel.extend({ viewType: params.viewType, }; + // _editionViewType is a dict whose keys are field names and which is populated when a field + // is edited with the viewType as value. This is useful for one2manys to determine whether + // or not a field is readonly (using the readonly modifiers of the view in which the field + // has been edited) + dataPoint._editionViewType = {}; + dataPoint.evalModifiers = this._evalModifiers.bind(this, dataPoint); dataPoint.getContext = this._getContext.bind(this, dataPoint); dataPoint.getDomain = this._getDomain.bind(this, dataPoint); @@ -2507,7 +2559,7 @@ var BasicModel = AbstractModel.extend({ .then(function () { // save initial changes, so they can be restored later, // if we need to discard. - self.save(record.id, {savePoint: true}) + self.save(record.id, {savePoint: true}); return record.id; }); @@ -2582,8 +2634,7 @@ var BasicModel = AbstractModel.extend({ * @param {string[]} fields changed fields * @param {string} [viewType] current viewType. If not set, we will assume * main viewType from the record - * @returns {Deferred} The returned deferred can fail, in which case the - * fail value will be the warning message received from the server + * @returns {Deferred} */ _performOnChange: function (record, fields, viewType) { var self = this; diff --git a/addons/web/static/src/js/views/view_dialogs.js b/addons/web/static/src/js/views/view_dialogs.js index a19e440913e..4035ea6e63e 100644 --- a/addons/web/static/src/js/views/view_dialogs.js +++ b/addons/web/static/src/js/views/view_dialogs.js @@ -222,6 +222,7 @@ var FormViewDialog = ViewDialog.extend({ stayInEdit: true, reload: false, savePoint: this.shouldSaveLocally, + viewType: 'form', }); } return $.when(def).then(function () { -- cgit v1.2.3