From 3052a434947f71955a5e287e373164be46f8860c Mon Sep 17 00:00:00 2001 From: Martin Geubelle Date: Mon, 4 Sep 2017 11:36:26 +0200 Subject: [FIX] web: edit editable list view after resequence In an editable list view with the widget `handle`, resequencing a record and try to edit it afterwards won't work. The selected record isn't the correct one, as if the resequencing had no impact. A small refactoring has been done to fix this because the resequence logic was placed in the ListController (but should logically be placed in the model). Several views are using the resequencing ; the code is thus now placed in the BasicModel. The list renderer state is then updated after the resequence. The resequencing and the line edition have been placed in a common mutex to avoid problems when the user tries to edit a record while the resequence rpc isn't finished yet. --- .../web/static/src/js/views/basic/basic_model.js | 49 ++++++++++++++++++++++ 1 file changed, 49 insertions(+) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 9c4599f1683..202545c2c8f 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -779,6 +779,55 @@ var BasicModel = AbstractModel.extend({ parent.data = _.without(parent.data, elementID); } }, + /** + * Resequences records. + * + * @param {string} modelName the resIDs model + * @param {Array[integer]} resIDs the new sequence of ids + * @param {string} parentID the localID of the parent + * @param {object} [options] + * @param {integer} [options.offset] + * @param {string} [options.field] the field name used as sequence + * @returns {Deferred} resolves to the local id of the parent + */ + resequence: function (modelName, resIDs, parentID, options) { + options = options || {}; + if ((resIDs.length <= 1)) { + return $.when(parentID); // there is nothing to sort + } + var self = this; + var data = this.localData[parentID]; + var params = { + model: modelName, + ids: resIDs, + }; + if (options.offset) { + params.offset = options.offset; + } + if (options.field) { + params.field = options.field; + } + return this._rpc({ + route: '/web/dataset/resequence', + params: params, + }) + .then(function () { + data.data = _.sortBy(data.data, function (d) { + return _.indexOf(resIDs, self.localData[d].res_id); + }); + data.res_ids = []; + _.each(data.data, function (d) { + var dataPoint = self.localData[d]; + if (dataPoint.type === 'record') { + data.res_ids.push(dataPoint.res_id); + } else { + data.res_ids = data.res_ids.concat(dataPoint.res_ids); + } + }); + self._updateParentResIDs(data); + return parentID; + }); + }, /** * Save a local resource, if needed. This is a complicated operation, * - it needs to check all changes, -- cgit v1.2.3 From 14cb9af0d672e544484d6dc7b3432722217f4e10 Mon Sep 17 00:00:00 2001 From: Martin Geubelle Date: Thu, 7 Sep 2017 17:19:38 +0200 Subject: [FIX] web: only send changed fields on one2many after onchange The server response to an `onchange` on a one2many is a list of commands. In case of one2many update (command 1), the server returns all fields (and not only the modified ones). When saving the record, this is the client responsability to specify which fields really changed. Before this commit, all fields were sent at `write` and not only the changed ones. This was causing issues when the user couldn't write on the field ; an error was triggered even though the user didn't change anything. To fix this, we only tag as changed (in the datapoint `_changes` attribute) the fields that have a different value after the onchange. --- .../web/static/src/js/views/basic/basic_model.js | 91 +++++++++++++++------- 1 file changed, 61 insertions(+), 30 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 202545c2c8f..cf5a18488a6 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1234,6 +1234,7 @@ var BasicModel = AbstractModel.extend({ record._rawChanges[name] = val; return; } + var oldValue = record._changes[name] || record.data[name]; var id; if (field.type === 'many2one') { id = false; @@ -1246,29 +1247,42 @@ var BasicModel = AbstractModel.extend({ var data = _.isArray(val) ? {id: val[0], display_name: val[1]} : {id: val}; - rec = self._makeDataPoint({ - context: record.context, - data: data, - modelName: field.relation, - parentID: record.id, - }); - id = rec.id; + if (!oldValue || (self.localData[oldValue].res_id !== data.id)) { + // only register a change if the value has changed + rec = self._makeDataPoint({ + context: record.context, + data: data, + modelName: field.relation, + parentID: record.id, + }); + id = rec.id; + record._changes[name] = id; + } + } else { + record._changes[name] = false; } - record._changes[name] = id; } else if (field.type === 'reference') { id = false; if (val) { var ref = val.split(','); - rec = self._makeDataPoint({ - context: record.context, - data: {id: parseInt(ref[1])}, - modelName: ref[0], - parentID: record.id, - }); - defs.push(self._fetchNameGet(rec)); - id = rec.id; + var modelName = ref[0]; + var resID = parseInt(ref[1]); + if (!oldValue || self.localData[oldValue].res_id !== resID || + self.localData[oldValue].model !== modelName) { + // only register a change if the value has changed + rec = self._makeDataPoint({ + context: record.context, + data: {id: parseInt(ref[1])}, + modelName: modelName, + parentID: record.id, + }); + defs.push(self._fetchNameGet(rec)); + id = rec.id; + record._changes[name] = id; + } + } else { + record._changes[name] = id; } - record._changes[name] = id; } else if (field.type === 'one2many' || field.type === 'many2many') { var listId = record._changes[name] || record.data[name]; var list; @@ -1288,25 +1302,39 @@ var BasicModel = AbstractModel.extend({ viewType: fieldInfo.viewType, }); } + // TODO: before registering the changes, verify that the x2many + // value has changed record._changes[name] = list.id; var shouldLoad = false; list._changes = list._changes || []; _.each(val, function (command) { + var rec; if (command[0] === 0 || command[0] === 1) { // CREATE or UPDATE - var params = { - context: list.context, - fields: list.fields, - fieldsInfo: list.fieldsInfo, - modelName: list.model, - parentID: list.id, - viewType: list.viewType, - }; - if (command[0] === 1) { - params.res_id = command[1]; + if (command[0] === 1 && command[1]) { + // updating an existing record + var recID = _.find(list.data, function (childID) { + var child = self.localData[childID]; + return child.res_id === command[1]; + }); + rec = self.localData[recID]; + } + if (!rec) { + var params = { + context: list.context, + fields: list.fields, + fieldsInfo: list.fieldsInfo, + modelName: list.model, + parentID: list.id, + viewType: list.viewType, + }; + if (command[0] === 1) { + params.res_id = command[1]; + } + rec = self._makeDataPoint(params); + list._cache[rec.res_id] = rec.id; } - rec = self._makeDataPoint(params); - list._cache[rec.res_id] = rec.id; + list._changes.push({operation: 'ADD', id: rec.id}); if (command[0] === 1) { list._changes.push({operation: 'UPDATE', id: rec.id}); @@ -1327,7 +1355,10 @@ var BasicModel = AbstractModel.extend({ defs.push(self._readUngroupedList(list)); } } else { - record._changes[name] = self._parseServerValue(field, val); + var newValue = self._parseServerValue(field, val); + if (newValue !== oldValue) { + record._changes[name] = newValue; + } } }); return $.when.apply($, defs); -- cgit v1.2.3