From 91ce61ba568d83190e7ed2c168fd56101474683b 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: [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#60091 Signed-off-by: Alexandre Kühn (aku) --- .../src/components/chat_window/chat_window.js | 3 ++ .../chat_window_manager_tests.js | 60 ++++++++++++++++++++-- .../src/components/message_list/message_list.js | 40 +++++++++++---- .../components/thread_icon/thread_icon_tests.js | 6 ++- .../thread_textual_typing_status_tests.js | 33 ++++++++---- .../src/components/thread_view/thread_view.js | 12 +++++ 6 files changed, 127 insertions(+), 27 deletions(-) (limited to 'addons/mail/static/src/components') 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. * -- cgit v1.2.3