From ec0ba10cea12492fae307bfad48768d6fef598e0 Mon Sep 17 00:00:00 2001 From: zel-odoo Date: Tue, 9 Apr 2024 17:11:22 +0200 Subject: [PATCH] [FIX] mail: read from db when send notif in write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- addons/mail/models/discuss/discuss_channel.py | 60 +++++++------- .../mock_server/models/discuss_channel.js | 78 +++++++++++-------- .../tests/discuss/test_discuss_channel.py | 5 +- 3 files changed, 76 insertions(+), 67 deletions(-) diff --git a/addons/mail/models/discuss/discuss_channel.py b/addons/mail/models/discuss/discuss_channel.py index 63c148b5fe1..484b0ba080f 100644 --- a/addons/mail/models/discuss/discuss_channel.py +++ b/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 diff --git a/addons/mail/static/tests/helpers/mock_server/models/discuss_channel.js b/addons/mail/static/tests/helpers/mock_server/models/discuss_channel.js index 2599a006281..4ae52c51d91 100644 --- a/addons/mail/static/tests/helpers/mock_server/models/discuss_channel.js +++ b/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; }); }, diff --git a/addons/mail/tests/discuss/test_discuss_channel.py b/addons/mail/tests/discuss/test_discuss_channel.py index df10bc156f7..e48bd1da5bb 100644 --- a/addons/mail/tests/discuss/test_discuss_channel.py +++ b/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, } }, }]