[MERGE] [FIX] mail, email_template: fix attachment duplication and clean attachment management

The way attachments are linked to the document has been cleaned. Posting method message_post may receives 2 arguments:
- attachment_ids: those linked to the wizard model (mail.compose.message) are linked to the document (res_model, res_id)
- attachments: the tuples are used to create new attachments linked to the document
The wizard does not set the res_model and res_id of attachments anymore, delegating this job to message_post.

The mail.message and mail.compose.message now use their respective attachment_ids field when possible. This is done instead of reading/creating new attachments based on the attachments tuple each time a mail.compose.message is processed. Email templates now also use attachment_ids, in particular when generating emails, instead of using the attachments tuple. Only reports are still generated on the fly and put into attachments instead of attachment_ids.

A cron job has been added to unlink 'lost' attachments. Those are attachments:
- linked to 'mail.compose.message'
- with res_id=0 (due Chatter used in minimal mode or reports generated by templates before the wizard has an ID)
- with no activity for more than one day (create_date and write_date)

bzr revid: tde@openerp.com-20130411112033-mqph9vjlcjkoolfs
This commit is contained in:
Thibault Delavallée 2013-04-11 13:20:33 +02:00
commit f8a2299525
7 changed files with 109 additions and 39 deletions

View File

@ -295,8 +295,7 @@ class email_template(osv.osv):
'copyvalue': self.build_expression(field_value.name, False, null_value or False),
'null_value': null_value or False
})
return {'value':result}
return {'value': result}
def generate_email(self, cr, uid, template_id, res_id, context=None):
"""Generates an email from the template for given (model, res_id) pair.
@ -349,11 +348,13 @@ class email_template(osv.osv):
report_name += ext
attachments.append((report_name, result))
attachment_ids = []
# Add template attachments
for attach in template.attachment_ids:
attachments.append((attach.datas_fname, attach.datas))
attachment_ids.append(attach.id)
values['attachments'] = attachments
values['attachment_ids'] = attachment_ids
return values
def send_mail(self, cr, uid, template_id, res_id, force_send=False, context=None):
@ -368,28 +369,34 @@ class email_template(osv.osv):
was executed for this message only.
:returns: id of the mail.message that was created
"""
if context is None: context = {}
if context is None:
context = {}
mail_mail = self.pool.get('mail.mail')
ir_attachment = self.pool.get('ir.attachment')
# create a mail_mail based on values, without attachments
values = self.generate_email(cr, uid, template_id, res_id, context=context)
assert 'email_from' in values, 'email_from is missing or empty after template rendering, send_mail() cannot proceed'
attachments = values.pop('attachments') or {}
del values['email_recipients'] # TODO Properly use them.
assert values.get('email_from'), 'email_from is missing or empty after template rendering, send_mail() cannot proceed'
del values['email_recipients'] # TODO Properly use them.
attachment_ids = values.pop('attachment_ids', [])
attachments = values.pop('attachments', [])
msg_id = mail_mail.create(cr, uid, values, context=context)
# link attachments
attachment_ids = []
for fname, fcontent in attachments.iteritems():
# manage attachments
for attachment in attachments:
attachment_data = {
'name': fname,
'datas_fname': fname,
'datas': fcontent,
'res_model': mail_mail._name,
'name': attachment[0],
'datas_fname': attachment[0],
'datas': attachment[1],
'res_model': 'mail.message',
'res_id': msg_id,
}
context.pop('default_type', None)
attachment_ids.append(ir_attachment.create(cr, uid, attachment_data, context=context))
if attachment_ids:
values['attachment_ids'] = [(6, 0, attachment_ids)]
mail_mail.write(cr, uid, msg_id, {'attachment_ids': [(6, 0, attachment_ids)]}, context=context)
if force_send:
mail_mail.send(cr, uid, [msg_id], context=context)
return msg_id

View File

@ -48,8 +48,8 @@ class test_message_compose(TestMailBase):
_body_html1 = 'Fans of Pigs, unite !'
_body_html2 = 'I am angry !'
_attachments = [
{'name': 'First', 'datas_fname': 'first.txt', 'datas': base64.b64encode('My first attachment')},
{'name': 'Second', 'datas_fname': 'second.txt', 'datas': base64.b64encode('My second attachment')}
{'name': 'First', 'datas_fname': 'first.txt', 'datas': base64.b64encode('My first attachment'), 'res_model': 'res.partner', 'res_id': self.partner_admin_id},
{'name': 'Second', 'datas_fname': 'second.txt', 'datas': base64.b64encode('My second attachment'), 'res_model': 'res.partner', 'res_id': self.partner_admin_id},
]
_attachments_test = [('first.txt', 'My first attachment'), ('second.txt', 'My second attachment')]
@ -115,12 +115,20 @@ class test_message_compose(TestMailBase):
self.assertEqual(compose.subject, _subject1, 'mail.compose.message subject incorrect')
self.assertIn(_body_html1, compose.body, 'mail.compose.message body incorrect')
self.assertEqual(set(message_pids), set(partner_ids), 'mail.compose.message partner_ids incorrect')
# Test: mail.compose.message: attachments
# Test: mail.message: attachments
# Test: mail.compose.message: attachments (owner has not been modified)
for attach in compose.attachment_ids:
self.assertEqual(attach.res_model, 'mail.group', 'mail.message attachment res_model incorrect')
self.assertEqual(attach.res_id, self.group_pigs_id, 'mail.message attachment res_id incorrect')
self.assertIn((attach.name, base64.b64decode(attach.datas)), _attachments_test,
self.assertEqual(attach.res_model, 'res.partner', 'mail.compose.message attachment res_model through templat was overriden')
self.assertEqual(attach.res_id, self.partner_admin_id, 'mail.compose.message attachment res_id incorrect')
self.assertIn((attach.datas_fname, base64.b64decode(attach.datas)), _attachments_test,
'mail.message attachment name / data incorrect')
# Test: mail.message: attachments
mail_compose.send_mail(cr, uid, [compose_id])
group_pigs.refresh()
message_pigs = group_pigs.message_ids[0]
for attach in message_pigs.attachment_ids:
self.assertEqual(attach.res_model, 'mail.group', 'mail.compose.message attachment res_model through templat was overriden')
self.assertEqual(attach.res_id, self.group_pigs_id, 'mail.compose.message attachment res_id incorrect')
self.assertIn((attach.datas_fname, base64.b64decode(attach.datas)), _attachments_test,
'mail.message attachment name / data incorrect')
# ----------------------------------------

View File

@ -61,25 +61,43 @@ class mail_compose_message(osv.TransientModel):
'template_id': fields.selection(_get_templates, 'Template', size=-1),
}
def send_mail(self, cr, uid, ids, context=None):
""" Override of send_mail to duplicate attachments linked to the email.template.
Indeed, basic mail.compose.message wizard duplicates attachments in mass
mailing mode. But in 'single post' mode, attachments of an email template
also have to be duplicated to avoid changing their ownership. """
for wizard in self.browse(cr, uid, ids, context=context):
if not wizard.attachment_ids or wizard.composition_mode == 'mass_mail' or not wizard.template_id:
continue
template = self.pool.get('email.template').browse(cr, uid, wizard.template_id, context=context)
new_attachment_ids = []
for attachment in wizard.attachment_ids:
if attachment in template.attachment_ids:
new_attachment_ids.append(self.pool.get('ir.attachment').copy(cr, uid, attachment.id, {'res_model': 'mail.compose.message', 'res_id': wizard.id}, context=context))
else:
new_attachment_ids.append(attachment.id)
self.write(cr, uid, wizard.id, {'attachment_ids': [(6, 0, new_attachment_ids)]}, context=context)
return super(mail_compose_message, self).send_mail(cr, uid, ids, context=context)
def onchange_template_id(self, cr, uid, ids, template_id, composition_mode, model, res_id, context=None):
""" - mass_mailing: we cannot render, so return the template values
- normal mode: return rendered values """
if template_id and composition_mode == 'mass_mail':
values = self.pool.get('email.template').read(cr, uid, template_id, ['subject', 'body_html'], context)
values = self.pool.get('email.template').read(cr, uid, template_id, ['subject', 'body_html', 'attachment_ids'], context)
values.pop('id')
elif template_id:
# FIXME odo: change the mail generation to avoid attachment duplication
values = self.generate_email_for_composer(cr, uid, template_id, res_id, context=context)
# transform attachments into attachment_ids
values['attachment_ids'] = []
# transform attachments into attachment_ids; not attached to the document because this will
# be done further in the posting process, allowing to clean database if email not send
values['attachment_ids'] = values.pop('attachment_ids', [])
ir_attach_obj = self.pool.get('ir.attachment')
for attach_fname, attach_datas in values.pop('attachments', []):
data_attach = {
'name': attach_fname,
'datas': attach_datas,
'datas_fname': attach_fname,
'res_model': model,
'res_id': res_id,
'res_model': 'mail.compose.message',
'res_id': 0,
'type': 'binary', # override default_type from context, possibly meant for another model!
}
values['attachment_ids'].append(ir_attach_obj.create(cr, uid, data_attach, context=context))
@ -122,7 +140,7 @@ class mail_compose_message(osv.TransientModel):
mail.compose.message, transform email_cc and email_to into partner_ids """
template_values = self.pool.get('email.template').generate_email(cr, uid, template_id, res_id, context=context)
# filter template values
fields = ['body_html', 'subject', 'email_to', 'email_recipients', 'email_cc', 'attachments']
fields = ['body_html', 'subject', 'email_to', 'email_recipients', 'email_cc', 'attachment_ids', 'attachments']
values = dict((field, template_values[field]) for field in fields if template_values.get(field))
values['body'] = values.pop('body_html', '')
@ -151,6 +169,8 @@ class mail_compose_message(osv.TransientModel):
values = self.generate_email_for_composer(cr, uid, wizard.template_id, res_id, context=context)
else:
values = {}
# remove attachments as they should not be rendered
values.pop('attachment_ids', None)
# get values to return
email_dict = super(mail_compose_message, self).render_message(cr, uid, wizard, res_id, context)
values.update(email_dict)

View File

@ -27,6 +27,19 @@
<field name="priority">1000</field>
</record>
<record id="ir_cron_mail_garbage_collect_attachments" model="ir.cron">
<field name="name">Garbage Collect Mail Attachments</field>
<field eval="True" name="active" />
<field name="user_id" ref="base.user_root" />
<field name="interval_number">1</field>
<field name="interval_type">weeks</field>
<field name="numbercall">-1</field>
<field eval="False" name="doall" />
<field name="model">mail.thread</field>
<field name="function">_garbage_collect_attachments</field>
<field name="args">()</field>
</record>
<!-- Discussion subtype for messaging / Chatter -->
<record id="mt_comment" model="mail.message.subtype">
<field name="name">Discussions</field>

View File

@ -768,7 +768,7 @@ class mail_message(osv.Model):
attachments_to_delete = []
for message in self.browse(cr, uid, ids, context=context):
for attach in message.attachment_ids:
if attach.res_model == self._name and attach.res_id == message.id:
if attach.res_model == self._name and (attach.res_id == message.id or attach.res_id == 0):
attachments_to_delete.append(attach.id)
if attachments_to_delete:
self.pool.get('ir.attachment').unlink(cr, uid, attachments_to_delete, context=context)

View File

@ -389,6 +389,26 @@ class mail_thread(osv.AbstractModel):
return [('message_unread', '=', True)]
return []
def _garbage_collect_attachments(self, cr, uid, context=None):
""" Garbage collect lost mail attachments. Those are attachments
- linked to res_model 'mail.compose.message', the composer wizard
- with res_id 0, because they were created outside of an existing
wizard (typically user input through Chatter or reports
created on-the-fly by the templates)
- unused since at least one day (create_date and write_date)
"""
limit_date = datetime.datetime.utcnow() - datetime.timedelta(days=1)
limit_date_str = datetime.datetime.strftime(limit_date, tools.DEFAULT_SERVER_DATETIME_FORMAT)
ir_attachment_obj = self.pool.get('ir.attachment')
attach_ids = ir_attachment_obj.search(cr, uid, [
('res_model', '=', 'mail.compose.message'),
('res_id', '=', 0),
('create_date', '<', limit_date_str),
('write_date', '<', limit_date_str),
], context=context)
ir_attachment_obj.unlink(cr, uid, attach_ids, context=context)
return True
#------------------------------------------------------
# Email specific
#------------------------------------------------------
@ -1026,7 +1046,6 @@ class mail_thread(osv.AbstractModel):
if attachment_ids:
filtered_attachment_ids = ir_attachment.search(cr, SUPERUSER_ID, [
('res_model', '=', 'mail.compose.message'),
('res_id', '=', 0),
('create_uid', '=', uid),
('id', 'in', attachment_ids)], context=context)
if filtered_attachment_ids:

View File

@ -19,10 +19,8 @@
#
##############################################################################
import base64
import re
from openerp import tools
from openerp import SUPERUSER_ID
from openerp.osv import osv
from openerp.osv import fields
@ -220,31 +218,36 @@ class mail_compose_message(osv.TransientModel):
email(s), rendering any template patterns on the fly if needed. """
if context is None:
context = {}
ir_attachment_obj = self.pool.get('ir.attachment')
active_ids = context.get('active_ids')
is_log = context.get('mail_compose_log', False)
for wizard in self.browse(cr, uid, ids, context=context):
mass_mail_mode = wizard.composition_mode == 'mass_mail'
active_model_pool = self.pool.get(wizard.model if wizard.model else 'mail.thread')
active_model_pool_name = wizard.model if wizard.model else 'mail.thread'
active_model_pool = self.pool.get(active_model_pool_name)
# wizard works in batch mode: [res_id] or active_ids
res_ids = active_ids if mass_mail_mode and wizard.model and active_ids else [wizard.res_id]
for res_id in res_ids:
# default values, according to the wizard options
# mail.message values, according to the wizard options
post_values = {
'subject': wizard.subject,
'body': wizard.body,
'parent_id': wizard.parent_id and wizard.parent_id.id,
'partner_ids': [partner.id for partner in wizard.partner_ids],
'attachments': [(attach.datas_fname or attach.name, base64.b64decode(attach.datas)) for attach in wizard.attachment_ids],
'attachment_ids': [attach.id for attach in wizard.attachment_ids],
}
# mass mailing: render and override default values
if mass_mail_mode and wizard.model:
email_dict = self.render_message(cr, uid, wizard, res_id, context=context)
new_partner_ids = email_dict.pop('partner_ids', [])
post_values['partner_ids'] += new_partner_ids
new_attachments = email_dict.pop('attachments', [])
post_values['attachments'] += new_attachments
post_values['partner_ids'] += email_dict.pop('partner_ids', [])
post_values['attachments'] = email_dict.pop('attachments', [])
attachment_ids = []
for attach_id in post_values.pop('attachment_ids'):
new_attach_id = ir_attachment_obj.copy(cr, uid, attach_id, {'res_model': self._name, 'res_id': wizard.id}, context=context)
attachment_ids.append(new_attach_id)
post_values['attachment_ids'] = attachment_ids
post_values.update(email_dict)
# post the message
subtype = 'mail.mt_comment'