Merge pull request #15820 from kamaljohnson/fix-imap-folder-append-to-not-working-issue

frappe/email/doctype/email_account/email_account.py

@@ -421,10 +421,10 @@ class EmailAccount(Document):
 	def get_failed_attempts_count(self):
 		return cint(frappe.cache().get('{0}:email-account-failed-attempts'.format(self.name)))
 
-	def receive(self, test_mails=None):
+	def receive(self):
 		"""Called by scheduler to receive emails from this EMail account using POP3/IMAP."""
 		exceptions = []
-		inbound_mails = self.get_inbound_mails(test_mails=test_mails)
+		inbound_mails = self.get_inbound_mails()
 		for mail in inbound_mails:
 			try:
 				communication = mail.process()
@@ -457,20 +457,19 @@ class EmailAccount(Document):
 		if exceptions:
 			raise Exception(frappe.as_json(exceptions))
 
-	def get_inbound_mails(self, test_mails=None) -> List[InboundMail]:
+	def get_inbound_mails(self) -> List[InboundMail]:
 		"""retrive and return inbound mails.
 
 		"""
 		mails = []
 
-		def process_mail(messages):
+		def process_mail(messages, append_to=None):
 			for index, message in enumerate(messages.get("latest_messages", [])):
 				uid = messages['uid_list'][index] if messages.get('uid_list') else None
-				seen_status = 1 if messages.get('seen_status', {}).get(uid) == 'SEEN' else 0
-				mails.append(InboundMail(message, self, uid, seen_status))
-
-		if frappe.local.flags.in_test:
-			return [InboundMail(msg, self) for msg in test_mails or []]
+				seen_status = messages.get('seen_status', {}).get(uid)
+				if self.email_sync_option != 'UNSEEN' or seen_status != "SEEN":
+					# only append the emails with status != 'SEEN' if sync option is set to 'UNSEEN'
+					mails.append(InboundMail(message, self, uid, seen_status, append_to))
 
 		if not self.enable_incoming:
 			return []
@@ -481,10 +480,10 @@ class EmailAccount(Document):
 			if self.use_imap:
 				# process all given imap folder
 				for folder in self.imap_folder:
-					email_server.select_imap_folder(folder.folder_name)
-					email_server.settings['uid_validity'] = folder.uidvalidity
-					messages = email_server.get_messages(folder=folder.folder_name) or {}
-					process_mail(messages)
+					if email_server.select_imap_folder(folder.folder_name):
+						email_server.settings['uid_validity'] = folder.uidvalidity
+						messages = email_server.get_messages(folder=f'"{folder.folder_name}"') or {}
+						process_mail(messages, folder.append_to)
 			else:
 				# process the pop3 account
 				messages = email_server.get_messages() or {}
@@ -494,7 +493,6 @@ class EmailAccount(Document):
 		except Exception:
 			frappe.log_error(title=_("Error while connecting to email account {0}").format(self.name))
 			return []
-
 		return mails
 
 	def handle_bad_emails(self, uid, raw, reason):

frappe/email/doctype/email_account/test_email_account.py

@@ -14,11 +14,11 @@ from frappe.core.doctype.communication.email import make
 from frappe.desk.form.load import get_attachments
 from frappe.email.doctype.email_account.email_account import notify_unreplied
 
+from unittest.mock import patch
+
 make_test_records("User")
 make_test_records("Email Account")
 
-
-
 class TestEmailAccount(unittest.TestCase):
 	@classmethod
 	def setUpClass(cls):
@@ -45,10 +45,21 @@ class TestEmailAccount(unittest.TestCase):
 	def test_incoming(self):
 		cleanup("test_sender@example.com")
 
-		test_mails = [self.get_test_mail('incoming-1.raw')]
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					self.get_test_mail('incoming-1.raw')
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
 
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"})
 		self.assertTrue("test_receiver@example.com" in comm.recipients)
@@ -72,11 +83,21 @@ class TestEmailAccount(unittest.TestCase):
 		existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'})
 		frappe.delete_doc("File", existing_file.name)
 
-		with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-2.raw"), "r") as testfile:
-			test_mails = [testfile.read()]
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					self.get_test_mail('incoming-2.raw')
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
 
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"})
 		self.assertTrue("test_receiver@example.com" in comm.recipients)
@@ -93,11 +114,21 @@ class TestEmailAccount(unittest.TestCase):
 	def test_incoming_attached_email_from_outlook_plain_text_only(self):
 		cleanup("test_sender@example.com")
 
-		with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-3.raw"), "r") as f:
-			test_mails = [f.read()]
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					self.get_test_mail('incoming-3.raw')
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
 
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"})
 		self.assertTrue("From: "Microsoft Outlook" <test_sender@example.com>" in comm.content)
@@ -106,11 +137,21 @@ class TestEmailAccount(unittest.TestCase):
 	def test_incoming_attached_email_from_outlook_layers(self):
 		cleanup("test_sender@example.com")
 
-		with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-4.raw"), "r") as f:
-			test_mails = [f.read()]
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					self.get_test_mail('incoming-4.raw')
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
 
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"})
 		self.assertTrue("From: "Microsoft Outlook" <test_sender@example.com>" in comm.content)
@@ -151,11 +192,23 @@ class TestEmailAccount(unittest.TestCase):
 		with open(os.path.join(os.path.dirname(__file__), "test_mails", "reply-1.raw"), "r") as f:
 			raw = f.read()
 			raw = raw.replace("<-- in-reply-to -->", sent_mail.get("Message-Id"))
-			test_mails = [raw]
 
 		# parse reply
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					raw
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
+
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		sent = frappe.get_doc("Communication", sent_name)
 
@@ -173,8 +226,20 @@ class TestEmailAccount(unittest.TestCase):
 			test_mails.append(f.read())
 
 		# parse reply
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': test_mails,
+				'seen_status': {
+					2: 'UNSEEN',
+					3: 'UNSEEN'
+				},
+				'uid_list': [2, 3]
+			}
+		}
+
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm_list = frappe.get_all("Communication", filters={"sender":"test_sender@example.com"},
 			fields=["name", "reference_doctype", "reference_name"])
@@ -197,11 +262,22 @@ class TestEmailAccount(unittest.TestCase):
 
 		# 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)]
+			messages = {
+				# append_to = ToDo
+				'"INBOX"': {
+					'latest_messages': [
+						f.read().replace('{{ message_id }}', last_mail.message_id)
+					],
+					'seen_status': {
+						2: 'UNSEEN'
+					},
+					'uid_list': [2]
+				}
+			}
 
 		# pull the mail
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm_list = frappe.get_all("Communication", filters={"sender":"test_sender@example.com"},
 			fields=["name", "reference_doctype", "reference_name"])
@@ -213,10 +289,21 @@ class TestEmailAccount(unittest.TestCase):
 	def test_auto_reply(self):
 		cleanup("test_sender@example.com")
 
-		test_mails = [self.get_test_mail('incoming-1.raw')]
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					self.get_test_mail('incoming-1.raw')
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
 
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
-		email_account.receive(test_mails=test_mails)
+		TestEmailAccount.mocked_email_receive(email_account, messages)
 
 		comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"})
 		self.assertTrue(frappe.db.get_value("Email Queue", {"reference_doctype": comm.reference_doctype,
@@ -246,6 +333,91 @@ class TestEmailAccount(unittest.TestCase):
 		with self.assertRaises(Exception):
 			email_account.validate()
 
+	def test_append_to(self):
+		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
+		mail_content = self.get_test_mail(fname="incoming-2.raw")
+
+		inbound_mail = InboundMail(mail_content, email_account, 12345, 1, 'ToDo')
+		communication = inbound_mail.process()
+		# the append_to for the email is set to ToDO in "_Test Email Account 1"
+		self.assertEqual(communication.reference_doctype, 'ToDo')
+		self.assertTrue(communication.reference_name)
+		self.assertTrue(frappe.db.exists(communication.reference_doctype, communication.reference_name))
+
+	def test_append_to_with_imap_folders(self):
+		mail_content_1 = self.get_test_mail(fname="incoming-1.raw")
+		mail_content_2 = self.get_test_mail(fname="incoming-2.raw")
+		mail_content_3 = self.get_test_mail(fname="incoming-3.raw")
+
+		messages = {
+			# append_to = ToDo
+			'"INBOX"': {
+				'latest_messages': [
+					mail_content_1,
+					mail_content_2
+				],
+				'seen_status': {
+					0: 'UNSEEN',
+					1: 'UNSEEN'
+				},
+				'uid_list': [0,1]
+			},
+			# append_to = Communication
+			'"Test Folder"': {
+				'latest_messages': [
+					mail_content_3
+				],
+				'seen_status': {
+					2: 'UNSEEN'
+				},
+				'uid_list': [2]
+			}
+		}
+
+		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
+		mails = TestEmailAccount.mocked_get_inbound_mails(email_account, messages)
+		self.assertEqual(len(mails), 3)
+
+		inbox_mails = 0
+		test_folder_mails = 0
+
+		for mail in mails:
+			communication = mail.process()
+			if mail.append_to == 'ToDo':
+				inbox_mails += 1
+				self.assertEqual(communication.reference_doctype, 'ToDo')
+				self.assertTrue(communication.reference_name)
+				self.assertTrue(frappe.db.exists(communication.reference_doctype, communication.reference_name))
+			else:
+				test_folder_mails += 1
+				self.assertEqual(communication.reference_doctype, None)
+
+		self.assertEqual(inbox_mails, 2)
+		self.assertEqual(test_folder_mails, 1)
+
+	@patch("frappe.email.receive.EmailServer.select_imap_folder", return_value=True)
+	@patch("frappe.email.receive.EmailServer.logout", side_effect=lambda: None)
+	def mocked_get_inbound_mails(email_account, messages={}, mocked_logout=None, mocked_select_imap_folder=None):
+		from frappe.email.receive import EmailServer
+
+		def get_mocked_messages(**kwargs):
+			return messages.get(kwargs["folder"], {})
+
+		with patch.object(EmailServer, "get_messages", side_effect=get_mocked_messages):
+			mails = email_account.get_inbound_mails()
+
+		return mails
+
+	@patch("frappe.email.receive.EmailServer.select_imap_folder", return_value=True)
+	@patch("frappe.email.receive.EmailServer.logout", side_effect=lambda: None)
+	def mocked_email_receive(email_account, messages={}, mocked_logout=None, mocked_select_imap_folder=None):
+		def get_mocked_messages(**kwargs):
+			return messages.get(kwargs["folder"], {})
+
+		from frappe.email.receive import EmailServer
+		with patch.object(EmailServer, "get_messages", side_effect=get_mocked_messages):
+			email_account.receive()
+
 class TestInboundMail(unittest.TestCase):
 	@classmethod
 	def setUpClass(cls):
@@ -313,11 +485,11 @@ class TestInboundMail(unittest.TestCase):
 
 		email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
 		inbound_mail = InboundMail(mail_content, email_account, 12345, 1)
-		new_communiction = inbound_mail.process()
+		new_communication = inbound_mail.process()
 
 		# Make sure that uid is changed to new uid
-		self.assertEqual(new_communiction.uid, 12345)
-		self.assertEqual(communication.name, new_communiction.name)
+		self.assertEqual(new_communication.uid, 12345)
+		self.assertEqual(communication.name, new_communication.name)
 
 	def test_find_parent_email_queue(self):
 		"""If the mail is reply to the already sent mail, there will be a email queue record.

frappe/email/doctype/email_account/test_records.json

@@ -20,7 +20,7 @@
 		"pop3_server": "pop.test.example.com",
 		"no_remaining":"0",
 		"append_to": "ToDo",
-		"imap_folder": [{"folder_name": "INBOX", "append_to": "ToDo"}],
+		"imap_folder": [{"folder_name": "INBOX", "append_to": "ToDo"}, {"folder_name": "Test Folder", "append_to": "Communication"}],
 		"track_email_status": 1
 	},
 	{

frappe/email/doctype/email_queue/email_queue.py

@@ -479,21 +479,24 @@ class QueueBuilder:
 
 		EmailUnsubscribe = DocType("Email Unsubscribe")
 
-		unsubscribed = (
-			frappe.qb.from_(EmailUnsubscribe).select(
-				EmailUnsubscribe.email
-			).where(
-				EmailUnsubscribe.email.isin(all_ids)
-				& (
-					(
-						(EmailUnsubscribe.reference_doctype == self.reference_doctype)
-						& (EmailUnsubscribe.reference_name == self.reference_name)
-					) | (
-						EmailUnsubscribe.global_unsubscribe == 1
+		if len(all_ids) > 0:
+			unsubscribed = (
+				frappe.qb.from_(EmailUnsubscribe).select(
+					EmailUnsubscribe.email
+				).where(
+					EmailUnsubscribe.email.isin(all_ids)
+					& (
+						(
+							(EmailUnsubscribe.reference_doctype == self.reference_doctype)
+							& (EmailUnsubscribe.reference_name == self.reference_name)
+						) | (
+							EmailUnsubscribe.global_unsubscribe == 1
+						)
 					)
-				)
-			).distinct()
-		).run(pluck=True)
+				).distinct()
+			).run(pluck=True)
+		else:
+			unsubscribed = None
 
 		self._unsubscribed_user_emails = unsubscribed or []
 		return self._unsubscribed_user_emails

frappe/email/receive.py

@@ -108,7 +108,8 @@ class EmailServer:
 				raise
 
 	def select_imap_folder(self, folder):
-		self.imap.select(folder)
+		res = self.imap.select(f'"{folder}"')
+		return res[0] == 'OK'	# The folder exsits TODO: handle other resoponses too
 
 	def logout(self):
 		if cint(self.settings.use_imap):
@@ -582,10 +583,11 @@ class Email:
 class InboundMail(Email):
 	"""Class representation of incoming mail along with mail handlers.
 	"""
-	def __init__(self, content, email_account, uid=None, seen_status=None):
+	def __init__(self, content, email_account, uid=None, seen_status=None, append_to=None):
 		super().__init__(content)
 		self.email_account = email_account
 		self.uid = uid or -1
+		self.append_to = append_to
 		self.seen_status = seen_status or 0
 
 		# System documents related to this mail
@@ -623,15 +625,18 @@ class InboundMail(Email):
 		if self.parent_communication():
 			data['in_reply_to'] = self.parent_communication().name
 
+		append_to = self.append_to if self.email_account.use_imap else self.email_account.append_to
+
 		if self.reference_document():
 			data['reference_doctype'] = self.reference_document().doctype
 			data['reference_name'] = self.reference_document().name
-		elif self.email_account.append_to and self.email_account.append_to != 'Communication':
-			reference_doc = self._create_reference_document(self.email_account.append_to)
-			if reference_doc:
-				data['reference_doctype'] = reference_doc.doctype
-				data['reference_name'] = reference_doc.name
-				data['is_first'] = True
+		else: 
+			if append_to and append_to != 'Communication':
+				reference_doc = self._create_reference_document(append_to)
+				if reference_doc:
+					data['reference_doctype'] = reference_doc.doctype
+					data['reference_name'] = reference_doc.name
+			data['is_first'] = True
 
 		if self.is_notification():
 			# Disable notifications for notification.

frappe/tests/test_email.py

@@ -3,6 +3,8 @@
 
 import unittest, frappe, re, email
 
+from frappe.email.doctype.email_account.test_email_account import TestEmailAccount 
+
 test_dependencies = ['Email Account']
 
 class TestEmail(unittest.TestCase):
@@ -173,12 +175,35 @@ class TestEmail(unittest.TestCase):
 		frappe.db.delete("Communication", {"sender": "sukh@yyy.com"})
 
 		with open(frappe.get_app_path('frappe', 'tests', 'data', 'email_with_image.txt'), 'r') as raw:
-			mails = email_account.get_inbound_mails(test_mails=[raw.read()])
+			messages = {
+				# append_to = ToDo
+				'"INBOX"': {
+					'latest_messages': [
+						raw.read()
+					],
+					'seen_status': {
+						2: 'UNSEEN'
+					},
+					'uid_list': [2]
+				}	
+			}
+
+			email_account = frappe.get_doc("Email Account", "_Test Email Account 1")
+			changed_flag = False
+			if not email_account.enable_incoming: 
+				email_account.enable_incoming = True
+				changed_flag = True
+			mails = TestEmailAccount.mocked_get_inbound_mails(email_account, messages)
+
+			# mails = email_account.get_inbound_mails(test_mails=[raw.read()])
 			communication = mails[0].process()
 
 		self.assertTrue(re.search('''<img[^>]*src=["']/private/files/rtco1.png[^>]*>''', communication.content))
 		self.assertTrue(re.search('''<img[^>]*src=["']/private/files/rtco2.png[^>]*>''', communication.content))
 
+		if changed_flag:
+			email_account.enable_incoming = False
+
 
 if __name__ == '__main__':
 	frappe.connect()