[FIX] bus, mail: message notif should not increment counter twice

Before this commit, a new unread message could contribute to unread
counter of conversation twice.

Steps to reproduce:

1. Log in as Demo and start a conversation with Mitchell Admin
2. Log in as Admin: if this is the first message in the conversation,
   counter is correct and shows (1)
3. Log out as Admin
4. Send another message to Admin
5. Log in as Admin and notice that the counter is doubled
6. If the conversation was still open, counter is correct!

This happens because some notification are pending dispatch before
fetching messaging state initially. The counter at init_messaging
fetch takes into account the notification. However, when handling
the message notifcation, it's wrongly considered as a new
notification, thus incrementing the counter when it shouldn't.

This commit fixes the issue by fetching the last bus notification
when fetching init_messaging. Doing so allow to determine whether
a notification is newer or older than init_messaging state.
This helps deciding whether the counter should be incremented or not.

Task-3357960

closes odoo/odoo#124767

X-original-commit: 081ab772dece86f1b5a5d4b3f6227b4b311d9206
Signed-off-by: Stockbauer Matthieu (tsm) <tsm@odoo.com>

addons/bus/models/bus.py

@@ -103,6 +103,10 @@ class ImBus(models.Model):
             })
         return result
 
+    def _bus_last_id(self):
+        last = self.env['bus.bus'].search([], order='id desc', limit=1)
+        return last.id if last else 0
+
 
 #----------------------------------------------------------
 # Dispatcher

addons/bus/static/src/services/bus_service.js

@@ -64,6 +64,7 @@ export const busService = {
             const { type } = messageEv.data;
             let { data } = messageEv.data;
             if (type === "notification") {
+                data.forEach((d) => (d.message.id = d.id)); // put notification id in notif message
                 multiTab.setSharedValue("last_notification_id", data[data.length - 1].id);
                 data = data.map((notification) => notification.message);
                 for (const { type, payload } of data) {

addons/bus/static/tests/bus_tests.js

@@ -413,7 +413,7 @@ QUnit.module(
                 await env2.services["bus_service"].start();
                 await updateLastNotificationDeferred;
                 // Second bus service sends the last known notification id.
-                assert.verifySteps([`initialize_connection - 1`]);
+                assert.verifySteps([`initialize_connection - 2`]);
             }
         );
 

addons/bus/static/tests/helpers/mock_server.js

@@ -59,7 +59,7 @@ patch(MockServer.prototype, "bus", {
         const values = [];
         for (const notification of notifications) {
             const [type, payload] = notification.slice(1, notification.length);
-            values.push({ id: this.lastBusNotificationId++, message: { payload, type } });
+            values.push({ id: ++this.lastBusNotificationId, message: { payload, type } });
             if (this.debug) {
                 console.log("%c[bus]", "color: #c6e; font-weight: bold;", type, payload);
             }
@@ -70,7 +70,7 @@ patch(MockServer.prototype, "bus", {
      * Simulate the lost of the connection by simulating a closeEvent on
      * the worker websocket.
      *
-     * @param {number} clodeCode the code to close the connection with.
+     * @param {number} closeCode the code to close the connection with.
      */
     _simulateConnectionLost(closeCode) {
         this.websocketWorker.websocket.close(closeCode);

addons/mail/models/discuss/mail_guest.py

@@ -113,6 +113,7 @@ class MailGuest(models.Model):
             'current_user_id': False,
             'current_user_settings': False,
             'hasLinkPreviewFeature': self.env['mail.link.preview']._is_link_preview_enabled(),
+            'initBusId': self.env['bus.bus'].sudo()._bus_last_id(),
             'menu_id': False,
             'needaction_inbox_counter': False,
             'odoobot': {

addons/mail/models/res_users.py

@@ -267,6 +267,7 @@ class Users(models.Model):
             'current_user_id': self.id,
             'current_user_settings': self.env['res.users.settings']._find_or_create_for_user(self)._res_users_settings_format(),
             'hasLinkPreviewFeature': self.env['mail.link.preview']._is_link_preview_enabled(),
+            'initBusId': self.env['bus.bus'].sudo()._bus_last_id(),
             'internalUserGroupId': self.env.ref('base.group_user').id,
             'menu_id': self.env['ir.model.data']._xmlid_to_res_id('mail.menu_root_discuss'),
             'needaction_inbox_counter': self.partner_id._get_needaction_count(),

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

@@ -121,6 +121,7 @@ export class Messaging {
             this.insertCannedResponse(code);
         });
         this.store.hasLinkPreviewFeature = data.hasLinkPreviewFeature;
+        this.store.initBusId = data.initBusId;
         this.isReady.resolve();
         this.store.isMessagingReady = true;
     }
@@ -482,16 +483,22 @@ export class Messaging {
             if (message.isSelfAuthored) {
                 channel.seen_message_id = message.id;
             } else {
-                channel.message_unread_counter++;
+                if (notif.id > this.store.initBusId) {
+                    channel.message_unread_counter++;
+                }
                 if (message.isNeedaction) {
                     const inbox = this.store.discuss.inbox;
                     if (!inbox.messages.includes(message)) {
                         inbox.messages.push(message);
-                        inbox.counter++;
+                        if (notif.id > this.store.initBusId) {
+                            inbox.counter++;
+                        }
                     }
                     if (!channel.needactionMessages.includes(message)) {
                         channel.needactionMessages.push(message);
-                        channel.message_needaction_counter++;
+                        if (notif.id > this.store.initBusId) {
+                            channel.message_needaction_counter++;
+                        }
                     }
                 }
             }

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

@@ -54,6 +54,16 @@ export class Store {
      */
     guest = null;
 
+    /**
+     * The last id of bus notification at the time for fetch init_messaging.
+     * When receiving a notification:
+     * - if id greater than this value: the notification is newer than init_messaging state.
+     * - if same id or lower: the notification is older than init_messaging state.
+     * This is useful to determine whether we should increment or decrement a counter based
+     * on init_messaging state.
+     */
+    initBusId = 0;
+
     /**
      * Indicates whether the current user is using the application through the
      * public page.

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

@@ -34,6 +34,7 @@ patch(MockServer.prototype, "mail/models/res_users", {
             current_user_settings: this._mockResUsersSettings_ResUsersSettingsFormat(
                 userSettings.id
             ),
+            initBusId: this.lastBusNotificationId,
             menu_id: false, // not useful in QUnit tests
             needaction_inbox_counter: this._mockResPartner_GetNeedactionCount(user.partner_id),
             odoobot: this._mockResPartnerMailPartnerFormat(this.odoobotId).get(this.odoobotId),

addons/test_discuss_full/tests/test_performance.py

@@ -108,6 +108,7 @@ class TestDiscussFullPerformance(TransactionCase):
             The point of having a separate getter is to allow it to be overriden.
         """
         return {
+            'initBusId': self.env['bus.bus'].sudo()._bus_last_id(),
             'hasLinkPreviewFeature': True,
             'needaction_inbox_counter': 1,
             'starred_counter': 1,