[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

1 files changed, 31 insertions, 6 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 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