From cbfb1b909cdf73659c3bedcd6718b09c311dbbf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Tue, 26 Sep 2023 16:16:19 +0200 Subject: [PATCH] [FIX] mail: do not notify chat window state when received from bus The server is supposedly already aware of changes that are coming from itself, it doesn't need to be notified again. This prevents each extra open tab from making useless RPC. This also fixes race conditions in situations where: - close (manually): notify - open (manually): notify - close (from bus): notify close again <--- this is the bug - open (from bus): already no notify for open from bus (ok) - close (from bus again): and notify close yet again Final state is close, whereas the user manually set open last. With the fix, it becomes: - close (manually): notify - open (manually): notify - close (from bus) - open (from bus) Finally state is open as expected, no extra RPC. Note that is this example, there is still a problem, where an obsolete value from the server (the first close from the bus) will overwrite the newer local state (open), causing a potential flicker. This will be fixed in a future commit. runbot-24379 runbot-24994 Part-of: odoo/odoo#136698 --- addons/mail/static/src/core/common/chat_window_service.js | 7 ++++--- .../mail/static/src/core/web/chat_window_service_patch.js | 4 ++-- .../src/discuss/core/web/chat_window_service_patch.js | 7 +++++-- .../src/discuss/core/web/discuss_core_web_service.js | 4 ++-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/addons/mail/static/src/core/common/chat_window_service.js b/addons/mail/static/src/core/common/chat_window_service.js index eb64efbec48..27d1453e3a2 100644 --- a/addons/mail/static/src/core/common/chat_window_service.js +++ b/addons/mail/static/src/core/common/chat_window_service.js @@ -109,7 +109,8 @@ export class ChatWindowService { chatWindow.thread.state = "folded"; } - async close(chatWindow, { escape = false } = {}) { + async close(chatWindow, options = {}) { + const { escape = false } = options; if (!chatWindow.hidden && this.maxVisible < this.store.discuss.chatWindows.length) { const swaped = this.hidden[0]; swaped.hidden = false; @@ -126,10 +127,10 @@ export class ChatWindowService { if (escape && this.store.discuss.chatWindows.length > 0) { this.focus(this.store.discuss.chatWindows[index - 1]); } - await this._onClose(chatWindow); + await this._onClose(chatWindow, options); chatWindow.delete(); } - async _onClose(chatWindow) {} + async _onClose(chatWindow, options) {} } export const chatWindowService = { diff --git a/addons/mail/static/src/core/web/chat_window_service_patch.js b/addons/mail/static/src/core/web/chat_window_service_patch.js index 67d03688a67..f6cbfe27e70 100644 --- a/addons/mail/static/src/core/web/chat_window_service_patch.js +++ b/addons/mail/static/src/core/web/chat_window_service_patch.js @@ -5,7 +5,7 @@ import { ChatWindowService } from "@mail/core/common/chat_window_service"; import { patch } from "@web/core/utils/patch"; patch(ChatWindowService.prototype, { - async _onClose(chatWindow) { + async _onClose(chatWindow, options) { if (this.ui.isSmall && !this.store.discuss.isActive) { // If we are in mobile and discuss is not open, it means the // chat window was opened from the messaging menu. In that @@ -15,6 +15,6 @@ patch(ChatWindowService.prototype, { // ensure messaging menu is opened before chat window is closed await Promise.resolve(); } - await super._onClose(...arguments); + await super._onClose(chatWindow, options); }, }); diff --git a/addons/mail/static/src/discuss/core/web/chat_window_service_patch.js b/addons/mail/static/src/discuss/core/web/chat_window_service_patch.js index 2017a8173ca..858046bfc37 100644 --- a/addons/mail/static/src/discuss/core/web/chat_window_service_patch.js +++ b/addons/mail/static/src/discuss/core/web/chat_window_service_patch.js @@ -5,9 +5,12 @@ import { ChatWindowService } from "@mail/core/common/chat_window_service"; import { patch } from "@web/core/utils/patch"; patch(ChatWindowService.prototype, { - async _onClose(chatWindow) { + async _onClose(chatWindow, options) { + const { notifyState = true } = options; await super._onClose(...arguments); - this.notifyState(chatWindow); + if (notifyState) { + this.notifyState(chatWindow); + } }, hide(chatWindow) { super.hide(...arguments); diff --git a/addons/mail/static/src/discuss/core/web/discuss_core_web_service.js b/addons/mail/static/src/discuss/core/web/discuss_core_web_service.js index f1ca5801cad..74c5b3e91e7 100644 --- a/addons/mail/static/src/discuss/core/web/discuss_core_web_service.js +++ b/addons/mail/static/src/discuss/core/web/discuss_core_web_service.js @@ -83,7 +83,7 @@ export class DiscussCoreWeb { this.store.ChatWindow.insert({ thread: chat }); } }); - this.busService.subscribe("mail.record/insert", async (payload) => { + this.busService.subscribe("mail.record/insert", (payload) => { if (payload.Thread) { const data = payload.Thread; const thread = this.store.Thread.get(data); @@ -94,7 +94,7 @@ export class DiscussCoreWeb { chatWindow.thread?.eq(thread) ); if (chatWindow) { - await this.chatWindowService.close(chatWindow); + this.chatWindowService.close(chatWindow, { notifyState: false }); } } else { this.store.ChatWindow.insert({