From 1af59ce16cea79767438efaacc02a9f675c88362 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 10:42:19 +0530 Subject: [PATCH 1/8] refactor: Move mention list generation logic to server-side - Moved mention list generation logic to server-side to get latest mention list everytime - To indicate group option, added a users icon. --- frappe/desk/search.py | 34 ++++++++++++++++++ .../public/js/frappe/form/controls/comment.js | 35 +++++++------------ frappe/public/js/frappe/form/footer/footer.js | 2 +- .../js/frappe/form/footer/form_timeline.js | 2 +- 4 files changed, 49 insertions(+), 24 deletions(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 6181261fc2..b3af1ea6f5 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -221,3 +221,37 @@ def validate_and_sanitize_search_inputs(fn, instance, args, kwargs): return [] return fn(**kwargs) + + +@frappe.whitelist() +def get_names_for_mentions(search_term): + users_for_mentions = frappe.cache().get_value('users_for_mentions', get_users_for_mentions) + user_groups = frappe.cache().get_value('users_groups', get_user_groups) + + filtered_mentions = [] + for mention_data in users_for_mentions + user_groups: + if search_term.lower() not in mention_data.value.lower(): + continue + + mention_data['link'] = frappe.utils.get_url_to_form( + 'User Group' if mention_data.get('is_group') else 'User', + mention_data['id'] + ) + + filtered_mentions.append(mention_data) + + return sorted(filtered_mentions, key=lambda d: d['value']) + +def get_users_for_mentions(): + return frappe.get_all('User', + fields=['name as id', 'full_name as value'], + filters={ + 'name': ['not in', ('Administrator', 'Guest')], + 'allowed_in_mentions': True, + 'user_type': 'System User', + }) + +def get_user_groups(): + return frappe.get_all('User Group', fields=['name as id', 'name as value'], update={ + 'is_group': True + }) diff --git a/frappe/public/js/frappe/form/controls/comment.js b/frappe/public/js/frappe/form/controls/comment.js index 59b53bf59e..f20d496b11 100644 --- a/frappe/public/js/frappe/form/controls/comment.js +++ b/frappe/public/js/frappe/form/controls/comment.js @@ -78,35 +78,26 @@ frappe.ui.form.ControlComment = frappe.ui.form.ControlTextEditor.extend({ }, get_mention_options() { - if (!(this.mentions && this.mentions.length)) { + if (!this.enable_mentions) { return null; } - - const at_values = this.mentions.slice(); - + let me = this; return { allowedChars: /^[A-Za-z0-9_]*$/, mentionDenotationChars: ["@"], isolateCharacter: true, - source: function (searchTerm, renderList, mentionChar) { - let values; + source: frappe.utils.debounce(async function(search_term, renderList) { + let method = me.mention_search_method || 'frappe.desk.search.get_names_for_mentions'; + let values = await frappe.xcall(method, { + search_term + }); + renderList(values, search_term); + }, 300), + renderItem(item) { + let value = item.value; + return `${value} ${item.is_group ? frappe.utils.icon('users') : ''}`; - if (mentionChar === "@") { - values = at_values; - } - - if (searchTerm.length === 0) { - renderList(values, searchTerm); - } else { - const matches = []; - for (let i = 0; i < values.length; i++) { - if (~values[i].value.toLowerCase().indexOf(searchTerm.toLowerCase())) { - matches.push(values[i]); - } - } - renderList(matches, searchTerm); - } - }, + } }; }, diff --git a/frappe/public/js/frappe/form/footer/footer.js b/frappe/public/js/frappe/form/footer/footer.js index a1dabedff0..63d8b0b57d 100644 --- a/frappe/public/js/frappe/form/footer/footer.js +++ b/frappe/public/js/frappe/form/footer/footer.js @@ -24,7 +24,7 @@ frappe.ui.form.Footer = Class.extend({ parent: this.wrapper.find(".comment-box"), render_input: true, only_input: true, - mentions: frappe.utils.get_names_for_mentions(), + enable_mentions: true, df: { fieldtype: 'Comment', fieldname: 'comment' diff --git a/frappe/public/js/frappe/form/footer/form_timeline.js b/frappe/public/js/frappe/form/footer/form_timeline.js index bd64c504ca..ab83ed2f71 100644 --- a/frappe/public/js/frappe/form/footer/form_timeline.js +++ b/frappe/public/js/frappe/form/footer/form_timeline.js @@ -492,7 +492,7 @@ class FormTimeline extends BaseTimeline { fieldname: 'comment', label: 'Comment' }, - mentions: frappe.utils.get_names_for_mentions(), + enable_mentions: true, render_input: true, only_input: true, no_wrapper: true From d8c777e98cc12d159b7f000885d630cbf759d7e5 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 10:43:07 +0530 Subject: [PATCH 2/8] refactor: Remove unnecessary code --- frappe/boot.py | 2 -- frappe/public/js/frappe/desk.js | 11 ----------- frappe/public/js/frappe/utils/utils.js | 25 ------------------------- 3 files changed, 38 deletions(-) diff --git a/frappe/boot.py b/frappe/boot.py index 65a07b15e5..0dfcb8d1b4 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -42,8 +42,6 @@ def get_bootinfo(): bootinfo.user_info = get_user_info() bootinfo.sid = frappe.session['sid'] - bootinfo.user_groups = frappe.get_all('User Group', pluck="name") - bootinfo.modules = {} bootinfo.module_list = [] load_desktop_data(bootinfo) diff --git a/frappe/public/js/frappe/desk.js b/frappe/public/js/frappe/desk.js index c093a73689..2331766710 100644 --- a/frappe/public/js/frappe/desk.js +++ b/frappe/public/js/frappe/desk.js @@ -114,8 +114,6 @@ frappe.Application = Class.extend({ dialog.get_close_btn().toggle(false); }); - this.setup_user_group_listeners(); - // listen to build errors this.setup_build_error_listener(); @@ -593,15 +591,6 @@ frappe.Application = Class.extend({ } }, - setup_user_group_listeners() { - frappe.realtime.on('user_group_added', (user_group) => { - frappe.boot.user_groups && frappe.boot.user_groups.push(user_group); - }); - frappe.realtime.on('user_group_deleted', (user_group) => { - frappe.boot.user_groups = (frappe.boot.user_groups || []).filter(el => el !== user_group); - }); - }, - setup_energy_point_listeners() { frappe.realtime.on('energy_point_alert', (message) => { frappe.show_alert(message); diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 7ce30a525c..8e6a458c3e 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1272,31 +1272,6 @@ Object.assign(frappe.utils, { `); }, - get_names_for_mentions() { - let names_for_mentions = Object.keys(frappe.boot.user_info || []) - .filter(user => { - return !["Administrator", "Guest"].includes(user) - && frappe.boot.user_info[user].allowed_in_mentions - && frappe.boot.user_info[user].user_type === 'System User'; - }) - .map(user => { - return { - id: frappe.boot.user_info[user].name, - value: frappe.boot.user_info[user].fullname, - }; - }); - - frappe.boot.user_groups && frappe.boot.user_groups.map(group => { - names_for_mentions.push({ - id: group, - value: group, - is_group: true, - link: frappe.utils.get_form_link('User Group', group) - }); - }); - - return names_for_mentions; - }, print(doctype, docname, print_format, letterhead, lang_code) { let w = window.open( frappe.urllib.get_full_url( From 959b27ef0ebb55da72499cac56985423e06339ed Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 10:44:09 +0530 Subject: [PATCH 3/8] fix: Bust user / user group cache on change --- frappe/core/doctype/user/user.py | 7 +++++++ frappe/core/doctype/user_group/user_group.py | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 0462de8643..a4d13a57e0 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -56,6 +56,7 @@ class User(Document): def after_insert(self): create_notification_settings(self.name) + frappe.cache().delete_key('users_for_mentions') def validate(self): self.check_demo() @@ -129,6 +130,9 @@ class User(Document): if self.time_zone: frappe.defaults.set_default("time_zone", self.time_zone, self.name) + if self.has_value_changed('allow_in_mentions') or self.has_value_changed('user_type'): + frappe.cache().delete_key('users_for_mentions') + def has_website_permission(self, ptype, user, verbose=False): """Returns true if current user is the session user""" return self.name == frappe.session.user @@ -389,6 +393,9 @@ class User(Document): # delete notification settings frappe.delete_doc("Notification Settings", self.name, ignore_permissions=True) + if self.get('allow_in_mentions'): + frappe.cache().delete_key('users_for_mentions') + def before_rename(self, old_name, new_name, merge=False): self.check_demo() diff --git a/frappe/core/doctype/user_group/user_group.py b/frappe/core/doctype/user_group/user_group.py index 64bffa06d0..b1d0fede4c 100644 --- a/frappe/core/doctype/user_group/user_group.py +++ b/frappe/core/doctype/user_group/user_group.py @@ -9,7 +9,7 @@ import frappe class UserGroup(Document): def after_insert(self): - frappe.publish_realtime('user_group_added', self.name) + frappe.cache().delete_key('user_groups') def on_trash(self): - frappe.publish_realtime('user_group_deleted', self.name) + frappe.cache().delete_key('user_groups') From 22a2803f7b36608eb085d3fdb94771d82e498cd5 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 10:44:46 +0530 Subject: [PATCH 4/8] fix: Show pointer on hovering on mention list --- frappe/public/scss/common/quill.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/public/scss/common/quill.scss b/frappe/public/scss/common/quill.scss index d15ca7e036..6dd0853635 100644 --- a/frappe/public/scss/common/quill.scss +++ b/frappe/public/scss/common/quill.scss @@ -105,6 +105,7 @@ padding: 10px 12px; height: initial; line-height: initial; + cursor: pointer; &.selected { background-color: var(--control-bg); From 183af96465a44cbf2526a22764603c1f0bdb8a01 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 10:53:20 +0530 Subject: [PATCH 5/8] fix: Link user mention to User Profile --- frappe/desk/search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index b3af1ea6f5..588120588c 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -234,7 +234,7 @@ def get_names_for_mentions(search_term): continue mention_data['link'] = frappe.utils.get_url_to_form( - 'User Group' if mention_data.get('is_group') else 'User', + 'User Group' if mention_data.get('is_group') else 'User Profile', mention_data['id'] ) From 395eed225ff0a03babcc9f6fe38350cd6b85f717 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 11:03:18 +0530 Subject: [PATCH 6/8] style: Fix formatting --- frappe/desk/search.py | 8 ++++---- frappe/public/js/frappe/form/controls/comment.js | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 588120588c..065dc5c3ed 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -246,10 +246,10 @@ def get_users_for_mentions(): return frappe.get_all('User', fields=['name as id', 'full_name as value'], filters={ - 'name': ['not in', ('Administrator', 'Guest')], - 'allowed_in_mentions': True, - 'user_type': 'System User', - }) + 'name': ['not in', ('Administrator', 'Guest')], + 'allowed_in_mentions': True, + 'user_type': 'System User', + }) def get_user_groups(): return frappe.get_all('User Group', fields=['name as id', 'name as value'], update={ diff --git a/frappe/public/js/frappe/form/controls/comment.js b/frappe/public/js/frappe/form/controls/comment.js index f20d496b11..7efc60b61d 100644 --- a/frappe/public/js/frappe/form/controls/comment.js +++ b/frappe/public/js/frappe/form/controls/comment.js @@ -96,7 +96,6 @@ frappe.ui.form.ControlComment = frappe.ui.form.ControlTextEditor.extend({ renderItem(item) { let value = item.value; return `${value} ${item.is_group ? frappe.utils.icon('users') : ''}`; - } }; }, From 43a4f1861e30f1a762e4ea1a74921b05412c3e00 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 11:09:13 +0530 Subject: [PATCH 7/8] fix: Typo --- frappe/desk/search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 065dc5c3ed..3c9109eca9 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -226,7 +226,7 @@ def validate_and_sanitize_search_inputs(fn, instance, args, kwargs): @frappe.whitelist() def get_names_for_mentions(search_term): users_for_mentions = frappe.cache().get_value('users_for_mentions', get_users_for_mentions) - user_groups = frappe.cache().get_value('users_groups', get_user_groups) + user_groups = frappe.cache().get_value('user_groups', get_user_groups) filtered_mentions = [] for mention_data in users_for_mentions + user_groups: From 9bb0ea0301bc6b28ffaacfa9638cc87102bcfcb2 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 30 Apr 2021 12:43:55 +0530 Subject: [PATCH 8/8] fix: Show icon instead of a different color for group --- .../js/frappe/form/controls/quill-mention/blots/mention.js | 1 + frappe/public/scss/common/quill.scss | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/quill-mention/blots/mention.js b/frappe/public/js/frappe/form/controls/quill-mention/blots/mention.js index d6907158f9..4a3c2d2eba 100644 --- a/frappe/public/js/frappe/form/controls/quill-mention/blots/mention.js +++ b/frappe/public/js/frappe/form/controls/quill-mention/blots/mention.js @@ -12,6 +12,7 @@ class MentionBlot extends Embed { denotationChar.innerHTML = data.denotationChar; node.appendChild(denotationChar); node.innerHTML += data.value; + node.innerHTML += `${data.isGroup === 'true' ? frappe.utils.icon('users') : ''}`; node.dataset.id = data.id; node.dataset.value = data.value; node.dataset.denotationChar = data.denotationChar; diff --git a/frappe/public/scss/common/quill.scss b/frappe/public/scss/common/quill.scss index 6dd0853635..24e8293cf3 100644 --- a/frappe/public/scss/common/quill.scss +++ b/frappe/public/scss/common/quill.scss @@ -197,5 +197,8 @@ } .mention[data-is-group="true"] { - background-color: var(--group-mention-bg-color); + .icon { + margin-top: -2px; + margin-left: 4px; + } }