[FIX] bus, mail: fix redundant bus subscriptions

Since 16.3, opening the web client on the Odoo backends results in two
subscribe events being sent through the bus websocket.

Discuss listens to the thread changes to know when the subscription
should be refreshed. In order to do so, the last subscription that was
made is kept. This issue is that this subscription is empty in the
first place so the first evaluation always considers the subscription
should be made.

In order to fix this issue, this PR refine the condition to determine
if the subscription should be renewed:
- The last subscription is different from the last one
- The user joined a channel after the bus initialization
- The user left a channel after the bus initialization

In order to test those scenario in a reliable way, this PR also
backports odoo#147455

closes odoo/odoo#154170

X-original-commit: 109b45f
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
Signed-off-by: Matthieu Stockbauer (tsm) <tsm@odoo.com>
This commit is contained in:
tsm-odoo
2024-02-15 18:21:31 +00:00
parent 9117487844
commit 4f0b262dbe
10 changed files with 156 additions and 22 deletions
@@ -29,7 +29,7 @@ export const busService = {
let isActive = false;
let isInitialized = false;
let isUsingSharedWorker = browser.SharedWorker && !isIosApp();
const startTs = new Date().getTime();
const startedAt = luxon.DateTime.now().set({ milliseconds: 0 });
const connectionInitializedDeferred = new Deferred();
/**
@@ -101,7 +101,7 @@ export const busService = {
debug: odoo.debug,
lastNotificationId: multiTab.getSharedValue("last_notification_id", 0),
uid,
startTs,
startTs: startedAt.valueOf(),
});
}
@@ -201,6 +201,7 @@ export const busService = {
callback(detail)
);
},
startedAt,
};
},
};
@@ -70,10 +70,13 @@ class TestGetDiscussChannel(TestImLivechatCommon, MailCommon):
})
self.assertFalse(channel_info['anonymous_name'])
self.assertEqual(channel_info['anonymous_country'], {'code': 'BE', 'id': belgium.id, 'name': 'Belgium'})
operator_member = self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', '=', operator.partner_id.id)])
visitor_member = self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', '=', test_user.partner_id.id)])
self.assertEqual(channel_info['channelMembers'], [['ADD', [
{
'thread': {'id': channel_info['id'], 'model': "discuss.channel"},
'id': self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', '=', operator.partner_id.id)]).id,
'create_date': fields.Datetime.to_string(operator_member.create_date),
'id': operator_member.id,
'persona': {
'active': True,
'country': False,
@@ -86,7 +89,8 @@ class TestGetDiscussChannel(TestImLivechatCommon, MailCommon):
},
{
'thread': {'id': channel_info['id'], 'model': "discuss.channel"},
'id': self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', '=', test_user.partner_id.id)]).id,
'create_date': fields.Datetime.to_string(visitor_member.create_date),
'id': visitor_member.id,
'persona': {
'active': True,
'country': {
@@ -112,13 +116,15 @@ class TestGetDiscussChannel(TestImLivechatCommon, MailCommon):
'user_id': operator.id,
'channel_id': self.livechat_channel.id,
})
operator_member = self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', '=', operator.partner_id.id)])
self.assertEqual(channel_info['operator_pid'], [operator.partner_id.id, "Michel Operator"])
self.assertFalse(channel_info['anonymous_name'])
self.assertEqual(channel_info['anonymous_country'], False)
self.assertEqual(channel_info['channelMembers'], [['ADD', [
{
'thread': {'id': channel_info['id'], 'model': "discuss.channel"},
'id': self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', '=', operator.partner_id.id)]).id,
'create_date': fields.Datetime.to_string(operator_member.create_date),
'id': operator_member.id,
'persona': {
'active': True,
'country': False,
@@ -3,6 +3,7 @@
import logging
import requests
import odoo
from odoo import api, fields, models, _
from odoo.exceptions import AccessError, UserError, ValidationError
from odoo.osv import expression
@@ -180,7 +181,7 @@ class ChannelMember(models.Model):
def _discuss_channel_member_format(self, fields=None):
if not fields:
fields = {'id': True, 'channel': {}, 'persona': {}}
fields = {'id': True, 'channel': {}, 'persona': {}, 'create_date': True}
members_formatted_data = {}
for member in self:
data = {}
@@ -201,6 +202,8 @@ class ChannelMember(models.Model):
data['custom_notifications'] = member.custom_notifications
if 'mute_until_dt' in fields:
data['mute_until_dt'] = member.mute_until_dt
if 'create_date' in fields:
data['create_date'] = odoo.fields.Datetime.to_string(member.create_date)
members_formatted_data[member] = data
return members_formatted_data
@@ -1,6 +1,7 @@
/* @odoo-module */
import { Record } from "@mail/core/common/record";
import { deserializeDateTime } from "@web/core/l10n/dates";
/**
* @class ChannelMember
@@ -22,6 +23,8 @@ export class ChannelMember extends Record {
return super.insert(...arguments);
}
/** @type {string} */
create_date;
/** @type {number} */
id;
persona = Record.one("Persona", { inverse: "channelMembers" });
@@ -36,6 +39,10 @@ export class ChannelMember extends Record {
getLangName() {
return this.persona.lang_name;
}
get memberSince() {
return deserializeDateTime(this.create_date);
}
}
ChannelMember.register();
@@ -49,7 +49,7 @@ export class Store extends BaseStore {
return this.imStatusTrackedPersonas.map((persona) => persona.id);
}
lastChannelSubscription = "";
knownChannelIds = new Set();
/** This is the current logged partner */
user = Record.one("Persona");
/** This is the current logged guest */
@@ -203,22 +203,34 @@ export class Store extends BaseStore {
}
updateBusSubscription() {
const channelIds = [];
const ids = Object.keys(this.Thread.records).sort(); // Ensure channels processed in same order.
for (const id of ids) {
const thread = this.Thread.records[id];
if (thread.model === "discuss.channel" && !thread.isTransient) {
channelIds.push(id);
if (!thread.hasSelfAsMember) {
this.env.services["bus_service"].addChannel(`discuss.channel_${thread.id}`);
if (!this.isMessagingReady) {
return;
}
const allSelfChannelIds = new Set();
for (const thread of Object.values(this.Thread.records)) {
if (thread.model === "discuss.channel" && thread.hasSelfAsMember) {
if (thread.selfMember.memberSince < this.env.services["bus_service"].startedAt) {
this.knownChannelIds.add(thread.id);
}
allSelfChannelIds.add(thread.id);
}
}
const channels = JSON.stringify(channelIds);
if (this.isMessagingReady && this.lastChannelSubscription !== channels) {
let shouldUpdateChannels = false;
for (const id of allSelfChannelIds) {
if (!this.knownChannelIds.has(id)) {
shouldUpdateChannels = true;
this.knownChannelIds.add(id);
}
}
for (const id of this.knownChannelIds) {
if (!allSelfChannelIds.has(id)) {
shouldUpdateChannels = true;
this.knownChannelIds.delete(id);
}
}
if (shouldUpdateChannels) {
this.env.services["bus_service"].forceUpdateChannels();
}
this.lastChannelSubscription = channels;
}
}
Store.register();
@@ -437,9 +437,11 @@ export class Thread extends Record {
}
get hasSelfAsMember() {
return this.channelMembers.some((channelMember) =>
channelMember.persona?.eq(this._store.self)
);
return Boolean(this.selfMember);
}
get selfMember() {
return this.channelMembers.find((member) => member.persona.eq(this._store.self));
}
get invitationLink() {
@@ -31,6 +31,7 @@ export class DiscussCoreCommon {
setup() {
this.messagingService.isReady.then((data) => {
this.store.updateBusSubscription();
Record.MAKE_UPDATE(() => {
for (const channelData of data.channels) {
this.insertInitChannel(channelData);
@@ -4,7 +4,9 @@ import { startServer } from "@bus/../tests/helpers/mock_python_environment";
import { start } from "@mail/../tests/helpers/test_utils";
import { click, contains } from "@web/../tests/utils";
import { assertSteps, click, contains, insertText, step } from "@web/../tests/utils";
import { patchWebsocketWorkerWithCleanup } from "@bus/../tests/helpers/mock_websocket";
import { patchDate } from "@web/../tests/helpers/utils";
QUnit.module("discuss");
@@ -19,3 +21,45 @@ QUnit.test("Member list and settings menu are exclusive", async () => {
await contains(".o-discuss-CallSettings");
await contains(".o-discuss-ChannelMemberList", { count: 0 });
});
QUnit.test("bus subscription is refreshed when channel is joined", async () => {
const pyEnv = await startServer();
pyEnv["discuss.channel"].create([{ name: "General" }, { name: "Sales" }]);
patchWebsocketWorkerWithCleanup({
_sendToServer({ event_name, data }) {
if (event_name === "subscribe") {
step(`subscribe - ${JSON.stringify(data.channels)}`);
}
},
});
const later = luxon.DateTime.now().plus({ seconds: 2 });
patchDate(later.year, later.month, later.day, later.hour, later.minute, later.second);
const { openDiscuss } = await start();
await assertSteps(["subscribe - []"]);
await openDiscuss();
await assertSteps([]);
await click(".o-mail-DiscussSidebar i[title='Add or join a channel']");
await insertText(".o-discuss-ChannelSelector input", "new channel");
await click(".o-discuss-ChannelSelector-suggestion");
await assertSteps(["subscribe - []"]);
});
QUnit.test("bus subscription is refreshed when channel is left", async () => {
const pyEnv = await startServer();
pyEnv["discuss.channel"].create({ name: "General" });
patchWebsocketWorkerWithCleanup({
_sendToServer({ event_name, data }) {
if (event_name === "subscribe") {
step(`subscribe - ${JSON.stringify(data.channels)}`);
}
},
});
const later = luxon.DateTime.now().plus({ seconds: 2 });
patchDate(later.year, later.month, later.day, later.hour, later.minute, later.second);
const { openDiscuss } = await start();
await assertSteps(["subscribe - []"]);
await openDiscuss();
await assertSteps([]);
await click("[title='Leave this channel']");
await assertSteps(["subscribe - []"]);
});
@@ -64,6 +64,7 @@ patch(MockServer.prototype, {
thread: { id: member.channel_id, model: "discuss.channel" },
id: member.id,
persona,
create_date: member.create_date,
};
dataList.push(data);
}
@@ -158,6 +158,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_general.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_general.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_general.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -210,6 +213,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_channel_public_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_channel_public_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_channel_public_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -262,6 +268,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_channel_public_2.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_channel_public_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_channel_public_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -314,6 +323,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_channel_group_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_channel_group_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_channel_group_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -366,6 +378,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_channel_group_2.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_channel_group_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_channel_group_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -419,6 +434,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_group_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_group_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_group_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -441,6 +459,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_group_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_group_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[12].partner_id).create_date
),
'id': self.channel_group_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[12].partner_id).id,
'persona': {
'active': True,
@@ -509,6 +530,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_chat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -531,6 +555,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[14].partner_id).create_date
),
'id': self.channel_chat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[14].partner_id).id,
'persona': {
'active': True,
@@ -599,6 +626,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_2.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_chat_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -621,6 +651,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_2.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[15].partner_id).create_date
),
'id': self.channel_chat_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[15].partner_id).id,
'persona': {
'active': True,
@@ -689,6 +722,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_3.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_3.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_chat_3.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -711,6 +747,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_3.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_3.channel_member_ids.filtered(lambda m: m.partner_id == self.users[2].partner_id).create_date
),
'id': self.channel_chat_3.channel_member_ids.filtered(lambda m: m.partner_id == self.users[2].partner_id).id,
'persona': {
'active': True,
@@ -779,6 +818,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_4.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_4.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_chat_4.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -801,6 +843,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_chat_4.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_chat_4.channel_member_ids.filtered(lambda m: m.partner_id == self.users[3].partner_id).create_date
),
'id': self.channel_chat_4.channel_member_ids.filtered(lambda m: m.partner_id == self.users[3].partner_id).id,
'persona': {
'active': True,
@@ -873,6 +918,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_livechat_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_livechat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_livechat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -889,6 +937,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_livechat_1.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_livechat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[1].partner_id).create_date
),
'id': self.channel_livechat_1.channel_member_ids.filtered(lambda m: m.partner_id == self.users[1].partner_id).id,
'persona': {
'active': True,
@@ -960,6 +1011,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_livechat_2.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).create_date
),
'id': self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.partner_id == self.users[0].partner_id).id,
'persona': {
'active': True,
@@ -976,6 +1030,9 @@ class TestDiscussFullPerformance(HttpCase):
'id': self.channel_livechat_2.id,
'model': "discuss.channel",
},
'create_date': fields.Datetime.to_string(
self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.guest_id).create_date
),
'id': self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.guest_id).id,
'persona': {
'id': self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.guest_id).guest_id.id,