From b09f0c99be2b13d33986414485556d87da186d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Tue, 13 Feb 2018 16:49:43 +0100 Subject: [FIX] web: discard new record with empty required field Before this commit, there is an issue with one2many lists with records that have been created on 'default_get': if the record has a required field that is empty, when the user clicks on this empty required field and then somewhere else, the record is removed from the list. The cause of this bug is that these records have invalid field changes, here the required field that is empty. As a consequence, it checks whether the record is "new", and if so, removes it visually from the editable list. (Note: a record is "new" when it is not stored in DB). This behaviour is expected when discarding records created from "Add an item". However, that is not intended for records that have been created as default items of the list. These records are still considered as "new" items, because they are not stored in DB, but they should stay in the list on discard. This commit solves the issue by making an additional check before assuming that the record for which we discard changes comes from "Add an item": if the record in the list has been created before user interacts on the list (in other words, the record has been added in the list when the user accesses it), then we should not remove the record from the list, even though the record has some invalid fields from the savepoint (e.g. empty required). opw-806650 --- .../web/static/src/js/views/basic/basic_model.js | 24 ++++++++++++++++++++++ 1 file changed, 24 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 b5b668a43a1..68fcd05fab1 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -477,6 +477,30 @@ var BasicModel = AbstractModel.extend({ } return _t("New"); }, + /** + * Returns true if a record can be abandoned from a list datapoint. + * + * A record cannot be abandonned if it has been registered as "added" + * in the parent's savepoint, otherwise it can be abandonned. + * + * This is useful when discarding changes on this record, as it means that + * we must keep the record even if some fields are invalids (e.g. required + * field is empty). + * + * @param {string} id id for a local resource + * @returns {boolean} + */ + canBeAbandoned: function (id) { + var data = this.localData[id]; + var parent = this.localData[data.parentID]; + var abandonable = true; + if (parent) { + abandonable = !_.some(parent._savePoint, function (entry) { + return entry.operation === 'ADD' && entry.id === id; + }); + } + return abandonable; + }, /** * Returns true if a record is dirty. A record is considered dirty if it has * some unsaved changes, marked by the _isDirty property on the record or -- cgit v1.2.3 From cf03101447cf572417274f06f232056d6b944237 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 15 Feb 2018 10:09:44 +0100 Subject: [FIX] web: BasicModel: fetch nested x2manys after onchange When onchange RPCs return a command 4 (LINK TO), the res_id of the record to link is given, and the BasicModel has to read the fields in the view for that record. When one of those fields is an x2many (for instance, displayed with widget many2many_tags), the subrecords in the relation must be read as well (at least their display_name). Before this rev., this last part wasn't done by the BasicModel, and as a consequence, the many2many_tags widget was always empty, even though there were records in the relation. Same thing applied to reference fields, but this is way more uncommon. This rev. ensures that x2manys and reference fields are correctly fetched in this scenario. --- addons/web/static/src/js/views/basic/basic_model.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 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 68fcd05fab1..ff4ed6d9147 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1470,7 +1470,12 @@ var BasicModel = AbstractModel.extend({ list._changes = [{operation: 'REMOVE_ALL'}]; } }); - defs.push(self._readUngroupedList(list)); + var def = self._readUngroupedList(list).then(function () { + var x2ManysDef = self._fetchX2ManysBatched(list); + var referencesDef = self._fetchReferencesBatched(list); + return $.when(x2ManysDef, referencesDef); + }); + defs.push(def); } else { var newValue = self._parseServerValue(field, val); if (newValue !== oldValue) { @@ -1975,7 +1980,9 @@ var BasicModel = AbstractModel.extend({ _.each(list.data, function (dataPoint) { var record = self.localData[dataPoint]; var value = record.data[fieldName]; - if (value) { + // if the reference field has already been fetched, the value is a + // datapoint ID, and in this case there's nothing to do + if (value && !self.localData[value]) { var model = value.split(',')[0]; var resID = value.split(',')[1]; if (!(model in toFetch)) { -- cgit v1.2.3 From ff019aa52f4857b45ebad301757970d99df37aee Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 15 Feb 2018 10:14:15 +0100 Subject: [FIX] web: BasicModel: onchange on X2M: handle command 6 Before this rev., when an onchange returned a command 6 for an x2many field, it was completely ignored by the BasicModel. --- .../web/static/src/js/views/basic/basic_model.js | 38 ++++++++++++++-------- 1 file changed, 25 insertions(+), 13 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 ff4ed6d9147..6eef41f149d 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1452,22 +1452,15 @@ var BasicModel = AbstractModel.extend({ defs.push(self._applyOnChange(command[2], rec)); } else if (command[0] === 4) { // LINK TO - rec = self.localData[list._cache[command[1]]]; - if (rec) { - // modifications done on a record are discarded if - // the onchange uses a LINK TO - self.discardChanges(rec.id); - } - // the dataPoint id will be set when the record will be - // fetched (for now, this dataPoint may not exist yet) - list._changes.push({ - operation: 'ADD', - id: rec ? rec.id : null, - resID: command[1], - }); + linkRecord(list, command[1]); } else if (command[0] === 5) { // DELETE ALL list._changes = [{operation: 'REMOVE_ALL'}]; + } else if (command[0] === 6) { + list._changes = [{operation: 'REMOVE_ALL'}]; + _.each(command[2], function (resID) { + linkRecord(list, resID); + }); } }); var def = self._readUngroupedList(list).then(function () { @@ -1484,6 +1477,25 @@ var BasicModel = AbstractModel.extend({ } }); return $.when.apply($, defs); + + // inner function that adds a record (based on its res_id) to a list + // dataPoint (used for onchanges that return commands 4 (LINK TO) or + // commands 6 (REPLACE WITH)) + function linkRecord (list, resID) { + rec = self.localData[list._cache[resID]]; + if (rec) { + // modifications done on a record are discarded if the onchange + // uses a LINK TO or a REPLACE WITH + self.discardChanges(rec.id); + } + // the dataPoint id will be set when the record will be fetched (for + // now, this dataPoint may not exist yet) + list._changes.push({ + operation: 'ADD', + id: rec ? rec.id : null, + resID: resID, + }); + } }, /** * When an operation is applied to a x2many field, the field widgets -- cgit v1.2.3