[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

addons/web/static/src/js/views/basic/basic_controller.js

@@ -332,7 +332,9 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
                 }
                 self.model.discardChanges(recordID);
                 if (self.model.isNew(recordID)) {
-                    self._abandonRecord(recordID);
+                    if (self.model.canBeAbandoned(recordID)) {
+                        self._abandonRecord(recordID);
+                    }
                     return;
                 }
                 return self._confirmSave(recordID);

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

addons/web/static/tests/fields/relational_fields_tests.js

@@ -1154,6 +1154,60 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('list in form: discard newly added element with empty required field', function (assert) {
+        // This test simulates discarding a record that has been created with
+        // one of its required field that is empty. When we discard the changes
+        // on this empty field, it should not assume that this record should be
+        // abandonned, since it has been added (even though it is a new record).
+        assert.expect(6);
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form>' +
+                    '<sheet>' +
+                        '<field name="p">' +
+                            '<tree editable="bottom">' +
+                                '<field name="display_name"/>' +
+                                '<field name="trululu" required="1"/>' +
+                            '</tree>' +
+                        '</field>' +
+                    '</sheet>' +
+                '</form>',
+            mockRPC: function (route, args) {
+                if (args.method === 'default_get') {
+                    return $.when({p: [[0, 0, {display_name: 'new record', trululu: false}]]});
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        assert.strictEqual($('tr.o_data_row').length, 1,
+            "should have created the new record in the o2m");
+
+        var requiredElement = $('td.o_data_cell.o_required_modifier');
+        assert.strictEqual(requiredElement.length, 1,
+            "should have a required field on this record");
+        assert.strictEqual(requiredElement.text(), "",
+            "should have empty string in the required field on this record");
+
+        requiredElement.click();
+        // discard by clicking on body
+        $('body').click();
+
+        assert.strictEqual($('tr.o_data_row').length, 1,
+            "should still have the record in the o2m");
+
+        // update selector of required field element
+        requiredElement = $('td.o_data_cell.o_required_modifier');
+        assert.strictEqual(requiredElement.length, 1,
+            "should still have the required field on this record");
+        assert.strictEqual(requiredElement.text(), "",
+            "should still have empty string in the required field on this record");
+        form.destroy();
+    });
+
     QUnit.test('list in form: default_get with x2many create', function (assert) {
         assert.expect(5);