Skip to content

Commit 440e6a5

Browse files
committed
[FIX] mail: fix non deterministic bus subscription test
Before this PR, the `bus subscription is refreshed when channel is left` test was sometimes failing. This actually reveals a real issue: if a channel is joined a leave very quickly, the bus subscription is not updated. This occurs because we rely on the last subscription made and the one that should be made to detect if channels differ. Since the `updateBusSubscription` method is debounced, we can miss information. This PR replaces the complicated `updateBusSubscription` method by a `onAdd/onDelete`. This is much more reliable and more efficient since there is no need to walk through every channel to detect changes. This PR also remove a test that was redundant that the failing one. fixes runbot-55292,57645,56232 closes odoo#155720 Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
1 parent 12409d6 commit 440e6a5

File tree

5 files changed

+16
-48
lines changed

5 files changed

+16
-48
lines changed

addons/mail/static/src/core/common/channel_member_model.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class ChannelMember extends Record {
4848
}
4949

5050
get memberSince() {
51-
return deserializeDateTime(this.create_date);
51+
return this.create_date ? deserializeDateTime(this.create_date) : undefined;
5252
}
5353
}
5454

addons/mail/static/src/core/common/store_service.js

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -185,38 +185,10 @@ export class Store extends BaseStore {
185185

186186
setup() {
187187
super.setup();
188-
this.updateBusSubscription = debounce(this.updateBusSubscription, 0); // Wait for thread fully inserted.
189-
}
190-
191-
updateBusSubscription() {
192-
if (!this.isMessagingReady) {
193-
return;
194-
}
195-
const allSelfChannelIds = new Set();
196-
for (const thread of Object.values(this.Thread.records)) {
197-
if (thread.model === "discuss.channel" && thread.hasSelfAsMember) {
198-
if (thread.selfMember.memberSince < this.env.services["bus_service"].startedAt) {
199-
this.knownChannelIds.add(thread.id);
200-
}
201-
allSelfChannelIds.add(thread.id);
202-
}
203-
}
204-
let shouldUpdateChannels = false;
205-
for (const id of allSelfChannelIds) {
206-
if (!this.knownChannelIds.has(id)) {
207-
shouldUpdateChannels = true;
208-
this.knownChannelIds.add(id);
209-
}
210-
}
211-
for (const id of this.knownChannelIds) {
212-
if (!allSelfChannelIds.has(id)) {
213-
shouldUpdateChannels = true;
214-
this.knownChannelIds.delete(id);
215-
}
216-
}
217-
if (shouldUpdateChannels) {
218-
this.env.services["bus_service"].forceUpdateChannels();
219-
}
188+
this.updateBusSubscription = debounce(
189+
() => this.env.services.bus_service.forceUpdateChannels(),
190+
0
191+
);
220192
}
221193
}
222194
Store.register();
@@ -231,7 +203,6 @@ export const storeService = {
231203
const store = makeStore(env);
232204
store.discuss = {};
233205
store.discuss.activeTab = "main";
234-
Record.onChange(store.Thread, "records", () => store.updateBusSubscription());
235206
services.ui.bus.addEventListener("resize", () => {
236207
store.discuss.activeTab = "main";
237208
if (

addons/mail/static/src/core/common/thread_model.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export class Thread extends Record {
3838
thread.isLoadedDeferred.then(() => def.resolve());
3939
}
4040
});
41-
Record.onChange(thread, "channelMembers", () => this.store.updateBusSubscription());
4241
return thread;
4342
}
4443
/**
@@ -178,6 +177,17 @@ export class Thread extends Record {
178177
this._store.discuss.ringingThreads.delete(this);
179178
},
180179
});
180+
toggleBusSubscription = Record.attr(false, {
181+
compute() {
182+
return (
183+
this.model === "discuss.channel" &&
184+
this.selfMember?.memberSince >= this._store.env.services.bus_service.startedAt
185+
);
186+
},
187+
onUpdate() {
188+
this._store.updateBusSubscription();
189+
},
190+
});
181191
invitedMembers = Record.many("ChannelMember");
182192
chatPartner = Record.one("Persona");
183193
composer = Record.one("Composer", { inverse: "thread", onDelete: (r) => r.delete() });

addons/mail/static/src/discuss/core/common/discuss_core_common_service.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export class DiscussCoreCommon {
3131

3232
setup() {
3333
this.messagingService.isReady.then((data) => {
34-
this.store.updateBusSubscription();
3534
Record.MAKE_UPDATE(() => {
3635
for (const channelData of data.channels) {
3736
this.insertInitChannel(channelData);

addons/mail/static/tests/crosstab/crosstab_tests.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* @odoo-module */
22

33
import { startServer } from "@bus/../tests/helpers/mock_python_environment";
4-
import { waitUntilSubscribe } from "@bus/../tests/helpers/websocket_event_deferred";
54

65
import { start } from "@mail/../tests/helpers/test_utils";
76

@@ -110,17 +109,6 @@ QUnit.test("Channel subscription is renewed when channel is added from invite",
110109
await assertSteps(["update-channels"]);
111110
});
112111

113-
QUnit.test("Channel subscription is renewed when channel is left", async () => {
114-
const pyEnv = await startServer();
115-
pyEnv["discuss.channel"].create({ name: "Sales" });
116-
const { openDiscuss } = await start();
117-
await waitUntilSubscribe();
118-
openDiscuss();
119-
await click(".o-mail-DiscussSidebarChannel .btn[title='Leave this channel']");
120-
await contains(".o-mail-DiscussSidebarChannel", { count: 0 });
121-
await waitUntilSubscribe();
122-
});
123-
124112
QUnit.test("Adding attachments", async () => {
125113
const pyEnv = await startServer();
126114
const channelId = pyEnv["discuss.channel"].create({ name: "Hogwarts Legacy" });

0 commit comments

Comments
 (0)