From a15c03e0bdea20e5cee0da90c738cab580ea5c0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Mon, 17 Dec 2018 14:23:24 +0000 Subject: [PATCH] [FIX] mail_bot: hide Odoobot request when denied (Firefox) Before this commit, the notification "Odoobot has a request" would stay on screen even though the user blocked them in Firefox. This case occurred when the user clicks on "Not Now", which tells the browser that the push notifications should be blocked until the next page reload. This commit fixes this issue, so that the notification is removed when clicking on "Not Now", in addition to cleaning tests related to push notifications. Task-ID 1918476 closes odoo/odoo#29584 --- .../mail_bot/static/src/js/mailbot_service.js | 41 +++++-- .../static/src/js/systray_messaging_menu.js | 1 + .../tests/systray_messaging_menu_tests.js | 111 ++++++++++++++---- 3 files changed, 122 insertions(+), 31 deletions(-) diff --git a/addons/mail_bot/static/src/js/mailbot_service.js b/addons/mail_bot/static/src/js/mailbot_service.js index a75780ab090..d27487305ae 100644 --- a/addons/mail_bot/static/src/js/mailbot_service.js +++ b/addons/mail_bot/static/src/js/mailbot_service.js @@ -12,17 +12,16 @@ var MailBotService = AbstractService.extend({ * @override */ start: function () { - var self = this; + this._hasRequest = (window.Notification && window.Notification.permission === "default") || false; if ('odoobot_initialized' in session && ! session.odoobot_initialized) { - setTimeout(function () { - session.odoobot_initialized = true; - self._rpc({ - model: 'mail.channel', - method: 'init_odoobot', - }); - }, 2*60*1000); + this._showOdoobotTimeout(); } }, + + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + /** * Get the previews related to the OdooBot (conversation not included). * For instance, when there is no conversation with OdooBot and OdooBot has @@ -52,7 +51,31 @@ var MailBotService = AbstractService.extend({ * @returns {boolean} */ hasRequest: function () { - return window.Notification && window.Notification.permission === "default"; + return this._hasRequest; + }, + /** + * Called when user either accepts or refuses push notifications. + */ + removeRequest: function () { + this._hasRequest = false; + }, + + //-------------------------------------------------------------------------- + // Private + //-------------------------------------------------------------------------- + + /** + * @private + */ + _showOdoobotTimeout: function () { + var self = this; + setTimeout(function () { + session.odoobot_initialized = true; + self._rpc({ + model: 'mail.channel', + method: 'init_odoobot', + }); + }, 2*60*1000); }, }); diff --git a/addons/mail_bot/static/src/js/systray_messaging_menu.js b/addons/mail_bot/static/src/js/systray_messaging_menu.js index 05c0ee54f24..2ef1fc56d93 100644 --- a/addons/mail_bot/static/src/js/systray_messaging_menu.js +++ b/addons/mail_bot/static/src/js/systray_messaging_menu.js @@ -56,6 +56,7 @@ return MessagingMenu.include({ * @param {string} value */ _handleResponseNotificationPermission: function (value) { + this.call('mailbot_service', 'removeRequest'); if (value !== 'granted') { this.call('bus_service', 'sendNotification', _t('Permission denied'), _t('Odoo will not have the permission to send native notifications on this device.')); diff --git a/addons/mail_bot/static/tests/systray_messaging_menu_tests.js b/addons/mail_bot/static/tests/systray_messaging_menu_tests.js index a04aba7056a..f4edc452aa0 100644 --- a/addons/mail_bot/static/tests/systray_messaging_menu_tests.js +++ b/addons/mail_bot/static/tests/systray_messaging_menu_tests.js @@ -24,31 +24,44 @@ QUnit.module('MessagingMenu', { }, }; - // Patch mailbot_service so that it does do not do any RPC, and - // the detection of push notifications permissions can be simulated in - // the test cases. By default, shows 'OdooBot has a request' + this.services = _.extend({}, mailTestUtils.getMailServices(), { + mailbot_service: MailBotService + }); + + // By default, permission are to ask user for push notification ("default"). + // Use requestPermissionDef to simulate permission change, e.g. "granted" + this.requestPermissionDef = $.Deferred(); + this.ORIGINAL_WINDOW_NOTIFICATION = window.Notification; + window.Notification = { + permission: "default", + requestPermission: function () { + return self.requestPermissionDef; + }, + }; + + // Patch mailbot_service so that it does do not do any RPC this.hasMailbotRequest = true; testUtils.patch(MailBotService, { /** * @override */ - start: function () {}, - /** - * @override - * @returns {boolean} - */ - hasRequest: function () { - return self.hasMailbotRequest; - }, + _showOdoobotTimeout: function () {}, }); - this.services = _.extend({}, mailTestUtils.getMailServices(), { - mailbot_service: MailBotService + // Patch Bus Service so that it does not play any audio (may raise + // Uncaught rejected Promise due to Chrome autoplay policy: https://goo.gl/xX8pDD) + testUtils.patch(this.services.bus_service, { + /** + * @override + */ + _beep: function () {}, }); }, afterEach: function () { - // unpatch MailBotService + // unpatch MailBotService and BusService testUtils.unpatch(MailBotService); + testUtils.unpatch(this.services.bus_service); + window.Notification = this.ORIGINAL_WINDOW_NOTIFICATION; } }); @@ -62,13 +75,13 @@ QUnit.test('messaging menu widget: rendering with OdooBot has a request', functi }); messagingMenu.appendTo($('#qunit-fixture')); - assert.strictEqual(messagingMenu.$('.o_notification_counter').length, 1, + assert.containsOnce(messagingMenu, '.o_notification_counter', "should display a notification counter next to the messaging menu"); assert.strictEqual(messagingMenu.$('.o_notification_counter').text(), '1', "should display a counter of '1' next to the messaging menu"); - messagingMenu.$('.dropdown-toggle').click(); - assert.strictEqual(messagingMenu.$('.o_preview_info').length, 1, + testUtils.dom.click(messagingMenu.$('.dropdown-toggle')); + assert.containsOnce(messagingMenu, '.o_preview_info', "should display a preview in the messaging menu"); assert.strictEqual(messagingMenu.$('.o_preview_name').text().trim(), 'OdooBot has a request', @@ -79,10 +92,10 @@ QUnit.test('messaging menu widget: rendering with OdooBot has a request', functi messagingMenu.destroy(); }); -QUnit.test('messaging menu widget: rendering without OdooBot has a request', function (assert) { +QUnit.test('messaging menu widget: rendering without OdooBot has a request (denied)', function (assert) { assert.expect(3); - this.hasMailbotRequest = false; + window.Notification.permission = 'denied'; var messagingMenu = new MessagingMenu(); testUtils.addMockEnvironment(messagingMenu, { @@ -91,12 +104,66 @@ QUnit.test('messaging menu widget: rendering without OdooBot has a request', fun }); messagingMenu.appendTo($('#qunit-fixture')); - assert.strictEqual(messagingMenu.$('.o_notification_counter').length, 1, + assert.containsOnce(messagingMenu, '.o_notification_counter', "should display a notification counter next to the messaging menu"); assert.strictEqual(messagingMenu.$('.o_notification_counter').text(), '0', "should display a counter of '0' next to the messaging menu"); - messagingMenu.$('.dropdown-toggle').click(); - assert.strictEqual(messagingMenu.$('.o_preview_info').length, 0, + testUtils.dom.click(messagingMenu.$('.dropdown-toggle')); + assert.containsNone(messagingMenu, '.o_preview_info', + "should display no preview in the messaging menu"); + + messagingMenu.destroy(); +}); + +QUnit.test('messaging menu widget: rendering without OdooBot has a request (accepted)', function (assert) { + assert.expect(3); + + window.Notification.permission = 'granted'; + + var messagingMenu = new MessagingMenu(); + testUtils.addMockEnvironment(messagingMenu, { + data: this.data, + services: this.services, + }); + messagingMenu.appendTo($('#qunit-fixture')); + + assert.containsOnce(messagingMenu, '.o_notification_counter', + "should display a notification counter next to the messaging menu"); + assert.strictEqual(messagingMenu.$('.o_notification_counter').text(), '0', + "should display a counter of '0' next to the messaging menu"); + testUtils.dom.click(messagingMenu.$('.dropdown-toggle')); + assert.containsNone(messagingMenu, '.o_preview_info', + "should display no preview in the messaging menu"); + + messagingMenu.destroy(); +}); + +QUnit.test('messaging menu widget: respond to notification prompt', function (assert) { + assert.expect(4); + + var messagingMenu = new MessagingMenu(); + testUtils.addMockEnvironment(messagingMenu, { + data: this.data, + services: this.services, + }); + messagingMenu.appendTo($('#qunit-fixture')); + + assert.containsOnce(messagingMenu, '.o_notification_counter', + "should display a notification counter next to the messaging menu"); + assert.strictEqual(messagingMenu.$('.o_notification_counter').text(), '1', + "should display a counter of '1' next to the messaging menu"); + + testUtils.dom.click(messagingMenu.$('.dropdown-toggle')); + testUtils.dom.click(messagingMenu.$('.o_preview_info')); + + // simulate "default" response, which is equivalent to "Not Now" in Firefox. + this.requestPermissionDef.resolve("default"); + + assert.strictEqual(messagingMenu.$('.o_notification_counter').text(), '0', + "should display a counter of '0' next to the messaging menu"); + + testUtils.dom.click(messagingMenu.$('.dropdown-toggle')); + assert.containsNone(messagingMenu, '.o_preview_info', "should display no preview in the messaging menu"); messagingMenu.destroy();