[FIX] mail: avoid useless (un)link when using replace

Side effect of this change: dependencies are not longer triggered when replace
is not actually doing anything.

task-2270780

closes odoo/odoo#59515

X-original-commit: 75a44675873d65fdf17f2205772b05a0f59bb42c
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Sébastien Theys
2020-10-08 13:21:06 +00:00
parent 217f907142
commit b503c7f9f0
5 changed files with 169 additions and 71 deletions
@@ -30,9 +30,10 @@ QUnit.module('thread_needaction_preview_tests.js', {
};
this.start = async params => {
const { env, widget } = await start(Object.assign({}, params, {
const { afterEvent, env, widget } = await start(Object.assign({}, params, {
data: this.data,
}));
this.afterEvent = afterEvent;
this.env = env;
this.widget = widget;
};
@@ -45,8 +46,22 @@ QUnit.module('thread_needaction_preview_tests.js', {
QUnit.test('mark as read', async function (assert) {
assert.expect(4);
this.data['mail.message'].records.push({
id: 21,
model: 'res.partner',
needaction: true,
needaction_partner_ids: [this.data.currentPartnerId],
res_id: 11,
});
this.data['mail.notification'].records.push({
mail_message_id: 21,
notification_status: 'sent',
notification_type: 'inbox',
res_partner_id: this.data.currentPartnerId,
});
await this.start({
hasChatWindow: true,
hasMessagingMenu: true,
async mockRPC(route, args) {
if (route.includes('set_message_done')) {
assert.step('set_message_done');
@@ -54,18 +69,14 @@ QUnit.test('mark as read', async function (assert) {
return this._super(...arguments);
},
});
const thread = this.env.models['mail.thread'].create({
id: 11,
model: 'mail.channel',
});
this.env.models['mail.message'].create({
id: 21,
isNeedaction: true,
originThread: [['link', thread]],
});
await this.createThreadNeedactionPreviewComponent({
threadLocalId: thread.localId,
});
await afterNextRender(() => this.afterEvent({
eventName: 'o-thread-cache-loaded-messages',
func: () => document.querySelector('.o_MessagingMenu_toggler').click(),
message: "should wait until inbox loaded initial needaction messages",
predicate: ({ threadCache }) => {
return threadCache.thread.model === 'mail.box' && threadCache.thread.id === 'inbox';
},
}));
assert.containsOnce(
document.body,
'.o_ThreadNeedactionPreview_markAsRead',
@@ -89,8 +100,22 @@ QUnit.test('mark as read', async function (assert) {
QUnit.test('click on preview should mark as read and open the thread', async function (assert) {
assert.expect(5);
this.data['mail.message'].records.push({
id: 21,
model: 'res.partner',
needaction: true,
needaction_partner_ids: [this.data.currentPartnerId],
res_id: 11,
});
this.data['mail.notification'].records.push({
mail_message_id: 21,
notification_status: 'sent',
notification_type: 'inbox',
res_partner_id: this.data.currentPartnerId,
});
await this.start({
hasChatWindow: true,
hasMessagingMenu: true,
async mockRPC(route, args) {
if (route.includes('set_message_done')) {
assert.step('set_message_done');
@@ -98,18 +123,14 @@ QUnit.test('click on preview should mark as read and open the thread', async fun
return this._super(...arguments);
},
});
const thread = this.env.models['mail.thread'].create({
id: 11,
model: 'mail.channel',
});
this.env.models['mail.message'].create({
id: 21,
isNeedaction: true,
originThread: [['link', thread]],
});
await this.createThreadNeedactionPreviewComponent({
threadLocalId: thread.localId,
});
await afterNextRender(() => this.afterEvent({
eventName: 'o-thread-cache-loaded-messages',
func: () => document.querySelector('.o_MessagingMenu_toggler').click(),
message: "should wait until inbox loaded initial needaction messages",
predicate: ({ threadCache }) => {
return threadCache.thread.model === 'mail.box' && threadCache.thread.id === 'inbox';
},
}));
assert.containsOnce(
document.body,
'.o_ThreadNeedactionPreview',
@@ -152,22 +173,32 @@ QUnit.test('click on expand from chat window should close the chat window and op
"should redirect to the model of the thread"
);
});
this.data['mail.message'].records.push({
id: 21,
model: 'res.partner',
needaction: true,
needaction_partner_ids: [this.data.currentPartnerId],
res_id: 11,
});
this.data['mail.notification'].records.push({
mail_message_id: 21,
notification_status: 'sent',
notification_type: 'inbox',
res_partner_id: this.data.currentPartnerId,
});
await this.start({
env: { bus },
hasChatWindow: true,
hasMessagingMenu: true,
});
const thread = this.env.models['mail.thread'].create({
id: 11,
model: 'res.partner',
});
this.env.models['mail.message'].create({
id: 21,
isNeedaction: true,
originThread: [['link', thread]],
});
await this.createThreadNeedactionPreviewComponent({
threadLocalId: thread.localId,
});
await afterNextRender(() => this.afterEvent({
eventName: 'o-thread-cache-loaded-messages',
func: () => document.querySelector('.o_MessagingMenu_toggler').click(),
message: "should wait until inbox loaded initial needaction messages",
predicate: ({ threadCache }) => {
return threadCache.thread.model === 'mail.box' && threadCache.thread.id === 'inbox';
},
}));
assert.containsOnce(
document.body,
'.o_ThreadNeedactionPreview',
@@ -206,8 +237,22 @@ QUnit.test('[technical] opening a non-channel chat window should not call channe
// window, because there is no server sync of fold state for them.
assert.expect(3);
this.data['mail.message'].records.push({
id: 21,
model: 'res.partner',
needaction: true,
needaction_partner_ids: [this.data.currentPartnerId],
res_id: 11,
});
this.data['mail.notification'].records.push({
mail_message_id: 21,
notification_status: 'sent',
notification_type: 'inbox',
res_partner_id: this.data.currentPartnerId,
});
await this.start({
hasChatWindow: true,
hasMessagingMenu: true,
async mockRPC(route, args) {
if (route.includes('channel_fold')) {
const message = "should not call channel_fold when opening a non-channel chat window";
@@ -218,18 +263,14 @@ QUnit.test('[technical] opening a non-channel chat window should not call channe
return this._super(...arguments);
},
});
const thread = this.env.models['mail.thread'].create({
id: 11,
model: 'mail.channel',
});
this.env.models['mail.message'].create({
id: 21,
isNeedaction: true,
originThread: [['link', thread]],
});
await this.createThreadNeedactionPreviewComponent({
threadLocalId: thread.localId,
});
await afterNextRender(() => this.afterEvent({
eventName: 'o-thread-cache-loaded-messages',
func: () => document.querySelector('.o_MessagingMenu_toggler').click(),
message: "should wait until inbox loaded initial needaction messages",
predicate: ({ threadCache }) => {
return threadCache.thread.model === 'mail.box' && threadCache.thread.id === 'inbox';
},
}));
assert.containsOnce(
document.body,
'.o_ThreadNeedactionPreview',
+67 -18
View File
@@ -414,11 +414,7 @@ class ModelField {
}
break;
case 'replace':
// TODO IMP replace should not unlink-all (task-2270780)
if (this._setRelationUnlink(record, currentValue, options)) {
hasChanged = true;
}
if (this._setRelationLink(record, val[1], options)) {
if (this._setRelationReplace(record, val[1], options)) {
hasChanged = true;
}
break;
@@ -528,20 +524,9 @@ class ModelField {
* @returns {boolean} whether the value changed for the current field
*/
_setRelationInsertAndReplace(record, data, options) {
// unlink must be done before insert:
// because unlink might trigger delete due to causality and new data
// shouldn't be deleted just after being inserted
// TODO IMP insert-and-replace should not unlink-all (task-2270780)
let hasChanged = false;
if (this._setRelationUnlink(record, this.read(record), options)) {
hasChanged = true;
}
const OtherModel = this.env.models[this.to];
const other = this.env.modelManager._insert(OtherModel, data);
if (this._setRelationLink(record, other, options)) {
hasChanged = true;
}
return hasChanged;
const newValue = this.env.modelManager._insert(OtherModel, data);
return this._setRelationReplace(record, newValue, options);
}
/**
@@ -636,6 +621,70 @@ class ModelField {
return true;
}
/**
* Set a 'replace' operation on this relational field.
*
* @private
* @param {mail.model} record
* @param {mail.model|mail.model[]} newValue
* @param {Object} [options]
* @returns {boolean} whether the value changed for the current field
*/
_setRelationReplace(record, newValue, options) {
if (['one2one', 'many2one'].includes(this.relationType)) {
// for x2one replace is just link
return this._setRelationLinkX2One(record, newValue, options);
}
// for x2many: smart process to avoid unnecessary unlink/link
let hasChanged = false;
let hasToReorder = false;
const otherRecordsSet = this.read(record);
const otherRecordsList = [...otherRecordsSet];
const recordsToReplaceList = [...this._convertX2ManyValue(newValue)];
const recordsToReplaceSet = new Set(recordsToReplaceList);
// records to link
const recordsToLink = [];
for (let i = 0; i < recordsToReplaceList.length; i++) {
const recordToReplace = recordsToReplaceList[i];
if (!otherRecordsSet.has(recordToReplace)) {
recordsToLink.push(recordToReplace);
}
if (otherRecordsList[i] !== recordToReplace) {
hasToReorder = true;
}
}
if (this._setRelationLinkX2Many(record, recordsToLink, options)) {
hasChanged = true;
}
// records to unlink
const recordsToUnlink = [];
for (let i = 0; i < otherRecordsList.length; i++) {
const otherRecord = otherRecordsList[i];
if (!recordsToReplaceSet.has(otherRecord)) {
recordsToUnlink.push(otherRecord);
}
if (recordsToReplaceList[i] !== otherRecord) {
hasToReorder = true;
}
}
if (this._setRelationUnlinkX2Many(record, recordsToUnlink, options)) {
hasChanged = true;
}
// reorder result
if (hasToReorder) {
otherRecordsSet.clear();
for (const record of recordsToReplaceList) {
otherRecordsSet.add(record);
}
hasChanged = true;
}
return hasChanged;
}
/**
* Set an 'unlink' operation on this relational field.
*
@@ -287,6 +287,9 @@ function factory(dependencies) {
}
return list;
}, []);
if (channelIds.length === 0) {
return;
}
const channelPreviews = await this.env.services.rpc({
model: 'mail.channel',
method: 'channel_fetch_preview',
@@ -262,6 +262,10 @@ function factory(dependencies) {
if (!extraDomain && messages.length < limit) {
this.update({ isAllHistoryLoaded: true });
}
this.env.messagingBus.trigger('o-thread-cache-loaded-messages', {
fetchedMessages: messages,
threadCache: this,
});
return messages;
}
@@ -1300,11 +1300,12 @@ MockServer.include({
),
},
]);
// NOTE server is sending grouped notifications per channel_ids but
// this optimization is not needed here.
const data = { type: 'mark_as_read', message_ids: [message.id], channel_ids: message.channel_ids };
const busNotifications = [[[false, 'res.partner', this.currentPartnerId], data]];
this._widget.call('bus_service', 'trigger', 'notification', busNotifications);
}
// NOTE: server is also sending channel_ids, not done here for simplicity.
const data = { type: 'mark_as_read', message_ids: ids };
const busNotifications = [[[false, 'res.partner', this.currentPartnerId], data]];
this._widget.call('bus_service', 'trigger', 'notification', busNotifications);
},
/**
* Simulates `toggle_message_starred` on `mail.message`.