[FIX] mail: fix load older/newer init value and preview fetch

The issue is that "load older" was called every time the thread was made
visible, in particular after chat window unfold.

The load more button will naturally be visible when first loading a
thread, it only makes sense to check its final visibility and not
intermediate steps.

But there was a bug where the loading mechanism was relying on the
initial "load older" to fetch messages, because the messaging menu
preview was incorrectly setting the thread as already loaded, when it
just fetched the last message.

Moreover, the "last message" displayed in the preview was not correctly
computed, and it could end up being the last "currently fetched" message
and not the last of all messages (when jumping in the past), and it
could also be a non-persistent message.

runbot-35533

closes odoo/odoo#143129

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Sébastien Theys
2023-11-25 01:23:19 +00:00
parent 69acc54411
commit 1c7b149aa7
11 changed files with 210 additions and 83 deletions
@@ -109,6 +109,14 @@ export class Message extends Record {
linkPreviews = Record.many("LinkPreview", { inverse: "message" });
/** @type {number[]} */
needaction_partner_ids = [];
originThread = Record.one("Thread", {
inverse: "allMessages",
compute() {
if (this.model && this.res_id) {
return { model: this.model, id: this.res_id };
}
},
});
/** @type {number[]} */
history_partner_ids = [];
parentMessage = Record.one("Message");
@@ -241,10 +249,6 @@ export class Message extends Record {
return candidates.has(this.subject?.toLowerCase());
}
get originThread() {
return this._store.Thread.get({ model: this.model, id: this.res_id });
}
get resUrl() {
return `${url("/web")}#model=${this.model}&id=${this.res_id}`;
}
+18 -10
View File
@@ -81,16 +81,24 @@ export class Thread extends Component {
* scrollable (in other cases).
*/
this.scrollableRef = this.props.scrollRef ?? useRef("messages");
this.loadOlderState = useVisible("load-older", () => {
if (this.loadOlderState.isVisible && !this.isJumpingRecent) {
this.threadService.fetchMoreMessages(this.props.thread);
}
});
this.loadNewerState = useVisible("load-newer", () => {
if (this.loadNewerState.isVisible && !this.isJumpingRecent) {
this.threadService.fetchMoreMessages(this.props.thread, "newer");
}
});
this.loadOlderState = useVisible(
"load-older",
() => {
if (this.loadOlderState.isVisible && !this.isJumpingRecent) {
this.threadService.fetchMoreMessages(this.props.thread);
}
},
{ init: null }
);
this.loadNewerState = useVisible(
"load-newer",
() => {
if (this.loadNewerState.isVisible && !this.isJumpingRecent) {
this.threadService.fetchMoreMessages(this.props.thread, "newer");
}
},
{ init: null }
);
this.presentThresholdState = useVisible(
"present-treshold",
() => this.updateShowJumpPresent(),
@@ -143,6 +143,9 @@ export class Thread extends Record {
uuid;
/** @type {string} */
model;
allMessages = Record.many("Message", {
inverse: "originThread",
});
/** @type {boolean} */
areAttachmentsLoaded = false;
attachments = Record.many("Attachment");
@@ -398,6 +401,14 @@ export class Thread extends Record {
return [...this.messages].reverse().find((msg) => Number.isInteger(msg.id));
}
get newestPersistentNotEmptyOfAllMessage() {
const allPersistentMessages = this.allMessages.filter(
(message) => Number.isInteger(message.id) && !message.isEmpty
);
allPersistentMessages.sort((m1, m2) => m2.id - m1.id);
return allPersistentMessages[0];
}
get oldestPersistentMessage() {
return this.messages.find((msg) => Number.isInteger(msg.id));
}
@@ -83,12 +83,13 @@ export class ThreadService {
/**
* @param {import("models").Thread} thread
*/
async markAsRead(thread) {
if (!thread.isLoaded) {
await thread.isLoadedDeferred;
await new Promise(setTimeout);
markAsRead(thread) {
const newestPersistentMessage = thread.newestPersistentNotEmptyOfAllMessage;
if (!newestPersistentMessage && !thread.isLoaded) {
thread.isLoadedDeferred
.then(() => new Promise(setTimeout))
.then(() => this.markAsRead(thread));
}
const newestPersistentMessage = thread.newestPersistentMessage;
thread.seen_message_id = newestPersistentMessage?.id ?? false;
if (
thread.message_unread_counter > 0 &&
@@ -98,13 +99,15 @@ export class ThreadService {
this.rpc("/discuss/channel/set_last_seen_message", {
channel_id: thread.id,
last_message_id: newestPersistentMessage.id,
}).then(() => {
this.updateSeen(thread, newestPersistentMessage.id);
}).catch((e) => {
if (e.code !== 404) {
throw e;
}
});
})
.then(() => {
this.updateSeen(thread, newestPersistentMessage.id);
})
.catch((e) => {
if (e.code !== 404) {
throw e;
}
});
} else if (newestPersistentMessage) {
this.updateSeen(thread);
}
@@ -113,7 +116,7 @@ export class ThreadService {
}
}
updateSeen(thread, lastSeenId = thread.newestPersistentMessage?.id) {
updateSeen(thread, lastSeenId = thread.newestPersistentNotEmptyOfAllMessage?.id) {
const lastReadIndex = thread.messages.findIndex((message) => message.id === lastSeenId);
let newNeedactionCounter = 0;
let newUnreadCounter = 0;
@@ -143,7 +146,7 @@ export class ThreadService {
needactionMessages: [],
message_unread_counter: 0,
message_needaction_counter: 0,
seen_message_id: thread.newestPersistentMessage?.id,
seen_message_id: thread.newestPersistentNotEmptyOfAllMessage?.id,
});
}
@@ -327,15 +330,9 @@ export class ThreadService {
for (const preview of previews) {
const thread = this.store.Thread.get({ model: "discuss.channel", id: preview.id });
const message = this.store.Message.insert(preview.last_message, { html: true });
if (!thread.isLoaded) {
thread.messages.push(message);
if (message.isNeedaction) {
thread.needactionMessages.add(message);
}
if (message.isNeedaction) {
thread.needactionMessages.add(message);
}
thread.isLoaded = true;
thread.loadOlder = true;
thread.status = "ready";
}
}
});
@@ -867,10 +864,16 @@ export class ThreadService {
}
if (thread?.model === "discuss.channel") {
if (persona.type === "partner") {
return url(`/discuss/channel/${thread.id}/partner/${persona.id}/avatar_128`, urlParams);
return url(
`/discuss/channel/${thread.id}/partner/${persona.id}/avatar_128`,
urlParams
);
}
if (persona.type === "guest") {
return url(`/discuss/channel/${thread.id}/guest/${persona.id}/avatar_128`, urlParams);
return url(
`/discuss/channel/${thread.id}/guest/${persona.id}/avatar_128`,
urlParams
);
}
}
if (persona.type === "partner" && persona?.id) {
@@ -167,14 +167,28 @@ export class MessagingMenu extends Component {
if (b.message_unread_counter > 0 && a.message_unread_counter === 0) {
return 1;
}
if (!a.newestPersistentMessage?.datetime && b.newestPersistentMessage?.datetime) {
if (
!a.newestPersistentNotEmptyOfAllMessage?.datetime &&
b.newestPersistentNotEmptyOfAllMessage?.datetime
) {
return 1;
}
if (!b.newestPersistentMessage?.datetime && a.newestPersistentMessage?.datetime) {
if (
!b.newestPersistentNotEmptyOfAllMessage?.datetime &&
a.newestPersistentNotEmptyOfAllMessage?.datetime
) {
return -1;
}
if (a.newestPersistentMessage?.datetime && b.newestPersistentMessage?.datetime) {
return b.newestPersistentMessage.datetime - a.newestPersistentMessage.datetime;
if (
a.newestPersistentNotEmptyOfAllMessage?.datetime &&
b.newestPersistentNotEmptyOfAllMessage?.datetime &&
a.newestPersistentNotEmptyOfAllMessage?.datetime !==
b.newestPersistentNotEmptyOfAllMessage?.datetime
) {
return (
b.newestPersistentNotEmptyOfAllMessage.datetime -
a.newestPersistentNotEmptyOfAllMessage.datetime
);
}
return b.localId > a.localId ? 1 : -1;
});
@@ -72,7 +72,7 @@
</t>
</t>
<t t-foreach="threads" t-as="thread" t-key="thread.localId">
<t t-set="message" t-value="thread.isChatChannel or (thread.type === 'channel' and thread.needactionMessages.length === 0) ? thread.newestMessage : thread.needactionMessages.at(-1)"/>
<t t-set="message" t-value="thread.isChatChannel or (thread.type === 'channel' and thread.needactionMessages.length === 0) ? thread.newestPersistentNotEmptyOfAllMessage : thread.needactionMessages.at(-1)"/>
<NotificationItem
body="message?.inlineBody"
counter="thread.needactionCounter"
+19 -30
View File
@@ -6,6 +6,7 @@ import {
onWillPatch,
onWillUnmount,
useComponent,
useEffect,
useRef,
useState,
} from "@odoo/owl";
@@ -165,37 +166,25 @@ export function useAutoScroll(refName, shouldScrollPredicate = () => true) {
export function useVisible(refName, cb, { init = false } = {}) {
const ref = useRef(refName);
const state = { isVisible: init };
const observer = new IntersectionObserver((entries) => {
for (const entry of entries) {
const newVal = entry.isIntersecting;
if (state.isVisible !== newVal) {
state.isVisible = newVal;
cb();
}
}
});
let el;
onMounted(observe);
onWillUnmount(() => {
if (!el) {
return;
}
observer.unobserve(el);
});
onPatched(observe);
function observe() {
if (ref.el !== el) {
if (el) {
observer.unobserve(el);
state.isVisible = false;
}
if (ref.el) {
observer.observe(ref.el);
}
}
el = ref.el;
function setValue(value) {
state.isVisible = value;
cb();
}
const observer = new IntersectionObserver((entries) => {
setValue(entries.at(-1).isIntersecting);
});
useEffect(
(el) => {
if (el) {
observer.observe(el);
return () => {
setValue(false);
observer.unobserve(el);
};
}
},
() => [ref.el]
);
return state;
}
@@ -982,7 +982,7 @@ QUnit.test("chat window: scroll conservation on toggle discuss", async () => {
const { openDiscuss, openView } = await start();
await click(".o_menu_systray .dropdown-toggle:has(i[aria-label='Messages'])");
await click(".o-mail-NotificationItem");
await contains(".o-mail-Message", { count: 31 });
await contains(".o-mail-Message", { count: 30 });
await contains(".o-mail-ChatWindow .o-mail-Thread", { scroll: "bottom" });
await scroll(".o-mail-ChatWindow .o-mail-Thread", 142);
openDiscuss(null);
@@ -992,7 +992,7 @@ QUnit.test("chat window: scroll conservation on toggle discuss", async () => {
res_model: "discuss.channel",
views: [[false, "list"]],
});
await contains(".o-mail-Message", { count: 31 });
await contains(".o-mail-Message", { count: 30 });
await contains(".o-mail-ChatWindow .o-mail-Thread", { scroll: 142 });
});
@@ -1011,7 +1011,7 @@ QUnit.test(
await start();
await click(".o_menu_systray .dropdown-toggle:has(i[aria-label='Messages'])");
await click(".o-mail-NotificationItem");
await contains(".o-mail-Message", { count: 31 });
await contains(".o-mail-Message", { count: 30 });
await contains(".o-mail-ChatWindow .o-mail-Thread", { scroll: "bottom" });
await scroll(".o-mail-ChatWindow .o-mail-Thread", 142);
// fold chat window
@@ -1020,7 +1020,7 @@ QUnit.test(
await contains(".o-mail-ChatWindow .o-mail-Thread", { count: 0 });
// unfold chat window
await click(".o-mail-ChatWindow-command[title='Open']");
await contains(".o-mail-Message", { count: 31 });
await contains(".o-mail-Message", { count: 30 });
await contains(".o-mail-ChatWindow .o-mail-Thread", { scroll: 142 });
}
);
@@ -1040,7 +1040,7 @@ QUnit.test(
const { openDiscuss, openView } = await start();
await click(".o_menu_systray .dropdown-toggle:has(i[aria-label='Messages'])");
await click(".o-mail-NotificationItem");
await contains(".o-mail-Message", { count: 31 });
await contains(".o-mail-Message", { count: 30 });
await contains(".o-mail-ChatWindow .o-mail-Thread", { scroll: "bottom" });
await scroll(".o-mail-ChatWindow .o-mail-Thread", 142);
// fold chat window
@@ -1054,7 +1054,7 @@ QUnit.test(
});
// unfold chat window
await click(".o-mail-ChatWindow-command[title='Open']");
await contains(".o-mail-ChatWindow .o-mail-Message", { count: 31 });
await contains(".o-mail-ChatWindow .o-mail-Message", { count: 30 });
await contains(".o-mail-ChatWindow .o-mail-Thread", { scroll: 142 });
}
);
@@ -5,7 +5,7 @@ import { startServer } from "@bus/../tests/helpers/mock_python_environment";
import { patchUiSize } from "@mail/../tests/helpers/patch_ui_size";
import { start } from "@mail/../tests/helpers/test_utils";
import { triggerHotkey } from "@web/../tests/helpers/utils";
import { patchWithCleanup, triggerHotkey } from "@web/../tests/helpers/utils";
import { click, contains, insertText } from "@web/../tests/utils";
QUnit.module("messaging menu");
@@ -58,3 +58,98 @@ QUnit.test("new message [REQUIRE FOCUS]", async () => {
await click(".o-mail-MessagingMenu button", { text: "New Message" });
await contains(".o-mail-ChatWindow .o-discuss-ChannelSelector input:focus");
});
QUnit.test("channel preview ignores empty message", async () => {
const pyEnv = await startServer();
const partnerId = pyEnv["res.partner"].create({ name: "Demo" });
const channelId = pyEnv["discuss.channel"].create({
name: "General",
});
pyEnv["mail.message"].create({
author_id: partnerId,
body: "<p>before last</p>",
model: "discuss.channel",
res_id: channelId,
});
pyEnv["mail.message"].create({
author_id: partnerId,
body: "<p></p>",
model: "discuss.channel",
res_id: channelId,
});
const { openDiscuss } = await start();
await openDiscuss(channelId);
await contains(".o-mail-Message", { text: "before last" });
await click(".o_menu_systray .dropdown-toggle:has(i[aria-label='Messages'])");
await contains(".o-mail-NotificationItem-text", { text: "Demo: before last" });
});
QUnit.test("channel preview ignores transient message", async () => {
const pyEnv = await startServer();
const partnerId = pyEnv["res.partner"].create({ name: "Demo" });
const channelId = pyEnv["discuss.channel"].create({
name: "General",
});
pyEnv["mail.message"].create({
author_id: partnerId,
body: "<p>test</p>",
model: "discuss.channel",
res_id: channelId,
});
const { openDiscuss } = await start();
await openDiscuss(channelId);
await insertText(".o-mail-Composer-input", "/who");
await click(".o-mail-Composer-send:enabled");
await contains(".o_mail_notification", { text: "You are alone in this channel." });
await click(".o_menu_systray .dropdown-toggle:has(i[aria-label='Messages'])");
await contains(".o-mail-NotificationItem-text", { text: "Demo: test" });
});
QUnit.test("channel preview ignores messages from the past", async () => {
// make scroll behavior instantaneous.
patchWithCleanup(Element.prototype, {
scrollIntoView() {
return super.scrollIntoView(true);
},
});
const pyEnv = await startServer();
const channelId = pyEnv["discuss.channel"].create({ name: "General" });
const messageId = pyEnv["mail.message"].create({
body: "first message",
message_type: "comment",
model: "discuss.channel",
res_id: channelId,
});
for (let i = 0; i < 100; i++) {
pyEnv["mail.message"].create({
body: `message ${i}`,
message_type: "comment",
model: "discuss.channel",
res_id: channelId,
});
}
pyEnv["mail.message"].create({
body: "last message",
message_type: "comment",
model: "discuss.channel",
parent_id: messageId,
res_id: channelId,
});
const { env, openDiscuss } = await start();
await openDiscuss(channelId);
await contains(".o-mail-Message", { count: 30 });
await contains(".o-mail-Message", { text: "last message" });
await contains(".o-mail-Thread", { scroll: "bottom" });
await click(".o-mail-MessageInReply-content", { text: "first message" });
await contains(".o-mail-Message", { text: "last message", count: 0 });
await click(".o_menu_systray .dropdown-toggle:has(i[aria-label='Messages'])");
await contains(".o-mail-NotificationItem-text", { text: "You: last message" });
pyEnv.withUser(pyEnv.currentUserId, () =>
env.services.rpc("/mail/message/post", {
post_data: { body: "new message", message_type: "comment" },
thread_id: channelId,
thread_model: "discuss.channel",
})
);
await contains(".o-mail-NotificationItem-text", { text: "You: new message" });
});
@@ -427,10 +427,11 @@ QUnit.test("mark unread channel as read", async (assert) => {
Command.create({ message_unread_counter: 1, partner_id: pyEnv.currentPartnerId }),
Command.create({ partner_id: partnerId }),
],
name: "My Channel",
});
const [messagId_1] = pyEnv["mail.message"].create([
{ author_id: partnerId, model: "discuss.channel", res_id: channelId },
{ author_id: partnerId, model: "discuss.channel", res_id: channelId },
{ author_id: partnerId, body: "not empty", model: "discuss.channel", res_id: channelId },
{ author_id: partnerId, body: "not empty", model: "discuss.channel", res_id: channelId },
]);
const [currentMemberId] = pyEnv["discuss.channel.member"].search([
["channel_id", "=", channelId],
@@ -447,8 +448,8 @@ QUnit.test("mark unread channel as read", async (assert) => {
await click(".o_menu_systray i[aria-label='Messages']");
await triggerEvents(".o-mail-NotificationItem", ["mouseenter"]);
await click(".o-mail-NotificationItem [title='Mark As Read']");
assert.verifySteps(["set_last_seen_message"]);
await contains(".o-mail-NotificationItem.text-muted");
assert.verifySteps(["set_last_seen_message"]);
await triggerEvents(".o-mail-NotificationItem", ["mouseenter"]);
await contains(".o-mail-NotificationItem [title='Mark As Read']", { count: 0 });
await contains(".o-mail-ChatWindow", { count: 0 });
@@ -248,11 +248,13 @@ QUnit.test("marked as read thread notifications are ordered by last message date
]);
pyEnv["mail.message"].create([
{
body: "not empty",
date: "2019-01-01 00:00:00",
model: "discuss.channel",
res_id: channelId_1,
},
{
body: "not empty",
date: "2020-01-01 00:00:00",
model: "discuss.channel",
res_id: channelId_2,