pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] mail: do not return members of channels in channel_info

Browse Source 
This PR has 3 parts:

The members fix itself, which is necessary for performance reasons especially
at init messaging, as per described in the task.

Adding the partner name in the typing payload, because due to not knowing all
members anymore, it is necessary to properly display who is typing.

Restrict restoring scroll position only if the corresponding scroll height is
available, to make the scroll feature and particularly the corresponding tests
more robust.

task-2360020

closes odoo/odoo#60616

X-original-commit: 91ce61ba568d83190e7ed2c168fd56101474683b
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Sébastien Theys
2020-10-23 12:27:57 +00:00
parent 1a509642b9
commit 562fdd67f2
15 changed files with 241 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/im_livechat/static/src/components/thread_icon/thread_icon_tests.js
+2 -1
View File
@@ -70,8 +70,9 @@ QUnit.test('livechat: public website visitor is typing', async function (assert)
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: this.env.messaging.publicPartner.id,
is_typing: true,
partner_id: this.env.messaging.publicPartner.id,
partner_name: this.env.messaging.publicPartner.name,
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
addons/im_livechat/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js
+2 -1
View File
@@ -66,8 +66,9 @@ QUnit.test('receive visitor typing status "is typing"', async function (assert)
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: this.env.messaging.publicPartner.id,
is_typing: true,
partner_id: this.env.messaging.publicPartner.id,
partner_name: this.env.messaging.publicPartner.name,
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
addons/im_livechat/static/src/models/messaging_notification_handler/messaging_notification_handler.js
+5 -1
View File
@@ -13,7 +13,7 @@ registerInstancePatchModel('mail.messaging_notification_handler', 'im_livechat/s
* @override
*/
_handleNotificationChannelTypingStatus(channelId, data) {
const { partner_id } = data;
const { partner_id, partner_name } = data;
const channel = this.env.models['mail.thread'].findFromIdentifyingData({
id: channelId,
model: 'mail.channel',
@@ -22,15 +22,19 @@ registerInstancePatchModel('mail.messaging_notification_handler', 'im_livechat/s
return;
}
let partnerId;
let partnerName;
if (partner_id === this.env.messaging.publicPartner.id) {
// Some shenanigans that this is a typing notification
// from public partner.
partnerId = channel.correspondent.id;
partnerName = channel.correspondent.name;
} else {
partnerId = partner_id;
partnerName = partner_name;
}
this._super(channelId, Object.assign(data, {
partner_id: partnerId,
partner_name: partnerName,
}));
},
});
addons/mail/models/mail_channel.py
+7 -2
View File
@@ -603,8 +603,12 @@ class Channel(models.Model):
info['is_pinned'] = partner_channel.is_pinned
# add members infos
partner_ids = channel_partners.mapped('partner_id').ids
info['members'] = [partner_infos[partner] for partner in partner_ids]
if channel.channel_type != 'channel':
# avoid sending potentially a lot of members for big channels
# exclude chat and other small channels from this optimization because they are
# assumed to be smaller and it's important to know the member list for them
partner_ids = channel_partners.mapped('partner_id').ids
info['members'] = [partner_infos[partner] for partner in partner_ids]
if channel.channel_type != 'channel':
info['seen_partners_info'] = [{
'id': cp.id,
@@ -851,6 +855,7 @@ class Channel(models.Model):
'info': 'typing_status',
'is_typing': is_typing,
'partner_id': self.env.user.partner_id.id,
'partner_name': self.env.user.partner_id.name,
}
notifications.append([(self._cr.dbname, 'mail.channel', channel.id), data]) # notify backend users
notifications.append([channel.uuid, data]) # notify frontend users
addons/mail/static/src/components/chat_window/chat_window.js
+3
View File
@@ -135,6 +135,9 @@ class ChatWindow extends Component {
if (!this._threadRef.comp || !this.chatWindow.threadViewer) {
return;
}
this.chatWindow.threadViewer.saveThreadCacheScrollHeightAsInitial(
this._threadRef.comp.getScrollHeight()
);
this.chatWindow.threadViewer.saveThreadCacheScrollPositionsAsInitial(
this._threadRef.comp.getScrollTop()
);
addons/mail/static/src/components/chat_window_manager/chat_window_manager_tests.js
+55 -5
View File
@@ -23,12 +23,13 @@ QUnit.module('chat_window_manager_tests.js', {
beforeEach(this);
this.start = async params => {
const { env, widget } = await start(Object.assign(
const { afterEvent, env, widget } = await start(Object.assign(
{ hasChatWindow: true, hasMessagingMenu: true },
params,
{ data: this.data }
));
this.debug = params && params.debug;
this.afterEvent = afterEvent;
this.env = env;
this.widget = widget;
};
@@ -812,7 +813,20 @@ QUnit.test('[technical] chat window: scroll conservation on toggle home menu', a
document.querySelector(`.o_MessagingMenu_dropdownMenu .o_NotificationList_preview`).click()
);
// Set a scroll position to chat window
document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
await this.afterEvent({
eventName: 'o-component-message-list-scrolled',
func: () => {
document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
},
message: "should wait until channel 20 scrolled to 142 after setting this value manually",
predicate: ({ scrollTop, threadViewer }) => {
return (
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20 &&
scrollTop === 142
);
},
});
assert.strictEqual(
document.querySelector(`.o_ThreadView_messageList`).scrollTop,
142,
@@ -826,7 +840,18 @@ QUnit.test('[technical] chat window: scroll conservation on toggle home menu', a
"chat window scrollTop should still be the same after home menu is hidden"
);
await afterNextRender(() => this.showHomeMenu());
await this.afterEvent({
eventName: 'o-component-message-list-scrolled',
func: () => this.showHomeMenu(),
message: "should wait until channel 20 restored its scroll to 142 after hiding the home menu",
predicate: ({ scrollTop, threadViewer }) => {
return (
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20 &&
scrollTop === 142
);
},
});
assert.strictEqual(
document.querySelector(`.o_ThreadView_messageList`).scrollTop,
142,
@@ -1552,7 +1577,20 @@ QUnit.test('chat window with a thread: keep scroll position in message list on f
document.querySelector(`.o_NotificationList_preview`).click()
);
// Set a scroll position to chat window
document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
await this.afterEvent({
eventName: 'o-component-message-list-scrolled',
func: () => {
document.querySelector(`.o_ThreadView_messageList`).scrollTop = 142;
},
message: "should wait until channel 20 scrolled to 142 after setting this value manually",
predicate: ({ scrollTop, threadViewer }) => {
return (
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20 &&
scrollTop === 142
);
},
});
assert.strictEqual(
document.querySelector(`.o_ThreadView_messageList`).scrollTop,
142,
@@ -1568,7 +1606,19 @@ QUnit.test('chat window with a thread: keep scroll position in message list on f
);
// unfold chat window
await afterNextRender(() => document.querySelector('.o_ChatWindow_header').click());
await afterNextRender(() => this.afterEvent({
eventName: 'o-component-message-list-scrolled',
func: () => document.querySelector('.o_ChatWindow_header').click(),
message: "should wait until channel 20 restored its scroll position to 142",
predicate: ({ scrollTop, threadViewer }) => {
return (
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20 &&
scrollTop === 142
);
},
}));
assert.strictEqual(
document.querySelector(`.o_ThreadView_messageList`).scrollTop,
142,
addons/mail/static/src/components/message_list/message_list.js
+31 -9
View File
@@ -159,6 +159,13 @@ class MessageList extends Component {
return message.date.format('LL');
}
/**
* @returns {integer}
*/
getScrollHeight() {
return this.el.scrollHeight;
}
/**
* @returns {integer}
*/
@@ -323,9 +330,13 @@ class MessageList extends Component {
if (threadCache.messages.length > 0) {
if (this.threadView.threadCacheInitialScrollPosition !== undefined) {
if (this.props.hasScrollAdjust) {
this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
if (this.el.scrollHeight === this.threadView.threadCacheInitialScrollHeight) {
this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
isProcessed = true;
}
} else {
isProcessed = true;
}
isProcessed = true;
} else {
const lastMessage = threadCache.lastMessage;
if (this.messageRefFromId(lastMessage.id)) {
@@ -348,8 +359,9 @@ class MessageList extends Component {
* @param {Object} hint
*/
_adjustFromChatWindowUnfolded(hint) {
this._adjustScrollFromModel();
this.threadView.markComponentHintProcessed(hint);
if (this._adjustScrollFromModel()) {
this.threadView.markComponentHintProcessed(hint);
}
}
/**
@@ -357,8 +369,9 @@ class MessageList extends Component {
* @param {Object} hint
*/
_adjustFromHomeMenuHidden(hint) {
this._adjustScrollFromModel();
this.threadView.markComponentHintProcessed(hint);
if (this._adjustScrollFromModel()) {
this.threadView.markComponentHintProcessed(hint);
}
}
/**
@@ -366,8 +379,9 @@ class MessageList extends Component {
* @param {Object} hint
*/
_adjustFromHomeMenuShown(hint) {
this._adjustScrollFromModel();
this.threadView.markComponentHintProcessed(hint);
if (this._adjustScrollFromModel()) {
this.threadView.markComponentHintProcessed(hint);
}
}
/**
@@ -423,14 +437,21 @@ class MessageList extends Component {
/**
* @private
* @returns {boolean} whether the adjustment should be considered processed
*/
_adjustScrollFromModel() {
if (
this.threadView.threadCacheInitialScrollPosition !== undefined &&
this.props.hasScrollAdjust
) {
this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
if (this.el.scrollHeight === this.threadView.threadCacheInitialScrollHeight) {
this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
return true;
} else {
return false;
}
}
return true;
}
/**
@@ -560,6 +581,7 @@ class MessageList extends Component {
? scrollTop >= this.el.scrollHeight - this.el.clientHeight - margin
: scrollTop <= margin;
this.threadView.update({ hasAutoScrollOnMessageReceived });
this.threadView.threadViewer.saveThreadCacheScrollHeightAsInitial(this.el.scrollHeight);
this.threadView.threadViewer.saveThreadCacheScrollPositionsAsInitial(scrollTop);
if (!this._isAutoLoadOnScrollActive) {
return;
addons/mail/static/src/components/thread_icon/thread_icon_tests.js
+4 -2
View File
@@ -75,8 +75,9 @@ QUnit.test('chat: correspondent is typing', async function (assert) {
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: true,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -96,8 +97,9 @@ QUnit.test('chat: correspondent is typing', async function (assert) {
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: false,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
addons/mail/static/src/components/thread_textual_typing_status/thread_textual_typing_status_tests.js
+22 -11
View File
@@ -65,8 +65,9 @@ QUnit.test('receive other member typing status "is typing"', async function (ass
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: true,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -103,8 +104,9 @@ QUnit.test('receive other member typing status "is typing" then "no longer is ty
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: true,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -119,8 +121,9 @@ QUnit.test('receive other member typing status "is typing" then "no longer is ty
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: false,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -159,8 +162,9 @@ QUnit.test('assume other member typing status becomes "no longer is typing" afte
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: true,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -206,8 +210,9 @@ QUnit.test ('other member typing status "is typing" refreshes 60 seconds timer o
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: true,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -222,8 +227,9 @@ QUnit.test ('other member typing status "is typing" refreshes 60 seconds timer o
await this.env.testUtils.advanceTime(50 * 1000);
const typingData = {
info: 'typing_status',
partner_id: 17,
is_typing: true,
partner_id: 17,
partner_name: "Demo",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -272,8 +278,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 10,
is_typing: true,
partner_id: 10,
partner_name: "Other10",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -288,8 +295,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 11,
is_typing: true,
partner_id: 11,
partner_name: "Other11",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -304,8 +312,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 12,
is_typing: true,
partner_id: 12,
partner_name: "Other12",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -320,8 +329,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 10,
is_typing: false,
partner_id: 10,
partner_name: "Other10",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
@@ -336,8 +346,9 @@ QUnit.test('receive several other members typing status "is typing"', async func
await afterNextRender(() => {
const typingData = {
info: 'typing_status',
partner_id: 10,
is_typing: true,
partner_id: 10,
partner_name: "Other10",
};
const notification = [[false, 'mail.channel', 20], typingData];
this.widget.call('bus_service', 'trigger', 'notification', [notification]);
addons/mail/static/src/components/thread_view/thread_view.js
+12
View File
@@ -61,6 +61,18 @@ class ThreadView extends Component {
this._composerRef.comp.focusout();
}
/**
* Get the scroll height in the message list.
*
* @returns {integer|undefined}
*/
getScrollHeight() {
if (!this._messageListRef.comp) {
return undefined;
}
return this._messageListRef.comp.getScrollHeight();
}
/**
* Get the scroll position in the message list.
*
addons/mail/static/src/models/messaging_initializer/messaging_initializer.js
+11 -3
View File
@@ -153,9 +153,17 @@ function factory(dependencies) {
// there might be a lot of channels, insert each of them one by
// one asynchronously to avoid blocking the UI
await this.async(() => new Promise(resolve => setTimeout(resolve)));
const channel = this.env.models['mail.thread'].insert(
this.env.models['mail.thread'].convertData(channelData)
);
const convertedData = this.env.models['mail.thread'].convertData(channelData);
if (!convertedData.members) {
// channel_info does not return all members of channel for
// performance reasons, but code is expecting to know at
// least if the current partner is member of it.
// (e.g. to know when to display "invited" notification)
// Current partner can always be assumed to be a member of
// channels received at init.
convertedData.members = [['link', this.env.messaging.currentPartner]];
}
const channel = this.env.models['mail.thread'].insert(convertedData);
// flux specific: channels received at init have to be
// considered pinned. task-2284357
if (!channel.isPinned) {
addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
+17 -3
View File
@@ -108,6 +108,7 @@ function factory(dependencies) {
is_typing,
last_message_id,
partner_id,
partner_name,
} = data;
switch (info) {
case 'channel_fetched':
@@ -124,6 +125,7 @@ function factory(dependencies) {
return this._handleNotificationChannelTypingStatus(channelId, {
is_typing,
partner_id,
partner_name,
});
default:
return this._handleNotificationChannelMessage(channelId, data);
@@ -344,8 +346,9 @@ function factory(dependencies) {
* @param {Object} param1
* @param {boolean} param1.is_typing
* @param {integer} param1.partner_id
* @param {string} param1.partner_name
*/
_handleNotificationChannelTypingStatus(channelId, { is_typing, partner_id }) {
_handleNotificationChannelTypingStatus(channelId, { is_typing, partner_id, partner_name }) {
const channel = this.env.models['mail.thread'].findFromIdentifyingData({
id: channelId,
model: 'mail.channel',
@@ -353,7 +356,10 @@ function factory(dependencies) {
if (!channel) {
return;
}
const partner = this.env.models['mail.partner'].insert({ id: partner_id });
const partner = this.env.models['mail.partner'].insert({
id: partner_id,
name: partner_name,
});
if (partner === this.env.messaging.currentPartner) {
// Ignore management of current partner is typing notification.
return;
@@ -469,7 +475,15 @@ function factory(dependencies) {
const convertedData = this.env.models['mail.thread'].convertData(
Object.assign({ model: 'mail.channel' }, data)
);
if (!convertedData.members) {
// channel_info does not return all members of channel for
// performance reasons, but code is expecting to know at
// least if the current partner is member of it.
// (e.g. to know when to display "invited" notification)
// Current partner can always be assumed to be a member of
// channels received through this notification.
convertedData.members = [['link', this.env.messaging.currentPartner]];
}
let channel = this.env.models['mail.thread'].findFromIdentifyingData(convertedData);
const wasCurrentPartnerMember = (
channel &&
addons/mail/static/src/models/thread_view/thread_view.js
+29
View File
@@ -76,6 +76,21 @@ function factory(dependencies) {
// Private
//----------------------------------------------------------------------
/**
* @private
* @returns {integer|undefined}
*/
_computeThreadCacheInitialScrollHeight() {
if (!this.threadCache) {
return clear();
}
const threadCacheInitialScrollHeight = this.threadCacheInitialScrollHeights[this.threadCache.localId];
if (threadCacheInitialScrollHeight !== undefined) {
return threadCacheInitialScrollHeight;
}
return clear();
}
/**
* @private
* @returns {integer|undefined}
@@ -264,6 +279,13 @@ function factory(dependencies) {
inverse: 'threadViews',
related: 'threadViewer.threadCache',
}),
threadCacheInitialScrollHeight: attr({
compute: '_computeThreadCacheInitialScrollHeight',
dependencies: [
'threadCache',
'threadCacheInitialScrollHeights',
],
}),
threadCacheInitialScrollPosition: attr({
compute: '_computeThreadCacheInitialScrollPosition',
dependencies: [
@@ -277,6 +299,13 @@ function factory(dependencies) {
threadCacheIsLoading: attr({
related: 'threadCache.isLoading',
}),
/**
* List of saved initial scroll heights of thread caches.
*/
threadCacheInitialScrollHeights: attr({
default: {},
related: 'threadViewer.threadCacheInitialScrollHeights',
}),
/**
* List of saved initial scroll positions of thread caches.
*/
addons/mail/static/src/models/thread_view/thread_viewer.js
+30 -1
View File
@@ -13,7 +13,27 @@ function factory(dependencies) {
//----------------------------------------------------------------------
/**
* @param {string} scrollTop
* @param {integer} scrollHeight
*/
saveThreadCacheScrollHeightAsInitial(scrollHeight) {
if (!this.threadCache) {
return;
}
if (this.chatter) {
// Initial scroll height is disabled for chatter because it is
// too complex to handle correctly and less important
// functionally.
return;
}
this.update({
threadCacheInitialScrollHeights: Object.assign({}, this.threadCacheInitialScrollHeights, {
[this.threadCache.localId]: scrollHeight,
}),
});
}
/**
* @param {integer} scrollTop
*/
saveThreadCacheScrollPositionsAsInitial(scrollTop) {
if (!this.threadCache) {
@@ -230,6 +250,15 @@ function factory(dependencies) {
'thread',
],
}),
/**
* Determines the initial scroll height of thread caches, which is the
* scroll height at the time the last scroll position was saved.
* Useful to only restore scroll position when the corresponding height
* is available, otherwise the restore makes no sense.
*/
threadCacheInitialScrollHeights: attr({
default: {},
}),
/**
* Determines the initial scroll positions of thread caches.
* Useful to restore scroll position on changing back to this
addons/mail/static/tests/helpers/mock_server.js
+11 -3
View File
@@ -743,12 +743,16 @@ MockServer.include({
['is_read', '=', false],
['mail_message_id', 'in', messages.map(message => message.id)],
]).length;
return Object.assign({}, channel, {
const res = Object.assign({}, channel, {
info: extra_info,
last_message_id: lastMessageId,
members,
message_needaction_counter: messageNeedactionCounter,
});
if (channel.channel_type === 'channel') {
delete res.members;
}
return res;
});
},
/**
@@ -983,10 +987,12 @@ MockServer.include({
} else {
partner_id = this.currentPartnerId;
}
const partner = this._getRecords('res.partner', [['id', '=', partner_id]]);
const data = {
'info': 'typing_status',
'is_typing': is_typing,
'partner_id': partner_id,
'partner_name': partner.name,
};
const notifications = [];
for (const channel of channels) {
@@ -1714,9 +1720,11 @@ MockServer.include({
{ active_test: false }
)[0];
return {
"id": partner.id,
"display_name": partner.display_name,
"active": partner.active,
"display_name": partner.display_name,
"id": partner.id,
"im_status": partner.im_status,
"name": partner.name,
};
},
});