From 5c12a8b82e8f9dd2be7df06f9998345c2f394ab2 Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Wed, 7 Jun 2023 07:39:16 +0000 Subject: [PATCH] [FIX] mail: load after message order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In [1] the order by `id ASC` has been removed from the `_message_fetch` method. This is wrong since we want to load the messages coming DIRECTLY after the given id thus, this order is required. In order to solve this issue, this commit ensures the order of the messages returned by the `_message_fetch` method is always in descending order as it was done implicitly before [2]. [1]: https://github.com/odoo/odoo/pull/123664 [2]: https://github.com/odoo/odoo/pull/116666 task-3349175 closes odoo/odoo#124169 X-original-commit: 074a5fea5a7f08c7018620cb9b2b956b3a871344 Signed-off-by: Alexandre Kühn (aku) Signed-off-by: Didier Debondt (did) --- addons/mail/controllers/discuss/channel.py | 2 +- addons/mail/models/mail_message.py | 9 ++- .../src/js/tours/load_message_order_tour.js | 62 +++++++++++++++++++ addons/mail/tests/discuss/__init__.py | 1 + .../mail/tests/discuss/test_load_messages.py | 21 +++++++ 5 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 addons/mail/static/src/js/tours/load_message_order_tour.js create mode 100644 addons/mail/tests/discuss/test_load_messages.py diff --git a/addons/mail/controllers/discuss/channel.py b/addons/mail/controllers/discuss/channel.py index 814999ae98b..1a645f3fb39 100644 --- a/addons/mail/controllers/discuss/channel.py +++ b/addons/mail/controllers/discuss/channel.py @@ -58,7 +58,7 @@ class ChannelController(http.Controller): messages = channel_member_sudo.env["mail.message"]._message_fetch(domain, before, after, around, limit) if not request.env.user._is_public() and not around: messages.set_message_done() - return messages.sorted("id", reverse=True).message_format() + return messages.message_format() @http.route("/discuss/channel/pinned_messages", methods=["POST"], type="json", auth="public") def discuss_channel_pins(self, channel_id): diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 887423881b9..584c432a6cb 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -932,17 +932,20 @@ class Message(models.Model): :param around: fetch messages around this message id i.e. limit//2 before and limit//2 after. :param limit: the maximum amount of messages to get; - :returns: record set of mail.message + :returns: record set of mail.message sorted in descending order. """ if around: messages_before = self.search(domain=[*domain, ('id', '<=', around)], limit=limit // 2, order="id DESC") messages_after = self.search(domain=[*domain, ('id', '>', around)], limit=limit // 2, order='id ASC') - return messages_after + messages_before + return (messages_after + messages_before).sorted('id', reverse=True) if before: domain = expression.AND([domain, [('id', '<', before)]]) if after: domain = expression.AND([domain, [('id', '>', after)]]) - return self.search(domain, limit=limit) + message_ids = self.search(domain, limit=limit, order='id ASC' if after else 'id DESC') + if after: + message_ids = message_ids.sorted('id', reverse=True) + return message_ids def message_format(self, format_reply=True, msg_vals=None): """ Get the message values in the format for web client. Since message diff --git a/addons/mail/static/src/js/tours/load_message_order_tour.js b/addons/mail/static/src/js/tours/load_message_order_tour.js new file mode 100644 index 00000000000..29deb4f11c5 --- /dev/null +++ b/addons/mail/static/src/js/tours/load_message_order_tour.js @@ -0,0 +1,62 @@ +/* @odoo-module */ + +import { registry } from "@web/core/registry"; + +registry.category("web_tour.tours").add("mail.load_message_order", { + test: true, + steps: [ + { + trigger: ".o-mail-DiscussCategoryItem:contains(MyTestChannel)", + run: "click", + }, + { + trigger: "*[title='Pinned Messages']", + run: "click", + }, + { + trigger: ".o-discuss-PinnedMessagesPanel-jump", + run: "click", + }, + { + // Messages depends on FETCH_LIMIT (currently set to 30) in + // the thread service. Thus, at first load the message range + // will be (31 - 60). This trigger ensures the next messages + // are fetched after jumping to the message. + trigger: ".o-mail-Thread .o-mail-Message:first:not(:contains(31))", + async run() { + // ensure 1 - 16 are loaded in order: 15 below and the + // one we're loading messages around. + const messages = Array.from( + document.querySelectorAll(".o-mail-Thread .o-mail-Message-content") + ).map((el) => el.innerText); + for (let i = 0; i < 16; i++) { + if (messages[i] !== (i + 1).toString()) { + throw new Error("Wrong message order after loading around"); + } + } + const thread = document.querySelector(".o-mail-Thread"); + thread.scrollTop = thread.scrollHeight - thread.clientHeight; + }, + }, + { + // After jumping to the pinned message, the message range + // was (1 -16): 15 before (but none were found), 15 after + // and the pinned message itself. This trigger ensures the + // next messages are fetched after scrolling to the bottom. + trigger: ".o-mail-Thread .o-mail-Message:contains(17)", + run() { + // ensure 1 - 46 are loaded in order. + const messages = Array.from( + document.querySelectorAll(".o-mail-Thread .o-mail-Message-content") + ).map((el) => el.innerText); + for (let i = 0; i < 46; i++) { + if (messages[i] !== (i + 1).toString()) { + throw new Error("Wrong message order after loading after"); + } + } + const thread = document.querySelector(".o-mail-Thread"); + thread.scrollTop = thread.scrollHeight - thread.clientHeight; + }, + }, + ], +}); diff --git a/addons/mail/tests/discuss/__init__.py b/addons/mail/tests/discuss/__init__.py index 869faaf2e50..a12db1a96a8 100644 --- a/addons/mail/tests/discuss/__init__.py +++ b/addons/mail/tests/discuss/__init__.py @@ -3,4 +3,5 @@ from . import test_discuss_channel from . import test_discuss_channel_as_guest from . import test_discuss_channel_member +from . import test_load_messages from . import test_rtc diff --git a/addons/mail/tests/discuss/test_load_messages.py b/addons/mail/tests/discuss/test_load_messages.py new file mode 100644 index 00000000000..29fffc2f239 --- /dev/null +++ b/addons/mail/tests/discuss/test_load_messages.py @@ -0,0 +1,21 @@ + +import odoo.tests +from odoo import Command + + +@odoo.tests.tagged('post_install', '-at_install') +class TestLoadMessages(odoo.tests.HttpCase): + def test_01_load_message_order(self): + partner_admin = self.env.ref('base.partner_admin') + channel_id = self.env["discuss.channel"].create({ + "name": "MyTestChannel", + "channel_member_ids": [Command.create({"partner_id": partner_admin.id})], + }) + self.env["mail.message"].create([{ + "body": n, + "model": "discuss.channel", + "pinned_at": odoo.fields.Datetime.now() if n == 1 else None, + "res_id": channel_id.id, + "author_id": partner_admin.id, + } for n in range(1, 61)]) + self.start_tour("/web#action=mail.action_discuss", "mail.load_message_order", login="admin")