From 69dc6d845a0ae08277e2994f4a31187b7fe5eb71 Mon Sep 17 00:00:00 2001 From: Rodolpho Lima Date: Fri, 8 Sep 2023 18:30:38 +0200 Subject: [PATCH] [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 --- addons/mass_mailing/models/mailing.py | 47 ++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index 082bed3c1a0..71566ef5b47 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -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()