[FIX] mail: cleanup sub-channel code and tests

- `_to_store` should not be called manually - `Store` constructor already adds the record - code should not assume order of channels in store insert - tour is made more robust and easier to debug by adding extra asserts closes odoo#184310 Signed-off-by: Alexandre Kühn (aku) <[email protected]>
stenlyk · Oct 21, 2024 · b62bf44 · b62bf44
1 parent 21d6314
commit b62bf44
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 8 deletions.
diff --git a/addons/mail/controllers/discuss/channel.py b/addons/mail/controllers/discuss/channel.py
@@ -158,7 +158,7 @@ def discuss_channel_sub_channel_create(self, parent_channel_id, from_message_id=
             raise NotFound()
         sub_channel = channel._create_sub_channel(from_message_id, name)
         return {
-            "data": Store(sub_channel).add(sub_channel).get_result(),
+            "data": Store(sub_channel).get_result(),
             "sub_channel": Store.one_id(sub_channel),
         }
 

diff --git a/addons/mail/models/discuss/discuss_channel.py b/addons/mail/models/discuss/discuss_channel.py
@@ -889,7 +889,7 @@ def _to_store(self, store: Store):
                ORDER BY discuss_channel_member.id ASC
         """, {'channel_ids': tuple(self.ids), 'current_partner_id': current_partner.id or None, 'current_guest_id': current_guest.id or None})
         all_needed_members = self.env['discuss.channel.member'].browse([m['id'] for m in self.env.cr.dictfetchall()])
-        all_needed_members._to_store(Store())  # prefetch in batch
+        Store(all_needed_members)  # prefetch in batch
         members_by_channel = defaultdict(lambda: self.env['discuss.channel.member'])
         invited_members_by_channel = defaultdict(lambda: self.env['discuss.channel.member'])
         member_of_current_user_by_channel = defaultdict(lambda: self.env['discuss.channel.member'])

diff --git a/addons/mail/static/src/discuss/core/public_web/thread_model_patch.js b/addons/mail/static/src/discuss/core/public_web/thread_model_patch.js
@@ -115,18 +115,22 @@ patch(Thread.prototype, {
         }
         const limit = 30;
         const data = await rpc("/discuss/channel/sub_channel/fetch", {
-            before: this.lastSubChannelLoaded,
+            before: this.lastSubChannelLoaded?.id,
             limit,
             parent_channel_id: this.id,
             search_term: searchTerm,
         });
-        const { Thread: subChannels = [] } = this.store.insert(data, { html: true });
+        const { Thread: threads = [] } = this.store.insert(data, { html: true });
         if (searchTerm) {
             // Ignore holes in the sub-channel list that may arise when
             // searching for a specific term.
             return;
         }
-        this.lastSubChannelLoaded = subChannels.at(-1)?.id;
+        const subChannels = threads.filter((thread) => this.eq(thread.parent_channel_id));
+        this.lastSubChannelLoaded = subChannels.reduce(
+            (min, channel) => (!min || channel.id < min.id ? channel : min),
+            this.lastSubChannelLoaded
+        );
         if (subChannels.length < limit) {
             this.loadSubChannelsDone = true;
         }

diff --git a/addons/mail/static/tests/tours/discuss_sub_channel_search_tour.js b/addons/mail/static/tests/tours/discuss_sub_channel_search_tour.js
@@ -15,6 +15,7 @@ registry.category("web_tour.tours").add("test_discuss_sub_channel_search", {
                     await contains(".o-mail-SubChannelList-thread", {
                         text: `Sub Channel ${i}`,
                     });
+                    await contains(".o-mail-SubChannelList-thread", { count: 30 });
                 }
             },
         },
@@ -30,9 +31,7 @@ registry.category("web_tour.tours").add("test_discuss_sub_channel_search", {
         {
             trigger: ".o-mail-SubChannelList-thread:contains(Sub Channel 10)",
             async run() {
-                await contains(".o-mail-SubChannelList-thread", {
-                    count: 1,
-                });
+                await contains(".o-mail-SubChannelList-thread", { count: 1 });
             },
         },
         {
@@ -42,6 +41,7 @@ registry.category("web_tour.tours").add("test_discuss_sub_channel_search", {
         {
             trigger: ".o-mail-SubChannelList-thread:contains(Sub Channel 99)",
             async run() {
+                await contains(".o-mail-SubChannelList-thread", { count: 31 });
                 // Already fetched sub channels are shown in addition to the one
                 // that was fetched during the search.
                 for (let i = 99; i > 69; i--) {
@@ -57,6 +57,7 @@ registry.category("web_tour.tours").add("test_discuss_sub_channel_search", {
         {
             trigger: ".o-mail-SubChannelList-thread:contains(Sub Channel 40)",
             async run() {
+                await contains(".o-mail-SubChannelList-thread", { count: 61 });
                 for (let i = 99; i > 39; i--) {
                     await contains(".o-mail-SubChannelList-thread", {
                         text: `Sub Channel ${i}`,
@@ -68,6 +69,7 @@ registry.category("web_tour.tours").add("test_discuss_sub_channel_search", {
         {
             trigger: ".o-mail-SubChannelList-thread:contains(Sub Channel 11)",
             async run() {
+                await contains(".o-mail-SubChannelList-thread", { count: 90 });
                 for (let i = 99; i > 9; i--) {
                     await contains(".o-mail-SubChannelList-thread", {
                         text: `Sub Channel ${i}`,
@@ -79,6 +81,7 @@ registry.category("web_tour.tours").add("test_discuss_sub_channel_search", {
         {
             trigger: ".o-mail-SubChannelList-thread:contains(Sub Channel 0)",
             async run() {
+                await contains(".o-mail-SubChannelList-thread", { count: 100 });
                 for (let i = 99; i > 0; i--) {
                     await contains(".o-mail-SubChannelList-thread", {
                         text: `Sub Channel ${i}`,