[FIX] mail, im_livechat: improve performance of discuss sidebar item

Avoid iterating over all channels every time a new channel is added.

Part of task-2702450

closes odoo/odoo#81235

X-original-commit: f5958f7ae15acff6a1ab6fc1bdf795b4c1f1e071
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
This commit is contained in:
Sébastien Theys
2021-12-10 14:33:43 +00:00
parent bf460685fc
commit e80af1ce11
10 changed files with 325 additions and 116 deletions
@@ -3,7 +3,7 @@
<t t-inherit="mail.DiscussSidebar" t-inherit-mode="extension">
<xpath expr="//*[@name='beforeCategoryChat']" position="before">
<t t-set="categoryLivechat" t-value="messaging.discuss.categoryLivechat"/>
<t t-if="categoryLivechat and categoryLivechat.selectedChannels.length">
<t t-if="categoryLivechat and categoryLivechat.categoryItems.length">
<DiscussSidebarCategory
className="'o_DiscussSidebar_category o_DiscussSidebar_categoryLivechat'"
categoryLocalId="categoryLivechat.localId"
@@ -99,4 +99,14 @@ patchRecordMethods('mail.thread', {
_computeIsChatChannel() {
return this.channel_type === 'livechat' || this._super();
},
/**
* @override
*/
_getDiscussSidebarCategory() {
switch (this.channel_type) {
case 'livechat':
return this.messaging.discuss.categoryLivechat;
}
return this._super();
}
});
@@ -39,7 +39,7 @@
/>
</div>
</t>
<t t-foreach="category.categoryItems" t-as="item" t-key="item.localId">
<t t-foreach="category.filteredCategoryItems" t-as="item" t-key="item.localId">
<DiscussSidebarCategoryItem
className="'o_DiscussSidebarCategory_item'"
categoryItemLocalId="item.localId"
@@ -24,6 +24,7 @@ export class ModelField {
related,
relationType,
required = false,
sort,
to,
} = {}) {
/**
@@ -97,6 +98,26 @@ export class ModelField {
* null or empty string are NOT considered empty value, meaning these values meet the requirement.
*/
this.required = required;
/**
* Determines the name of the function on record that returns the
* definition on how this field is sorted (only makes sense for
* relational x2many).
*
* It must contain a function returning the definition instead of the
* definition directly (to allow the definition to depend on the value
* of another field).
*
* The definition itself should be a list of operations, and each
* operation itself should be a list of 2 elements: the first is the
* name of a supported compare method, and the second is a dot separated
* relation path, starting from the current record.
*
* When determining the order of one record compared to another, each
* compare operation will be applied in the order provided, stopping at
* the first operation that is able to determine an order for the two
* records.
*/
this.sort = sort;
/**
* This prop only makes sense in a relational field. Determine which
* model name this relation refers to.
@@ -0,0 +1,156 @@
/** @odoo-module */
import { Listener } from '@mail/model/model_listener';
import { followRelations } from '@mail/model/model_utils';
/**
* Defines a set containing the relation records of the given field on the given
* record. The behavior of this set depends on the field properties.
*/
export class RelationSet {
/**
* @param {mail.model} record
* @param {ModelField} field
*/
constructor(record, field) {
this.record = record;
this.field = field;
this.set = new Set();
if (this.field.sort) {
this.sortArray = new Array();
this.sortListenerByValue = new Map();
}
}
/**
* @returns {integer}
*/
get size() {
return this.set.size;
}
/**
* @param {*} value
*/
add(value) {
if (this.set.has(value)) {
return;
}
this.set.add(value);
if (this.field.sort) {
this.sortArray.push(value);
const listener = new Listener({
isPartOfUpdateCycle: true,
name: `sort of ${value} in ${this.field} of ${this.record}`,
onChange: info => {
// access all useful values of current record (and relations) to mark them as dependencies
this.record.modelManager.startListening(listener);
const compareDefinition = this.record[this.field.sort]();
const relatedPathSet = new Set(compareDefinition.map(operation => operation[1])); // only keep unique paths to avoid unnecessary listeners
for (const relatedPath of relatedPathSet) {
followRelations(value, relatedPath);
}
this.record.modelManager.stopListening(listener);
// sort outside of listening to avoid registering listeners for all other items (they already added their own listeners)
const compareFunction = (a, b) => {
for (const [compareMethod, relatedPath] of compareDefinition) {
const valA = followRelations(a, relatedPath);
const valB = followRelations(b, relatedPath);
switch (compareMethod) {
case 'defined-first': {
if (!valA && !valB) {
return 0;
}
if (!valA) {
return 1;
}
if (!valB) {
return -1;
}
break;
}
case 'case-insensitive-asc':
if (valA.toLowerCase() === valB.toLowerCase()) {
break;
}
return valA.toLowerCase() < valB.toLowerCase() ? -1 : 1;
case 'smaller-first':
if (valA === valB) {
break;
}
return valA - valB;
case 'greater-first':
if (valA === valB) {
break;
}
return valB - valA;
default:
throw Error(`sort compare method "${compareMethod}" is not supported.`);
}
}
return 0;
};
// Naive method: re-sort the complete array every time. Ideally each item should
// be inserted/moved at its correct place immediately, but this can be optimized
// eventually if necessary.
this.sortArray.sort(compareFunction);
// Similarly naive approach: the field is marked as changed even if sort didn't
// actually move any record.
this.record.modelManager._markRecordFieldAsChanged(this.record, this.field);
},
});
this.sortListenerByValue.set(value, listener);
listener.onChange({ reason: 'initial call' });
}
}
/**
* Removes all elements.
*/
clear() {
for (const record of this.set) {
this.delete(record);
}
}
/**
* @param {*} value
* @returns {boolean} whether the value was present
*/
delete(value) {
const wasPresent = this.set.delete(value);
if (!wasPresent) {
return false;
}
if (this.field.sort) {
// remove sort of current value
const index = this.sortArray.indexOf(value);
this.sortArray.splice(index, 1);
// remove listener on current value
const listener = this.sortListenerByValue.get(value);
this.sortListenerByValue.delete(listener);
this.record.modelManager.removeListener(listener);
}
return true;
}
/**
* @param {*} value
* @returns {boolean} whether the value is present
*/
has(value) {
return this.set.has(value);
}
/**
* @returns {iterator}
*/
[Symbol.iterator]() {
if (this.field.sort) {
return this.sortArray[Symbol.iterator]();
}
return this.set[Symbol.iterator]();
}
}
+26 -19
View File
@@ -3,7 +3,9 @@
import { registry } from '@mail/model/model_core';
import { ModelField } from '@mail/model/model_field';
import { FieldCommand, unlinkAll } from '@mail/model/model_field_command';
import { RelationSet } from '@mail/model/model_field_relation_set';
import { Listener } from '@mail/model/model_listener';
import { followRelations } from '@mail/model/model_utils';
import { makeDeferred } from '@mail/utils/deferred/deferred';
/**
@@ -506,6 +508,7 @@ export class ModelManager {
'related',
'relationType',
'required',
'sort',
'to',
].includes(key)
);
@@ -521,6 +524,9 @@ export class ModelManager {
if (field.required && !(['one2one', 'many2one'].includes(field.relationType))) {
throw new Error(`Relational field(${fieldName}) on ${Model} has "required" true with a relation of type "${field.relationType}" but "required" is only supported for "one2one" and "many2one".`);
}
if (field.sort && !(['one2many', 'many2many'].includes(field.relationType))) {
throw new Error(`Relational field "${Model}/${fieldName}" has "sort" with a relation of type "${field.relationType}" but "sort" is only supported for "one2many" and "many2many".`);
}
}
// 3. Computed field.
if (field.compute && !(typeof field.compute === 'string')) {
@@ -618,7 +624,7 @@ export class ModelManager {
throw new Error(`${field} on ${Model} defines its inverse as field(${field.inverse}) on ${RelatedModel}, but it does not exist.`);
}
if (inverseField.inverse !== fieldName) {
throw new Error(`The name of ${field} on ${Model} does not match with the name defined in its inverse ${inverseField} on ${RelatedModel}.`)
throw new Error(`The name of ${field} on ${Model} does not match with the name defined in its inverse ${inverseField} on ${RelatedModel}.`);
}
if (![Model.name, 'mail.model'].includes(inverseField.to)) {
throw new Error(`${field} on ${Model} has its inverse ${inverseField} on ${RelatedModel} referring to an invalid model (model(${inverseField.to})).`);
@@ -693,7 +699,7 @@ export class ModelManager {
record.__values[field.fieldName] = undefined;
if (field.fieldType === 'relation') {
if (['one2many', 'many2many'].includes(field.relationType)) {
record.__values[field.fieldName] = new Set();
record.__values[field.fieldName] = new RelationSet(record, field);
}
}
}
@@ -885,16 +891,7 @@ export class ModelManager {
onChange: (info) => {
this.startListening(listener);
for (const dependency of onChange.dependencies) {
let target = record;
for (const field of dependency.split('.')) {
if (!target.constructor.__fieldMap[field]) {
throw Error(`onChange dependency "${field}" does not exist on ${record.constructor}.`);
}
target = target[field];
if (!target) {
break;
}
}
followRelations(record, dependency);
}
this.stopListening(listener);
record[onChange.methodName]();
@@ -1108,6 +1105,22 @@ export class ModelManager {
}
}
/**
* Marks the given field of the given record as changed.
*
* @param {mail.model} record
* @param {ModelField} field
*/
_markRecordFieldAsChanged(record, field) {
for (const [listener, infoList] of this._listenersObservingFieldOfLocalId.get(record.localId).get(field)) {
this._markListenerToNotify(listener, {
listener,
reason: `_update: ${field} of ${record}`,
infoList,
});
}
}
/**
* Notifies the listeners that have been flagged to be notified and for
* which the `onChange` function was defined to be called after the update
@@ -1304,13 +1317,7 @@ export class ModelManager {
throw new Error(`read-only ${field} on ${record} was updated`);
}
hasChanged = true;
for (const [listener, infoList] of this._listenersObservingFieldOfLocalId.get(record.localId).get(field)) {
this._markListenerToNotify(listener, {
listener,
reason: `_update: ${field} of ${record}`,
infoList,
});
}
this._markRecordFieldAsChanged(record, field);
}
if (hasChanged) {
this._updatedRecordsCheckRequired.add(record);
@@ -0,0 +1,24 @@
/** @odoo-module **/
/**
* Follows the given related path starting from the given record, and returns
* the resulting value, or undefined if a relation can't be followed because it
* is undefined.
*
* @param {mail.model} record
* @param {string} relatedPath field names, dot separated to follow relations
* @returns {any}
*/
export function followRelations(record, relatedPath) {
let target = record;
for (const field of relatedPath.split('.')) {
if (!target.constructor.__fieldMap[field]) {
throw Error(`field(${field}) does not exist on ${target}`);
}
target = target[field];
if (!target) {
break;
}
}
return target;
}
@@ -73,22 +73,6 @@ registerModel({
}
return clear();
},
/**
* @private
* @returns {mail.discuss_sidebar_category_item[]}
*/
_computeCategoryItems() {
let channels = this.selectedSortedChannels;
const searchValue = this.messaging.discuss.sidebarQuickSearchValue;
if (searchValue) {
const qsVal = searchValue.toLowerCase();
channels = channels.filter(t => {
const nameVal = t.displayName.toLowerCase();
return nameVal.includes(qsVal);
});
}
return insertAndReplace(channels.map(c => ({ channel: replace(c) })));
},
/**
* @private
* @returns {integer}
@@ -96,11 +80,27 @@ registerModel({
_computeCounter() {
switch (this.counterComputeMethod) {
case 'needaction':
return this.selectedChannels.filter(thread => thread.message_needaction_counter > 0).length;
return this.categoryItems.filter(categoryItem => categoryItem.channel.message_needaction_counter > 0).length;
case 'unread':
return this.selectedChannels.filter(thread => thread.localMessageUnreadCounter > 0).length;
return this.categoryItems.filter(categoryItem => categoryItem.channel.localMessageUnreadCounter > 0).length;
}
},
/**
* @private
* @returns {FieldCommand}
*/
_computeFilteredCategoryItems() {
let categoryItems = this.categoryItems;
const searchValue = this.messaging.discuss.sidebarQuickSearchValue;
if (searchValue) {
const qsVal = searchValue.toLowerCase();
categoryItems = categoryItems.filter(categoryItem => {
const nameVal = categoryItem.channel.displayName.toLowerCase();
return nameVal.includes(qsVal);
});
}
return replace(categoryItems);
},
/**
* @private
* @returns {boolean}
@@ -110,68 +110,26 @@ registerModel({
},
/**
* @private
* @returns {mail.thread[]}
* @returns {Array[]}
*/
_computeSelectedChannels() {
return replace(this.messaging.models['mail.thread'].all(thread =>
thread.model === 'mail.channel' &&
thread.isPinned &&
this.supportedChannelTypes.includes(thread.channel_type))
);
},
/**
*
* @private
* @returns {mail.thread[]}
*/
_computeSelectedSortedChannels() {
_sortDefinitionCategoryItems() {
switch (this.sortComputeMethod) {
case 'name':
return replace(this._sortByDisplayName());
return [
['defined-first', 'channel'],
['defined-first', 'channel.displayName'],
['case-insensitive-asc', 'channel.displayName'],
['smaller-first', 'channel.id'],
];
case 'last_action':
return replace(this._sortByLastInterestDateTime());
return [
['defined-first', 'channel'],
['defined-first', 'channel.lastInterestDateTime'],
['greater-first', 'channel.lastInterestDateTime'],
['greater-first', 'channel.id'],
];
}
},
/**
* Sorts `selectedChannels` by `displayName` in
* case-insensitive alphabetical order.
*
* @private
* @returns {mail.thread[]}
*/
_sortByDisplayName() {
return this.selectedChannels.sort((t1, t2) => {
if (t1.displayName && !t2.displayName) {
return -1;
} else if (!t1.displayName && t2.displayName) {
return 1;
} else if (t1.displayName && t2.displayName && t1.displayName !== t2.displayName) {
return t1.displayName.toLowerCase() < t2.displayName.toLowerCase() ? -1 : 1;
} else {
return t1.id - t2.id;
}
});
},
/**
* Sorts `selectedChannels` by `lastInterestDateTime`.
* The most recent one will come first.
*
* @private
* @returns {mail.thread[]}
*/
_sortByLastInterestDateTime() {
return this.selectedChannels.sort((t1, t2) => {
if (t1.lastInterestDateTime && !t2.lastInterestDateTime) {
return -1;
} else if (!t1.lastInterestDateTime && t2.lastInterestDateTime) {
return 1;
} else if (t1.lastInterestDateTime && t2.lastInterestDateTime && t1.lastInterestDateTime !== t2.lastInterestDateTime) {
return t2.lastInterestDateTime - t1.lastInterestDateTime;
} else {
return t2.id - t1.id;
}
});
},
/**
* Changes the category open states when clicked.
*/
@@ -270,13 +228,13 @@ registerModel({
*/
commandAddTitleText: attr(),
/**
* The sorted category items which belong to the category.
* These items are also filtered by `sidebarQuickSearchValue`.
* Determines the discuss sidebar category items that are displayed by
* this discuss sidebar category.
*/
categoryItems: one2many('mail.discuss_sidebar_category_item', {
compute: '_computeCategoryItems',
inverse: 'category',
isCausal: true,
sort: '_sortDefinitionCategoryItems',
}),
/**
* The total amount unread/action-needed threads in the category.
@@ -299,6 +257,14 @@ registerModel({
inverse: 'categoryChat',
readonly: true,
}),
/**
* Determines the filtered and sorted discuss sidebar category items
* that are displayed by this discuss sidebar category.
*/
filteredCategoryItems: one2many('mail.discuss_sidebar_category_item', {
compute: '_computeFilteredCategoryItems',
readonly: true,
}),
/**
* Display name of the category.
*/
@@ -343,19 +309,6 @@ registerModel({
* The placeholder text used when a new item is being added in UI.
*/
newItemPlaceholderText: attr(),
/**
* Channels which belong to the category,
*/
selectedChannels: one2many('mail.thread', {
compute: '_computeSelectedChannels',
}),
/**
* Channels which belongs to the category,
* and sorted based on the `supported_channel_type`.
*/
selectedSortedChannels: one2many('mail.thread', {
compute: '_computeSelectedSortedChannels',
}),
/**
* The key used in the server side for the category state
*/
@@ -1,9 +1,8 @@
/** @odoo-module **/
import { registerModel } from '@mail/model/model_core';
import { attr, many2one } from '@mail/model/model_field';
import { attr, many2one, one2one } from '@mail/model/model_field';
import { clear, link } from '@mail/model/model_field_command';
import { isEventHandled } from '@mail/utils/utils';
import Dialog from 'web.Dialog';
@@ -213,7 +212,7 @@ registerModel({
compute: '_computeAvatarUrl',
}),
/**
* States the discuss sidebar category displaying this.
* Determines the discuss sidebar category displaying this item.
*/
category: many2one('mail.discuss_sidebar_category', {
inverse: 'categoryItems',
@@ -265,7 +264,7 @@ registerModel({
/**
* The related channel thread.
*/
channel: many2one('mail.thread', {
channel: one2one('mail.thread', {
inverse: 'discussSidebarCategoryItem',
readonly: true,
required: true,
+40 -1
View File
@@ -1259,6 +1259,23 @@ registerModel({
}
return unlink();
},
/**
* @private
* @returns {FieldCommand}
*/
_computeDiscussSidebarCategoryItem() {
if (this.model !== 'mail.channel') {
return clear();
}
if (!this.isPinned) {
return clear();
}
const discussSidebarCategory = this._getDiscussSidebarCategory();
if (!discussSidebarCategory) {
return clear();
}
return insertAndReplace({ category: replace(discussSidebarCategory) });
},
/**
* @private
* @returns {string}
@@ -1705,6 +1722,22 @@ registerModel({
_computeVideoCount() {
return this.rtcSessions.filter(session => session.videoStream).length;
},
/**
* Returns the discuss sidebar category that corresponds to this channel
* type.
*
* @private
* @returns {mail.discuss_sidebar_category}
*/
_getDiscussSidebarCategory() {
switch (this.channel_type) {
case 'channel':
return this.messaging.discuss.categoryChannel;
case 'chat':
case 'group':
return this.messaging.discuss.categoryChat;
}
},
/**
* @private
* @param {Object} param0
@@ -1941,9 +1974,15 @@ registerModel({
* States the description of this thread. Only applies to channels.
*/
description: attr(),
discussSidebarCategoryItem: one2many('mail.discuss_sidebar_category_item', {
/**
* Determines the discuss sidebar category item that displays this
* thread (if any). Only applies to channels.
*/
discussSidebarCategoryItem: one2one('mail.discuss_sidebar_category_item', {
compute: '_computeDiscussSidebarCategoryItem',
inverse: 'channel',
isCausal: true,
readonly: true,
}),
displayName: attr({
compute: '_computeDisplayName',