[FIX] mass_mailing: multiple attachments for same image

Before this commit, base64-encoded images were converted to attachments
only for the body_html field, leaving the body_arch's content unchanged.
As a result, every time a record was updated, the body_html content,
which is generated from the body_arch's content, would have the same
base64-encoded images, which were converted again to new attachments.

Moreover:
- the created attachments were not linked to the mailing record (no
  res_model and res_id).
- the attachment name contained "cropped_image", even for images that
  were not a result of cropping.
- when the image conversion happened for a record in creation, the
  resulting attachment name was "cropped_image_mailing_False_0" (or any
  other number, the relevant part is "False", where the record's id was
  expected).
- the body_arch could take an excessively large amount of space in the
  database table, due to its base64-encoded images (see [1]).

This commit makes sure the body_arch has its images converted to
attachments as well, and also makes sure such attachments are reused
when replacing the image sources in the body_html, instead of creating
new ones.

Besides, the "cropped_image_mailing" in the attachment's name was
replaced by the more generic "image_mailing".

Lastly, when a record is being created, the conversion to
attachments now happens after the call to super().create, so that the
record's id is known when creating attachments.

task-3479586

[1]: https://github.com/odoo/odoo/pull/128452

X-original-commit: 9c5a56d2be9c1a480d526755327c79e2daa90a35
Part-of: odoo/odoo#152065
This commit is contained in:
Rodolpho Lima
2024-02-01 07:06:55 +00:00
parent 3e41ca178e
commit 69dc6d845a
+38 -9
View File
@@ -505,8 +505,6 @@ class MassMailing(models.Model):
def create(self, vals_list):
ab_testing_cron = self.env.ref('mass_mailing.ir_cron_mass_mailing_ab_testing').sudo()
for values in vals_list:
if values.get('body_html'):
values['body_html'] = self._convert_inline_images_to_urls(values['body_html'])
if values.get('ab_testing_schedule_datetime'):
at = fields.Datetime.from_string(values['ab_testing_schedule_datetime'])
ab_testing_cron._trigger(at=at)
@@ -514,9 +512,16 @@ class MassMailing(models.Model):
mailings._create_ab_testing_utm_campaigns()
mailings._fix_attachment_ownership()
for values, mailing in zip(vals_list, mailings):
if values.get('body_arch'):
mailing.body_arch = mailing._convert_inline_images_to_urls(mailing.body_arch)
if values.get('body_html'):
mailing.body_html = mailing._convert_inline_images_to_urls(mailing.body_html)
return mailings
def write(self, values):
if values.get('body_arch'):
values['body_arch'] = self._convert_inline_images_to_urls(values['body_arch'])
if values.get('body_html'):
values['body_html'] = self._convert_inline_images_to_urls(values['body_html'])
# If ab_testing is already enabled on a mailing and the campaign is removed, we raise a ValidationError
@@ -1289,14 +1294,14 @@ class MassMailing(models.Model):
# TOOLS
# ------------------------------------------------------
def _convert_inline_images_to_urls(self, body_html):
def _convert_inline_images_to_urls(self, html_content):
"""
Find inline base64 encoded images, make an attachement out of
them and replace the inline image with an url to the attachement.
Find VML v:image elements, crop their source images, make an attachement
out of them and replace their source with an url to the attachement.
"""
root = lxml.html.fromstring(body_html)
root = lxml.html.fromstring(html_content)
did_modify_body = False
conversion_info = [] # list of tuples (image: base64 image, node: lxml node, old_url: string or None))
@@ -1357,16 +1362,40 @@ class MassMailing(models.Model):
if did_modify_body:
return lxml.html.tostring(root, encoding='unicode')
return body_html
return html_content
def _create_attachments_from_inline_images(self, b64images):
if not b64images:
return []
attachments = self.env['ir.attachment'].create([{
'datas': b64image,
'name': f"cropped_image_mailing_{self.id}_{i}",
'type': 'binary',} for i, b64image in enumerate(b64images)])
IrAttachment = self.env['ir.attachment']
existing_attachments = dict(IrAttachment.search([
('res_model', '=', 'mailing.mailing'),
('res_id', '=', self.id),
]).mapped(lambda record: (record.checksum, record)))
attachments, vals_for_attachs = [], []
next_img_id = len(existing_attachments)
for b64image in b64images:
checksum = IrAttachment._compute_checksum(base64.b64decode(b64image))
existing_attach = existing_attachments.get(checksum)
# Existing_attach can be None, in which case it acts as placeholder
# for attachment to be created.
attachments.append(existing_attach)
if not existing_attach:
vals_for_attachs.append({
'datas': b64image,
'name': f"image_mailing_{self.id}_{next_img_id}",
'type': 'binary',
'res_id': self.id,
'res_model': 'mailing.mailing'
})
next_img_id += 1
new_attachments = iter(IrAttachment.create(vals_for_attachs))
# Replace None entries by newly created attachments.
attachments = [(attach or next(new_attachments)) for attach in attachments]
urls = []
for attachment in attachments:
attachment.generate_access_token()