From 3bdca7192434646014b2abe2251124afdc1e9908 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 3 Dec 2021 10:37:30 +0530 Subject: [PATCH 01/63] fix: delete Event Producer Last Update on trash event of Event Producer --- .../event_streaming/doctype/event_producer/event_producer.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frappe/event_streaming/doctype/event_producer/event_producer.py b/frappe/event_streaming/doctype/event_producer/event_producer.py index 05771a89d3..a6c2a257fa 100644 --- a/frappe/event_streaming/doctype/event_producer/event_producer.py +++ b/frappe/event_streaming/doctype/event_producer/event_producer.py @@ -54,6 +54,11 @@ class EventProducer(Document): self.db_set('incoming_change', 0) self.reload() + def on_trash(self): + last_update = frappe.db.get_value('Event Producer Last Update', dict(event_producer=self.name)) + if last_update: + frappe.delete_doc('Event Producer Last Update', last_update) + def check_url(self): valid_url_schemes = ("http", "https") frappe.utils.validate_url(self.producer_url, throw=True, valid_schemes=valid_url_schemes) From e862ae83da1e19abea889fcfe5e6366975201547 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Thu, 2 Dec 2021 21:07:06 +0530 Subject: [PATCH 02/63] fix: fixed list of Field objects as fields in get_values tests: added test for list of field objects --- frappe/database/database.py | 4 ++-- frappe/tests/test_db.py | 28 ++++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index f489cea7de..64f09c1835 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -583,7 +583,7 @@ class Database(object): if not isinstance(fields, Criterion): for field in fields: - if "(" in field or " as " in field: + if "(" in str(field) or " as " in str(field): field_objects.append(PseudoColumn(field)) else: field_objects.append(field) @@ -842,7 +842,7 @@ class Database(object): cache_count = frappe.cache().get_value('doctype:count:{}'.format(dt)) if cache_count is not None: return cache_count - query = self.query.build_conditions(table=dt, filters=filters).select(Count("*")) + query = self.query.get_sql(table=dt, filters=filters, fields=Count("*")) if filters: count = self.sql(query, debug=debug)[0][0] return count diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 60c8db6ab6..6501d753ff 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -24,10 +24,30 @@ class TestDB(unittest.TestCase): self.assertNotEqual(frappe.db.get_value("User", {"name": ["!=", "Guest"]}), "Guest") self.assertEqual(frappe.db.get_value("User", {"name": ["<", "Adn"]}), "Administrator") self.assertEqual(frappe.db.get_value("User", {"name": ["<=", "Administrator"]}), "Administrator") - self.assertEqual(frappe.db.get_value("User", {}, ["Max(name)"], order_by=None), frappe.db.sql("SELECT Max(name) FROM tabUser")[0][0]) - self.assertEqual(frappe.db.get_value("User", {}, "Min(name)", order_by=None), frappe.db.sql("SELECT Min(name) FROM tabUser")[0][0]) - self.assertIn("for update", frappe.db.get_value("User", Field("name") == "Administrator", for_update=True, run=False).lower()) - + self.assertEqual( + frappe.db.get_value("User", {}, ["Max(name)"], order_by=None), + frappe.db.sql("SELECT Max(name) FROM tabUser")[0][0], + ) + self.assertEqual( + frappe.db.get_value("User", {}, "Min(name)", order_by=None), + frappe.db.sql("SELECT Min(name) FROM tabUser")[0][0], + ) + self.assertIn( + "for update", + frappe.db.get_value( + "User", Field("name") == "Administrator", for_update=True, run=False + ).lower(), + ) + doctype = frappe.qb.DocType("User") + self.assertEqual( + frappe.qb.from_(doctype).select(doctype.name, doctype.email).run(), + frappe.db.get_values( + doctype, + filters={}, + fieldname=[doctype.name, doctype.email], + order_by=None, + ), + ) self.assertEqual(frappe.db.sql("""SELECT name FROM `tabUser` WHERE name > 's' ORDER BY MODIFIED DESC""")[0][0], frappe.db.get_value("User", {"name": [">", "s"]})) From 6f7d030e82ae3b2664f56a6fd9989cfbfc1f1648 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 3 Dec 2021 16:06:23 +0530 Subject: [PATCH 03/63] fix: IndexError while handling sql timeout error --- frappe/database/database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index f489cea7de..ea56acff27 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -171,10 +171,10 @@ class Database(object): frappe.errprint(query) elif self.is_deadlocked(e): - raise frappe.QueryDeadlockError + raise frappe.QueryDeadlockError(e) elif self.is_timedout(e): - raise frappe.QueryTimeoutError + raise frappe.QueryTimeoutError(e) if ignore_ddl and (self.is_missing_column(e) or self.is_missing_table(e) or self.cant_drop_field_or_key(e)): pass From db951d2369d8ca3a13f9f5667706b4a002185983 Mon Sep 17 00:00:00 2001 From: Summayya Hashmani <58825865+sumaiya2908@users.noreply.github.com> Date: Fri, 3 Dec 2021 17:22:07 +0530 Subject: [PATCH 04/63] frefactor: padd separate padding (#15178) Co-authored-by: Summayya Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/public/scss/website/index.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/public/scss/website/index.scss b/frappe/public/scss/website/index.scss index 9c84e99a5a..58f5ca79c6 100644 --- a/frappe/public/scss/website/index.scss +++ b/frappe/public/scss/website/index.scss @@ -266,7 +266,8 @@ h5.modal-title { .login-content.container { background-color: var(--fg-color); - padding: 45px 0px; + padding-bottom: 45px; + padding-top: 45px; box-shadow: var(--shadow-base); border-radius: var(--border-radius-md); max-width: 400px; From c5df17e3561c083a68f477ffe56403747aaf4d9c Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Fri, 3 Dec 2021 15:34:41 +0100 Subject: [PATCH 05/63] fix: cannot uninstall app with virtual doctype (#15136) * Update installer.py * fix: Drop table only if it exists * revert: "Update installer.py" This reverts commit 0e8370ede8a9c2b1c0687e5c216ecf67566da0f5. Co-authored-by: Suraj Shetty --- frappe/installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/installer.py b/frappe/installer.py index 9eed44ea15..cd6526c788 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -324,7 +324,7 @@ def _delete_doctypes(doctypes: List[str], dry_run: bool) -> None: print(f"* dropping Table for '{doctype}'...") if not dry_run: frappe.delete_doc("DocType", doctype, ignore_on_trash=True) - frappe.db.sql_ddl(f"drop table `tab{doctype}`") + frappe.db.sql_ddl(f"DROP TABLE IF EXISTS `tab{doctype}`") def post_install(rebuild_website=False): From 850cd54b890856af30403542289facdeec42fad4 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 3 Dec 2021 17:07:16 +0100 Subject: [PATCH 06/63] refactor: module profile --- .../doctype/module_profile/module_profile.js | 17 +- .../module_profile/module_profile.json | 10 +- .../doctype/role_profile/role_profile.json | 227 +++++------------- frappe/core/doctype/user/user.js | 4 +- frappe/public/js/frappe/module_editor.js | 74 +++--- 5 files changed, 132 insertions(+), 200 deletions(-) diff --git a/frappe/core/doctype/module_profile/module_profile.js b/frappe/core/doctype/module_profile/module_profile.js index 9c92042dda..57b563157c 100644 --- a/frappe/core/doctype/module_profile/module_profile.js +++ b/frappe/core/doctype/module_profile/module_profile.js @@ -1,19 +1,24 @@ // Copyright (c) 2020, Frappe Technologies and contributors // For license information, please see license.txt -frappe.ui.form.on('Module Profile', { - refresh: function(frm) { +frappe.ui.form.on("Module Profile", { + refresh: function (frm) { + debugger; if (has_common(frappe.user_roles, ["Administrator", "System Manager"])) { if (!frm.module_editor && frm.doc.__onload && frm.doc.__onload.all_modules) { - let module_area = $('
') - .appendTo(frm.fields_dict.module_html.wrapper); - + const module_area = $(frm.fields_dict.module_html.wrapper); frm.module_editor = new frappe.ModuleEditor(frm, module_area); } } if (frm.module_editor) { - frm.module_editor.refresh(); + frm.module_editor.show(); + } + }, + + validate: function (frm) { + if (frm.module_editor) { + frm.module_editor.set_modules_in_table(); } } }); diff --git a/frappe/core/doctype/module_profile/module_profile.json b/frappe/core/doctype/module_profile/module_profile.json index 0e4e56962e..32bc757427 100644 --- a/frappe/core/doctype/module_profile/module_profile.json +++ b/frappe/core/doctype/module_profile/module_profile.json @@ -34,11 +34,17 @@ } ], "index_web_pages_for_search": 1, - "links": [], - "modified": "2021-01-03 15:36:52.622696", + "links": [ + { + "link_doctype": "User", + "link_fieldname": "module_profile" + } + ], + "modified": "2021-12-03 15:47:21.296443", "modified_by": "Administrator", "module": "Core", "name": "Module Profile", + "naming_rule": "By fieldname", "owner": "Administrator", "permissions": [ { diff --git a/frappe/core/doctype/role_profile/role_profile.json b/frappe/core/doctype/role_profile/role_profile.json index 4b3f35aa57..7cd60a16d1 100644 --- a/frappe/core/doctype/role_profile/role_profile.json +++ b/frappe/core/doctype/role_profile/role_profile.json @@ -1,175 +1,80 @@ { - "allow_copy": 0, - "allow_guest_to_view": 0, - "allow_import": 0, - "allow_rename": 0, - "autoname": "role_profile", - "beta": 0, - "creation": "2017-08-31 04:16:38.764465", - "custom": 0, - "docstatus": 0, - "doctype": "DocType", - "document_type": "", - "editable_grid": 1, - "engine": "InnoDB", + "actions": [], + "autoname": "role_profile", + "creation": "2017-08-31 04:16:38.764465", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "role_profile", + "roles_html", + "roles" + ], "fields": [ { - "allow_bulk_edit": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "role_profile", - "fieldtype": "Data", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Role Name", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, + "fieldname": "role_profile", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Role Name", + "reqd": 1, "unique": 1 - }, + }, { - "allow_bulk_edit": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "roles_html", - "fieldtype": "HTML", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Roles HTML", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "fieldname": "roles_html", + "fieldtype": "HTML", + "label": "Roles HTML", + "read_only": 1 + }, { - "allow_bulk_edit": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "roles", - "fieldtype": "Table", - "hidden": 1, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Roles Assigned", - "length": 0, - "no_copy": 0, - "options": "Has Role", - "permlevel": 1, - "precision": "", - "print_hide": 1, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "unique": 0 + "fieldname": "roles", + "fieldtype": "Table", + "hidden": 1, + "label": "Roles Assigned", + "options": "Has Role", + "permlevel": 1, + "print_hide": 1, + "read_only": 1 } - ], - "has_web_view": 0, - "hide_heading": 0, - "hide_toolbar": 0, - "idx": 0, - "image_view": 0, - "in_create": 0, - "is_submittable": 0, - "issingle": 0, - "istable": 0, - "max_attachments": 0, - "modified": "2017-10-17 11:05:11.183066", - "modified_by": "Administrator", - "module": "Core", - "name": "Role Profile", - "name_case": "", - "owner": "Administrator", + ], + "links": [ + { + "link_doctype": "User", + "link_fieldname": "role_profile_name" + } + ], + "modified": "2021-12-03 15:45:45.270963", + "modified_by": "Administrator", + "module": "Core", + "name": "Role Profile", + "naming_rule": "Expression (old style)", + "owner": "Administrator", "permissions": [ { - "amend": 0, - "apply_user_permissions": 0, - "cancel": 0, - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "if_owner": 0, - "import": 0, - "permlevel": 0, - "print": 1, - "read": 1, - "report": 1, - "role": "System Manager", - "set_user_permissions": 0, - "share": 1, - "submit": 0, + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, "write": 1 - }, + }, { - "amend": 0, - "apply_user_permissions": 0, - "cancel": 0, - "create": 0, - "delete": 0, - "email": 1, - "export": 1, - "if_owner": 0, - "import": 0, - "permlevel": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "System Manager", - "set_user_permissions": 0, - "share": 1, - "submit": 0, + "email": 1, + "export": 1, + "permlevel": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, "write": 1 } - ], - "quick_entry": 0, - "read_only": 0, - "read_only_onload": 0, - "show_name_in_global_search": 0, - "sort_field": "modified", - "sort_order": "DESC", - "title_field": "role_profile", - "track_changes": 1, - "track_seen": 0 + ], + "sort_field": "modified", + "sort_order": "DESC", + "title_field": "role_profile", + "track_changes": 1 } \ No newline at end of file diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index 2ce7413aa7..5b3a1affd9 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -50,7 +50,7 @@ frappe.ui.form.on('User', { let d = frm.add_child("block_modules"); d.module = v.module; }); - frm.module_editor && frm.module_editor.refresh(); + frm.module_editor && frm.module_editor.show(); } }); } @@ -180,7 +180,7 @@ frappe.ui.form.on('User', { frm.roles_editor.show(); } - frm.module_editor && frm.module_editor.refresh(); + frm.module_editor && frm.module_editor.show(); if(frappe.session.user==doc.name) { // update display settings diff --git a/frappe/public/js/frappe/module_editor.js b/frappe/public/js/frappe/module_editor.js index 5e2ca4bc83..ff0cfc2426 100644 --- a/frappe/public/js/frappe/module_editor.js +++ b/frappe/public/js/frappe/module_editor.js @@ -1,38 +1,54 @@ frappe.ModuleEditor = class ModuleEditor { constructor(frm, wrapper) { - this.wrapper = $('
').appendTo(wrapper); this.frm = frm; - this.make(); - } - make() { - var me = this; - this.frm.doc.__onload.all_modules.forEach(function(m) { - $(repl('
\ -
', {module: m})).appendTo(me.wrapper); + this.wrapper = wrapper; + const block_modules = this.frm.doc.block_modules.map(row => row.module); + this.multicheck = frappe.ui.form.make_control({ + parent: wrapper, + df: { + fieldname: "block_modules", + fieldtype: "MultiCheck", + select_all: true, + columns: 3, + get_data: () => { + return this.frm.doc.__onload.all_modules.map(module => { + return { + label: __(module), + value: module, + checked: !block_modules.includes(module), + }; + }); + }, + on_change: () => { + this.set_modules_in_table(); + this.frm.dirty(); + } + }, + render_input: true }); - this.bind(); } - refresh() { - var me = this; - this.wrapper.find(".block-module-check").prop("checked", true); - $.each(this.frm.doc.block_modules, function(i, d) { - me.wrapper.find(".block-module-check[data-module='"+ d.module +"']").prop("checked", false); - }); + + show() { + const block_modules = this.frm.doc.block_modules.map(row => row.module); + const all_modules = this.frm.doc.__onload.all_modules; + this.multicheck.selected_options = all_modules.filter(m => !block_modules.includes(m)); + this.multicheck.refresh_input(); } - bind() { - var me = this; - this.wrapper.on("change", ".block-module-check", function() { - var module = $(this).attr('data-module'); - if ($(this).prop("checked")) { - // remove from block_modules - me.frm.doc.block_modules = $.map(me.frm.doc.block_modules || [], function(d) { - if (d.module != module) { - return d; - } - }); - } else { - me.frm.add_child("block_modules", {"module": module}); + + set_modules_in_table() { + let block_modules = this.frm.doc.block_modules || []; + let unchecked_options = this.multicheck.get_unchecked_options(); + + block_modules.map(module_doc => { + if (!unchecked_options.includes(module_doc.module)) { + frappe.model.clear_doc(module_doc.doctype, module_doc.name); + } + }); + + unchecked_options.map(module => { + if (!block_modules.find(d => d.module === module)) { + let module_doc = frappe.model.add_child(this.frm.doc, "Block Module", "block_modules"); + module_doc.module = module; } }); } From 504f8743c9ac818f1cacc7c1424340ebcc24c5a9 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 3 Dec 2021 17:24:57 +0100 Subject: [PATCH 07/63] fix: remove debugger --- frappe/core/doctype/module_profile/module_profile.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/module_profile/module_profile.js b/frappe/core/doctype/module_profile/module_profile.js index 57b563157c..3714d31ade 100644 --- a/frappe/core/doctype/module_profile/module_profile.js +++ b/frappe/core/doctype/module_profile/module_profile.js @@ -3,7 +3,6 @@ frappe.ui.form.on("Module Profile", { refresh: function (frm) { - debugger; if (has_common(frappe.user_roles, ["Administrator", "System Manager"])) { if (!frm.module_editor && frm.doc.__onload && frm.doc.__onload.all_modules) { const module_area = $(frm.fields_dict.module_html.wrapper); From 8ead1d9c487b5f7f3e86016b0404a700cc11ba8c Mon Sep 17 00:00:00 2001 From: this-gavagai Date: Mon, 6 Dec 2021 13:13:43 +0545 Subject: [PATCH 08/63] fix: Clarified docstatus transition exceptions (#15194) * [fix] Clarified docstatus transition exceptions Exceptions issued by the document.py `check_docstatus_transition` method are potentially very misleading. In cases where an invalid docstatus is used, users receive an confusing exception stating "Cannot change docstatus from 0 to 2" or "Cannot change docstatus from 1 to 0". This PR adds an additional exception message when an invalid docstatus is used. * fix: Clarified docstatus transition exceptions Added additional clarifications to exception messages --- frappe/model/document.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index fcdadf48e6..2260406125 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -750,8 +750,10 @@ class Document(BaseDocument): elif self.docstatus==1: self._action = "submit" self.check_permission("submit") + elif self.docstatus==2: + raise frappe.DocstatusTransitionError(_("Cannot change docstatus from 0 (Draft) to 2 (Cancelled)")) else: - raise frappe.DocstatusTransitionError(_("Cannot change docstatus from 0 to 2")) + raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) elif docstatus==1: if self.docstatus==1: @@ -760,8 +762,10 @@ class Document(BaseDocument): elif self.docstatus==2: self._action = "cancel" self.check_permission("cancel") + elif self.docstatus==0: + raise frappe.DocstatusTransitionError(_("Cannot change docstatus from 1 (Submitted) to 0 (Draft)")) else: - raise frappe.DocstatusTransitionError(_("Cannot change docstatus from 1 to 0")) + raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) elif docstatus==2: raise frappe.ValidationError(_("Cannot edit cancelled document")) From 3243fb2083593fdc42b59f609935a758ae399bad Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 6 Dec 2021 13:04:27 +0530 Subject: [PATCH 09/63] fix: misc fixes --- frappe/database/database.py | 17 ++++++++--------- frappe/database/query.py | 5 ++--- frappe/query_builder/builder.py | 33 +++++++++++---------------------- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 6a4e781b44..0f325a746e 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -511,14 +511,10 @@ class Database(object): # Get coulmn and value of the single doctype Accounts Settings account_settings = frappe.db.get_singles_dict("Accounts Settings") """ - result = self.sql(""" - SELECT field, value - FROM `tabSingles` - WHERE doctype = %s - """, doctype) - + result = self.query.get_sql( + "Singles", filters={"doctype": doctype}, fields=["field", "value"] + ).run() dict_ = frappe._dict(result) - return dict_ @staticmethod @@ -547,8 +543,11 @@ class Database(object): if fieldname in self.value_cache[doctype]: return self.value_cache[doctype][fieldname] - val = self.sql("""select `value` from - `tabSingles` where `doctype`=%s and `field`=%s""", (doctype, fieldname)) + val = self.query.get_sql( + table="Singles", + filters={"doctype": doctype, "field": fieldname}, + fields="value", + ).run() val = val[0][0] if val else None df = frappe.get_meta(doctype).get_field(fieldname) diff --git a/frappe/database/query.py b/frappe/database/query.py index 69328cb206..6d2be5fa25 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -286,14 +286,13 @@ class Query: ): criterion = self.build_conditions(table, filters, **kwargs) if isinstance(fields, (list, tuple)): - query = criterion.select(*kwargs.get("field_objects")) + query = criterion.select(*kwargs.get("field_objects", fields)) elif isinstance(fields, Criterion): query = criterion.select(fields) else: - if fields=="*": - query = criterion.select(fields) + query = criterion.select(fields) return query diff --git a/frappe/query_builder/builder.py b/frappe/query_builder/builder.py index 630cfea222..a65d50fdeb 100644 --- a/frappe/query_builder/builder.py +++ b/frappe/query_builder/builder.py @@ -18,16 +18,6 @@ class Base: table_name = get_table_name(table_name) return Table(table_name, *args, **kwargs) - -class MariaDB(Base, MySQLQuery): - Field = terms.Field - - @classmethod - def from_(cls, table, *args, **kwargs): - if isinstance(table, str): - table = cls.DocType(table) - return super().from_(table, *args, **kwargs) - @classmethod def into(cls, table, *args, **kwargs): if isinstance(table, str): @@ -40,6 +30,17 @@ class MariaDB(Base, MySQLQuery): table = cls.DocType(table) return super().update(table, *args, **kwargs) + +class MariaDB(Base, MySQLQuery): + Field = terms.Field + + @classmethod + def from_(cls, table, *args, **kwargs): + if isinstance(table, str): + table = cls.DocType(table) + return super().from_(table, *args, **kwargs) + + class Postgres(Base, PostgreSQLQuery): field_translation = {"table_name": "relname", "table_rows": "n_tup_ins"} schema_translation = {"tables": "pg_stat_all_tables"} @@ -69,15 +70,3 @@ class Postgres(Base, PostgreSQLQuery): table = cls.DocType(table) return super().from_(table, *args, **kwargs) - - @classmethod - def into(cls, table, *args, **kwargs): - if isinstance(table, str): - table = cls.DocType(table) - return super().into(table, *args, **kwargs) - - @classmethod - def update(cls, table, *args, **kwargs): - if isinstance(table, str): - table = cls.DocType(table) - return super().update(table, *args, **kwargs) \ No newline at end of file From a574c1ba88cd76d74065bfbcbf2c9c78dd208d0b Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 2 Dec 2021 23:20:40 +0530 Subject: [PATCH 10/63] chore: patching ValueWrapper --- frappe/query_builder/terms.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 frappe/query_builder/terms.py diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py new file mode 100644 index 0000000000..c221dcb28e --- /dev/null +++ b/frappe/query_builder/terms.py @@ -0,0 +1,27 @@ +from typing import Any, Optional, Dict +from pypika.terms import ValueWrapper +from pypika.utils import format_alias_sql + + +class NamedParameterWrapper(): + def __init__(self, parameters: Dict[str, Any]): + self.parameters = parameters + + def update_parameters(self, param_key: Any, param_value: Any, **kwargs): + self.parameters[param_key[1:]] = param_value + + def get_sql(self, **kwargs): + return f'@param{len(self.parameters) + 1}' + + +class ParameterizedValueWrapper(ValueWrapper): + def get_sql(self, quote_char: Optional[str] = None, secondary_quote_char: str = "'", param_wrapper= None, **kwargs: Any) -> str: + if param_wrapper is None: + sql = self.get_value_sql(quote_char=quote_char, secondary_quote_char=secondary_quote_char, **kwargs) + return format_alias_sql(sql, self.alias, quote_char=quote_char, **kwargs) + else: + value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) + param_sql = param_wrapper.get_sql(**kwargs) + param_wrapper.update_parameters(param_key=param_sql, param_value=value_sql, **kwargs) + + return format_alias_sql(param_sql, self.alias, quote_char=quote_char, **kwargs) \ No newline at end of file From 9fdacedfc80889c81c4887c2f2f2581e5d0d56f6 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 2 Dec 2021 23:23:25 +0530 Subject: [PATCH 11/63] feat: sanitise frappe.qb --- frappe/query_builder/__init__.py | 5 +++++ frappe/query_builder/terms.py | 8 ++++---- frappe/query_builder/utils.py | 14 ++++++++++---- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/frappe/query_builder/__init__.py b/frappe/query_builder/__init__.py index 9c7432142f..06d499678f 100644 --- a/frappe/query_builder/__init__.py +++ b/frappe/query_builder/__init__.py @@ -1,2 +1,7 @@ +from frappe.query_builder.terms import ParameterizedValueWrapper +import pypika + +pypika.terms.ValueWrapper = ParameterizedValueWrapper + from pypika import * from frappe.query_builder.utils import Column, DocType, get_query_builder, patch_query_execute, patch_query_aggregation diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py index c221dcb28e..c09d9595fb 100644 --- a/frappe/query_builder/terms.py +++ b/frappe/query_builder/terms.py @@ -1,4 +1,4 @@ -from typing import Any, Optional, Dict +from typing import Any, List, Optional, Dict from pypika.terms import ValueWrapper from pypika.utils import format_alias_sql @@ -8,10 +8,10 @@ class NamedParameterWrapper(): self.parameters = parameters def update_parameters(self, param_key: Any, param_value: Any, **kwargs): - self.parameters[param_key[1:]] = param_value + self.parameters[param_key[2:-2]] = param_value def get_sql(self, **kwargs): - return f'@param{len(self.parameters) + 1}' + return f'%(param{len(self.parameters) + 1})s' class ParameterizedValueWrapper(ValueWrapper): @@ -20,7 +20,7 @@ class ParameterizedValueWrapper(ValueWrapper): sql = self.get_value_sql(quote_char=quote_char, secondary_quote_char=secondary_quote_char, **kwargs) return format_alias_sql(sql, self.alias, quote_char=quote_char, **kwargs) else: - value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) + value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) if not isinstance(self.value,int) else self.value param_sql = param_wrapper.get_sql(**kwargs) param_wrapper.update_parameters(param_key=param_sql, param_value=value_sql, **kwargs) diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index a7f52df012..7922825725 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -10,6 +10,7 @@ import frappe from .builder import MariaDB, Postgres from pypika.terms import PseudoColumn +from frappe.query_builder.terms import NamedParameterWrapper class db_type_is(Enum): MARIADB = "mariadb" @@ -53,12 +54,16 @@ def patch_query_execute(): This excludes the use of `frappe.db.sql` method while executing the query object """ - def execute_query(query, *args, **kwargs): - query = str(query) + query, params = prepare_query(query) + return frappe.db.sql(query, params, *args, **kwargs) + + def prepare_query(query): + params = {} + query = query.get_sql(param_wrapper = NamedParameterWrapper(params)) if frappe.flags.in_safe_exec and not query.lower().strip().startswith("select"): raise frappe.PermissionError('Only SELECT SQL allowed in scripting') - return frappe.db.sql(query, *args, **kwargs) + return query, params query_class = get_attr(str(frappe.qb).split("'")[1]) builder_class = get_type_hints(query_class._builder).get('return') @@ -67,6 +72,7 @@ def patch_query_execute(): raise BuilderIdentificationFailed builder_class.run = execute_query + builder_class.walk = prepare_query def patch_query_aggregation(): @@ -77,4 +83,4 @@ def patch_query_aggregation(): frappe.qb.max = _max frappe.qb.min = _min frappe.qb.avg = _avg - frappe.qb.sum = _sum \ No newline at end of file + frappe.qb.sum = _sum From 6120b4b3c1dbf17bc783be16ba41a5c73d5c5df1 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Sat, 4 Dec 2021 20:12:48 +0530 Subject: [PATCH 12/63] fix: extend named parameters to frappe.qb.function --- frappe/query_builder/__init__.py | 3 ++- frappe/query_builder/terms.py | 28 +++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/frappe/query_builder/__init__.py b/frappe/query_builder/__init__.py index 06d499678f..bf7be84c51 100644 --- a/frappe/query_builder/__init__.py +++ b/frappe/query_builder/__init__.py @@ -1,7 +1,8 @@ -from frappe.query_builder.terms import ParameterizedValueWrapper +from frappe.query_builder.terms import ParameterizedValueWrapper, ParameterizedFunction import pypika pypika.terms.ValueWrapper = ParameterizedValueWrapper +pypika.terms.Function = ParameterizedFunction from pypika import * from frappe.query_builder.utils import Column, DocType, get_query_builder, patch_query_execute, patch_query_aggregation diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py index c09d9595fb..2032cd8497 100644 --- a/frappe/query_builder/terms.py +++ b/frappe/query_builder/terms.py @@ -1,5 +1,6 @@ -from typing import Any, List, Optional, Dict -from pypika.terms import ValueWrapper +from typing import Any, Dict, Optional + +from pypika.terms import Function, ValueWrapper from pypika.utils import format_alias_sql @@ -23,5 +24,26 @@ class ParameterizedValueWrapper(ValueWrapper): value_sql = self.get_value_sql(quote_char=quote_char, **kwargs) if not isinstance(self.value,int) else self.value param_sql = param_wrapper.get_sql(**kwargs) param_wrapper.update_parameters(param_key=param_sql, param_value=value_sql, **kwargs) + return format_alias_sql(param_sql, self.alias, quote_char=quote_char, **kwargs) + + +class ParameterizedFunction(Function): + def get_sql(self, **kwargs: Any) -> str: + with_alias = kwargs.pop("with_alias", False) + with_namespace = kwargs.pop("with_namespace", False) + quote_char = kwargs.pop("quote_char", None) + dialect = kwargs.pop("dialect", None) + param_wrapper = kwargs.pop("param_wrapper", None) + + function_sql = self.get_function_sql(with_namespace=with_namespace, quote_char=quote_char, param_wrapper=param_wrapper, dialect=dialect) + + if self.schema is not None: + function_sql = "{schema}.{function}".format( + schema=self.schema.get_sql(quote_char=quote_char, dialect=dialect, **kwargs), + function=function_sql, + ) + + if with_alias: + return format_alias_sql(function_sql, self.alias, quote_char=quote_char, **kwargs) - return format_alias_sql(param_sql, self.alias, quote_char=quote_char, **kwargs) \ No newline at end of file + return function_sql From aa855afe089d209d2a4796c8497d21ed5d12578a Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Mon, 6 Dec 2021 12:12:56 +0530 Subject: [PATCH 13/63] test: test for patches through walk --- frappe/tests/test_query_builder.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_query_builder.py b/frappe/tests/test_query_builder.py index 7a0935a63b..1d63d2041c 100644 --- a/frappe/tests/test_query_builder.py +++ b/frappe/tests/test_query_builder.py @@ -2,7 +2,7 @@ import unittest from typing import Callable import frappe -from frappe.query_builder.functions import GroupConcat, Match +from frappe.query_builder.functions import Coalesce, GroupConcat, Match from frappe.query_builder.utils import db_type_is @@ -49,6 +49,25 @@ class TestBuilderBase(object): self.assertIsInstance(query.run, Callable) self.assertIsInstance(data, list) + def test_walk(self): + DocType = frappe.qb.DocType('DocType') + query = ( + frappe.qb.from_(DocType) + .select(DocType.name) + .where((DocType.owner == "Administrator' --") + & (Coalesce(DocType.search_fields == "subject")) + ) + ) + self.assertTrue("walk" in dir(query)) + query, params = query.walk() + + self.assertIn("%(param1)s", query) + self.assertIn("%(param2)s", query) + self.assertIn("param1",params) + self.assertEqual(params["param1"],"Administrator' --") + self.assertEqual(params["param2"],"subject") + + @run_only_if(db_type_is.MARIADB) class TestBuilderMaria(unittest.TestCase, TestBuilderBase): def test_adding_tabs_in_from(self): @@ -59,7 +78,6 @@ class TestBuilderMaria(unittest.TestCase, TestBuilderBase): "SELECT * FROM `__Auth`", frappe.qb.from_("__Auth").select("*").get_sql() ) - @run_only_if(db_type_is.POSTGRES) class TestBuilderPostgres(unittest.TestCase, TestBuilderBase): def test_adding_tabs_in_from(self): From 8340639afdeab4414ae54af67ef0b4e23d5a8a64 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 6 Dec 2021 14:51:37 +0530 Subject: [PATCH 14/63] fix: Allow Fetch From for a different link field of the same DocType --- frappe/core/doctype/doctype/doctype.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.js b/frappe/core/doctype/doctype/doctype.js index 1c52070063..b907ebc0bc 100644 --- a/frappe/core/doctype/doctype/doctype.js +++ b/frappe/core/doctype/doctype/doctype.js @@ -143,11 +143,10 @@ frappe.ui.form.on("DocField", { curr_value.doctype = doctype; curr_value.fieldname = fieldname; } - let curr_df_link_doctype = row.fieldtype == "Link" ? row.options : null; let doctypes = frm.doc.fields .filter(df => df.fieldtype == "Link") - .filter(df => df.options && df.options != curr_df_link_doctype) + .filter(df => df.options && df.fieldname != row.fieldname) .map(df => ({ label: `${df.options} (${df.fieldname})`, value: df.fieldname From da4160e2dd0c9c47e207b51689d9a276221b50af Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 3 Dec 2021 18:22:25 +0530 Subject: [PATCH 15/63] 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 | 140 ++++++++++++------ .../email/doctype/newsletter/newsletter.json | 110 ++++++++------ frappe/email/doctype/newsletter/newsletter.py | 31 +++- .../newsletter/templates/newsletter.html | 4 +- 4 files changed, 192 insertions(+), 93 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 = '
    ' + links.map(link => `
  • ${link}
    • `).join('') + '
'; + 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'); + 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(); }, - 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 - }); + send_test_email(frm) { + let d = new frappe.ui.Dialog({ + title: __('Send Test Email'), + fields: [ + { + label: __('Email'), + fieldname: 'email', + fieldtype: 'Data', + options: 'Email', } - frm.get_field('schedule_send').$input.trigger('change'); + ], + 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); + }); } }); - - - 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'); + 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 16/63] 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 17/63] 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 18/63] 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 19/63] 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 20/63] 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 21/63] 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 22/63] 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 23/63] 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 24/63] 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 | 107 ++++---------- .../newsletter/templates/newsletter.html | 10 +- .../newsletter_email_group.json | 134 +++++------------- 4 files changed, 105 insertions(+), 191 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() - - newsletter_recipients = test_emails or self.get_pending_recipients() - self.send_newsletter(emails=newsletter_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.validate() + self.validate_send() + + recipients = self.get_pending_recipients() + self.send_newsletter(emails=recipients) + + 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 %}