From 8d96bfd0805a7ec7f1445f87327329025c837ebc Mon Sep 17 00:00:00 2001 From: Xavier Dubuc Date: Thu, 3 Sep 2020 16:24:19 +0000 Subject: [PATCH] [FIX] mail, sms, snailmail: prevent message_fetch for hidden threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Automatically load messages from a thread cache only if the thread cache is used in at least one thread view. Ensure thread viewer don't keep thread view records alive when they are not displayed. This change is particularly important for performance, to avoid spamming RPC at init if many threads are opened in chat windows. task-2310623 closes odoo/odoo#57121 X-original-commit: 8d3413ff647de8d98c6858062dd07dde4160af95 Related: odoo/enterprise#12979 Signed-off-by: Alexandre Kühn (aku) Signed-off-by: Sébastien Theys (seb) --- .../src/components/chat_window/chat_window.js | 31 +-- .../components/chat_window/chat_window.xml | 54 ++-- .../chat_window_manager_tests.js | 116 ++++++++ .../static/src/components/chatter/chatter.xml | 33 ++- .../chatter_topbar/chatter_topbar.xml | 2 +- .../static/src/components/discuss/discuss.js | 15 +- .../static/src/components/discuss/discuss.xml | 2 +- .../discuss/tests/discuss_pinned_tests.js | 8 +- .../components/discuss/tests/discuss_tests.js | 86 ++++-- .../src/components/message/message_tests.js | 86 ++++-- .../components/message_list/message_list.js | 26 +- .../components/thread_view/thread_view.xml | 4 +- .../thread_view/thread_view_tests.js | 43 ++- .../src/models/chat_window/chat_window.js | 128 ++++++--- .../chat_window_manager.js | 4 +- .../mail/static/src/models/chatter/chatter.js | 51 +++- .../mail/static/src/models/discuss/discuss.js | 119 +++++--- .../mail/static/src/models/message/message.js | 45 +++ .../messaging_initializer.js | 17 +- .../models/messaging_menu/messaging_menu.js | 2 +- .../messaging_notification_handler.js | 14 +- .../src/models/thread_cache/thread_cache.js | 211 +++++++------- .../src/models/thread_view/thread_view.js | 202 +++++++------- .../src/models/thread_view/thread_viewer.js | 257 ++++++++++++++++++ .../static/src/widgets/discuss/discuss.js | 12 +- .../widgets/form_renderer/form_renderer.js | 2 +- .../form_renderer/form_renderer_tests.js | 12 +- .../mail/static/tests/helpers/mock_server.js | 5 + addons/mail/views/assets.xml | 1 + .../src/components/message/message_tests.js | 14 +- .../src/components/message/message_tests.js | 56 ++-- 31 files changed, 1161 insertions(+), 497 deletions(-) create mode 100644 addons/mail/static/src/models/thread_view/thread_viewer.js 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 43353c6086f..a5d0b2c9a20 100644 --- a/addons/mail/static/src/components/chat_window/chat_window.js +++ b/addons/mail/static/src/components/chat_window/chat_window.js @@ -115,12 +115,10 @@ class ChatWindow extends Component { isDoFocus: false, isFocused: true, }); - if (this.chatWindow.isFolded) { - return; - } - if (!this.chatWindow.thread) { + if (this._inputRef.comp) { this._inputRef.comp.focus(); - } else { + } + if (this._threadRef.comp) { this._threadRef.comp.focus(); } } @@ -134,7 +132,10 @@ class ChatWindow extends Component { * @private */ _saveThreadScrollTop() { - this.chatWindow.threadView.saveThreadCacheScrollPositionsAsInitial( + if (!this._threadRef.comp || !this.chatWindow.threadViewer) { + return; + } + this.chatWindow.threadViewer.saveThreadCacheScrollPositionsAsInitial( this._threadRef.comp.getScrollTop() ); } @@ -248,9 +249,7 @@ class ChatWindow extends Component { this.chatWindow.unfold(); this.chatWindow.focus(); } else { - if (this.chatWindow.thread) { - this._saveThreadScrollTop(); - } + this._saveThreadScrollTop(); this.chatWindow.fold(); } } @@ -336,12 +335,7 @@ class ChatWindow extends Component { * @private */ async _onWillHideHomeMenu() { - if (!this.chatWindow.thread) { - return; - } - if (!this.chatWindow.isFolded) { - this._saveThreadScrollTop(); - } + this._saveThreadScrollTop(); } /** @@ -353,12 +347,7 @@ class ChatWindow extends Component { * @private */ async _onWillShowHomeMenu() { - if (!this.chatWindow.thread) { - return; - } - if (!this.chatWindow.isFolded) { - this._saveThreadScrollTop(); - } + this._saveThreadScrollTop(); } } diff --git a/addons/mail/static/src/components/chat_window/chat_window.xml b/addons/mail/static/src/components/chat_window/chat_window.xml index f92a32cf2fe..240cf0f974c 100644 --- a/addons/mail/static/src/components/chat_window/chat_window.xml +++ b/addons/mail/static/src/components/chat_window/chat_window.xml @@ -20,35 +20,33 @@ t-on-o-clicked="_onClickedHeader" t-ref="header" /> - - - + + + +
+ + To: + + - - -
- - To: - - -
- +
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 efba64cecde..220f4278fa9 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 @@ -1753,6 +1753,122 @@ QUnit.test('[technical] chat window with a thread: keep scroll position in messa ); }); +QUnit.test('chat window does not fetch messages if hidden', async function (assert) { + /** + * computation uses following info: + * ([mocked] global window width: 900px) + * (others: @see `mail/static/src/models/chat_window_manager/chat_window_manager.js:visual`) + * + * - chat window width: 325px + * - start/end/between gap width: 10px/10px/5px + * - hidden menu width: 200px + * - global width: 1080px + * + * Enough space for 2 visible chat windows, and one hidden chat window: + * 3 visible chat windows: + * 10 + 325 + 5 + 325 + 5 + 325 + 10 = 1000 > 900 + * 2 visible chat windows + hidden menu: + * 10 + 325 + 5 + 325 + 10 + 200 + 5 = 875 < 900 + */ + assert.expect(14); + + // 3 channels are expected to be found in the messaging menu, each with a + // random unique id that will be referenced in the test + this.data['mail.channel'].records = [ + { + id: 10, + is_minimized: true, + name: "Channel #10", + state: 'open', + }, + { + id: 11, + is_minimized: true, + name: "Channel #11", + state: 'open', + }, + { + id: 12, + is_minimized: true, + name: "Channel #12", + state: 'open', + }, + ]; + await this.start({ + env: { + browser: { + innerWidth: 900, + }, + }, + mockRPC(route, args) { + if (args.method === 'message_fetch') { + // domain should be like [['channel_id', 'in', [X]]] with X the channel id + const channel_ids = args.kwargs.domain[0][2]; + assert.strictEqual(channel_ids.length, 1, "messages should be fetched channel per channel"); + assert.step(`rpc:message_fetch:${channel_ids[0]}`); + } + return this._super(...arguments); + }, + }); + + assert.containsN( + document.body, + '.o_ChatWindow', + 2, + "2 chat windows should be visible" + ); + assert.containsNone( + document.body, + `.o_ChatWindow[data-thread-local-id="${ + this.env.models['mail.thread'].find(t => + t.model === 'mail.channel' && t.id === 12 + ).localId + }"]`, + "chat window for Channel #12 should be hidden" + ); + assert.containsOnce( + document.body, + '.o_ChatWindowHiddenMenu', + "chat window hidden menu should be displayed" + ); + assert.verifySteps( + ['rpc:message_fetch:10', 'rpc:message_fetch:11'], + "messages should be fetched for the two visible chat windows" + ); + + await afterNextRender(() => + document.querySelector('.o_ChatWindowHiddenMenu_dropdownToggle').click() + ); + assert.containsOnce( + document.body, + '.o_ChatWindowHiddenMenu_chatWindowHeader', + "1 hidden chat window should be listed in hidden menu" + ); + + await afterNextRender(() => + document.querySelector('.o_ChatWindowHiddenMenu_chatWindowHeader').click() + ); + assert.containsN( + document.body, + '.o_ChatWindow', + 2, + "2 chat windows should still be visible" + ); + assert.containsOnce( + document.body, + `.o_ChatWindow[data-thread-local-id="${ + this.env.models['mail.thread'].find(t => + t.model === 'mail.channel' && t.id === 12 + ).localId + }"]`, + "chat window for Channel #12 should now be visible" + ); + assert.verifySteps( + ['rpc:message_fetch:12'], + "messages should now be fetched for Channel #12" + ); +}); + }); }); }); diff --git a/addons/mail/static/src/components/chatter/chatter.xml b/addons/mail/static/src/components/chatter/chatter.xml index e5601dd7974..53550da516d 100644 --- a/addons/mail/static/src/components/chatter/chatter.xml +++ b/addons/mail/static/src/components/chatter/chatter.xml @@ -8,7 +8,7 @@ class="o_Chatter_topbar" chatterLocalId="chatter.localId" /> - + - - - - - - - + + + + + diff --git a/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml b/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml index 3512f93332e..6911dfb959e 100644 --- a/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml +++ b/addons/mail/static/src/components/chatter_topbar/chatter_topbar.xml @@ -5,7 +5,7 @@
- +