From da4160e2dd0c9c47e207b51689d9a276221b50af Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 3 Dec 2021 18:22:25 +0530 Subject: [PATCH 01/14] fix: Newsletter enhancements and fixes - Organize fields into sections - Buttons for Send now and Scheduled sending - Buttons to Send test email and to Check broken links - Remove Test section --- frappe/email/doctype/newsletter/newsletter.js | 144 ++++++++++++------ .../email/doctype/newsletter/newsletter.json | 110 +++++++------ frappe/email/doctype/newsletter/newsletter.py | 31 +++- .../newsletter/templates/newsletter.html | 4 +- 4 files changed, 194 insertions(+), 95 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.js b/frappe/email/doctype/newsletter/newsletter.js index 3277d8e9ee..a7cbcf702a 100644 --- a/frappe/email/doctype/newsletter/newsletter.js +++ b/frappe/email/doctype/newsletter/newsletter.js @@ -4,69 +4,123 @@ frappe.ui.form.on('Newsletter', { refresh(frm) { let doc = frm.doc; - if (!doc.__islocal && !cint(doc.email_sent) && !doc.__unsaved - && in_list(frappe.boot.user.can_write, doc.doctype)) { - frm.add_custom_button(__('Send Now'), function() { - frappe.confirm(__("Do you really want to send this email newsletter?"), function() { - frm.call('send_emails').then(() => { - frm.refresh(); - }); + let can_write = in_list(frappe.boot.user.can_write, doc.doctype); + if (!frm.is_new() && !frm.is_dirty() && !doc.email_sent && can_write) { + frm.add_custom_button(__('Send a test email'), () => { + frm.events.send_test_email(frm); + }, __('Preview')); + + frm.add_custom_button(__('Check broken links'), () => { + frm.call('find_broken_links').then(r => { + let links = r.message; + if (links) { + let html = ''; + frappe.msgprint({ + title: __("Broken Links"), + message: __("Following links are broken in the email content: {0}", [html]), + indicator: "red" + }) + } else { + frappe.msgprint({ + title: _("No Broken Links"), + message: _("No broken links found in the email content"), + indicator: "green" + }) + } + }) + }, __('Preview')); + + frm.add_custom_button(__('Send now'), () => { + frappe.confirm(__("Do you really want to send this email newsletter?"), function () { + frm.call('send_emails').then(() => frm.refresh()); }); - }, "fa fa-play", "btn-success"); + }, __('Send')); + + frm.add_custom_button(__('Schedule sending'), () => { + frm.events.schedule_send_dialog(frm); + }, __('Send')); } frm.events.setup_dashboard(frm); - if (doc.__islocal && !doc.send_from) { + if (frm.is_new() && !doc.sender_email) { let { fullname, email } = frappe.user_info(doc.owner); - frm.set_value('send_from', `${fullname} <${email}>`); + frm.set_value('sender_email', email); + frm.set_value('sender_name', fullname); } }, - onload_post_render(frm) { - frm.trigger('setup_schedule_send'); - }, - - setup_schedule_send(frm) { - let today = new Date(); - - // setting datepicker options to set min date & min time - today.setHours(today.getHours() + 1 ); - frm.get_field('schedule_send').$input.datepicker({ - maxMinutes: 0, - minDate: today, - timeFormat: 'hh:00:00', - onSelect: function (fd, d, picker) { - if (!d) return; - var date = d.toDateString(); - if (date === today.toDateString()) { - picker.update({ - minHours: (today.getHours() + 1) - }); - } else { - picker.update({ - minHours: 0 - }); - } - frm.get_field('schedule_send').$input.trigger('change'); + schedule_send_dialog(frm) { + let hours = frappe.utils.range(24); + let time_slots = hours.map(hour => { + return `${(hour + '').padStart(2, '0')}:00`; + }); + let d = new frappe.ui.Dialog({ + title: __('Schedule Newsletter'), + fields: [ + { + label: __('Date'), + fieldname: 'date', + fieldtype: 'Date', + options: { + minDate: new Date() + } + }, + { + label: __('Time'), + fieldname: 'time', + fieldtype: 'Select', + options: time_slots, + }, + ], + primary_action_label: __('Schedule'), + primary_action({ date, time }) { + frm.set_value('schedule_sending', 1); + frm.set_value('schedule_send', `${date} ${time}`); + d.hide(); } }); + if (frm.doc.schedule_sending) { + let parts = frm.doc.schedule_send.split(' '); + if (parts.length === 2) { + let [date, time] = parts; + d.set_value('date', date); + d.set_value('time', time); + } + } + d.show(); + }, - - const $tp = frm.get_field('schedule_send').datepicker.timepicker; - $tp.$minutes.parent().css('display', 'none'); - $tp.$minutesText.css('display', 'none'); - $tp.$minutesText.prev().css('display', 'none'); - $tp.$seconds.parent().css('display', 'none'); + send_test_email(frm) { + let d = new frappe.ui.Dialog({ + title: __('Send Test Email'), + fields: [ + { + label: __('Email'), + fieldname: 'email', + fieldtype: 'Data', + options: 'Email', + } + ], + primary_action_label: __('Send'), + primary_action({ email }) { + d.get_primary_btn().text(__('Sending...')).prop('disabled', true); + frm.call('send_test_email', { email }) + .then(() => { + d.get_primary_btn().text(__('Send again')).prop('disabled', false); + }); + } + }); + d.show(); }, setup_dashboard(frm) { - if(!frm.doc.__islocal && cint(frm.doc.email_sent) + if (!frm.doc.__islocal && cint(frm.doc.email_sent) && frm.doc.__onload && frm.doc.__onload.status_count) { var stat = frm.doc.__onload.status_count; var total = frm.doc.scheduled_to_send; - if(total) { - $.each(stat, function(k, v) { + if (total) { + $.each(stat, function (k, v) { stat[k] = flt(v * 100 / total, 2) + '%'; }); diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index dcd19ed33c..ccb2ca8181 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -7,29 +7,33 @@ "document_type": "Other", "engine": "InnoDB", "field_order": [ + "from_section", + "sender_name", + "column_break_5", + "sender_email", + "column_break_7", "send_from", - "schedule_sending", - "schedule_send", "recipients", "email_group", "email_sent", - "newsletter_content", + "subject_section", "subject", + "preview_text", + "newsletter_content", "content_type", "message", "message_md", "message_html", - "section_break_13", "send_unsubscribe_link", "send_attachments", - "column_break_9", - "published", "send_webview_link", - "route", - "test_the_newsletter", - "test_email_id", - "test_send", - "scheduled_to_send" + "schedule_settings_section", + "scheduled_to_send", + "schedule_sending", + "schedule_send", + "publish_as_a_web_page_section", + "published", + "route" ], "fields": [ { @@ -43,7 +47,8 @@ "fieldname": "send_from", "fieldtype": "Data", "ignore_xss_filter": 1, - "label": "Sender" + "label": "Sender", + "read_only": 1 }, { "default": "0", @@ -89,30 +94,9 @@ { "fieldname": "route", "fieldtype": "Data", - "hidden": 1, "label": "Route", "read_only": 1 }, - { - "collapsible": 1, - "fieldname": "test_the_newsletter", - "fieldtype": "Section Break", - "label": "Testing" - }, - { - "description": "A Lead with this Email Address should exist", - "fieldname": "test_email_id", - "fieldtype": "Data", - "label": "Test Email Address", - "options": "Email" - }, - { - "depends_on": "eval: doc.test_email_id", - "fieldname": "test_send", - "fieldtype": "Button", - "label": "Test", - "options": "test_send" - }, { "fieldname": "scheduled_to_send", "fieldtype": "Int", @@ -122,13 +106,14 @@ { "fieldname": "recipients", "fieldtype": "Section Break", - "label": "Recipients" + "label": "To" }, { "depends_on": "eval: doc.schedule_sending", "fieldname": "schedule_send", "fieldtype": "Datetime", - "label": "Schedule Send", + "label": "Send Email At", + "read_only": 1, "read_only_depends_on": "eval: doc.email_sent" }, { @@ -161,13 +146,9 @@ "default": "0", "fieldname": "schedule_sending", "fieldtype": "Check", - "label": "Schedule Sending", + "label": "Schedule sending at a later time", "read_only_depends_on": "eval: doc.email_sent" }, - { - "fieldname": "column_break_9", - "fieldtype": "Column Break" - }, { "default": "0", "depends_on": "published", @@ -176,8 +157,51 @@ "label": "Send Web View Link" }, { - "fieldname": "section_break_13", - "fieldtype": "Section Break" + "fieldname": "from_section", + "fieldtype": "Section Break", + "label": "From" + }, + { + "fieldname": "sender_name", + "fieldtype": "Data", + "label": "Sender Name" + }, + { + "fieldname": "sender_email", + "fieldtype": "Data", + "label": "Sender Email", + "options": "Email", + "reqd": 1 + }, + { + "fieldname": "column_break_5", + "fieldtype": "Column Break" + }, + { + "fieldname": "column_break_7", + "fieldtype": "Column Break" + }, + { + "fieldname": "subject_section", + "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", + "label": "Publish as a web page" + }, + { + "depends_on": "schedule_sending", + "fieldname": "schedule_settings_section", + "fieldtype": "Section Break", + "label": "Scheduled Sending" } ], "has_web_view": 1, @@ -187,7 +211,7 @@ "is_published_field": "published", "links": [], "max_attachments": 3, - "modified": "2021-02-22 14:33:56.095380", + "modified": "2021-12-03 17:50:12.028162", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 12fe160c9d..acaa1dbcc1 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -30,10 +30,30 @@ class Newsletter(WebsiteGenerator): return self._recipients @frappe.whitelist() - def test_send(self): - test_emails = frappe.utils.split_emails(self.test_email_id) + def send_test_email(self, email): + test_emails = frappe.utils.split_emails(email) self.queue_all(test_emails=test_emails) - frappe.msgprint(_("Test email sent to {0}").format(self.test_email_id)) + frappe.msgprint(_("Test email sent to {0}").format(email), alert=True) + + @frappe.whitelist() + def find_broken_links(self): + from bs4 import BeautifulSoup + import requests + + html = self.get_message() + soup = BeautifulSoup(html, "html.parser") + links = soup.find_all("a") + images = soup.find_all("img") + broken_links = [] + for el in links + images: + url = el.attrs.get("href") or el.attrs.get("src") + try: + response = requests.head(url, verify=False, timeout=5) + if response.status_code >= 400: + broken_links.append(url) + except: + broken_links.append(url) + return broken_links @frappe.whitelist() def send_emails(self): @@ -75,8 +95,9 @@ class Newsletter(WebsiteGenerator): def validate_sender_address(self): """Validate self.send_from is a valid email address or not. """ - if self.send_from: - frappe.utils.validate_email_address(self.send_from, throw=True) + if self.sender_email: + frappe.utils.validate_email_address(self.sender_email, throw=True) + self.send_from = f"{self.sender_name} <{self.sender_email}>" if self.sender_name else self.sender_email def validate_recipient_address(self): """Validate if self.newsletter_recipients are all valid email addresses or not. diff --git a/frappe/email/doctype/newsletter/templates/newsletter.html b/frappe/email/doctype/newsletter/templates/newsletter.html index 733c7df6af..11ee19a550 100644 --- a/frappe/email/doctype/newsletter/templates/newsletter.html +++ b/frappe/email/doctype/newsletter/templates/newsletter.html @@ -36,7 +36,7 @@

- {{ doc.message }} + {{ doc.get_message() }}
@@ -51,7 +51,7 @@
{% for attachment in attachments %}

- + {{ attachment.file_name }}

From f6379fdf40ff75e93bd672218a7171697e5e95ec Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 3 Dec 2021 18:23:28 +0530 Subject: [PATCH 02/14] fix: allow options in datepicker via df.options ability to customize datepicker options via df.options --- frappe/public/js/frappe/form/controls/date.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/date.js b/frappe/public/js/frappe/form/controls/date.js index 9ad81c7e46..b9945060cd 100644 --- a/frappe/public/js/frappe/form/controls/date.js +++ b/frappe/public/js/frappe/form/controls/date.js @@ -73,7 +73,8 @@ frappe.ui.form.ControlDate = class ControlDate extends frappe.ui.form.ControlDat .text(this.today_text); this.update_datepicker_position(); - } + }, + ...(this.get_df_options()) }; } set_datepicker() { @@ -150,4 +151,19 @@ frappe.ui.form.ControlDate = class ControlDate extends frappe.ui.form.ControlDat } return value; } + get_df_options() { + let options = {}; + let df_options = this.df.options || ''; + if (typeof df_options === 'string') { + try { + options = JSON.parse(df_options); + } catch (error) { + console.warn(`Invalid JSON in options of "${this.df.fieldname}"`); + } + } + else if (typeof df_options === 'object') { + options = df_options; + } + return options; + } }; From 742d5e2e0691107657497b0574cef8c110782011 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 3 Dec 2021 18:23:51 +0530 Subject: [PATCH 03/14] fix: utility method to create a range of values mimics python's range function --- frappe/public/js/frappe/utils/utils.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 2dbad5427d..5f546c22da 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1376,5 +1376,18 @@ Object.assign(frappe.utils, { return array; } return undefined; + }, + + // simple implementation of python's range + range(start, end) { + if (!end) { + end = start; + start = 0; + } + let arr = []; + for (let i = start; i < end; i++) { + arr.push(i); + } + return arr; } }); From 02759631b498ebed638b2764585027378e4326ca Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 6 Dec 2021 15:35:41 +0530 Subject: [PATCH 04/14] fix: handle falsy return values in document methods problem: if a whitelisted document method returns a falsy value like `[]`, `{}`, `0` then response.message is not set and not returned in the response. this change checks if the return value is `None` and falsy values are returned properly in the response --- frappe/handler.py | 2 +- frappe/model/document.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/handler.py b/frappe/handler.py index 42c17261b4..35063ee9d6 100755 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -255,7 +255,7 @@ def run_doc_method(method, docs=None, dt=None, dn=None, arg=None, args=None): response = doc.run_method(method, **args) frappe.response.docs.append(doc) - if not response: + if response is None: return # build output as csv diff --git a/frappe/model/document.py b/frappe/model/document.py index 2260406125..bbba9b1492 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1130,12 +1130,16 @@ class Document(BaseDocument): collated in one dict and returned. Ideally, don't return values in hookable methods, set properties in the document.""" def add_to_return_value(self, new_return_value): + if new_return_value is None: + self._return_value = self.get("_return_value") + return + if isinstance(new_return_value, dict): if not self.get("_return_value"): self._return_value = {} self._return_value.update(new_return_value) else: - self._return_value = new_return_value or self.get("_return_value") + self._return_value = new_return_value def compose(fn, *hooks): def runner(self, method, *args, **kwargs): From 0d3bac55284ae3c6bff085211ea573d69e124e0a Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 6 Dec 2021 15:36:30 +0530 Subject: [PATCH 05/14] fix(ux): Show broken links as dashboard message --- frappe/email/doctype/newsletter/newsletter.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.js b/frappe/email/doctype/newsletter/newsletter.js index a7cbcf702a..e8978b8d0b 100644 --- a/frappe/email/doctype/newsletter/newsletter.js +++ b/frappe/email/doctype/newsletter/newsletter.js @@ -11,21 +11,18 @@ frappe.ui.form.on('Newsletter', { }, __('Preview')); frm.add_custom_button(__('Check broken links'), () => { + frm.dashboard.set_headline(__('Checking broken links...')); frm.call('find_broken_links').then(r => { + frm.dashboard.set_headline(''); let links = r.message; - if (links) { + if (links && links.length) { let html = '
    ' + links.map(link => `
  • ${link}
    • `).join('') + '
'; - frappe.msgprint({ - title: __("Broken Links"), - message: __("Following links are broken in the email content: {0}", [html]), - indicator: "red" - }) + frm.dashboard.set_headline(__("Following links are broken in the email content: {0}", [html])); } else { - frappe.msgprint({ - title: _("No Broken Links"), - message: _("No broken links found in the email content"), - indicator: "green" - }) + frm.dashboard.set_headline(__("No broken links found in the email content")); + setTimeout(() => { + frm.dashboard.set_headline(''); + }, 3000); } }) }, __('Preview')); From 9bdb5f2eb2404a4a6db855659d7c8f55cdb9b518 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 6 Dec 2021 17:04:24 +0530 Subject: [PATCH 06/14] 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 From 330677bb0a8960ab4624c3813bc15e1a6091544c Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 7 Dec 2021 15:40:55 +0530 Subject: [PATCH 07/14] fix: better sending status - show email sending progress in form dashboard --- frappe/email/doctype/newsletter/newsletter.js | 46 +++++++++++++++++++ frappe/email/doctype/newsletter/newsletter.py | 17 +++++++ 2 files changed, 63 insertions(+) diff --git a/frappe/email/doctype/newsletter/newsletter.js b/frappe/email/doctype/newsletter/newsletter.js index e8978b8d0b..d68736a00f 100644 --- a/frappe/email/doctype/newsletter/newsletter.js +++ b/frappe/email/doctype/newsletter/newsletter.js @@ -39,6 +39,7 @@ frappe.ui.form.on('Newsletter', { } frm.events.setup_dashboard(frm); + frm.events.setup_sending_status(frm); if (frm.is_new() && !doc.sender_email) { let { fullname, email } = frappe.user_info(doc.owner); @@ -145,5 +146,50 @@ frappe.ui.form.on('Newsletter', { ]); } } + }, + + setup_sending_status(frm) { + frm.call('get_sending_status').then(r => { + if (r.message) { + frm.events.update_sending_progress(frm, r.message.sent, r.message.total); + } + if (r.message.sent >= r.message.total) { + return; + } + if (frm.sending_status) return; + + frm.sending_status = setInterval(() => { + if (frm.doc.email_sent && frm.$wrapper.is(':visible')) { + frm.call('get_sending_status').then(r => { + if (r.message) { + let { sent, total } = r.message; + frm.events.update_sending_progress(frm, sent, total); + + if (sent >= total) { + clearInterval(frm.sending_status); + frm.sending_status = null; + return; + } + } + }); + } + }, 5000); + }); + }, + + update_sending_progress(frm, sent, total) { + if (sent >= total) { + frm.dashboard.hide_progress(); + return; + } + frm.dashboard.show_progress(__('Sending emails'), sent * 100 / total, __("{0} of {1} sent", [sent, total])); + }, + + on_hide(frm) { + if (frm.sending_status) { + clearInterval(frm.sending_status); + frm.sending_status = null; + } + }, } }); diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index b2146ed100..d84953db93 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -29,6 +29,23 @@ class Newsletter(WebsiteGenerator): self._recipients = self.get_recipients() return self._recipients + @frappe.whitelist() + def get_sending_status(self): + count_by_status = frappe.get_all("Email Queue", + filters={"reference_doctype": self.doctype, "reference_name": self.name}, + fields=["status", "count(name) as count"], + group_by="status", + order_by="status" + ) + sent = 0 + total = 0 + for row in count_by_status: + if row.status == "Sent": + sent = row.count + total += row.count + + return {'sent': sent, 'total': total} + @frappe.whitelist() def send_test_email(self, email): test_emails = frappe.utils.split_emails(email) From 1bb3c2d3f4797b76c400beb95982f7d51be0dde7 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 7 Dec 2021 15:41:38 +0530 Subject: [PATCH 08/14] fix: add on_hide event in form can be used to clearing events, for e.g., clearing setInterval --- frappe/public/js/frappe/form/form.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 27281d8927..e789b7241c 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -75,6 +75,10 @@ frappe.ui.form.Form = class FrappeForm { this.page = this.wrapper.page; this.layout_main = this.page.main.get(0); + this.$wrapper.on("hide", () => { + this.script_manager.trigger("on_hide"); + }); + this.toolbar = new frappe.ui.form.Toolbar({ frm: this, page: this.page From de7d0337a60be9ca3d6e59ab4d6141a595e8f2fe Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 7 Dec 2021 15:48:35 +0530 Subject: [PATCH 09/14] fix: various newsletter form ux fixes - Cancel Scheduling button - Show dashboard message if newsletter is scheduled --- frappe/email/doctype/newsletter/newsletter.js | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.js b/frappe/email/doctype/newsletter/newsletter.js index d68736a00f..72b8fa6993 100644 --- a/frappe/email/doctype/newsletter/newsletter.js +++ b/frappe/email/doctype/newsletter/newsletter.js @@ -28,7 +28,13 @@ frappe.ui.form.on('Newsletter', { }, __('Preview')); frm.add_custom_button(__('Send now'), () => { - frappe.confirm(__("Do you really want to send this email newsletter?"), function () { + if (frm.doc.schedule_send) { + frappe.confirm(__("This newsletter was scheduled to send on a later date. Are you sure you want to send it now?"), function () { + frm.call('send_emails').then(() => frm.refresh()); + }); + return; + } + frappe.confirm(__("Are you sure you want to send this newsletter now?"), function () { frm.call('send_emails').then(() => frm.refresh()); }); }, __('Send')); @@ -46,6 +52,8 @@ frappe.ui.form.on('Newsletter', { frm.set_value('sender_email', email); frm.set_value('sender_name', fullname); } + + frm.trigger('update_schedule_message'); }, schedule_send_dialog(frm) { @@ -74,8 +82,16 @@ frappe.ui.form.on('Newsletter', { primary_action_label: __('Schedule'), primary_action({ date, time }) { frm.set_value('schedule_sending', 1); - frm.set_value('schedule_send', `${date} ${time}`); + frm.set_value('schedule_send', `${date} ${time}:00`); d.hide(); + frm.save(); + }, + secondary_action_label: __('Cancel Scheduling'), + secondary_action() { + frm.set_value('schedule_sending', 0); + frm.set_value('schedule_send', ''); + d.hide(); + frm.save(); } }); if (frm.doc.schedule_sending) { @@ -83,7 +99,7 @@ frappe.ui.form.on('Newsletter', { if (parts.length === 2) { let [date, time] = parts; d.set_value('date', date); - d.set_value('time', time); + d.set_value('time', time.slice(0, 5)); } } d.show(); @@ -191,5 +207,13 @@ frappe.ui.form.on('Newsletter', { frm.sending_status = null; } }, + + update_schedule_message(frm) { + if (!frm.doc.email_sent && frm.doc.schedule_send) { + let datetime = frappe.datetime.global_date_format(frm.doc.schedule_send); + frm.dashboard.set_headline_alert(__('This newsletter is scheduled to be sent on {0}', [datetime.bold()])); + } else { + frm.dashboard.clear_headline(); + } } }); From 606a0d3809f868a5f55e3be02f5264b9929d6f26 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 7 Dec 2021 15:52:25 +0530 Subject: [PATCH 10/14] fix: various fixes - show published newsletters in list view - show published newsletter as web page - show status section after newsletter is sent - add email_sent_at and total_recipients field --- .../email/doctype/newsletter/newsletter.json | 45 +++++- frappe/email/doctype/newsletter/newsletter.py | 103 ++++---------- .../newsletter/templates/newsletter.html | 10 +- .../newsletter_email_group.json | 134 +++++------------- 4 files changed, 103 insertions(+), 189 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index 9d35671042..baabd4991e 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -7,6 +7,12 @@ "document_type": "Other", "engine": "InnoDB", "field_order": [ + "status_section", + "email_sent_at", + "column_break_3", + "total_recipients", + "column_break_12", + "email_sent", "from_section", "sender_name", "column_break_5", @@ -15,7 +21,6 @@ "send_from", "recipients", "email_group", - "email_sent", "subject_section", "subject", "newsletter_content", @@ -23,8 +28,8 @@ "message", "message_md", "message_html", - "send_unsubscribe_link", "attachments", + "send_unsubscribe_link", "send_webview_link", "schedule_settings_section", "scheduled_to_send", @@ -39,8 +44,9 @@ "fieldname": "email_group", "fieldtype": "Table", "in_standard_filter": 1, - "label": "Email Group", - "options": "Newsletter Email Group" + "label": "Audience", + "options": "Newsletter Email Group", + "reqd": 1 }, { "fieldname": "send_from", @@ -53,6 +59,7 @@ "default": "0", "fieldname": "email_sent", "fieldtype": "Check", + "hidden": 1, "label": "Email Sent", "no_copy": 1, "read_only": 1 @@ -91,6 +98,7 @@ "label": "Published" }, { + "depends_on": "published", "fieldname": "route", "fieldtype": "Data", "label": "Route", @@ -144,7 +152,6 @@ }, { "default": "0", - "depends_on": "published", "fieldname": "send_webview_link", "fieldtype": "Check", "label": "Send Web View Link" @@ -195,6 +202,32 @@ "fieldtype": "Table", "label": "Attachments", "options": "Newsletter Attachment" + }, + { + "fieldname": "email_sent_at", + "fieldtype": "Datetime", + "label": "Email Sent At", + "read_only": 1 + }, + { + "fieldname": "total_recipients", + "fieldtype": "Int", + "label": "Total Recipients", + "read_only": 1 + }, + { + "depends_on": "email_sent", + "fieldname": "status_section", + "fieldtype": "Section Break", + "label": "Status" + }, + { + "fieldname": "column_break_12", + "fieldtype": "Column Break" + }, + { + "fieldname": "column_break_3", + "fieldtype": "Column Break" } ], "has_web_view": 1, @@ -204,7 +237,7 @@ "is_published_field": "published", "links": [], "max_attachments": 3, - "modified": "2021-12-06 17:01:32.353153", + "modified": "2021-12-06 20:09:37.963141", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index d84953db93..aa6fa2c40a 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -15,13 +15,11 @@ from .exceptions import NewsletterAlreadySentError, NoRecipientFoundError, Newsl class Newsletter(WebsiteGenerator): - def onload(self): - self.setup_newsletter_status() - def validate(self): self.route = f"newsletters/{self.name}" self.validate_sender_address() self.validate_recipient_address() + self.validate_publishing() @property def newsletter_recipients(self) -> List[str]: @@ -48,8 +46,8 @@ class Newsletter(WebsiteGenerator): @frappe.whitelist() def send_test_email(self, email): - test_emails = frappe.utils.split_emails(email) - self.queue_all(test_emails=test_emails) + test_emails = frappe.utils.validate_email_address(email, throw=True) + self.send_newsletter(emails=test_emails) frappe.msgprint(_("Test email sent to {0}").format(email), alert=True) @frappe.whitelist() @@ -74,22 +72,11 @@ class Newsletter(WebsiteGenerator): @frappe.whitelist() def send_emails(self): - """send emails to leads and customers""" + """queue sending emails to recipients""" + self.schedule_sending = False + self.schedule_send = None self.queue_all() - frappe.msgprint(_("Email queued to {0} recipients").format(len(self.newsletter_recipients))) - - def setup_newsletter_status(self): - """Setup analytical status for current Newsletter. Can be accessible from desk. - """ - if self.email_sent: - status_count = frappe.get_all("Email Queue", - filters={"reference_doctype": self.doctype, "reference_name": self.name}, - fields=["status", "count(name)"], - group_by="status", - order_by="status", - as_list=True, - ) - self.get("__onload").status_count = dict(status_count) + frappe.msgprint(_("Email queued to {0} recipients").format(self.total_recipients)) def validate_send(self): """Validate if Newsletter can be sent. @@ -122,6 +109,10 @@ class Newsletter(WebsiteGenerator): for recipient in self.newsletter_recipients: frappe.utils.validate_email_address(recipient, throw=True) + def validate_publishing(self): + if self.send_webview_link and not self.published: + frappe.throw(_("Newsletter must be published to send webview link in email")) + def get_linked_email_queue(self) -> List[str]: """Get list of email queue linked to this newsletter. """ @@ -154,29 +145,19 @@ class Newsletter(WebsiteGenerator): x for x in self.newsletter_recipients if x not in self.get_success_recipients() ] - def queue_all(self, test_emails: List[str] = None): - """Queue Newsletter to all the recipients generated from the `Email Group` - table - - Args: - test_email (List[str], optional): Send test Newsletter to the passed set of emails. - Defaults to None. + def queue_all(self): + """Queue Newsletter to all the recipients generated from the `Email Group` table """ - if test_emails: - for test_email in test_emails: - frappe.utils.validate_email_address(test_email, throw=True) - else: - self.validate() - self.validate_send() + self.validate() + self.validate_send() - newsletter_recipients = test_emails or self.get_pending_recipients() - self.send_newsletter(emails=newsletter_recipients) + recipients = self.get_pending_recipients() + self.send_newsletter(emails=recipients) - if not test_emails: - self.email_sent = True - self.schedule_send = frappe.utils.now_datetime() - self.scheduled_to_send = len(newsletter_recipients) - self.save() + self.email_sent = True + self.email_sent_at = frappe.utils.now() + self.total_recipients = len(recipients) + self.save() def get_newsletter_attachments(self) -> List[Dict[str, str]]: """Get list of attachments on current Newsletter @@ -251,21 +232,6 @@ class Newsletter(WebsiteGenerator): }, ) - def get_context(self, context): - newsletters = get_newsletter_list("Newsletter", None, None, 0) - if newsletters: - newsletter_list = [d.name for d in newsletters] - if self.name not in newsletter_list: - frappe.redirect_to_message( - _("Permission Error"), _("You are not permitted to view the newsletter.") - ) - frappe.local.flags.redirect_location = frappe.local.response.location - raise frappe.Redirect - else: - context.attachments = self.get_attachments() - context.no_cache = 1 - context.show_sidebar = True - @frappe.whitelist(allow_guest=True) def confirmed_unsubscribe(email, group): @@ -348,35 +314,14 @@ def confirm_subscription(email, email_group=_("Website")): def get_list_context(context=None): context.update({ - "show_sidebar": True, "show_search": True, - 'no_breadcrumbs': True, - "title": _("Newsletter"), - "get_list": get_newsletter_list, + "no_breadcrumbs": True, + "title": _("Newsletters"), + "filters": {"published": 1}, "row_template": "email/doctype/newsletter/templates/newsletter_row.html", }) -def get_newsletter_list(doctype, txt, filters, limit_start, limit_page_length=20, order_by="modified"): - email_group_list = frappe.db.sql('''SELECT eg.name - FROM `tabEmail Group` eg, `tabEmail Group Member` egm - WHERE egm.unsubscribed=0 - AND eg.name=egm.email_group - AND egm.email = %s''', frappe.session.user) - email_group_list = [d[0] for d in email_group_list] - - if email_group_list: - return frappe.db.sql('''SELECT n.name, n.subject, n.message, n.modified - FROM `tabNewsletter` n, `tabNewsletter Email Group` neg - WHERE n.name = neg.parent - AND n.email_sent=1 - AND n.published=1 - AND neg.email_group in ({0}) - ORDER BY n.modified DESC LIMIT {1} OFFSET {2} - '''.format(','.join(['%s'] * len(email_group_list)), - limit_page_length, limit_start), email_group_list, as_dict=1) - - def send_scheduled_email(): """Send scheduled newsletter to the recipients.""" scheduled_newsletter = frappe.get_all( diff --git a/frappe/email/doctype/newsletter/templates/newsletter.html b/frappe/email/doctype/newsletter/templates/newsletter.html index 11ee19a550..1244f4c49a 100644 --- a/frappe/email/doctype/newsletter/templates/newsletter.html +++ b/frappe/email/doctype/newsletter/templates/newsletter.html @@ -1,6 +1,6 @@ {% extends "templates/web.html" %} -{% block title %} {{ _("Newsletter") }} {% endblock %} +{% block title %} {{ doc.subject }} {% endblock %} {% block page_content %}