From 846d66ae9d97e7040e9936367a3078c3ea7640f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 25 Aug 2022 15:37:55 +0000 Subject: [PATCH] [PRF] mail: improve check assignment by doing it in batch When assigning activities to people we check target user has the rights to access to document. Otherwise they end up with notifications and activities for records they can't see. This code is currently a loop doing browse and access checks one record at a time. We improve it by grouping activities by related record model and target user, allowing to batch some calls. Note that this somehow reverts odoo/odoo@c38a0d67680818dbb2e3334106e183aa307f5734 and odoo/odoo@a98c15f44358d6342632a8bd461fb476461f35ae as normally odoo/odoo@16351e48792bd35c6802646e5da38042709a785b should be sufficient to handle MC use cases. Lack of clear unit tests for those corner cases make it difficult to clearly reproduce issue anyway. Task-2883589 (Activity performance and cleaning) Part-of: odoo/odoo#93682 --- addons/mail/models/mail_activity.py | 39 ++++++++++++++++------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index f5904c772a3..8be9c6abf71 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -244,27 +244,32 @@ class MailActivity(models.Model): is to allow assigned user to handle their activities. For that purpose assigned user should be able to at least read the document. We therefore raise an UserError if the assigned user has no access to the document. """ - for activity in self: - model = self.env[activity.res_model].with_user(activity.user_id).with_context(allowed_company_ids=activity.user_id.company_ids.ids) - try: - model.check_access_rights('read') - except exceptions.AccessError: - raise exceptions.UserError( - _('Assigned user %s has no access to the document and is not able to handle this activity.', - activity.user_id.display_name)) - else: + for model, activity_data in self._classify_by_model().items(): + # group activities / user, in order to batch the check of ACLs + per_user = dict() + for activity in activity_data['activities'].filtered(lambda act: act.user_id): + if activity.user_id not in per_user: + per_user[activity.user_id] = activity + else: + per_user[activity.user_id] += activity + for user, activities in per_user.items(): + RecordModel = self.env[model].with_user(user).with_context( + allowed_company_ids=user.company_ids.ids + ) try: - target_user = activity.user_id - target_record = self.env[activity.res_model].browse(activity.res_id) - if hasattr(target_record, 'company_id') and ( - target_record.company_id != target_user.company_id and ( - len(target_user.sudo().company_ids) > 1)): - return # in that case we skip the check, assuming it would fail because of the company - model.browse(activity.res_id).check_access_rule('read') + RecordModel.check_access_rights('read') except exceptions.AccessError: raise exceptions.UserError( _('Assigned user %s has no access to the document and is not able to handle this activity.', - activity.user_id.display_name)) + user.display_name)) + else: + try: + target_records = self.env[model].browse(activities.mapped('res_id')) + target_records.check_access_rule('read') + except exceptions.AccessError: + raise exceptions.UserError( + _('Assigned user %s has no access to the document and is not able to handle this activity.', + user.display_name)) # ------------------------------------------------------ # ORM overrides