From e887208b27751611c58177c6412e7a6d869431b9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 18:32:23 +0530 Subject: [PATCH 1/5] refactor: deferred_insert * Fallback to insert in realtime if redis conn fails * Allow List and str inputs for deferred insert API * Set doctype value unconditionally in insert_record * Use frappe.logger instead of print * Add type hints --- frappe/deferred_insert.py | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/frappe/deferred_insert.py b/frappe/deferred_insert.py index b1338a73b0..f9f1b1c777 100644 --- a/frappe/deferred_insert.py +++ b/frappe/deferred_insert.py @@ -1,12 +1,25 @@ import json +from typing import Dict, List, Union, TYPE_CHECKING import frappe - +import redis from frappe.utils import cstr +if TYPE_CHECKING: + from frappe.model.document import Document + queue_prefix = 'insert_queue_for_' -def deferred_insert(doctype, records): - frappe.cache().rpush(queue_prefix + doctype, records) +def deferred_insert(doctype: str, records: Union[List[Union[Dict, "Document"]], str]): + if isinstance(records, (dict, list)): + _records = json.dumps(records) + else: + _records = records + + try: + frappe.cache().rpush(f"{queue_prefix}{doctype}", _records) + except redis.exceptions.ConnectionError: + for record in records: + insert_record(record, doctype) def save_to_db(): queue_keys = frappe.cache().get_keys(queue_prefix) @@ -27,17 +40,15 @@ def save_to_db(): frappe.db.commit() -def insert_record(record, doctype): - if not record.get('doctype'): - record['doctype'] = doctype +def insert_record(record: Union[Dict, "Document"], doctype: str): + setattr(record, "doctype", doctype) try: - doc = frappe.get_doc(record) - doc.insert() + frappe.get_doc(record).insert() except Exception as e: - print(e, doctype) + frappe.logger().error(f"Error while inserting deferred {doctype} record: {e}") -def get_key_name(key): +def get_key_name(key: str) -> str: return cstr(key).split('|')[1] -def get_doctype_name(key): +def get_doctype_name(key: str) -> str: return cstr(key).split(queue_prefix)[1] From da5a5eda024954ea577687ed9728a01319ce419b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 12 Apr 2022 12:00:11 +0530 Subject: [PATCH 2/5] fix: incorrect type in make_property_setter query (#16584) * fix: incorrect type in make_property_setter query * fix: Remove redundant as_list kwarg Co-authored-by: gavin --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 32c8a75a2d..37c282f04a 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1536,7 +1536,7 @@ def make_property_setter( .select(DocField_doctype.parent) .where(DocField_doctype.fieldname == args.fieldname) .distinct() - ).run(as_list=True) + ).run(pluck=True) else: doctype_list = [args.doctype] From f6c879ab1d6f92a13c34fd3a2237121c67846089 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 18:38:36 +0530 Subject: [PATCH 3/5] refactor: Route History * Use newer deferred_insert APIs * Refactor query to use QB notation * Styled with black-ish --- .../doctype/route_history/route_history.py | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/frappe/desk/doctype/route_history/route_history.py b/frappe/desk/doctype/route_history/route_history.py index f0aa867c8a..bb10fb08f2 100644 --- a/frappe/desk/doctype/route_history/route_history.py +++ b/frappe/desk/doctype/route_history/route_history.py @@ -1,11 +1,11 @@ -# Copyright (c) 2021, Frappe Technologies and contributors +# Copyright (c) 2022, Frappe Technologies and contributors # License: MIT. See LICENSE -import json - import frappe from frappe.deferred_insert import deferred_insert as _deferred_insert from frappe.model.document import Document +from frappe.query_builder.utils import DocType +from frappe.query_builder.functions import Count class RouteHistory(Document): @@ -14,31 +14,31 @@ class RouteHistory(Document): def flush_old_route_records(): """Deletes all route records except last 500 records per user""" - records_to_keep_limit = 500 - users = frappe.db.sql(''' - SELECT `user` - FROM `tabRoute History` - GROUP BY `user` - HAVING count(`name`) > %(limit)s - ''', { - "limit": records_to_keep_limit - }) + RouteHistory = DocType("Route History") + + users = ( + frappe.qb.from_(RouteHistory) + .select(RouteHistory.user) + .groupby(RouteHistory.user) + .having(Count(RouteHistory.name) > records_to_keep_limit) + ).run(pluck=True) for user in users: - user = user[0] - last_record_to_keep = frappe.db.get_all('Route History', - filters={'user': user}, - limit=1, + last_record_to_keep = frappe.get_all( + "Route History", + filters={"user": user}, limit_start=500, - fields=['modified'], - order_by='modified desc' + fields=["modified"], + order_by="modified desc", + limit=1, + ) + + frappe.db.delete( + "Route History", + {"modified": ("<=", last_record_to_keep[0].modified), "user": user}, ) - frappe.db.delete("Route History", { - "modified": ("<=", last_record_to_keep[0].modified), - "user": user - }) @frappe.whitelist() def deferred_insert(routes): @@ -51,10 +51,16 @@ def deferred_insert(routes): for route in frappe.parse_json(routes) ] - _deferred_insert("Route History", json.dumps(routes)) + _deferred_insert("Route History", routes) + @frappe.whitelist() def frequently_visited_links(): - return frappe.get_all('Route History', fields=['route', 'count(name) as count'], filters={ - 'user': frappe.session.user - }, group_by="route", order_by="count desc", limit=5) + return frappe.get_all( + "Route History", + fields=["route", "count(name) as count"], + filters={"user": frappe.session.user}, + group_by="route", + order_by="count desc", + limit=5, + ) From a0bdf499b589ac91267ce7384b84a9cef1f9f3e8 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 12 Apr 2022 12:26:36 +0530 Subject: [PATCH 4/5] style: Resolve conflicts with linters post codebase format --- frappe/desk/doctype/route_history/route_history.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/route_history/route_history.py b/frappe/desk/doctype/route_history/route_history.py index 082bf84a47..e712a5bb11 100644 --- a/frappe/desk/doctype/route_history/route_history.py +++ b/frappe/desk/doctype/route_history/route_history.py @@ -4,8 +4,8 @@ import frappe from frappe.deferred_insert import deferred_insert as _deferred_insert from frappe.model.document import Document +from frappe.query_builder import DocType from frappe.query_builder.functions import Count -from frappe.query_builder.utils import DocType class RouteHistory(Document): From 18108eef65389faa90d824e243419937d4e3e7ad Mon Sep 17 00:00:00 2001 From: gavin Date: Tue, 12 Apr 2022 12:29:05 +0530 Subject: [PATCH 5/5] fix(desk): Use Int control for Long Int (#16572) Type `Long Int` doesn't have a JS control, we can use the Int's instead. Prior to this, forms of DocTypes with Long Int fields would just not render :trollface: Screenshot 2022-04-11 at 3 36 40 PM --- frappe/public/js/frappe/form/controls/int.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/public/js/frappe/form/controls/int.js b/frappe/public/js/frappe/form/controls/int.js index 12652bf86e..a7e6c9bc7e 100644 --- a/frappe/public/js/frappe/form/controls/int.js +++ b/frappe/public/js/frappe/form/controls/int.js @@ -41,3 +41,5 @@ frappe.ui.form.ControlInt = class ControlInt extends frappe.ui.form.ControlData return cint(this.eval_expression(value), null); } }; + +frappe.ui.form.ControlLongInt = frappe.ui.form.ControlInt;