From a6ebc20b7373120056b4ec3bcf3518a0240d6b49 Mon Sep 17 00:00:00 2001 From: "Didier (did)" Date: Thu, 3 Aug 2023 12:58:04 +0000 Subject: [PATCH] [FIX] mail: only display followers with mail.mt_comment activated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, recipient list in chatter when using composer in "Send message" mode where listing emails of all followers, instead of only subscribers of "Send message" (= `mail.mt_comment`). This commit solves this issue by loading and showing followers that are recipients in this list. Because this feature and follower list lazy-load some data, they need to manage they own list for the "load-more" aspect. That's why the diff shows some duplicated code as with `followers` field of JS thread model. closes odoo/odoo#134225 X-original-commit: ba43502b22f8f00e7a2318b341302b809e7129dc Signed-off-by: Alexandre Kühn (aku) --- addons/mail/models/mail_thread.py | 16 +++++++++- addons/mail/models/res_users.py | 1 + .../src/core/common/messaging_service.js | 1 + .../static/src/core/common/thread_model.js | 7 ++++ addons/mail/static/src/core/web/chatter.js | 20 ++++-------- .../src/core/web/follower_subtype_dialog.js | 4 +++ .../static/src/core/web/recipient_list.js | 2 +- .../static/src/core/web/recipient_list.xml | 7 ++-- .../static/src/core/web/thread_model_patch.js | 11 +++++++ .../src/core/web/thread_service_patch.js | 32 +++++++++++++++++++ .../static/tests/composer/composer_tests.js | 16 ---------- .../mock_server/controllers/discuss.js | 8 +++++ .../helpers/mock_server/models/mail_thread.js | 5 ++- .../tests/web/follower_list_menu_tests.js | 12 +++---- .../tests/test_performance.py | 1 + 15 files changed, 100 insertions(+), 43 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 1100f9aedb0..6e78e9c63fd 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -3966,12 +3966,18 @@ class MailThread(models.AbstractModel): return True - def message_get_followers(self, after=None, limit=100): + def message_get_followers(self, after=None, limit=100, filter_recipients=False): self.ensure_one() domain = [ ("res_id", "=", self.id), ("res_model", "=", self._name), ] + if filter_recipients: + subtype_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment') + domain = expression.AND([domain, [ + ('subtype_ids', '=', subtype_id), + ('partner_id', '!=', self.env.user.partner_id.id), + ]]) if after: domain = expression.AND([domain, [('id', '>', after)]]) return self.env["mail.followers"].search(domain, limit=limit, order='id ASC')._format_for_chatter() @@ -4145,6 +4151,14 @@ class MailThread(models.AbstractModel): ])._format_for_chatter() res['selfFollower'] = self_follower[0] if len(self_follower) > 0 else None res['followers'] = self.message_get_followers() + subtype_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment') + res['recipientsCount'] = self.env['mail.followers'].search_count([ + ("res_id", "=", self.id), + ("res_model", "=", self._name), + ('partner_id', '!=', self.env.user.partner_id.id), + ('subtype_ids', '=', subtype_id), + ]) + res['recipients'] = self.message_get_followers(filter_recipients=True) if 'suggestedRecipients' in request_list: res['suggestedRecipients'] = self._message_get_suggested_recipients()[self.id] return res diff --git a/addons/mail/models/res_users.py b/addons/mail/models/res_users.py index 4e4a6ab890b..e527efbe569 100644 --- a/addons/mail/models/res_users.py +++ b/addons/mail/models/res_users.py @@ -258,6 +258,7 @@ class Users(models.Model): 'initBusId': self.env['bus.bus'].sudo()._bus_last_id(), 'internalUserGroupId': self.env.ref('base.group_user').id, 'menu_id': self.env['ir.model.data']._xmlid_to_res_id('mail.menu_root_discuss'), + 'mt_comment_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment'), 'needaction_inbox_counter': self.partner_id._get_needaction_count(), 'odoobot': odoobot.sudo().mail_partner_format().get(odoobot), 'shortcodes': self.env['mail.shortcode'].sudo().search_read([], ['source', 'substitution']), diff --git a/addons/mail/static/src/core/common/messaging_service.js b/addons/mail/static/src/core/common/messaging_service.js index 85ac9206dea..87c720651df 100644 --- a/addons/mail/static/src/core/common/messaging_service.js +++ b/addons/mail/static/src/core/common/messaging_service.js @@ -87,6 +87,7 @@ export class Messaging { this.store.discuss.inbox.counter = data.needaction_inbox_counter; this.store.internalUserGroupId = data.internalUserGroupId; this.store.discuss.starred.counter = data.starred_counter; + this.store.mt_comment_id = data.mt_comment_id; this.store.discuss.isActive = data.menu_id === this.router.current.hash?.menu_id || this.router.hash?.action === "mail.action_discuss"; diff --git a/addons/mail/static/src/core/common/thread_model.js b/addons/mail/static/src/core/common/thread_model.js index 93c3e4cf17c..d9d37ba84bf 100644 --- a/addons/mail/static/src/core/common/thread_model.js +++ b/addons/mail/static/src/core/common/thread_model.js @@ -76,6 +76,13 @@ export class Thread extends Record { return thread; } + setup() {} + + constructor() { + super(); + this.setup(); + } + /** @type {number} */ id; /** @type {string} */ diff --git a/addons/mail/static/src/core/web/chatter.js b/addons/mail/static/src/core/web/chatter.js index aabbeacce91..0294e22744f 100644 --- a/addons/mail/static/src/core/web/chatter.js +++ b/addons/mail/static/src/core/web/chatter.js @@ -30,7 +30,7 @@ import { Dropdown } from "@web/core/dropdown/dropdown"; import { _t } from "@web/core/l10n/translation"; import { usePopover } from "@web/core/popover/popover_hook"; import { useService } from "@web/core/utils/hooks"; -import { escape } from "@web/core/utils/strings"; +import { escapeHTML } from "@web/core/utils/strings"; import { useThrottleForAnimation } from "@web/core/utils/timing"; import { FileUploader } from "@web/views/fields/file_handler"; @@ -220,17 +220,9 @@ export class Chatter extends Component { * @returns {string} */ get toRecipientsText() { - const allFollowers = []; - if (this.state.thread.selfFollower) { - allFollowers.push(this.state.thread.selfFollower); - } - allFollowers.push(...this.state.thread.followers); - const followers = allFollowers.slice(0, 5).map(({ partner }) => { - if (partner.eq(this.store.self)) { - return `me`; - } + const recipients = [...this.state.thread.recipients].slice(0, 5).map(({ partner }) => { const text = partner.email ? partner.emailWithoutDomain : partner.name; - return `${escape( + return `${escapeHTML( text )}`; }); @@ -238,10 +230,10 @@ export class Chatter extends Component { this.store.env.services["user"].lang?.replace("_", "-"), { type: "unit" } ); - if (allFollowers.length > 5) { - followers.push("…"); + if (this.state.thread.recipients.size > 5) { + recipients.push("…"); } - return markup(formatter.format(followers)); + return markup(formatter.format(recipients)); } /** diff --git a/addons/mail/static/src/core/web/follower_subtype_dialog.js b/addons/mail/static/src/core/web/follower_subtype_dialog.js index 84ba26a4d18..6f59946683d 100644 --- a/addons/mail/static/src/core/web/follower_subtype_dialog.js +++ b/addons/mail/static/src/core/web/follower_subtype_dialog.js @@ -27,6 +27,7 @@ export class FollowerSubtypeDialog extends Component { setup() { this.rpc = useService("rpc"); + this.store = useState(useService("mail.store")); this.state = useState({ /** @type {SubtypeData[]} */ subtypes: [], @@ -60,6 +61,9 @@ export class FollowerSubtypeDialog extends Component { subtype_ids: selectedSubtypes.map((subtype) => subtype.id), } ); + if (!selectedSubtypes.some((subtype) => subtype.id === this.store.mt_comment_id)) { + this.env.services["mail.thread"].removeRecipient(this.props.follower); + } this.env.services.notification.add( _t("The subscription preferences were successfully applied."), { type: "success" } diff --git a/addons/mail/static/src/core/web/recipient_list.js b/addons/mail/static/src/core/web/recipient_list.js index 820b7edc922..6fe45992c18 100644 --- a/addons/mail/static/src/core/web/recipient_list.js +++ b/addons/mail/static/src/core/web/recipient_list.js @@ -18,7 +18,7 @@ export class RecipientList extends Component { this.threadService = useState(useService("mail.thread")); this.loadMoreState = useVisible("load-more", () => { if (this.loadMoreState.isVisible) { - this.threadService.loadMoreFollowers(this.props.thread); + this.threadService.loadMoreRecipients(this.props.thread); } }); } diff --git a/addons/mail/static/src/core/web/recipient_list.xml b/addons/mail/static/src/core/web/recipient_list.xml index b58ced52a28..5b4c7613b9d 100644 --- a/addons/mail/static/src/core/web/recipient_list.xml +++ b/addons/mail/static/src/core/web/recipient_list.xml @@ -4,11 +4,10 @@
    -
  • -
  • - +
  • +
    • - Load more + Load more
diff --git a/addons/mail/static/src/core/web/thread_model_patch.js b/addons/mail/static/src/core/web/thread_model_patch.js index a59fc19a48a..1fcf67ca32c 100644 --- a/addons/mail/static/src/core/web/thread_model_patch.js +++ b/addons/mail/static/src/core/web/thread_model_patch.js @@ -5,6 +5,17 @@ import { Thread } from "@mail/core/common/thread_model"; import { patch } from "@web/core/utils/patch"; patch(Thread.prototype, { + /** @type {integer|undefined} */ + recipientsCount: undefined, + /** @type {Number} */ + mt_comment_id: undefined, + recipients: undefined, + setup() { + this.recipients = new Set(); + }, + get recipientsFullyLoaded() { + return this.recipientsCount === this.recipients.size; + }, /** * @returns {import("@mail/core/web/activity_model").Activity[]} */ diff --git a/addons/mail/static/src/core/web/thread_service_patch.js b/addons/mail/static/src/core/web/thread_service_patch.js index 15c509d8e48..09b9bb4e38e 100644 --- a/addons/mail/static/src/core/web/thread_service_patch.js +++ b/addons/mail/static/src/core/web/thread_service_patch.js @@ -94,6 +94,15 @@ patch(ThreadService.prototype, { thread.followers.add(follower); } } + thread.recipientsCount = result.recipientsCount; + for (const recipientData of result.recipients) { + thread.recipients.add( + this.store.Follower.insert({ + followedThread: thread, + ...recipientData, + }) + ); + } } if ("suggestedRecipients" in result) { this.insertSuggestedRecipients(thread, result.suggestedRecipients); @@ -180,6 +189,22 @@ patch(ThreadService.prototype, { } } }, + async loadMoreRecipients(thread) { + const recipients = await this.orm.call( + thread.model, + "message_get_followers", + [[thread.id], Array.from(thread.recipients).at(-1).id], + { filter_recipients: true } + ); + for (const data of recipients) { + thread.recipients.add( + this.store.Follower.insert({ + followedThread: thread, + ...data, + }) + ); + } + }, open(thread, replaceNewMessageChatWindow) { if (!this.store.discuss.isActive && !this.ui.isSmall) { this._openChatWindow(thread, replaceNewMessageChatWindow); @@ -200,6 +225,12 @@ patch(ThreadService.prototype, { } super.open(thread, replaceNewMessageChatWindow); }, + /** + * @param {import("@mail/core/common/follower_model").Follower} follower + */ + removeRecipient(recipient) { + recipient.followedThread.recipients.delete(recipient); + }, /** * @param {import("@mail/core/common/follower_model").Follower} follower */ @@ -214,6 +245,7 @@ patch(ThreadService.prototype, { } else { thread.followers.delete(follower); } + this.removeRecipient(follower); follower.delete(); }, unpin(thread) { diff --git a/addons/mail/static/tests/composer/composer_tests.js b/addons/mail/static/tests/composer/composer_tests.js index 61ef6428e0b..8fded8e5b33 100644 --- a/addons/mail/static/tests/composer/composer_tests.js +++ b/addons/mail/static/tests/composer/composer_tests.js @@ -753,22 +753,6 @@ QUnit.test("remove an uploading attachment", async () => { await contains(".o-mail-Composer .o-mail-AttachmentCard", { count: 0 }); }); -QUnit.test("Show a thread name in the recipient status text.", async () => { - const pyEnv = await startServer(); - const partnerId = pyEnv["res.partner"].create({ name: "test name", email: "test@odoo.com" }); - pyEnv["mail.followers"].create({ - is_active: true, - partner_id: partnerId, - res_id: partnerId, - res_model: "res.partner", - }); - const { openFormView } = await start(); - openFormView("res.partner", partnerId); - await click("button", { text: "Send message" }); - await contains(".o-mail-Chatter div", { text: "To: test" }); - await contains('span[title="test@odoo.com"]'); -}); - QUnit.test("Show recipient list when there is more than 5 followers.", async () => { const pyEnv = await startServer(); const partnerIds = pyEnv["res.partner"].create([ diff --git a/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js b/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js index 3d346c3f1f7..e0c88039c38 100644 --- a/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js +++ b/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js @@ -560,6 +560,14 @@ patch(MockServer.prototype, { ? this._mockMailFollowers_FormatForChatter(selfFollower.id)[0] : false; res["followers"] = this._mockMailThreadMessageGetFollowers(thread_model, [thread_id]); + res["recipientsCount"] = (thread.message_follower_ids || []).length - 1; + res["recipients"] = this._mockMailThreadMessageGetFollowers( + thread_model, + [thread_id], + undefined, + 100, + { filter_recipients: true } + ); } if (request_list.includes("suggestedRecipients")) { res["suggestedRecipients"] = this._mockMailThread_MessageGetSuggestedRecipients( diff --git a/addons/mail/static/tests/helpers/mock_server/models/mail_thread.js b/addons/mail/static/tests/helpers/mock_server/models/mail_thread.js index d5f62bb83f7..0386c74be52 100644 --- a/addons/mail/static/tests/helpers/mock_server/models/mail_thread.js +++ b/addons/mail/static/tests/helpers/mock_server/models/mail_thread.js @@ -123,7 +123,7 @@ patch(MockServer.prototype, { * @param {integer} [limit=100] * @returns {Object[]} */ - _mockMailThreadMessageGetFollowers(model, ids, after, limit = 100) { + _mockMailThreadMessageGetFollowers(model, ids, after, limit = 100, kwargs = {}) { const domain = [ ["res_id", "=", ids[0]], ["res_model", "=", model], @@ -131,6 +131,9 @@ patch(MockServer.prototype, { if (after) { domain.push(["id", ">", after]); } + if (kwargs.filter_recipients) { + domain.push(["partner_id", "!=", this.pyEnv.currentPartnerId]); + } const followers = this.getRecords("mail.followers", domain).sort( (f1, f2) => (f1.id < f2.id ? -1 : 1) // sorted from lowest ID to highest ID (i.e. from oldest to youngest) ); diff --git a/addons/mail/static/tests/web/follower_list_menu_tests.js b/addons/mail/static/tests/web/follower_list_menu_tests.js index 0af847f192e..95c224b2bd2 100644 --- a/addons/mail/static/tests/web/follower_list_menu_tests.js +++ b/addons/mail/static/tests/web/follower_list_menu_tests.js @@ -253,11 +253,11 @@ QUnit.test("Load 100 recipients at once", async () => { ); const { openFormView } = await start(); await openFormView("res.partner", partnerIds[0]); - await contains("button[title='Show Followers']", { text: "210" }); - await click("button", { text: "Send message" }); - await contains(".o-mail-Chatter div", { - text: "To: me, partner1, partner2, partner3, partner4, …", - }); + await contains("button[title='Show Followers']:contains(210)"); + await click("button:contains(Send message)"); + await contains( + ".o-mail-Chatter:contains('partner1, partner2, partner3, partner4, partner5, …')" + ); await contains("button[title='Show all recipients']"); await click("button[title='Show all recipients']"); await contains(".o-mail-RecipientList li", { count: 100 }); @@ -266,7 +266,7 @@ QUnit.test("Load 100 recipients at once", async () => { await contains(".o-mail-RecipientList li", { count: 200 }); await new Promise(setTimeout); // give enough time for the useVisible hook to register load more as hidden await scroll(".o-mail-RecipientList", "bottom"); - await contains(".o-mail-RecipientList li", { count: 210 }); + await contains(".o-mail-RecipientList li", { count: 209 }); await contains(".o-mail-RecipientList span", { count: 0, text: "Load more" }); }); diff --git a/addons/test_discuss_full/tests/test_performance.py b/addons/test_discuss_full/tests/test_performance.py index fd164000022..7838adfae0b 100644 --- a/addons/test_discuss_full/tests/test_performance.py +++ b/addons/test_discuss_full/tests/test_performance.py @@ -1008,6 +1008,7 @@ class TestDiscussFullPerformance(HttpCase): ], 'internalUserGroupId': self.env.ref('base.group_user').id, 'menu_id': self.env['ir.model.data']._xmlid_to_res_id('mail.menu_root_discuss'), + 'mt_comment_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment'), 'odoobot': { 'active': False, 'email': 'odoobot@example.com',