[FIX] mail: mark message as seen only when thread composer is focused

task-2272510

closes odoo/odoo#53088

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

8 files changed, 152 insertions, 94 deletions

diff --git a/addons/mail/static/src/components/composer/composer.js b/addons/mail/static/src/components/composer/composer.js
index f2135835a4d..63d85a0fb95 100644
--- a/addons/mail/static/src/components/composer/composer.js
+++ b/addons/mail/static/src/components/composer/composer.js
@@ -48,35 +48,13 @@ class Composer extends Component {
          * Reference of the text input component.
          */
         this._textInputRef = useRef('textInput');
-        /**
-         * Tracked focus counter from props. Useful to determine whether it
-         * should auto focus this composer when patched.
-         */
-        this._focusCount = 0;
-        // to focus if the prop changes
-        this._lastComposer = undefined;
         this._onClickCaptureGlobal = this._onClickCaptureGlobal.bind(this);
     }
 
     mounted() {
-        this._lastComposer = this.composer;
-        if (this.props.isFocusOnMount) {
-            this.focus();
-        }
         document.addEventListener('click', this._onClickCaptureGlobal, true);
     }
 
-    patched() {
-        // focus when changing composer
-        if (
-            this.props.isFocusOnMount &&
-            this._lastComposer !== this.composer
-        ) {
-            this.focus();
-        }
-        this._lastComposer = this.composer;
-    }
-
     willUnmount() {
         document.removeEventListener('click', this._onClickCaptureGlobal, true);
     }
@@ -338,7 +316,6 @@ Object.assign(Composer, {
         isCompact: true,
         isDiscardOnClickAway: false,
         isExpandable: false,
-        isFocusOnMount: false,
         isLog: false,
     },
     props: {
@@ -382,7 +359,6 @@ Object.assign(Composer, {
         isCompact: Boolean,
         isDiscardOnClickAway: Boolean,
         isExpandable: Boolean,
-        isFocusOnMount: Boolean,
         isLog: Boolean,
     },
     template: 'mail.Composer',
diff --git a/addons/mail/static/src/components/composer_text_input/composer_text_input.js b/addons/mail/static/src/components/composer_text_input/composer_text_input.js
index 198f563d1c7..1be0e77a98b 100644
--- a/addons/mail/static/src/components/composer_text_input/composer_text_input.js
+++ b/addons/mail/static/src/components/composer_text_input/composer_text_input.js
@@ -161,6 +161,20 @@ class ComposerTextInput extends Component {
     /**
      * @private
      */
+    _onFocusinTextarea() {
+        this.composer.update({ hasFocus: true });
+    }
+
+    /**
+     * @private
+     */
+    _onFocusoutTextarea() {
+        this.composer.update({ hasFocus: false });
+    }
+
+    /**
+     * @private
+     */
     _onInputTextarea() {
         if (this._textareaLastInputValue !== this._textareaRef.el.value) {
             this.composer.handleCurrentPartnerIsTyping();
diff --git a/addons/mail/static/src/components/composer_text_input/composer_text_input.xml b/addons/mail/static/src/components/composer_text_input/composer_text_input.xml
index f68b295cb8d..949f1065a56 100644
--- a/addons/mail/static/src/components/composer_text_input/composer_text_input.xml
+++ b/addons/mail/static/src/components/composer_text_input/composer_text_input.xml
@@ -36,7 +36,7 @@
                         </div>
                     </div>
                 </t>
-                <textarea class="o_ComposerTextInput_textarea" t-esc="composer.textInputContent" placeholder="Write something..." t-on-keydown="_onKeydownTextarea" t-on-keyup="_onKeyupTextarea" t-on-input="_onInputTextarea" t-ref="textarea"/>
+                <textarea class="o_ComposerTextInput_textarea" t-esc="composer.textInputContent" placeholder="Write something..." t-on-focusin="_onFocusinTextarea" t-on-focusout="_onFocusoutTextarea" t-on-keydown="_onKeydownTextarea" t-on-keyup="_onKeyupTextarea" t-on-input="_onInputTextarea" t-ref="textarea"/>
             </t>
         </div>
     </t>
diff --git a/addons/mail/static/src/components/discuss/discuss.xml b/addons/mail/static/src/components/discuss/discuss.xml
index 789c7a465c8..f20f08b409b 100644
--- a/addons/mail/static/src/components/discuss/discuss.xml
+++ b/addons/mail/static/src/components/discuss/discuss.xml
@@ -97,7 +97,6 @@
                 hasDiscardButton="true"
                 hasThreadName="true"
                 isDiscardOnClickAway="true"
-                isFocusOnMount="true"
                 t-on-o-message-posted="_onReplyingToMessageMessagePosted"
             />
         </t>
diff --git a/addons/mail/static/src/components/discuss/discuss_tests.js b/addons/mail/static/src/components/discuss/discuss_tests.js
index 7a37349041f..5fe7f90c142 100644
--- a/addons/mail/static/src/components/discuss/discuss_tests.js
+++ b/addons/mail/static/src/components/discuss/discuss_tests.js
@@ -1951,7 +1951,7 @@ QUnit.test('new messages separator', async function (assert) {
     // visible. This is necessary in order to display 'new messages' and not
     // remove from DOM right away from seeing last message.
     // AKU TODO: thread specific test
-    assert.expect(5);
+    assert.expect(6);
 
     let step = 0;
     Object.assign(this.data.initMessaging, {
@@ -2000,26 +2000,26 @@ QUnit.test('new messages separator', async function (assert) {
             return this._super(...arguments);
         },
     });
-    assert.strictEqual(
-        document.querySelectorAll(`
-            .o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_message
-        `).length,
+    assert.containsN(
+        document.body,
+        '.o_MessageList_message',
         25,
         "should have 25 messages"
     );
-    assert.strictEqual(
-        document.querySelectorAll(`
-            .o_Discuss_thread .o_ThreadViewer_messageList .o_MessageList_separatorNewMessages
-        `).length,
-        0,
+    assert.containsNone(
+        document.body,
+        '.o_MessageList_separatorNewMessages',
         "should not display 'new messages' separator"
     );
 
+    // scroll to top
     document.querySelector(`.o_Discuss_thread .o_ThreadViewer_messageList`).scrollTop = 0;
+    // composer is focused by default, we remove that focus
+    document.querySelector('.o_ComposerTextInput_textarea').blur();
     // simulate receiving a new message
     const data = {
         author_id: [36, "User26"],
-        body: "<p>boddy26</p>",
+        body: "<p>body26</p>",
         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,12 +314,14 @@ 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', [
@@ -334,10 +336,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 +352,84 @@ 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_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);
+        }
+    });
+    const thread = this.env.models['mail.thread'].create({
+        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: "<p>fdsfsd</p>",
+            author_id: [10, "Fred"],
+            model: "mail.channel",
+            channel_ids: [100],
+        }]
+    ];
+    await afterNextRender(() =>
+        this.widget.call('bus_service', 'trigger', 'notification', notifications)
+    );
+    assert.verifySteps(
+        ['rpc:channel_seen'],
+        "Channel should have been mark as seen directly"
+    );
+});
 });
 });
 });