From c53b0ac18880bbb5687d2da203aa160f31946092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Wed, 13 Jun 2018 15:21:13 +0200 Subject: [FIX] web: do no drop invalid record in list on discard Revision on b09f0c99 The intent of the previous commit was to no drop records in a list on discard in the following cases: - when they have been created from a `default_get` - when they have been created from an `onchange` that result from a `default_get` Before the commit above, the kind of records that were abandoned in those cases are stated as invalid, which happens when a record has a required field that is empty. The case of an `onchange` that is not involved in a `default_get` was not considered in the fixes above. In fact, this is the intent behaviour in most cases. However, if the items in the list are created from such an `onchange`, we should not drop them. This commit fixes the bug where records in a list that are created from an `onchange` are wrongly abandoned. Here is the new logic to abandon a record on discard in a list: - the record that is marked as "do not abandon" should not be abandoned; - a record that is registered as a "not new addition" in the list should not be dropped; - a record that is registered as a "new addition" in the list, and has been updated afterwards, should not be dropped; - a record that is not new should not be dropped; opw-806650 --- .../web/static/src/js/views/basic/basic_model.js | 37 ++++++++++++++++++---- 1 file changed, 31 insertions(+), 6 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 da0d58664c2..99bf22cc6bd 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -573,13 +573,16 @@ var BasicModel = AbstractModel.extend({ /** * Returns true if a record can be abandoned. * - * A record can be abandonned if it is a new record, except if - * this datapoint has been specifically tagged as "do not abandon". + * Case for not abandoning the record: * - * Example: + * 1. flagged as 'no abandon' (i.e. during a `default_get`, including any + * `onchange` from a `default_get`) + * 2. registered in a list on addition + * 2.1. registered as non-new addition + * 2.2. registered as new additon on update + * 3. record is not new * - * - Discard record from "Add an item" => "New" record => abandon - * - Discard record from `default_get`/`onchange` => do not abandon + * Otherwise, the record can be abandoned. * * 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 @@ -589,7 +592,29 @@ var BasicModel = AbstractModel.extend({ * @returns {boolean} */ canBeAbandoned: function (id) { - return !this.localData[id]._noAbandon && this.isNew(id); + // 1. no drop if flagged + if (this.localData[id]._noAbandon) { + return false; + } + // 2. no drop in a list on "ADD in some cases + var record = this.localData[id]; + var parent = this.localData[record.parentID]; + if (parent) { + var entry = _.findWhere(parent._savePoint, {operation: 'ADD', id: id}); + if (entry) { + // 2.1. no drop on non-new addition in list + if (!entry.isNew) { + return false; + } + // 2.2. no drop on new addition on "UPDATE" + var lastEntry = _.last(parent._savePoint); + if (lastEntry.operation === 'UPDATE' && lastEntry.id === id) { + return false; + } + } + } + // 3. drop new records + return this.isNew(id); }, /** * Returns true if a record is dirty. A record is considered dirty if it has -- cgit v1.2.3 From 8ace39f8bc52fb63b07c1453a102cb8d91adecf2 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 20 Jun 2018 15:11:33 +0200 Subject: [FIX] web: onchange in non inline one2many list Let's assume a one2many list with two fields A and B, and an onchange on A that sets B. Before this rev., on new records, the onchange was only triggered if the one2many list was defined inline. Rev. 13c885195 was an attempt to fix the onchange issue in x2manys with non inline views, but it only worked on existing records. This rev. fixes the problem for both cases (existing and new records), and thus reverts the fix of 13c885195. The idea of this fix is to make both cases behave similarly, w.r.t. fields and viewFields stuff in the fieldsView. In the inline case (like in the main view case), fields and viewFields are the same. In the non inline case, viewFields contained more information than fields. As fields is used everywhere in BasicModel (and viewFields is basically ignored), the fix is to set fields to viewFields. --- addons/web/static/src/js/views/basic/basic_model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (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 4630ba82191..8c6b6308609 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -2567,7 +2567,7 @@ var BasicModel = AbstractModel.extend({ count: ids.length, context: record.context, fieldsInfo: fieldsInfo, - fields: view ? view.viewFields : fieldInfo.relatedFields, + fields: view ? view.fields : fieldInfo.relatedFields, limit: fieldInfo.limit, modelName: field.relation, res_ids: ids, -- cgit v1.2.3 From 06d89a8440866fe6cb7b97ea79a65e9faff61843 Mon Sep 17 00:00:00 2001 From: Nicolas Lempereur Date: Tue, 19 Jun 2018 14:00:27 +0200 Subject: [FIX] web: onchange on not loaded inline list view Let's assume a one2many A inside a form view, such that the one2many views (list, not editable, and form) are defined inline, and there is an onchange on A. There is another inline one2many B editable list displayed in the inner form view (and B isn't in the list). There are two records on the one2many A, both of them having one record in their one2many B. Edit the first record of A, and edit it's subrecord in B. Click on save. This triggers an onchange on A and the onchange returns a command 5 (DELETE_ALL), and two commands 1 (UPDATE). Now edit the second record of A, and simply close the dialog (e.g. 'Discard'). Save the main record: the record in one2many B of the second record of A has been duplicated. This occured because the code in BasicModel messed up information coming from the onchange (UPDATE command) and from the read (performed when opening the second record), thus producing a CREATE command instead of an UPDATE one. This rev. ensures to properly mix the read data with the onchange result, by transfering the changes over the list datapoint created after the read. opw-1836785 closes #25330 --- addons/web/static/src/js/views/basic/basic_model.js | 5 +++++ 1 file changed, 5 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 99bf22cc6bd..82bb9e586a8 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -2578,6 +2578,11 @@ var BasicModel = AbstractModel.extend({ relationField: field.relation_field, viewType: view ? view.type : fieldInfo.viewType, }); + // set existing changes to the list + if (record._changes && record._changes[fieldName]) { + list._changes = self.localData[record._changes[fieldName]]._changes; + record._changes[fieldName] = list.id; + } record.data[fieldName] = list.id; if (!fieldInfo.__no_fetch) { var def = self._readUngroupedList(list).then(function () { -- cgit v1.2.3