diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index a77f42c68ee..6667447cd39 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -55,7 +55,7 @@ class MailActivity(models.Model): res_model = fields.Char( 'Related Document Model', index=True, related='res_model_id.model', compute_sudo=True, store=True, readonly=True) - res_id = fields.Many2oneReference(string='Related Document ID', index=True, required=True, model_field='res_model') + res_id = fields.Many2oneReference(string='Related Document ID', index=True, model_field='res_model') res_name = fields.Char( 'Document Name', compute='_compute_res_name', compute_sudo=True, store=True, help="Display name of the related document.", readonly=True) @@ -95,6 +95,15 @@ class MailActivity(models.Model): # access can_write = fields.Boolean(compute='_compute_can_write', help='Technical field to hide buttons if the current user has no access.') + _sql_constraints = [ + # Required on a Many2one reference field is not sufficient as actually + # writing 0 is considered as a valid value, because this is an integer field. + # We therefore need a specific constraint check. + ('check_res_id_is_set', + 'CHECK(res_id IS NOT NULL AND res_id !=0 )', + 'Activities have to be linked to records with a not null res_id.') + ] + @api.onchange('previous_activity_type_id') def _compute_has_recommended_activities(self): for record in self: diff --git a/addons/mass_mailing/models/mailing_trace.py b/addons/mass_mailing/models/mailing_trace.py index 18a242c9297..01862e015b4 100644 --- a/addons/mass_mailing/models/mailing_trace.py +++ b/addons/mass_mailing/models/mailing_trace.py @@ -68,7 +68,7 @@ class MailingTrace(models.Model): source_id = fields.Many2one(related='mass_mailing_id.source_id') # document model = fields.Char(string='Document model', required=True) - res_id = fields.Many2oneReference(string='Document ID', model_field='model', required=True) + res_id = fields.Many2oneReference(string='Document ID', model_field='model') # campaign data mass_mailing_id = fields.Many2one('mailing.mailing', string='Mailing', index=True, ondelete='cascade') campaign_id = fields.Many2one( @@ -103,6 +103,15 @@ class MailingTrace(models.Model): links_click_ids = fields.One2many('link.tracker.click', 'mailing_trace_id', string='Links click') links_click_datetime = fields.Datetime('Clicked On', help='Stores last click datetime in case of multi clicks.') + _sql_constraints = [ + # Required on a Many2one reference field is not sufficient as actually + # writing 0 is considered as a valid value, because this is an integer field. + # We therefore need a specific constraint check. + ('check_res_id_is_set', + 'CHECK(res_id IS NOT NULL AND res_id !=0 )', + 'Traces have to be linked to records with a not null res_id.') + ] + @api.depends('trace_type', 'mass_mailing_id') def _compute_display_name(self): for trace in self: diff --git a/addons/mass_mailing/tests/test_mailing_internals.py b/addons/mass_mailing/tests/test_mailing_internals.py index f22cd437ba3..8eedacd9dc4 100644 --- a/addons/mass_mailing/tests/test_mailing_internals.py +++ b/addons/mass_mailing/tests/test_mailing_internals.py @@ -3,8 +3,8 @@ from ast import literal_eval from datetime import datetime - from freezegun import freeze_time +from psycopg2 import IntegrityError from odoo.addons.base.tests.test_ir_cron import CronMixinCase from odoo.addons.mass_mailing.tests.common import MassMailCommon @@ -193,6 +193,40 @@ class TestMassMailValues(MassMailCommon): ) self.assertEqual(mailing_form.mailing_model_real, 'res.partner') + @mute_logger('odoo.sql_db') + @users('user_marketing') + def test_mailing_trace_values(self): + recipient = self.partner_employee + + # both void and 0 are invalid, document should have an id != 0 + with self.assertRaises(IntegrityError): + self.env['mailing.trace'].create({ + 'model': recipient._name, + }) + with self.assertRaises(IntegrityError): + self.env['mailing.trace'].create({ + 'model': recipient._name, + 'res_id': 0, + }) + with self.assertRaises(IntegrityError): + self.env['mailing.trace'].create({ + 'res_id': 3, + }) + + activity = self.env['mailing.trace'].create({ + 'model': recipient._name, + 'res_id': recipient.id, + }) + with self.assertRaises(IntegrityError): + activity.write({'model': False}) + activity.flush() + with self.assertRaises(IntegrityError): + activity.write({'res_id': False}) + activity.flush() + with self.assertRaises(IntegrityError): + activity.write({'res_id': 0}) + activity.flush() + class TestMassMailFeatures(MassMailCommon, CronMixinCase): diff --git a/addons/test_mail/tests/test_mail_activity.py b/addons/test_mail/tests/test_mail_activity.py index 1997ab7be32..dbee2bde2f7 100644 --- a/addons/test_mail/tests/test_mail_activity.py +++ b/addons/test_mail/tests/test_mail_activity.py @@ -4,6 +4,7 @@ from datetime import date, datetime, timedelta from dateutil.relativedelta import relativedelta from freezegun import freeze_time +from psycopg2 import IntegrityError from unittest.mock import patch from unittest.mock import DEFAULT @@ -201,7 +202,6 @@ class TestActivityFlow(TestActivityCommon): activity.with_user(self.user_admin).write({'user_id': self.user_employee.id}) self.assertEqual(activity.user_id, self.user_employee) - @mute_logger('odoo.addons.mail.models.mail_mail') def test_activity_summary_sync(self): """ Test summary from type is copied on activities if set (currently only in form-based onchange) """ ActivityType = self.env['mail.activity.type'] @@ -210,8 +210,9 @@ class TestActivityFlow(TestActivityCommon): 'summary': 'Email Summary', }) call_activity_type = ActivityType.create({'name': 'call'}) - with Form(self.env['mail.activity'].with_context(default_res_model_id=self.env.ref('base.model_res_partner'))) as ActivityForm: - ActivityForm.res_model_id = self.env.ref('base.model_res_partner') + with Form(self.env['mail.activity'].with_context(default_res_model_id=self.env['ir.model']._get_id('mail.test.activity'))) as ActivityForm: + ActivityForm.res_model_id = self.env['ir.model']._get('mail.test.activity') + ActivityForm.res_id = self.test_record.id ActivityForm.activity_type_id = call_activity_type # activity summary should be empty @@ -225,6 +226,46 @@ class TestActivityFlow(TestActivityCommon): # activity summary remains unchanged from change of activity type as call activity doesn't have default summary self.assertEqual(ActivityForm.summary, email_activity_type.summary) + @mute_logger('odoo.sql_db') + def test_activity_values(self): + """ Test activities are created with right model / res_id values linking + to records without void values. 0 as res_id especially is not wanted. """ + # creating activities on a temporary record generates activities with res_id + # being 0, which is annoying -> never create activities in transient mode + temp_record = self.env['mail.test.activity'].new({'name': 'Test'}) + with self.assertRaises(IntegrityError): + activity = temp_record.activity_schedule('test_mail.mail_act_test_todo', user_id=self.user_employee.id) + + test_record = self.env['mail.test.activity'].browse(self.test_record.ids) + + with self.assertRaises(IntegrityError): + self.env['mail.activity'].create({ + 'res_model_id': self.env['ir.model']._get_id(test_record._name), + }) + with self.assertRaises(IntegrityError): + self.env['mail.activity'].create({ + 'res_model_id': self.env['ir.model']._get_id(test_record._name), + 'res_id': False, + }) + with self.assertRaises(IntegrityError): + self.env['mail.activity'].create({ + 'res_id': test_record.id, + }) + + activity = self.env['mail.activity'].create({ + 'res_id': test_record.id, + 'res_model_id': self.env['ir.model']._get_id(test_record._name), + }) + with self.assertRaises(IntegrityError): + activity.write({'res_model_id': False}) + activity.flush() + with self.assertRaises(IntegrityError): + activity.write({'res_id': False}) + activity.flush() + with self.assertRaises(IntegrityError): + activity.write({'res_id': 0}) + activity.flush() + @tests.tagged('mail_activity') class TestActivityMixin(TestActivityCommon):