[FIX] mail: incorrect unread counter on message post

Before this PR, a race condition occurred when posting a message: if
the bus notification was received before the result of the message
post RPC, the unread counter was set to the number of messages in the
channel.

This occurs because the `discuss.channel.member/seen` notification is
processed before the `discuss.channel/new_message` one so the message
is not yet inserted in `thread.messages`.

Actually, the channel seen notification is not needed in this case: the
last seen message of the author is the message received in the new
message notification.

Steps to reproduce the issue:
- Go to discuss as admin
- Open a chat with demo
- Set network throttling to "fast 3g"
- Send a message in the chat
- The unread counter is now set to "1" which is incorrect

closes #159404

X-original-commit: 039d596
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
Signed-off-by: Matthieu Stockbauer (tsm) <tsm@odoo.com>

addons/mail/models/discuss/discuss_channel.py

@@ -666,7 +666,7 @@ def _message_post_after_hook(self, message, msg_vals):
         """
         Automatically set the message posted by the current user as seen for themselves.
         """
-        self._set_last_seen_message(message)
+        self._set_last_seen_message(message, notify=False)
         return super()._message_post_after_hook(message, msg_vals)
 
     def _check_can_update_message_content(self, message):
@@ -1039,12 +1039,14 @@ def _channel_seen(self, last_message_id=None, allow_older=False):
         self._set_last_seen_message(last_message, allow_older=allow_older)
         return last_message.id
 
-    def _set_last_seen_message(self, last_message, allow_older=False):
+    def _set_last_seen_message(self, last_message, allow_older=False, notify=True):
         """
         Set last seen message of `self` channels for the current persona.
         :param last_message: the message to set as last seen message
         :param allow_order: whether to allow setting and older message
         as the last seen message.
+        :param notify: whether to send a `discuss.channel.member/seen`
+        notification.
         """
         current_partner, current_guest = self.env["res.partner"]._get_current_persona()
         if not current_partner and not current_guest:
@@ -1065,16 +1067,17 @@ def _set_last_seen_message(self, last_message, allow_older=False):
             'seen_message_id': last_message.id,
             'last_seen_dt': fields.Datetime.now(),
         })
-        data = {
-            'channel_id': self.id,
-            'id': member.id,
-            'last_message_id': last_message.id,
-        }
-        data['partner_id' if current_partner else 'guest_id'] = current_partner.id if current_partner else current_guest.id
-        target = current_partner or current_guest
-        if self.channel_type in self._types_allowing_seen_infos():
-            target = self
-        self.env['bus.bus']._sendone(target, 'discuss.channel.member/seen', data)
+        if notify:
+            data = {
+                'channel_id': self.id,
+                'id': member.id,
+                'last_message_id': last_message.id,
+            }
+            data['partner_id' if current_partner else 'guest_id'] = current_partner.id if current_partner else current_guest.id
+            target = current_partner or current_guest
+            if self.channel_type in self._types_allowing_seen_infos():
+                target = self
+            self.env['bus.bus']._sendone(target, 'discuss.channel.member/seen', data)
 
     def _types_allowing_seen_infos(self):
         """ Return the channel types which allow sending seen infos notification

addons/mail/static/src/discuss/core/common/discuss_core_common_service.js

@@ -245,6 +245,15 @@ export class DiscussCoreCommon {
             this.threadService.markAsRead(channel);
         }
         this.env.bus.trigger("discuss.channel/new_message", { channel, message });
+        const authorMember = channel.channelMembers.find(({ persona }) =>
+            persona?.eq(message.author)
+        );
+        if (authorMember) {
+            authorMember.seen_message_id = message;
+        }
+        if (authorMember?.eq(channel.selfMember)) {
+            this.threadService.updateSeen(authorMember.thread, message.id);
+        }
     }
 }
 

addons/mail/static/tests/core/out_of_focus.test.js

@@ -4,12 +4,15 @@ import {
     contains,
     defineMailModels,
     onRpcBefore,
+    openDiscuss,
     start,
     startServer,
     step,
 } from "../mail_test_helpers";
-import { mockService, serverState } from "@web/../tests/web_test_helpers";
+import { Command, mockService, serverState } from "@web/../tests/web_test_helpers";
 import { presenceService } from "@bus/services/presence_service";
+import { rpc } from "@web/core/network/rpc";
+import { withUser } from "@web/../tests/_framework/mock_server/mock_server";
 
 describe.current.tags("desktop");
 defineMailModels();
@@ -25,8 +28,15 @@ test("Spaces in notifications are not encoded", async () => {
         isOdooFocused: () => false,
     }));
     const pyEnv = await startServer();
-    const channelId = pyEnv["discuss.channel"].create({ channel_type: "chat" });
-    const channel = pyEnv["discuss.channel"].search_read([["id", "=", channelId]])[0];
+    const bobUserId = pyEnv["res.users"].create({ name: "bob" });
+    const bobPartnerId = pyEnv["res.partner"].create({ name: "bob", user_ids: [bobUserId] });
+    const channelId = pyEnv["discuss.channel"].create({
+        channel_type: "chat",
+        channel_member_ids: [
+            Command.create({ partner_id: serverState.partnerId }),
+            Command.create({ partner_id: bobPartnerId }),
+        ],
+    });
     await start();
     await assertSteps([
         `/mail/action - ${JSON.stringify({
@@ -36,15 +46,13 @@ test("Spaces in notifications are not encoded", async () => {
             context: { lang: "en", tz: "taht", uid: serverState.userId, allowed_company_ids: [1] },
         })}`,
     ]);
-    // send after init_messaging because bus subscription is done after init_messaging
-    pyEnv["bus.bus"]._sendone(channel, "discuss.channel/new_message", {
-        id: channelId,
-        message: {
-            body: "Hello world!",
-            id: 126,
-            model: "discuss.channel",
-            res_id: channelId,
-        },
-    });
+    await openDiscuss();
+    await withUser(bobUserId, () =>
+        rpc("/mail/message/post", {
+            post_data: { body: "Hello world!", message_type: "comment" },
+            thread_id: channelId,
+            thread_model: "discuss.channel",
+        })
+    );
     await contains(".o_notification:has(.o_notification_bar.bg-info)", { text: "Hello world!" });
 });

addons/mail/static/tests/discuss_app/discuss.test.js

@@ -1120,9 +1120,23 @@ test("no out-of-focus notif on non-needaction message in channel", async () => {
 test("receive new chat messages: out of odoo focus (tab title)", async () => {
     let stepCount = 0;
     const pyEnv = await startServer();
+    const bobUserId = pyEnv["res.users"].create({ name: "bob" });
+    const bobPartnerId = pyEnv["res.partner"].create({ name: "bob", user_ids: [bobUserId] });
     const [channelId_1, channelId_2] = pyEnv["discuss.channel"].create([
-        { channel_type: "chat" },
-        { channel_type: "chat" },
+        {
+            channel_type: "chat",
+            channel_member_ids: [
+                Command.create({ partner_id: serverState.partnerId }),
+                Command.create({ partner_id: bobPartnerId }),
+            ],
+        },
+        {
+            channel_type: "chat",
+            channel_member_ids: [
+                Command.create({ partner_id: serverState.partnerId }),
+                Command.create({ partner_id: bobPartnerId }),
+            ],
+        },
     ]);
     mockService("presence", () => ({
         ...presenceService.start(),
@@ -1146,40 +1160,36 @@ test("receive new chat messages: out of odoo focus (tab title)", async () => {
             }
         },
     }));
-    await start();
+    const env = await start();
+    rpc = rpcWithEnv(env);
     await openDiscuss();
     await contains(".o-mail-DiscussSidebarChannel", { count: 2 });
-    const channel_1 = pyEnv["discuss.channel"].search_read([["id", "=", channelId_1]])[0];
     // simulate receiving a new message in chat 1 with odoo out-of-focused
-    pyEnv["bus.bus"]._sendone(channel_1, "discuss.channel/new_message", {
-        id: channelId_1,
-        message: {
-            id: 126,
-            model: "discuss.channel",
-            res_id: channelId_1,
-        },
-    });
+    await withUser(bobUserId, () =>
+        rpc("/mail/message/post", {
+            post_data: { body: "Hello world!", message_type: "comment" },
+            thread_id: channelId_1,
+            thread_model: "discuss.channel",
+        })
+    );
     await assertSteps(["set_title_part"]);
-    const channel_2 = pyEnv["discuss.channel"].search_read([["id", "=", channelId_2]])[0];
     // simulate receiving a new message in chat 2 with odoo out-of-focused
-    pyEnv["bus.bus"]._sendone(channel_2, "discuss.channel/new_message", {
-        id: channelId_2,
-        message: {
-            id: 127,
-            model: "discuss.channel",
-            res_id: channelId_2,
-        },
-    });
+    await withUser(bobUserId, () =>
+        rpc("/mail/message/post", {
+            post_data: { body: "Hello world!", message_type: "comment" },
+            thread_id: channelId_2,
+            thread_model: "discuss.channel",
+        })
+    );
     await assertSteps(["set_title_part"]);
     // simulate receiving another new message in chat 2 with odoo focused
-    pyEnv["bus.bus"]._sendone(channel_2, "discuss.channel/new_message", {
-        id: channelId_2,
-        message: {
-            id: 128,
-            model: "discuss.channel",
-            res_id: channelId_2,
-        },
-    });
+    await withUser(bobUserId, () =>
+        rpc("/mail/message/post", {
+            post_data: { body: "Hello world!", message_type: "comment" },
+            thread_id: channelId_2,
+            thread_model: "discuss.channel",
+        })
+    );
     await assertSteps(["set_title_part"]);
 });
 

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

@@ -374,9 +374,6 @@ patch(MockServer.prototype, {
                         message: Object.assign(messageFormat, { temporary_id }),
                     },
                 ]);
-                if (message.author_id === this.pyEnv.currentPartnerId) {
-                    this._mockDiscussChannel_ChannelSeen(ids, message.id);
-                }
             }
         }
         this.pyEnv["bus.bus"]._sendmany(notifications);

addons/mail/static/tests/mock_server/mock_models/discuss_channel.js

@@ -481,7 +481,7 @@ export class DiscussChannel extends models.ServerModel {
         const memberOfCurrentUser = this._find_or_create_member_for_self(channel.id);
         if (memberOfCurrentUser && memberOfCurrentUser.is_pinned !== pinned) {
             DiscussChannelMember.write([memberOfCurrentUser.id], {
-                unpin_dt: pinned ? false : serializeDateTime(today())
+                unpin_dt: pinned ? false : serializeDateTime(today()),
             });
         }
         const [partner] = ResPartner.read(this.env.user.partner_id);
@@ -786,7 +786,7 @@ export class DiscussChannel extends models.ServerModel {
         });
         const messageData = MailThread.message_post.call(this, [id], kwargs);
         if (kwargs.author_id === this.env.user?.partner_id) {
-            this._set_last_seen_message([channel.id], messageData.id);
+            this._set_last_seen_message([channel.id], messageData.id, false);
         }
         // simulate compute of message_unread_counter
         const memberOfCurrentUser = this._find_or_create_member_for_self(channel.id);
@@ -974,13 +974,14 @@ export class DiscussChannel extends models.ServerModel {
     /**
      * @param {number[]} ids
      * @param {number} message_id
+     * @param {boolean} [notify=true]
      */
-    _set_last_seen_message(ids, message_id) {
-        const kwargs = parseModelParams(arguments, "ids", "message_id");
+    _set_last_seen_message(ids, message_id, notify) {
+        const kwargs = parseModelParams(arguments, "ids", "message_id", "notify");
         ids = kwargs.ids;
         delete kwargs.ids;
         message_id = kwargs.message_id;
-
+        notify = kwargs.notify ?? true;
         /** @type {import("mock_models").BusBus} */
         const BusBus = this.env["bus.bus"];
         /** @type {import("mock_models").DiscussChannelMember} */
@@ -995,18 +996,20 @@ export class DiscussChannel extends models.ServerModel {
                 seen_message_id: message_id,
             });
         }
-        const [channel] = this.search_read([["id", "in", ids]]);
-        const [partner, guest] = ResPartner._get_current_persona();
-        let target = guest ?? partner;
-        if (this._types_allowing_seen_infos().includes(channel.channel_type)) {
-            target = channel;
+        if (notify) {
+            const [channel] = this.search_read([["id", "in", ids]]);
+            const [partner, guest] = ResPartner._get_current_persona();
+            let target = guest ?? partner;
+            if (this._types_allowing_seen_infos().includes(channel.channel_type)) {
+                target = channel;
+            }
+            BusBus._sendone(target, "discuss.channel.member/seen", {
+                channel_id: channel.id,
+                id: memberOfCurrentUser?.id,
+                last_message_id: message_id,
+                [guest ? "guest_id" : "partner_id"]: guest?.id ?? partner?.id,
+            });
         }
-        BusBus._sendone(target, "discuss.channel.member/seen", {
-            channel_id: channel.id,
-            id: memberOfCurrentUser?.id,
-            last_message_id: message_id,
-            [guest ? "guest_id" : "partner_id"]: guest?.id ?? partner?.id,
-        });
     }
 
     _types_allowing_seen_infos() {

addons/mail/tests/discuss/test_rtc.py

@@ -99,7 +99,6 @@ def test_10_start_call_in_chat_should_invite_all_members_to_call(self):
         with self.assertBus(
             [
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update new session
-                (self.cr.dbname, 'discuss.channel', channel.id),  # channel_seen after posting join message
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post "started a live conference" (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id, "members"),  # update of pin state (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update of last interest (not asserted below)
@@ -180,7 +179,6 @@ def test_11_start_call_in_group_should_invite_all_members_to_call(self):
         with self.assertBus(
             [
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update new session
-                (self.cr.dbname, 'discuss.channel', channel.id),  # channel_seen after posting join message
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post "started a live conference" (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id, "members"),  # update of pin state (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update of last interest (not asserted below)
@@ -624,10 +622,8 @@ def test_30_add_members_while_in_call_should_invite_new_members_to_call(self):
         channel_member_test_guest = channel.sudo().channel_member_ids.filtered(lambda member: member.guest_id == test_guest)
         found_bus_notifs = self.assertBusNotifications(
             [
-                (self.cr.dbname, 'discuss.channel', channel.id),  # channel joined -- seen (not asserted below)
                 (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # channel joined  -- last_interest (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post -- new_message (not asserted below)
-                (self.cr.dbname, 'discuss.channel', channel.id),  # message_post -- seen (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id, "members"),  # update of pin state (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post -- last_interest (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # new members (not asserted below)