boddy26
", + body: "body26
", channel_ids: [20], date: "2019-04-20 10:00:00", id: 126, @@ -2030,33 +2030,36 @@ QUnit.test('new messages separator', async function (assert) { }; await afterNextRender(() => { const notifications = [[['my-db', 'mail.channel', 20], data]]; - this.widget.call('bus_service', 'trigger', 'notification', notifications); + this.widget.call('bus_service', 'trigger', 'notification', notifications) }); - assert.strictEqual( - document.querySelectorAll(` - .o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_message - `).length, + assert.containsN( + document.body, + '.o_MessageList_message', 26, "should have 26 messages" ); - assert.strictEqual( - document.querySelectorAll(` - .o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_separatorNewMessages - `).length, - 1, + assert.containsOnce( + document.body, + '.o_MessageList_separatorNewMessages', "should display 'new messages' separator" ); - // scroll to bottom await afterNextRender(() => { document.querySelector(`.o_Discuss_thread .o_ThreadViewer_messageList`).scrollTop = document.querySelector(`.o_Discuss_thread .o_ThreadViewer_messageList`).scrollHeight; }); - assert.strictEqual( - document.querySelectorAll(` - .o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_separatorNewMessages - `).length, - 0, + + assert.containsOnce( + document.body, + '.o_MessageList_separatorNewMessages', + "should still display 'new messages' separator as composer is not focused" + ); + await afterNextRender(() => + document.querySelector('.o_ComposerTextInput_textarea').focus() + ); + assert.containsNone( + document.body, + '.o_MessageList_separatorNewMessages', "should no longer display 'new messages' separator (message seen)" ); }); @@ -3491,19 +3494,17 @@ QUnit.test('mark channel as seen on last message visible', async function (asser return this._super(...arguments); }, }); - assert.strictEqual( - document.querySelectorAll(` - .o_DiscussSidebar_item[data-thread-local-id="${ - this.env.models['mail.thread'].find(thread => - thread.id === 10 && - thread.model === 'mail.channel' - ).localId - }"] - `).length, - 1, + assert.containsOnce( + document.body, + `.o_DiscussSidebar_item[data-thread-local-id="${ + this.env.models['mail.thread'].find(thread => + thread.id === 10 && + thread.model === 'mail.channel' + ).localId + }"]`, "should have discuss sidebar item with the channel" ); - assert.ok( + assert.hasClass( document.querySelector(` .o_DiscussSidebar_item[data-thread-local-id="${ this.env.models['mail.thread'].find(thread => @@ -3511,7 +3512,8 @@ QUnit.test('mark channel as seen on last message visible', async function (asser thread.model === 'mail.channel' ).localId }"] - `).classList.contains('o-unread'), + `), + 'o-unread', "sidebar item of channel ID 10 should be unread" ); @@ -3525,7 +3527,7 @@ QUnit.test('mark channel as seen on last message visible', async function (asser }"] `).click() ); - assert.notOk( + assert.doesNotHaveClass( document.querySelector(` .o_DiscussSidebar_item[data-thread-local-id="${ this.env.models['mail.thread'].find(thread => @@ -3533,7 +3535,8 @@ QUnit.test('mark channel as seen on last message visible', async function (asser thread.model === 'mail.channel' ).localId }"] - `).classList.contains('o-unread'), + `), + 'o-unread', "sidebar item of channel ID 10 should not longer be unread" ); }); 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 c948689c6d1..f9106fd73c1 100644 --- a/addons/mail/static/src/components/message_list/message_list.js +++ b/addons/mail/static/src/components/message_list/message_list.js @@ -374,23 +374,17 @@ class MessageList extends Component { /** * @private */ - _checkThreadMarkAsRead() { + _checkMostRecentMessageIsVisible() { const thread = this.threadViewer.thread; const threadCache = this.threadViewer.threadCache; - if (!threadCache) { - return; - } - if (threadCache.messages.length === 0) { - return; - } - if (!this.mostRecentMessageRef) { - return; - } - if ( + const lastMessageIsVisible = + threadCache && + threadCache.messages.length > 0 && + this.mostRecentMessageRef && threadCache === thread.mainCache && - this.mostRecentMessageRef.isPartiallyVisible() - ) { - thread.markAsSeen(); + this.mostRecentMessageRef.isPartiallyVisible(); + if (lastMessageIsVisible) { + this.threadViewer.handleVisibleMessage(this.mostRecentMessageRef.message); } } @@ -460,7 +454,7 @@ class MessageList extends Component { * @private */ _update() { - this._checkThreadMarkAsRead(); + this._checkMostRecentMessageIsVisible(); } //-------------------------------------------------------------------------- @@ -492,7 +486,7 @@ class MessageList extends Component { if (this._isLoadMoreVisible()) { this._loadMore(); } - this._checkThreadMarkAsRead(); + this._checkMostRecentMessageIsVisible(); } } diff --git a/addons/mail/static/src/components/thread_viewer/thread_viewer.xml b/addons/mail/static/src/components/thread_viewer/thread_viewer.xml index e292ac4e4ec..331d61c86b0 100644 --- a/addons/mail/static/src/components/thread_viewer/thread_viewer.xml +++ b/addons/mail/static/src/components/thread_viewer/thread_viewer.xml @@ -31,7 +31,6 @@ hasCurrentPartnerAvatar="props.hasComposerCurrentPartnerAvatar" hasSendButton="props.hasComposerSendButton" hasThreadTyping="props.hasComposerThreadTyping" - isFocusOnMount="!env.messaging.device.isMobile" showAttachmentsExtensions="props.showComposerAttachmentsExtensions" showAttachmentsFilenames="props.showComposerAttachmentsFilenames" t-ref="composer" diff --git a/addons/mail/static/src/components/thread_viewer/thread_viewer_tests.js b/addons/mail/static/src/components/thread_viewer/thread_viewer_tests.js index 1c7d24a86e1..259e877cb34 100644 --- a/addons/mail/static/src/components/thread_viewer/thread_viewer_tests.js +++ b/addons/mail/static/src/components/thread_viewer/thread_viewer_tests.js @@ -286,8 +286,8 @@ QUnit.test('message list asc order', async function (assert) { assert.verifySteps(['message_fetch_1', 'message_fetch_2']); }); -QUnit.test('mark channel as fetched when a new message is loaded and as seen when message is visible', async function (assert) { - assert.expect(7); +QUnit.test('mark channel as fetched when a new message is loaded and as seen when focusing composer', async function (assert) { + assert.expect(8); await this.start({ mockRPC(route, args) { @@ -302,7 +302,7 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe 'mail.channel', 'channel_fetched is called on the right channel model' ); - assert.step('rpc_channel_fetched:called'); + assert.step('rpc:channel_fetch'); } else if (args.method === 'channel_seen') { assert.strictEqual( args.args[0][0], @@ -314,7 +314,79 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe 'mail.channel', 'channel_seeb is called on the right channel model' ); - assert.step('rpc_channel_seen:called'); + assert.step('rpc:channel_seen'); + } + return this._super(...arguments); + } + }); + const thread = this.env.models['mail.thread'].create({ + // FIXME should not be needed, see task-2277537 + composer: [['create']], // avoid initial focus + id: 100, + isPinned: true, // just to avoid joinChannel to be called + members: [['insert', [ + { + email: "john@example.com", + id: this.env.session.partner_id, + name: "John", + }, + { + email: "fred@example.com", + id: 10, + name: "Fred", + }, + ]]], + message_unread_counter: 1, // seen would not be called if not > 0 + model: 'mail.channel', + }); + + const threadViewer = this.env.models['mail.thread_viewer'].create({ thread: [['link', thread]] }); + await this.createThreadViewerComponent(threadViewer, { hasComposer: true }); + const notifications = [ + [['myDB', 'mail.channel', 100], { + channelId: 100, + id: 1, + body: "fdsfsd
", + author_id: [10, "Fred"], + model: "mail.channel", + channel_ids: [100], + }] + ]; + + await afterNextRender(() => + this.widget.call('bus_service', 'trigger', 'notification', notifications) + ); + assert.verifySteps( + ['rpc:channel_fetch'], + "Channel should have been fetched but not seen yet" + ); + + await afterNextRender(() => thread.composer.focus()); + assert.verifySteps( + ['rpc:channel_seen'], + "Channel should have been marked as seen after threadViewer got the focus" + ); +}); + +QUnit.test('mark channel as fetched and seen when a new message is loaded if composer is focused', async function (assert) { + assert.expect(4); + + await this.start({ + mockRPC(route, args) { + if (args.method === 'channel_fetched' && args.args[0] === 100) { + throw new Error("'channel_fetched' RPC must not be called for created channel as message is directly seen"); + } else if (args.method === 'channel_seen') { + assert.strictEqual( + args.args[0][0], + 100, + 'channel_seen is called on the right channel id' + ); + assert.strictEqual( + args.model, + 'mail.channel', + 'channel_seen is called on the right channel model' + ); + assert.step('rpc:channel_seen'); } return this._super(...arguments); } @@ -334,10 +406,12 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe name: "Fred", }, ]]], + message_unread_counter: 1, // seen would not be called if not > 0 model: 'mail.channel', }); + const threadViewer = this.env.models['mail.thread_viewer'].create({ thread: [['link', thread]] }); - await this.createThreadViewerComponent(threadViewer); + await this.createThreadViewerComponent(threadViewer, { hasComposer: true }); const notifications = [ [['myDB', 'mail.channel', 100], { channelId: 100, @@ -348,15 +422,14 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe channel_ids: [100], }] ]; - await afterNextRender(() => { - this.widget.call('bus_service', 'trigger', 'notification', notifications); - }); + await afterNextRender(() => + this.widget.call('bus_service', 'trigger', 'notification', notifications) + ); assert.verifySteps( - ['rpc_channel_fetched:called', 'rpc_channel_seen:called'], - "Channel should have been fetched before having been seen" + ['rpc:channel_seen'], + "Channel should have been mark as seen directly" ); }); - }); }); }); diff --git a/addons/mail/static/src/models/composer/composer.js b/addons/mail/static/src/models/composer/composer.js index 343e0d372c0..fa6e2698eb0 100644 --- a/addons/mail/static/src/models/composer/composer.js +++ b/addons/mail/static/src/models/composer/composer.js @@ -554,6 +554,9 @@ function factory(dependencies) { 'mainSuggestedPartners', ], }), + hasFocus: attr({ + default: false, + }), hasSuggestedPartners: attr({ compute: '_computeHasSuggestedPartners', dependencies: [ diff --git a/addons/mail/static/src/models/thread/thread.js b/addons/mail/static/src/models/thread/thread.js index 16f52ab861f..e1fd74a5d77 100644 --- a/addons/mail/static/src/models/thread/thread.js +++ b/addons/mail/static/src/models/thread/thread.js @@ -10,6 +10,23 @@ function factory(dependencies) { class Thread extends dependencies['mail.model'] { + /** + * FIXME With this, whenever client is aware of new thread, this will + * (almost) always focus its composer when displayed. This shouldn't be + * the case, instead auto-focus of composer is flow-specific. + * See task-2277537 + * + * @override + */ + static create(data) { + if (!data.composer) { + data.composer = [['create', { + isDoFocus: true, + }]]; + } + return super.create(data); + } + /** * @override */ @@ -1140,7 +1157,6 @@ function factory(dependencies) { dependencies: ['viewersChatWindow'], }), composer: one2one('mail.composer', { - autocreate: true, inverse: 'thread', isCausal: true, }), diff --git a/addons/mail/static/src/models/thread_viewer/thread_viewer.js b/addons/mail/static/src/models/thread_viewer/thread_viewer.js index 4da88fd9513..834c07d878a 100644 --- a/addons/mail/static/src/models/thread_viewer/thread_viewer.js +++ b/addons/mail/static/src/models/thread_viewer/thread_viewer.js @@ -67,6 +67,15 @@ function factory(dependencies) { }); } + /** + * @param {mail.message} message + */ + handleVisibleMessage(message) { + if (!this.lastVisibleMessage || this.lastVisibleMessage.id < message.id) { + this.update({ lastVisibleMessage: [['replace', message]] }); + } + } + /** * @param {string} scrollTop */ @@ -96,6 +105,24 @@ function factory(dependencies) { return [['replace', this.thread.cache(this.stringifiedDomain)]]; } + /** + * Not a real field, used to trigger `thread.markAsSeen` when one of + * the dependencies changes. + * + * @private + * @returns {boolean} + */ + _computeThreadShouldBeSetAsSeen() { + // FIXME condition should not be on "composer is focused" but "thread viewer is active" + // See task-2277543 + const lastMessageIsVisible = this.lastVisibleMessage && + this.lastVisibleMessage === this.lastMessage; + if (lastMessageIsVisible && this.hasComposerFocus && this.thread) { + this.thread.markAsSeen(); + } + return true; + } + /** * @private * @returns {integer|undefined} @@ -171,7 +198,7 @@ function factory(dependencies) { } if ( - this.thread && this.threadCache.isLoading && + this.thread && this.threadCache && this.threadCache.isLoading && !this.isShowingLoading && !this._isPreparingLoading ) { this._prepareLoading(); @@ -218,6 +245,20 @@ function factory(dependencies) { componentHintList: attr({ default: [], }), + composer: many2one('mail.composer', { + related: 'thread.composer', + }), + hasComposerFocus: attr({ + related: 'composer.hasFocus', + }), + lastMessage: many2one('mail.message', { + related: 'threadCache.lastMessage', + }), + /** + * Most recent message in the current thread viewer that has been shown + * to the current partner. + */ + lastVisibleMessage: many2one('mail.message'), messages: many2many('mail.message', { related: 'threadCache.messages', }), @@ -255,6 +296,19 @@ function factory(dependencies) { threadCaches: many2many('mail.thread_cache', { related: 'thread.caches', }), + /** + * Not a real field, used to trigger `thread.markAsSeen` when one of + * the dependencies changes. + */ + threadShouldBeSetAsSeen: attr({ + compute: '_computeThreadShouldBeSetAsSeen', + dependencies: [ + 'hasComposerFocus', + 'lastMessage', + 'lastVisibleMessage', + 'threadCache', + ], + }), uncheckedMessages: many2many('mail.message', { related: 'threadCache.uncheckedMessages', }),