From ee0bb635deb8bd63e375c55085e421fb0ef55c2c Mon Sep 17 00:00:00 2001 From: Anupam K Date: Wed, 19 Aug 2020 17:53:09 +0530 Subject: [PATCH 1/6] fix: newsletter fix --- .../email/doctype/newsletter/newsletter.json | 39 ++++++++++++++++--- frappe/email/doctype/newsletter/newsletter.py | 34 ++++++++-------- frappe/patches.txt | 1 + .../v13_0/update_newsletter_content_type.py | 12 ++++++ 4 files changed, 65 insertions(+), 21 deletions(-) create mode 100644 frappe/patches/v13_0/update_newsletter_content_type.py diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index 01f75be954..6babf212aa 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -15,7 +15,10 @@ "email_sent", "newsletter_content", "subject", + "content_type", "message", + "message_md", + "message_html", "send_unsubscribe_link", "send_attachments", "published", @@ -50,7 +53,8 @@ }, { "fieldname": "newsletter_content", - "fieldtype": "Section Break" + "fieldtype": "Section Break", + "label": "Content" }, { "fieldname": "subject", @@ -61,11 +65,12 @@ "reqd": 1 }, { + "depends_on": "eval: doc.content_type === 'Rich Text'", "fieldname": "message", "fieldtype": "Text Editor", "in_list_view": 1, "label": "Message", - "reqd": 1 + "mandatory_depends_on": "eval: doc.content_type === 'Rich Text'" }, { "default": "1", @@ -87,16 +92,20 @@ "read_only": 1 }, { + "collapsible": 1, "fieldname": "test_the_newsletter", - "fieldtype": "Section Break" + "fieldtype": "Section Break", + "label": "Testing" }, { "description": "A Lead with this Email Address should exist", "fieldname": "test_email_id", "fieldtype": "Data", - "label": "Test Email Address" + "label": "Test Email Address", + "options": "Email" }, { + "depends_on": "eval: doc.test_email_id", "fieldname": "test_send", "fieldtype": "Button", "label": "Test", @@ -127,6 +136,26 @@ "fieldname": "send_attachments", "fieldtype": "Check", "label": "Send Attachments" + }, + { + "fieldname": "content_type", + "fieldtype": "Select", + "label": "Content Type", + "options": "Rich Text\nMarkdown\nHTML" + }, + { + "depends_on": "eval:doc.content_type === 'Markdown'", + "fieldname": "message_md", + "fieldtype": "Markdown Editor", + "label": "Message (Markdown)", + "mandatory_depends_on": "eval:doc.content_type === 'Markdown'" + }, + { + "depends_on": "eval:doc.content_type === 'HTML'", + "fieldname": "message_html", + "fieldtype": "HTML Editor", + "label": "Message (HTML)", + "mandatory_depends_on": "eval:doc.content_type === 'HTML'" } ], "has_web_view": 1, @@ -135,7 +164,7 @@ "is_published_field": "published", "links": [], "max_attachments": 3, - "modified": "2020-05-12 18:09:40.137138", + "modified": "2020-08-19 17:54:13.290593", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 48688afdb6..08a0e3d247 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -11,7 +11,7 @@ from frappe.utils.verified_command import get_signed_params, verify_request from frappe.utils.background_jobs import enqueue from frappe.email.queue import send from frappe.email.doctype.email_group.email_group import add_subscribers -from frappe.utils import parse_addr, now_datetime +from frappe.utils import parse_addr, now_datetime, markdown from frappe.utils import validate_email_address @@ -29,8 +29,8 @@ class Newsletter(WebsiteGenerator): def test_send(self, doctype="Lead"): self.recipients = frappe.utils.split_emails(self.test_email_id) - self.queue_all() - frappe.msgprint(_("Scheduled to send to {0}").format(self.test_email_id)) + self.queue_all(test_email=True) + frappe.msgprint(_("Test email is send to {0}").format(self.test_email_id)) def send_emails(self): """send emails to leads and customers""" @@ -41,20 +41,13 @@ class Newsletter(WebsiteGenerator): if self.recipients: if getattr(frappe.local, "is_ajax", False): - self.validate_send() - # using default queue with a longer timeout as this isn't a scheduled task - enqueue(send_newsletter, queue='default', timeout=6000, event='send_newsletter', - newsletter=self.name) - - else: self.queue_all() - - frappe.msgprint(_("Scheduled to send to {0} recipients").format(len(self.recipients))) + frappe.msgprint(_("Email queued to {0} recipients").format(len(self.recipients))) else: frappe.msgprint(_("Newsletter should have atleast one recipient")) - def queue_all(self): + def queue_all(self, test_email=False): if not self.get("recipients"): # in case it is called via worker self.recipients = self.get_recipients() @@ -80,7 +73,7 @@ class Newsletter(WebsiteGenerator): frappe.throw(_("Unable to find attachment {0}").format(file.name)) send(recipients=self.recipients, sender=sender, - subject=self.subject, message=self.message, + subject=self.subject, message=self.get_message(), reference_doctype=self.doctype, reference_name=self.name, add_unsubscribe_link=self.send_unsubscribe_link, attachments=attachments, unsubscribe_method="/unsubscribe", @@ -90,9 +83,18 @@ class Newsletter(WebsiteGenerator): if not frappe.flags.in_test: frappe.db.auto_commit_on_many_writes = False - self.db_set("email_sent", 1) - self.db_set("schedule_send", now_datetime()) - self.db_set("scheduled_to_send", len(self.recipients)) + if not test_email: + self.db_set("email_sent", 1) + self.db_set("schedule_send", now_datetime()) + self.db_set("scheduled_to_send", len(self.recipients)) + + def get_message(self): + if self.content_type == 'Rich Text': + return self.message + elif self.content_type == 'Markdown': + return markdown(self.message_md) + elif self.content_type == 'HTML': + return self.message_html def get_recipients(self): """Get recipients from Email Group""" diff --git a/frappe/patches.txt b/frappe/patches.txt index 75750ab59c..bf92486cbf 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -297,3 +297,4 @@ frappe.patches.v13_0.replace_old_data_import # 2020-06-24 frappe.patches.v13_0.create_custom_dashboards_cards_and_charts frappe.patches.v13_0.rename_is_custom_field_in_dashboard_chart frappe.patches.v13_0.generate_theme_files_in_public_folder +frappe.patches.v13_0.update_newsletter_content_type \ No newline at end of file diff --git a/frappe/patches/v13_0/update_newsletter_content_type.py b/frappe/patches/v13_0/update_newsletter_content_type.py new file mode 100644 index 0000000000..0b32fb49ed --- /dev/null +++ b/frappe/patches/v13_0/update_newsletter_content_type.py @@ -0,0 +1,12 @@ +# Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors +# MIT License. See license.txt + +from __future__ import unicode_literals +import frappe + +def execute(): + frappe.reload_doc('email', 'doctype', 'Newsletter') + frappe.db.sql(""" + UPDATE tabNewsletter + SET content_type = 'Rich Text' + """) \ No newline at end of file From 178a576b6a39a252415c9ef5b49a57a7cef2c034 Mon Sep 17 00:00:00 2001 From: Anupam K Date: Mon, 24 Aug 2020 20:11:25 +0530 Subject: [PATCH 2/6] fix: review changes --- frappe/email/doctype/newsletter/newsletter.js | 4 ---- frappe/email/doctype/newsletter/newsletter.json | 8 +++++--- frappe/email/doctype/newsletter/newsletter.py | 14 +++++++------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.js b/frappe/email/doctype/newsletter/newsletter.js index 3d69c0cfad..3277d8e9ee 100644 --- a/frappe/email/doctype/newsletter/newsletter.js +++ b/frappe/email/doctype/newsletter/newsletter.js @@ -14,10 +14,6 @@ frappe.ui.form.on('Newsletter', { }); }, "fa fa-play", "btn-success"); } - if (!doc.__islocal && cint(doc.email_sent)) { - frm.set_df_property('schedule_send', "read_only", 1); - frm.set_df_property('schedule_sending', "read_only", 1); - } frm.events.setup_dashboard(frm); diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index fac211ab49..1dd6115b43 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -125,7 +125,8 @@ "depends_on": "eval: doc.schedule_sending", "fieldname": "schedule_send", "fieldtype": "Datetime", - "label": "Schedule Send" + "label": "Schedule Send", + "read_only_depends_on": "eval: doc.email_sent" }, { "default": "0", @@ -157,7 +158,8 @@ "default": "0", "fieldname": "schedule_sending", "fieldtype": "Check", - "label": "Schedule Sending" + "label": "Schedule Sending", + "read_only_depends_on": "eval: doc.email_sent" } ], "has_web_view": 1, @@ -167,7 +169,7 @@ "is_published_field": "published", "links": [], "max_attachments": 3, - "modified": "2020-08-20 10:22:24.560288", + "modified": "2020-08-24 19:59:37.262500", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index afc0fa638b..929855ea30 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -27,7 +27,7 @@ class Newsletter(WebsiteGenerator): def test_send(self, doctype="Lead"): self.recipients = frappe.utils.split_emails(self.test_email_id) self.queue_all(test_email=True) - frappe.msgprint(_("Test email is send to {0}").format(self.test_email_id)) + frappe.msgprint(_("Test email sent to {0}").format(self.test_email_id)) def send_emails(self): """send emails to leads and customers""" @@ -86,12 +86,12 @@ class Newsletter(WebsiteGenerator): self.db_set("scheduled_to_send", len(self.recipients)) def get_message(self): - if self.content_type == 'Rich Text': - return self.message - elif self.content_type == 'Markdown': - return markdown(self.message_md) - elif self.content_type == 'HTML': - return self.message_html + + return { + 'Rich Text': self.message, + 'Markdown': markdown(self.message_md), + 'HTML': self.message_html + }[self.content_type] def get_recipients(self): """Get recipients from Email Group""" From dcce45b558f1a7d602214540f81f53a1ad3b888d Mon Sep 17 00:00:00 2001 From: Anupam K Date: Mon, 31 Aug 2020 11:54:56 +0530 Subject: [PATCH 3/6] fix: test case --- frappe/email/doctype/newsletter/test_newsletter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index bb339165d3..ee7f123b7e 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/frappe/email/doctype/newsletter/test_newsletter.py @@ -67,6 +67,7 @@ class TestNewsletter(unittest.TestCase): "doctype": "Newsletter", "subject": "_Test Newsletter", "send_from": "Test Sender ", + "content_type": "Rich Text", "message": "Testing my news.", "published": published, "schedule_sending": bool(schedule_send), From 8f819da4ff7afdf9c218d0bf45417ee9fe167704 Mon Sep 17 00:00:00 2001 From: Anupam K Date: Tue, 1 Sep 2020 12:43:58 +0530 Subject: [PATCH 4/6] fix: failed test case --- frappe/email/doctype/newsletter/newsletter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 929855ea30..14bd7eb1dc 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -37,8 +37,8 @@ class Newsletter(WebsiteGenerator): self.recipients = self.get_recipients() if self.recipients: + self.queue_all() if getattr(frappe.local, "is_ajax", False): - self.queue_all() frappe.msgprint(_("Email queued to {0} recipients").format(len(self.recipients))) else: From aecb77ce9b5023dab233e55ee918cb87f169812e Mon Sep 17 00:00:00 2001 From: Anupam K Date: Wed, 2 Sep 2020 12:19:34 +0530 Subject: [PATCH 5/6] fix: removing condition --- frappe/email/doctype/newsletter/newsletter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 14bd7eb1dc..0a0a13a6ce 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -38,8 +38,7 @@ class Newsletter(WebsiteGenerator): if self.recipients: self.queue_all() - if getattr(frappe.local, "is_ajax", False): - frappe.msgprint(_("Email queued to {0} recipients").format(len(self.recipients))) + frappe.msgprint(_("Email queued to {0} recipients").format(len(self.recipients))) else: frappe.msgprint(_("Newsletter should have atleast one recipient")) From 87c06ef5a49a86150680f9102f81f0883a567285 Mon Sep 17 00:00:00 2001 From: Anupam K Date: Wed, 2 Sep 2020 17:19:00 +0530 Subject: [PATCH 6/6] fix: tabs --- frappe/patches/v13_0/update_newsletter_content_type.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/patches/v13_0/update_newsletter_content_type.py b/frappe/patches/v13_0/update_newsletter_content_type.py index 0b32fb49ed..01f4bacd16 100644 --- a/frappe/patches/v13_0/update_newsletter_content_type.py +++ b/frappe/patches/v13_0/update_newsletter_content_type.py @@ -5,8 +5,8 @@ from __future__ import unicode_literals import frappe def execute(): - frappe.reload_doc('email', 'doctype', 'Newsletter') - frappe.db.sql(""" - UPDATE tabNewsletter - SET content_type = 'Rich Text' - """) \ No newline at end of file + frappe.reload_doc('email', 'doctype', 'Newsletter') + frappe.db.sql(""" + UPDATE tabNewsletter + SET content_type = 'Rich Text' + """) \ No newline at end of file