From 562fdd67f2d8573b1817054227f16fec386b92dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Fri, 23 Oct 2020 11:03:09 +0000 Subject: [PATCH] [FIX] mail: do not return members of channels in channel_info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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#60616 X-original-commit: 91ce61ba568d83190e7ed2c168fd56101474683b Signed-off-by: Alexandre Kühn (aku) --- .../thread_icon/thread_icon_tests.js | 3 +- .../thread_textual_typing_status_tests.js | 3 +- .../messaging_notification_handler.js | 6 +- addons/mail/models/mail_channel.py | 9 ++- .../src/components/chat_window/chat_window.js | 3 + .../chat_window_manager_tests.js | 60 +++++++++++++++++-- .../components/message_list/message_list.js | 40 ++++++++++--- .../thread_icon/thread_icon_tests.js | 6 +- .../thread_textual_typing_status_tests.js | 33 ++++++---- .../src/components/thread_view/thread_view.js | 12 ++++ .../messaging_initializer.js | 14 ++++- .../messaging_notification_handler.js | 20 ++++++- .../src/models/thread_view/thread_view.js | 29 +++++++++ .../src/models/thread_view/thread_viewer.js | 31 +++++++++- .../mail/static/tests/helpers/mock_server.js | 14 ++++- 15 files changed, 241 insertions(+), 42 deletions(-) diff --git a/addons/im_livechat/static/src/components/thread_icon/thread_icon_tests.js b/addons/im_livechat/static/src/components/thread_icon/thread_icon_tests.js index 6ff7767af25..a8dceda1484 100644 --- a/addons/im_livechat/static/src/components/thread_icon/thread_icon_tests.js +++ b/addons/im_livechat/static/src/components/thread_icon/thread_icon_tests.js @@ -70,8 +70,9 @@ QUnit.test('livechat: public website visitor is typing', async function (assert) await afterNextRender(() => { const typingData = { info: 'typing_status', - partner_id: this.env.messaging.publicPartner.id, is_typing: true, + partner_id: this.env.messaging.publicPartner.id, + partner_name: this.env.messaging.publicPartner.name, }; const notification = [[false, 'mail.channel', 20], typingData]; this.widget.call('bus_service', 'trigger', 'notification', [notification]); diff --git a/addons/im_livechat/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js b/addons/im_livechat/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js index b3cbe96e277..dcc04fe741a 100644 --- a/addons/im_livechat/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js +++ b/addons/im_livechat/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js @@ -66,8 +66,9 @@ QUnit.test('receive visitor typing status "is typing"', async function (assert) await afterNextRender(() => { const typingData = { info: 'typing_status', - partner_id: this.env.messaging.publicPartner.id, is_typing: true, + partner_id: this.env.messaging.publicPartner.id, + partner_name: this.env.messaging.publicPartner.name, }; const notification = [[false, 'mail.channel', 20], typingData]; this.widget.call('bus_service', 'trigger', 'notification', [notification]); diff --git a/addons/im_livechat/static/src/models/messaging_notification_handler/messaging_notification_handler.js b/addons/im_livechat/static/src/models/messaging_notification_handler/messaging_notification_handler.js index 25e71660ebe..ca08208907d 100644 --- a/addons/im_livechat/static/src/models/messaging_notification_handler/messaging_notification_handler.js +++ b/addons/im_livechat/static/src/models/messaging_notification_handler/messaging_notification_handler.js @@ -13,7 +13,7 @@ registerInstancePatchModel('mail.messaging_notification_handler', 'im_livechat/s * @override */ _handleNotificationChannelTypingStatus(channelId, data) { - const { partner_id } = data; + const { partner_id, partner_name } = data; const channel = this.env.models['mail.thread'].findFromIdentifyingData({ id: channelId, model: 'mail.channel', @@ -22,15 +22,19 @@ registerInstancePatchModel('mail.messaging_notification_handler', 'im_livechat/s return; } let partnerId; + let partnerName; if (partner_id === this.env.messaging.publicPartner.id) { // Some shenanigans that this is a typing notification // from public partner. partnerId = channel.correspondent.id; + partnerName = channel.correspondent.name; } else { partnerId = partner_id; + partnerName = partner_name; } this._super(channelId, Object.assign(data, { partner_id: partnerId, + partner_name: partnerName, })); }, }); diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index b6dcde47403..eb5fca1bdfe 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -603,8 +603,12 @@ class Channel(models.Model): info['is_pinned'] = partner_channel.is_pinned # add members infos - partner_ids = channel_partners.mapped('partner_id').ids - info['members'] = [partner_infos[partner] for partner in partner_ids] + if channel.channel_type != 'channel': + # avoid sending potentially a lot of members for big channels + # exclude chat and other small channels from this optimization because they are + # assumed to be smaller and it's important to know the member list for them + partner_ids = channel_partners.mapped('partner_id').ids + info['members'] = [partner_infos[partner] for partner in partner_ids] if channel.channel_type != 'channel': info['seen_partners_info'] = [{ 'id': cp.id, @@ -851,6 +855,7 @@ class Channel(models.Model): 'info': 'typing_status', 'is_typing': is_typing, 'partner_id': self.env.user.partner_id.id, + 'partner_name': self.env.user.partner_id.name, } notifications.append([(self._cr.dbname, 'mail.channel', channel.id), data]) # notify backend users notifications.append([channel.uuid, data]) # notify frontend users 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 @@ -159,6 +159,13 @@ class MessageList extends Component { return message.date.format('LL'); } + /** + * @returns {integer} + */ + getScrollHeight() { + return this.el.scrollHeight; + } + /** * @returns {integer} */ @@ -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 @@ -61,6 +61,18 @@ class ThreadView extends Component { this._composerRef.comp.focusout(); } + /** + * 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. * 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 @@ -76,6 +76,21 @@ function factory(dependencies) { // Private //---------------------------------------------------------------------- + /** + * @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} @@ -264,6 +279,13 @@ function factory(dependencies) { inverse: 'threadViews', related: 'threadViewer.threadCache', }), + threadCacheInitialScrollHeight: attr({ + compute: '_computeThreadCacheInitialScrollHeight', + dependencies: [ + 'threadCache', + 'threadCacheInitialScrollHeights', + ], + }), threadCacheInitialScrollPosition: attr({ compute: '_computeThreadCacheInitialScrollPosition', dependencies: [ @@ -277,6 +299,13 @@ function factory(dependencies) { threadCacheIsLoading: attr({ 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. */ 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) { @@ -230,6 +250,15 @@ function factory(dependencies) { 'thread', ], }), + /** + * 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 diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js index 6167682c572..5d60c66cd3a 100644 --- a/addons/mail/static/tests/helpers/mock_server.js +++ b/addons/mail/static/tests/helpers/mock_server.js @@ -743,12 +743,16 @@ MockServer.include({ ['is_read', '=', false], ['mail_message_id', 'in', messages.map(message => message.id)], ]).length; - return Object.assign({}, channel, { + const res = Object.assign({}, channel, { info: extra_info, last_message_id: lastMessageId, members, message_needaction_counter: messageNeedactionCounter, }); + if (channel.channel_type === 'channel') { + delete res.members; + } + return res; }); }, /** @@ -983,10 +987,12 @@ MockServer.include({ } else { partner_id = this.currentPartnerId; } + const partner = this._getRecords('res.partner', [['id', '=', partner_id]]); const data = { 'info': 'typing_status', 'is_typing': is_typing, 'partner_id': partner_id, + 'partner_name': partner.name, }; const notifications = []; for (const channel of channels) { @@ -1714,9 +1720,11 @@ MockServer.include({ { active_test: false } )[0]; return { - "id": partner.id, - "display_name": partner.display_name, "active": partner.active, + "display_name": partner.display_name, + "id": partner.id, + "im_status": partner.im_status, + "name": partner.name, }; }, });