[FIX] mail: read from db when send notif in write

When writing in a discuss channel, the updated value sent to the client
should be read from the database, not directly from the values passed to
the write method.

Partially backport of https://github.com/odoo/odoo/pull/139563

closes odoo/odoo#158860

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

addons/mail/models/discuss/discuss_channel.py

@@ -271,33 +271,25 @@ class Channel(models.Model):
             failing_channels = self.filtered(lambda channel: channel.channel_type != vals.get('channel_type'))
             if failing_channels:
                 raise UserError(_('Cannot change the channel type of: %(channel_names)s', channel_names=', '.join(failing_channels.mapped('name'))))
+        old_vals = {channel: channel._channel_basic_info() for channel in self}
+        result = super().write(vals)
         notifications = []
         for channel in self:
-            current_val = channel.read(vals.keys())[0]
+            info = channel._channel_basic_info()
             diff = {}
-            for key in vals.keys():
-                if current_val.get(key) != vals.get(key) and key != "image_128":
-                    diff[key] = vals[key]
+            for key, value in info.items():
+                if value != old_vals[channel][key]:
+                    diff[key] = value
             if diff:
                 notifications.append([channel, "mail.record/insert", {
                     "Thread": {
-                        "id": current_val["id"],
+                        "id": channel.id,
                         "model": "discuss.channel",
                         **diff
                     }
                 }])
-        result = super().write(vals)
         if vals.get('group_ids'):
             self._subscribe_users_automatically()
-        if 'image_128' in vals:
-            for channel in self:
-                notifications.append([channel, 'mail.record/insert', {
-                    'Thread': {
-                        'avatarCacheKey': channel._get_avatar_cache_key(),
-                        'id': channel.id,
-                        'model': "discuss.channel",
-                    }
-                }])
         self.env['bus.bus']._sendmany(notifications)
         return result
 
@@ -777,6 +769,24 @@ class Channel(models.Model):
             self.add_members(guest_ids=guest.ids, post_joined_message=post_joined_message)
         return self.env.user.partner_id if not guest else self.env["res.partner"], guest
 
+    def _channel_basic_info(self):
+        self.ensure_one()
+        return {
+            'avatarCacheKey': self._get_avatar_cache_key(),
+            'channel_type': self.channel_type,
+            'memberCount': self.member_count,
+            'id': self.id,
+            'name': self.name,
+            'defaultDisplayMode': self.default_display_mode,
+            'description': self.description,
+            'uuid': self.uuid,
+            'group_based_subscription': bool(self.group_ids),
+            'create_uid': self.create_uid.id,
+            'authorizedGroupFullName': self.group_public_id.full_name,
+            'allow_public_upload': self.allow_public_upload,
+            'model': "discuss.channel",
+        }
+
     def _channel_info(self):
         """ Get the informations header for the current channels
             :returns a list of channels values
@@ -820,24 +830,8 @@ class Channel(models.Model):
             if (current_partner and member.partner_id == current_partner) or (current_guest and member.guest_id == current_guest):
                 member_of_current_user_by_channel[member.channel_id] = member
         for channel in self:
-            info = {
-                'avatarCacheKey': channel._get_avatar_cache_key(),
-                'channel_type': channel.channel_type,
-                'memberCount': channel.member_count,
-                'id': channel.id,
-                'name': channel.name,
-                'defaultDisplayMode': channel.default_display_mode,
-                'description': channel.description,
-                'uuid': channel.uuid,
-                'state': 'open',
-                'is_editable': channel.is_editable,
-                'is_minimized': False,
-                'group_based_subscription': bool(channel.group_ids),
-                'create_uid': channel.create_uid.id,
-                'authorizedGroupFullName': channel.group_public_id.full_name,
-                'allow_public_upload': channel.allow_public_upload,
-                'model': "discuss.channel",
-            }
+            info = channel._channel_basic_info()
+            info["is_editable"] = channel.is_editable
             # find the channel member state
             if current_partner or current_guest:
                 info['message_needaction_counter'] = channel.message_needaction_counter

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

@@ -14,16 +14,21 @@ patch(MockServer.prototype, {
      */
     mockWrite(model, args) {
         const notifications = [];
+        const old_info = {};
+        if (model == "discuss.channel") {
+            Object.assign(old_info, this._mockDiscussChannelBasicInfo(args[0][0]));
+        }
+        const mockWriteResult = super.mockWrite(...arguments);
         if (model == "discuss.channel") {
-            const vals = args[1];
             const [channel] = this.getRecords(model, [["id", "=", args[0][0]]]);
-            if (channel) {
-                const diff = {};
-                for (const key in vals) {
-                    if (channel[key] != vals[key] && key !== "image_128") {
-                        diff[key] = vals[key];
-                    }
+            const info = this._mockDiscussChannelBasicInfo(channel.id);
+            const diff = {};
+            for (const key of Object.keys(info)) {
+                if (info[key] !== old_info[key]) {
+                    diff[key] = info[key];
                 }
+            }
+            if (Object.keys(diff).length) {
                 notifications.push([
                     channel,
                     "mail.record/insert",
@@ -37,7 +42,6 @@ patch(MockServer.prototype, {
                 ]);
             }
         }
-        const mockWriteResult = super.mockWrite(...arguments);
         if (notifications.length) {
             this.pyEnv["bus.bus"]._sendmany(notifications);
         }
@@ -595,6 +599,39 @@ patch(MockServer.prototype, {
         );
         return this._mockDiscussChannelChannelInfo([id])[0];
     },
+    /**
+     * Simulates `_channel_basic_info` on `discuss.channel`.
+     *
+     * @private
+     * @param {integer} id
+     * @returns {Object[]}
+     */
+    _mockDiscussChannelBasicInfo(id) {
+        const [channel] = this.getRecords("discuss.channel", [["id", "=", id]]);
+        const [group_public_id] = this.getRecords("res.groups", [
+            ["id", "=", channel.group_public_id],
+        ]);
+        const res = assignDefined({}, channel, [
+            "allow_public_upload",
+            "avatarCacheKey",
+            "channel_type",
+            "create_uid",
+            "defaultDisplayMode",
+            "description",
+            "group_based_subscription",
+            "id",
+            "name",
+            "uuid",
+        ]);
+        Object.assign(res, {
+            memberCount: this.pyEnv["discuss.channel.member"].searchCount([
+                ["channel_id", "=", channel.id],
+            ]),
+            authorizedGroupFullName: group_public_id ? group_public_id.name : false,
+            model: "discuss.channel",
+        });
+        return res;
+    },
     /**
      * Simulates `channel_info` on `discuss.channel`.
      *
@@ -608,37 +645,17 @@ patch(MockServer.prototype, {
             const members = this.getRecords("discuss.channel.member", [
                 ["id", "in", channel.channel_member_ids],
             ]);
+            const res = this._mockDiscussChannelBasicInfo(channel.id);
             const messages = this.getRecords("mail.message", [
                 ["model", "=", "discuss.channel"],
                 ["res_id", "=", channel.id],
             ]);
-            const [group_public_id] = this.getRecords("res.groups", [
-                ["id", "=", channel.group_public_id],
-            ]);
             const messageNeedactionCounter = this.getRecords("mail.notification", [
                 ["res_partner_id", "=", this.pyEnv.currentPartnerId],
                 ["is_read", "=", false],
                 ["mail_message_id", "in", messages.map((message) => message.id)],
             ]).length;
-            const res = assignDefined({}, channel, [
-                "id",
-                "name",
-                "defaultDisplayMode",
-                "description",
-                "uuid",
-                "create_uid",
-                "group_based_subscription",
-                "avatarCacheKey",
-            ]);
-            Object.assign(res, {
-                channel_type: channel.channel_type,
-                memberCount: this.pyEnv["discuss.channel.member"].searchCount([
-                    ["channel_id", "=", channel.id],
-                ]),
-                message_needaction_counter: messageNeedactionCounter,
-                authorizedGroupFullName: group_public_id ? group_public_id.name : false,
-                model: "discuss.channel",
-            });
+            res.message_needaction_counter = messageNeedactionCounter;
             const memberOfCurrentUser = this._mockDiscussChannelMember__getAsSudoFromContext(
                 channel.id
             );
@@ -714,7 +731,6 @@ patch(MockServer.prototype, {
                     ),
                 ],
             ];
-            res.allow_public_upload = channel.allow_public_upload;
             return res;
         });
     },

addons/mail/tests/discuss/test_discuss_channel.py

@@ -348,7 +348,7 @@ class TestChannelInternals(MailCommon):
 
     def test_channel_write_should_send_notification(self):
         channel = self.env['discuss.channel'].create({"name": "test", "description": "test"})
-        # do the operation once before the assert to grab the value to expect
+        self.env['bus.bus'].search([]).unlink()
         with self.assertBus(
             [(self.cr.dbname, 'discuss.channel', channel.id)],
             [{
@@ -363,7 +363,6 @@ class TestChannelInternals(MailCommon):
             }]
         ):
             channel.name = "test test"
-            channel.description = "test"
 
     def test_channel_write_should_send_notification_if_image_128_changed(self):
         channel = self.env['discuss.channel'].create({'name': '', 'uuid': 'test-uuid'})
@@ -378,9 +377,9 @@ class TestChannelInternals(MailCommon):
                 "type": "mail.record/insert",
                 "payload": {
                     'Thread': {
-                        "avatarCacheKey": avatar_cache_key,
                         "id": channel.id,
                         'model': "discuss.channel",
+                        "avatarCacheKey": avatar_cache_key,
                     }
                 },
             }]