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

[FIX] mail: fix discuss search filters and related issues

Browse Source 
- don't reset `stringifiedDomain` when changing thread (this led to the
  control panel displaying obsolete filters)
- refresh non-main cache that are currently displayed when there is a known
  change of messages in the same thread's main cache
- refresh non-main-cache on displaying them, to fetch potentially new messages
- fix discuss "select all" conditions

task-2171873
task-2282374

closes odoo/odoo#59644

X-original-commit: b89dc5f949b3ad062fc4197a4152066c207e1bf9
Related: odoo/enterprise#13962
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Sébastien Theys
2020-10-09 12:56:24 +00:00
parent 4c2a03aa8f
commit a8dad56d6b
11 changed files with 524 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/static/src/components/discuss/tests/discuss_domain_tests.js
+407
View File
@@ -0,0 +1,407 @@
odoo.define('mail/static/src/components/discuss/tests/discuss_domain_tests.js', function (require) {
'use strict';
const {
afterEach,
afterNextRender,
beforeEach,
start,
} = require('mail/static/src/utils/test_utils.js');
QUnit.module('mail', {}, function () {
QUnit.module('components', {}, function () {
QUnit.module('discuss', {}, function () {
QUnit.module('discuss_domain_tests.js', {
beforeEach() {
beforeEach(this);
this.start = async params => {
const { afterEvent, env, widget } = await start(Object.assign({}, params, {
autoOpenDiscuss: true,
data: this.data,
hasDiscuss: true,
}));
this.afterEvent = afterEvent;
this.env = env;
this.widget = widget;
};
},
afterEach() {
afterEach(this);
},
});
QUnit.test('discuss should filter messages based on given domain', async function (assert) {
assert.expect(2);
this.data['mail.message'].records.push({
body: "test",
needaction: true,
needaction_partner_ids: [this.data.currentPartnerId],
}, {
body: "not empty",
needaction: true,
needaction_partner_ids: [this.data.currentPartnerId],
});
await this.start();
assert.containsN(
document.body,
'.o_Message',
2,
"should have 2 messages in Inbox initially"
);
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
// simulate control panel search
this.env.messaging.discuss.update({
stringifiedDomain: JSON.stringify([['body', 'ilike', 'test']]),
});
},
message: "should wait until search filter is applied",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.box' &&
threadViewer.thread.id === 'inbox'
);
},
});
assert.containsOnce(
document.body,
'.o_Message',
"should only have the 1 message containing 'test' remaining after doing a search"
);
});
QUnit.test('discuss should keep filter domain on changing thread', async function (assert) {
assert.expect(3);
this.data['mail.channel'].records.push({ id: 20 });
this.data['mail.message'].records.push({
body: "test",
channel_ids: [20],
}, {
body: "not empty",
channel_ids: [20],
});
await this.start();
const channel = this.env.models['mail.thread'].findFromIdentifyingData({
id: 20,
model: 'mail.channel',
});
assert.containsNone(
document.body,
'.o_Message',
"should have no message in Inbox initially"
);
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
// simulate control panel search
this.env.messaging.discuss.update({
stringifiedDomain: JSON.stringify([['body', 'ilike', 'test']]),
});
},
message: "should wait until search filter is applied",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.box' &&
threadViewer.thread.id === 'inbox'
);
},
});
assert.containsNone(
document.body,
'.o_Message',
"should have still no message in Inbox after doing a search"
);
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${channel.localId}"]
`).click();
},
message: "should wait until channel 20 is loaded after clicking on it",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20
);
},
});
assert.containsOnce(
document.body,
'.o_Message',
"should only have the 1 message containing 'test' in channel 20 (due to the domain still applied on changing thread)"
);
});
QUnit.test('discuss should refresh filtered thread on receiving new message', async function (assert) {
assert.expect(2);
this.data['mail.channel'].records.push({ id: 20 });
await this.start();
const channel = this.env.models['mail.thread'].findFromIdentifyingData({
id: 20,
model: 'mail.channel',
});
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${channel.localId}"]
`).click();
},
message: "should wait until channel 20 is loaded after clicking on it",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20
);
},
});
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
// simulate control panel search
this.env.messaging.discuss.update({
stringifiedDomain: JSON.stringify([['body', 'ilike', 'test']]),
});
},
message: "should wait until search filter is applied",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20
);
},
});
assert.containsNone(
document.body,
'.o_Message',
"should have initially no message in channel 20 matching the search 'test'"
);
// simulate receiving a message
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => this.env.services.rpc({
route: '/mail/chat_post',
params: {
uuid: channel.uuid,
message_content: "test",
},
}),
message: "should wait until channel 20 refreshed its filtered message list",
predicate: data => {
return (
data.threadViewer.thread.model === 'mail.channel' &&
data.threadViewer.thread.id === 20 &&
data.hint.type === 'messages-loaded'
);
},
});
assert.containsOnce(
document.body,
'.o_Message',
"should only have the 1 message containing 'test' in channel 20 after just receiving it"
);
});
QUnit.test('discuss should refresh filtered thread on changing thread', async function (assert) {
assert.expect(4);
this.data['mail.channel'].records.push({ id: 20 }, { id: 21 });
await this.start();
const channel20 = this.env.models['mail.thread'].findFromIdentifyingData({
id: 20,
model: 'mail.channel',
});
const channel21 = this.env.models['mail.thread'].findFromIdentifyingData({
id: 21,
model: 'mail.channel',
});
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${channel20.localId}"]
`).click();
},
message: "should wait until channel 20 is loaded after clicking on it",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20
);
},
});
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
// simulate control panel search
this.env.messaging.discuss.update({
stringifiedDomain: JSON.stringify([['body', 'ilike', 'test']]),
});
},
message: "should wait until search filter is applied",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20
);
},
});
assert.containsNone(
document.body,
'.o_Message',
"should have initially no message in channel 20 matching the search 'test'"
);
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${channel21.localId}"]
`).click();
},
message: "should wait until channel 21 is loaded after clicking on it",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 21
);
},
});
assert.containsNone(
document.body,
'.o_Message',
"should have no message in channel 21 matching the search 'test'"
);
// simulate receiving a message on channel 20 while channel 21 is displayed
await this.env.services.rpc({
route: '/mail/chat_post',
params: {
uuid: channel20.uuid,
message_content: "test",
},
});
assert.containsNone(
document.body,
'.o_Message',
"should still have no message in channel 21 matching the search 'test' after receiving a message on channel 20"
);
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${channel20.localId}"]
`).click();
},
message: "should wait until channel 20 is loaded with the new message after clicking on it",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 20 &&
threadViewer.threadCache.fetchedMessages.length === 1
);
},
});
assert.containsOnce(
document.body,
'.o_Message',
"should now have the 1 message containing 'test' in channel 20 when displaying it, after having received the message while the channel was not visible"
);
});
QUnit.test('select all and unselect all buttons should work on filtered thread', async function (assert) {
assert.expect(4);
this.data['mail.channel'].records.push({
id: 20,
is_moderator: true,
moderation: true,
name: "general",
});
this.data['mail.message'].records.push({
body: "<p>test</p>",
model: 'mail.channel',
moderation_status: 'pending_moderation',
res_id: 20,
});
await this.start();
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
document.querySelector(`
.o_DiscussSidebar_item[data-thread-local-id="${this.env.messaging.moderation.localId}"]
`).click();
},
message: "should wait until moderation box is loaded after clicking on it",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.box' &&
threadViewer.thread.id === 'moderation'
);
},
});
await this.afterEvent({
eventName: 'o-thread-view-hint-processed',
func: () => {
// simulate control panel search
this.env.messaging.discuss.update({
stringifiedDomain: JSON.stringify([['body', 'ilike', 'test']]),
});
},
message: "should wait until search filter is applied",
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.box' &&
threadViewer.thread.id === 'moderation'
);
},
});
assert.containsOnce(
document.body,
'.o_Message',
"should only have the 1 message containing 'test' in moderation box"
);
assert.notOk(
document.querySelector('.o_Message_checkbox').checked,
"the moderation checkbox should not be checked initially"
);
await afterNextRender(() => document.querySelector('.o_widget_Discuss_controlPanelButtonSelectAll').click());
assert.ok(
document.querySelector('.o_Message_checkbox').checked,
"the moderation checkbox should be checked after clicking on 'select all'"
);
await afterNextRender(() => document.querySelector('.o_widget_Discuss_controlPanelButtonUnselectAll').click());
assert.notOk(
document.querySelector('.o_Message_checkbox').checked,
"the moderation checkbox should be unchecked after clicking on 'unselect all'"
);
});
});
});
});
});
addons/mail/static/src/components/message_list/message_list.js
+6 -3
View File
@@ -124,12 +124,18 @@ class MessageList extends Component {
case 'home-menu-shown':
this._adjustFromHomeMenuShown(hint);
break;
case 'messages-loaded':
this.threadView.markComponentHintProcessed(hint);
break;
case 'message-received':
this._adjustFromMessageReceived(hint);
break;
case 'more-messages-loaded':
this._adjustFromMoreMessagesLoaded(hint);
break;
case 'new-messages-loaded':
this.threadView.markComponentHintProcessed(hint);
break;
}
}
this._willPatchSnapshot = undefined;
@@ -333,9 +339,6 @@ class MessageList extends Component {
isProcessed = true;
}
if (isProcessed) {
this.env.messagingBus.trigger('o-component-message-list-thread-cache-changed', {
threadViewer: this.threadView.threadViewer,
});
this.threadView.markComponentHintProcessed(hint);
}
}
addons/mail/static/src/components/thread_view/thread_view_tests.js
+7 -3
View File
@@ -741,13 +741,17 @@ QUnit.test('basic rendering of canceled notification', async function (assert) {
}]],
});
await this.afterEvent({
eventName: 'o-component-message-list-thread-cache-changed',
eventName: 'o-thread-view-hint-processed',
func: () => {
this.createThreadViewComponent(threadViewer.threadView);
},
message: "thread become loaded with messages",
predicate: ({ threadViewer }) => {
return threadViewer.thread.model === 'mail.channel' && threadViewer.thread.id === 11;
predicate: ({ hint, threadViewer }) => {
return (
hint.type === 'messages-loaded' &&
threadViewer.thread.model === 'mail.channel' &&
threadViewer.thread.id === 11
);
},
});
addons/mail/static/src/models/discuss/discuss.js
+1 -1
View File
@@ -175,7 +175,6 @@ function factory(dependencies) {
*/
async openThread(thread) {
this.update({
stringifiedDomain: '[]',
thread: [['link', thread]],
});
this.focus();
@@ -509,6 +508,7 @@ function factory(dependencies) {
}),
/**
* Determines the domain to apply when fetching messages for `this.thread`.
* This value should only be written by the control panel.
*/
stringifiedDomain: attr({
default: '[]',
addons/mail/static/src/models/messaging_menu/messaging_menu.js
+2 -15
View File
@@ -50,16 +50,11 @@ function factory(dependencies) {
return;
}
const inbox = this.env.messaging.inbox;
if (
!inbox ||
!inbox.mainCache ||
inbox.mainCache.isLoaded ||
inbox.mainCache.isLoading
) {
if (!inbox || !inbox.mainCache) {
return;
}
// populate some needaction messages on threads.
inbox.mainCache.update({ hasToLoadMessages: true });
inbox.mainCache.update({ isCacheRefreshRequested: true });
}
/**
@@ -127,8 +122,6 @@ function factory(dependencies) {
'isOpen',
'messagingInbox',
'messagingInboxMainCache',
'messagingInboxMainCacheIsLoaded',
'messagingInboxMainCacheIsLoading',
],
}),
/**
@@ -152,12 +145,6 @@ function factory(dependencies) {
messagingInboxMainCache: one2one('mail.thread_cache', {
related: 'messagingInbox.mainCache',
}),
messagingInboxMainCacheIsLoaded: attr({
related: 'messagingInboxMainCache.isLoaded',
}),
messagingInboxMainCacheIsLoading: attr({
related: 'messagingInboxMainCache.isLoading',
}),
};
MessagingMenu.modelName = 'mail.messaging_menu';
addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
-6
View File
@@ -551,8 +551,6 @@ function factory(dependencies) {
const inboxMailbox = this.env.messaging.inbox;
// 1. move messages from inbox to history
// AKU TODO: flag other caches to invalidate
// task-2171873
for (const message_id of message_ids) {
// We need to ignore all not yet known messages because we don't want them
// to be shown partially as they would be linked directly to mainCache
@@ -625,10 +623,6 @@ function factory(dependencies) {
continue;
}
message.update({ isStarred: starred });
if (!starred) {
// AKU TODO: flag starred other caches for invalidation
// task-2171873
}
starredMailbox.update({
counter: starred
? starredMailbox.counter + 1
addons/mail/static/src/models/thread/thread.js
+3 -3
View File
@@ -1803,6 +1803,9 @@ function factory(dependencies) {
suggestedRecipientInfoList: one2many('mail.suggested_recipient_info', {
inverse: 'thread',
}),
threadViews: one2many('mail.thread_view', {
inverse: 'thread',
}),
/**
* States the `mail.activity` that belongs to `this` and that are due
* specifically today.
@@ -1825,9 +1828,6 @@ function factory(dependencies) {
dependencies: ['orderedOtherTypingMembers'],
}),
uuid: attr(),
threadViews: one2many('mail.thread_view', {
inverse: 'thread',
}),
};
Thread.modelName = 'mail.thread';
addons/mail/static/src/models/thread_cache/thread_cache.js
+92 -15
View File
@@ -20,7 +20,7 @@ function factory(dependencies) {
return;
}
if (!this.isLoaded) {
this.update({ hasToLoadMessages: true });
this.update({ isCacheRefreshRequested: true });
return;
}
this.update({ isLoadingMore: true });
@@ -31,7 +31,7 @@ function factory(dependencies) {
limit,
}));
for (const threadView of this.threadViews) {
threadView.addComponentHint('more-messages-loaded');
threadView.addComponentHint('more-messages-loaded', { fetchedMessages });
}
this.update({ isLoadingMore: false });
if (fetchedMessages.length < limit) {
@@ -48,14 +48,18 @@ function factory(dependencies) {
return;
}
if (!this.isLoaded) {
this.update({ hasToLoadMessages: true });
this.update({ isCacheRefreshRequested: true });
return;
}
const messageIds = this.fetchedMessages.map(message => message.id);
return this._loadMessages({
const fetchedMessages = this._loadMessages({
extraDomain: [['id', '>', Math.max(...messageIds)]],
limit: false,
});
for (const threadView of this.threadViews) {
threadView.addComponentHint('new-messages-loaded', { fetchedMessages });
}
return fetchedMessages;
}
//----------------------------------------------------------------------
@@ -140,8 +144,6 @@ function factory(dependencies) {
}
let messages = this.fetchedMessages;
if (this.stringifiedDomain !== '[]') {
// AKU TODO: flag for invalidation if there are newer messages
// in thread. task-2171873
return [['replace', messages]];
}
// main cache: adjust with newer messages
@@ -178,13 +180,34 @@ function factory(dependencies) {
* @returns {boolean}
*/
_computeHasToLoadMessages() {
return (
this.thread &&
!this.thread.isTemporary &&
this.threadViews.length > 0 &&
!this.isLoaded &&
!this.isLoading
);
if (!this.thread) {
// happens during destroy or compute executed in wrong order
return false;
}
const wasCacheRefreshRequested = this.isCacheRefreshRequested;
// mark hint as processed
this.update({ isCacheRefreshRequested: false });
if (this.thread.isTemporary) {
// temporary threads don't exist on the server
return false;
}
if (!wasCacheRefreshRequested && this.threadViews.length === 0) {
// don't load message that won't be used
return false;
}
if (!wasCacheRefreshRequested && (this.isLoaded || this.isLoading)) {
// avoid duplicate RPC
return false;
}
const isMainCache = this.thread.mainCache === this;
if (isMainCache && (this.isLoaded || this.isLoading)) {
// Ignore request on the main cache if it is already loaded or
// loading. Indeed the main cache is automatically sync with
// server updates already, so there is never a need to refresh
// it past the first time.
return false;
}
return true;
}
/**
@@ -279,7 +302,31 @@ function factory(dependencies) {
if (!this.hasToLoadMessages) {
return;
}
this._loadMessages();
this._loadMessages().then(fetchedMessages => {
for (const threadView of this.threadViews) {
threadView.addComponentHint('messages-loaded', { fetchedMessages });
}
});
}
/**
* Handles change of messages on this thread cache. This is useful to
* refresh non-main caches that are currently displayed when the main
* cache receives updates. This is necessary because only the main cache
* is aware of changes in real time.
*/
_onMessagesChanged() {
if (!this.thread) {
return;
}
if (this.thread.mainCache !== this) {
return;
}
for (const threadView of this.thread.threadViews) {
if (threadView.threadCache) {
threadView.threadCache.update({ isCacheRefreshRequested: true });
}
}
}
}
@@ -315,15 +362,19 @@ function factory(dependencies) {
dependencies: ['threadMessages'],
}),
/**
* Determines whether `this` should load initial messages.
* Determines whether `this` should load initial messages. This field is
* computed and should be considered read-only.
* @see `isCacheRefreshRequested` to request manual refresh of messages.
*/
hasToLoadMessages: attr({
compute: '_computeHasToLoadMessages',
dependencies: [
'isCacheRefreshRequested',
'isLoaded',
'isLoading',
'thread',
'threadIsTemporary',
'threadMainCache',
'threadViews',
],
}),
@@ -339,6 +390,14 @@ function factory(dependencies) {
isLoadingMore: attr({
default: false,
}),
/**
* Determines whether `this` should consider refreshing its messages.
* This field is a hint that may or may not lead to an actual refresh.
* @see `hasToLoadMessages`
*/
isCacheRefreshRequested: attr({
default: false,
}),
/**
* Last message that has been fetched by this thread cache.
*
@@ -378,6 +437,18 @@ function factory(dependencies) {
'hasToLoadMessages',
],
}),
/**
* Not a real field, used to trigger `_onMessagesChanged` when one of
* the dependencies changes.
*/
onMessagesChanged: attr({
compute: '_onMessagesChanged',
dependencies: [
'messages',
'thread',
'threadMainCache',
],
}),
/**
* Ordered list of messages that have been fetched by this cache.
*
@@ -408,6 +479,12 @@ function factory(dependencies) {
threadIsTemporary: attr({
related: 'thread.isTemporary',
}),
/**
* Serves as compute dependency.
*/
threadMainCache: many2one('mail.thread_cache', {
related: 'thread.mainCache',
}),
threadMessages: many2many('mail.message', {
related: 'thread.messages',
}),
addons/mail/static/src/models/thread_view/thread_view.js
+3
View File
@@ -118,6 +118,9 @@ function factory(dependencies) {
*/
_onThreadCacheChanged() {
this.addComponentHint('change-of-thread-cache');
if (this.threadCache) {
this.threadCache.update({ isCacheRefreshRequested: true });
}
}
/**
addons/mail/static/src/widgets/discuss/discuss.js
+2 -2
View File
@@ -361,7 +361,7 @@ const DiscussWidget = AbstractAction.extend({
_onClickSelectAll() {
this.env.models['mail.message'].checkAll(
this.discuss.thread,
this.discuss.threadStringifiedDomain
this.discuss.stringifiedDomain
);
},
/**
@@ -370,7 +370,7 @@ const DiscussWidget = AbstractAction.extend({
_onClickUnselectAll() {
this.env.models['mail.message'].uncheckAll(
this.discuss.thread,
this.discuss.threadStringifiedDomain
this.discuss.stringifiedDomain
);
},
/**
addons/mail/views/assets.xml
+1
View File
@@ -281,6 +281,7 @@
<script type="text/javascript" src="/mail/static/src/components/composer_suggestion/composer_suggestion_command_tests.js"/>
<script type="text/javascript" src="/mail/static/src/components/composer_suggestion/composer_suggestion_partner_tests.js"/>
<script type="text/javascript" src="/mail/static/src/components/dialog_manager/dialog_manager_tests.js"></script>
<script type="text/javascript" src="/mail/static/src/components/discuss/tests/discuss_domain_tests.js"/>
<script type="text/javascript" src="/mail/static/src/components/discuss/tests/discuss_inbox_tests.js"></script>
<script type="text/javascript" src="/mail/static/src/components/discuss/tests/discuss_moderation_tests.js"></script>
<script type="text/javascript" src="/mail/static/src/components/discuss/tests/discuss_pinned_tests.js"></script>