[FIX] mail: do not return members of channels in channel_info

This PR has 3 parts:

The members fix itself, which is necessary for performance reasons especially
at init messaging, as per described in the task.

Adding the partner name in the typing payload, because due to not knowing all
members anymore, it is necessary to properly display who is typing.

Restrict restoring scroll position only if the corresponding scroll height is
available, to make the scroll feature and particularly the corresponding tests
more robust.

task-2360020

closes odoo/odoo#60091

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>

10 files changed, 214 insertions, 34 deletions

diff --git a/addons/mail/static/src/components/chat_window/chat_window.js b/addons/mail/static/src/components/chat_window/chat_window.js
index 4480be9db11..46e60bbea54 100644
--- a/addons/mail/static/src/components/chat_window/chat_window.js
+++ b/addons/mail/static/src/components/chat_window/chat_window.js
@@ -135,6 +135,9 @@ class ChatWindow extends Component {
         if (!this._threadRef.comp || !this.chatWindow.threadViewer) {
             return;
         }
+        this.chatWindow.threadViewer.saveThreadCacheScrollHeightAsInitial(
+            this._threadRef.comp.getScrollHeight()
+        );
         this.chatWindow.threadViewer.saveThreadCacheScrollPositionsAsInitial(
             this._threadRef.comp.getScrollTop()
         );
diff --git a/addons/mail/static/src/components/chat_window_manager/chat_window_manager_tests.js b/addons/mail/static/src/components/chat_window_manager/chat_window_manager_tests.js
index bf9db6c0276..b5abc5b2236 100644
--- a/addons/mail/static/src/components/chat_window_manager/chat_window_manager_tests.js
+++ b/addons/mail/static/src/components/chat_window_manager/chat_window_manager_tests.js
@@ -23,12 +23,13 @@ QUnit.module('chat_window_manager_tests.js', {
         beforeEach(this);
 
         this.start = async params => {
-            const { env, widget } = await start(Object.assign(
+            const { afterEvent, env, widget } = await start(Object.assign(
                 { hasChatWindow: true, hasMessagingMenu: true },
                 params,
                 { data: this.data }
             ));
             this.debug = params && params.debug;
+            this.afterEvent = afterEvent;
             this.env = env;
             this.widget = widget;
         };
@@ -812,7 +813,20 @@ QUnit.test('[technical] chat window: scroll conservation on toggle home menu', a
         document.querySelector(`.o_MessagingMenu_dropdownMenu .o_NotificationList_preview`).click()
     );
     // Set a scroll position to chat window
-    document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
+    await this.afterEvent({
+        eventName: 'o-component-message-list-scrolled',
+        func: () => {
+            document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
+        },
+        message: "should wait until channel 20 scrolled to 142 after setting this value manually",
+        predicate: ({ scrollTop, threadViewer }) => {
+            return (
+                threadViewer.thread.model === 'mail.channel' &&
+                threadViewer.thread.id === 20 &&
+                scrollTop === 142
+            );
+        },
+    });
     assert.strictEqual(
         document.querySelector(`.o_ThreadView_messageList`).scrollTop,
         142,
@@ -826,7 +840,18 @@ QUnit.test('[technical] chat window: scroll conservation on toggle home menu', a
         "chat window scrollTop should still be the same after home menu is hidden"
     );
 
-    await afterNextRender(() => this.showHomeMenu());
+    await this.afterEvent({
+        eventName: 'o-component-message-list-scrolled',
+        func: () => this.showHomeMenu(),
+        message: "should wait until channel 20 restored its scroll to 142 after hiding the home menu",
+        predicate: ({ scrollTop, threadViewer }) => {
+            return (
+                threadViewer.thread.model === 'mail.channel' &&
+                threadViewer.thread.id === 20 &&
+                scrollTop === 142
+            );
+        },
+    });
     assert.strictEqual(
         document.querySelector(`.o_ThreadView_messageList`).scrollTop,
         142,
@@ -1552,7 +1577,20 @@ QUnit.test('chat window with a thread: keep scroll position in message list on f
         document.querySelector(`.o_NotificationList_preview`).click()
     );
     // Set a scroll position to chat window
-    document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
+    await this.afterEvent({
+        eventName: 'o-component-message-list-scrolled',
+        func: () => {
+            document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
+        },
+        message: "should wait until channel 20 scrolled to 142 after setting this value manually",
+        predicate: ({ scrollTop, threadViewer }) => {
+            return (
+                threadViewer.thread.model === 'mail.channel' &&
+                threadViewer.thread.id === 20 &&
+                scrollTop === 142
+            );
+        },
+    });
     assert.strictEqual(
         document.querySelector(`.o_ThreadView_messageList`).scrollTop,
         142,
@@ -1568,7 +1606,19 @@ QUnit.test('chat window with a thread: keep scroll position in message list on f
     );
 
     // unfold chat window
-    await afterNextRender(() => document.querySelector('.o_ChatWindow_header').click());
+    await afterNextRender(() => this.afterEvent({
+        eventName: 'o-component-message-list-scrolled',
+        func: () => document.querySelector('.o_ChatWindow_header').click(),
+        message: "should wait until channel 20 restored its scroll position to 142",
+        predicate: ({ scrollTop, threadViewer }) => {
+            return (
+                threadViewer.thread.model === 'mail.channel' &&
+                threadViewer.thread.id === 20 &&
+                scrollTop === 142
+
+            );
+        },
+    }));
     assert.strictEqual(
         document.querySelector(`.o_ThreadView_messageList`).scrollTop,
         142,
diff --git a/addons/mail/static/src/components/message_list/message_list.js b/addons/mail/static/src/components/message_list/message_list.js
index 708af154cb8..d24e1ccf6fc 100644
--- a/addons/mail/static/src/components/message_list/message_list.js
+++ b/addons/mail/static/src/components/message_list/message_list.js
@@ -162,6 +162,13 @@ class MessageList extends Component {
     /**
      * @returns {integer}
      */
+    getScrollHeight() {
+        return this.el.scrollHeight;
+    }
+
+    /**
+     * @returns {integer}
+     */
     getScrollTop() {
         return this.el.scrollTop;
     }
@@ -323,9 +330,13 @@ class MessageList extends Component {
         if (threadCache.messages.length > 0) {
             if (this.threadView.threadCacheInitialScrollPosition !== undefined) {
                 if (this.props.hasScrollAdjust) {
-                    this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
+                    if (this.el.scrollHeight === this.threadView.threadCacheInitialScrollHeight) {
+                        this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
+                        isProcessed = true;
+                    }
+                } else {
+                    isProcessed = true;
                 }
-                isProcessed = true;
             } else {
                 const lastMessage = threadCache.lastMessage;
                 if (this.messageRefFromId(lastMessage.id)) {
@@ -348,8 +359,9 @@ class MessageList extends Component {
      * @param {Object} hint
      */
     _adjustFromChatWindowUnfolded(hint) {
-        this._adjustScrollFromModel();
-        this.threadView.markComponentHintProcessed(hint);
+        if (this._adjustScrollFromModel()) {
+            this.threadView.markComponentHintProcessed(hint);
+        }
     }
 
     /**
@@ -357,8 +369,9 @@ class MessageList extends Component {
      * @param {Object} hint
      */
     _adjustFromHomeMenuHidden(hint) {
-        this._adjustScrollFromModel();
-        this.threadView.markComponentHintProcessed(hint);
+        if (this._adjustScrollFromModel()) {
+            this.threadView.markComponentHintProcessed(hint);
+        }
     }
 
     /**
@@ -366,8 +379,9 @@ class MessageList extends Component {
      * @param {Object} hint
      */
     _adjustFromHomeMenuShown(hint) {
-        this._adjustScrollFromModel();
-        this.threadView.markComponentHintProcessed(hint);
+        if (this._adjustScrollFromModel()) {
+            this.threadView.markComponentHintProcessed(hint);
+        }
     }
 
     /**
@@ -423,14 +437,21 @@ class MessageList extends Component {
 
     /**
      * @private
+     * @returns {boolean} whether the adjustment should be considered processed
      */
     _adjustScrollFromModel() {
         if (
             this.threadView.threadCacheInitialScrollPosition !== undefined &&
             this.props.hasScrollAdjust
         ) {
-            this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
+            if (this.el.scrollHeight === this.threadView.threadCacheInitialScrollHeight) {
+                this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
+                return true;
+            } else {
+                return false;
+            }
         }
+        return true;
     }
 
     /**
@@ -560,6 +581,7 @@ class MessageList extends Component {
             ? scrollTop >= this.el.scrollHeight - this.el.clientHeight - margin
             : scrollTop <= margin;
         this.threadView.update({ hasAutoScrollOnMessageReceived });
+        this.threadView.threadViewer.saveThreadCacheScrollHeightAsInitial(this.el.scrollHeight);
         this.threadView.threadViewer.saveThreadCacheScrollPositionsAsInitial(scrollTop);
         if (!this._isAutoLoadOnScrollActive) {
             return;
diff --git a/addons/mail/static/src/components/thread_icon/thread_icon_tests.js b/addons/mail/static/src/components/thread_icon/thread_icon_tests.js
index 3e4a44607e5..21966cc206f 100644
--- a/addons/mail/static/src/components/thread_icon/thread_icon_tests.js
+++ b/addons/mail/static/src/components/thread_icon/thread_icon_tests.js
@@ -75,8 +75,9 @@ QUnit.test('chat: correspondent is typing', async function (assert) {
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: true,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -96,8 +97,9 @@ QUnit.test('chat: correspondent is typing', async function (assert) {
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: false,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
diff --git a/addons/mail/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js b/addons/mail/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js
index 885bbcbacfc..4c525d193fa 100644
--- a/addons/mail/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js
+++ b/addons/mail/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js
@@ -65,8 +65,9 @@ QUnit.test('receive other member typing status "is typing"', async function (ass
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: true,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -103,8 +104,9 @@ QUnit.test('receive other member typing status "is typing" then "no longer is ty
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: true,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -119,8 +121,9 @@ QUnit.test('receive other member typing status "is typing" then "no longer is ty
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: false,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -159,8 +162,9 @@ QUnit.test('assume other member typing status becomes "no longer is typing" afte
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: true,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -206,8 +210,9 @@ QUnit.test ('other member typing status "is typing" refreshes 60 seconds timer o
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 17,
             is_typing: true,
+            partner_id: 17,
+            partner_name: "Demo",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -222,8 +227,9 @@ QUnit.test ('other member typing status "is typing" refreshes 60 seconds timer o
     await this.env.testUtils.advanceTime(50 * 1000);
     const typingData = {
         info: 'typing_status',
-        partner_id: 17,
         is_typing: true,
+        partner_id: 17,
+        partner_name: "Demo",
     };
     const notification = [[false, 'mail.channel', 20], typingData];
     this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -272,8 +278,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 10,
             is_typing: true,
+            partner_id: 10,
+            partner_name: "Other10",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -288,8 +295,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 11,
             is_typing: true,
+            partner_id: 11,
+            partner_name: "Other11",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -304,8 +312,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 12,
             is_typing: true,
+            partner_id: 12,
+            partner_name: "Other12",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -320,8 +329,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 10,
             is_typing: false,
+            partner_id: 10,
+            partner_name: "Other10",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -336,8 +346,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
     await afterNextRender(() => {
         const typingData = {
             info: 'typing_status',
-            partner_id: 10,
             is_typing: true,
+            partner_id: 10,
+            partner_name: "Other10",
         };
         const notification = [[false, 'mail.channel', 20], typingData];
         this.widget.call('bus_service', 'trigger', 'notification', [notification]);
diff --git a/addons/mail/static/src/components/thread_view/thread_view.js b/addons/mail/static/src/components/thread_view/thread_view.js
index 395fac68474..42e513be888 100644
--- a/addons/mail/static/src/components/thread_view/thread_view.js
+++ b/addons/mail/static/src/components/thread_view/thread_view.js
@@ -62,6 +62,18 @@ class ThreadView extends Component {
     }
 
     /**
+     * Get the scroll height in the message list.
+     *
+     * @returns {integer|undefined}
+     */
+    getScrollHeight() {
+        if (!this._messageListRef.comp) {
+            return undefined;
+        }
+        return this._messageListRef.comp.getScrollHeight();
+    }
+
+    /**
      * Get the scroll position in the message list.
      *
      * @returns {integer|undefined}
diff --git a/addons/mail/static/src/models/messaging_initializer/messaging_initializer.js b/addons/mail/static/src/models/messaging_initializer/messaging_initializer.js
index 9fc0c052448..f0ed367d092 100644
--- a/addons/mail/static/src/models/messaging_initializer/messaging_initializer.js
+++ b/addons/mail/static/src/models/messaging_initializer/messaging_initializer.js
@@ -153,9 +153,17 @@ function factory(dependencies) {
                 // there might be a lot of channels, insert each of them one by
                 // one asynchronously to avoid blocking the UI
                 await this.async(() => new Promise(resolve => setTimeout(resolve)));
-                const channel = this.env.models['mail.thread'].insert(
-                    this.env.models['mail.thread'].convertData(channelData)
-                );
+                const convertedData = this.env.models['mail.thread'].convertData(channelData);
+                if (!convertedData.members) {
+                    // channel_info does not return all members of channel for
+                    // performance reasons, but code is expecting to know at
+                    // least if the current partner is member of it.
+                    // (e.g. to know when to display "invited" notification)
+                    // Current partner can always be assumed to be a member of
+                    // channels received at init.
+                    convertedData.members = [['link', this.env.messaging.currentPartner]];
+                }
+                const channel = this.env.models['mail.thread'].insert(convertedData);
                 // flux specific: channels received at init have to be
                 // considered pinned. task-2284357
                 if (!channel.isPinned) {
diff --git a/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js b/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
index 4b3baded714..783b43e4f20 100644
--- a/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
+++ b/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
@@ -108,6 +108,7 @@ function factory(dependencies) {
                 is_typing,
                 last_message_id,
                 partner_id,
+                partner_name,
             } = data;
             switch (info) {
                 case 'channel_fetched':
@@ -124,6 +125,7 @@ function factory(dependencies) {
                     return this._handleNotificationChannelTypingStatus(channelId, {
                         is_typing,
                         partner_id,
+                        partner_name,
                     });
                 default:
                     return this._handleNotificationChannelMessage(channelId, data);
@@ -344,8 +346,9 @@ function factory(dependencies) {
          * @param {Object} param1
          * @param {boolean} param1.is_typing
          * @param {integer} param1.partner_id
+         * @param {string} param1.partner_name
          */
-        _handleNotificationChannelTypingStatus(channelId, { is_typing, partner_id }) {
+        _handleNotificationChannelTypingStatus(channelId, { is_typing, partner_id, partner_name }) {
             const channel = this.env.models['mail.thread'].findFromIdentifyingData({
                 id: channelId,
                 model: 'mail.channel',
@@ -353,7 +356,10 @@ function factory(dependencies) {
             if (!channel) {
                 return;
             }
-            const partner = this.env.models['mail.partner'].insert({ id: partner_id });
+            const partner = this.env.models['mail.partner'].insert({
+                id: partner_id,
+                name: partner_name,
+            });
             if (partner === this.env.messaging.currentPartner) {
                 // Ignore management of current partner is typing notification.
                 return;
@@ -469,7 +475,15 @@ function factory(dependencies) {
             const convertedData = this.env.models['mail.thread'].convertData(
                 Object.assign({ model: 'mail.channel' }, data)
             );
-
+            if (!convertedData.members) {
+                // channel_info does not return all members of channel for
+                // performance reasons, but code is expecting to know at
+                // least if the current partner is member of it.
+                // (e.g. to know when to display "invited" notification)
+                // Current partner can always be assumed to be a member of
+                // channels received through this notification.
+                convertedData.members = [['link', this.env.messaging.currentPartner]];
+            }
             let channel = this.env.models['mail.thread'].findFromIdentifyingData(convertedData);
             const wasCurrentPartnerMember = (
                 channel &&
diff --git a/addons/mail/static/src/models/thread_view/thread_view.js b/addons/mail/static/src/models/thread_view/thread_view.js
index 0c6cdf6d458..f381f3d86f9 100644
--- a/addons/mail/static/src/models/thread_view/thread_view.js
+++ b/addons/mail/static/src/models/thread_view/thread_view.js
@@ -80,6 +80,21 @@ function factory(dependencies) {
          * @private
          * @returns {integer|undefined}
          */
+        _computeThreadCacheInitialScrollHeight() {
+            if (!this.threadCache) {
+                return clear();
+            }
+            const threadCacheInitialScrollHeight = this.threadCacheInitialScrollHeights[this.threadCache.localId];
+            if (threadCacheInitialScrollHeight !== undefined) {
+                return threadCacheInitialScrollHeight;
+            }
+            return clear();
+        }
+
+        /**
+         * @private
+         * @returns {integer|undefined}
+         */
         _computeThreadCacheInitialScrollPosition() {
             if (!this.threadCache) {
                 return clear();
@@ -264,6 +279,13 @@ function factory(dependencies) {
             inverse: 'threadViews',
             related: 'threadViewer.threadCache',
         }),
+        threadCacheInitialScrollHeight: attr({
+            compute: '_computeThreadCacheInitialScrollHeight',
+            dependencies: [
+                'threadCache',
+                'threadCacheInitialScrollHeights',
+            ],
+        }),
         threadCacheInitialScrollPosition: attr({
             compute: '_computeThreadCacheInitialScrollPosition',
             dependencies: [
@@ -278,6 +300,13 @@ function factory(dependencies) {
             related: 'threadCache.isLoading',
         }),
         /**
+         * List of saved initial scroll heights of thread caches.
+         */
+        threadCacheInitialScrollHeights: attr({
+            default: {},
+            related: 'threadViewer.threadCacheInitialScrollHeights',
+        }),
+        /**
          * List of saved initial scroll positions of thread caches.
          */
         threadCacheInitialScrollPositions: attr({
diff --git a/addons/mail/static/src/models/thread_view/thread_viewer.js b/addons/mail/static/src/models/thread_view/thread_viewer.js
index 3c643339eff..1e48e4ad805 100644
--- a/addons/mail/static/src/models/thread_view/thread_viewer.js
+++ b/addons/mail/static/src/models/thread_view/thread_viewer.js
@@ -13,7 +13,27 @@ function factory(dependencies) {
         //----------------------------------------------------------------------
 
         /**
-         * @param {string} scrollTop
+         * @param {integer} scrollHeight
+         */
+        saveThreadCacheScrollHeightAsInitial(scrollHeight) {
+            if (!this.threadCache) {
+                return;
+            }
+            if (this.chatter) {
+                // Initial scroll height is disabled for chatter because it is
+                // too complex to handle correctly and less important
+                // functionally.
+                return;
+            }
+            this.update({
+                threadCacheInitialScrollHeights: Object.assign({}, this.threadCacheInitialScrollHeights, {
+                    [this.threadCache.localId]: scrollHeight,
+                }),
+            });
+        }
+
+        /**
+         * @param {integer} scrollTop
          */
         saveThreadCacheScrollPositionsAsInitial(scrollTop) {
             if (!this.threadCache) {
@@ -231,6 +251,15 @@ function factory(dependencies) {
             ],
         }),
         /**
+         * Determines the initial scroll height of thread caches, which is the
+         * scroll height at the time the last scroll position was saved.
+         * Useful to only restore scroll position when the corresponding height
+         * is available, otherwise the restore makes no sense.
+         */
+        threadCacheInitialScrollHeights: attr({
+            default: {},
+        }),
+        /**
          * Determines the initial scroll positions of thread caches.
          * Useful to restore scroll position on changing back to this
          * thread cache. Note that this is only applied when opening