[email] threading based on message_id in Email Queue and test case

frappe/__init__.py

@@ -388,22 +388,17 @@ 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:
-		import email.queue
-		email.queue.send(recipients=recipients, sender=sender,
-			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, 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, in_reply_to=in_reply_to, retry=retry)
+	if not delayed:
+		now = True
+
+	import email.queue
+	email.queue.send(recipients=recipients, sender=sender,
+		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, in_reply_to=in_reply_to,
+		send_after=send_after, expose_recipients=expose_recipients, send_priority=send_priority,
+		communication=communication, now=now)
 
 whitelisted = []
 guest_methods = []

frappe/desk/doctype/event/event.py

@@ -5,7 +5,7 @@ from __future__ import unicode_literals
 import frappe
 
 from frappe.utils import (getdate, cint, add_months, date_diff, add_days,
-	nowdate, get_datetime_str, cstr, get_datetime)
+	nowdate, get_datetime_str, cstr, get_datetime, now_datetime)
 from frappe.model.document import Document
 from frappe.utils.user import get_enabled_system_users
 
@@ -13,6 +13,9 @@ weekdays = ["monday", "tuesday", "wednesday", "thursday", "friday", "saturday",
 
 class Event(Document):
 	def validate(self):
+		if not self.starts_on:
+			self.starts_on = now_datetime()
+
 		if self.starts_on and self.ends_on and get_datetime(self.starts_on) > get_datetime(self.ends_on):
 			frappe.msgprint(frappe._("Event end must be after start"), raise_exception=True)
 

frappe/email/doctype/email_account/email_account.py

@@ -365,20 +365,17 @@ class EmailAccount(Document):
 	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}`'''
+		Message-ID is formatted as `{message_id}@{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)
-
-				# parent doctype has '-' instead of ' ' and '--' instead of '-'
-				parent_doctype = parent_doctype.replace('--', '%').replace('-', ' ').replace('%', '-')
+			email_queue = frappe.db.get_value('Email Queue', dict(message_id=in_reply_to), ['reference_doctype', 'reference_name'])
+			if email_queue:
+				parent_doctype, parent_name = email_queue
 			else:
+				reference, domain = in_reply_to.split("@", 1)
 				parent_doctype, parent_name = 'Communication', reference
 
 			if frappe.db.exists(parent_doctype, parent_name):

frappe/email/doctype/email_account/test_email_account.py

@@ -170,5 +170,30 @@ class TestEmailAccount(unittest.TestCase):
 		self.assertEquals(comm_list[0].reference_doctype, comm_list[1].reference_doctype)
 		self.assertEquals(comm_list[0].reference_name, comm_list[1].reference_name)
 
+	def test_threading_by_message_id(self):
+		frappe.db.sql("""delete from tabCommunication""")
+		frappe.db.sql("""delete from `tabEmail Queue`""")
 
+		# reference document for testing
+		event = frappe.get_doc(dict(doctype='Event', subject='test-message')).insert()
 
+		# send a mail against this
+		frappe.sendmail(recipients='test@example.com', subject='test message for threading',
+			message='testing', reference_doctype=event.doctype, reference_name=event.name)
+
+		last_mail = frappe.get_doc('Email Queue', dict(reference_name=event.name))
+
+		# get test mail with message-id as in-reply-to
+		with open(os.path.join(os.path.dirname(__file__), "test_mails", "reply-4.raw"), "r") as f:
+			test_mails = [f.read().replace('{{ message_id }}', last_mail.message_id)]
+
+		# pull the mail
+		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
+		email_account.receive(test_mails=test_mails)
+
+		comm_list = frappe.get_all("Communication", filters={"sender":"test_sender@example.com"},
+			fields=["name", "reference_doctype", "reference_name"])
+
+		# check if threaded correctly
+		self.assertEquals(comm_list[0].reference_doctype, event.doctype)
+		self.assertEquals(comm_list[0].reference_name, event.name)

frappe/email/doctype/email_account/test_mails/reply-4.raw

@@ -0,0 +1,75 @@
+From: <test_sender@example.com>
+Content-Type: multipart/alternative;
+	boundary="Apple-Mail=_29597CF7-20DD-4184-B3FA-85582C5C4361"
+Message-Id: <07D687F6-10AA-4B9F-82DE-27753096164E@gmail.com>
+Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
+X-Smtp-Server: 73CC8281-7E8F-4B47-8324-D5DA86EEDD4F
+Subject: Re: What did you work on today?
+Date: Thu, 10 Nov 2016 16:04:43 +0530
+X-Universally-Unique-Identifier: A4D9669F-179C-42D8-A3D3-AA6A8C49A6F2
+References: {{ message_id }}
+To: test_in@iwebnotes.com
+In-Reply-To: {{ message_id }}
+
+
+--Apple-Mail=_29597CF7-20DD-4184-B3FA-85582C5C4361
+Content-Transfer-Encoding: quoted-printable
+Content-Type: text/plain;
+	charset=us-ascii
+
+Testing another reply!
+
+> On 10-Nov-2016, at 3:20 PM, Frappe <test@erpnext.com> wrote:
+>=20
+> Please share what did you do today. If you reply by midnight, your =
+response will be recorded!
+>=20
+> This email was sent to rmehta@gmail.com
+> Unsubscribe from this list =
+<http://demo-test.erpnext.com.dev/api/method/frappe.email.queue.unsubscrib=
+e?email=3Drmehta%40gmail.com&name=3D26cc3e5a5d&doctype=3DDaily+Work+Summar=
+y&_signature=3D2c7ab37e6d775e5a481e9b4376154a41>
+> Sent via ERPNext <https://erpnext.com/?source=3Dvia_email_footer>
+
+
+--Apple-Mail=_29597CF7-20DD-4184-B3FA-85582C5C4361
+Content-Transfer-Encoding: 7bit
+Content-Type: text/html;
+	charset=us-ascii
+
+<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Testing another reply!<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 10-Nov-2016, at 3:20 PM, Frappe &lt;<a href="mailto:test@erpnext.com" class="">test@erpnext.com</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class="">
+
+
+<meta name="viewport" content="width=device-width" class="">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
+<title class="">What did you work on today?</title>
+
+<div style="line-height: 1.5; color: #36414C;" class="">
+<!-- body -->
+<div style="font-family: -apple-system, BlinkMacSystemFont,
+	    " segoe="" ui",="" "roboto",="" "oxygen",="" "ubuntu",="" "cantarell",="" "fira="" sans",="" "droid="" "helvetica="" neue",="" sans-serif;="" font-size:="" 14px;="" padding:="" 10px;"="" class=""><p class="">Please share what did you do today. If you reply by midnight, your response will be recorded!</p>
+
+</div>
+
+<!-- footer -->
+<div style="margin-top: 30px; font-family: Helvetica, Arial, sans-serif; font-size: 11px;
+    margin-bottom: 15px; border-top: 1px solid #d1d8dd;" data-email-footer="true" class="">
+	<div style="margin: 15px auto; padding: 0px 7px; text-align: center; color: #8d99a6;" class="">
+			This email was sent to <a href="mailto:rmehta@gmail.com" class="">rmehta@gmail.com</a>
+			<p style="margin: 15px auto;" class="">
+				<a href="http://demo-test.erpnext.com.dev/api/method/frappe.email.queue.unsubscribe?email=rmehta%40gmail.com&amp;name=26cc3e5a5d&amp;doctype=Daily+Work+Summary&amp;_signature=2c7ab37e6d775e5a481e9b4376154a41" style="color: #8d99a6; text-decoration: underline;
+					target=" _blank"="" class="">Unsubscribe from this list
+				</a>
+			</p>
+		</div><div style="margin: 15px auto;" class=""><div style="text-align: center;" class="">
+	<a href="https://erpnext.com/?source=via_email_footer" target="_blank" style="color: #8d99a6;" class="">
+		Sent via ERPNext
+	</a>
+</div></div>
+</div>
+<!-- /footer -->
+
+<div class="print-html"></div>
+</div>
+</div></blockquote></div><br class=""></div></body></html>
+--Apple-Mail=_29597CF7-20DD-4184-B3FA-85582C5C4361--

frappe/email/doctype/email_queue/email_queue.json

@@ -24,6 +24,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Sender", 
@@ -52,6 +53,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 1, 
    "in_standard_filter": 0, 
    "label": "Recipient", 
@@ -80,6 +82,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Message", 
@@ -108,6 +111,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 1, 
    "in_standard_filter": 1, 
    "label": "Status", 
@@ -136,6 +140,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Error", 
@@ -152,6 +157,35 @@
    "set_only_once": 0, 
    "unique": 0
   }, 
+  {
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "fieldname": "message_id", 
+   "fieldtype": "Data", 
+   "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": "Message ID", 
+   "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": 1, 
+   "set_only_once": 0, 
+   "unique": 0
+  }, 
   {
    "allow_on_submit": 0, 
    "bold": 0, 
@@ -163,6 +197,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Reference DocType", 
@@ -191,6 +226,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Reference DocName", 
@@ -218,6 +254,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Communication", 
@@ -247,6 +284,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Send After", 
@@ -276,6 +314,7 @@
    "ignore_user_permissions": 0, 
    "ignore_xss_filter": 0, 
    "in_filter": 0, 
+   "in_global_search": 0, 
    "in_list_view": 0, 
    "in_standard_filter": 0, 
    "label": "Priority", 
@@ -305,7 +344,7 @@
  "issingle": 0, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2016-11-07 05:29:46.316408", 
+ "modified": "2016-11-17 16:45:02.296617", 
  "modified_by": "Administrator", 
  "module": "Email", 
  "name": "Email Queue", 

frappe/email/email_body.py

@@ -59,6 +59,7 @@ class EMail:
 		self.html_set = False
 
 		self.email_account = email_account or get_outgoing_email_account()
+		self.set_message_id()
 
 	def set_html(self, message, text_content = None, footer=None, print_html=None, formatted=None):
 		"""Attach message in the html portion of multipart/alternative"""
@@ -182,10 +183,8 @@ 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, doctype, name):
-		message_id = "<{random}.{doctype}.{name}@{site}>".format(
-			doctype=doctype.replace('-', '--').replace(' ', '-'), name=name, site=frappe.local.site, random=random_string(10))
-		self.msg_root["Message-Id"] = message_id
+	def set_message_id(self):
+		self.msg_root["Message-Id"] = get_message_id()
 
 	def set_in_reply_to(self, in_reply_to):
 		"""Used to send the Message-Id of a received email back as In-Reply-To"""
@@ -241,6 +240,12 @@ def get_formatted_html(subject, message, footer=None, print_html=None, email_acc
 
 	return scrub_urls(rendered_email)
 
+def get_message_id():
+	'''Returns Message ID created from doctype and name'''
+	return "<{unique}@{site}>".format(
+			site=frappe.local.site,
+			unique=email.utils.make_msgid(random_string(10)).split('@')[0].split('<')[1])
+
 def get_signature(email_account):
 	if email_account and email_account.add_signature and email_account.signature:
 		return "<br><br>" + email_account.signature

frappe/email/queue.py

@@ -19,7 +19,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=(), in_reply_to=None, send_after=None,
-		expose_recipients=False, send_priority=1, communication=None):
+		expose_recipients=False, send_priority=1, communication=None, now=False):
 	"""Add email to sending queue (Email Queue)
 
 	:param recipients: List of recipients.
@@ -36,6 +36,7 @@ def send(recipients=None, sender=None, subject=None, message=None, reference_doc
 	: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
+	:param now: Send immediately (don't send in the background)
 	"""
 	if not unsubscribe_method:
 		unsubscribe_method = "/api/method/frappe.email.queue.unsubscribe"
@@ -101,9 +102,13 @@ def send(recipients=None, sender=None, subject=None, message=None, reference_doc
 			email_content = email_content.replace("<!-- cc message -->", cc_message)
 			email_text_context = cc_message + "\n" + email_text_context
 		# add to queue
-		add(email, sender, subject, email_content, email_text_context, reference_doctype,
+		email_queue = add(email, sender, subject, email_content, email_text_context, reference_doctype,
 			reference_name, attachments, reply_to, cc, in_reply_to, send_after, send_priority, email_account=email_account, communication=communication)
 
+		if now:
+			send_one(email_queue.name, now=True)
+
+
 def add(email, sender, subject, formatted, text_content=None,
 	reference_doctype=None, reference_name=None, attachments=None, reply_to=None,
 	cc=(), in_reply_to=None, send_after=None, send_priority=1, email_account=None,
@@ -118,12 +123,10 @@ def add(email, sender, subject, formatted, text_content=None,
 			text_content=text_content, attachments=attachments, reply_to=reply_to,
 			cc=cc, email_account=email_account)
 
-		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)
 
+		e.message_id = mail.msg_root["Message-Id"].strip(" <>")
 		e.message = cstr(mail.as_string())
 		e.sender = mail.sender
 
@@ -137,6 +140,8 @@ def add(email, sender, subject, formatted, text_content=None,
 	e.send_after = send_after
 	e.db_insert()
 
+	return e
+
 def check_email_limit(recipients):
 	# if using settings from site_config.json, check email limit
 	# No limit for own email settings
@@ -347,8 +352,10 @@ def send_one(email, smtpserver=None, auto_commit=True, now=False):
 			log('frappe.email.queue.flush', unicode(e))
 
 def clear_outbox():
-	"""Remove mails older than 31 days in Outbox. Called daily via scheduler."""
-	frappe.db.sql("""delete from `tabEmail Queue` where
+	"""Remove low priority older than 31 days in Outbox and expire mails not sent for 7 days.
+
+	Called daily via scheduler."""
+	frappe.db.sql("""delete from `tabEmail Queue` where priority=0
 		datediff(now(), modified) > 31""")
 
 	frappe.db.sql("""update `tabEmail Queue` set status='Expired'