From 9bdb5f2eb2404a4a6db855659d7c8f55cdb9b518 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 6 Dec 2021 17:04:24 +0530 Subject: [PATCH] fix: Explicit attachments table The newsletter content may contain images that get "attached" to the newsletter document. If this is the case, you can't selectively include attachments in the newsletter as it attaches all the attachments. An explicit attachments table solves this problem. --- .../email/doctype/email_queue/email_queue.py | 14 ++++++--- .../email/doctype/newsletter/newsletter.json | 23 +++++--------- frappe/email/doctype/newsletter/newsletter.py | 13 +------- .../doctype/newsletter_attachment/__init__.py | 0 .../newsletter_attachment.json | 31 +++++++++++++++++++ .../newsletter_attachment.py | 8 +++++ 6 files changed, 58 insertions(+), 31 deletions(-) create mode 100644 frappe/email/doctype/newsletter_attachment/__init__.py create mode 100644 frappe/email/doctype/newsletter_attachment/newsletter_attachment.json create mode 100644 frappe/email/doctype/newsletter_attachment/newsletter_attachment.py diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 4489a68cac..077a5dd40b 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -283,9 +283,14 @@ class SendMailContext: if attachment.get('fcontent'): continue - fid = attachment.get("fid") - if fid: - _file = frappe.get_doc("File", fid) + file_filters = {} + if attachment.get('fid'): + file_filters['name'] = attachment.get('fid') + elif attachment.get('file_url'): + file_filters['file_url'] = attachment.get('file_url') + + if file_filters: + _file = frappe.get_doc("File", file_filters) fcontent = _file.get_content() attachment.update({ 'fname': _file.file_name, @@ -293,6 +298,7 @@ class SendMailContext: 'parent': message_obj }) attachment.pop("fid", None) + attachment.pop("file_url", None) add_attachment(**attachment) elif attachment.get("print_format_attachment") == 1: @@ -503,7 +509,7 @@ class QueueBuilder: if self._attachments: # store attachments with fid or print format details, to be attached on-demand later for att in self._attachments: - if att.get('fid'): + if att.get('fid') or att.get('file_url'): attachments.append(att) elif att.get("print_format_attachment") == 1: if not att.get('lang', None): diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index ccb2ca8181..9d35671042 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -18,14 +18,13 @@ "email_sent", "subject_section", "subject", - "preview_text", "newsletter_content", "content_type", "message", "message_md", "message_html", "send_unsubscribe_link", - "send_attachments", + "attachments", "send_webview_link", "schedule_settings_section", "scheduled_to_send", @@ -116,12 +115,6 @@ "read_only": 1, "read_only_depends_on": "eval: doc.email_sent" }, - { - "default": "0", - "fieldname": "send_attachments", - "fieldtype": "Check", - "label": "Send Attachments" - }, { "fieldname": "content_type", "fieldtype": "Select", @@ -186,12 +179,6 @@ "fieldtype": "Section Break", "label": "Subject" }, - { - "description": "Preview Text appears in the inbox after the subject line", - "fieldname": "preview_text", - "fieldtype": "Data", - "label": "Preview Text" - }, { "fieldname": "publish_as_a_web_page_section", "fieldtype": "Section Break", @@ -202,6 +189,12 @@ "fieldname": "schedule_settings_section", "fieldtype": "Section Break", "label": "Scheduled Sending" + }, + { + "fieldname": "attachments", + "fieldtype": "Table", + "label": "Attachments", + "options": "Newsletter Attachment" } ], "has_web_view": 1, @@ -211,7 +204,7 @@ "is_published_field": "published", "links": [], "max_attachments": 3, - "modified": "2021-12-03 17:50:12.028162", + "modified": "2021-12-06 17:01:32.353153", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index acaa1dbcc1..b2146ed100 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -164,18 +164,7 @@ class Newsletter(WebsiteGenerator): def get_newsletter_attachments(self) -> List[Dict[str, str]]: """Get list of attachments on current Newsletter """ - attachments = [] - - if self.send_attachments: - files = frappe.get_all( - "File", - filters={"attached_to_doctype": "Newsletter", "attached_to_name": self.name}, - order_by="creation desc", - pluck="name", - ) - attachments.extend({"fid": file} for file in files) - - return attachments + return [{"file_url": row.attachment} for row in self.attachments] def send_newsletter(self, emails: List[str]): """Trigger email generation for `emails` and add it in Email Queue. diff --git a/frappe/email/doctype/newsletter_attachment/__init__.py b/frappe/email/doctype/newsletter_attachment/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/email/doctype/newsletter_attachment/newsletter_attachment.json b/frappe/email/doctype/newsletter_attachment/newsletter_attachment.json new file mode 100644 index 0000000000..e2add0ed64 --- /dev/null +++ b/frappe/email/doctype/newsletter_attachment/newsletter_attachment.json @@ -0,0 +1,31 @@ +{ + "actions": [], + "allow_rename": 1, + "creation": "2021-12-06 16:37:40.652468", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "attachment" + ], + "fields": [ + { + "fieldname": "attachment", + "fieldtype": "Attach", + "in_list_view": 1, + "label": "Attachment", + "reqd": 1 + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2021-12-06 16:37:47.481057", + "modified_by": "Administrator", + "module": "Email", + "name": "Newsletter Attachment", + "owner": "Administrator", + "permissions": [], + "sort_field": "modified", + "sort_order": "DESC" +} \ No newline at end of file diff --git a/frappe/email/doctype/newsletter_attachment/newsletter_attachment.py b/frappe/email/doctype/newsletter_attachment/newsletter_attachment.py new file mode 100644 index 0000000000..7842badbe1 --- /dev/null +++ b/frappe/email/doctype/newsletter_attachment/newsletter_attachment.py @@ -0,0 +1,8 @@ +# Copyright (c) 2021, Frappe Technologies and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + +class NewsletterAttachment(Document): + pass