fix: Show user fullname in mentions (#7592)

* fix: Show user fullname in mentions

* fix: Remove unused import

* fix: Update tests

frappe/core/doctype/user/test_user.py

@@ -268,21 +268,38 @@ class TestUser(unittest.TestCase):
 		self.assertEqual(result['feedback']['password_policy_validation_passed'], True)
 
 	def test_comment_mentions(self):
-		user_name = "@test.comment@example.com"
-		self.assertEqual(extract_mentions(user_name)[0], "test.comment@example.com")
-		user_name = "@test.comment@test-example.com"
-		self.assertEqual(extract_mentions(user_name)[0], "test.comment@test-example.com")
-		user_name = "Testing comment, @test-user please check."
-		self.assertEqual(extract_mentions(user_name)[0], "test-user")
-		user_name = "Testing comment, @test.user@example.com please check."
-		self.assertEqual(extract_mentions(user_name)[0], "test.user@example.com")
-		user_name = "<div>@test_user@example.com and @test.again@example1.com</div><div>This is a test.</div>"
-		self.assertEqual(extract_mentions(user_name)[0], "test_user@example.com")
-		self.assertEqual(extract_mentions(user_name)[1], "test.again@example1.com")
-		user_name = "<div>@user@example.com</a> Test @test-comment@xyz.com</div><div>Test for comment mentions @test@abc.com</div>"
-		self.assertEqual(extract_mentions(user_name)[0], "user@example.com")
-		self.assertEqual(extract_mentions(user_name)[1], "test-comment@xyz.com")
-		self.assertEqual(extract_mentions(user_name)[2], "test@abc.com")
+		comment = '''
+			<span class="mention" data-id="test.comment@example.com" data-value="Test" data-denotation-char="@">
+				<span><span class="ql-mention-denotation-char">@</span>Test</span>
+			</span>
+		'''
+		self.assertEqual(extract_mentions(comment)[0], "test.comment@example.com")
+
+		comment = '''
+			<div>
+				Testing comment,
+				<span class="mention" data-id="test.comment@example.com" data-value="Test" data-denotation-char="@">
+					<span><span class="ql-mention-denotation-char">@</span>Test</span>
+				</span>
+				please check
+			</div>
+		'''
+		self.assertEqual(extract_mentions(comment)[0], "test.comment@example.com")
+		comment = '''
+			<div>
+				Testing comment for
+				<span class="mention" data-id="test_user@example.com" data-value="Test" data-denotation-char="@">
+					<span><span class="ql-mention-denotation-char">@</span>Test</span>
+				</span>
+				and
+				<span class="mention" data-id="test.again@example1.com" data-value="Test" data-denotation-char="@">
+					<span><span class="ql-mention-denotation-char">@</span>Test</span>
+				</span>
+				please check
+			</div>
+		'''
+		self.assertEqual(extract_mentions(comment)[0], "test_user@example.com")
+		self.assertEqual(extract_mentions(comment)[1], "test.again@example1.com")
 
 def delete_contact(user):
 	frappe.db.sql("DELETE FROM `tabContact` WHERE `email_id`= %s", user)

frappe/core/doctype/user/user.py

@@ -9,6 +9,7 @@ from frappe import throw, msgprint, _
 from frappe.utils.password import update_password as _update_password
 from frappe.desk.notifications import clear_notifications
 from frappe.utils.user import get_system_managers
+from bs4 import BeautifulSoup
 import frappe.permissions
 import frappe.share
 import re
@@ -943,11 +944,13 @@ def notify_admin_access_to_system_manager(login_manager=None):
 		)
 
 def extract_mentions(txt):
-	"""Find all instances of @name in the string.
-	The mentions will be separated by non-word characters or may appear at the start of the string"""
-	txt = txt.replace("<div>", "<div> ")
-	txt = re.sub(r'(<[a-zA-Z\/][^>]*>)', '', txt)
-	return re.findall(r'(?:[^\w\.\-\@]|^)@([\w\.\-\@]*)', txt)
+	"""Find all instances of @mentions in the html."""
+	soup = BeautifulSoup(txt, 'html.parser')
+	emails = []
+	for mention in soup.find_all(class_='mention'):
+		email = mention['data-id']
+		emails.append(email)
+	return emails
 
 def handle_password_test_fail(result):
 	suggestions = result['feedback']['suggestions'][0] if result['feedback']['suggestions'] else ''

frappe/desk/form/utils.py

@@ -5,7 +5,7 @@ from __future__ import unicode_literals
 import frappe, json
 import frappe.desk.form.meta
 import frappe.desk.form.load
-from frappe.utils.html_utils import clean_email_html
+from frappe.utils.html_utils import sanitize_html
 from frappe.desk.form.document_follow import follow_document
 
 from frappe import _
@@ -64,7 +64,7 @@ def add_comment(reference_doctype, reference_name, content, comment_email):
 		doctype = 'Comment',
 		reference_doctype = reference_doctype,
 		reference_name = reference_name,
-		content = clean_email_html(content),
+		content = sanitize_html(content),
 		comment_email = comment_email,
 		comment_type = 'Comment'
 	)).insert(ignore_permissions = True)
@@ -124,4 +124,4 @@ def get_pdf_link(doctype, docname, print_format='Standard', no_letterhead=0):
 		docname = docname,
 		print_format = print_format,
 		no_letterhead = no_letterhead
-	)
+	)

frappe/public/js/frappe/form/controls/comment.js

@@ -82,12 +82,7 @@ frappe.ui.form.ControlComment = frappe.ui.form.ControlTextEditor.extend({
 			return null;
 		}
 
-		const at_values = this.mentions.map((value, i) => {
-			return {
-				id: i,
-				value
-			};
-		});
+		const at_values = this.mentions.slice();
 
 		return {
 			allowedChars: /^[A-Za-z0-9_]*$/,

frappe/public/js/frappe/form/footer/timeline.js

@@ -387,24 +387,6 @@ frappe.ui.form.Timeline = class Timeline {
 				c.content_html = frappe.utils.strip_whitespace(c.content_html);
 			}
 
-			// bold @mentions
-			if(c.comment_type==="Comment" &&
-				// avoid adding <b> tag a 2nd time
-				!c.content_html.match(/(^|\W)<b>(@[^\s]+)<\/b>/)
-			) {
-				/*
-					Replace the email ids by only displaying the string which
-					occurs before the second `@` to enhance the mentions.
-					Eg.
-					@abc@a-example.com will be converted to
-					@abc with the below line of code.
-				*/
-
-				c.content_html = c.content_html.replace(/(<[a][^>]*>)/g, "");
-				// bold the @mentions
-				c.content_html = c.content_html.replace(/(@[^\s@]*)@[^\s@|<]*/g, "<b>$1</b>");
-			}
-
 			if (this.is_communication_or_comment(c)) {
 				c.user_content = true;
 				if (!$.isArray(c._liked_by)) {
@@ -764,7 +746,12 @@ frappe.ui.form.Timeline = class Timeline {
 			.filter(user => !["Administrator", "Guest"].includes(user));
 		valid_users = valid_users
 			.filter(user => frappe.boot.user_info[user].allowed_in_mentions==1);
-		return valid_users.map(user => frappe.boot.user_info[user].name);
+		return valid_users.map(user => {
+			return {
+				id: frappe.boot.user_info[user].name,
+				value: frappe.boot.user_info[user].fullname,
+			}
+		});
 	}
 
 	setup_comment_like() {

frappe/public/less/form.less

@@ -374,6 +374,16 @@ h6.uppercase, .h6.uppercase {
 	}
 }
 
+.timeline-item-content .mention {
+	background-color: transparent;
+	padding: 0;
+	font-weight: bold;
+
+	span {
+		margin: 0;
+	}
+}
+
 .timeline {
 	position: relative;
 }

frappe/utils/html_utils.py

@@ -161,7 +161,8 @@ acceptable_attributes = [
 	'urn', 'valign', 'value', 'variable', 'volume', 'vspace', 'vrml',
 	'width', 'wrap', 'xml:lang', 'data-row', 'data-list', 'data-language',
 	'data-value', 'role', 'frameborder', 'allowfullscreen', 'spellcheck',
-	'data-mode', 'data-gramm', 'data-placeholder', 'data-comment'
+	'data-mode', 'data-gramm', 'data-placeholder', 'data-comment',
+	'data-id', 'data-denotation-char'
 ]
 
 mathml_attributes = [