[MERGE/FIX] web_tour,*: remove step delay in tests

For historical reasons, the tour manager executed each step of tour
in a setTimeout (10ms). When a step could be executed (i.e. when
its trigger selector has a match in the DOM), the element matching
its trigger was saved as a jQuery element (the $anchor), and in the
setTimeout, the action (e.g. click) was performed on that element.

However, it could happen that, after the delay, the $anchor was no
longer in the DOM, either because
 1) the tip selector had no match anymore, meaning that the element
    had been removed from the DOM meanwhile
 2) the tip selector still had a match in the DOM, but that element
    had been rerendered meanwhile
Case 2) occured sometimes when the main_flow_tour was executed in
community, when trying to open a Manufacturing Order after having
reloaded the Manufacturing Order list view (because the row of the
order to open that was saved as $anchor was the one of the list
before the reload, and that list was re-rendered during the delay).

This rev. removes the default delay of 10ms, but the feature is
kept such that one can still manually run a tour slowly (e.g. to
debug or to make a demo). It also adapts some tours that didn't pass
anymore without the delay, and it fixes a bug in the kanban view that
has been spotted thanks to the delay removal.

closes odoo/odoo#30106

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

@@ -38,6 +38,10 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         FieldManagerMixin.init.call(this, this.model);
         this.handle = params.initialState.id;
         this.mode = params.mode || 'readonly';
+        // savingDef is used to ensure that we always wait for pending save
+        // operations to complete before checking if there are changes to
+        // discard when discardChanges is called
+        this.savingDef = $.when();
     },
     /**
      * @override
@@ -102,14 +106,15 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         return true;
     },
     /**
-     * Waits for the mutex to be unlocked and then calls _.discardChanges.
+     * Waits for the mutex to be unlocked and for changes to be saved, then
+     * calls _.discardChanges.
      * This ensures that the confirm dialog isn't displayed directly if there is
      * a pending 'write' rpc.
      *
      * @see _.discardChanges
      */
     discardChanges: function (recordID, options) {
-        return this.mutex.exec(function () {})
+        return $.when(this.mutex.getUnlockedDef(), this.savingDef)
             .then(this._discardChanges.bind(this, recordID || this.handle, options));
     },
     /**
@@ -183,11 +188,12 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         // mutex-protected as commitChanges function of x2m has to be aware of
         // all final changes made to a row.
         var self = this;
-        return this.mutex.getUnlockedDef().then(function () {
+        this.savingDef = this.mutex.getUnlockedDef().then(function () {
             return self.renderer.commitChanges(recordID || self.handle).then(function () {
                 return self.mutex.exec(self._saveRecord.bind(self, recordID, options));
             });
         });
+        return this.savingDef;
     },
     /**
      * @override

addons/web/static/src/js/views/kanban/kanban_controller.js

@@ -210,15 +210,17 @@ var KanbanController = BasicController.extend({
      */
     _onAddColumn: function (event) {
         var self = this;
-        this.model.createGroup(event.data.value, this.handle).then(function () {
-            var state = self.model.get(self.handle, {raw: true});
-            var ids = _.pluck(state.data, 'res_id').filter(_.isNumber);
-            return self._resequenceColumns(ids);
-        }).then(function () {
-            return self.update({}, {reload: false});
-        }).then(function () {
-            self._updateButtons();
-            self.renderer.quickCreateToggleFold();
+        this.mutex.exec(function () {
+            return self.model.createGroup(event.data.value, self.handle).then(function () {
+                var state = self.model.get(self.handle, {raw: true});
+                var ids = _.pluck(state.data, 'res_id').filter(_.isNumber);
+                return self._resequenceColumns(ids);
+            }).then(function () {
+                return self.update({}, {reload: false});
+            }).then(function () {
+                self._updateButtons();
+                self.renderer.quickCreateToggleFold();
+            });
         });
     },
     /**
@@ -271,11 +273,16 @@ var KanbanController = BasicController.extend({
      * @private
      */
     _onButtonNew: function () {
+        var self = this;
         var state = this.model.get(this.handle, {raw: true});
         var quickCreateEnabled = this.quickCreateEnabled && viewUtils.isQuickCreateEnabled(state);
         if (this.on_create === 'quick_create' && quickCreateEnabled && state.data.length) {
-            // Activate the quick create in the first column
-            this.renderer.addQuickCreate();
+            // activate the quick create in the first column when the mutex is
+            // unlocked, to ensure that there is no pending re-rendering that
+            // would remove it (e.g. if we are currently adding a new column)
+            this.mutex.getUnlockedDef().then(function () {
+                self.renderer.addQuickCreate();
+            });
         } else if (this.on_create && this.on_create !== 'quick_create') {
             // Execute the given action
             this.do_action(this.on_create, {

addons/web/static/tests/views/form_tests.js

@@ -7254,6 +7254,69 @@ QUnit.module('Views', {
         form.destroy();
     });
 
+    QUnit.test('call canBeRemoved while saving', function (assert) {
+        assert.expect(10);
+
+        this.data.partner.onchanges = {
+            foo: function (obj) {
+                obj.display_name = obj.foo === 'trigger onchange' ? 'changed' : 'default';
+            },
+        };
+
+        var onchangeDef;
+        var createDef = $.Deferred();
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form><field name="display_name"/><field name="foo"/></form>',
+            mockRPC: function (route, args) {
+                var result = this._super.apply(this, arguments);
+                if (args.method === 'onchange') {
+                    return $.when(onchangeDef).then(_.constant(result));
+                }
+                if (args.method === 'create') {
+                    return $.when(createDef).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        // edit foo to trigger a delayed onchange
+        onchangeDef = $.Deferred();
+        testUtils.fields.editInput(form.$('.o_field_widget[name=foo]'), 'trigger onchange');
+
+        assert.strictEqual(form.$('.o_field_widget[name=display_name]').val(), 'default');
+
+        // save (will wait for the onchange to return), and will be delayed as well
+        testUtils.dom.click(form.$buttons.find('.o_form_button_save'));
+
+        assert.hasClass(form.$('.o_form_view'), 'o_form_editable');
+        assert.strictEqual(form.$('.o_field_widget[name=display_name]').val(), 'default');
+
+        // simulate a click on the breadcrumbs to leave the form view
+        form.canBeRemoved();
+
+        assert.hasClass(form.$('.o_form_view'), 'o_form_editable');
+        assert.strictEqual(form.$('.o_field_widget[name=display_name]').val(), 'default');
+
+        // unlock the onchange
+        onchangeDef.resolve();
+
+        assert.hasClass(form.$('.o_form_view'), 'o_form_editable');
+        assert.strictEqual(form.$('.o_field_widget[name=display_name]').val(), 'changed');
+
+        // unlock the create
+        createDef.resolve();
+
+        assert.hasClass(form.$('.o_form_view'), 'o_form_readonly');
+        assert.strictEqual(form.$('.o_field_widget[name=display_name]').text(), 'changed');
+        assert.containsNone(document.body, '.modal',
+            "should not display the 'Changes will be discarded' dialog");
+
+        form.destroy();
+    });
+
     QUnit.module('FormViewTABMainButtons');
 
     QUnit.test('using tab in an empty required string field should not move to the next field',function(assert) {

addons/web/static/tests/views/kanban_tests.js

@@ -2161,6 +2161,67 @@ QUnit.module('Views', {
         kanban.destroy();
     });
 
+    QUnit.test('quick create record while adding a new column', function (assert) {
+        assert.expect(10);
+
+        var def = $.Deferred();
+        var kanban = createView({
+            View: KanbanView,
+            model: 'partner',
+            data: this.data,
+            arch: '<kanban on_create="quick_create">' +
+                        '<templates><t t-name="kanban-box">' +
+                            '<div><field name="foo"/></div>' +
+                        '</t></templates>' +
+                    '</kanban>',
+            groupBy: ['product_id'],
+            mockRPC: function (route, args) {
+                var result = this._super.apply(this, arguments);
+                if (args.method === 'name_create' && args.model === 'product') {
+                    return def.then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        assert.containsN(kanban, '.o_kanban_group', 2);
+        assert.containsN(kanban, '.o_kanban_group:first .o_kanban_record', 2);
+
+        // add a new column
+        assert.containsOnce(kanban, '.o_column_quick_create');
+        assert.isNotVisible(kanban.$('.o_column_quick_create input'));
+
+        testUtils.dom.click(kanban.$('.o_quick_create_folded'));
+
+        assert.isVisible(kanban.$('.o_column_quick_create input'));
+
+        testUtils.fields.editInput(kanban.$('.o_column_quick_create input'), 'new column');
+        testUtils.dom.click(kanban.$('.o_column_quick_create button.o_kanban_add'));
+
+        assert.containsN(kanban, '.o_kanban_group', 2);
+
+        // click to add a new record
+        testUtils.dom.click(kanban.$buttons.find('.o-kanban-button-new'));
+
+        // should wait for the column to be created (and view to be re-rendered
+        // before opening the quick create
+        assert.containsNone(kanban, '.o_kanban_quick_create');
+
+        // unlock column creation
+        def.resolve();
+
+        assert.containsN(kanban, '.o_kanban_group', 3);
+        assert.containsOnce(kanban, '.o_kanban_quick_create');
+
+        // quick create record in first column
+        testUtils.fields.editInput(kanban.$('.o_kanban_quick_create input'), 'new record');
+        testUtils.dom.click(kanban.$('.o_kanban_quick_create .o_kanban_add'));
+
+        assert.containsN(kanban, '.o_kanban_group:first .o_kanban_record', 3);
+
+        kanban.destroy();
+    });
+
     QUnit.test('many2many_tags in kanban views', function (assert) {
         assert.expect(12);
 

addons/web_tour/static/src/js/tour_manager.js

@@ -32,7 +32,7 @@ return core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, {
         this.tours = {};
         this.consumed_tours = consumed_tours || [];
         this.running_tour = local_storage.getItem(get_running_key());
-        this.running_step_delay = parseInt(local_storage.getItem(get_running_delay_key()), 10) || 10;
+        this.running_step_delay = parseInt(local_storage.getItem(get_running_delay_key()), 10) || 0;
         this.edition = (_.last(session.server_version_info) === 'e') ? 'enterprise' : 'community';
         this._log = [];
         console.log('Tour Manager is ready.  running_tour=' + this.running_tour);
@@ -171,7 +171,10 @@ return core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, {
             if (this.running_tour && this.running_tour_timeout === undefined) {
                 this._set_running_tour_timeout(this.running_tour, this.active_tooltips[this.running_tour]);
             }
-            this._check_for_tooltip(this.active_tooltips[tour_name], tour_name);
+            var self = this;
+            setTimeout(function () {
+                self._check_for_tooltip(self.active_tooltips[tour_name], tour_name);
+            });
         } else {
             for (var tourName in this.active_tooltips) {
                 var tip = this.active_tooltips[tourName];
@@ -366,11 +369,23 @@ return core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, {
     },
     _to_next_running_step: function (tip, tour_name) {
         if (this.running_tour !== tour_name) return;
+        var self = this;
         this._stop_running_tour_timeout();
+        if (this.running_step_delay) {
+            // warning: due to the delay, it may happen that the $anchor isn't
+            // in the DOM anymore when exec is called, either because:
+            // - it has been removed from the DOM meanwhile and the tip's
+            //   selector doesn't match anything anymore
+            // - it has been re-rendered and thus the selector still has a match
+            //   in the DOM, but executing the step with that $anchor won't work
+            _.delay(exec, this.running_step_delay);
+        } else {
+            exec();
+        }
 
-        var action_helper = new RunningTourActionHelper(tip.widget);
-        _.delay((function () {
-            do_before_unload(this._consume_tip.bind(this, tip, tour_name));
+        function exec() {
+            var action_helper = new RunningTourActionHelper(tip.widget);
+            do_before_unload(self._consume_tip.bind(self, tip, tour_name));
 
             if (typeof tip.run === "function") {
                 tip.run.call(tip.widget, action_helper);
@@ -380,7 +395,7 @@ return core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, {
             } else {
                 action_helper.auto();
             }
-        }).bind(this), this.running_step_delay);
+        }
     },
 
     /**

addons/website_sale/static/src/js/website_sale_tour_buy.js

@@ -21,7 +21,7 @@ tour.register('shop_buy_product', {
         },
         {
             content: "select conference chair",
-            trigger: '.oe_product_cart a:contains("Conference Chair")',
+            trigger: '.oe_product_cart:first a:contains("Conference Chair")',
         },
         {
             content: "select Conference Chair Aluminium",

addons/website_sale_wishlist/static/src/js/website_sale_tour_wishlist.js

@@ -47,14 +47,9 @@ tour.register('shop_wishlist', {
             },
         },
         {
-            content: "check that logged in and search for Customizable Desk",
-            extra_trigger: "li span:contains('Mitchell Admin')",
-            trigger: 'form input[name="search"]',
-            run: "text Customizable Desk",
-        },
-        {
-            content: "submit search",
-            trigger: 'form:has(input[name="search"]) .oe_search_button',
+            content: "check that logged in",
+            trigger: "li span:contains('Mitchell Admin')",
+            run: function () {},
         },
         {
             content: "click on Customizable Desk",

odoo/addons/test_main_flows/static/src/js/tour.js

@@ -211,7 +211,6 @@ tour.register('main_flow_tour', {
     position: 'bottom',
 }, {
     trigger: ".breadcrumb-item:first",
-    extra_trigger: ".o_form_readonly", // FIXME: this is required due to an issue in tour_manager (see [*])
     content: _t("Use the breadcrumbs to <b>go back to products</b>."),
     position: "bottom"
 }, {
@@ -542,12 +541,10 @@ tour.register('main_flow_tour', {
     trigger: ".o_menu_sections a[data-menu-xmlid='mrp.menu_mrp_manufacturing']",
     content: _t('Click on Operations menuitem'),
     position: 'bottom',
-    edition: 'enterprise',
 }, {
     trigger: ".o_menu_sections a[data-menu-xmlid='mrp.menu_mrp_production_action']",
     content: _t('Open manufacturing orders'),
     position: 'bottom',
-    edition: 'enterprise',
 }, {
     trigger: '.o_data_row:has(.o_data_cell:contains("the_flow.product")):first',
     content: _t('Select the generated manufacturing order'),
@@ -755,7 +752,6 @@ tour.register('main_flow_tour', {
 }, {
     edition: "enterprise",
     trigger: ".o_selected_row .o_field_widget[name=name]",
-    extra_trigger: ".o_selected_row .o_field_widget[name=partner_id] .o_external_button", // FIXME: this is required due to an issue in tour_manager (see [*])
     content: _t('Let\'s enter a name.'),
     position: "bottom",
     run: "text the_flow.statement.line",
@@ -781,16 +777,3 @@ tour.register('main_flow_tour', {
     position: "bottom",
 }]);
 });
-
-/*
- * [*] FIXME: issue in tour_manager:
- *     The JQuery element of a step is registered as soon as its trigger and
- *     extra_trigger elements are visible in the DOM, but it's action is delayed
- *     to handle the 'running_step_delay' option (0 by default, but even with 0,
- *     the action is executed after finishing the current execution block, which
- *     may produce a re-rendering of the part of the DOM containing the element).
- *     When this happens, the action is executed on an jquery element not present
- *     in the DOM anymore.
- *     To avoid this, we add an 'extra_trigger' to wait for the updated jquery
- *     element before activating the step (and thus registering the JQuery element).
- */