[FIX] mail: handle transient messages with (un)seen notions

task-2343887

closes odoo/odoo#60906

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

9 files changed, 151 insertions, 17 deletions

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 b5abc5b2236..b70f61475dd 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
@@ -1930,7 +1930,6 @@ QUnit.test('new message separator is shown in a chat window of a chat on receivi
             is_minimized: true,
             is_pinned: false,
             members: [this.data.currentPartnerId, 10],
-            message_unread_counter: 0,
             uuid: 'channel-10-uuid',
         },
     ];
diff --git a/addons/mail/static/src/components/discuss/tests/discuss_tests.js b/addons/mail/static/src/components/discuss/tests/discuss_tests.js
index 77f7e875e42..91a578e7b1b 100644
--- a/addons/mail/static/src/components/discuss/tests/discuss_tests.js
+++ b/addons/mail/static/src/components/discuss/tests/discuss_tests.js
@@ -1890,7 +1890,16 @@ QUnit.test('new messages separator [REQUIRE FOCUS]', async function (assert) {
 QUnit.test('restore thread scroll position', async function (assert) {
     assert.expect(6);
     // channels expected to be rendered, with random unique id that will be referenced in the test
-    this.data['mail.channel'].records.push({ id: 11 }, { id: 12 });
+    this.data['mail.channel'].records.push(
+        {
+            id: 11,
+            seen_message_id: 50, // prevent race condition with mark as seen (task-2372339)
+        },
+        {
+            id: 12,
+            seen_message_id: 50, // prevent race condition with mark as seen (task-2372339)
+        },
+    );
     for (let i = 1; i <= 25; i++) {
         this.data['mail.message'].records.push({
             body: "not empty",
diff --git a/addons/mail/static/src/components/message_author_prefix/message_author_prefix.xml b/addons/mail/static/src/components/message_author_prefix/message_author_prefix.xml
index de8992e4b8f..eddc2b01c39 100644
--- a/addons/mail/static/src/components/message_author_prefix/message_author_prefix.xml
+++ b/addons/mail/static/src/components/message_author_prefix/message_author_prefix.xml
@@ -7,7 +7,7 @@
                 <t t-if="message.author and message.author === env.messaging.currentPartner">
                     <i class="o_MessageAuthorPrefixIcon fa fa-mail-reply"/>You:
                 </t>
-                <t t-elif="thread and thread.channel_type === 'channel'">
+                <t t-elif="thread and message.author !== thread.correspondent">
                     <t t-esc="message.author.nameOrDisplayName"/>:
                 </t>
             </t>
diff --git a/addons/mail/static/src/components/message_list/message_list.xml b/addons/mail/static/src/components/message_list/message_list.xml
index fbd44eb5aba..432cdc5059a 100644
--- a/addons/mail/static/src/components/message_list/message_list.xml
+++ b/addons/mail/static/src/components/message_list/message_list.xml
@@ -43,12 +43,12 @@
                 <!-- MESSAGES -->
                 <t t-set="current_day" t-value="0"/>
                 <t t-set="prev_message" t-value="0"/>
-                <t t-if="threadView.thread.model === 'mail.channel' and threadView.thread.localMessageUnreadCounter > 0 and !threadView.thread.lastSeenByCurrentPartnerMessageId">
-                    <div class="o_MessageList_separator o_MessageList_separatorNewMessages o_MessageList_item" t-att-class="{ 'o-disable-animation': env.disableAnimation }" t-transition="fade">
-                        <hr class="o_MessageList_separatorLine o_MessageList_separatorLineNewMessages"/><span class="o_MessageList_separatorLabel o_MessageList_separatorLabelNewMessages">New messages</span>
-                    </div>
-                </t>
                 <t t-foreach="orderedMessages" t-as="message" t-key="message.localId">
+                    <t t-if="message === threadView.thread.messageAfterNewMessageSeparator">
+                        <div class="o_MessageList_separator o_MessageList_separatorNewMessages o_MessageList_item" t-att-class="{ 'o-disable-animation': env.disableAnimation }" t-transition="fade">
+                            <hr class="o_MessageList_separatorLine o_MessageList_separatorLineNewMessages"/><span class="o_MessageList_separatorLabel o_MessageList_separatorLabelNewMessages">New messages</span>
+                        </div>
+                    </t>
                     <t t-if="!message.isEmpty">
                         <t t-set="message_day" t-value="getDateDay(message)"/>
                         <t t-if="current_day !== message_day">
@@ -76,11 +76,6 @@
                             t-ref="{{ message.localId }}"
                         />
                         <t t-set="prev_message" t-value="message"/>
-                        <t t-if="threadView.thread.model === 'mail.channel' and threadView.thread.localMessageUnreadCounter > 0 and threadView.thread.lastSeenByCurrentPartnerMessageId === message.id">
-                            <div class="o_MessageList_separator o_MessageList_separatorNewMessages o_MessageList_item" t-att-class="{ 'o-disable-animation': env.disableAnimation }" t-transition="fade">
-                                <hr class="o_MessageList_separatorLine o_MessageList_separatorLineNewMessages"/><span class="o_MessageList_separatorLabel o_MessageList_separatorLabelNewMessages">New messages</span>
-                            </div>
-                        </t>
                     </t>
                 </t>
                 <!-- LOADING (if order desc)-->
diff --git a/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js b/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js
index 483597a7796..8224c088611 100644
--- a/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js
+++ b/addons/mail/static/src/components/messaging_menu/messaging_menu_tests.js
@@ -289,6 +289,10 @@ QUnit.test('basic rendering', async function (assert) {
 QUnit.test('counter is taking into account failure notification', async function (assert) {
     assert.expect(2);
 
+    this.data['mail.channel'].records.push({
+        id: 31,
+        seen_message_id: 11,
+    });
     // message that is expected to have a failure
     this.data['mail.message'].records.push({
         id: 11, // random unique id, will be used to link failure to message
diff --git a/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview.xml b/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview.xml
index b6a5c896174..3047003f292 100644
--- a/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview.xml
+++ b/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview.xml
@@ -39,9 +39,9 @@
                     </div>
                     <div class="o_ThreadNeedactionPreview_core">
                         <span class="o_ThreadNeedactionPreview_coreItem o_ThreadNeedactionPreview_inlineText" t-att-class="{ 'o-empty': inlineLastNeedactionMessageBody.length === 0 }">
-                            <t t-if="thread.lastMessage and thread.lastMessage.author">
+                            <t t-if="thread.lastNeedactionMessage and thread.lastNeedactionMessage.author">
                                 <MessageAuthorPrefix
-                                    messageLocalId="thread.lastMessage.localId"
+                                    messageLocalId="thread.lastNeedactionMessage.localId"
                                     threadLocalId="thread.localId"
                                 />
                             </t>
diff --git a/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview_tests.js b/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview_tests.js
index 9e2f19ffbaf..fe01b8f7c7d 100644
--- a/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview_tests.js
+++ b/addons/mail/static/src/components/thread_needaction_preview/thread_needaction_preview_tests.js
@@ -292,6 +292,59 @@ QUnit.test('[technical] opening a non-channel chat window should not call channe
     );
 });
 
+QUnit.test('preview should display last needaction message preview even if there is a more recent message that is not needaction in the thread', async function (assert) {
+    assert.expect(2);
+
+    this.data['res.partner'].records.push({
+        id: 11,
+        name: "Stranger",
+    });
+    this.data['mail.message'].records.push({
+        author_id: 11,
+        body: "I am the oldest but needaction",
+        id: 21,
+        model: 'res.partner',
+        needaction: true,
+        needaction_partner_ids: [this.data.currentPartnerId],
+        res_id: 11,
+    });
+    this.data['mail.message'].records.push({
+        author_id: this.data.currentPartnerId,
+        body: "I am more recent",
+        id: 22,
+        model: 'res.partner',
+        res_id: 11,
+    });
+    this.data['mail.notification'].records.push({
+        mail_message_id: 21,
+        notification_status: 'sent',
+        notification_type: 'inbox',
+        res_partner_id: this.data.currentPartnerId,
+    });
+    await this.start({
+        hasChatWindow: true,
+        hasMessagingMenu: true,
+    });
+    await afterNextRender(() => this.afterEvent({
+        eventName: 'o-thread-cache-loaded-messages',
+        func: () => document.querySelector('.o_MessagingMenu_toggler').click(),
+        message: "should wait until inbox loaded initial needaction messages",
+        predicate: ({ threadCache }) => {
+            return threadCache.thread.model === 'mail.box' && threadCache.thread.id === 'inbox';
+        },
+    }));
+    assert.containsOnce(
+        document.body,
+        '.o_ThreadNeedactionPreview_inlineText',
+        "should have a preview from the last message"
+    );
+    assert.strictEqual(
+        document.querySelector('.o_ThreadNeedactionPreview_inlineText').textContent,
+        'Stranger: I am the oldest but needaction',
+        "the displayed message should be the one that needs action even if there is a more recent message that is not needaction on the thread"
+    );
+});
+
 });
 });
 });
diff --git a/addons/mail/static/src/components/thread_preview/thread_preview.js b/addons/mail/static/src/components/thread_preview/thread_preview.js
index b10e8ee2586..92fcb6101d1 100644
--- a/addons/mail/static/src/components/thread_preview/thread_preview.js
+++ b/addons/mail/static/src/components/thread_preview/thread_preview.js
@@ -108,8 +108,8 @@ class ThreadPreview extends Component {
      * @param {MouseEvent} ev
      */
     _onClickMarkAsRead(ev) {
-        if (this.thread.lastMessage) {
-            this.thread.markAsSeen(this.thread.lastMessage.id);
+        if (this.thread.lastNonTransientMessage) {
+            this.thread.markAsSeen(this.thread.lastNonTransientMessage);
         }
     }
 
diff --git a/addons/mail/static/src/components/thread_view/thread_view_tests.js b/addons/mail/static/src/components/thread_view/thread_view_tests.js
index 7a73f587e1b..89ae34a0f8e 100644
--- a/addons/mail/static/src/components/thread_view/thread_view_tests.js
+++ b/addons/mail/static/src/components/thread_view/thread_view_tests.js
@@ -1367,6 +1367,80 @@ QUnit.test('[technical] message list with a full page of empty messages should s
     );
 });
 
+QUnit.test('first unseen message should be directly preceded by the new message separator if there is a transient message just before it while composer is not focused [REQUIRE FOCUS]', async function (assert) {
+    // The goal of removing the focus is to ensure the thread is not marked as seen automatically.
+    // Indeed that would trigger channel_seen no matter what, which is already covered by other tests.
+    // The goal of this test is to cover the conditions specific to transient messages,
+    // and the conditions from focus would otherwise shadow them.
+    assert.expect(3);
+
+    this.data['mail.channel_command'].records.push({ name: 'who' });
+    // Needed partner & user to allow simulation of message reception
+    this.data['res.partner'].records.push({
+        id: 11,
+        name: "Foreigner partner",
+    });
+    this.data['res.users'].records.push({
+        id: 42,
+        name: "Foreigner user",
+        partner_id: 11,
+    });
+    this.data['mail.channel'].records = [{
+        channel_type: 'channel',
+        id: 20,
+        is_pinned: true,
+        name: "General",
+        uuid: 'channel20uuid',
+    }];
+    await this.start();
+    const thread = this.env.models['mail.thread'].findFromIdentifyingData({
+        id: 20,
+        model: 'mail.channel'
+    });
+    const threadViewer = this.env.models['mail.thread_viewer'].create({
+        hasThreadView: true,
+        thread: [['link', thread]],
+    });
+    await this.createThreadViewComponent(threadViewer.threadView, { hasComposer: true });
+    // send a command that leads to receiving a transient message
+    document.querySelector('.o_ComposerTextInput_textarea').focus();
+    await afterNextRender(() => document.execCommand('insertText', false, "/who"));
+    await afterNextRender(() => {
+        document.querySelector('.o_Composer_buttonSend').focus();
+        document.querySelector('.o_Composer_buttonSend').click();
+    });
+
+    // simulate receiving a message
+    await afterNextRender(() => this.env.services.rpc({
+        route: '/mail/chat_post',
+        params: {
+            context: {
+                mockedUserId: 42,
+            },
+            uuid: 'channel20uuid',
+            message_content: "test",
+        },
+    }));
+    assert.containsN(
+        document.body,
+        '.o_Message',
+        2,
+        "should display 2 messages (the transient & the received message), after posting a command"
+    );
+    assert.containsOnce(
+        document.body,
+        '.o_MessageList_separatorNewMessages',
+        "separator should be shown as a message has been received"
+    );
+    assert.containsOnce(
+        document.body,
+        `.o_Message[data-message-local-id="${
+            this.env.models['mail.message'].find(m => m.isTransient).localId
+        }"] + .o_MessageList_separatorNewMessages`,
+        "separator should be shown just after transient message"
+    );
+});
+
 });
 });
 });