From f6525c6b6ce04534e7fb2c282191243b8d3c4707 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 14 Jun 2017 13:42:38 +0200 Subject: [PATCH] [FIX] web: BasicModel: onchanges: send all values Since commit ebd17217c4, values of readonly fields inside x2many fields weren't sent for onchanges, whereas they should. This was for example an issue on the stock.picking form view (which is in state 'draft' by default). If the user added a stock.move, the state field on the move wasn't sent for the onchange, and thus the state field on the picking was set to false. --- .../static/src/js/views/basic/basic_model.js | 42 ++++++++++++------- addons/web/static/tests/views/form_tests.js | 15 +++++-- 2 files changed, 39 insertions(+), 18 deletions(-) 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 ae48a98dd13..d897a74c696 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -736,7 +736,7 @@ var BasicModel = AbstractModel.extend({ // id never changes, and should not be written delete record._changes.id; } - var changes = self._generateChanges(record, options.viewType); + var changes = self._generateChanges(record, {viewType: options.viewType}); if (method === 'create') { var fieldNames = record.getFieldNames(); @@ -1877,18 +1877,28 @@ 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 + * @param {Object} [options] + * @param {boolean} [options.changesOnly=true] if true, only generates + * commands for fields that have changed (concerns x2many fields only) + * @param {boolean} [options.withReadonly=false] if false, doesn't generate + * changes for readonly fields + * @param {string} [options.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, viewType) { - viewType = viewType || record.viewType; + _generateChanges: function (record, options) { + options = options || {}; + var viewType = options.viewType || record.viewType; var changes = _.extend({}, record._changes); - var commands = this._generateX2ManyCommands(record, true); + var withReadonly = options.withReadonly || false; + var commands = this._generateX2ManyCommands(record, { + changesOnly: 'changesOnly' in options ? options.changesOnly : true, + withReadonly: withReadonly, + }); for (var fieldName in record.fields) { // remove readonly fields from the list of changes - if (fieldName in changes || fieldName in commands) { + if (!withReadonly && fieldName in changes || fieldName in commands) { var editionViewType = record._editionViewType[fieldName] || viewType; if (this._isFieldReadonly(record, fieldName, editionViewType)) { delete changes[fieldName]; @@ -1923,7 +1933,7 @@ var BasicModel = AbstractModel.extend({ * @returns {Object} the data */ _generateOnChangeData: function (record) { - var commands = this._generateX2ManyCommands(record, false); + var commands = this._generateX2ManyCommands(record, {withReadonly: true}); var data = _.extend(this.get(record.id, {raw: true}).data, commands); // one2many records have a parentID @@ -1946,12 +1956,16 @@ var BasicModel = AbstractModel.extend({ * or write them... * * @param {Object} record + * @param {Object} [options] * @param {boolean} [changesOnly=false] if true, only generates commands for * fields that have changed + * @param {boolean} [options.withReadonly=false] if false, doesn't generate + * changes for readonly fields in commands * @returns {Object} a map from some field names to commands */ - _generateX2ManyCommands: function (record, changesOnly) { + _generateX2ManyCommands: function (record, options) { var self = this; + options = options || {}; var commands = {}; var data = _.extend({}, record.data, record._changes); var type; @@ -1965,7 +1979,7 @@ var BasicModel = AbstractModel.extend({ continue; } var list = this.localData[data[fieldName]]; - if (changesOnly && !list._changes) { + if (options.changesOnly && !list._changes) { // if only changes are requested, skip if there is no change continue; } @@ -1983,7 +1997,7 @@ var BasicModel = AbstractModel.extend({ // generate update commands for records that have been // updated (it may happen with editable lists) _.each(relData, function (relRecord) { - var changes = self._generateChanges(relRecord); + var changes = self._generateChanges(relRecord, options); if (!_.isEmpty(changes)) { var command = x2ManyCommands.update(relRecord.res_id, changes); commands[fieldName].push(command); @@ -2002,7 +2016,7 @@ var BasicModel = AbstractModel.extend({ if (_.contains(keptIds, relIds[i])) { // this is an id that already existed relRecord = _.findWhere(relData, {res_id: relIds[i]}); - changes = this._generateChanges(relRecord); + changes = this._generateChanges(relRecord, options); if (!_.isEmpty(changes)) { command = x2ManyCommands.update(relRecord.res_id, changes); didChange = true; @@ -2013,11 +2027,11 @@ var BasicModel = AbstractModel.extend({ } else if (_.contains(addedIds, relIds[i])) { // this is a new id relRecord = _.findWhere(relData, {res_id: relIds[i]}); - changes = this._generateChanges(relRecord); + changes = this._generateChanges(relRecord, options); commands[fieldName].push(x2ManyCommands.create(changes)); } } - if (changesOnly && !didChange && addedIds.length === 0 && removedIds.length === 0) { + if (options.changesOnly && !didChange && addedIds.length === 0 && removedIds.length === 0) { // in this situation, we have no changed ids, no added // ids and no removed ids, so we can safely ignore the // last changes diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index e578f3862c8..411b4ffa3d9 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/addons/web/static/tests/views/form_tests.js @@ -4981,12 +4981,13 @@ QUnit.module('Views', { }); QUnit.test('readonly fields are not sent when saving', function (assert) { - assert.expect(5); + assert.expect(6); // 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 () {}, + p: function () {}, }; var checkOnchange = false; @@ -5007,8 +5008,14 @@ QUnit.module('Views', { '', 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.args[2] === 'display_name') { // onchange on field display_name + assert.strictEqual(args.args[1].foo, 'foo value', + "readonly fields value should be sent for onchanges"); + } else { // onchange on field p + assert.deepEqual(args.args[1].p, [ + [0, false, {display_name: 'readonly', foo: 'foo value'}] + ], "readonly fields value should be sent for onchanges"); + } } if (args.method === 'create') { assert.deepEqual(args.args[0], { @@ -5025,10 +5032,10 @@ QUnit.module('Views', { 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 + checkOnchange = false; form.$('.o_data_row').click(); // re-open previous record assert.strictEqual($('.modal .o_field_widget[name=foo]').text(), 'foo value',