[fix] set Message-Id in email based on reference and tag replies to the same object

frappe/__init__.py

@@ -359,8 +359,8 @@ def sendmail(recipients=(), sender="", subject="No Subject", message="No Message
 		as_markdown=False, delayed=True, reference_doctype=None, reference_name=None,
 		unsubscribe_method=None, unsubscribe_params=None, unsubscribe_message=None,
 		attachments=None, content=None, doctype=None, name=None, reply_to=None,
-		cc=(), show_as_cc=(), message_id=None, in_reply_to=None, send_after=None, expose_recipients=False,
-		send_priority=1, communication=None, retry=1):
+		cc=(), show_as_cc=(), in_reply_to=None, send_after=None, expose_recipients=False,
+		send_priority=1, communication=None, retry=1, now=None):
 	"""Send email using user's default **Email Account** or global default **Email Account**.
 
 
@@ -377,7 +377,6 @@ def sendmail(recipients=(), sender="", subject="No Subject", message="No Message
 	:param unsubscribe_params: Unsubscribe paramaters to be loaded on the unsubscribe_method [optional] (dict).
 	:param attachments: List of attachments.
 	:param reply_to: Reply-To email id.
-	:param message_id: Used for threading. If a reply is received to this email, Message-Id is sent back as In-Reply-To in received email.
 	:param in_reply_to: Used to send the Message-Id of a received email back as In-Reply-To.
 	:param send_after: Send after the given datetime.
 	:param expose_recipients: Display all recipients in the footer message - "This email was sent to"
@@ -389,6 +388,8 @@ def sendmail(recipients=(), sender="", subject="No Subject", message="No Message
 		from markdown2 import markdown
 		message = markdown(message)
 
+	if now!=None:
+		delayed = not now
 
 	import email
 	if delayed:
@@ -397,12 +398,12 @@ def sendmail(recipients=(), sender="", subject="No Subject", message="No Message
 			subject=subject, message=message,
 			reference_doctype = doctype or reference_doctype, reference_name = name or reference_name,
 			unsubscribe_method=unsubscribe_method, unsubscribe_params=unsubscribe_params, unsubscribe_message=unsubscribe_message,
-			attachments=attachments, reply_to=reply_to, cc=cc, show_as_cc=show_as_cc, message_id=message_id, in_reply_to=in_reply_to,
+			attachments=attachments, reply_to=reply_to, cc=cc, show_as_cc=show_as_cc, in_reply_to=in_reply_to,
 			send_after=send_after, expose_recipients=expose_recipients, send_priority=send_priority, communication=communication)
 	else:
 		email.sendmail(recipients, sender=sender,
 			subject=subject, msg=content or message, attachments=attachments, reply_to=reply_to,
-			cc=cc, message_id=message_id, in_reply_to=in_reply_to, retry=retry)
+			cc=cc, in_reply_to=in_reply_to, retry=retry)
 
 whitelisted = []
 guest_methods = []

frappe/commands/scheduler.py

@@ -27,7 +27,7 @@ def trigger_scheduler_event(context, event):
 		try:
 			frappe.init(site=site)
 			frappe.connect()
-			frappe.utils.scheduler.trigger(site, event, now=context.force)
+			frappe.utils.scheduler.trigger(site, event, now=True)
 		finally:
 			frappe.destroy()
 

frappe/core/doctype/communication/email.py

@@ -136,7 +136,6 @@ def _notify(doc, print_html=None, print_format=None, attachments=None,
 		reference_doctype=doc.reference_doctype,
 		reference_name=doc.reference_name,
 		attachments=doc.attachments,
-		message_id=doc.name,
 		unsubscribe_message=unsubscribe_message,
 		delayed=True,
 		communication=doc.name

frappe/email/__init__.py

@@ -6,15 +6,12 @@ import frappe
 
 from frappe.email.email_body import get_email
 from frappe.email.smtp import send
-from frappe.utils import markdown
 
-def sendmail(recipients, sender='', msg='', subject='[No Subject]', attachments=None, content=None,
-	reply_to=None, cc=(), message_id=None, in_reply_to=None, retry=1):
+def sendmail(recipients, sender='', msg='', subject='[No Subject]', attachments=None,
+	content=None, reply_to=None, cc=(), in_reply_to=None, retry=1):
 	"""send an html email as multipart with attachments and all"""
 	mail = get_email(recipients, sender, content or msg, subject, attachments=attachments,
 		reply_to=reply_to, cc=cc)
-	if message_id:
-		mail.set_message_id(message_id)
 	if in_reply_to:
 		mail.set_in_reply_to(in_reply_to)
 

frappe/email/doctype/email_account/email_account.py

@@ -270,46 +270,50 @@ class EmailAccount(Document):
 
 		If no thread id is found and `append_to` is set for the email account,
 		it will create a new parent transaction (e.g. Issue)"""
-		in_reply_to = (email.mail.get("In-Reply-To") or "").strip(" <>")
 		parent = None
 
-		if self.append_to:
-			# set subject_field and sender_field
-			meta_module = frappe.get_meta_module(self.append_to)
-			meta = frappe.get_meta(self.append_to)
-			subject_field = getattr(meta_module, "subject_field", "subject")
-			if not meta.get_field(subject_field):
-				subject_field = None
-			sender_field = getattr(meta_module, "sender_field", "sender")
-			if not meta.get_field(sender_field):
-				sender_field = None
+		parent = self.find_parent_from_in_reply_to(email, communication)
 
-		if in_reply_to:
-			if "@{0}".format(frappe.local.site) in in_reply_to:
+		if not parent:
+			self.set_sender_field_and_subject_field()
 
-				# reply to a communication sent from the system
-				in_reply_to, domain = in_reply_to.split("@", 1)
+		if not parent and self.append_to:
+			parent = self.find_parent_based_on_subject_and_sender(email, communication)
 
-				if frappe.db.exists("Communication", in_reply_to):
-					parent = frappe.get_doc("Communication", in_reply_to)
+		if not parent and self.append_to and self.append_to!="Communication":
+			parent = self.create_new_parent(email, communication)
 
-					# set in_reply_to of current communication
-					communication.in_reply_to = in_reply_to
+		if parent:
+			communication.reference_doctype = parent.doctype
+			communication.reference_name = parent.name
+
+	def set_sender_field_and_subject_field(self):
+		'''Identify the sender and subject fields from the `append_to` DocType'''
+		# set subject_field and sender_field
+		meta_module = frappe.get_meta_module(self.append_to)
+		meta = frappe.get_meta(self.append_to)
+
+		self.subject_field = getattr(meta_module, "subject_field", "subject")
+		if not meta.get_field(self.subject_field):
+			self.subject_field = None
 
-					if parent.reference_name:
-						parent = frappe.get_doc(parent.reference_doctype,
-							parent.reference_name)
+		self.sender_field = getattr(meta_module, "sender_field", "sender")
+		if not meta.get_field(self.sender_field):
+			self.sender_field = None
 
-		if not parent and self.append_to and sender_field:
-			if subject_field:
+	def find_parent_based_on_subject_and_sender(self, email, communication):
+		'''Find parent document based on subject and sender match'''
+
+		if self.append_to and self.sender_field:
+			if self.subject_field:
 				# try and match by subject and sender
 				# if sent by same sender with same subject,
 				# append it to old coversation
 				subject = strip(re.sub("^\s*(Re|RE)[^:]*:\s*", "", email.subject))
 
 				parent = frappe.db.get_all(self.append_to, filters={
-					sender_field: email.from_email,
-					subject_field: ("like", "%{0}%".format(subject)),
+					self.sender_field: email.from_email,
+					self.subject_field: ("like", "%{0}%".format(subject)),
 					"creation": (">", (get_datetime() - relativedelta(days=10)).strftime(DATE_FORMAT))
 				}, fields="name")
 
@@ -318,42 +322,73 @@ class EmailAccount(Document):
 				# and subject is atleast 10 chars long
 				if not parent and len(subject) > 10 and is_system_user(email.from_email):
 					parent = frappe.db.get_all(self.append_to, filters={
-						subject_field: ("like", "%{0}%".format(subject)),
+						self.subject_field: ("like", "%{0}%".format(subject)),
 						"creation": (">", (get_datetime() - relativedelta(days=10)).strftime(DATE_FORMAT))
 					}, fields="name")
 
 			if parent:
 				parent = frappe.get_doc(self.append_to, parent[0].name)
+				return parent
 
-		if not parent and self.append_to and self.append_to!="Communication":
-			# no parent found, but must be tagged
-			# insert parent type doc
-			parent = frappe.new_doc(self.append_to)
 
-			if subject_field:
-				parent.set(subject_field, email.subject)
+	def create_new_parent(self, email, communication):
+		'''If no parent found, create a new reference document'''
 
-			if sender_field:
-				parent.set(sender_field, email.from_email)
+		# no parent found, but must be tagged
+		# insert parent type doc
+		parent = frappe.new_doc(self.append_to)
 
-			parent.flags.ignore_mandatory = True
+		if self.subject_field:
+			parent.set(self.subject_field, email.subject)
 
-			try:
-				parent.insert(ignore_permissions=True)
-			except frappe.DuplicateEntryError:
-				# try and find matching parent
-				parent_name = frappe.db.get_value(self.append_to, {sender_field: email.from_email})
-				if parent_name:
-					parent.name = parent_name
-				else:
-					parent = None
+		if self.sender_field:
+			parent.set(self.sender_field, email.from_email)
 
-			# NOTE if parent isn't found and there's no subject match, it is likely that it is a new conversation thread and hence is_first = True
-			communication.is_first = True
+		parent.flags.ignore_mandatory = True
 
-		if parent:
-			communication.reference_doctype = parent.doctype
-			communication.reference_name = parent.name
+		try:
+			parent.insert(ignore_permissions=True)
+		except frappe.DuplicateEntryError:
+			# try and find matching parent
+			parent_name = frappe.db.get_value(self.append_to, {self.sender_field: email.from_email})
+			if parent_name:
+				parent.name = parent_name
+			else:
+				parent = None
+
+		# NOTE if parent isn't found and there's no subject match, it is likely that it is a new conversation thread and hence is_first = True
+		communication.is_first = True
+
+		return parent
+
+	def find_parent_from_in_reply_to(self, communication, email):
+		'''Returns parent reference if embedded in In-Reply-To header
+
+		Message-ID is formatted as `{random}.{doctype}.{name}@{site}`'''
+		parent = None
+		in_reply_to = (email.mail.get("In-Reply-To") or "").strip(" <>")
+
+		if in_reply_to and "@{0}".format(frappe.local.site) in in_reply_to:
+
+			# reply to a communication sent from the system
+			reference, domain = in_reply_to.split("@", 1)
+			if '.' in reference:
+				t, parent_doctype, parent_name = reference.split('.', 2)
+			else:
+				parent_doctype, parent_name = 'Communication', reference
+
+			if frappe.db.exists(parent_doctype, parent_name):
+				parent = frappe.get_doc(parent_doctype, parent_name)
+
+				# set in_reply_to of current communication
+				if parent_doctype=='Communication':
+					communication.in_reply_to = in_reply_to
+
+				if parent.reference_name:
+					parent = frappe.get_doc(parent.reference_doctype,
+						parent.reference_name)
+
+		return parent
 
 	def send_auto_reply(self, communication, email):
 		"""Send auto reply if set."""
@@ -373,7 +408,6 @@ class EmailAccount(Document):
 					 frappe.get_template("templates/emails/auto_reply.html").render(communication.as_dict()),
 				reference_doctype = communication.reference_doctype,
 				reference_name = communication.reference_name,
-				message_id = communication.name,
 				in_reply_to = email.mail.get("Message-Id"), # send back the Message-Id as In-Reply-To
 				unsubscribe_message = unsubscribe_message)
 

frappe/email/email_body.py

@@ -6,7 +6,7 @@ import frappe
 from frappe.utils.pdf import get_pdf
 from frappe.email.smtp import get_outgoing_email_account
 from frappe.utils import (get_url, scrub_urls, strip, expand_relative_urls, cint,
-	split_emails, to_markdown, markdown, encode)
+	split_emails, to_markdown, markdown, encode, random_string)
 import email.utils
 
 def get_email(recipients, sender='', msg='', subject='[No Subject]',
@@ -182,8 +182,9 @@ class EMail:
 			sender_name, sender_email = email.utils.parseaddr(self.sender)
 			self.sender = email.utils.formataddr((sender_name or self.email_account.name, self.email_account.email_id))
 
-	def set_message_id(self, message_id):
-		self.msg_root["Message-Id"] = "<{0}@{1}>".format(message_id, frappe.local.site)
+	def set_message_id(self, doctype, name):
+		self.msg_root["Message-Id"] = "<{random}.{doctype}.{name}@{site}>".format(
+			doctype=doctype, name=name, site=frappe.local.site, random=random_string(10))
 
 	def set_in_reply_to(self, in_reply_to):
 		"""Used to send the Message-Id of a received email back as In-Reply-To"""

frappe/email/queue.py

@@ -18,7 +18,7 @@ class EmailLimitCrossedError(frappe.ValidationError): pass
 
 def send(recipients=None, sender=None, subject=None, message=None, reference_doctype=None,
 		reference_name=None, unsubscribe_method=None, unsubscribe_params=None, unsubscribe_message=None,
-		attachments=None, reply_to=None, cc=(), show_as_cc=(), message_id=None, in_reply_to=None, send_after=None,
+		attachments=None, reply_to=None, cc=(), show_as_cc=(), in_reply_to=None, send_after=None,
 		expose_recipients=False, send_priority=1, communication=None):
 	"""Add email to sending queue (Email Queue)
 
@@ -33,7 +33,6 @@ def send(recipients=None, sender=None, subject=None, message=None, reference_doc
 	:param unsubscribe_params: additional params for unsubscribed links. default are name, doctype, email
 	:param attachments: Attachments to be sent.
 	:param reply_to: Reply to be captured here (default inbox)
-	:param message_id: Used for threading. If a reply is received to this email, Message-Id is sent back as In-Reply-To in received email.
 	:param in_reply_to: Used to send the Message-Id of a received email back as In-Reply-To.
 	:param send_after: Send this email after the given datetime. If value is in integer, then `send_after` will be the automatically set to no of days from current date.
 	:param communication: Communication link to be set in Email Queue record
@@ -103,11 +102,12 @@ def send(recipients=None, sender=None, subject=None, message=None, reference_doc
 			email_text_context = cc_message + "\n" + email_text_context
 		# add to queue
 		add(email, sender, subject, email_content, email_text_context, reference_doctype,
-			reference_name, attachments, reply_to, cc, message_id, in_reply_to, send_after, send_priority, email_account=email_account, communication=communication)
+			reference_name, attachments, reply_to, cc, in_reply_to, send_after, send_priority, email_account=email_account, communication=communication)
 
 def add(email, sender, subject, formatted, text_content=None,
 	reference_doctype=None, reference_name=None, attachments=None, reply_to=None,
-	cc=(), message_id=None, in_reply_to=None, send_after=None, send_priority=1, email_account=None, communication=None):
+	cc=(), in_reply_to=None, send_after=None, send_priority=1, email_account=None,
+	communication=None):
 	"""Add to Email Queue"""
 	e = frappe.new_doc('Email Queue')
 	e.recipient = email
@@ -115,10 +115,11 @@ def add(email, sender, subject, formatted, text_content=None,
 
 	try:
 		mail = get_email(email, sender=sender, formatted=formatted, subject=subject,
-			text_content=text_content, attachments=attachments, reply_to=reply_to, cc=cc, email_account=email_account)
+			text_content=text_content, attachments=attachments, reply_to=reply_to,
+			cc=cc, email_account=email_account)
 
-		if message_id:
-			mail.set_message_id(message_id)
+		if reference_doctype and reference_name:
+			mail.set_message_id(reference_doctype, reference_name)
 
 		if in_reply_to:
 			mail.set_in_reply_to(in_reply_to)

frappe/hooks.py

@@ -121,8 +121,7 @@ scheduler_events = {
 	"hourly": [
 		"frappe.model.utils.link_count.update_link_count",
 		'frappe.model.utils.list_settings.sync_list_settings',
-		"frappe.utils.error.collect_error_snapshots",
-		"frappe.integration_broker.doctype.integration_service.integration_service.trigger_integration_service_events"
+		"frappe.utils.error.collect_error_snapshots"
 	],
 	"daily": [
 		"frappe.email.queue.clear_outbox",