[FIX] mail: new message notif on channel only with needaction

Before this commit, any new message in a channel of type "channel"
was automatically opening a chat window and showing new message
counter in tab title when out-of-focus.

This behaviour is only intended for important messages. In chat
(group chat, DM chat, livechat), all new messages are considered
as important so this is good. However, for channels, these are
intended for communication with many users, and we only want to
notify on messages that are explicitly flagged as "needaction".
As a reminder, message are needaction through `@mention` or
reply-to for example.

This commit fixes the issue by limiting notifying out-of-focus of
new messages in channel "channel" to only needaction. Also the
auto-opening of chat window as a consequence from this new message
is also limited to needaction messages.

closes odoo/odoo#149943

Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>

addons/mail/static/src/core/common/thread_service.js

@@ -953,13 +953,9 @@ export class ThreadService {
      * @param {Message} message
      */
     notifyMessageToUser(thread, message) {
-        if (
-            thread.type === "channel" &&
-            message.recipients?.includes(this.store.user) &&
-            message.notIn(thread.needactionMessages)
-        ) {
-            thread.needactionMessages.add(message);
-            thread.message_needaction_counter++;
+        let notify = thread.type !== "channel";
+        if (thread.type === "channel" && message.recipients?.includes(this.store.user)) {
+            notify = true;
         }
         if (
             thread.chatPartner?.eq(this.store.odoobot) ||
@@ -970,8 +966,10 @@ export class ThreadService {
         ) {
             return;
         }
-        this.store.ChatWindow.insert({ thread });
-        this.outOfFocusService.notify(message, thread);
+        if (notify) {
+            this.store.ChatWindow.insert({ thread });
+            this.outOfFocusService.notify(message, thread);
+        }
     }
 
     /**

addons/mail/static/tests/discuss_app/discuss_tests.js

@@ -3,7 +3,6 @@
 import { startServer } from "@bus/../tests/helpers/mock_python_environment";
 import { makeFakePresenceService } from "@bus/../tests/helpers/mock_services";
 import { TEST_USER_IDS } from "@bus/../tests/helpers/test_constants";
-import { waitNotifications } from "@bus/../tests/helpers/websocket_event_deferred";
 
 import { Command } from "@mail/../tests/helpers/command";
 import { patchUiSize } from "@mail/../tests/helpers/patch_ui_size";
@@ -12,7 +11,6 @@ import { start } from "@mail/../tests/helpers/test_utils";
 import {
     editInput,
     makeDeferred,
-    nextTick,
     patchWithCleanup,
     triggerHotkey,
 } from "@web/../tests/helpers/utils";
@@ -945,78 +943,135 @@ QUnit.test("auto-focus composer on opening thread [REQUIRE FOCUS]", async () =>
     await contains(".o-mail-Composer-input:focus");
 });
 
-QUnit.test(
-    "receive new chat message: out of odoo focus (notification, channel)",
-    async (assert) => {
-        const pyEnv = await startServer();
-        const channelId = pyEnv["discuss.channel"].create({ channel_type: "chat" });
-        const { env, openDiscuss } = await start({
-            services: {
-                presence: makeFakePresenceService({ isOdooFocused: () => false }),
-            },
-        });
-        openDiscuss();
-        patchWithCleanup(env.services["title"], {
-            setParts(parts) {
-                if (parts._chat) {
-                    step("set_title_part");
-                    assert.strictEqual(parts._chat, "1 Message");
-                }
-            },
-        });
-        const channel = pyEnv["discuss.channel"].searchRead([["id", "=", channelId]])[0];
-        // simulate receiving a new message with odoo out-of-focused
-        pyEnv["bus.bus"]._sendone(channel, "discuss.channel/new_message", {
-            id: channelId,
-            message: {
-                id: 126,
-                model: "discuss.channel",
-                res_id: channelId,
-            },
-        });
-        await assertSteps(["set_title_part"]);
-    }
-);
-
-QUnit.test("receive new chat message: out of odoo focus (notification, chat)", async (assert) => {
+QUnit.test("no out-of-focus notif on non-needaction message in channel", async (assert) => {
     const pyEnv = await startServer();
-    const channelId = pyEnv["discuss.channel"].create({ channel_type: "chat" });
-    const { env, openDiscuss } = await start({
+    const partnerId = pyEnv["res.partner"].create({ name: "Dumbledore" });
+    const userId = pyEnv["res.users"].create({ partner_id: partnerId });
+    const channelId = pyEnv["discuss.channel"].create({
+        channel_member_ids: [
+            Command.create({ partner_id: pyEnv.currentPartnerId }),
+            Command.create({ partner_id: partnerId }),
+        ],
+        channel_type: "channel",
+    });
+    const { env } = await start({
         services: {
             presence: makeFakePresenceService({ isOdooFocused: () => false }),
         },
     });
-    openDiscuss();
     patchWithCleanup(env.services["title"], {
         setParts(parts) {
             if (parts._chat) {
                 step("set_title_part");
-                assert.strictEqual(parts._chat, "1 Message");
             }
         },
     });
-    const channel = pyEnv["discuss.channel"].searchRead([["id", "=", channelId]])[0];
-    // simulate receiving a new message with odoo out-of-focused
-    pyEnv["bus.bus"]._sendone(channel, "discuss.channel/new_message", {
-        id: channelId,
-        message: {
-            id: 126,
-            model: "discuss.channel",
-            res_id: channelId,
+    await contains(".o_menu_systray i[aria-label='Messages']");
+    await contains(".o-mail-ChatWindow", { count: 0 });
+    // simulate receving new message
+    pyEnv.withUser(userId, () =>
+        env.services.rpc("/mail/message/post", {
+            post_data: { body: "New message", message_type: "comment" },
+            thread_id: channelId,
+            thread_model: "discuss.channel",
+        })
+    );
+    await click(".o_menu_systray i[aria-label='Messages']");
+    await contains(".o-mail-NotificationItem", { text: "Dumbledore: New message" });
+    await contains(".o-mail-ChatWindow", { count: 0 });
+    await assertSteps([]);
+});
+
+QUnit.test("out-of-focus notif on needaction message in channel", async (assert) => {
+    const pyEnv = await startServer();
+    const partnerId = pyEnv["res.partner"].create({ name: "Dumbledore" });
+    const userId = pyEnv["res.users"].create({ partner_id: partnerId });
+    const channelId = pyEnv["discuss.channel"].create({
+        channel_member_ids: [
+            Command.create({ partner_id: pyEnv.currentPartnerId }),
+            Command.create({ partner_id: partnerId }),
+        ],
+        channel_type: "channel",
+    });
+    const { env } = await start({
+        services: {
+            presence: makeFakePresenceService({ isOdooFocused: () => false }),
         },
     });
-    await assertSteps(["set_title_part"]);
+    patchWithCleanup(env.services["title"], {
+        setParts(parts) {
+            if (parts._chat) {
+                step(`set_title_part:${parts._chat}`);
+            }
+        },
+    });
+    await contains(".o_menu_systray i[aria-label='Messages']");
+    await contains(".o-mail-ChatWindow", { count: 0 });
+    // simulate receiving a new needaction message with odoo out-of-focused
+    const currentPartnerId = pyEnv.currentPartnerId;
+    pyEnv.withUser(userId, () =>
+        env.services.rpc("/mail/message/post", {
+            post_data: {
+                body: "@Michell Admin",
+                partner_ids: [currentPartnerId],
+                message_type: "comment",
+            },
+            thread_id: channelId,
+            thread_model: "discuss.channel",
+        })
+    );
+    await contains(".o-mail-ChatWindow");
+    await assertSteps(["set_title_part:1 Message"]);
+});
+
+QUnit.test("receive new chat message: out of odoo focus (notification, chat)", async (assert) => {
+    const pyEnv = await startServer();
+    const partnerId = pyEnv["res.partner"].create({ name: "Dumbledore" });
+    const userId = pyEnv["res.users"].create({ partner_id: partnerId });
+    const channelId = pyEnv["discuss.channel"].create({
+        channel_member_ids: [
+            Command.create({ partner_id: pyEnv.currentPartnerId }),
+            Command.create({ partner_id: partnerId }),
+        ],
+        channel_type: "chat",
+    });
+    const { env } = await start({
+        services: {
+            presence: makeFakePresenceService({ isOdooFocused: () => false }),
+        },
+    });
+    patchWithCleanup(env.services["title"], {
+        setParts(parts) {
+            if (parts._chat) {
+                step(`set_title_part:${parts._chat}`);
+            }
+        },
+    });
+    await contains(".o_menu_systray i[aria-label='Messages']");
+    await contains(".o-mail-ChatWindow", { count: 0 });
+    // simulate receiving a new message with odoo out-of-focused
+    pyEnv.withUser(userId, () =>
+        env.services.rpc("/mail/message/post", {
+            post_data: {
+                body: "New message",
+                message_type: "comment",
+            },
+            thread_id: channelId,
+            thread_model: "discuss.channel",
+        })
+    );
+    await contains(".o-mail-ChatWindow");
+    await assertSteps(["set_title_part:1 Message"]);
 });
 
 QUnit.test("no out-of-focus notification on receiving self messages in chat", async () => {
     const pyEnv = await startServer();
     const channelId = pyEnv["discuss.channel"].create({ channel_type: "chat" });
-    const { env, openDiscuss } = await start({
+    const { env } = await start({
         services: {
             presence: makeFakePresenceService({ isOdooFocused: () => false }),
         },
     });
-    openDiscuss();
     patchWithCleanup(env.services["title"], {
         setParts(parts) {
             if (parts._chat) {
@@ -1024,20 +1079,22 @@ QUnit.test("no out-of-focus notification on receiving self messages in chat", as
             }
         },
     });
-    const channel = pyEnv["discuss.channel"].searchRead([["id", "=", channelId]])[0];
+    await contains(".o_menu_systray i[aria-label='Messages']");
+    await contains(".o-mail-ChatWindow", { count: 0 });
     // simulate receiving a new message of self with odoo out-of-focused
-    pyEnv["bus.bus"]._sendone(channel, "discuss.channel/new_message", {
-        id: channelId,
-        message: {
-            author: { id: pyEnv.currentPartnerId, type: "partner" },
-            id: 126,
-            model: "discuss.channel",
-            res_id: channelId,
-        },
-    });
-    await waitNotifications([env, "discuss.channel/new_message"]);
-    // weak test, no guarantee to wait long enough for the potential step to trigger
-    await nextTick();
+    pyEnv.withUser(pyEnv.currentUserId, () =>
+        env.services.rpc("/mail/message/post", {
+            post_data: {
+                body: "New message",
+                message_type: "comment",
+            },
+            thread_id: channelId,
+            thread_model: "discuss.channel",
+        })
+    );
+    await click(".o_menu_systray i[aria-label='Messages']");
+    await contains(".o-mail-NotificationItem", { text: "You: New message" });
+    await contains(".o-mail-ChatWindow", { count: 0 });
     assertSteps([]);
 });
 

addons/mail/static/tests/helpers/mock_server/models/mail_thread.js

@@ -224,6 +224,13 @@ patch(MockServer.prototype, {
         });
         delete values.subtype_xmlid;
         const messageId = this.pyEnv["mail.message"].create(values);
+        for (const partnerId of kwargs.partner_ids || []) {
+            this.pyEnv["mail.notification"].create({
+                mail_message_id: messageId,
+                notification_type: "inbox",
+                res_partner_id: partnerId,
+            });
+        }
         this._mockMailThread_NotifyThread(model, ids, messageId, context?.temporary_id);
         return Object.assign(this._mockMailMessageMessageFormat([messageId])[0], {
             temporary_id: context?.temporary_id,