From 5f1dce419b36f30fe237c647b18890db0fcb8a6b Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 5 Sep 2012 17:19:50 +0200 Subject: [PATCH] [IMP] mail*: implement auto-deletion mechanism for mail.message, mail.mail and the attachment This is based on the auto_delete flag on mail.mail and the assumption that mail.mail records created without a parent message will be deleted as a whole, including the parent message and its attachments. This is contrasted with mail.mail records created for notifications regarding a mail.message, and therefore created with an existing `mail_message_id` parent. For this last case the parent must not be deleted along with the child mail.mail, as it exists on its own. bzr revid: odo@openerp.com-20120905151950-7jv6hi0x5fx5iytp --- addons/email_template/email_template.py | 12 +++++---- addons/mail/mail_followers.py | 15 ++++++----- addons/mail/mail_mail.py | 34 +++++++++++++++++-------- addons/mail/mail_message.py | 12 +++++++++ addons/mail/tests/test_mail.py | 33 ++++++++---------------- 5 files changed, 61 insertions(+), 45 deletions(-) diff --git a/addons/email_template/email_template.py b/addons/email_template/email_template.py index b68005bd997..74a9ae99a4c 100644 --- a/addons/email_template/email_template.py +++ b/addons/email_template/email_template.py @@ -156,6 +156,10 @@ class email_template(osv.osv): 'copyvalue': fields.char('Placeholder Expression', help="Final placeholder expression, to be copy-pasted in the desired template field."), } + _defaults = { + 'auto_delete': True, + } + def create_action(self, cr, uid, ids, context=None): vals = {} action_obj = self.pool.get('ir.actions.act_window') @@ -277,14 +281,12 @@ class email_template(osv.osv): context = {} report_xml_pool = self.pool.get('ir.actions.report.xml') template = self.get_email_template(cr, uid, template_id, res_id, context) - values = {'model': template.model_id.model} - + values = {} for field in ['subject', 'body_html', 'email_from', 'email_to', 'email_cc', 'reply_to']: values[field] = self.render_template(cr, uid, getattr(template, field), template.model, res_id, context=context) \ or False - if template.user_signature: signature = self.pool.get('res.users').browse(cr, uid, uid, context).signature values['body_html'] = append_content_to_html(values['body_html'], signature) @@ -292,8 +294,8 @@ class email_template(osv.osv): if values['body_html']: values['body'] = html_sanitize(values['body_html']) - values.update(mail_server_id = template.mail_server_id.id or False, - auto_delete = template.auto_delete, + values.update(mail_server_id=template.mail_server_id.id or False, + auto_delete=template.auto_delete, model=template.model, res_id=res_id or False) diff --git a/addons/mail/mail_followers.py b/addons/mail/mail_followers.py index 9205aa0b933..2aba685c4fa 100644 --- a/addons/mail/mail_followers.py +++ b/addons/mail/mail_followers.py @@ -100,11 +100,12 @@ class mail_notification(osv.Model): if signature: body_html = tools.append_content_to_html(body_html, signature) - towrite = { + mail_values = { 'mail_message_id': msg.id, 'email_to': [], - 'auto_delete': False, + 'auto_delete': True, 'body_html': body_html, + 'state': 'outgoing', } for partner in self.pool.get('res.partner').browse(cr, uid, partner_ids, context=context): @@ -123,10 +124,10 @@ class mail_notification(osv.Model): # Partner wants to receive only emails if partner.notification_email_send == 'email' and msg.type != 'email': continue - if partner.email not in towrite['email_to']: - towrite['email_to'].append(partner.email) - if towrite['email_to']: - towrite['email_to'] = ', '.join(towrite['email_to']) - email_notif_id = mail_mail.create(cr, uid, towrite, context=context) + if partner.email not in mail_values['email_to']: + mail_values['email_to'].append(partner.email) + if mail_values['email_to']: + mail_values['email_to'] = ', '.join(mail_values['email_to']) + email_notif_id = mail_mail.create(cr, uid, mail_values, context=context) mail_mail.send(cr, uid, [email_notif_id], context=context) return True diff --git a/addons/mail/mail_mail.py b/addons/mail/mail_mail.py index 36b71653556..7b593c5b998 100644 --- a/addons/mail/mail_mail.py +++ b/addons/mail/mail_mail.py @@ -54,8 +54,12 @@ class mail_mail(osv.Model): 'email_from': fields.char('From', help='Message sender, taken from user preferences.'), 'email_to': fields.text('To', help='Message recipients'), 'email_cc': fields.char('Cc', help='Carbon copy message recipients'), - 'reply_to':fields.char('Reply-To', help='Preferred response address for the message'), + 'reply_to': fields.char('Reply-To', help='Preferred response address for the message'), 'body_html': fields.text('Rich-text Contents', help="Rich-text/HTML message"), + + # Auto-detected based on create() - if 'mail_message_id' was passed then this mail is a notification + # and during unlink() we will cascade delete the parent and its attachments + 'notification': fields.boolean('Is Notification') } def _get_default_from(self, cr, uid, context=None): @@ -69,6 +73,19 @@ class mail_mail(osv.Model): 'email_from': lambda self, cr, uid, ctx=None: self._get_default_from(cr, uid, ctx), } + def create(self, cr, uid, values, context=None): + if 'notification' not in values and values.get('mail_message_id'): + values['notification'] = True + return super(mail_mail,self).create(cr, uid, values, context=context) + + def unlink(self, cr, uid, ids, context=None): + # cascade-delete the parent message for all mails that are not created for a notification + ids_to_cascade = self.search(cr, uid, [('notification','=',False),('id','in',ids)]) + parent_msg_ids = [m.mail_message_id.id for m in self.browse(cr, uid, ids_to_cascade, context=context)] + res = super(mail_mail,self).unlink(cr, uid, ids, context=context) + self.pool.get('mail.message').unlink(cr, uid, parent_msg_ids, context=context) + return res + def mark_outgoing(self, cr, uid, ids, context=None): return self.write(cr, uid, ids, {'state':'outgoing'}, context=context) @@ -106,20 +123,17 @@ class mail_mail(osv.Model): _logger.exception("Failed processing mail queue") return res - def _postprocess_sent_message(self, cr, uid, message, context=None): - """Perform any post-processing necessary after sending ``message`` + def _postprocess_sent_message(self, cr, uid, mail, context=None): + """Perform any post-processing necessary after sending ``mail`` successfully, including deleting it completely along with its - attachment if the ``auto_delete`` flag of the message was set. + attachment if the ``auto_delete`` flag of the mail was set. Overridden by subclasses for extra post-processing behaviors. - :param browse_record message: the message that was just sent + :param browse_record mail: the mail that was just sent :return: True """ - if message.auto_delete: - self.pool.get('ir.attachment').unlink(cr, uid, - [x.id for x in message.attachment_ids], - context=context) - message.unlink() + if mail.auto_delete: + mail.unlink() return True def _send_get_mail_subject(self, cr, uid, mail, force=False, context=None): diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index bca46a3c91c..a9266ff07fc 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -297,6 +297,18 @@ class mail_message(osv.Model): self.notify(cr, uid, newid, context=context) return newid + def unlink(self, cr, uid, ids, context=None): + # cascade-delete attachments that are directly attached to the message (should only happen + # for mail.messages that act as parent for a standalone mail.mail record. + attachments_to_delete = [] + for mail in self.browse(cr, uid, ids, context=context): + for attach in mail.attachment_ids: + if attach.res_model == 'mail.message' and attach.res_id == mail.id: + attachments_to_delete.append(attach.id) + if attachments_to_delete: + self.pool.get('ir.attachment').unlink(cr, uid, attachments_to_delete, context=context) + return super(mail_message,self).unlink(cr, uid, ids, context=context) + def notify(self, cr, uid, newid, context=None): """ Add the related record followers to the destination partner_ids. Call mail_notification.notify to manage the email sending diff --git a/addons/mail/tests/test_mail.py b/addons/mail/tests/test_mail.py index f8fd02bfb7e..b24ba94b891 100644 --- a/addons/mail/tests/test_mail.py +++ b/addons/mail/tests/test_mail.py @@ -19,7 +19,6 @@ # ############################################################################## -import base64 import tools from openerp.tests import common @@ -252,17 +251,13 @@ class test_mail(common.TransactionCase): # CASE1: post comment, body and subject specified msg_id = self.mail_group.message_post(cr, uid, self.group_pigs_id, body=_body1, subject=_subject, type='comment') message = self.mail_message.browse(cr, uid, msg_id) - mail_ids = self.mail_mail.search(cr, uid, [], limit=1) - mail = self.mail_mail.browse(cr, uid, mail_ids[0]) sent_email = self._build_email_kwargs + + self.assertFalse(self.mail_mail.search(cr, uid, [('mail_message_id','=',msg_id)]), 'mail.mail notifications should have been auto-deleted!') # Test: mail_message: subject is _subject, body is _body1 (no formatting done) self.assertEqual(message.subject, _subject, 'mail.message subject incorrect') self.assertEqual(message.body, _body1, 'mail.message body incorrect') - # Test: mail_mail: subject is _subject, body_html is _mail_body1 (signature appended) - self.assertEqual(mail.subject, _subject, 'mail.mail subject incorrect') - self.assertEqual(mail.body_html, _mail_body1, 'mail.mail body_html incorrect') - self.assertEqual(mail.mail_message_id.id, msg_id, 'mail_mail.mail_message_id is not the id of its related mail_message)') # Test: sent_email: email send by server: correct subject, body; body_alternative self.assertEqual(sent_email['subject'], _subject, 'sent_email subject incorrect') self.assertEqual(sent_email['body'], _mail_body1, 'sent_email body incorrect') @@ -283,18 +278,13 @@ class test_mail(common.TransactionCase): msg_id2 = self.mail_group.message_post(cr, uid, self.group_pigs_id, body=_body2, type='email', partner_ids=[(6, 0, [p_d_id])], parent_id=msg_id, attachments=_attachments) message = self.mail_message.browse(cr, uid, msg_id2) - mail_ids = self.mail_mail.search(cr, uid, [], limit=1) - mail = self.mail_mail.browse(cr, uid, mail_ids[0]) sent_email = self._build_email_kwargs + self.assertFalse(self.mail_mail.search(cr, uid, [('mail_message_id','=',msg_id2)]), 'mail.mail notifications should have been auto-deleted!') # Test: mail_message: subject is False, body is _body2 (no formatting done), parent_id is msg_id self.assertEqual(message.subject, False, 'mail.message subject incorrect') self.assertEqual(message.body, _body2, 'mail.message body incorrect') self.assertEqual(message.parent_id.id, msg_id, 'mail.message parent_id incorrect') - # Test: mail_mail: subject is False, body_html is _mail_body2 (signature appended) - self.assertEqual(mail.subject, False, 'mail.mail subject is incorrect') - self.assertEqual(mail.body_html, _mail_body2, 'mail.mail body_html incorrect') - self.assertEqual(mail.mail_message_id.id, msg_id2, 'mail_mail.mail_message_id incorrect') # Test: sent_email: email send by server: correct subject, body, body_alternative self.assertEqual(sent_email['subject'], _mail_subject, 'sent_email subject incorrect') self.assertEqual(sent_email['body'], _mail_body2, 'sent_email body incorrect') @@ -314,7 +304,7 @@ class test_mail(common.TransactionCase): self.assertEqual(message.attachment_ids[i].name, _attachments[i][0], 'mail.message attachment name incorrect') self.assertEqual(message.attachment_ids[i].res_model, 'mail.group', 'mail.message attachment res_model incorrect') self.assertEqual(message.attachment_ids[i].res_id, self.group_pigs_id, 'mail.message attachment res_id incorrect') - self.assertEqual(base64.b64decode(message.attachment_ids[i].datas), _attachments[i][1], 'mail.message attachment data incorrect') + self.assertEqual(message.attachment_ids[i].datas.decode('base64'), _attachments[i][1], 'mail.message attachment data incorrect') def test_21_message_compose_wizard(self): """ Tests designed for the mail.compose.message wizard. """ @@ -333,8 +323,8 @@ class test_mail(common.TransactionCase): _body_html = 'Pigs rules' _msg_body2 = 'Pigs rules' _attachments = [ - {'name': 'First', 'datas': base64.b64encode('My first attachment')}, - {'name': 'Second', 'datas': base64.b64encode('My second attachment')} + {'name': 'First', 'datas': 'My first attachment'.encode('base64')}, + {'name': 'Second', 'datas': 'My second attachment'.encode('base64')} ] # Create partners @@ -400,7 +390,7 @@ class test_mail(common.TransactionCase): self.assertEqual(message.attachment_ids[i].name, _attachments[i]['name'], 'mail.message attachment name incorrect') self.assertEqual(message.attachment_ids[i].res_model, 'mail.group', 'mail.message attachment res_model incorrect') self.assertEqual(message.attachment_ids[i].res_id, self.group_pigs_id, 'mail.message attachment res_id incorrect') - self.assertEqual(base64.b64decode(message.attachment_ids[i].datas), base64.b64decode(_attachments[i]['datas']), 'mail.message attachment data incorrect') + self.assertEqual(message.attachment_ids[i].datas.decode('base64'), _attachments[i]['datas'].decode('base64'), 'mail.message attachment data incorrect') # CASE3 - Create in mass_mail composition mode that should work with or without email_template installed compose_id = mail_compose.create(cr, uid, @@ -491,8 +481,7 @@ class test_mail(common.TransactionCase): ('read', '=', False) ]) na_count = self.mail_message._needaction_count(cr, uid, domain = []) - self.assertEqual(len(notif_ids), na_count, - 'Number of unread notifications (%s) does not match the needaction count (%s)' % (len(notif_ids), na_count)) + self.assertEqual(len(notif_ids), na_count, 'unread notifications count does not match needaction count') # Post 4 message on group_pigs for dummy in range(4): @@ -504,13 +493,11 @@ class test_mail(common.TransactionCase): ('read', '=', False) ]) na_count = self.mail_message._needaction_count(cr, uid, domain = []) - self.assertEqual(len(notif_ids), na_count, - 'Number of unread notifications after posting messages (%s) does not match the needaction count (%s)' % (len(notif_ids), na_count)) + self.assertEqual(len(notif_ids), na_count, 'unread notifications count does not match needaction count') # Check there are 4 needaction on mail.message with particular domain na_count = self.mail_message._needaction_count(cr, uid, domain = [('model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id)]) - self.assertEqual(na_count, 4, - 'Number of posted message (4) does not match the needaction count with domain mail.group - group pigs (%s)' % (na_count)) + self.assertEqual(na_count, 4, 'posted message count does not match needaction count') def test_50_thread_parent_resolution(self): """Verify parent/child relationships are correctly established when processing incoming mails"""