From 821156ce90f7e2a7ed8c1efea2ae5d2433561acb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Mon, 20 Nov 2023 11:00:50 +0100 Subject: [PATCH] [FIX] {test_}mail: remove activity assignation check How to reproduce: - Install hr with demo data - Create a user without access to hr and turn it into an employee - Create a user for Abigail - Assign Abigail as manager of the new user - Click on "onboarding plan" in the chatter of the new user - Then in the dialog, click on "Schedule" button You get the error "Assigned user test has no access to the document and is not able to handle this activity." because the new user has no access to the record employee on which those activities are scheduled. As activities for which the user has no access to the underlying record are now displayed in the systray (with no access to the record), we remove the check that prevent assigning an activity to a user on a record he has no access to. Technical note: before odoo/odoo#149965, activities scheduled manually were created with the flag "automated" set to True and when this flag is set the check that ensures that the user has access to the record is skipped. With odoo/odoo#149965, as the "automated" flag is set to False when scheduling activities manually, an error is trigerred if the user has no access to the underlying record. Here we always skip that test and mark the method as deprecated because the user can see the activity no matter the access he has on the underlying record. Task-3598836 closes odoo/odoo#155576 Signed-off-by: Thibault Delavallee (tde) --- addons/mail/models/mail_activity.py | 18 ++++++++---------- addons/test_mail/tests/test_mail_activity.py | 15 +++++++-------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 01256dfe141..7cb6cc93230 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -260,9 +260,8 @@ class MailActivity(models.Model): # if available; otherwise fall back on read for read, write for other operations. activity_to_documents = dict() for activity in remaining_sudo: - # write / unlink: if not updating self or assigned, limit to automated activities to avoid - # updating other people's activities. As unlinking a document bypasses access rights checks - # on related activities this will not prevent people from deleting documents with activities + # write / unlink: As unlinking a document bypasses access rights checks on related activities + # this will not prevent people from deleting documents with activities # create / read: just check rights on related document activity_to_documents.setdefault(activity.res_model, list()).append(activity.res_id) for doc_model, doc_ids in activity_to_documents.items(): @@ -283,7 +282,12 @@ class MailActivity(models.Model): """ Check assigned user (user_id field) has access to the document. Purpose 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. """ + raise an UserError if the assigned user has no access to the document. + + .. deprecated:: 17.0 + Deprecated method, we don't check access to the underlying records anymore + as user can new see activities without having access to the underlying records. + """ for model, activity_data in self._classify_by_model().items(): # group activities / user, in order to batch the check of ACLs per_user = dict() @@ -327,14 +331,10 @@ class MailActivity(models.Model): readable_user_partners = self.env.user.partner_id # when creating activities for other: send a notification to assigned user; - # in case of manually done activity also check target has rights on document - # otherwise we prevent its creation. Automated activities are checked since - # they are integrated into business flows that should not crash. if self.env.context.get('mail_activity_quick_update'): activities_to_notify = self.env['mail.activity'] else: activities_to_notify = activities.filtered(lambda act: act.user_id != self.env.user) - activities_to_notify.filtered(lambda act: not act.automated)._check_access_assignation() if activities_to_notify: to_sudo = activities_to_notify.filtered(lambda act: act.user_id.partner_id not in readable_user_partners) other = activities_to_notify - to_sudo @@ -370,8 +370,6 @@ class MailActivity(models.Model): if values.get('user_id'): if values['user_id'] != self.env.uid: - to_check = user_changes.filtered(lambda act: not act.automated) - to_check._check_access_assignation() if not self.env.context.get('mail_activity_quick_update', False): user_changes.action_notify() for activity in user_changes: diff --git a/addons/test_mail/tests/test_mail_activity.py b/addons/test_mail/tests/test_mail_activity.py index ee80cc6632d..53945a9adc9 100644 --- a/addons/test_mail/tests/test_mail_activity.py +++ b/addons/test_mail/tests/test_mail_activity.py @@ -124,15 +124,14 @@ class TestActivityRights(TestActivityCommon): [('id', '=', test_activity.id)], ['summary']) - # cannot create activities for people that cannot access record + # can create activities for people that cannot access record with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash): - with self.assertRaises(exceptions.UserError): - activity = self.env['mail.activity'].create({ - 'activity_type_id': self.env.ref('test_mail.mail_act_test_todo').id, - 'res_model_id': self.env.ref('test_mail.model_mail_test_activity').id, - 'res_id': self.test_record.id, - 'user_id': self.user_employee.id, - }) + self.env['mail.activity'].create({ + 'activity_type_id': self.env.ref('test_mail.mail_act_test_todo').id, + 'res_model_id': self.env.ref('test_mail.model_mail_test_activity').id, + 'res_id': self.test_record.id, + 'user_id': self.user_employee.id, + }) # cannot create activities if no access to the document with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash):