pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP][FIX] mail, mass_mailing: add constraint for required res_id

Browse Source 
Activities and mailing trace are linked to documents through a model and
a many2one reference field. The latter one is required but is implemented like
an integer field, meaning writing 0 is actually a valid value for 'required'.

In this commit we add a constraint to ensure we never update activities with
a void res_id value. Same for mailing traces. This allows to remove the
required attribute on fields to avoid having redundant sql constraints.

Source: internal feedback about activities with 0 as res_id

Task-2694133

Part-of: odoo/odoo#81292
This commit is contained in:
Thibault Delavallée
2022-02-01 13:38:49 +00:00
parent f17b3aaaec
commit 04ef6bee10
4 changed files with 99 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/models/mail_activity.py
+10 -1
View File
@@ -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:
addons/mass_mailing/models/mailing_trace.py
+10 -1
View File
@@ -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:
addons/mass_mailing/tests/test_mailing_internals.py
+35 -1
View File
@@ -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):
addons/test_mail/tests/test_mail_activity.py
+44 -3
View File
@@ -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):