Merge pull request #17773 from frappe/mergify/bp/version-14-hotfix/pr-17377

refactor: Email Domain (backport #17377)

frappe/email/doctype/email_domain/email_domain.js

@@ -1,28 +1,22 @@
 frappe.ui.form.on("Email Domain", {
-	email_id: function (frm) {
-		frm.set_value("domain_name", frm.doc.email_id.split("@")[1]);
-	},
-
-	refresh: function (frm) {
-		if (frm.doc.email_id) {
-			frm.set_value("domain_name", frm.doc.email_id.split("@")[1]);
-		}
-
-		if (frm.doc.__islocal != 1 && frappe.route_flags.return_to_email_account) {
-			var route = frappe.get_prev_route();
-			delete frappe.route_flags.return_to_email_account;
-			frappe.route_flags.set_domain_values = true;
-
-			(frappe.route_options = {
-				domain: frm.doc.name,
-				use_imap: frm.doc.use_imap,
-				email_server: frm.doc.email_server,
-				use_ssl: frm.doc.use_ssl,
-				smtp_server: frm.doc.smtp_server,
-				use_tls: frm.doc.use_tls,
-				smtp_port: frm.doc.smtp_port,
-			}),
-				frappe.set_route(route);
+	onload: function (frm) {
+		if (!frm.doc.__islocal) {
+			frm.dashboard.clear_headline();
+			let msg = __(
+				"Changing any setting will reflect on all the email accounts associated with this domain."
+			);
+			frm.dashboard.set_headline_alert(msg);
+		} else {
+			if (!frm.doc.attachment_limit) {
+				frappe.call({
+					method: "frappe.core.api.file.get_max_file_size",
+					callback: function (r) {
+						if (!r.exc) {
+							frm.set_value("attachment_limit", Number(r.message) / (1024 * 1024));
+						}
+					},
+				});
+			}
 		}
 	},
 });

frappe/email/doctype/email_domain/email_domain.json

@@ -8,11 +8,11 @@
  "field_order": [
   "email_settings",
   "domain_name",
-  "email_id",
   "mailbox_settings",
   "email_server",
   "use_imap",
   "use_ssl",
+  "column_break_9",
   "incoming_port",
   "attachment_limit",
   "append_to",
@@ -20,8 +20,9 @@
   "smtp_server",
   "use_tls",
   "use_ssl_for_outgoing",
-  "append_emails_to_sent_folder",
-  "smtp_port"
+  "column_break_18",
+  "smtp_port",
+  "append_emails_to_sent_folder"
  ],
  "fields": [
   {
@@ -31,26 +32,20 @@
   {
    "fieldname": "domain_name",
    "fieldtype": "Data",
-   "label": "domain name",
-   "read_only": 1,
+   "label": "Domain Name",
+   "reqd": 1,
    "unique": 1
   },
-  {
-   "fieldname": "email_id",
-   "fieldtype": "Data",
-   "label": "Example Email Address",
-   "options": "Email",
-   "reqd": 1
-  },
   {
    "fieldname": "mailbox_settings",
-   "fieldtype": "Section Break"
+   "fieldtype": "Section Break",
+   "label": "Incoming Settings"
   },
   {
    "description": "e.g. pop.gmail.com / imap.gmail.com",
    "fieldname": "email_server",
    "fieldtype": "Data",
-   "label": "Email Server",
+   "label": "Incoming Server",
    "reqd": 1
   },
   {
@@ -66,7 +61,6 @@
    "label": "Use SSL"
   },
   {
-   "default": "1",
    "description": "Ignore attachments over this size",
    "fieldname": "attachment_limit",
    "fieldtype": "Int",
@@ -83,13 +77,14 @@
   },
   {
    "fieldname": "outgoing_mail_settings",
-   "fieldtype": "Section Break"
+   "fieldtype": "Section Break",
+   "label": "Outgoing Settings"
   },
   {
    "description": "e.g. smtp.gmail.com",
    "fieldname": "smtp_server",
    "fieldtype": "Data",
-   "label": "SMTP Server",
+   "label": "Outgoing Server",
    "reqd": 1
   },
   {
@@ -120,15 +115,29 @@
    "default": "0",
    "fieldname": "use_ssl_for_outgoing",
    "fieldtype": "Check",
-   "label": "Use SSL for Outgoing"
+   "label": "Use SSL"
+  },
+  {
+   "fieldname": "column_break_9",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "column_break_18",
+   "fieldtype": "Column Break"
   }
  ],
  "icon": "icon-inbox",
- "links": [],
- "modified": "2019-12-18 15:57:34.445308",
+ "links": [
+  {
+   "link_doctype": "Email Account",
+   "link_fieldname": "domain"
+  }
+ ],
+ "modified": "2022-07-14 13:01:48.719727",
  "modified_by": "Administrator",
  "module": "Email",
  "name": "Email Domain",
+ "naming_rule": "By fieldname",
  "owner": "Administrator",
  "permissions": [
   {
@@ -142,5 +151,7 @@
   }
  ],
  "sort_field": "modified",
- "sort_order": "DESC"
+ "sort_order": "DESC",
+ "states": [],
+ "track_changes": 1
 }

frappe/email/doctype/email_domain/email_domain.py

@@ -1,101 +1,51 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and contributors
 # License: MIT. See LICENSE
 
-import imaplib
-import poplib
 import smtplib
+from functools import wraps
 
 import frappe
 from frappe import _
+from frappe.email.receive import Timed_IMAP4, Timed_IMAP4_SSL, Timed_POP3, Timed_POP3_SSL
 from frappe.email.utils import get_port
 from frappe.model.document import Document
-from frappe.utils import cint, cstr, validate_email_address
+from frappe.utils import cint
 
 
-class EmailDomain(Document):
-	def autoname(self):
-		if self.domain_name:
-			self.name = self.domain_name
-
-	def validate(self):
-		"""Validate email id and check POP3/IMAP and SMTP connections is enabled."""
-		logger = frappe.logger()
+def get_error_message(event):
+	return {
+		"incoming": (_("Incoming email account not correct"), _("Error connecting via IMAP/POP3: {e}")),
+		"outgoing": (_("Outgoing email account not correct"), _("Error connecting via SMTP: {e}")),
+	}[event]
 
-		if self.email_id:
-			validate_email_address(self.email_id, True)
 
-		if frappe.local.flags.in_patch or frappe.local.flags.in_test:
-			return
-
-		if not frappe.local.flags.in_install and not frappe.local.flags.in_patch:
+def handle_error(event):
+	def decorator(fn):
+		@wraps(fn)
+		def wrapper(*args, **kwargs):
+			err_title, err_message = get_error_message(event)
 			try:
-				if self.use_imap:
-					logger.info(
-						"Checking incoming IMAP email server {host}:{port} ssl={ssl}...".format(
-							host=self.email_server, port=get_port(self), ssl=self.use_ssl
-						)
-					)
-					if self.use_ssl:
-						test = imaplib.IMAP4_SSL(self.email_server, port=get_port(self))
-					else:
-						test = imaplib.IMAP4(self.email_server, port=get_port(self))
-
-				else:
-					logger.info(
-						"Checking incoming POP3 email server {host}:{port} ssl={ssl}...".format(
-							host=self.email_server, port=get_port(self), ssl=self.use_ssl
-						)
-					)
-					if self.use_ssl:
-						test = poplib.POP3_SSL(self.email_server, port=get_port(self))
-					else:
-						test = poplib.POP3(self.email_server, port=get_port(self))
-
+				fn(*args, **kwargs)
 			except Exception as e:
-				logger.warn(f'Incoming email account "{self.email_server}" not correct', exc_info=e)
 				frappe.throw(
-					title=_("Incoming email account not correct"),
-					msg=f'Error connecting IMAP/POP3 "{self.email_server}": {e}',
+					title=err_title,
+					msg=err_message.format(e=e),
 				)
 
-			finally:
-				try:
-					if self.use_imap:
-						test.logout()
-					else:
-						test.quit()
-				except Exception:
-					pass
+		return wrapper
+
+	return decorator
 
-			try:
-				if self.get("use_ssl_for_outgoing"):
-					if not self.get("smtp_port"):
-						self.smtp_port = 465
-
-					logger.info(
-						"Checking outgoing SMTPS email server {host}:{port}...".format(
-							host=self.smtp_server, port=self.smtp_port
-						)
-					)
-					sess = smtplib.SMTP_SSL(
-						(self.smtp_server or "").encode("utf-8"), cint(self.smtp_port) or None
-					)
-				else:
-					if self.use_tls and not self.smtp_port:
-						self.smtp_port = 587
-					logger.info(
-						"Checking outgoing SMTP email server {host}:{port} STARTTLS={tls}...".format(
-							host=self.smtp_server, port=self.get("smtp_port"), tls=self.use_tls
-						)
-					)
-					sess = smtplib.SMTP(cstr(self.smtp_server or ""), cint(self.smtp_port) or None)
-				sess.quit()
-			except Exception as e:
-				logger.warn(f'Outgoing email account "{self.smtp_server}" not correct', exc_info=e)
-				frappe.throw(
-					title=_("Outgoing email account not correct"),
-					msg=f'Error connecting SMTP "{self.smtp_server}": {e}',
-				)
+
+class EmailDomain(Document):
+	def validate(self):
+		"""Validate POP3/IMAP and SMTP connections."""
+
+		if frappe.local.flags.in_patch or frappe.local.flags.in_test or frappe.local.flags.in_install:
+			return
+
+		self.validate_incoming_server_conn()
+		self.validate_outgoing_server_conn()
 
 	def on_update(self):
 		"""update all email accounts using this domain"""
@@ -121,3 +71,26 @@ class EmailDomain(Document):
 				frappe.msgprint(
 					_("Error has occurred in {0}").format(email_account.name), raise_exception=e.__class__
 				)
+
+	@handle_error("incoming")
+	def validate_incoming_server_conn(self):
+		self.incoming_port = get_port(self)
+
+		conn_method = Timed_POP3_SSL if self.use_ssl else Timed_POP3
+		if self.use_imap:
+			conn_method = Timed_IMAP4_SSL if self.use_ssl else Timed_IMAP4
+
+		incoming_conn = conn_method(self.email_server, port=self.incoming_port)
+		incoming_conn.logout() if self.use_imap else incoming_conn.quit()
+
+	@handle_error("outgoing")
+	def validate_outgoing_server_conn(self):
+		conn_method = smtplib.SMTP
+
+		if self.use_ssl_for_outgoing:
+			self.smtp_port = self.smtp_port or 465
+			conn_method = smtplib.SMTP_SSL
+		elif self.use_tls:
+			self.smtp_port = self.smtp_port or 587
+
+		conn_method((self.smtp_server or ""), cint(self.smtp_port) or 0).quit()