diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js index 76adef0a154..01bbd9cd2fe 100644 --- a/addons/web/static/src/js/views/basic/basic_controller.js +++ b/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 diff --git a/addons/web/static/src/js/views/kanban/kanban_controller.js b/addons/web/static/src/js/views/kanban/kanban_controller.js index 6ffbad9612a..cf3404ae420 100644 --- a/addons/web/static/src/js/views/kanban/kanban_controller.js +++ b/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, { diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index 8099c58092a..e5b5f0cf36f 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/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: '
', + 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) { diff --git a/addons/web/static/tests/views/kanban_tests.js b/addons/web/static/tests/views/kanban_tests.js index 87e4ebd2748..dbe8e748d33 100644 --- a/addons/web/static/tests/views/kanban_tests.js +++ b/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: '' + + '' + + '
' + + '
' + + '
', + 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); diff --git a/addons/web_tour/static/src/js/tour_manager.js b/addons/web_tour/static/src/js/tour_manager.js index 60b36ddc5c9..32e7af075bf 100644 --- a/addons/web_tour/static/src/js/tour_manager.js +++ b/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); + } }, /** diff --git a/addons/website_sale/static/src/js/website_sale_tour_buy.js b/addons/website_sale/static/src/js/website_sale_tour_buy.js index 1797a59ea9b..87ae9564b65 100644 --- a/addons/website_sale/static/src/js/website_sale_tour_buy.js +++ b/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", diff --git a/addons/website_sale_wishlist/static/src/js/website_sale_tour_wishlist.js b/addons/website_sale_wishlist/static/src/js/website_sale_tour_wishlist.js index b03e0108fb2..8089d8fb3e7 100644 --- a/addons/website_sale_wishlist/static/src/js/website_sale_tour_wishlist.js +++ b/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", diff --git a/odoo/addons/test_main_flows/static/src/js/tour.js b/odoo/addons/test_main_flows/static/src/js/tour.js index 047b0c40543..c49ae1cb4eb 100644 --- a/odoo/addons/test_main_flows/static/src/js/tour.js +++ b/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 go back to products."), 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). - */