[Fix] Email and Contact fixes (#5519)

* use multiselect for email dialog, improv email fetch query

* patch to create contact for all user

* append number if same name found for company

* update tests

frappe/contacts/doctype/contact/contact.py

@@ -9,6 +9,7 @@ from frappe.model.document import Document
 from frappe.core.doctype.dynamic_link.dynamic_link import deduplicate_dynamic_links
 from six import iteritems
 from past.builtins import cmp
+from frappe.model.naming import append_number_if_name_exists
 
 import functools
 
@@ -18,6 +19,9 @@ class Contact(Document):
 		self.name = " ".join(filter(None,
 			[cstr(self.get(f)).strip() for f in ["first_name", "last_name"]]))
 
+		if frappe.db.exists("Contact", self.name):
+			self.name = append_number_if_name_exists('Contact', self.name)
+
 		# concat party name if reqd
 		for link in self.links:
 			self.name = self.name + '-' + link.link_name.strip()

frappe/core/doctype/feedback_trigger/test_feedback_trigger.py

@@ -23,6 +23,7 @@ class TestFeedbackTrigger(unittest.TestCase):
 		new_user.add_roles("System Manager")
 
 	def tearDown(self):
+		frappe.db.sql("delete from tabContact where email_id='test-feedback@example.com'")
 		frappe.delete_doc("User", "test-feedback@example.com")
 		frappe.delete_doc("Feedback Trigger", "ToDo")
 		frappe.db.sql('delete from `tabEmail Queue`')

frappe/core/doctype/user/test_user.py

@@ -45,6 +45,7 @@ class TestUser(unittest.TestCase):
 		new_user.save()
 		self.assertEqual(new_user.user_type, 'Website User')
 
+		delete_contact(new_user.name)
 		frappe.delete_doc('User', new_user.name)
 
 
@@ -55,6 +56,7 @@ class TestUser(unittest.TestCase):
 		delete_doc("Role","_Test Role 2")
 
 		if frappe.db.exists("User", "_test@example.com"):
+			delete_contact("_test@example.com")
 			delete_doc("User", "_test@example.com")
 
 		user = frappe.copy_doc(test_records[1])
@@ -63,6 +65,7 @@ class TestUser(unittest.TestCase):
 
 		frappe.get_doc({"doctype": "ToDo", "description": "_Test"}).insert()
 
+		delete_contact("_test@example.com")
 		delete_doc("User", "_test@example.com")
 
 		self.assertTrue(not frappe.db.sql("""select * from `tabToDo` where owner=%s""",
@@ -127,6 +130,7 @@ class TestUser(unittest.TestCase):
 		self.assertRaises(MaxUsersReachedError, user.add_roles, 'System Manager')
 
 		if frappe.db.exists('User', 'test_max_users@example.com'):
+			delete_contact('test_max_users@example.com')
 			frappe.delete_doc('User', 'test_max_users@example.com')
 
 		# Clear the user limit
@@ -218,6 +222,7 @@ class TestUser(unittest.TestCase):
 		})
 		comm.insert(ignore_permissions=True)
 
+		delete_contact(new_user.name)
 		frappe.delete_doc('User', new_user.name)
 		self.assertFalse(frappe.db.exists('User', new_user.name))
 
@@ -235,6 +240,7 @@ class TestUser(unittest.TestCase):
 		self.assertEqual(frappe.db.get_value("User", "test_deactivate_additional_users@example.com", "enabled"), 0)
 
 		if frappe.db.exists("User", "test_deactivate_additional_users@example.com"):
+			delete_contact('test_deactivate_additional_users@example.com')
 			frappe.delete_doc('User', 'test_deactivate_additional_users@example.com')
 
 		# Clear the user limit
@@ -259,3 +265,6 @@ class TestUser(unittest.TestCase):
 		# Score 4; should pass
 		result = test_password_strength("Eastern_43A1W")
 		self.assertEqual(result['feedback']['password_policy_validation_passed'], True)
+
+def delete_contact(user):
+	frappe.db.sql("delete from tabContact where email_id='%s'" % frappe.db.escape(user))

frappe/core/doctype/user/user.py

@@ -93,6 +93,7 @@ class User(Document):
 		clear_notifications(user=self.name)
 		frappe.clear_cache(user=self.name)
 		self.send_password_notification(self.__new_password)
+		create_contact(self)
 		if self.name not in ('Administrator', 'Guest') and not self.user_image:
 			frappe.enqueue('frappe.core.doctype.user.user.update_gravatar', name=self.name)
 
@@ -1033,3 +1034,18 @@ def update_roles(role_profile):
 		user = frappe.get_doc('User', d)
 		user.set('roles', [])
 		user.add_roles(*roles)
+
+def create_contact(user):
+	if user.name in ["Administrator", "Guest"]: return
+
+	if not frappe.db.get_value("Contact", {"email_id": user.email}):
+		frappe.get_doc({
+			"doctype": "Contact",
+			"first_name": user.first_name,
+			"last_name": user.last_name,
+			"email_id": user.email,
+			"user": user.name,
+			"gender": user.gender,
+			"phone": user.phone,
+			"mobile_no": user.mobile_no
+		}).insert(ignore_permissions=True)

frappe/email/__init__.py

@@ -3,35 +3,26 @@
 
 from __future__ import unicode_literals
 import frappe
+from frappe.desk.reportview import build_match_conditions
 
 def sendmail_to_system_managers(subject, content):
 	frappe.sendmail(recipients=get_system_managers(), subject=subject, content=content)
 
 @frappe.whitelist()
-def get_contact_list(txt):
+def get_contact_list():
 	"""Returns contacts (from autosuggest)"""
-	txt = txt.replace('%', '')
 
-	def get_users():
-		return filter(None, frappe.db.sql_list('select email from tabUser where email like %s',
-			('%' + txt + '%')))
 	try:
-		out = filter(None, frappe.db.sql_list("""select distinct email_id from `tabContact` 
-			where email_id like %(txt)s or concat(first_name, " ", last_name) like %(txt)s order by
-			if (locate( %(_txt)s, concat(first_name, " ", last_name)), locate( %(_txt)s, concat(first_name, " ", last_name)), 99999),
-			if (locate( %(_txt)s, email_id), locate( %(_txt)s, email_id), 99999)""",
-		        {'txt': "%%%s%%" % frappe.db.escape(txt),
-	            '_txt': txt.replace("%", "")
-		        })
-		)
-		if not out:
-			out = get_users()
-	except Exception as e:
-		if e.args[0]==1146:
-			# no Contact, use User
-			out = get_users()
-		else:
-			raise
+		match_conditions = build_match_conditions('Contact')
+		out = frappe.db.sql("""select email_id as value,
+			concat(first_name, ifnull(concat(' ',last_name), '' )) as description
+			from tabContact
+			where {0}
+		""".format(match_conditions), as_dict=True)
+		out = filter(None, out)
+
+	except:
+		raise
 
 	return out
 

frappe/patches.txt

@@ -212,3 +212,4 @@ frappe.patches.v11_0.rename_standard_reply_to_email_template
 execute:frappe.delete_doc_if_exists('Page', 'user-permissions')
 frappe.patches.v10_0.set_no_copy_to_workflow_state
 frappe.patches.v10_0.increase_single_table_column_length
+frappe.patches.v11_0.create_contact_for_user

frappe/patches/v11_0/create_contact_for_user.py

@@ -0,0 +1,10 @@
+from __future__ import unicode_literals
+import frappe
+from frappe.core.doctype.user.user import create_contact
+
+def execute():
+	""" Create Contact for each User if not present """
+
+	users = frappe.get_all('User', filters={"name": ('not in', 'Administrator, Guest')}, fields=["*"])
+	for user in users:
+		create_contact(user)

frappe/public/js/frappe/views/communication.js

@@ -46,11 +46,19 @@ frappe.views.CommunicationComposer = Class.extend({
 	},
 
 	get_fields: function() {
+		let contactList = [];
+		frappe.call({
+			method: "frappe.email.get_contact_list",
+			async: false,
+			callback: function(r) {
+				contactList = r.message;
+			}
+		});
 		var fields= [
-			{label:__("To"), fieldtype:"Data", reqd: 0, fieldname:"recipients",length:524288},
+			{label:__("To"), fieldtype:"MultiSelect", reqd: 0, fieldname:"recipients",options:contactList},
 			{fieldtype: "Section Break", collapsible: 1, label: __("CC, BCC & Email Template")},
-			{label:__("CC"), fieldtype:"Data", fieldname:"cc", length:524288},
-			{label:__("BCC"), fieldtype:"Data", fieldname:"bcc", length:524288},
+			{label:__("CC"), fieldtype:"MultiSelect", fieldname:"cc",options:contactList},
+			{label:__("BCC"), fieldtype:"MultiSelect", fieldname:"bcc",options:contactList},
 			{label:__("Email Template"), fieldtype:"Link", options:"Email Template",
 				fieldname:"email_template"},
 			{fieldtype: "Section Break"},
@@ -104,7 +112,6 @@ frappe.views.CommunicationComposer = Class.extend({
 		this.setup_print();
 		this.setup_attach();
 		this.setup_email();
-		this.setup_awesomplete();
 		this.setup_last_edited_communication();
 		this.setup_email_template();
 
@@ -608,56 +615,5 @@ frappe.views.CommunicationComposer = Class.extend({
 			content = "<div><br></div>" + reply;
 		}
 		fields.content.set_value(content);
-	},
-	setup_awesomplete: function() {
-		var me = this;
-		[
-			this.dialog.fields_dict.recipients.input,
-			this.dialog.fields_dict.cc.input,
-			this.dialog.fields_dict.bcc.input
-		].map(function(input) {
-			me.setup_awesomplete_for_input(input);
-		});
-	},
-	setup_awesomplete_for_input: function(input) {
-		function split(val) {
-			return val.split( /,\s*/ );
-		}
-		function extractLast(term) {
-			return split(term).pop();
-		}
-
-		var awesomplete = new Awesomplete(input, {
-			minChars: 0,
-			maxItems: 99,
-			autoFirst: true,
-			list: [],
-			item: function(item, input) {
-				return $('<li>').text(item.value).get(0);
-			},
-			filter: function(text, input) { return true },
-			replace: function(text) {
-				var before = this.input.value.match(/^.+,\s*|/)[0];
-				this.input.value = before + text + ", ";
-			}
-		});
-		var delay_timer;
-		var $input = $(input);
-		$input.on("input", function(e) {
-			clearTimeout(delay_timer);
-			delay_timer = setTimeout(function() {
-				var term = e.target.value;
-				frappe.call({
-					method:'frappe.email.get_contact_list',
-					args: {
-						'txt': extractLast(term) || '%'
-					},
-					quiet: true,
-					callback: function(r) {
-						awesomplete.list = r.message || [];
-					}
-				});
-			},250);
-		});
 	}
 });