[REF] web: restore editable list views behaviors

Since the new views, the editable list view lost some behaviors. These
behaviors were indeed implemented in the form controller/renderer but
as the new editable list view does not use an inline form view, these
behaviors had to be implemented in the basic controller/renderer.
For this to work, the list editable renderer had to be changed as it
was doing work that should be done by the controller.

The problem is even more complex because the x2m fields are using a
list renderer but not a list controller. So moving code from the
renderer to the controller obviously broke the x2m fields. Right now,
the problem is solved by catching renderer events and forwarding new
ones to the form controller (which handles the x2m specifically).

The initial goal of this commit was to share the validation of records
on save. Indeed, the fields were marked as invalid in the form view but
not in the list view. Also, before the new views, the editable list
view had different behaviors if they were used for a x2m field or not.
As these behaviors are making sense, this commit tries to restore them.

Basically, what we want is:
- When a record is saved (form view save or list view line leaving),
the invalid fields are marked (in red), the names of the related fields
are notified to the user and the record is not left.
- When a record is discarded (form view discard or list view discard),
the user is asked to confirm if the record is dirty before making the
record readonly.
- When a record is discarded, if the record is a new one, then the
record should be abandoned (removed as if never existed). In the form
view this induces to go back in the history and in the list view, to
remove a row.
- For x2m fields, the notification of invalid fields is not triggered
but the user is instead asked if he wants to discard the changes made
to the row (indeed, this replaces the list "Discard" button, as non
existent for x2m lists).
- ...

Saving, discarding, marking the fields as invalid and other behaviors
are thus now shared behavior of basic views.

The management of the dirty flag has also been moved to the model
as it was handle by the controller for the form view but by the
renderer for the list view. Now this flag is directly managed in the
basic model (the model can have changes thanks to the `_changes`
property but not be dirty (this is the case for creations)).
This change however created a problem. The view manager is currently
keeping asking if there are changes to discard at each action which
might lead to leave a dirty record (appswitcher / url change / ...).
It however did not discard anything as leaving if the user is ok with
it will lead to an implicit discard. However, as the view manager might
ask for this discard multiple times by second, the controller was
marking the record as not dirty the first time but without discarding
the changes. This is more complex to do now, as the dirty state is
part of the model and that the renderer should match the model data.
To solve this problem, the view manager now actually discard changes
explicitely when asked to. Even though this had been optimized to not
cause any rerender in some cases where it is not needed, this could
cause some performance decreasing. However, this makes some cases more
logical (opening the app switcher on a dirty form view then going back
to the form view by hitting the "go back" button left the form view
untouched although the user asked to discard it). This solution will
be improved with the view manager refactoring.

This commit is also making use of the `commitChanges` system which had
been implemented for HTML fields. Indeed, these fields cannot know
about all of their changes, so when hitting the save button, we asked
those fields to commit their value. Using this system is a great way
to make the `isValid` method of x2m fields synchronous. Indeed, before
this commit, the method was sometimes asking the user if he wants to
discard an invalid line before save. That case can be handled by the
x2m `commitChanges` method: we consider that saving the lines of x2m is
an operation that has to be done before considering the save, so we ask
all the x2m fields to do so at that time. Also, the system was broken
since a recent commit: we indeed protected the changes - save order
with a mutex but unfortunately, the `commitChanges` method was part of
the save and the changes it triggered were not able to be considered
because of this mutex. This had not been detected by tests as there is
not current way to test html fields.

5 files changed, 27 insertions, 98 deletions

diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index dbe91188242..fa5b1224728 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -1022,7 +1022,7 @@ QUnit.module('basic_fields', {
 
     QUnit.module('HandleWidget');
 
-    QUnit.test('handle widget', function (assert) {
+    QUnit.test('handle widget in x2m', function (assert) {
         assert.expect(6);
 
         this.data.partner.records[0].p = [2, 4];
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index db981d78b88..a33a8f076af 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -258,7 +258,7 @@ QUnit.module('relational_fields', {
             event.data.callback({user_context: {}});
         });
 
-        form._toEditMode();
+        form.$buttons.find('.o_form_button_edit').click();
         var $dropdown = form.$('.o_form_field_many2one input').autocomplete('widget');
 
         form.$('.o_form_field_many2one input').click();
diff --git a/addons/web/static/tests/views/basic_model_tests.js b/addons/web/static/tests/views/basic_model_tests.js
index 6b87a90598c..5f4f94352df 100644
--- a/addons/web/static/tests/views/basic_model_tests.js
+++ b/addons/web/static/tests/views/basic_model_tests.js
@@ -1156,32 +1156,6 @@ QUnit.module('Views', {
         model.destroy();
     });
 
-    QUnit.test('get method works on unset x2many, after save without reload', function (assert) {
-        assert.expect(3);
-
-        var model = createModel({
-            Model: BasicModel,
-            data: this.data,
-        });
-
-        var params = {
-            fieldNames: ['product_ids'],
-            fields: this.data.partner.fields,
-            modelName: 'partner',
-            type: 'record',
-        };
-
-        model.load(params).then(function (resultID) {
-            assert.ok(model.isDirty(resultID), "datapoint should be dirty");
-            model.save(resultID, {reload: false});
-            var record = model.get(resultID, {raw: true});
-            assert.deepEqual(record.data.product_ids, [],
-                "should have correct value for product_ids field");
-            assert.notOk(model.isDirty(resultID), "datapoint should not be dirty");
-        });
-        model.destroy();
-    });
-
     QUnit.test('call makeRecord with a pre-fetched many2one field', function (assert) {
         assert.expect(3);
         var rpcCount = 0;
@@ -1346,7 +1320,7 @@ QUnit.module('Views', {
         var model = createModel({
             Model: BasicModel,
             data: this.data,
-            mockRPC: function(route, args) {
+            mockRPC: function (route, args) {
                 assert.step(args.method);
                 return this._super(route, args);
             },
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index f59c484b3c9..2eb5f6e127b 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -620,17 +620,17 @@ QUnit.module('Views', {
 
         assert.strictEqual(form.mode, 'readonly', 'form view should be in readonly mode');
         assert.ok(form.$('.o_form_view').hasClass('o_form_readonly'),
-                    'form view should have .o_form_readonly');
+            'form view should be .o_form_readonly');
         assert.ok(form.$buttons.find('.o_form_buttons_view').is(':visible'),
             'readonly buttons should be visible');
         assert.ok(!form.$buttons.find('.o_form_buttons_edit').is(':visible'),
             'edit buttons should not be visible');
         form.$buttons.find('.o_form_button_edit').click();
         assert.strictEqual(form.mode, 'edit', 'form view should be in edit mode');
-        assert.ok(form.$el.hasClass('o_form_editable'),
-                    'form view should have .o_form_editable');
-        assert.ok(!form.$el.hasClass('o_form_readonly'),
-                    'form view should not have .o_form_readonly');
+        assert.ok(form.$('.o_form_view').hasClass('o_form_editable'),
+            'form view should be .o_form_editable');
+        assert.ok(!form.$('.o_form_view').hasClass('o_form_readonly'),
+            'form view should not be .o_form_readonly');
         assert.ok(!form.$buttons.find('.o_form_buttons_view').is(':visible'),
             'readonly buttons should not be visible');
         assert.ok(form.$buttons.find('.o_form_buttons_edit').is(':visible'),
@@ -1456,7 +1456,7 @@ QUnit.module('Views', {
 
         // click on discard
         form.$buttons.find('.o_form_button_cancel').click();
-        assert.ok(!$('.modal').length, 'no confirm modal should be displayed');
+        assert.ok(!$('.modal:visible').length, 'no confirm modal should be displayed');
         assert.strictEqual(form.$('.o_form_field').text(), 'yop', 'field in readonly should display yop');
 
         assert.strictEqual(nbWrite, 0, 'no write RPC should have been done');
@@ -1768,7 +1768,7 @@ QUnit.module('Views', {
 
         // click on the pager to switch to the next record and cancel the confirm request
         form.pager.$('.o_pager_next').click(); // click on next
-        assert.strictEqual($('.modal').length, 0, 'no confirm modal should be displayed');
+        assert.strictEqual($('.modal:visible').length, 0, 'no confirm modal should be displayed');
         assert.strictEqual(form.pager.$('.o_pager_value').text(), "2", 'pager value should be 2');
 
         assert.strictEqual(form.$('.o_priority .fa-star-o').length, 2,
@@ -1780,7 +1780,7 @@ QUnit.module('Views', {
             'priority widget should have been updated');
 
         form.pager.$('.o_pager_next').click(); // click on next
-        assert.strictEqual($('.modal').length, 0, 'no confirm modal should be displayed');
+        assert.strictEqual($('.modal:visible').length, 0, 'no confirm modal should be displayed');
         assert.strictEqual(form.pager.$('.o_pager_value').text(), "1", 'pager value should be 1');
 
         // switch to edit mode
@@ -1798,38 +1798,6 @@ QUnit.module('Views', {
         form.destroy();
     });
 
-    QUnit.test('handling dirty state: canBeDiscarded should be idempotent', function (assert) {
-        assert.expect(3);
-
-        var form = createView({
-            View: FormView,
-            model: 'partner',
-            data: this.data,
-            arch: '<form string="Partners">' +
-                    '<field name="foo"></field>' +
-                '</form>',
-            res_id: 1,
-        });
-
-        // switch to edit mode
-        form.$buttons.find('.o_form_button_edit').click();
-        assert.strictEqual(form.$('.o_form_input').val(), 'yop', 'input should contain yop');
-
-        // edit the foo field to make it dirty
-        form.$('.o_form_input').val('new value').trigger('input');
-
-        // discard changes once
-        form.canBeDiscarded();
-        assert.strictEqual($('.modal').length, 1, 'a confirm modal should be displayed');
-        $('.modal .modal-footer .btn-primary').click(); // click on confirm
-
-        // discard changes a second time
-        form.canBeDiscarded();
-        assert.strictEqual($('.modal').length, 0, 'no confirm modal should be displayed');
-
-        form.destroy();
-    });
-
     QUnit.test('restore local state when switching to another record', function (assert) {
         assert.expect(4);
 
@@ -2047,7 +2015,7 @@ QUnit.module('Views', {
         assert.strictEqual(form.pager.$('.o_pager_limit').text(), "3", 'pager limit should be 3');
         assert.strictEqual(form.$('span:contains(yop)').length, 1,
             'should have a field with foo value for record 1');
-        assert.ok(!$('.modal').length, 'no confirm modal should be displayed');
+        assert.ok(!$('.modal:visible').length, 'no confirm modal should be displayed');
 
         // open sidebar
         form.sidebar.$('button.o_dropdown_toggler_btn').click();
@@ -2347,7 +2315,8 @@ QUnit.module('Views', {
         assert.strictEqual(form.$('.oe_stat_button').length, 1,
             "button box should be displayed in edit on an existing record");
 
-        // create mode
+        // create mode (leave edition first!)
+        form.$buttons.find('.o_form_button_cancel').click();
         form.$buttons.find('.o_form_button_create').click();
         assert.strictEqual(form.$('.oe_stat_button').length, 0,
             "button box should not be displayed when creating a new record");
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index 0167c5ab453..d3c46c3f9f7 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -1483,9 +1483,10 @@ QUnit.module('Views', {
         list.destroy();
     });
 
-    QUnit.test('discarding (or not) changes after row edition', function (assert) {
-        assert.expect(8);
+    QUnit.test('leaving unvalid rows in edition', function (assert) {
+        assert.expect(4);
 
+        var warnings = 0;
         var list = createView({
             View: ListView,
             model: 'foo',
@@ -1495,6 +1496,11 @@ QUnit.module('Views', {
                     '<field name="foo" required="1"/>' +
                     '<field name="bar"/>' +
                 '</tree>',
+            intercepts: {
+                warning: function (ev) {
+                    warnings++;
+                },
+            },
         });
 
         // Start first line edition
@@ -1509,33 +1515,13 @@ QUnit.module('Views', {
         $secondFooTd.click();
 
         assert.strictEqual($firstFooTd.parent('.o_selected_row').length, 1,
-            "first line should still be in edition as cannot be auto discarded");
-        assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
-            "no other line should be in edition");
-
-        // Prevent discard of first line
-        $('.modal-footer:visible > .btn-default').click();
-
-        assert.strictEqual($firstFooTd.parent('.o_selected_row').length, 1,
-            "first line should still be in edition as not discarded");
-        assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
-            "no other line should be in edition");
-
-        // Retry starting other line edition
-        $secondFooTd.click();
-
-        assert.strictEqual($firstFooTd.parent('.o_selected_row').length, 1,
-            "first line should still be in edition as cannot be auto discarded");
-        assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
-            "no other line should be in edition");
-
-        // Discard first line
-        $('.modal-footer:visible > .btn-primary').click();
-
-        assert.strictEqual($secondFooTd.parent('.o_selected_row').length, 1,
-            "second line should now be in edition");
+            "first line should still be in edition as invalid");
         assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
             "no other line should be in edition");
+        assert.strictEqual($firstFooTd.find('input.o_form_invalid').length, 1,
+            "the required field should be marked as invalid");
+        assert.strictEqual(warnings, 1,
+            "a warning should have been displayed");
 
         list.destroy();
     });