[FIX] web: apply onchange even if warning

The onchange route returns field values, but it can also return
a warning, that needs to be displayed in a dialog. When it does,
the new values must be applied anyway, which wasn't the case
before this rev.

We generalized the 'warning' custom event handled by the webclient
so that it can display a warning in a dialog (and not only through
the NotificationManager).

4 files changed, 53 insertions, 18 deletions

diff --git a/addons/web/static/src/js/chrome/abstract_web_client.js b/addons/web/static/src/js/chrome/abstract_web_client.js
index 1caa03f2955..5a84f4fe4b3 100644
--- a/addons/web/static/src/js/chrome/abstract_web_client.js
+++ b/addons/web/static/src/js/chrome/abstract_web_client.js
@@ -16,6 +16,7 @@ var concurrency = require('web.concurrency');
 var core = require('web.core');
 var crash_manager = require('web.crash_manager');
 var data_manager = require('web.data_manager');
+var Dialog = require('web.Dialog');
 var Loading = require('web.Loading');
 var mixins = require('web.mixins');
 var NotificationManager = require('web.notification').NotificationManager;
@@ -23,6 +24,7 @@ var session = require('web.session');
 var Widget = require('web.Widget');
 
 var _t = core._t;
+var qweb = core.qweb;
 
 var AbstractWebClient = Widget.extend(mixins.ServiceProvider, {
     custom_events: {
@@ -46,11 +48,7 @@ var AbstractWebClient = Widget.extend(mixins.ServiceProvider, {
                 this.notification_manager.notify(e.data.title, e.data.message, e.data.sticky);
             }
         },
-        warning: function (e) {
-            if(this.notification_manager) {
-                this.notification_manager.warn(e.data.title, e.data.message, e.data.sticky);
-            }
-        },
+        warning: '_displayWarning',
         load_views: function (event) {
             var params = {
                 model: event.data.modelName,
@@ -284,6 +282,33 @@ var AbstractWebClient = Widget.extend(mixins.ServiceProvider, {
     toggle_fullscreen: function (fullscreen) {
         this.$el.toggleClass('o_fullscreen', fullscreen);
     },
+
+    //--------------------------------------------------------------------------
+    // Handlers
+    //--------------------------------------------------------------------------
+
+    /**
+     * Displays a warning in a dialog of with the NotificationManager
+
+     * @param {OdooEvent} e
+     * @param {string} e.data.message the warning's message
+     * @param {string} e.data.title the warning's title
+     * @param {string} [e.data.type] 'dialog' to display in a dialog
+     * @param {boolean} [e.data.sticky] whether or not the warning should be
+     *   sticky (if displayed with the NotificationManager)
+     */
+    _displayWarning: function (e) {
+        var data = e.data;
+        if (data.type === 'dialog') {
+            new Dialog(this, {
+                size: 'medium',
+                title: data.title,
+                $content: qweb.render("CrashManager.warning", data),
+            }).open();
+        } else if (this.notification_manager) {
+            this.notification_manager.warn(e.data.title, e.data.message, e.data.sticky);
+        }
+    },
 });
 
 return AbstractWebClient;
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 5f1e6ccbb9c..b16bf1e10b0 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -969,7 +969,11 @@ var BasicModel = AbstractModel.extend({
             var args = [idList, currentData, fields, onchange_spec, context];
             return self.performModelRPC(record.model, 'onchange', args, {}).then(function (result) {
                 if (result.warning) {
-                    return $.Deferred().reject(result.warning);
+                    self.trigger_up('warning', {
+                        message: result.warning.message,
+                        title: result.warning.title,
+                        type: 'dialog',
+                    });
                 }
                 var defs = [];
                 _.each(result.value, function (val, name) {
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 f110f9be8bf..00607157a1f 100644
--- a/addons/web/static/src/js/views/field_manager_mixin.js
+++ b/addons/web/static/src/js/views/field_manager_mixin.js
@@ -86,12 +86,6 @@ var FieldManagerMixin = {
             } else {
                 self._confirmChange(dataPointID, result, event);
             }
-        }).fail(function (warning) {
-            new Dialog(self, {
-                size: 'medium',
-                title: warning.title,
-                $content: qweb.render("CrashManager.warning", warning)
-            }).open();
         });
     },
     /**
diff --git a/addons/web/static/test/views/form_tests.js b/addons/web/static/test/views/form_tests.js
index 69f597a5ac2..f768ce21589 100644
--- a/addons/web/static/test/views/form_tests.js
+++ b/addons/web/static/test/views/form_tests.js
@@ -1657,7 +1657,7 @@ QUnit.module('Views', {
     });
 
     QUnit.test('display a dialog if onchange result is a warning', function (assert) {
-        assert.expect(2);
+        assert.expect(5);
 
         this.data.partner.onchanges = { foo: true };
 
@@ -1672,7 +1672,7 @@ QUnit.module('Views', {
             mockRPC: function (route, args) {
                 if (args.method === 'onchange') {
                     return $.when({
-                        valu: {},
+                        value: { int_field: 10 },
                         warning: {
                             title: "Warning",
                             message: "You must first select a partner"
@@ -1681,16 +1681,28 @@ QUnit.module('Views', {
                 }
                 return this._super.apply(this, arguments);
             },
+            intercepts: {
+                warning: function (event) {
+                    assert.strictEqual(event.data.type, 'dialog',
+                        "should have triggered an event with the correct data");
+                    assert.strictEqual(event.data.title, "Warning",
+                        "should have triggered an event with the correct data");
+                    assert.strictEqual(event.data.message, "You must first select a partner",
+                        "should have triggered an event with the correct data");
+                },
+            },
         });
 
         form.$buttons.find('.o_form_button_edit').click();
 
+        assert.strictEqual(form.$('.o_form_input[name=int_field]').val(), '9',
+            "'int_field' value should be 9 before the change");
+
         form.$('input').first().val("tralala").trigger('input');
 
-        assert.strictEqual($('.modal .modal-title:contains(Warning)').length, 1,
-            "should have rendered a modal dialog with correct title");
-        assert.strictEqual($('.modal .modal-body:contains(You must first select a partner)').length, 1,
-            "should have rendered a modal dialog with correct content");
+        assert.strictEqual(form.$('.o_form_input[name=int_field]').val(), '10',
+            "the onchange should have been correctly applied");
+
         form.destroy();
     });