diff --git a/addons/bus/static/src/js/services/assets_watchdog_service.js b/addons/bus/static/src/js/services/assets_watchdog_service.js index 114988a2afa..5567ed03cb3 100644 --- a/addons/bus/static/src/js/services/assets_watchdog_service.js +++ b/addons/bus/static/src/js/services/assets_watchdog_service.js @@ -8,7 +8,7 @@ export const assetsWatchdogService = { start(env, { notification }) { const assets = {}; - let assetsChangedNotificationId = null; + let isNotificationDisplayed = false; let bundleNotifTimerID = null; env.bus.on("WEB_CLIENT_READY", null, async () => { @@ -28,13 +28,13 @@ export const assetsWatchdogService = { * Displays one notification on user's screen when assets have changed */ function displayBundleChangedNotification() { - if (!assetsChangedNotificationId) { + if (!isNotificationDisplayed) { // Wrap the notification inside a delay. // The server may be overwhelmed with recomputing assets // We wait until things settle down browser.clearTimeout(bundleNotifTimerID); bundleNotifTimerID = browser.setTimeout(() => { - assetsChangedNotificationId = notification.create( + notification.add( env._t("The page appears to be out of date."), { title: env._t("Refresh"), @@ -50,10 +50,11 @@ export const assetsWatchdogService = { }, ], onClose: () => { - assetsChangedNotificationId = null; + isNotificationDisplayed = false; }, } ); + isNotificationDisplayed = true; }, getBundleNotificationDelay()); } } diff --git a/addons/calendar/static/src/js/services/calendar_notification_service.js b/addons/calendar/static/src/js/services/calendar_notification_service.js index 89686c287ba..78c1a20d480 100644 --- a/addons/calendar/static/src/js/services/calendar_notification_service.js +++ b/addons/calendar/static/src/js/services/calendar_notification_service.js @@ -10,7 +10,7 @@ export const calendarNotificationService = { start(env, { action, notification, rpc }) { let calendarNotifTimeouts = {}; let nextCalendarNotifTimeout = null; - const calendarNotif = {}; + const displayedNotifications = new Set(); env.bus.on("WEB_CLIENT_READY", null, async () => { const legacyEnv = owl.Component.env; @@ -38,16 +38,16 @@ export const calendarNotificationService = { // For each notification, set a timeout to display it notifications.forEach(function (notif) { const key = notif.event_id + "," + notif.alarm_id; - if (key in calendarNotif) { + if (displayedNotifications.has(key)) { return; } calendarNotifTimeouts[key] = browser.setTimeout(function () { - const notificationID = notification.create(notif.message, { + const notificationRemove = notification.add(notif.message, { title: notif.title, type: "warning", sticky: true, onClose: () => { - delete calendarNotif[key]; + displayedNotifications.delete(key); }, buttons: [ { @@ -55,7 +55,7 @@ export const calendarNotificationService = { primary: true, onClick: async () => { await rpc("/calendar/notify_ack"); - notification.close(calendarNotif[key]); + notificationRemove(); }, }, { @@ -67,18 +67,18 @@ export const calendarNotificationService = { resId: notif.event_id, } ); - notification.close(calendarNotif[key]); + notificationRemove(); }, }, { name: env._t("Snooze"), onClick: () => { - notification.close(calendarNotif[key]); + notificationRemove(); }, }, ], }); - calendarNotif[key] = notificationID; + displayedNotifications.add(key); }, notif.timer * 1000); lastNotifTimer = Math.max(lastNotifTimer, notif.timer); }); diff --git a/addons/web/static/src/core/errors/error_handlers.js b/addons/web/static/src/core/errors/error_handlers.js index 48d01e9ca6b..fdbd4b11fbe 100644 --- a/addons/web/static/src/core/errors/error_handlers.js +++ b/addons/web/static/src/core/errors/error_handlers.js @@ -109,7 +109,7 @@ errorHandlerRegistry.add("rpcErrorHandler", rpcErrorHandler, { sequence: 97 }); // Lost connection errors // ----------------------------------------------------------------------------- -let connectionLostNotifId = null; +let connectionLostNotifRemove = null; /** * @param {OdooEnv} env * @param {UncaughError} error @@ -121,12 +121,12 @@ function lostConnectionHandler(env, error, originalError) { return false; } if (originalError instanceof ConnectionLostError) { - if (connectionLostNotifId) { + if (connectionLostNotifRemove) { // notification already displayed (can occur if there were several // concurrent rpcs when the connection was lost) return true; } - connectionLostNotifId = env.services.notification.create( + connectionLostNotifRemove = env.services.notification.add( env._t("Connection lost. Trying to reconnect..."), { sticky: true } ); @@ -135,9 +135,11 @@ function lostConnectionHandler(env, error, originalError) { env.services .rpc("/web/webclient/version_info", {}) .then(function () { - env.services.notification.close(connectionLostNotifId); - connectionLostNotifId = null; - env.services.notification.create( + if (connectionLostNotifRemove) { + connectionLostNotifRemove(); + connectionLostNotifRemove = null; + } + env.services.notification.add( env._t("Connection restored. You are back online."), { type: "info", diff --git a/addons/web/static/src/core/notifications/notification.js b/addons/web/static/src/core/notifications/notification.js index 6359ec50bdb..34a7b7ec1ed 100644 --- a/addons/web/static/src/core/notifications/notification.js +++ b/addons/web/static/src/core/notifications/notification.js @@ -1,12 +1,9 @@ /** @odoo-module **/ -import { useService } from "../service_hook"; - const { Component, hooks } = owl; export class Notification extends Component { setup() { - this.notificationService = useService("notification"); if (this.props.onClose) { hooks.onWillUnmount(() => this.props.onClose()); } @@ -34,7 +31,6 @@ export class Notification extends Component { Notification.template = "web.NotificationWowl"; Notification.props = { - id: { type: Number }, message: { type: String }, title: { type: [String, Boolean], optional: true }, type: { diff --git a/addons/web/static/src/core/notifications/notification.xml b/addons/web/static/src/core/notifications/notification.xml index d041a906d7a..577ab48ad3b 100644 --- a/addons/web/static/src/core/notifications/notification.xml +++ b/addons/web/static/src/core/notifications/notification.xml @@ -6,8 +6,7 @@ role="alert" aria-live="assertive" aria-atomic="true">
diff --git a/addons/web/static/src/core/notifications/notification_container.js b/addons/web/static/src/core/notifications/notification_container.js index f732252a274..64a6fde8634 100644 --- a/addons/web/static/src/core/notifications/notification_container.js +++ b/addons/web/static/src/core/notifications/notification_container.js @@ -10,17 +10,57 @@ export class NotificationContainer extends Component { setup() { this.notifications = []; const { bus } = useService("notification"); - bus.on("UPDATE", this, (notifications) => { - this.notifications = notifications; + this.bus = bus; + this.bus.on("ADD", this, this.add); + this.bus.on("REMOVE", this, this.remove); + } + __destroy() { + this.bus.off("ADD", this, this.add); + this.bus.off("REMOVE", this, this.remove); + super.__destroy(); + } + + /** + * @param {Object} params + * @param {number} params.id + * @param {{ + * name: string; + * icon?: string; + * primary?: boolean; + * onClick: () => void + * }[]} params.buttons + * @param {string} params.className + * @param {string} params.message + * @param {boolean} params.messageIsHtml + * @param {() => void} params.onClose + * @param {string} params.title + * @param {"warning" | "danger" | "success" | "info"} params.type + */ + add(params) { + const { id, ...props } = params; + this.notifications.push({ id, props }); + this.render(); + } + /** + * @param {number} id + */ + remove(id) { + const index = this.notifications.findIndex((n) => n.id === id); + if (index > -1) { + this.notifications.splice(index, 1); this.render(); - }); + } } } NotificationContainer.template = tags.xml`
- +
`; NotificationContainer.components = { NotificationComponent }; diff --git a/addons/web/static/src/core/notifications/notification_service.js b/addons/web/static/src/core/notifications/notification_service.js index 05752f05021..d9a48020037 100644 --- a/addons/web/static/src/core/notifications/notification_service.js +++ b/addons/web/static/src/core/notifications/notification_service.js @@ -10,38 +10,42 @@ const AUTOCLOSE_DELAY = 4000; export const notificationService = { start() { let notifId = 0; - let notifications = []; const bus = new EventBus(); - function close(id, wait = 0) { - function _close() { - const index = notifications.findIndex((n) => n.id === id); - if (index > -1) { - notifications.splice(index, 1); - bus.trigger("UPDATE", notifications); + return { + bus, + /** + * @param {string} message + * @param {Object} [options] + * @param {{ + * name: string; + * icon?: string; + * primary?: boolean; + * onClick: () => void + * }[]} [options.buttons] + * @param {string} [options.className] + * @param {boolean} [options.messageIsHtml] + * @param {() => void} [options.onClose] + * @param {boolean} [options.sticky] + * @param {string} [options.title] + * @param {"warning" | "danger" | "success" | "info"} [options.type] + */ + add(message, options = {}) { + const notif = Object.assign({}, options, { + id: ++notifId, + message, + }); + const sticky = notif.sticky; + delete notif.sticky; + bus.trigger("ADD", notif); + if (!sticky) { + browser.setTimeout(() => bus.trigger("REMOVE", notif.id), AUTOCLOSE_DELAY); } - } - if (wait > 0) { - browser.setTimeout(_close, wait); - } else { - _close(); - } - } - function create(message, options) { - const notif = Object.assign({}, options, { - id: ++notifId, - message, - }); - const sticky = notif.sticky; - delete notif.sticky; - notifications.push(notif); - bus.trigger("UPDATE", notifications); - if (!sticky) { - browser.setTimeout(() => close(notif.id), AUTOCLOSE_DELAY); - } - return notif.id; - } - return { close, create, bus }; + return () => { + bus.trigger("REMOVE", notif.id); + }; + }, + }; }, }; diff --git a/addons/web/static/src/legacy/action_adapters.js b/addons/web/static/src/legacy/action_adapters.js index fcd9ece0309..95e4f773726 100644 --- a/addons/web/static/src/legacy/action_adapters.js +++ b/addons/web/static/src/legacy/action_adapters.js @@ -104,7 +104,7 @@ class ActionAdapter extends ComponentAdapter { message: payload.message, }); } else { - this.notifications.create(payload.message, { + this.notifications.add(payload.message, { className: payload.className, sticky: payload.sticky, title: payload.title, diff --git a/addons/web/static/src/legacy/utils.js b/addons/web/static/src/legacy/utils.js index 0261d138f72..9164e584a33 100644 --- a/addons/web/static/src/legacy/utils.js +++ b/addons/web/static/src/legacy/utils.js @@ -196,6 +196,9 @@ export function makeLegacyNotificationService(legacyEnv) { return { dependencies: ["notification"], start(env) { + let notifId = 0; + const idsToRemoveFn = {}; + function notify({ title, message, @@ -224,7 +227,7 @@ export function makeLegacyNotificationService(legacyEnv) { }; }); - return env.services.notification.create(message, { + const removeFn = env.services.notification.add(message, { sticky, title, type, @@ -233,12 +236,23 @@ export function makeLegacyNotificationService(legacyEnv) { buttons, messageIsHtml, }); + const id = ++notifId; + idsToRemoveFn[id] = removeFn; + return id; } - function close(...args) { + function close(id, _, wait) { //the legacy close method had 3 arguments : the notification id, silent and wait. //the new close method only has 2 arguments : the notification id and wait. - return env.services.notification.close(args[0], args[2] ? args[2] : 0); + const removeFn = idsToRemoveFn[id]; + delete idsToRemoveFn[id]; + if (wait) { + browser.setTimeout(() => { + removeFn(id); + }, wait); + } else { + removeFn(id); + } } legacyEnv.services.notification = { notify, close }; diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js index ee375c42b9a..66e5b0b7038 100644 --- a/addons/web/static/src/webclient/actions/action_service.js +++ b/addons/web/static/src/webclient/actions/action_service.js @@ -607,7 +607,7 @@ function makeActionManager(env) { "A popup window has been blocked. You may need to change your " + "browser settings to allow popup windows for this page." ); - env.services.notification.create(msg, { + env.services.notification.add(msg, { sticky: true, type: "warning", }); @@ -865,7 +865,7 @@ function makeActionManager(env) { const state = await wkhtmltopdfStateProm; // display a notification according to wkhtmltopdf's state if (state in WKHTMLTOPDF_MESSAGES) { - env.services.notification.create(WKHTMLTOPDF_MESSAGES[state], { + env.services.notification.add(WKHTMLTOPDF_MESSAGES[state], { sticky: true, title: env._t("Report"), }); diff --git a/addons/web/static/src/webclient/actions/client_actions.js b/addons/web/static/src/webclient/actions/client_actions.js index 915d4ea1cda..fec45e0a16a 100644 --- a/addons/web/static/src/webclient/actions/client_actions.js +++ b/addons/web/static/src/webclient/actions/client_actions.js @@ -19,7 +19,7 @@ export const displayNotificationAction = (env, action) => { return `${escape(link.label)}`; }); const message = sprintf(escape(params.message), ...links); - env.services.notification.create(message, options); + env.services.notification.add(message, options); return params.next; }; diff --git a/addons/web/static/src/webclient/effects/effect_service.js b/addons/web/static/src/webclient/effects/effect_service.js index 6897fb57afe..8f60e25e5b8 100644 --- a/addons/web/static/src/webclient/effects/effect_service.js +++ b/addons/web/static/src/webclient/effects/effect_service.js @@ -34,7 +34,7 @@ export const effectService = { */ function applyEffect(effect) { if (!user.showEffect) { - notification.create(effect.message, { sticky: false }); + notification.add(effect.message, { sticky: false }); } else { bus.trigger("UPDATE", effect); } diff --git a/addons/web/static/tests/core/errors/error_service_tests.js b/addons/web/static/tests/core/errors/error_service_tests.js index 901b5126055..a517e26f6b8 100644 --- a/addons/web/static/tests/core/errors/error_service_tests.js +++ b/addons/web/static/tests/core/errors/error_service_tests.js @@ -126,12 +126,13 @@ QUnit.test("handle CONNECTION_LOST_ERROR", async (assert) => { callback(); }, }); - const mockCreate = (message) => { + const mock = (message) => { assert.step(`create (${message})`); - return 1234; + return () => { + assert.step(`close`); + }; }; - const mockClose = (id) => assert.step(`close (${id})`); - serviceRegistry.add("notification", makeFakeNotificationService(mockCreate, mockClose), { + serviceRegistry.add("notification", makeFakeNotificationService(mock), { force: true, }); const values = [false, true]; // simulate the 'back online status' after 2 'version_info' calls @@ -157,7 +158,7 @@ QUnit.test("handle CONNECTION_LOST_ERROR", async (assert) => { "version_info", "set timeout (>2000)", "version_info", - "close (1234)", + "close", "create (Connection restored. You are back online.)", ]); }); diff --git a/addons/web/static/tests/core/notifications/notifications_tests.js b/addons/web/static/tests/core/notifications/notifications_tests.js index 5e894103c97..1603c9efa70 100644 --- a/addons/web/static/tests/core/notifications/notifications_tests.js +++ b/addons/web/static/tests/core/notifications/notifications_tests.js @@ -25,7 +25,7 @@ QUnit.test("can display a basic notification", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a basic notification"); + notifService.add("I'm a basic notification"); await nextTick(); assert.containsOnce(target, ".o_notification"); const notif = target.querySelector(".o_notification"); @@ -41,7 +41,7 @@ QUnit.test("title and message are escaped by default", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("Some message", { title: "Some title" }); + notifService.add("Some message", { title: "Some title" }); await nextTick(); assert.containsOnce(target, ".o_notification"); const notif = target.querySelector(".o_notification"); @@ -60,7 +60,7 @@ QUnit.test("notification with messageIsHtml option", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("Some message", { messageIsHtml: true }); + notifService.add("Some message", { messageIsHtml: true }); await nextTick(); assert.containsOnce(target, ".o_notification"); const notif = target.querySelector(".o_notification"); @@ -76,7 +76,7 @@ QUnit.test("can display a notification of type danger", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a danger notification", { type: "danger" }); + notifService.add("I'm a danger notification", { type: "danger" }); await nextTick(); assert.containsOnce(target, ".o_notification"); const notif = target.querySelector(".o_notification"); @@ -92,7 +92,7 @@ QUnit.test("can display a danger notification with a title", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a danger notification", { title: "Some title", type: "danger" }); + notifService.add("I'm a danger notification", { title: "Some title", type: "danger" }); await nextTick(); assert.containsOnce(target, ".o_notification"); const notif = target.querySelector(".o_notification"); @@ -109,7 +109,7 @@ QUnit.test("can display a notification with a button", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a notification with button", { + notifService.add("I'm a notification with button", { buttons: [ { name: "I'm a button", @@ -138,7 +138,7 @@ QUnit.test("can display a notification with a callback when closed", async (asse const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a sticky notification", { + notifService.add("I'm a sticky notification", { sticky: true, onClose: () => { assert.step("Notification closed"); @@ -166,7 +166,7 @@ QUnit.test("notifications aren't sticky by default", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a notification"); + notifService.add("I'm a notification"); await nextTick(); assert.containsOnce(target, ".o_notification"); timeoutCB(); // should close the notification @@ -184,7 +184,7 @@ QUnit.test("can display a sticky notification", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - notifService.create("I'm a sticky notification", { sticky: true }); + notifService.add("I'm a sticky notification", { sticky: true }); await nextTick(); assert.containsOnce(target, ".o_notification"); }); @@ -194,16 +194,16 @@ QUnit.test("can close sticky notification", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - let id = notifService.create("I'm a sticky notification", { sticky: true }); + const closeNotif = notifService.add("I'm a sticky notification", { sticky: true }); await nextTick(); assert.containsOnce(target, ".o_notification"); // close programmatically - notifService.close(id); + closeNotif(); await nextTick(); assert.containsNone(target, ".o_notification"); - id = notifService.create("I'm a sticky notification", { sticky: true }); + notifService.add("I'm a sticky notification", { sticky: true }); await nextTick(); assert.containsOnce(target, ".o_notification"); @@ -212,7 +212,8 @@ QUnit.test("can close sticky notification", async (assert) => { assert.containsNone(target, ".o_notification"); }); -QUnit.test("can close sticky notification with wait", async (assert) => { +// The timeout have to be done by the one that uses the notification service +QUnit.skip("can close sticky notification with wait", async (assert) => { let timeoutCB; patchWithCleanup(browser, { setTimeout: (cb, t) => { @@ -252,12 +253,12 @@ QUnit.test("can close a non-sticky notification", async (assert) => { const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - const id = notifService.create("I'm a sticky notification"); + const closeNotif = notifService.add("I'm a sticky notification"); await nextTick(); assert.containsOnce(target, ".o_notification"); // close the notification - notifService.close(id); + closeNotif(); await nextTick(); assert.containsNone(target, ".o_notification"); @@ -279,13 +280,13 @@ QUnit.test("close a non-sticky notification while another one remains", async (a const notifService = env.services.notification; await mount(NotificationContainer, { env, target }); - const id1 = notifService.create("I'm a non-sticky notification"); - const id2 = notifService.create("I'm a sticky notification", { sticky: true }); + const closeNotif1 = notifService.add("I'm a non-sticky notification"); + const closeNotif2 = notifService.add("I'm a sticky notification", { sticky: true }); await nextTick(); assert.containsN(target, ".o_notification", 2); // close the non sticky notification - notifService.close(id1); + closeNotif1(); await nextTick(); assert.containsOnce(target, ".o_notification"); @@ -295,7 +296,7 @@ QUnit.test("close a non-sticky notification while another one remains", async (a assert.containsOnce(target, ".o_notification"); // close the non sticky notification - notifService.close(id2); + closeNotif2(); await nextTick(); assert.containsNone(target, ".o_notification"); }); @@ -305,7 +306,7 @@ QUnit.test("notification coming when NotificationManager not mounted yet", async const notifService = env.services.notification; mount(NotificationContainer, { env, target }); - notifService.create("I'm a non-sticky notification"); + notifService.add("I'm a non-sticky notification"); await nextTick(); assert.containsOnce(target, ".o_notification"); }); diff --git a/addons/web/static/tests/helpers/mock_services.js b/addons/web/static/tests/helpers/mock_services.js index c6b949e485c..8c9c102f9e4 100644 --- a/addons/web/static/tests/helpers/mock_services.js +++ b/addons/web/static/tests/helpers/mock_services.js @@ -224,22 +224,16 @@ export const fakeTitleService = { }, }; -export function makeFakeNotificationService(createMock, closeMock) { +export function makeFakeNotificationService(mock) { return { start() { - function create() { - if (createMock) { - return createMock(...arguments); - } - } - function close() { - if (closeMock) { - return closeMock(...arguments); + function add() { + if (mock) { + return mock(...arguments); } } return { - create, - close, + add, }; }, };