[FIX] web: add support for discarding lines in o2m

This is not a complete work, but it introduces the basic infrastructure
to properly edit various more sophisticated editing behaviours.

The main focus of this commit is to handle discarding invalid lines in
one2many, potentially asking the user for confirmation.  So, the
relevant methods have to be modified to return some deferred, because
the operation is now asynchronous.

10 files changed, 387 insertions, 48 deletions

diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js
index 47802d5d8c7..6eac12a2dba 100644
--- a/addons/web/static/src/js/fields/abstract_field.js
+++ b/addons/web/static/src/js/fields/abstract_field.js
@@ -153,7 +153,7 @@ var AbstractField = Widget.extend({
 
         // the 'required' flag is basically only needed to determine if the widget
         // is in a valid state (not valid if empty and required)
-        this.required = options.required || false;
+        this.required = options.required || this.field.required;
 
         // the 'idForLabel' is the (html) id that should be set to the relevent
         // dom entity.  If done correctly, clicking on the corresponding label
@@ -202,7 +202,13 @@ var AbstractField = Widget.extend({
      * a field widget is valid if its value is valid and if there is a value when
      * it is required.  This is checked before saving a record, by the form view.
      *
-     * @returns {boolean}
+     * Note: the return value may be a boolean.  It is necessary for some fields
+     * (x2many) to sometimes ask confirmation to the user if the current dirty
+     * invalid line can be discarded.  This is an asynchronous operation, so we
+     * need the expressivity of something like a deferred.
+     *
+     * @returns {boolean|Deferred} true/false if the widget is valid, or a
+     *   deferred which resolves to true/false if the widget is valid or not
      */
     isValid: function () {
         var is_required = this.required;
diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index c29f958cc06..4ae30264ff0 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -560,6 +560,28 @@ var FieldX2Many = AbstractField.extend({
     isSet: function () {
         return true;
     },
+    /**
+     * This is a tricky override.  For relational fields, we don't just need to
+     * know if the field is valid.  We also need to discard some invalid lines,
+     * if the user is ok with it (or automatically if the invalid line is not
+     * dirty). This is the reason why we need to change the signature of the
+     * AbstractField isValid method to optionally return a deferred.
+     *
+     * @override
+     * @returns {boolean|Deferred} if there is a renderer (sub list or kanban
+     *   view), we may need to ask the user => it will return a deferred.
+     *   Otherwise, it returns a boolean
+     */
+    isValid: function () {
+        if (!this.renderer) {
+            return this._super.apply(this, arguments);
+        }
+        var def = $.Deferred();
+        this.renderer.canBeSaved()
+            .then(def.resolve.bind(def, true))
+            .fail(def.resolve.bind(def, false));
+        return def;
+    },
 
     //--------------------------------------------------------------------------
     // Private
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 0b5de486bb9..1fe0e52130b 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -545,6 +545,22 @@ var BasicModel = AbstractModel.extend({
         });
     },
     /**
+     * In some case, we may need to remove an element from a list, without going
+     * through the notifyChanges machinery.  The motivation for this is when the
+     * user click on 'Add an item' in a field one2many with a required field,
+     * then clicks somewhere else.  The new line need to be discarded, but we
+     * don't want to trigger a real notifyChanges (no need for that, and also,
+     * we don't want to rerender the UI).
+     *
+     * @param {string} elementID some valid element id. It is necessary that the
+     *   corresponding element has a parent.
+     */
+    removeLine: function (elementID) {
+        var record = this.localData[elementID];
+        var parent = this.localData[record.parentID];
+        parent._changes = _.without(parent._changes, elementID);
+    },
+    /**
      * Save a local resource, if needed.  This is a complicated operation,
      * - it needs to check all changes,
      * - generate commands for x2many fields,
@@ -1652,6 +1668,11 @@ var BasicModel = AbstractModel.extend({
                     removedIds = _.difference(list.res_ids, relIds);
                     addedIds = _.difference(relIds, list.res_ids);
                     keptIds = _.intersection(list.res_ids, relIds);
+
+                    // the didChange variable keep track of the fact that at
+                    // least one id was updated
+                    var didChange = false;
+
                     for (i = 0; i < keptIds.length; i++) {
                         relRecord = _.findWhere(relData, {res_id: keptIds[i]});
                         var command;
@@ -1659,11 +1680,18 @@ var BasicModel = AbstractModel.extend({
                             var r = this.get(relRecord.id, {raw: true});
                             var changes = _.pick(r.data, Object.keys(relRecord._changes));
                             command = x2ManyCommands.update(relRecord.res_id, changes);
+                            didChange = true;
                         } else {
                             command = x2ManyCommands.link_to(keptIds[i]);
                         }
                         commands[fieldName].push(command);
                     }
+                    if (!didChange && addedIds.length === 0 && removedIds.length === 0) {
+                        // in this situation, we have no changed ids, no added
+                        // ids and no removed ids, so we can safely ignore the
+                        // last changes
+                        commands[fieldName] = null;
+                    }
                     for (i = 0; i < addedIds.length; i++) {
                         relRecord = _.findWhere(relData, {res_id: addedIds[i]});
                         relRecord = this.get(relRecord.id, {raw: true});
diff --git a/addons/web/static/src/js/views/field_manager_mixin.js b/addons/web/static/src/js/views/field_manager_mixin.js
index 10d0c06ad76..9fcd34ffee2 100644
--- a/addons/web/static/src/js/views/field_manager_mixin.js
+++ b/addons/web/static/src/js/views/field_manager_mixin.js
@@ -12,6 +12,7 @@ var BasicModel = require('web.BasicModel');
 
 var FieldManagerMixin = {
     custom_events: {
+        discard_line: '_onDiscardLine',
         field_changed: '_onFieldChanged',
         load: '_onLoad',
     },
@@ -59,6 +60,19 @@ var FieldManagerMixin = {
     //--------------------------------------------------------------------------
 
     /**
+     * In some case, we may need to remove an element from a list, without going
+     * through the notifyChanges machinery.  The motivation for this is when the
+     * user click on 'Add an item' in a field one2many with a required field,
+     * then clicks somewhere else.  The new line need to be discarded, but we
+     * don't want to trigger a real notifyChanges (no need for that, and also,
+     * we don't want to rerender the UI).
+     *
+     * @param {OdooEvent} event
+     */
+    _onDiscardLine: function (event) {
+        this.model.removeLine(event.data.id);
+    },
+    /**
      * This is the main job of the FMM: deciding what to do when a controlled
      * field changes.  Most of the time, it notifies the model that a change
      * just occurred, then confirm the change.
diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js
index 22f4688bc70..d41ff592687 100644
--- a/addons/web/static/src/js/views/form/form_controller.js
+++ b/addons/web/static/src/js/views/form/form_controller.js
@@ -75,14 +75,11 @@ var FormController = BasicController.extend({
         return def;
     },
     /**
-     * @returns {boolean}
+     * @returns {Deferred}
      */
-    checkInvalidFields: function () {
-        var invalidFields = this.renderer.checkInvalidFields();
-        if (invalidFields.length) {
-            this._notifyInvalidFields(invalidFields);
-        }
-        return !!invalidFields.length;
+    canBeSaved: function () {
+        return this.renderer.canBeSaved()
+            .fail(this._notifyInvalidFields.bind(this));
     },
     /**
      * This method switches the form view in edit mode, with a new record.
@@ -225,17 +222,19 @@ var FormController = BasicController.extend({
             }
             return $.Deferred().resolve();
         } else {
-            if (this.checkInvalidFields()) {
-                return $.Deferred().reject();
-            } else {
-                return this.model.save(this.handle, {reload: shouldReload, savePoint: options.savePoint})
-                    .then(function () {
-                        if (!stayInEdit) {
-                            self._toReadOnlyMode();
-                        }
-                        self.isDirty = false;
+            return this.canBeSaved()
+                .then(function () {
+                    return self.model.save(self.handle, {
+                        reload: shouldReload,
+                        savePoint: options.savePoint
                     });
-            }
+                })
+                .then(function () {
+                    if (!stayInEdit) {
+                        self._toReadOnlyMode();
+                    }
+                    self.isDirty = false;
+                });
         }
     },
     /**
diff --git a/addons/web/static/src/js/views/form/form_renderer.js b/addons/web/static/src/js/views/form/form_renderer.js
index 2cdc2f91a1d..f417b53885d 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -42,22 +42,39 @@ return BasicRenderer.extend({
      * toggling the o_form_invalid css class.  It has to be done both on the
      * widget, and on the label, if there is a label.
      *
-     * @returns {string[]} the list of invalid field names
+     * @returns {Deferred}
+     *   if it fails, it gives the list of invalid field names
      */
-    checkInvalidFields: function () {
+    canBeSaved: function () {
         var self = this;
-        var invalid_fields = [];
-        _.each(this.widgets, function (widget) {
-            var is_valid = widget.isValid();
-            if (!is_valid) {
-                invalid_fields.push(widget.name);
+        var invalidFields = [];
+        var defs = [];
+
+        function markWidget(widget, isValid) {
+            if (!isValid) {
+                invalidFields.push(widget.name);
             }
-            widget.$el.toggleClass('o_form_invalid', !is_valid);
+            widget.$el.toggleClass('o_form_invalid', !isValid);
             var idForLabel = self.idsForLabels[widget.name];
             var $label = idForLabel ? self.$('label[for=' + idForLabel + ']') : $();
-            $label.toggleClass('o_form_invalid', !is_valid);
+            $label.toggleClass('o_form_invalid', !isValid);
+        }
+
+        _.each(this.widgets, function (widget) {
+            var isValid = widget.isValid();
+            if (isValid instanceof $.Deferred) {
+                defs.push(isValid.then(function (isValid) {
+                    markWidget(widget, isValid);
+                }));
+            } else {
+                markWidget(widget, isValid);
+            }
+        });
+        return $.when.apply($, defs).then(function () {
+            if (invalidFields.length) {
+                return $.Deferred().reject(invalidFields);
+            }
         });
-        return invalid_fields;
     },
     /**
      * returns the active tab pages for each notebook
diff --git a/addons/web/static/src/js/views/kanban/kanban_renderer.js b/addons/web/static/src/js/views/kanban/kanban_renderer.js
index 4fe030eb168..9e4e6865c0f 100644
--- a/addons/web/static/src/js/views/kanban/kanban_renderer.js
+++ b/addons/web/static/src/js/views/kanban/kanban_renderer.js
@@ -112,6 +112,12 @@ var KanbanRenderer = BasicRenderer.extend({
         this.widgets[0].addQuickCreate();
     },
     /**
+     * @returns {Deferred}
+     */
+    canBeSaved: function () {
+        return $.when();
+    },
+    /**
      * Removes a widget (record if ungrouped, column if grouped) from the view.
      *
      * @param {Widget} widget the instance of the widget to remove
diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js
index 337d62b1dc1..67dcecb03ed 100644
--- a/addons/web/static/src/js/views/list/list_editable_renderer.js
+++ b/addons/web/static/src/js/views/list/list_editable_renderer.js
@@ -13,6 +13,7 @@ odoo.define('web.EditableListRenderer', function (require) {
  */
 
 var core = require('web.core');
+var Dialog = require('web.Dialog');
 var Domain = require('web.Domain');
 var ListRenderer = require('web.ListRenderer');
 
@@ -59,6 +60,10 @@ ListRenderer.include({
 
         this.currentRow = null;
         this.currentCol = null;
+
+        // the dirtyRows object is there to keep track of lines that have been
+        // modified by the user
+        this.dirtyRows = {};
     },
     /**
      * @override
@@ -77,6 +82,36 @@ ListRenderer.include({
     //--------------------------------------------------------------------------
 
     /**
+     * @returns {Deferred}
+     */
+    canBeSaved: function () {
+        var self = this;
+        var invalidDirtyWidgets = [];
+        var invalidCleanWidgets = [];
+        _.each(this.widgets, function (widget) {
+            // Note: the isValid method may return a deferred in general, but
+            // not in this case.  Here, this method is called by a fieldone2many
+            // when it need to make sure the state is valid.  So, the widgets
+            // that we consider here are widgets that can be in a one2many, and
+            // we don't support a list inside a list (for now at least).
+            if (!widget.isValid()) {
+                if (self.dirtyRows[widget.dataPointID]) {
+                    invalidDirtyWidgets.push(widget);
+                } else {
+                    invalidCleanWidgets.push(widget);
+                }
+            }
+        });
+        if (invalidDirtyWidgets.length) {
+            // TODO: ask for confirmation, open dialog, and depending on user
+            // input, resolve and reject deferred
+        }
+        if (invalidCleanWidgets.length) {
+            this._unselectRow();
+        }
+        return $.when();
+    },
+    /**
      * This method is called by the controller, when a change has been confirmed
      * by the model.  In that situation, we need to properly update the cell
      * widgets.  (it is necessary, because the value could have been changed by
@@ -452,30 +487,84 @@ ListRenderer.include({
      * This method is called whenever we click/move outside of a row that was
      * in edit mode. This is the moment we save all accumulated changes on that
      * row, if needed.
+     *
+     * If the current row is valid, then it can be unselected. If it is not
+     * valid, we need to check if it is dirty. If it is not dirty, it can be
+     * unselected (and will be removed). If it is dirty, we need to ask the user
+     * for confirmation.  If the user agrees, the row will be removed, otherwise
+     * nothing happens (it is still selected)
+     *
+     * @returns {Deferred} The deferred resolves if the row was unselected (and
+     *   possibly removed).  If may be rejected, when the row is dirty and the
+     *   user refuses to discard its changes.
      */
     _unselectRow: function () {
         var self = this;
+
+        // We need to protect against calling this method when no row is
+        // selected.
         if (this.currentRow === null) {
-            return;
+            return $.when();
         }
-        // trigger the save (if the record isn't dirty, the datamodel won't save
-        // anything)
-        this.trigger_up('field_changed', {
-            dataPointID: this.state.data[this.currentRow].id,
-            changes: {},
-            force_save: true,
-        });
-        this.trigger_up('change_mode', {mode: 'readonly'});
-        var widgets = _.where(this.widgets, {__rowIndex: this.currentRow});
-        _.each(widgets, function (widget) {
-            self._setCellValue(self.currentRow, widget.__colIndex, true);
-            widget.destroy();
+
+        var discarded = false;
+
+        function discardLine() {
+            self.trigger_up('discard_line', {id: currentLineID});
+            discarded = true;
+            def.resolve();
+        }
+
+        var rowWidgets = _.where(this.widgets, {__rowIndex: this.currentRow});
+        var isRowValid = _.every(rowWidgets, function (w) { return w.isValid();});
+        var $row = this.$('tbody tr.o_selected_row');
+
+        var def;
+
+        if (isRowValid) {
+            // trigger the save (if the record isn't dirty, the datamodel won't save
+            // anything)
+            self.trigger_up('field_changed', {
+                dataPointID: self.state.data[self.currentRow].id,
+                changes: {},
+                force_save: true,
+            });
+            self.trigger_up('change_mode', {mode: 'readonly'});
+            $row.removeClass('o_selected_row');
+            $row.find('td').removeClass('o_edit_mode');
+            def = $.when();
+        } else {
+            def = $.Deferred();
+            var currentLineID = this.state.data[this.currentRow].id;
+            var isDirty = this.dirtyRows[currentLineID];
+            if (isDirty) {
+                // if the currently selected record is dirty, we want to ask for
+                // confirmation before discarding it
+                var message = _t("The line has been modified, your changes will be discarded. Are you sure you want to discard the changes ?");
+                var options = {
+                    title: _t("Warning"),
+                    confirm_callback: discardLine,
+                    cancel_callback: def.reject.bind(def)
+                };
+                var dialog = Dialog.confirm(this, message, options);
+                dialog.$modal.on('hidden.bs.modal', def.reject.bind(def));
+            } else {
+                discardLine();
+            }
+        }
+
+        return def.then(function finalizeUnselect() {
+            var widgets = _.where(self.widgets, {__rowIndex: self.currentRow});
+            _.each(widgets, function (widget) {
+                self._setCellValue(self.currentRow, widget.__colIndex, true);
+                widget.destroy();
+            });
+            if (discarded) {
+                $row.remove();
+            }
+            self.widgets = _.without(self.widgets, widgets);
+            self.currentRow = null;
         });
-        self.widgets = _.without(self.widgets, widgets);
-        var $row = this.$('tbody tr').eq(this.currentRow);
-        $row.removeClass('o_selected_row');
-        $row.find('td').removeClass('o_edit_mode');
-        this.currentRow = null;
     },
 
     //--------------------------------------------------------------------------
@@ -547,6 +636,7 @@ ListRenderer.include({
      * @param {OdooEvent} event
      */
     _onFieldChanged: function (event) {
+        this.dirtyRows[event.data.dataPointID] = true;
         var $td = event.target.$el.parent();
         $td.addClass('o_field_dirty');
     },
diff --git a/addons/web/static/src/js/views/view_dialogs.js b/addons/web/static/src/js/views/view_dialogs.js
index fe7ddddaf10..6f0fe8328c3 100644
--- a/addons/web/static/src/js/views/view_dialogs.js
+++ b/addons/web/static/src/js/views/view_dialogs.js
@@ -207,7 +207,7 @@ var FormViewDialog = ViewDialog.extend({
         var self = this;
         var def;
         if (this.options.on_save) {
-            if (this.form_view.checkInvalidFields()) {
+            if (this.form_view.canBeSaved()) {
                 return $.Deferred().reject();
             }
             def = this.options.on_save(this.form_view.model.get(this.form_view.handle));
diff --git a/addons/web/static/test/fields/relational_fields_tests.js b/addons/web/static/test/fields/relational_fields_tests.js
index 0cc697e96e5..68a50ee30e9 100644
--- a/addons/web/static/test/fields/relational_fields_tests.js
+++ b/addons/web/static/test/fields/relational_fields_tests.js
@@ -1312,6 +1312,163 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('one2many list (editable): discarding required empty data', function (assert) {
+        assert.expect(7);
+
+        this.data.turtle.fields.turtle_foo.required = true;
+        delete this.data.turtle.fields.turtle_foo.default;
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<group>' +
+                        '<field name="turtles">' +
+                            '<tree editable="top">' +
+                                '<field name="turtle_foo"/>' +
+                            '</tree>' +
+                        '</field>' +
+                    '</group>' +
+                '</form>',
+            res_id: 2,
+            mockRPC: function (route, args) {
+                if (args.method) {
+                    assert.step(args.method);
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        // edit mode, then click on Add an item, then click elsewhere
+        assert.strictEqual(form.$('tr.o_data_row').length, 0,
+            "should have 0 data rows");
+        form.$buttons.find('.o_form_button_edit').click();
+        form.$('.o_form_field_x2many_list_row_add a').click();
+        form.$('label.o_form_label').first().click();
+        assert.strictEqual(form.$('tr.o_data_row').length, 0,
+            "should still have 0 data rows");
+
+        // click on Add an item again, then click on save
+        form.$('.o_form_field_x2many_list_row_add a').click();
+        form.$buttons.find('.o_form_button_save').click();
+        assert.strictEqual(form.$('tr.o_data_row').length, 0,
+            "should still have 0 data rows");
+
+        assert.verifySteps(['read', 'default_get', 'default_get']);
+        form.destroy();
+    });
+
+    QUnit.test('unselecting a line with missing required data', function (assert) {
+        assert.expect(5);
+
+        this.data.turtle.fields.turtle_foo.required = true;
+        delete this.data.turtle.fields.turtle_foo.default;
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<group>' +
+                        '<field name="turtles">' +
+                            '<tree editable="top">' +
+                                '<field name="turtle_foo"/>' +
+                                '<field name="turtle_int"/>' +
+                            '</tree>' +
+                        '</field>' +
+                    '</group>' +
+                '</form>',
+            res_id: 2,
+        });
+
+        // edit mode, then click on Add an item, then click elsewhere
+        assert.strictEqual(form.$('tr.o_data_row').length, 0,
+            "should have 0 data rows");
+        form.$buttons.find('.o_form_button_edit').click();
+        form.$('.o_form_field_x2many_list_row_add a').click();
+        assert.strictEqual(form.$('tr.o_data_row').length, 1,
+            "should have 1 data rows");
+
+        // adding a value in the non required field, so it is dirty, but with
+        // a missing required field
+        form.$('input[name="turtle_int"]').val('12345').trigger('input');
+
+        // click elsewhere,
+        form.$('label.o_form_label').click();
+        assert.strictEqual($('.modal').length, 1,
+            'a confirmation model should be opened');
+
+        // click on cancel, the line should still be selected
+        $('.modal .modal-footer button.btn-default').click();
+        assert.strictEqual(form.$('tr.o_data_row.o_selected_row').length, 1,
+            "should still have 1 selected data row");
+
+        // click elsewhere, and click on ok (on the confirmation dialog)
+        form.$('label.o_form_label').click();
+        $('.modal .modal-footer button.btn-primary').click();
+        assert.strictEqual(form.$('tr.o_data_row').length, 0,
+            "should have 0 data rows (invalid line has been discarded");
+
+        form.destroy();
+    });
+
+    QUnit.test('editing a o2m, with required field and onchange', function (assert) {
+        assert.expect(12);
+
+        this.data.turtle.fields.turtle_foo.required = true;
+        delete this.data.turtle.fields.turtle_foo.default;
+        this.data.turtle.onchanges = {
+            turtle_foo: function (obj) {
+                obj.turtle_int = obj.turtle_foo.length;
+            },
+        };
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<group>' +
+                        '<field name="turtles">' +
+                            '<tree editable="top">' +
+                                '<field name="turtle_foo"/>' +
+                                '<field name="turtle_int"/>' +
+                            '</tree>' +
+                        '</field>' +
+                    '</group>' +
+                '</form>',
+            res_id: 2,
+            mockRPC: function (route, args) {
+                if (args.method) {
+                    assert.step(args.method);
+                }
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        // edit mode, then click on Add an item
+        assert.strictEqual(form.$('tr.o_data_row').length, 0,
+            "should have 0 data rows");
+        form.$buttons.find('.o_form_button_edit').click();
+        form.$('.o_form_field_x2many_list_row_add a').click();
+
+        // input some text in required turtle_foo field
+        form.$('input[name="turtle_foo"]').val('aubergine').trigger('input');
+        assert.strictEqual(form.$('input[name="turtle_int"]').val(), "9",
+            "onchange should have been triggered");
+
+        // save and check everything is fine
+        form.$buttons.find('.o_form_button_save').click();
+        assert.strictEqual(form.$('.o_data_row td:contains(aubergine)').length, 1,
+            "should have one row with turtle_foo value");
+        assert.strictEqual(form.$('.o_data_row td:contains(9)').length, 1,
+            "should have one row with turtle_int value");
+
+        assert.verifySteps(['read', 'default_get', 'onchange', 'onchange', 'write', 'read', 'read']);
+        form.destroy();
+    });
+
     QUnit.test('onchange in a one2many', function (assert) {
         assert.expect(1);