From 5494b0bc3eec6bb063017eaec61fd312c880e4b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 25 Oct 2018 08:53:05 +0000 Subject: [PATCH] [FIX] mail: do not prevent record update due to automated activities Currently creating activities assigned to user not having to the document raises an Error as people should not have activities they cannot handle directly on a document they cannot access. In some cases activities are created through business flow, like automatic activities creation when creating leave requests. Assigning an activity to someone that has no access to the document should not prevent from creating the leave request. This commit therefore does not check assigned user access on automated activities to avoid having blocked business flows. This commit is linked to task ID 1903484. closes odoo/odoo#28139 --- addons/mail/models/mail_activity.py | 10 ++++-- addons/test_mail/tests/test_mail_activity.py | 32 +++++++++++++++++--- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 42f30788b37..4a29e788d95 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -276,9 +276,12 @@ class MailActivity(models.Model): activity_user = activity.sudo(self.env.user) activity_user._check_access('create') - # check target user has rights on document otherwise we have to prevent activity creation + # 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 activity_user.user_id != self.env.user: - activity_user._check_access_assignation() + if not activity_user.automated: + activity_user._check_access_assignation() if not self.env.context.get('mail_activity_quick_update', False): activity_user.action_notify() @@ -298,7 +301,8 @@ class MailActivity(models.Model): if values.get('user_id'): if values['user_id'] != self.env.uid: - self._check_access_assignation() + to_check = self.filtered(lambda act: not act.automated) + to_check._check_access_assignation() if not self.env.context.get('mail_activity_quick_update', False): self.action_notify() for activity in self: diff --git a/addons/test_mail/tests/test_mail_activity.py b/addons/test_mail/tests/test_mail_activity.py index da55ebed4f7..4b3b2e98922 100644 --- a/addons/test_mail/tests/test_mail_activity.py +++ b/addons/test_mail/tests/test_mail_activity.py @@ -39,8 +39,23 @@ class TestActivityRights(TestActivityCommon): activity2 = self.test_record.activity_schedule('test_mail.mail_act_test_todo') activity2.write({'user_id': self.user_employee.id}) - def test_activity_security_user_noaccess(self): + def test_activity_security_user_noaccess_automated(self): + def _employee_crash(*args, **kwargs): + """ If employee is test employee, consider he has no access on document """ + recordset = args[0] + if recordset.env.uid == self.user_employee.id: + raise exceptions.AccessError('Hop hop hop Ernest, please step back.') + return DEFAULT + with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash): + activity = self.test_record.activity_schedule( + 'test_mail.mail_act_test_todo', + user_id=self.user_employee.id) + + activity2 = self.test_record.activity_schedule('test_mail.mail_act_test_todo') + activity2.write({'user_id': self.user_employee.id}) + + def test_activity_security_user_noaccess_manual(self): def _employee_crash(*args, **kwargs): """ If employee is test employee, consider he has no access on document """ recordset = args[0] @@ -50,11 +65,18 @@ class TestActivityRights(TestActivityCommon): with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash): with self.assertRaises(exceptions.UserError): - activity = self.test_record.activity_schedule( - 'test_mail.mail_act_test_todo', - user_id=self.user_employee.id) + 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, + }) - activity2 = self.test_record.activity_schedule('test_mail.mail_act_test_todo') + activity2 = 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, + }) with self.assertRaises(exceptions.UserError): activity2.write({'user_id': self.user_employee.id})