From 6dbf7a90a19e6ffcf9bfb2c42263af9a135a4ec7 Mon Sep 17 00:00:00 2001
From: Rushabh Mehta <rmehta@gmail.com>
Date: Wed, 24 Jun 2020 16:59:53 +0530
Subject: [PATCH] fix: threading of email replies where message-id is not
 returned

---
 .../doctype/email_account/email_account.py    | 37 ++++++++++++-------
 .../public/js/frappe/views/communication.js   | 23 +++++++-----
 frappe/utils/safe_exec.py                     |  1 +
 3 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py
index 2065f5558a..f45a99f61f 100755
--- a/frappe/email/doctype/email_account/email_account.py
+++ b/frappe/email/doctype/email_account/email_account.py
@@ -478,27 +478,38 @@ class EmailAccount(Document):
 
 		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 = frappe.as_unicode(strip(re.sub(r"(^\s*(fw|fwd|wg)[^:]*:|\s*(re|aw)[^:]*:\s*)*",
-					"", email.subject, 0, flags=re.IGNORECASE)))
+				if '#' in email.subject:
+					# try and match if ID is found
+					# document ID is appended to subject
+					# example "Re: Your email (#OPP-2020-2334343)"
+					parent_id = email.subject.rsplit('#', 1)[-1].strip(' ()')
+					if frappe.db.exists(self.append_to, parent_id):
+						parent = frappe._dict(name=parent_id)
 
-				parent = frappe.db.get_all(self.append_to, filters={
-					self.sender_field: email.from_email,
-					self.subject_field: ("like", "%{0}%".format(subject)),
-					"creation": (">", (get_datetime() - relativedelta(days=60)).strftime(DATE_FORMAT))
-				}, fields="name")
+				if not parent:
+					# try and match by subject and sender
+					# if sent by same sender with same subject,
+					# append it to old coversation
+					subject = frappe.as_unicode(strip(re.sub(r"(^\s*(fw|fwd|wg)[^:]*:|\s*(re|aw)[^:]*:\s*)*",
+						"", email.subject, 0, flags=re.IGNORECASE)))
+
+					parent = frappe.db.get_all(self.append_to, filters={
+						self.sender_field: email.from_email,
+						self.subject_field: ("like", "%{0}%".format(subject)),
+						"creation": (">", (get_datetime() - relativedelta(days=60)).strftime(DATE_FORMAT))
+					}, fields="name")
 
-				# match only subject field
-				# when the from_email is of a user in the system
-				# and subject is atleast 10 chars long
 				if not parent and len(subject) > 10 and is_system_user(email.from_email):
+					# match only subject field
+					# when the from_email is of a user in the system
+					# and subject is atleast 10 chars long
 					parent = frappe.db.get_all(self.append_to, filters={
 						self.subject_field: ("like", "%{0}%".format(subject)),
 						"creation": (">", (get_datetime() - relativedelta(days=60)).strftime(DATE_FORMAT))
 					}, fields="name")
 
+
+
 			if parent:
 				parent = frappe._dict(doctype=self.append_to, name=parent[0].name)
 				return parent
diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js
index 8dad5d9121..b21d46d686 100755
--- a/frappe/public/js/frappe/views/communication.js
+++ b/frappe/public/js/frappe/views/communication.js
@@ -174,16 +174,21 @@ frappe.views.CommunicationComposer = Class.extend({
 			}
 
 			if (!this.subject) {
-				if (this.frm.subject_field && this.frm.doc[this.frm.subject_field]) {
-					this.subject = __("Re: {0}", [this.frm.doc[this.frm.subject_field]]);
-				} else {
-					let title = this.frm.doc.name;
-					if(this.frm.meta.title_field && this.frm.doc[this.frm.meta.title_field]
-						&& this.frm.doc[this.frm.meta.title_field] != this.frm.doc.name) {
-						title = `${this.frm.doc[this.frm.meta.title_field]} (#${this.frm.doc.name})`;
-					}
-					this.subject = `${__(this.frm.doctype)}: ${title}`;
+				let title = this.frm.doc.name;
+				if (this.frm.meta.subject_field && this.frm.doc[this.frm.meta.subject_field]) {
+					title = this.frm.doc[this.frm.meta.subject_field];
+				} else if (this.frm.meta.title_field && this.frm.doc[this.frm.meta.title_field]) {
+					title = this.frm.doc[this.frm.meta.title_field];
 				}
+				this.subject = __("Re: {0}", [title]);
+			}
+
+			// always add an identifier to catch a reply
+			// some email clients (outlook) may not send the message id to identify
+			// the thread. So as a backup we use the name of the document as identifier
+			let identifier = `#{this.frm.doc.name}`;
+			if (!this.subject.includes(identifier)) {
+				this.subject = `{this.subject} ({identifier})`;
 			}
 		}
 
diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py
index daae75228e..826aed6082 100644
--- a/frappe/utils/safe_exec.py
+++ b/frappe/utils/safe_exec.py
@@ -69,6 +69,7 @@ def get_safe_globals():
 			get_url=frappe.utils.get_url,
 			render_template=frappe.render_template,
 			msgprint=frappe.msgprint,
+			throw=frappe.throw,
 
 			user=user,
 			get_fullname=frappe.utils.get_fullname,