From 7315076038fea37f8368ee691b56835ff3b64ab7 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Wed, 13 Oct 2021 14:06:34 +0530 Subject: [PATCH 01/26] refactor: converted queries --- frappe/defaults.py | 18 ++++++++---------- frappe/permissions.py | 17 +++++++++-------- frappe/sessions.py | 18 +++++++++--------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/frappe/defaults.py b/frappe/defaults.py index 75feabc332..796965e597 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -116,14 +116,11 @@ def set_default(key, value, parent, parenttype="__default"): :param value: Default value. :param parent: Usually, **User** to whom the default belongs. :param parenttype: [optional] default is `__default`.""" - if frappe.db.sql(''' - select - defkey - from - `tabDefaultValue` - where - defkey=%s and parent=%s - for update''', (key, parent)): + table = frappe.qb.DocType("DefaultValue") + result = frappe.qb.from_(table).where(table.defkey == key) \ + .where(table.parent == parent) \ + .select(table.defkey).for_update().run() + if result: frappe.db.delete("DefaultValue", { "defkey": key, "parent": parent @@ -191,8 +188,9 @@ def get_defaults_for(parent="__default"): if defaults==None: # sort descending because first default must get precedence - res = frappe.db.sql("""select defkey, defvalue from `tabDefaultValue` - where parent = %s order by creation""", (parent,), as_dict=1) + table = frappe.qb.DocType("DefaultValue") + res = frappe.qb.from_(table).where(table.parent == parent) \ + .select(table.defkey, table.defvalue).orderby("creation").run(as_dict=True) defaults = frappe._dict({}) for d in res: diff --git a/frappe/permissions.py b/frappe/permissions.py index 7ee1119ebb..34fcbee1a8 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -333,8 +333,8 @@ def get_all_perms(role): '''Returns valid permissions for a given role''' perms = frappe.get_all('DocPerm', fields='*', filters=dict(role=role)) custom_perms = frappe.get_all('Custom DocPerm', fields='*', filters=dict(role=role)) - doctypes_with_custom_perms = frappe.db.sql_list("""select distinct parent - from `tabCustom DocPerm`""") + query = frappe.qb.from_("Custom DocPerm").select("parent").distinct() + doctypes_with_custom_perms = frappe.db.sql_list(query) for p in perms: if p.parent not in doctypes_with_custom_perms: @@ -351,10 +351,12 @@ def get_roles(user=None, with_standard=True): def get(): if user == 'Administrator': - return [r[0] for r in frappe.db.sql("select name from `tabRole`")] # return all available roles + return [r[0] for r in frappe.qb.from_("Role").select("name").run()] # return all available roles else: - return [r[0] for r in frappe.db.sql("""select role from `tabHas Role` - where parent=%s and role not in ('All', 'Guest')""", (user,))] + ['All', 'Guest'] + table = frappe.qb.DocType("Has Role") + result = frappe.qb.form_(table).where(table.parent == user) \ + .where(table.role.notin(["All", "Guest"])).select(table.role).run() + return [r[0] for r in result] + ['All', 'Guest'] roles = frappe.cache().hget("roles", user, get) @@ -463,10 +465,9 @@ def update_permission_property(doctype, role, permlevel, ptype, value=None, vali name = frappe.get_value('Custom DocPerm', dict(parent=doctype, role=role, permlevel=permlevel)) + table = frappe.qb.DocType("Custom DocPerm") + frappe.qb.update(table).set(ptype, value).where(table.name == name).run() - frappe.db.sql(""" - update `tabCustom DocPerm` - set `{0}`=%s where name=%s""".format(ptype), (value, name)) if validate: validate_permissions_for_doctype(doctype) diff --git a/frappe/sessions.py b/frappe/sessions.py index ce104968ad..efa2779a23 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -16,6 +16,7 @@ import frappe.translate import redis from urllib.parse import unquote from frappe.cache_manager import clear_user_cache +from frappe.query_builder import Order @frappe.whitelist(allow_guest=True) def clear(user=None): @@ -61,18 +62,17 @@ def get_sessions_to_clear(user=None, keep_current=False, device=None): simultaneous_sessions = frappe.db.get_value('User', user, 'simultaneous_sessions') or 1 offset = simultaneous_sessions - 1 + table = frappe.qb.DocType("Sessions") + criterion = frappe.qb.from_(table).where(table.user == user) \ + .where(table.device.isin(device)) condition = '' if keep_current: - condition = ' AND sid != {0}'.format(frappe.db.escape(frappe.session.sid)) + criterion = criterion.where(table.sid != frappe.db.escape(frappe.session.sid)) - return frappe.db.sql_list(""" - SELECT `sid` FROM `tabSessions` - WHERE `tabSessions`.user=%(user)s - AND device in %(device)s - {condition} - ORDER BY `lastupdate` DESC - LIMIT 100 OFFSET {offset}""".format(condition=condition, offset=offset), - {"user": user, "device": device}) + query = criterion.select(table.sid).offset(offset).limit(100) \ + .orderby(table.lastupdate, order=Order.desc) + + return frappe.db.sql_list(query) def delete_session(sid=None, user=None, reason="Session Expired"): from frappe.core.doctype.activity_log.feed import logout_feed From a621c4178cb20ffc19296226c6851ec5b8cbf6e7 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Wed, 13 Oct 2021 14:28:13 +0530 Subject: [PATCH 02/26] fix: fixing erroneous query conversions --- frappe/defaults.py | 2 +- frappe/permissions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/defaults.py b/frappe/defaults.py index 796965e597..3672bed511 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -190,7 +190,7 @@ def get_defaults_for(parent="__default"): # sort descending because first default must get precedence table = frappe.qb.DocType("DefaultValue") res = frappe.qb.from_(table).where(table.parent == parent) \ - .select(table.defkey, table.defvalue).orderby("creation").run(as_dict=True) + .select(table.defkey, table.defvalue).orderby("creation").run(as_dict=True) defaults = frappe._dict({}) for d in res: diff --git a/frappe/permissions.py b/frappe/permissions.py index 34fcbee1a8..494a0f1d14 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -354,7 +354,7 @@ def get_roles(user=None, with_standard=True): return [r[0] for r in frappe.qb.from_("Role").select("name").run()] # return all available roles else: table = frappe.qb.DocType("Has Role") - result = frappe.qb.form_(table).where(table.parent == user) \ + result = frappe.qb.from_(table).where(table.parent == user) \ .where(table.role.notin(["All", "Guest"])).select(table.role).run() return [r[0] for r in result] + ['All', 'Guest'] From e0a3e4efe3283ae08e88979262723b9e460a1aeb Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Wed, 13 Oct 2021 15:13:13 +0530 Subject: [PATCH 03/26] refactor: converted queries in share & translate --- frappe/share.py | 6 ++++-- frappe/translate.py | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/frappe/share.py b/frappe/share.py index 030feea8fa..cee1120066 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -128,8 +128,10 @@ def get_shared_doctypes(user=None): """Return list of doctypes in which documents are shared for the given user.""" if not user: user = frappe.session.user - - return frappe.db.sql_list("select distinct share_doctype from tabDocShare where (user=%s or everyone=1)", user) + table = frappe.qb.DocType("DocShare") + query = frappe.qb.from_(table).where(table.user == user | table.everyone == 1) \ + .select(table.share_doctype).distinct() + return frappe.db.sql_list(query) def get_share_name(doctype, name, user, everyone): if cint(everyone): diff --git a/frappe/translate.py b/frappe/translate.py index 6f3ed81dc2..3fc9fa826e 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -119,7 +119,8 @@ def set_default_language(lang): def get_lang_dict(): """Returns all languages in dict format, full name is the key e.g. `{"english":"en"}`""" - return dict(frappe.db.sql('select language_name, name from tabLanguage')) + result = dict(frappe.qb.from_("Language").select("language_name", "name").run()) + return result def get_dict(fortype, name=None): """Returns translation dict for a type of object. @@ -151,12 +152,12 @@ def get_dict(fortype, name=None): messages += get_messages_from_navbar() messages += get_messages_from_include_files() - messages += frappe.db.sql("select 'Print Format:', name from `tabPrint Format`") - messages += frappe.db.sql("select 'DocType:', name from tabDocType") - messages += frappe.db.sql("select 'Role:', name from tabRole") - messages += frappe.db.sql("select 'Module:', name from `tabModule Def`") - messages += frappe.db.sql("select '', format from `tabWorkspace Shortcut` where format is not null") - messages += frappe.db.sql("select '', title from `tabOnboarding Step`") + messages += frappe.qb.from_("Print Format").select("Print Format:", "name").run() + messages += frappe.qb.from_("DocType").select("DocType:", "name").run() + messages += frappe.qb.from_("Role").select("Role:", "name").run() + messages += frappe.qb.from_("Module Def").select("Module:", "name").run() + messages += frappe.qb.from_("Workspace Shortcut").where(frappe.qb.Field("format" != None)).select("").run() + messages += frappe.qb.from_("Onboarding Step").select("", "title").run() messages = deduplicate_messages(messages) message_dict = make_dict_from_messages(messages, load_user_translation=False) @@ -898,7 +899,8 @@ def get_translator_url(): def get_all_languages(with_language_name=False): """Returns all language codes ar, ch etc""" def get_language_codes(): - return frappe.db.sql_list('select name from tabLanguage') + query = frappe.qb.from_("Language").select("name") + return frappe.db.sql_list(query) def get_all_language_with_name(): return frappe.db.get_all('Language', ['language_code', 'language_name']) From e01d97b8df979e59a318cd3e1b03c5838f3b8e83 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 14 Oct 2021 01:16:46 +0530 Subject: [PATCH 04/26] refactor: replacing queries with frappe ORM --- frappe/defaults.py | 20 ++++++++++++-------- frappe/permissions.py | 16 ++++++++-------- frappe/translate.py | 15 ++++++++++----- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/frappe/defaults.py b/frappe/defaults.py index 3672bed511..eb98db449f 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -4,6 +4,7 @@ import frappe from frappe.desk.notifications import clear_notifications from frappe.cache_manager import clear_defaults_cache, common_default_keys +from frappe.query_builder import DocType # Note: DefaultValue records are identified by parenttype # __default, __global or 'User Permission' @@ -116,11 +117,11 @@ def set_default(key, value, parent, parenttype="__default"): :param value: Default value. :param parent: Usually, **User** to whom the default belongs. :param parenttype: [optional] default is `__default`.""" - table = frappe.qb.DocType("DefaultValue") - result = frappe.qb.from_(table).where(table.defkey == key) \ - .where(table.parent == parent) \ - .select(table.defkey).for_update().run() - if result: + table = DocType("DefaultValue") + key_exists = frappe.qb.from_(table).where( + (table.defkey == key) & (table.parent == parent) + ).select(table.defkey).for_update().run() + if key_exists: frappe.db.delete("DefaultValue", { "defkey": key, "parent": parent @@ -188,9 +189,12 @@ def get_defaults_for(parent="__default"): if defaults==None: # sort descending because first default must get precedence - table = frappe.qb.DocType("DefaultValue") - res = frappe.qb.from_(table).where(table.parent == parent) \ - .select(table.defkey, table.defvalue).orderby("creation").run(as_dict=True) + table = DocType("DefaultValue") + res = frappe.qb.from_(table).where( + table.parent == parent + ).select( + table.defkey, table.defvalue + ).orderby("creation").run(as_dict=True) defaults = frappe._dict({}) for d in res: diff --git a/frappe/permissions.py b/frappe/permissions.py index 494a0f1d14..0585b1e220 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -6,7 +6,7 @@ import frappe import frappe.share from frappe import _, msgprint from frappe.utils import cint - +from frappe.query_builder import DocType rights = ("select", "read", "write", "create", "delete", "submit", "cancel", "amend", "print", "email", "report", "import", "export", "set_user_permissions", "share") @@ -333,8 +333,7 @@ def get_all_perms(role): '''Returns valid permissions for a given role''' perms = frappe.get_all('DocPerm', fields='*', filters=dict(role=role)) custom_perms = frappe.get_all('Custom DocPerm', fields='*', filters=dict(role=role)) - query = frappe.qb.from_("Custom DocPerm").select("parent").distinct() - doctypes_with_custom_perms = frappe.db.sql_list(query) + doctypes_with_custom_perms = frappe.get_all("Custom DocPerm", pluck="parent", distinct=True) for p in perms: if p.parent not in doctypes_with_custom_perms: @@ -351,11 +350,12 @@ def get_roles(user=None, with_standard=True): def get(): if user == 'Administrator': - return [r[0] for r in frappe.qb.from_("Role").select("name").run()] # return all available roles + return frappe.get_all("Role", pluck="name") # return all available roles else: - table = frappe.qb.DocType("Has Role") - result = frappe.qb.from_(table).where(table.parent == user) \ - .where(table.role.notin(["All", "Guest"])).select(table.role).run() + table = DocType("Has Role") + result = frappe.qb.from_(table).where( + (table.parent == user) & (table.role.notin(["All", "Guest"])) + ).select(table.role).run() return [r[0] for r in result] + ['All', 'Guest'] roles = frappe.cache().hget("roles", user, get) @@ -465,7 +465,7 @@ def update_permission_property(doctype, role, permlevel, ptype, value=None, vali name = frappe.get_value('Custom DocPerm', dict(parent=doctype, role=role, permlevel=permlevel)) - table = frappe.qb.DocType("Custom DocPerm") + table = DocType("Custom DocPerm") frappe.qb.update(table).set(ptype, value).where(table.name == name).run() if validate: diff --git a/frappe/translate.py b/frappe/translate.py index 3fc9fa826e..3abffd3215 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -20,6 +20,7 @@ from typing import List, Union, Tuple import frappe from frappe.model.utils import InvalidIncludePath, render_include from frappe.utils import get_bench_path, is_html, strip, strip_html_tags +from frappe.query_builder import Field def get_language(lang_list: List = None) -> str: @@ -156,7 +157,7 @@ def get_dict(fortype, name=None): messages += frappe.qb.from_("DocType").select("DocType:", "name").run() messages += frappe.qb.from_("Role").select("Role:", "name").run() messages += frappe.qb.from_("Module Def").select("Module:", "name").run() - messages += frappe.qb.from_("Workspace Shortcut").where(frappe.qb.Field("format" != None)).select("").run() + messages += frappe.qb.from_("Workspace Shortcut").where(Field("format").isnotnull()).select("").run() messages += frappe.qb.from_("Onboarding Step").select("", "title").run() messages = deduplicate_messages(messages) @@ -324,13 +325,17 @@ def get_messages_for_app(app, deduplicate=True): # doctypes if modules: - for name in frappe.db.sql_list("""select name from tabDocType - where module in ({})""".format(modules)): + names = frappe.qb.from_("DocType").where( + Field("module").isin(modules) + ).select("name").run() + for name in names: messages.extend(get_messages_from_doctype(name)) # pages - for name, title in frappe.db.sql("""select name, title from tabPage - where module in ({})""".format(modules)): + result = frappe.qb.from_("Page").where( + Field("module").isin(modules) + ).select("name", "title").run() + for name, title in result: messages.append((None, title or name)) messages.extend(get_messages_from_page(name)) From d75aab4b3edf219471169b12487b3c3dcfd92be5 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 14 Oct 2021 03:28:53 +0530 Subject: [PATCH 05/26] fix: fixed erroneous query conversion --- frappe/share.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/share.py b/frappe/share.py index cee1120066..4d43990c54 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -129,8 +129,9 @@ def get_shared_doctypes(user=None): if not user: user = frappe.session.user table = frappe.qb.DocType("DocShare") - query = frappe.qb.from_(table).where(table.user == user | table.everyone == 1) \ - .select(table.share_doctype).distinct() + query = frappe.qb.from_(table).where( + (table.user == user) | (table.everyone == 1) + ).select(table.share_doctype).distinct() return frappe.db.sql_list(query) def get_share_name(doctype, name, user, everyone): From f5a5f975936390039672012b0225b79a1d714db9 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 14 Oct 2021 03:29:20 +0530 Subject: [PATCH 06/26] refactor: converted quries in sessions.py --- frappe/sessions.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/frappe/sessions.py b/frappe/sessions.py index efa2779a23..508badbd8f 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -17,6 +17,7 @@ import redis from urllib.parse import unquote from frappe.cache_manager import clear_user_cache from frappe.query_builder import Order +from frappe.query_builder import DocType @frappe.whitelist(allow_guest=True) def clear(user=None): @@ -65,12 +66,10 @@ def get_sessions_to_clear(user=None, keep_current=False, device=None): table = frappe.qb.DocType("Sessions") criterion = frappe.qb.from_(table).where(table.user == user) \ .where(table.device.isin(device)) - condition = '' if keep_current: criterion = criterion.where(table.sid != frappe.db.escape(frappe.session.sid)) - query = criterion.select(table.sid).offset(offset).limit(100) \ - .orderby(table.lastupdate, order=Order.desc) + query = criterion.select(table.sid).offset(offset).limit(100).orderby(table.lastupdate, order=Order.desc) return frappe.db.sql_list(query) @@ -80,7 +79,10 @@ def delete_session(sid=None, user=None, reason="Session Expired"): frappe.cache().hdel("session", sid) frappe.cache().hdel("last_db_session_update", sid) if sid and not user: - user_details = frappe.db.sql("""select user from tabSessions where sid=%s""", sid, as_dict=True) + table = DocType("Sessions") + user_details = frappe.qb.from_(table).where( + table.sid == sid + ).select(table.user).run(as_dict=True) if user_details: user = user_details[0].get("user") logout_feed(user, reason) @@ -91,7 +93,7 @@ def clear_all_sessions(reason=None): """This effectively logs out all users""" frappe.only_for("Administrator") if not reason: reason = "Deleted All Active Session" - for sid in frappe.db.sql_list("select sid from `tabSessions`"): + for sid in [r[0] for r in frappe.qb.from_("Sessions").select("sid").run()]: delete_session(sid, reason=reason) def get_expired_sessions(): From b05cd732f53dc3e4197b0819958bc86d9ea2965d Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 14 Oct 2021 14:45:15 +0530 Subject: [PATCH 07/26] ci: trigger build From 668051cfe9a7b3a6a30485a81b2c7fca380cb646 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 14 Oct 2021 22:54:53 +0530 Subject: [PATCH 08/26] refactor: refactored query using frappe.get_all --- frappe/translate.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/translate.py b/frappe/translate.py index 3abffd3215..f6a3880774 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -120,7 +120,7 @@ def set_default_language(lang): def get_lang_dict(): """Returns all languages in dict format, full name is the key e.g. `{"english":"en"}`""" - result = dict(frappe.qb.from_("Language").select("language_name", "name").run()) + result = frappe.get_all("Language", fields=["language_name", "name"], order_by="modified", as_list=True) return result def get_dict(fortype, name=None): @@ -904,8 +904,7 @@ def get_translator_url(): def get_all_languages(with_language_name=False): """Returns all language codes ar, ch etc""" def get_language_codes(): - query = frappe.qb.from_("Language").select("name") - return frappe.db.sql_list(query) + return frappe.get_all("Language", pluck="name", order_by="modified") def get_all_language_with_name(): return frappe.db.get_all('Language', ['language_code', 'language_name']) From 4ed10de918bfa05d9e38675868e42e92cede805f Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Thu, 14 Oct 2021 23:25:43 +0530 Subject: [PATCH 09/26] fix: fixed sider issues --- frappe/core/doctype/activity_log/activity_log.py | 9 ++++++--- frappe/sessions.py | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/activity_log/activity_log.py b/frappe/core/doctype/activity_log/activity_log.py index 183a1c264c..9c27e84740 100644 --- a/frappe/core/doctype/activity_log/activity_log.py +++ b/frappe/core/doctype/activity_log/activity_log.py @@ -7,6 +7,8 @@ from frappe.utils import get_fullname, now from frappe.model.document import Document from frappe.core.utils import set_timeline_doc import frappe +from frappe.query_builder import DocType, Interval +from frappe.query_builder.functions import Now class ActivityLog(Document): def before_insert(self): @@ -44,6 +46,7 @@ def clear_activity_logs(days=None): if not days: days = 90 - - frappe.db.sql("""delete from `tabActivity Log` where \ - creation< (NOW() - INTERVAL '{0}' DAY)""".format(days)) \ No newline at end of file + doctype = DocType("Activity Log") + frappe.qb.from_(doctype).where( + doctype.creation < (Now() - Interval(days=days)) + ).delete().run() \ No newline at end of file diff --git a/frappe/sessions.py b/frappe/sessions.py index 508badbd8f..05f3bba3cf 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -64,8 +64,7 @@ def get_sessions_to_clear(user=None, keep_current=False, device=None): offset = simultaneous_sessions - 1 table = frappe.qb.DocType("Sessions") - criterion = frappe.qb.from_(table).where(table.user == user) \ - .where(table.device.isin(device)) + criterion = frappe.qb.from_(table).where((table.user == user) & (table.device.isin(device))) if keep_current: criterion = criterion.where(table.sid != frappe.db.escape(frappe.session.sid)) From 2c088c81ba3f05cd9a9500cf3ef197e028f89c14 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 15 Oct 2021 05:00:46 +0530 Subject: [PATCH 10/26] refactor: Converted more queries --- .../core/doctype/log_settings/log_settings.py | 10 +++++-- frappe/core/doctype/user/user.py | 30 ++++++++++++------- .../patches/v11_0/remove_skip_for_doctype.py | 17 +++++------ 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 8a471b9173..5b361c9a13 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -5,6 +5,9 @@ import frappe from frappe import _ from frappe.model.document import Document +from frappe.query_builder import DocType, Interval +from frappe.query_builder.functions import Now + class LogSettings(Document): def clear_logs(self): @@ -13,9 +16,10 @@ class LogSettings(Document): self.clear_email_queue() def clear_error_logs(self): - frappe.db.sql(""" DELETE FROM `tabError Log` - WHERE `creation` < (NOW() - INTERVAL '{0}' DAY) - """.format(self.clear_error_log_after)) + table = DocType("Error Log") + frappe.db.delete(table, filters=( + table.creation < Now() - Interval(days=self.clear_error_log_after) + )) def clear_activity_logs(self): from frappe.core.doctype.activity_log.activity_log import clear_activity_logs diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index e4b94cdbb6..45f7d47a27 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -16,6 +16,7 @@ from frappe.utils.user import get_system_managers from frappe.website.utils import is_signup_disabled from frappe.rate_limiter import rate_limit from frappe.core.doctype.user_type.user_type import user_linked_with_permission_on_doctype +from frappe.query_builder import DocType STANDARD_USERS = ("Guest", "Administrator") @@ -366,15 +367,21 @@ class User(Document): # delete shares frappe.db.delete("DocShare", {"user": self.name}) # delete messages - frappe.db.sql("""delete from `tabCommunication` - where communication_type in ('Chat', 'Notification') - and reference_doctype='User' - and (reference_name=%s or owner=%s)""", (self.name, self.name)) - + table = DocType("Communication") + frappe.db.delete( + table, + filters=( + (table.communication_type.isin(["Chat", "Notification"])) + & (table.reference_doctype == "User") + & ((table.reference_name == self.name) | table.owner == self.name) + ), + run=False, + ) # unlink contact - frappe.db.sql("""update `tabContact` - set `user`=null - where `user`=%s""", (self.name)) + table = DocType("Contact") + frappe.qb.update(table).where( + table.user == self.name + ).set(table.user, None).run() # delete notification settings frappe.delete_doc("Notification Settings", self.name, ignore_permissions=True) @@ -421,9 +428,10 @@ class User(Document): frappe.rename_doc("Notification Settings", old_name, new_name, force=True, show_alert=False) # set email - frappe.db.sql("""UPDATE `tabUser` - SET email = %s - WHERE name = %s""", (new_name, new_name)) + table = DocType("User") + frappe.qb.update(table).where( + table.name == new_name + ).set("email", new_name).run() def append_roles(self, *roles): """Add roles to user""" diff --git a/frappe/patches/v11_0/remove_skip_for_doctype.py b/frappe/patches/v11_0/remove_skip_for_doctype.py index 638a5a0fd7..1063dca3ff 100644 --- a/frappe/patches/v11_0/remove_skip_for_doctype.py +++ b/frappe/patches/v11_0/remove_skip_for_doctype.py @@ -2,6 +2,7 @@ import frappe from frappe.desk.form.linked_with import get_linked_doctypes from frappe.patches.v11_0.replicate_old_user_permissions import get_doctypes_to_skip +from frappe.query_builder import Field # `skip_for_doctype` was a un-normalized way of storing for which # doctypes the user permission was applicable. @@ -72,16 +73,12 @@ def execute(): frappe.db.set_value('User Permission', user_permission.name, 'apply_to_all_doctypes', 1) if new_user_permissions_list: - frappe.db.sql(''' - INSERT INTO `tabUser Permission` - (`name`, `user`, `allow`, `for_value`, `applicable_for`, `apply_to_all_doctypes`, `creation`, `modified`) - VALUES {} - '''.format( # nosec - ', '.join(['%s'] * len(new_user_permissions_list)) - ), tuple(new_user_permissions_list)) + frappe.qb.into("User Permission").columns( + "name", "user", "allow", "for_value", "applicable_for", "apply_to_all_doctypes", "creation", "modified" + ).insert(tuple(new_user_permissions_list)).run() if user_permissions_to_delete: - frappe.db.sql('DELETE FROM `tabUser Permission` WHERE `name` in ({})' # nosec - .format(','.join(['%s'] * len(user_permissions_to_delete))), - tuple(user_permissions_to_delete) + frappe.db.delete( + "User Permission", + filters=(Field("name").isin(tuple(user_permissions_to_delete))) ) From 0cce6e2af87d26f9844e4ace38418f190cf10f2f Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 16 Oct 2021 09:21:38 +0530 Subject: [PATCH 11/26] fix: fixed erroneous queries in translate --- .../core/doctype/activity_log/activity_log.py | 7 ++-- .../core/doctype/log_settings/log_settings.py | 3 +- frappe/translate.py | 36 +++++++++++++++---- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/activity_log/activity_log.py b/frappe/core/doctype/activity_log/activity_log.py index 9c27e84740..69565a2c2a 100644 --- a/frappe/core/doctype/activity_log/activity_log.py +++ b/frappe/core/doctype/activity_log/activity_log.py @@ -9,6 +9,7 @@ from frappe.core.utils import set_timeline_doc import frappe from frappe.query_builder import DocType, Interval from frappe.query_builder.functions import Now +from pypika.terms import PseudoColumn class ActivityLog(Document): def before_insert(self): @@ -47,6 +48,6 @@ def clear_activity_logs(days=None): if not days: days = 90 doctype = DocType("Activity Log") - frappe.qb.from_(doctype).where( - doctype.creation < (Now() - Interval(days=days)) - ).delete().run() \ No newline at end of file + frappe.db.delete(doctype, filters=( + doctype.creation < PseudoColumn(f"({Now() - Interval(days=days)})") + )) \ No newline at end of file diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 5b361c9a13..008656f0f6 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -7,6 +7,7 @@ from frappe import _ from frappe.model.document import Document from frappe.query_builder import DocType, Interval from frappe.query_builder.functions import Now +from pypika.terms import PseudoColumn class LogSettings(Document): @@ -18,7 +19,7 @@ class LogSettings(Document): def clear_error_logs(self): table = DocType("Error Log") frappe.db.delete(table, filters=( - table.creation < Now() - Interval(days=self.clear_error_log_after) + table.creation < PseudoColumn(f"({Now() - Interval(days=self.clear_error_log_after)})") )) def clear_activity_logs(self): diff --git a/frappe/translate.py b/frappe/translate.py index f6a3880774..d0208bd379 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -21,6 +21,7 @@ import frappe from frappe.model.utils import InvalidIncludePath, render_include from frappe.utils import get_bench_path, is_html, strip, strip_html_tags from frappe.query_builder import Field +from pypika.terms import PseudoColumn def get_language(lang_list: List = None) -> str: @@ -153,12 +154,35 @@ def get_dict(fortype, name=None): messages += get_messages_from_navbar() messages += get_messages_from_include_files() - messages += frappe.qb.from_("Print Format").select("Print Format:", "name").run() - messages += frappe.qb.from_("DocType").select("DocType:", "name").run() - messages += frappe.qb.from_("Role").select("Role:", "name").run() - messages += frappe.qb.from_("Module Def").select("Module:", "name").run() - messages += frappe.qb.from_("Workspace Shortcut").where(Field("format").isnotnull()).select("").run() - messages += frappe.qb.from_("Onboarding Step").select("", "title").run() + messages += ( + frappe.qb.from_("Print Format") + .select(PseudoColumn("'Print Format:'"), "name") + .run() + ) + messages += ( + frappe.qb.from_("DocType") + .select(PseudoColumn("'DocType:'"), "name") + .run() + ) + messages += ( + frappe.qb.from_("Role").select(PseudoColumn("'Role:'"), "name").run() + ) + messages += ( + frappe.qb.from_("Module Def") + .select(PseudoColumn("'Module:'"), "name") + .run() + ) + messages += ( + frappe.qb.from_("Workspace Shortcut") + .where(Field("format").isnotnull()) + .select(PseudoColumn("''"), "format") + .run() + ) + messages += ( + frappe.qb.from_("Onboarding Step") + .select(PseudoColumn("''"), "title") + .run() + ) messages = deduplicate_messages(messages) message_dict = make_dict_from_messages(messages, load_user_translation=False) From 9df93d341bbde1e24135accc64066ea5d4715832 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 18 Oct 2021 18:56:08 +0530 Subject: [PATCH 12/26] fix: ignore `flags` in `frappe.client.bulk_update` --- frappe/client.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 21d10e8271..39f49e23fc 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -276,18 +276,17 @@ def bulk_update(docs): docs = json.loads(docs) failed_docs = [] for doc in docs: + doc.pop("flags", None) try: - ddoc = {key: val for key, val in doc.items() if key not in ['doctype', 'docname']} - doctype = doc['doctype'] - docname = doc['docname'] - doc = frappe.get_doc(doctype, docname) - doc.update(ddoc) - doc.save() - except: + existing_doc = frappe.get_doc(doc.pop("doctype"), doc.pop("docname")) + existing_doc.update(doc) + existing_doc.save() + except Exception: failed_docs.append({ 'doc': doc, 'exc': frappe.utils.get_traceback() }) + return {'failed_docs': failed_docs} @frappe.whitelist() From a4772d79231f551a016c60bdfc58da962892ff4f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 21 Oct 2021 16:56:51 +0530 Subject: [PATCH 13/26] fix: remove debug statement --- frappe/core/doctype/report/report.py | 2 +- frappe/core/doctype/user_type/user_type.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 6a54314667..be0346d869 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -105,7 +105,7 @@ class Report(Document): if not self.query.lower().startswith("select"): frappe.throw(_("Query must be a SELECT"), title=_('Report Document Error')) - result = [list(t) for t in frappe.db.sql(self.query, filters, debug=True)] + result = [list(t) for t in frappe.db.sql(self.query, filters)] columns = self.get_columns() or [cstr(c[0]) for c in frappe.db.get_description()] return [columns, result] diff --git a/frappe/core/doctype/user_type/user_type.py b/frappe/core/doctype/user_type/user_type.py index 79a90933e7..c1fd678141 100644 --- a/frappe/core/doctype/user_type/user_type.py +++ b/frappe/core/doctype/user_type/user_type.py @@ -195,7 +195,7 @@ def get_user_linked_doctypes(doctype, txt, searchfield, start, page_len, filters ['DocType', 'read_only', '=', 0], ['DocType', 'name', 'like', '%{0}%'.format(txt)]] doctypes = frappe.get_all('DocType', fields = ['`tabDocType`.`name`'], filters=filters, - order_by = '`tabDocType`.`idx` desc', limit_start=start, limit_page_length=page_len, as_list=1, debug=1) + order_by = '`tabDocType`.`idx` desc', limit_start=start, limit_page_length=page_len, as_list=1) custom_dt_filters = [['Custom Field', 'dt', 'like', '%{0}%'.format(txt)], ['Custom Field', 'options', '=', 'User'], ['Custom Field', 'fieldtype', '=', 'Link']] From 551f68d85a4eff50837fdae4f1e9760add78489a Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Thu, 21 Oct 2021 17:01:42 +0530 Subject: [PATCH 14/26] feat: `setDefault` for JS Objects --- frappe/public/js/frappe/utils/utils.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index b49dfa0280..2baff996c6 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -23,6 +23,14 @@ if (!Array.prototype.uniqBy) { }); } +// Python's dict.setdefault ported for JS objects +Object.defineProperty(Object.prototype, "setDefault", { + value: function(key, default_value) { + if (!(key in this)) this[key] = default_value; + return this[key]; + } +}); + // Pluralize String.prototype.plural = function(revert) { const plural = { From f2b319920ce17143159fd2c5232b2867038b5887 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Thu, 21 Oct 2021 17:24:46 +0530 Subject: [PATCH 15/26] refactor: improved design for fetching values using `add_fetch` --- cypress/integration/control_link.js | 8 +- frappe/desk/form/utils.py | 38 --------- frappe/public/js/frappe/form/controls/link.js | 80 ++++++++++--------- frappe/public/js/frappe/form/form.js | 24 ++++-- .../public/js/frappe/form/script_manager.js | 2 +- 5 files changed, 65 insertions(+), 87 deletions(-) diff --git a/cypress/integration/control_link.js b/cypress/integration/control_link.js index 7d44a71d06..957ee86855 100644 --- a/cypress/integration/control_link.js +++ b/cypress/integration/control_link.js @@ -49,19 +49,19 @@ context('Control Link', () => { it('should unset invalid value', () => { get_dialog_with_link().as('dialog'); - cy.intercept('GET', '/api/method/frappe.desk.form.utils.validate_link*').as('validate_link'); + cy.intercept('GET', '/api/method/frappe.client.get_value*').as('get_value'); cy.get('.frappe-control[data-fieldname=link] input') .type('invalid value', { delay: 100 }) .blur(); - cy.wait('@validate_link'); + cy.wait('@get_value'); cy.get('.frappe-control[data-fieldname=link] input').should('have.value', ''); }); it('should route to form on arrow click', () => { get_dialog_with_link().as('dialog'); - cy.intercept('GET', '/api/method/frappe.desk.form.utils.validate_link*').as('validate_link'); + cy.intercept('GET', '/api/method/frappe.client.get_value*').as('get_value'); cy.intercept('POST', '/api/method/frappe.desk.search.search_link').as('search_link'); cy.get('@todos').then(todos => { @@ -69,7 +69,7 @@ context('Control Link', () => { cy.get('@input').focus(); cy.wait('@search_link'); cy.get('@input').type(todos[0]).blur(); - cy.wait('@validate_link'); + cy.wait('@get_value'); cy.get('@input').focus(); cy.findByTitle('Open Link') .should('be.visible') diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index 1c954edff0..291767de10 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -16,44 +16,6 @@ def remove_attach(): file_name = frappe.form_dict.get('file_name') frappe.delete_doc('File', fid) -@frappe.whitelist() -def validate_link(): - """validate link when updated by user""" - import frappe - import frappe.utils - - value, options, fetch = frappe.form_dict.get('value'), frappe.form_dict.get('options'), frappe.form_dict.get('fetch') - - # no options, don't validate - if not options or options=='null' or options=='undefined': - frappe.response['message'] = 'Ok' - return - - valid_value = frappe.db.get_all(options, filters=dict(name=value), as_list=1, limit=1) - - if valid_value: - valid_value = valid_value[0][0] - - # get fetch values - if fetch: - # escape with "`" - fetch = ", ".join(("`{0}`".format(f.strip()) for f in fetch.split(","))) - fetch_value = None - try: - fetch_value = frappe.db.sql("select %s from `tab%s` where name=%s" - % (fetch, options, '%s'), (value,))[0] - except Exception as e: - error_message = str(e).split("Unknown column '") - fieldname = None if len(error_message)<=1 else error_message[1].split("'")[0] - frappe.msgprint(_("Wrong fieldname {0} in add_fetch configuration of custom client script").format(fieldname)) - frappe.errprint(frappe.get_traceback()) - - if fetch_value: - frappe.response['fetch_values'] = [frappe.utils.parse_val(c) for c in fetch_value] - - frappe.response['valid_value'] = valid_value - frappe.response['message'] = 'Ok' - @frappe.whitelist() def add_comment(reference_doctype, reference_name, content, comment_email, comment_by): diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index 83f3f8dd70..e7339372b3 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -451,51 +451,55 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat return this.validate_link_and_fetch(this.df, this.get_options(), this.docname, value); } - validate_link_and_fetch(df, doctype, docname, value) { - if(value) { - return new Promise((resolve) => { - var fetch = ''; - if(this.frm && this.frm.fetch_dict[df.fieldname]) { - fetch = this.frm.fetch_dict[df.fieldname].columns.join(', '); - } - // if default and no fetch, no need to validate - if (!fetch && df.__default_value && df.__default_value===value) { - resolve(value); - } + validate_link_and_fetch(df, options, docname, value) { + if (!value) return; - this.fetch_and_validate_link(resolve, df, doctype, docname, value, fetch); - }); - } - } + return new Promise((resolve) => { + const fetch_map = this.fetch_map; - fetch_and_validate_link(resolve, df, doctype, docname, value, fetch) { - frappe.call({ - method: 'frappe.desk.form.utils.validate_link', - type: "GET", - args: { - 'value': value, - 'options': doctype, - 'fetch': fetch - }, - no_spinner: true, - callback: (r) => { - if (r.message=='Ok') { - if (r.fetch_values && docname) { - this.set_fetch_values(df, docname, r.fetch_values); - } - resolve(r.valid_value); - } else { - resolve(""); - } + // if default and no fetch, no need to validate + if ($.isEmptyObject(fetch_map) && df.__default_value === value) { + return resolve(value); } + + frappe.db.get_value( + options, + value, + ["name", ...Object.values(fetch_map)], + (response) => { + if (!response.name) { + return resolve(""); + } + + if (docname) { + for (const [target_field, source_field] of Object.entries(fetch_map)) { + frappe.model.set_value( + df.parent, + docname, + target_field, + response[source_field], + df.fieldtype, + ); + } + } + + return resolve(response.name); + } + ) }); } - set_fetch_values(df, docname, fetch_values) { - var fl = this.frm.fetch_dict[df.fieldname].fields; - for(var i=0; i < fl.length; i++) { - frappe.model.set_value(df.parent, docname, fl[i], fetch_values[i], df.fieldtype); + get fetch_map() { + const fetch_map = {}; + if (!this.frm) return fetch_map; + + for (const key of ["*", this.df.parent]) { + if (this.frm.fetch_dict[key] && this.frm.fetch_dict[key][this.df.fieldname]) { + Object.assign(fetch_map, this.frm.fetch_dict[key][this.df.fieldname]); + } } + + return fetch_map; } }; diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index a095956dfe..75d68b12db 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1112,12 +1112,24 @@ frappe.ui.form.Form = class FrappeForm { } // UTILITIES - add_fetch(link_field, src_field, tar_field) { - if(!this.fetch_dict[link_field]) { - this.fetch_dict[link_field] = {'columns':[], 'fields':[]}; - } - this.fetch_dict[link_field].columns.push(src_field); - this.fetch_dict[link_field].fields.push(tar_field); + add_fetch(link_field, source_field, target_field, target_doctype) { + /* + Example fetch dict to get sender_email from email_id field in sender: + { + "Notification": { + "sender": { + "sender_email": "email_id" + } + } + } + */ + + if (!target_doctype) target_doctype = "*"; + + // Target field kept as key because source field could be non-unique + this.fetch_dict + .setDefault(target_doctype, {}) + .setDefault(link_field, {})[target_field] = source_field; } has_perm(ptype) { diff --git a/frappe/public/js/frappe/form/script_manager.js b/frappe/public/js/frappe/form/script_manager.js index f877a7cf8b..d1732ee702 100644 --- a/frappe/public/js/frappe/form/script_manager.js +++ b/frappe/public/js/frappe/form/script_manager.js @@ -196,7 +196,7 @@ frappe.ui.form.ScriptManager = class ScriptManager { 'Text Editor', 'Code', 'Link', 'Float', 'Int', 'Date', 'Select', 'Duration'].includes(df.fieldtype) || df.read_only==1) && df.fetch_from && df.fetch_from.indexOf(".")!=-1) { var parts = df.fetch_from.split("."); - me.frm.add_fetch(parts[0], parts[1], df.fieldname); + me.frm.add_fetch(parts[0], parts[1], df.fieldname, df.parent); } } From 7aa8059b579cfdaab0f1a1d750cb38e3e1975ad4 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Thu, 21 Oct 2021 19:11:17 +0530 Subject: [PATCH 16/26] test: add UI test to ensure `fetch_from` works --- cypress/integration/control_link.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cypress/integration/control_link.js b/cypress/integration/control_link.js index 957ee86855..2a81338c59 100644 --- a/cypress/integration/control_link.js +++ b/cypress/integration/control_link.js @@ -77,4 +77,19 @@ context('Control Link', () => { cy.location('pathname').should('eq', `/app/todo/${todos[0]}`); }); }); + + it('should fetch valid value', () => { + cy.get('@todos').then(todos => { + cy.visit(`/app/todo/${todos[0]}`); + cy.intercept('GET', '/api/method/frappe.client.get_value*').as('get_value'); + + cy.get('.frappe-control[data-fieldname=assigned_by] input').focus().as('input'); + cy.get('@input').type('Administrator', {delay: 100}).blur(); + cy.wait('@get_value'); + cy.get('.frappe-control[data-fieldname=assigned_by_full_name] .control-value').should( + 'contain', 'Administrator' + ); + }); + }); + }); From 128728a0c295f4df8ec3d568e423bf9b2795f374 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 21 Oct 2021 23:32:12 +0530 Subject: [PATCH 17/26] fix: debounce awesome_bar search When system language is not english each call to search translates all doctypes, reports etc to match them. This is computationally intensive and freezes DOM while typing. Debouncing to make it usable for now. Ideal solution: cache translations of standard objects somehow / debounce + async. refer: https://github.com/frappe/frappe/issues/13914 - replaced manual implementation of debouncing with frappe.utils.debounce - increased timeout to 500ms. - assuming ~30 wpm and 5 letter / word ~= 0.4 second / letter. --- .../js/frappe/ui/toolbar/awesome_bar.js | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js index 952fd62aa1..6d1d7228e3 100644 --- a/frappe/public/js/frappe/ui/toolbar/awesome_bar.js +++ b/frappe/public/js/frappe/ui/toolbar/awesome_bar.js @@ -50,38 +50,31 @@ frappe.search.AwesomeBar = class AwesomeBar { this.awesomplete = awesomplete; - $input.on("input", function(e) { + $input.on("input", frappe.utils.debounce(function(e) { var value = e.target.value; var txt = value.trim().replace(/\s\s+/g, ' '); var last_space = txt.lastIndexOf(' '); me.global_results = []; - // if(txt && txt.length > 1) { - // me.global.get_awesome_bar_options(txt.toLowerCase(), me); - // } - var $this = $(this); - clearTimeout($this.data('timeout')); + me.options = []; - $this.data('timeout', setTimeout(function(){ - me.options = []; - if(txt && txt.length > 1) { - if(last_space !== -1) { - me.set_specifics(txt.slice(0,last_space), txt.slice(last_space+1)); - } - me.add_defaults(txt); - me.options = me.options.concat(me.build_options(txt)); - me.options = me.options.concat(me.global_results); - } else { - me.options = me.options.concat( - me.deduplicate(frappe.search.utils.get_recent_pages(txt || ""))); - me.options = me.options.concat(frappe.search.utils.get_frequent_links()); + if (txt && txt.length > 1) { + if (last_space !== -1) { + me.set_specifics(txt.slice(0, last_space), txt.slice(last_space+1)); } - me.add_help(); + me.add_defaults(txt); + me.options = me.options.concat(me.build_options(txt)); + me.options = me.options.concat(me.global_results); + } else { + me.options = me.options.concat( + me.deduplicate(frappe.search.utils.get_recent_pages(txt || ""))); + me.options = me.options.concat(frappe.search.utils.get_frequent_links()); + } + me.add_help(); - awesomplete.list = me.deduplicate(me.options); - }, 100)); + awesomplete.list = me.deduplicate(me.options); - }); + }, 500)); var open_recent = function() { if (!this.autocomplete_open) { From 012edfe090006303897ca6a28c0f09412ec18f97 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 22 Oct 2021 01:15:46 +0530 Subject: [PATCH 18/26] refactor: made style changes & fixed query conversions --- frappe/sessions.py | 12 ++++++------ frappe/translate.py | 32 +++++++++++--------------------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/frappe/sessions.py b/frappe/sessions.py index 05f3bba3cf..72a439ab51 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -16,8 +16,8 @@ import frappe.translate import redis from urllib.parse import unquote from frappe.cache_manager import clear_user_cache -from frappe.query_builder import Order -from frappe.query_builder import DocType +from frappe.query_builder import Order, DocType + @frappe.whitelist(allow_guest=True) def clear(user=None): @@ -63,12 +63,12 @@ def get_sessions_to_clear(user=None, keep_current=False, device=None): simultaneous_sessions = frappe.db.get_value('User', user, 'simultaneous_sessions') or 1 offset = simultaneous_sessions - 1 - table = frappe.qb.DocType("Sessions") - criterion = frappe.qb.from_(table).where((table.user == user) & (table.device.isin(device))) + session = DocType("Sessions") + session_id = frappe.qb.from_(session).where((session.user == user) & (session.device.isin(device))) if keep_current: - criterion = criterion.where(table.sid != frappe.db.escape(frappe.session.sid)) + session_id = session_id.where(session.sid != frappe.db.escape(frappe.session.sid)) - query = criterion.select(table.sid).offset(offset).limit(100).orderby(table.lastupdate, order=Order.desc) + query = session_id.select(session.sid).offset(offset).limit(100).orderby(session.lastupdate, order=Order.desc) return frappe.db.sql_list(query) diff --git a/frappe/translate.py b/frappe/translate.py index d0208bd379..e5c1c9ef10 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -121,7 +121,7 @@ def set_default_language(lang): def get_lang_dict(): """Returns all languages in dict format, full name is the key e.g. `{"english":"en"}`""" - result = frappe.get_all("Language", fields=["language_name", "name"], order_by="modified", as_list=True) + result = dict(frappe.get_all("Language", fields=["language_name", "name"], order_by="modified", as_list=True)) return result def get_dict(fortype, name=None): @@ -156,33 +156,23 @@ def get_dict(fortype, name=None): messages += get_messages_from_include_files() messages += ( frappe.qb.from_("Print Format") - .select(PseudoColumn("'Print Format:'"), "name") - .run() - ) + .select(PseudoColumn("'Print Format:'"), "name")).run() messages += ( frappe.qb.from_("DocType") - .select(PseudoColumn("'DocType:'"), "name") - .run() - ) + .select(PseudoColumn("'DocType:'"), "name")).run() messages += ( frappe.qb.from_("Role").select(PseudoColumn("'Role:'"), "name").run() ) messages += ( frappe.qb.from_("Module Def") - .select(PseudoColumn("'Module:'"), "name") - .run() - ) + .select(PseudoColumn("'Module:'"), "name")).run() messages += ( frappe.qb.from_("Workspace Shortcut") .where(Field("format").isnotnull()) - .select(PseudoColumn("''"), "format") - .run() - ) + .select(PseudoColumn("''"), "format")).run() messages += ( frappe.qb.from_("Onboarding Step") - .select(PseudoColumn("''"), "title") - .run() - ) + .select(PseudoColumn("''"), "title")).run() messages = deduplicate_messages(messages) message_dict = make_dict_from_messages(messages, load_user_translation=False) @@ -349,17 +339,17 @@ def get_messages_for_app(app, deduplicate=True): # doctypes if modules: - names = frappe.qb.from_("DocType").where( + filtered_doctypes = frappe.qb.from_("DocType").where( Field("module").isin(modules) ).select("name").run() - for name in names: + for name in filtered_doctypes: messages.extend(get_messages_from_doctype(name)) # pages - result = frappe.qb.from_("Page").where( + filtered_pages = frappe.qb.from_("Page").where( Field("module").isin(modules) ).select("name", "title").run() - for name, title in result: + for name, title in filtered_pages: messages.append((None, title or name)) messages.extend(get_messages_from_page(name)) @@ -928,7 +918,7 @@ def get_translator_url(): def get_all_languages(with_language_name=False): """Returns all language codes ar, ch etc""" def get_language_codes(): - return frappe.get_all("Language", pluck="name", order_by="modified") + return frappe.get_all("Language", pluck="name") def get_all_language_with_name(): return frappe.db.get_all('Language', ['language_code', 'language_name']) From e46b1d11164ea605b48095198125c03a90adb639 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 22 Oct 2021 12:44:33 +0530 Subject: [PATCH 19/26] feat: Added pluck in sql to mirror sql_list --- frappe/database/database.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index e98cc22f41..a7efeb4b20 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -83,7 +83,8 @@ class Database(object): pass def sql(self, query, values=(), as_dict = 0, as_list = 0, formatted = 0, - debug=0, ignore_ddl=0, as_utf8=0, auto_commit=0, update=None, explain=False, run=True): + debug=0, ignore_ddl=0, as_utf8=0, auto_commit=0, update=None, + explain=False, run=True, pluck=False): """Execute a SQL query and fetch all rows. :param query: SQL query. @@ -178,6 +179,9 @@ class Database(object): if not self._cursor.description: return () + if pluck: + return [r[0] for r in self._cursor.fetchall()] + # scrub output if required if as_dict: ret = self.fetch_as_dict(formatted, as_utf8) @@ -233,7 +237,7 @@ class Database(object): except Exception: frappe.errprint("error in query explain") - def sql_list(self, query, values=(), debug=False): + def sql_list(self, query, values=(), debug=False, **kwargs): """Return data as list of single elements (first column). Example: @@ -241,7 +245,7 @@ class Database(object): # doctypes = ["DocType", "DocField", "User", ...] doctypes = frappe.db.sql_list("select name from DocType") """ - return [r[0] for r in self.sql(query, values, debug=debug)] + return [r[0] for r in self.sql(query, values, **kwargs, debug=debug)] def sql_ddl(self, query, values=(), debug=False): """Commit and execute a query. DDL (Data Definition Language) queries that alter schema From 616d97ce78d9ed41dc2ec8d3819f1e86461b9060 Mon Sep 17 00:00:00 2001 From: Umair Sayed Date: Fri, 22 Oct 2021 12:47:30 +0530 Subject: [PATCH 20/26] Changed default for Email Log from 90 to 30 days (#14543) --- frappe/core/doctype/log_settings/log_settings.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/log_settings/log_settings.json b/frappe/core/doctype/log_settings/log_settings.json index 8a2596b35c..f06d14f16b 100644 --- a/frappe/core/doctype/log_settings/log_settings.json +++ b/frappe/core/doctype/log_settings/log_settings.json @@ -49,7 +49,7 @@ "label": "Clear Activity Log After" }, { - "default": "90", + "default": "30", "description": "In Days", "fieldname": "clear_email_queue_after", "fieldtype": "Int", @@ -80,4 +80,4 @@ "sort_field": "modified", "sort_order": "DESC", "track_changes": 1 -} \ No newline at end of file +} From 0410a88aea03b70113b602aa45eb06f805339350 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 22 Oct 2021 13:31:36 +0530 Subject: [PATCH 21/26] refactor: replaced sql_list --- frappe/sessions.py | 4 ++-- frappe/share.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/sessions.py b/frappe/sessions.py index 72a439ab51..05d7f0238e 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -70,7 +70,7 @@ def get_sessions_to_clear(user=None, keep_current=False, device=None): query = session_id.select(session.sid).offset(offset).limit(100).orderby(session.lastupdate, order=Order.desc) - return frappe.db.sql_list(query) + return query.run(pluck=True) def delete_session(sid=None, user=None, reason="Session Expired"): from frappe.core.doctype.activity_log.feed import logout_feed @@ -92,7 +92,7 @@ def clear_all_sessions(reason=None): """This effectively logs out all users""" frappe.only_for("Administrator") if not reason: reason = "Deleted All Active Session" - for sid in [r[0] for r in frappe.qb.from_("Sessions").select("sid").run()]: + for sid in frappe.qb.from_("Sessions").select("sid").run(pluck=True): delete_session(sid, reason=reason) def get_expired_sessions(): diff --git a/frappe/share.py b/frappe/share.py index 4d43990c54..9b33198c9b 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -132,7 +132,7 @@ def get_shared_doctypes(user=None): query = frappe.qb.from_(table).where( (table.user == user) | (table.everyone == 1) ).select(table.share_doctype).distinct() - return frappe.db.sql_list(query) + return query.run(pluck=True) def get_share_name(doctype, name, user, everyone): if cint(everyone): From cb66e6508d92be0d20cf20d3b932cc8fefea8858 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 22 Oct 2021 14:17:52 +0530 Subject: [PATCH 22/26] refactor(minor): Use pluck & better var naming --- frappe/permissions.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index 0585b1e220..e21cec982e 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -353,10 +353,10 @@ def get_roles(user=None, with_standard=True): return frappe.get_all("Role", pluck="name") # return all available roles else: table = DocType("Has Role") - result = frappe.qb.from_(table).where( + roles = frappe.qb.from_(table).where( (table.parent == user) & (table.role.notin(["All", "Guest"])) - ).select(table.role).run() - return [r[0] for r in result] + ['All', 'Guest'] + ).select(table.role).run(pluck=True) + return roles + ['All', 'Guest'] roles = frappe.cache().hget("roles", user, get) From 5996d6829dff88400b16c4b3bb747188d9834a86 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 22 Oct 2021 14:33:18 +0530 Subject: [PATCH 23/26] test: fix flaky awesome_bar test --- cypress/integration/awesome_bar.js | 11 ++++++----- cypress/integration/recorder.js | 7 ------- cypress/support/commands.js | 4 ++-- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/cypress/integration/awesome_bar.js b/cypress/integration/awesome_bar.js index fb09b384a8..8e503cce46 100644 --- a/cypress/integration/awesome_bar.js +++ b/cypress/integration/awesome_bar.js @@ -7,12 +7,13 @@ context('Awesome Bar', () => { beforeEach(() => { cy.get('.navbar .navbar-home').click(); + cy.findByPlaceholderText('Search or type a command (Ctrl + G)').clear(); }); it('navigates to doctype list', () => { - cy.findByPlaceholderText('Search or type a command (Ctrl + G)').type('todo', { delay: 200 }); + cy.findByPlaceholderText('Search or type a command (Ctrl + G)').type('todo', { delay: 700 }); cy.get('.awesomplete').findByRole('listbox').should('be.visible'); - cy.findByPlaceholderText('Search or type a command (Ctrl + G)').type('{downarrow}{enter}', { delay: 100 }); + cy.findByPlaceholderText('Search or type a command (Ctrl + G)').type('{downarrow}{enter}', { delay: 700 }); cy.get('.title-text').should('contain', 'To Do'); @@ -21,7 +22,7 @@ context('Awesome Bar', () => { it('find text in doctype list', () => { cy.findByPlaceholderText('Search or type a command (Ctrl + G)') - .type('test in todo{downarrow}{enter}', { delay: 200 }); + .type('test in todo{downarrow}{enter}', { delay: 700 }); cy.get('.title-text').should('contain', 'To Do'); @@ -31,14 +32,14 @@ context('Awesome Bar', () => { it('navigates to new form', () => { cy.findByPlaceholderText('Search or type a command (Ctrl + G)') - .type('new blog post{downarrow}{enter}', { delay: 200 }); + .type('new blog post{downarrow}{enter}', { delay: 700 }); cy.get('.title-text:visible').should('have.text', 'New Blog Post'); }); it('calculates math expressions', () => { cy.findByPlaceholderText('Search or type a command (Ctrl + G)') - .type('55 + 32{downarrow}{enter}', { delay: 200 }); + .type('55 + 32{downarrow}{enter}', { delay: 700 }); cy.get('.modal-title').should('contain', 'Result'); cy.get('.msgprint').should('contain', '55 + 32 = 87'); diff --git a/cypress/integration/recorder.js b/cypress/integration/recorder.js index 7a62b2e6d9..caf1349e6e 100644 --- a/cypress/integration/recorder.js +++ b/cypress/integration/recorder.js @@ -13,13 +13,6 @@ context('Recorder', () => { }); }); - it('Navigate to Recorder', () => { - cy.visit('/app'); - cy.awesomebar('recorder'); - cy.findByTitle('Recorder').should('exist'); - cy.url().should('include', '/recorder/detail'); - }); - it('Recorder Empty State', () => { cy.findByTitle('Recorder').should('exist'); diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 6484370946..64a3b18b2f 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -241,7 +241,7 @@ Cypress.Commands.add('get_table_field', (tablefieldname, row_idx, fieldname, fie }); Cypress.Commands.add('awesomebar', text => { - cy.get('#navbar-search').type(`${text}{downarrow}{enter}`, {delay: 100}); + cy.get('#navbar-search').type(`${text}{downarrow}{enter}`, {delay: 700}); }); Cypress.Commands.add('new_form', doctype => { @@ -354,4 +354,4 @@ Cypress.Commands.add('click_listview_primary_button', (btn_name) => { Cypress.Commands.add('click_timeline_action_btn', (btn_name) => { cy.get('.timeline-message-box .custom-actions > .btn').contains(btn_name).click(); -}); \ No newline at end of file +}); From f08faf151c48a3f65b0dabb94ec2e986cf5fb515 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 22 Oct 2021 18:18:14 +0530 Subject: [PATCH 24/26] test(website): Clear db.value_cache on tearDown --- frappe/tests/test_website.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 688679a80f..25aa7b31ce 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -4,14 +4,13 @@ import frappe from frappe.utils import set_request from frappe.website.serve import get_response, get_response_content from frappe.website.utils import (build_response, clear_website_cache, get_home_page) -from tenacity import retry, stop_after_attempt, retry_if_exception_type - class TestWebsite(unittest.TestCase): def setUp(self): frappe.set_user('Guest') def tearDown(self): + frappe.db.value_cache = {} frappe.set_user('Administrator') def test_home_page(self): @@ -197,13 +196,8 @@ class TestWebsite(unittest.TestCase): delattr(frappe.hooks, 'page_renderer') frappe.cache().delete_key('app_hooks') - # TODO: Get rid of this retry logic - # Added since test is flaky and we can't figure out why at this point - @retry( - stop=stop_after_attempt(5), retry=retry_if_exception_type(AssertionError), - ) def test_printview_page(self): - content = get_response_content('/Language/en') + content = get_response_content('/Language/ru') self.assertIn('