Merge branch 'develop' into duplicate-listview-records

.github/helper/install.sh

@@ -17,7 +17,7 @@ if [ "$TYPE" == "server" ]; then
 fi
 
 if [ "$DB" == "mariadb" ];then
-      sudo apt install mariadb-client-10.3
+      sudo apt update && sudo apt install mariadb-client-10.3
       mysql --host 127.0.0.1 --port 3306 -u root -e "SET GLOBAL character_set_server = 'utf8mb4'";
       mysql --host 127.0.0.1 --port 3306 -u root -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'";
 

.gitignore

@@ -11,6 +11,7 @@ dist/
 frappe/docs/current
 frappe/public/dist
 .vscode
+.vs
 node_modules
 .kdev4/
 *.kdev4

codecov.yml

@@ -11,6 +11,13 @@ coverage:
         threshold: 0.5%
         flags:
           - server
+    patch:
+      default: false
+      server:
+        target: auto
+        threshold: 85%
+        flags:
+          - server
 
 comment:
   layout: "diff, flags"

frappe/core/doctype/data_import/importer.py

@@ -199,7 +199,7 @@ class Importer:
 		new_doc = frappe.new_doc(self.doctype)
 		new_doc.update(doc)
 
-		if (meta.autoname or "").lower() != "prompt":
+		if not doc.name and (meta.autoname or "").lower() != "prompt":
 			# name can only be set directly if autoname is prompt
 			new_doc.set("name", None)
 

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

frappe/core/doctype/file/file.py

@@ -569,6 +569,24 @@ class File(Document):
 		frappe.local.rollback_observers.append(self)
 		self.save()
 
+	@staticmethod
+	def zip_files(files):
+		from six import string_types
+
+		zip_file = io.BytesIO()
+		zf = zipfile.ZipFile(zip_file, "w", zipfile.ZIP_DEFLATED)
+		for _file in files:
+			if isinstance(_file, string_types):
+				_file = frappe.get_doc("File", _file)
+			if not isinstance(_file, File):
+				continue
+			if _file.is_folder:
+				continue
+			zf.writestr(_file.file_name, _file.get_content())
+		zf.close()
+		return zip_file.getvalue()
+
+
 def on_doctype_update():
 	frappe.db.add_index("File", ["attached_to_doctype", "attached_to_name"])
 
@@ -612,6 +630,16 @@ def move_file(file_list, new_parent, old_parent):
 	frappe.get_doc("File", old_parent).save()
 	frappe.get_doc("File", new_parent).save()
 
+
+@frappe.whitelist()
+def zip_files(files):
+	files = frappe.parse_json(files)
+	zipped_files = File.zip_files(files)
+	frappe.response["filename"] = "files.zip"
+	frappe.response["filecontent"] = zipped_files
+	frappe.response["type"] = "download"
+
+
 def setup_folder_path(filename, new_parent):
 	file = frappe.get_doc("File", filename)
 	file.folder = new_parent

frappe/core/doctype/module_profile/module_profile.js

@@ -1,19 +1,23 @@
 // 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) {
 		if (has_common(frappe.user_roles, ["Administrator", "System Manager"])) {
 			if (!frm.module_editor && frm.doc.__onload && frm.doc.__onload.all_modules) {
-				let module_area = $('<div style="min-height: 300px">')
-					.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();
 		}
 	}
 });

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": [
   {

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
 }

frappe/core/doctype/user/test_user.py

@@ -251,7 +251,7 @@ class TestUser(unittest.TestCase):
 		c = FrappeClient(url)
 		res1 = c.session.post(url, data=data, verify=c.verify, headers=c.headers)
 		res2 = c.session.post(url, data=data, verify=c.verify, headers=c.headers)
-		self.assertEqual(res1.status_code, 200)
+		self.assertEqual(res1.status_code, 400)
 		self.assertEqual(res2.status_code, 417)
 
 	def test_user_rename(self):

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

frappe/core/doctype/user/user.py

@@ -808,6 +808,7 @@ def reset_password(user):
 		return frappe.msgprint(_("Password reset instructions have been sent to your email"))
 
 	except frappe.DoesNotExistError:
+		frappe.local.response['http_status_code'] = 400
 		frappe.clear_messages()
 		return 'not found'
 

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
@@ -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)
@@ -583,7 +582,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 +841,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

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
 

frappe/desk/doctype/global_search_settings/global_search_settings.py

@@ -33,7 +33,7 @@ class GlobalSearchSettings(Document):
 
 def get_doctypes_for_global_search():
 	def get_from_db():
-		doctypes = frappe.get_list("Global Search DocType", fields=["document_type"], order_by="idx ASC")
+		doctypes = frappe.get_all("Global Search DocType", fields=["document_type"], order_by="idx ASC")
 		return [d.document_type for d in doctypes] or []
 
 	return frappe.cache().hget("global_search", "search_priorities", get_from_db)

frappe/desk/form/linked_with.py

@@ -410,11 +410,11 @@ def get_linked_docs(doctype, name, linkinfo=None, for_doctype=None):
 
 			try:
 				if link.get("filters"):
-					ret = frappe.get_list(doctype=dt, fields=fields, filters=link.get("filters"))
+					ret = frappe.get_all(doctype=dt, fields=fields, filters=link.get("filters"))
 
 				elif link.get("get_parent"):
 					if me and me.parent and me.parenttype == dt:
-						ret = frappe.get_list(doctype=dt, fields=fields,
+						ret = frappe.get_all(doctype=dt, fields=fields,
 							filters=[[dt, "name", '=', me.parent]])
 					else:
 						ret = None
@@ -426,7 +426,7 @@ def get_linked_docs(doctype, name, linkinfo=None, for_doctype=None):
 					if link.get("doctype_fieldname"):
 						filters.append([link.get('child_doctype'), link.get("doctype_fieldname"), "=", doctype])
 
-					ret = frappe.get_list(doctype=dt, fields=fields, filters=filters, or_filters=or_filters, distinct=True)
+					ret = frappe.get_all(doctype=dt, fields=fields, filters=filters, or_filters=or_filters, distinct=True)
 
 				else:
 					link_fieldnames = link.get("fieldname")
@@ -437,7 +437,7 @@ def get_linked_docs(doctype, name, linkinfo=None, for_doctype=None):
 						# dynamic link
 						if link.get("doctype_fieldname"):
 							filters.append([dt, link.get("doctype_fieldname"), "=", doctype])
-						ret = frappe.get_list(doctype=dt, fields=fields, filters=filters, or_filters=or_filters)
+						ret = frappe.get_all(doctype=dt, fields=fields, filters=filters, or_filters=or_filters)
 
 					else:
 						ret = None

frappe/desk/page/user_profile/user_profile_controller.js

@@ -17,21 +17,15 @@ class UserProfile {
 	show() {
 		let route = frappe.get_route();
 		this.user_id = route[1] || frappe.session.user;
-
-		//validate if user
-		if (route.length > 1) {
-			frappe.dom.freeze(__('Loading user profile') + '...');
-			frappe.db.exists('User', this.user_id).then(exists => {
-				frappe.dom.unfreeze();
-				if (exists) {
-					this.make_user_profile();
-				} else {
-					frappe.msgprint(__('User does not exist'));
-				}
-			});
-		} else {
-			frappe.set_route('user-profile', frappe.session.user);
-		}
+		frappe.dom.freeze(__('Loading user profile') + '...');
+		frappe.db.exists('User', this.user_id).then(exists => {
+			frappe.dom.unfreeze();
+			if (exists) {
+				this.make_user_profile();
+			} else {
+				frappe.msgprint(__('User does not exist'));
+			}
+		});
 	}
 
 	make_user_profile() {
@@ -74,8 +68,7 @@ class UserProfile {
 			primary_action_label: __('Go'),
 			primary_action: ({ user }) => {
 				dialog.hide();
-				this.user_id = user;
-				this.make_user_profile();
+				frappe.set_route('user-profile', user);
 			}
 		});
 		dialog.show();

frappe/desk/page/user_profile/user_profile_sidebar.html

@@ -51,10 +51,10 @@
 			<p><a class="edit-profile-link">{%=__("Edit Profile") %}</a></p>
 			<p><a class="user-settings-link">{%=__("User Settings") %}</a></p>
 			<p>
-				<a class="leaderboard-link" href="#leaderboard/User"
+				<a class="leaderboard-link" href="/app/leaderboard/User"
 					>{%=__("Leaderboard") %}</a
 				>
 			</p>
 		</div>
 	</div>
-</div>
+</div>

frappe/email/doctype/email_queue/email_queue.py

@@ -18,7 +18,7 @@ from frappe import _, safe_encode, task
 from frappe.model.document import Document
 from frappe.email.queue import get_unsubcribed_url, get_unsubscribe_message
 from frappe.email.email_body import add_attachment, get_formatted_html, get_email
-from frappe.utils import cint, split_emails, add_days, nowdate, cstr
+from frappe.utils import cint, split_emails, add_days, nowdate, cstr, get_hook_method
 from frappe.email.doctype.email_account.email_account import EmailAccount
 
 
@@ -121,9 +121,13 @@ class EmailQueue(Document):
 					continue
 
 				message = ctx.build_message(recipient.recipient)
-				if not frappe.flags.in_test:
-					ctx.smtp_session.sendmail(from_addr=self.sender, to_addrs=recipient.recipient, msg=message)
-				ctx.add_to_sent_list(recipient)
+				method = get_hook_method('override_email_send')
+				if method:
+					method(self, self.sender, recipient.recipient, message)
+				else:
+					if not frappe.flags.in_test:
+						ctx.smtp_session.sendmail(from_addr=self.sender, to_addrs=recipient.recipient, msg=message)
+					ctx.add_to_sent_list(recipient)
 
 			if frappe.flags.in_test:
 				frappe.flags.sent_mail = message
@@ -283,9 +287,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 +302,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 +513,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):

frappe/email/doctype/newsletter/newsletter.js

@@ -4,69 +4,137 @@
 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.dashboard.set_headline(__('Checking broken links...'));
+				frm.call('find_broken_links').then(r => {
+					frm.dashboard.set_headline('');
+					let links = r.message;
+					if (links && links.length) {
+						let html = '<ul>' + links.map(link => `<li>${link}</li>`).join('') + '</ul>';
+						frm.dashboard.set_headline(__("Following links are broken in the email content: {0}", [html]));
+					} else {
+						frm.dashboard.set_headline(__("No broken links found in the email content"));
+						setTimeout(() => {
+							frm.dashboard.set_headline('');
+						}, 3000);
+					}
+				});
+			}, __('Preview'));
+
+			frm.add_custom_button(__('Send now'), () => {
+				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());
 				});
-			}, "fa fa-play", "btn-success");
+			}, __('Send'));
+
+			frm.add_custom_button(__('Schedule sending'), () => {
+				frm.events.schedule_send_dialog(frm);
+			}, __('Send'));
 		}
 
 		frm.events.setup_dashboard(frm);
+		frm.events.setup_sending_status(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);
 		}
+
+		frm.trigger('update_schedule_message');
 	},
 
-	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}: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) {
+			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.slice(0, 5));
+			}
+		}
+		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) + '%';
 				});
 
@@ -94,5 +162,58 @@ 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;
+		}
+	},
+
+	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();
+		}
 	}
 });

frappe/email/doctype/newsletter/newsletter.json

@@ -7,48 +7,59 @@
  "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",
+  "sender_email",
+  "column_break_7",
   "send_from",
-  "schedule_sending",
-  "schedule_send",
   "recipients",
   "email_group",
-  "email_sent",
-  "newsletter_content",
+  "subject_section",
   "subject",
+  "newsletter_content",
   "content_type",
   "message",
   "message_md",
   "message_html",
-  "section_break_13",
+  "attachments",
   "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": [
   {
    "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",
    "fieldtype": "Data",
    "ignore_xss_filter": 1,
-   "label": "Sender"
+   "label": "Sender",
+   "read_only": 1
   },
   {
    "default": "0",
    "fieldname": "email_sent",
    "fieldtype": "Check",
+   "hidden": 1,
    "label": "Email Sent",
    "no_copy": 1,
    "read_only": 1
@@ -87,32 +98,12 @@
    "label": "Published"
   },
   {
+   "depends_on": "published",
    "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,21 +113,16 @@
   {
    "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"
   },
-  {
-   "default": "0",
-   "fieldname": "send_attachments",
-   "fieldtype": "Check",
-   "label": "Send Attachments"
-  },
   {
    "fieldname": "content_type",
    "fieldtype": "Select",
@@ -161,23 +147,87 @@
    "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",
    "fieldname": "send_webview_link",
    "fieldtype": "Check",
    "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"
+  },
+  {
+   "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"
+  },
+  {
+   "fieldname": "attachments",
+   "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,
@@ -187,7 +237,7 @@
  "is_published_field": "published",
  "links": [],
  "max_attachments": 3,
- "modified": "2021-02-22 14:33:56.095380",
+ "modified": "2021-12-06 20:09:37.963141",
  "modified_by": "Administrator",
  "module": "Email",
  "name": "Newsletter",

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]:
@@ -30,29 +28,55 @@ class Newsletter(WebsiteGenerator):
 		return self._recipients
 
 	@frappe.whitelist()
-	def test_send(self):
-		test_emails = frappe.utils.split_emails(self.test_email_id)
-		self.queue_all(test_emails=test_emails)
-		frappe.msgprint(_("Test email sent to {0}").format(self.test_email_id))
+	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.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()
+	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):
-		"""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.
@@ -75,8 +99,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.
@@ -84,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.
 		"""
@@ -116,45 +145,24 @@ 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
 		"""
-		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.
@@ -224,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):
@@ -321,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(

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 %}
     <style>
@@ -36,11 +36,11 @@
                 </p>
             </div>
             <div itemprop="articleBody" class="longform blog-text">
-                {{ doc.message }}
+                {{ doc.get_message() }}
             </div>
         </article>
 
-        {% if attachments %}
+        {% if doc.attachments %}
             <div>
                 <div class="row text-muted">
                     <div class="col-sm-12 h6 text-uppercase">
@@ -49,10 +49,10 @@
                 </div>
                 <div class="row">
                     <div class="col-sm-12">
-                        {% for attachment in attachments %}
+                        {% for attachment in doc.attachments %}
                             <p class="small">
-                                <a href="{{ attachment.file_url }}" target="blank"> 
-                                    {{ attachment.file_name }}
+                                <a href="{{ attachment.attachment }}" target="_blank">
+                                    {{ attachment.attachment }}
                                 </a>
                             </p>
                         {% endfor %}

frappe/email/doctype/newsletter/test_newsletter.py

@@ -14,7 +14,6 @@ from frappe.email.doctype.newsletter.exceptions import (
 from frappe.email.doctype.newsletter.newsletter import (
 	Newsletter,
 	confirmed_unsubscribe,
-	get_newsletter_list,
 	send_scheduled_email
 )
 from frappe.email.queue import flush
@@ -101,7 +100,8 @@ class TestNewsletterMixin:
 		doctype = "Newsletter"
 		newsletter_content = {
 			"subject": "_Test Newsletter",
-			"send_from": "Test Sender <test_sender@example.com>",
+			"sender_name": "Test Sender",
+			"sender_email": "test_sender@example.com",
 			"content_type": "Rich Text",
 			"message": "Testing my news.",
 		}
@@ -157,21 +157,6 @@ class TestNewsletter(TestNewsletterMixin, unittest.TestCase):
 			if email != to_unsubscribe:
 				self.assertTrue(email in recipients)
 
-	def test_portal(self):
-		self.send_newsletter(published=1)
-		frappe.set_user("test1@example.com")
-		newsletter_list = get_newsletter_list("Newsletter", None, None, 0)
-		self.assertEqual(len(newsletter_list), 1)
-
-	def test_newsletter_context(self):
-		context = frappe._dict()
-		newsletter_name = self.send_newsletter(published=1)
-		frappe.set_user("test2@example.com")
-		doc = frappe.get_doc("Newsletter", newsletter_name)
-		doc.get_context(context)
-		self.assertEqual(context.no_cache, 1)
-		self.assertTrue("attachments" not in list(context))
-
 	def test_schedule_send(self):
 		self.send_newsletter(schedule_send=add_days(getdate(), -1))
 
@@ -181,26 +166,32 @@ class TestNewsletter(TestNewsletterMixin, unittest.TestCase):
 		for email in emails:
 			self.assertTrue(email in recipients)
 
-	def test_newsletter_test_send(self):
-		"""Test "Test Send" functionality of Newsletter
+	def test_newsletter_send_test_email(self):
+		"""Test "Send Test Email" functionality of Newsletter
 		"""
 		newsletter = self.get_newsletter()
-		newsletter.test_email_id = choice(emails)
-		newsletter.test_send()
+		test_email = choice(emails)
+		newsletter.send_test_email(test_email)
 
 		self.assertFalse(newsletter.email_sent)
 		newsletter.save = MagicMock()
 		self.assertFalse(newsletter.save.called)
+		# check if the test email is in the queue
+		email_queue = frappe.db.get_all('Email Queue', filters=[
+			['reference_doctype', '=', 'Newsletter'],
+			['reference_name', '=', newsletter.name],
+			['Email Queue Recipient', 'recipient', '=', test_email]
+		])
+		self.assertTrue(email_queue)
 
 	def test_newsletter_status(self):
 		"""Test for Newsletter's stats on onload event
 		"""
 		newsletter = self.get_newsletter()
 		newsletter.email_sent = True
-		# had to use run_onload as calling .onload directly bought weird errors
-		# like TestNewsletter has no attribute "_TestNewsletter__onload"
-		run_onload(newsletter)
-		self.assertIsInstance(newsletter.get("__onload").status_count, dict)
+		result = newsletter.get_sending_status()
+		self.assertTrue('total' in result)
+		self.assertTrue('sent' in result)
 
 	def test_already_sent_newsletter(self):
 		newsletter = self.get_newsletter()
@@ -218,22 +209,6 @@ class TestNewsletter(TestNewsletterMixin, unittest.TestCase):
 			with self.assertRaises(NoRecipientFoundError):
 				newsletter.send_emails()
 
-	def test_send_newsletter_with_attachments(self):
-		newsletter = self.get_newsletter()
-		newsletter.reload()
-		file_attachment = frappe.get_doc({
-			"doctype": "File",
-			"file_name": "test1.txt",
-			"attached_to_doctype": newsletter.doctype,
-			"attached_to_name": newsletter.name,
-			"content": frappe.mock("paragraph")
-		})
-		file_attachment.save()
-		newsletter.send_attachments = True
-		newsletter_attachments = newsletter.get_newsletter_attachments()
-		self.assertEqual(len(newsletter_attachments), 1)
-		self.assertEqual(newsletter_attachments[0]["fid"], file_attachment.name)
-
 	def test_send_scheduled_email_error_handling(self):
 		newsletter = self.get_newsletter(schedule_send=add_days(getdate(), -1))
 		job_path = "frappe.email.doctype.newsletter.newsletter.Newsletter.queue_all"

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"
+}

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

frappe/email/doctype/newsletter_email_group/newsletter_email_group.json

@@ -1,106 +1,42 @@
 {
- "allow_copy": 0, 
- "allow_guest_to_view": 0,
- "allow_import": 0, 
- "allow_rename": 0, 
- "beta": 0, 
- "creation": "2017-02-26 16:20:52.654136", 
- "custom": 0, 
- "docstatus": 0, 
- "doctype": "DocType", 
- "document_type": "", 
- "editable_grid": 1, 
- "engine": "InnoDB", 
+ "actions": [],
+ "creation": "2017-02-26 16:20:52.654136",
+ "doctype": "DocType",
+ "editable_grid": 1,
+ "engine": "InnoDB",
+ "field_order": [
+  "email_group",
+  "total_subscribers"
+ ],
  "fields": [
   {
-   "allow_bulk_edit": 0,
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "email_group", 
-   "fieldtype": "Link", 
-   "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": "Email Group", 
-   "length": 0, 
-   "no_copy": 0, 
-   "options": "Email Group", 
-   "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, 
-   "translatable": 0,
-   "unique": 0
-  }, 
+   "columns": 7,
+   "fieldname": "email_group",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "label": "Email Group",
+   "options": "Email Group",
+   "reqd": 1
+  },
   {
-   "allow_bulk_edit": 0,
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
+   "columns": 3,
    "fetch_from": "email_group.total_subscribers",
-   "fieldname": "total_subscribers", 
-   "fieldtype": "Read Only", 
-   "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": "Total Subscribers", 
-   "length": 0, 
-   "no_copy": 0, 
-   "options": "", 
-   "permlevel": 0, 
-   "precision": "", 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0,
-   "unique": 0
+   "fieldname": "total_subscribers",
+   "fieldtype": "Read Only",
+   "in_list_view": 1,
+   "label": "Total Subscribers"
   }
- ], 
- "has_web_view": 0,
- "hide_heading": 0, 
- "hide_toolbar": 0, 
- "idx": 0, 
- "image_view": 0, 
- "in_create": 0, 
- "is_submittable": 0, 
- "issingle": 0, 
- "istable": 1, 
- "max_attachments": 0, 
- "modified": "2018-05-16 22:42:55.437367",
- "modified_by": "Administrator", 
- "module": "Email", 
- "name": "Newsletter Email Group", 
- "name_case": "", 
- "owner": "Administrator", 
- "permissions": [], 
- "quick_entry": 1, 
- "read_only": 0, 
- "read_only_onload": 0, 
- "show_name_in_global_search": 0, 
- "sort_field": "modified", 
- "sort_order": "DESC", 
- "track_changes": 1, 
- "track_seen": 0
+ ],
+ "istable": 1,
+ "links": [],
+ "modified": "2021-12-06 20:12:08.420240",
+ "modified_by": "Administrator",
+ "module": "Email",
+ "name": "Newsletter Email Group",
+ "owner": "Administrator",
+ "permissions": [],
+ "quick_entry": 1,
+ "sort_field": "modified",
+ "sort_order": "DESC",
+ "track_changes": 1
 }

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)

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

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):

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"))
@@ -1126,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):

frappe/modules/import_file.py

@@ -189,7 +189,7 @@ def update_modified(original_modified, doc):
 		).set(
 			singles_table.value,original_modified
 		).where(
-			singles_table.field == "modified"
+			singles_table["field"] == "modified",  # singles_table.field is a method of pypika Selectable
 		).where(
 			singles_table.doctype == doc["name"]
 		).run()

frappe/patches/v13_0/set_path_for_homepage_in_web_page_view.py

@@ -2,4 +2,4 @@ import frappe
 
 def execute():
 	frappe.reload_doc('website', 'doctype', 'web_page_view', force=True)
-	frappe.db.sql("""UPDATE `tabWeb Page View` set path="/" where path=''""")
+	frappe.db.sql("""UPDATE `tabWeb Page View` set path='/' where path=''""")

frappe/patches/v13_0/update_date_filters_in_user_settings.py

@@ -10,7 +10,7 @@ def execute():
 			select
 				* from `__UserSettings`
 			where
-				user="{user}"
+				user='{user}'
 		'''.format(user = user.user), as_dict=True)
 
 		for setting in user_settings:

frappe/patches/v14_0/copy_mail_data.py

@@ -3,7 +3,9 @@ import frappe
 
 
 def execute():
+	frappe.reload_doc("email", "doctype", "imap_folder")
 	frappe.reload_doc("email", "doctype", "email_account")
+
 	# patch for all Email Account with the flag use_imap
 	for email_account in frappe.get_list("Email Account", filters={"enable_incoming": 1, "use_imap": 1}):
 		# get all data from Email Account

frappe/permissions.py

@@ -108,7 +108,7 @@ def get_doc_permissions(doc, user=None, ptype=None):
 	meta = frappe.get_meta(doc.doctype)
 
 	def is_user_owner():
-		return (doc.get("owner") or "").lower() == frappe.session.user.lower()
+		return (doc.get("owner") or "").lower() == user.lower()
 
 	if has_controller_permissions(doc, ptype, user=user) is False:
 		push_perm_check_log('Not allowed via controller permission check')

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;
+	}
 };

frappe/public/js/frappe/form/controls/markdown_editor.js

@@ -2,14 +2,16 @@ frappe.ui.form.ControlMarkdownEditor = class ControlMarkdownEditor extends frapp
 	static editor_class = 'markdown'
 	make_ace_editor() {
 		super.make_ace_editor();
+		if (this.markdown_container) return;
 
-		this.ace_editor_target.wrap(`<div class="${this.editor_class}-container">`);
-		this.markdown_container = this.$input_wrapper.find(`.${this.constructor.editor_class}-container`);
+		let editor_class = this.constructor.editor_class;
+		this.ace_editor_target.wrap(`<div class="${editor_class}-container">`);
+		this.markdown_container = this.$input_wrapper.find(`.${editor_class}-container`);
 
 		this.editor.getSession().setUseWrapMode(true);
 
 		this.showing_preview = false;
-		this.preview_toggle_btn = $(`<button class="btn btn-default btn-xs ${this.editor_class}-toggle">${__('Preview')}</button>`)
+		this.preview_toggle_btn = $(`<button class="btn btn-default btn-xs ${editor_class}-toggle">${__('Preview')}</button>`)
 			.click(e => {
 				if (!this.showing_preview) {
 					this.update_preview();
@@ -25,7 +27,7 @@ frappe.ui.form.ControlMarkdownEditor = class ControlMarkdownEditor extends frapp
 			});
 		this.markdown_container.prepend(this.preview_toggle_btn);
 
-		this.markdown_preview = $(`<div class="${this.editor_class}-preview border rounded">`).hide();
+		this.markdown_preview = $(`<div class="${editor_class}-preview border rounded">`).hide();
 		this.markdown_container.append(this.markdown_preview);
 	}
 

frappe/public/js/frappe/form/footer/form_timeline.js

@@ -172,9 +172,11 @@ class FormTimeline extends BaseTimeline {
 
 	get_communication_timeline_contents() {
 		let communication_timeline_contents = [];
+		let icon_set = {Email: "mail", Phone: "call", Meeting: "calendar", Other: "dot-horizontal"};
 		(this.doc_info.communications|| []).forEach(communication => {
+			let medium = communication.communication_medium;
 			communication_timeline_contents.push({
-				icon: 'mail',
+				icon: icon_set[medium],
 				icon_size: 'sm',
 				creation: communication.creation,
 				is_card: true,

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

frappe/public/js/frappe/module_editor.js

@@ -1,38 +1,54 @@
 frappe.ModuleEditor = class ModuleEditor {
 	constructor(frm, wrapper) {
-		this.wrapper = $('<div class="row module-block-list"></div>').appendTo(wrapper);
 		this.frm = frm;
-		this.make();
-	}
-	make() {
-		var me = this;
-		this.frm.doc.__onload.all_modules.forEach(function(m) {
-			$(repl('<div class="col-sm-6"><div class="checkbox">\
-				<label><input type="checkbox" class="block-module-check" data-module="%(module)s">\
-				%(module)s</label></div></div>', {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;
 			}
 		});
 	}

frappe/public/js/frappe/request.js

@@ -481,6 +481,24 @@ frappe.request.report_error = function(xhr, request_opts) {
 		exc = "";
 	}
 
+	const copy_markdown_to_clipboard = () => {
+		const code_block = snippet => '```\n' + snippet + '\n```';
+		const traceback_info = [
+			'### App Versions',
+			code_block(JSON.stringify(frappe.boot.versions, null, "\t")),
+			'### Route',
+			code_block(frappe.get_route_str()),
+			'### Trackeback',
+			code_block(exc),
+			'### Request Data',
+			code_block(JSON.stringify(request_opts, null, "\t")),
+			'### Response Data',
+			code_block(JSON.stringify(data, null, '\t')),
+		].join("\n");
+		frappe.utils.copy_to_clipboard(traceback_info);
+	};
+
+
 	var show_communication = function() {
 		var error_report_message = [
 			'<h5>Please type some additional information that could help us reproduce this issue:</h5>',
@@ -532,6 +550,11 @@ frappe.request.report_error = function(xhr, request_opts) {
 						frappe.msgprint(__('Support Email Address Not Specified'));
 					}
 					frappe.error_dialog.hide();
+				},
+				secondary_action_label: __('Copy error to clipboard'),
+				secondary_action: () => {
+					copy_markdown_to_clipboard();
+					frappe.error_dialog.hide();
 				}
 			});
 			frappe.error_dialog.wrapper.classList.add('msgprint-dialog');

frappe/public/js/frappe/utils/utils.js

@@ -316,7 +316,7 @@ Object.assign(frappe.utils, {
 		}
 	},
 	get_scroll_position: function(element, additional_offset) {
-		let header_offset = $(".navbar").height() + $(".page-head:visible").height();
+		let header_offset = $(".navbar").height() + $(".page-head:visible").height() || $(".navbar").height();
 		let scroll_top = $(element).offset().top - header_offset - cint(additional_offset);
 		return scroll_top;
 	},
@@ -957,17 +957,24 @@ Object.assign(frappe.utils, {
 		return decoded;
 	},
 	copy_to_clipboard(string) {
-		let input = $("<input>");
-		$("body").append(input);
-		input.val(string).select();
+		const show_success_alert = () => {
+			frappe.show_alert({
+				indicator: 'green',
+				message: __('Copied to clipboard.')
+			});
+		};
+		if (navigator.clipboard && window.isSecureContext) {
+			navigator.clipboard.writeText(string).then(show_success_alert);
+		} else {
+			let input = $("<textarea>");
+			$("body").append(input);
+			input.val(string).select();
 
-		document.execCommand("copy");
-		input.remove();
+			document.execCommand("copy");
+			show_success_alert();
+			input.remove();
+		}
 
-		frappe.show_alert({
-			indicator: 'green',
-			message: __('Copied to clipboard.')
-		});
 	},
 	is_rtl(lang=null) {
 		return ["ar", "he", "fa", "ps"].includes(lang || frappe.boot.lang);
@@ -1376,5 +1383,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;
 	}
 });

frappe/public/js/frappe/views/communication.js

@@ -351,7 +351,7 @@ frappe.views.CommunicationComposer = class {
 	}
 
 	async set_values_from_last_edited_communication() {
-		if (this.txt) return;
+		if (this.txt || this.message) return;
 
 		const last_edited = this.get_last_edited_communication();
 		if (!last_edited.content) return;
@@ -713,7 +713,7 @@ frappe.views.CommunicationComposer = class {
 	async set_content() {
 		if (this.content_set) return;
 
-		let message = this.txt || "";
+		let message = this.txt || this.message || "";
 		if (!message && this.frm) {
 			const { doctype, docname } = this.frm;
 			message = await localforage.getItem(doctype + docname) || "";
@@ -727,7 +727,7 @@ frappe.views.CommunicationComposer = class {
 
 		const SALUTATION_END_COMMENT = "<!-- salutation-ends -->";
 		if (this.real_name && !message.includes(SALUTATION_END_COMMENT)) {
-			this.message = `
+			message = `
 				<p>${__('Dear {0},', [this.real_name], 'Salutation in new email')},</p>
 				${SALUTATION_END_COMMENT}<br>
 				${message}

frappe/public/js/frappe/views/file/file_view.js

@@ -169,6 +169,15 @@ frappe.views.FileView = class FileView extends frappe.views.ListView {
 				frappe.file_manager.paste(this.current_folder)
 			)
 			.hide();
+
+		this.page.add_actions_menu_item(__('Export as zip'), () => {
+			let docnames = this.get_checked_items(true);
+			if (docnames.length) {
+				open_url_post('/api/method/frappe.core.doctype.file.file.zip_files', {
+					files: JSON.stringify(docnames)
+				});
+			}
+		});
 	}
 
 	set_fields() {

frappe/public/scss/common/css_variables.scss

@@ -225,4 +225,7 @@
 	--checkbox-right-margin: var(--margin-xs);
 	--checkbox-size: 14px;
 	--checkbox-focus-shadow: 0 0 0 2px var(--gray-300);
+
+	--right-arrow-svg: url("data: image/svg+xml;utf8, <svg width='6' height='8' viewBox='0 0 6 8' fill='none' xmlns='http://www.w3.org/2000/svg'><path d='M1.25 7.5L4.75 4L1.25 0.5' stroke='%231F272E' stroke-linecap='round' stroke-linejoin='round'/></svg>");
+	--left-arrow-svg: url("data: image/svg+xml;utf8, <svg width='6' height='8' viewBox='0 0 6 8' fill='none' xmlns='http://www.w3.org/2000/svg'><path d='M7.5 9.5L4 6l3.5-3.5' stroke='%231F272E' stroke-linecap='round' stroke-linejoin='round'></path></svg>");
 }

frappe/public/scss/desk/css_variables.scss

@@ -63,7 +63,4 @@ $input-height: 28px !default;
 	// skeleton
 	--skeleton-bg: var(--gray-100);
 
-	--right-arrow-svg: url("data: image/svg+xml;utf8, <svg width='6' height='8' viewBox='0 0 6 8' fill='none' xmlns='http://www.w3.org/2000/svg'><path d='M1.25 7.5L4.75 4L1.25 0.5' stroke='%231F272E' stroke-linecap='round' stroke-linejoin='round'/></svg>");
-
-	--left-arrow-svg: url("data: image/svg+xml;utf8, <svg width='6' height='8' viewBox='0 0 6 8' fill='none' xmlns='http://www.w3.org/2000/svg'><path d='M7.5 9.5L4 6l3.5-3.5' stroke='%231F272E' stroke-linecap='round' stroke-linejoin='round'></path></svg>");
 }

frappe/public/scss/website/index.scss

@@ -112,9 +112,30 @@
 }
 
 .breadcrumb {
-	padding: 0;
+	padding-left: 0;
+	padding-right: 0;
 	font-size: $font-size-sm;
-	background-color: $breadcrumb-bg;
+}
+
+.breadcrumb-item {
+	+ .breadcrumb-item
+	 {
+		font-size: $font-size-sm;
+		&::before {
+			content: #{"/*!rtl:var(--left-arrow-svg);*/"}var(--right-arrow-svg);
+			display: inline-block;
+		}
+	}
+	a {
+		color: var(--text-color)
+	}
+	
+	li.disabled {
+		a {
+			color: var(--text-muted);
+			pointer-events: none;
+		}
+	}
 }
 
 a.card {
@@ -196,11 +217,14 @@ h5.modal-title {
 
 .btn-xs {
 	@extend .btn-sm;
+
 }
 
 .hidden-xs {
 	@extend .d-block;
-	@extend .d-sm-none;
+	@include media-breakpoint-between(xs, sm) {
+		display: none !important;
+	}
 }
 
 .visible-xs {
@@ -266,7 +290,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;

frappe/query_builder/__init__.py

@@ -1,2 +1,8 @@
+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

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)

frappe/query_builder/custom.py

@@ -1,7 +1,8 @@
-from typing import Optional
+from typing import Any, Optional
 
 from pypika.functions import DistinctOptionFunction
-from pypika.utils import builder
+from pypika.terms import Term
+from pypika.utils import builder, format_alias_sql, format_quotes
 
 import frappe
 
@@ -81,3 +82,23 @@ class TO_TSVECTOR(DistinctOptionFunction):
 			text (str): [ the text string that we match it against ]
 		"""
 		self._PLAINTO_TSQUERY = text
+
+
+class ConstantColumn(Term):
+	alias = None
+
+	def __init__(self, value: str) -> None:
+		"""[ Returns a pseudo column with a constant value in all the rows]
+
+		Args:
+			value (str): [ Value of the column ]
+		"""
+		self.value = value
+
+	def get_sql(self, quote_char: Optional[str] = None, **kwargs: Any) -> str:
+		return format_alias_sql(
+					format_quotes(self.value, kwargs.get("secondary_quote_char") or ""),
+					self.alias or self.value,
+					quote_char=quote_char,
+					**kwargs
+				)

frappe/query_builder/terms.py

@@ -0,0 +1,49 @@
+from typing import Any, Dict, Optional
+
+from pypika.terms import Function, 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[2:-2]] = param_value
+
+	def get_sql(self, **kwargs):
+		return f'%(param{len(self.parameters) + 1})s'
+
+
+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) 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 function_sql

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) # nosemgrep
+
+	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
+	frappe.qb.sum = _sum

frappe/templates/emails/newsletter.html

@@ -7,7 +7,7 @@
 {% if published and send_webview_link %}
 <div style="font-size: 12px; line-height: 20px;">
     <div>
-        Open in <a style="color: #687178; text-decoration: underline;" href="/newsletters/{{ name }}" target="_blank">web</a>
+        <a style="color: #687178; text-decoration: underline;" href="/newsletters/{{ name }}" target="_blank">View this email on the web</a>
     </div>
 </div>
 {% endif %}

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"]}))
 

frappe/tests/test_query_builder.py

@@ -2,7 +2,8 @@ import unittest
 from typing import Callable
 
 import frappe
-from frappe.query_builder.functions import GroupConcat, Match
+from frappe.query_builder.custom import ConstantColumn
+from frappe.query_builder.functions import Coalesce, GroupConcat, Match
 from frappe.query_builder.utils import db_type_is
 
 
@@ -23,7 +24,9 @@ class TestCustomFunctionsMariaDB(unittest.TestCase):
 			" MATCH('Notes') AGAINST ('+text*' IN BOOLEAN MODE)", query.get_sql()
 		)
 
-
+	def test_constant_column(self):
+		query = frappe.qb.from_("DocType").select("name", ConstantColumn("John").as_("User"))
+		self.assertEqual(query.get_sql(), "SELECT `name`,'John' `User` FROM `tabDocType`")
 @run_only_if(db_type_is.POSTGRES)
 class TestCustomFunctionsPostgres(unittest.TestCase):
 	def test_concat(self):
@@ -35,6 +38,9 @@ class TestCustomFunctionsPostgres(unittest.TestCase):
 			"TO_TSVECTOR('Notes') @@ PLAINTO_TSQUERY('text')", query.get_sql()
 		)
 
+	def test_constant_column(self):
+		query = frappe.qb.from_("DocType").select("name", ConstantColumn("John").as_("User"))
+		self.assertEqual(query.get_sql(), 'SELECT "name",\'John\' "User" FROM "tabDocType"')
 
 class TestBuilderBase(object):
 	def test_adding_tabs(self):
@@ -49,6 +55,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 +84,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):

frappe/utils/__init__.py

@@ -56,6 +56,12 @@ def get_email_address(user=None):
 def get_formatted_email(user, mail=None):
 	"""get Email Address of user formatted as: `John Doe <johndoe@example.com>`"""
 	fullname = get_fullname(user)
+	
+	method = get_hook_method('get_sender_details')
+	if method:
+		sender_name, mail = method()
+		# if method exists but sender_name is ""
+		fullname = sender_name or fullname
 
 	if not mail:
 		mail = get_email_address(user) or validate_email_address(user)
@@ -94,7 +100,7 @@ def validate_name(name, throw=False):
 		return False
 
 	name = name.strip()
-	match = re.match(r"^[\w][\w\'\-]*([ \w][\w\'\-]+)*$", name)
+	match = re.match(r"^[\w][\w\'\-]*( \w[\w\'\-]*)*$", name)
 
 	if not match and throw:
 		frappe.throw(frappe._("{0} is not a valid Name").format(name), frappe.InvalidNameError)
@@ -240,7 +246,9 @@ def get_traceback() -> str:
 		return ""
 
 	trace_list = traceback.format_exception(exc_type, exc_value, exc_tb)
-	return "".join(cstr(t) for t in trace_list)
+	bench_path = get_bench_path() + "/"
+
+	return "".join(cstr(t) for t in trace_list).replace(bench_path, "")
 
 def log(event, details):
 	frappe.logger().info(details)