From 2ef5e6bd1d2bba43de3c2b9ecb02838db88cd596 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 2 May 2023 17:40:09 +0200
Subject: [PATCH 01/42] fix: file permissions
---
frappe/core/doctype/file/file.json | 8 +-----
frappe/core/doctype/file/file.py | 44 +++++++++++++++---------------
frappe/hooks.py | 1 +
3 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/frappe/core/doctype/file/file.json b/frappe/core/doctype/file/file.json
index d6c4a99bc3..6c64bfe274 100644
--- a/frappe/core/doctype/file/file.json
+++ b/frappe/core/doctype/file/file.json
@@ -174,7 +174,7 @@
"icon": "fa fa-file",
"idx": 1,
"links": [],
- "modified": "2022-09-13 15:50:15.508251",
+ "modified": "2023-05-02 15:42:14.274901",
"modified_by": "Administrator",
"module": "Core",
"name": "File",
@@ -196,14 +196,8 @@
{
"create": 1,
"delete": 1,
- "email": 1,
- "export": 1,
- "if_owner": 1,
- "print": 1,
"read": 1,
- "report": 1,
"role": "All",
- "share": 1,
"write": 1
}
],
diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py
index 1323359030..2e88591f94 100755
--- a/frappe/core/doctype/file/file.py
+++ b/frappe/core/doctype/file/file.py
@@ -16,6 +16,7 @@ import frappe
from frappe import _
from frappe.database.schema import SPECIAL_CHAR_PATTERN
from frappe.model.document import Document
+from frappe.permissions import get_doctypes_with_read
from frappe.utils import call_hook_method, cint, get_files_path, get_hook_method, get_url
from frappe.utils.file_manager import is_safe_path
from frappe.utils.image import optimize_image, strip_exif_data
@@ -703,40 +704,39 @@ def on_doctype_update():
def has_permission(doc, ptype=None, user=None):
- has_access = False
user = user or frappe.session.user
if ptype == "create":
- has_access = frappe.has_permission("File", "create", user=user)
+ return frappe.has_permission("File", "create", user=user)
- if not doc.is_private or doc.owner in [user, "Guest"] or user == "Administrator":
- has_access = True
+ if not doc.is_private or doc.owner == user or user == "Administrator":
+ return True
if doc.attached_to_doctype and doc.attached_to_name:
attached_to_doctype = doc.attached_to_doctype
attached_to_name = doc.attached_to_name
- try:
- ref_doc = frappe.get_doc(attached_to_doctype, attached_to_name)
+ ref_doc = frappe.get_doc(attached_to_doctype, attached_to_name)
- if ptype in ["write", "create", "delete"]:
- has_access = ref_doc.has_permission("write")
+ if ptype in ["write", "create", "delete"]:
+ return ref_doc.has_permission("write")
+ else:
+ return ref_doc.has_permission("read")
- if ptype == "delete" and not has_access:
- frappe.throw(
- _(
- "Cannot delete file as it belongs to {0} {1} for which you do not have permissions"
- ).format(doc.attached_to_doctype, doc.attached_to_name),
- frappe.PermissionError,
- )
- else:
- has_access = ref_doc.has_permission("read")
- except frappe.DoesNotExistError:
- # if parent doc is not created before file is created
- # we cannot check its permission so we will use file's permission
- pass
+ return False
- return has_access
+
+def get_permission_query_conditions(user: str = None) -> str:
+ user = user or frappe.session.user
+ if user == "Administrator":
+ return ""
+
+ readable_doctypes = ", ".join(repr(dt) for dt in get_doctypes_with_read())
+ return f"""
+ (`tabFile`.`is_private` = 0)
+ OR (`tabFile`.`attached_to_doctype` IS NULL AND `tabFile`.`owner` = {user !r})
+ OR (`tabFile`.`attached_to_doctype` IN ({readable_doctypes}))
+ """
# Note: kept at the end to not cause circular, partial imports & maintain backwards compatibility
diff --git a/frappe/hooks.py b/frappe/hooks.py
index 5967486824..e30b300a58 100644
--- a/frappe/hooks.py
+++ b/frappe/hooks.py
@@ -108,6 +108,7 @@ permission_query_conditions = {
"Communication": "frappe.core.doctype.communication.communication.get_permission_query_conditions_for_communication",
"Workflow Action": "frappe.workflow.doctype.workflow_action.workflow_action.get_permission_query_conditions",
"Prepared Report": "frappe.core.doctype.prepared_report.prepared_report.get_permission_query_condition",
+ "File": "frappe.core.doctype.file.file.get_permission_query_conditions",
}
has_permission = {
From 9c785569f9d4d6cb3d8b03739d6c149f683ae947 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Wed, 3 May 2023 11:28:20 +0200
Subject: [PATCH 02/42] test: file permissions
---
frappe/core/doctype/file/test_file.py | 66 +++++++++++++++++++++------
1 file changed, 53 insertions(+), 13 deletions(-)
diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py
index 51e065f710..dbab111257 100644
--- a/frappe/core/doctype/file/test_file.py
+++ b/frappe/core/doctype/file/test_file.py
@@ -601,25 +601,27 @@ class TestAttachmentsAccess(FrappeTestCase):
def setUp(self) -> None:
frappe.db.delete("File", {"is_folder": 0})
- def test_attachments_access(self):
+ def test_list_private_attachments(self):
frappe.set_user("test4@example.com")
self.attached_to_doctype, self.attached_to_docname = make_test_doc()
frappe.get_doc(
{
"doctype": "File",
- "file_name": "test_user.txt",
+ "file_name": "test_user_attachment.txt",
"attached_to_doctype": self.attached_to_doctype,
"attached_to_name": self.attached_to_docname,
"content": "Testing User",
+ "is_private": 1,
}
).insert()
frappe.get_doc(
{
"doctype": "File",
- "file_name": "test_user_home.txt",
+ "file_name": "test_user_standalone.txt",
"content": "User Home",
+ "is_private": 1,
}
).insert()
@@ -628,18 +630,20 @@ class TestAttachmentsAccess(FrappeTestCase):
frappe.get_doc(
{
"doctype": "File",
- "file_name": "test_system_manager.txt",
+ "file_name": "test_sm_attachment.txt",
"attached_to_doctype": self.attached_to_doctype,
"attached_to_name": self.attached_to_docname,
"content": "Testing System Manager",
+ "is_private": 1,
}
).insert()
frappe.get_doc(
{
"doctype": "File",
- "file_name": "test_sm_home.txt",
+ "file_name": "test_sm_standalone.txt",
"content": "System Manager Home",
+ "is_private": 1,
}
).insert()
@@ -654,15 +658,51 @@ class TestAttachmentsAccess(FrappeTestCase):
file.file_name for file in get_files_in_folder("Home/Attachments")["files"]
]
- self.assertIn("test_sm_home.txt", system_manager_files)
- self.assertNotIn("test_sm_home.txt", user_files)
- self.assertIn("test_user_home.txt", system_manager_files)
- self.assertIn("test_user_home.txt", user_files)
+ self.assertIn("test_sm_standalone.txt", system_manager_files)
+ self.assertNotIn("test_sm_standalone.txt", user_files)
- self.assertIn("test_system_manager.txt", system_manager_attachments_files)
- self.assertNotIn("test_system_manager.txt", user_attachments_files)
- self.assertIn("test_user.txt", system_manager_attachments_files)
- self.assertIn("test_user.txt", user_attachments_files)
+ self.assertIn("test_user_standalone.txt", user_files)
+ self.assertNotIn("test_user_standalone.txt", system_manager_files)
+
+ self.assertIn("test_sm_attachment.txt", system_manager_attachments_files)
+ self.assertIn("test_sm_attachment.txt", user_attachments_files)
+ self.assertIn("test_user_attachment.txt", system_manager_attachments_files)
+ self.assertIn("test_user_attachment.txt", user_attachments_files)
+
+ def test_list_public_single_file(self):
+ """Ensure that users are able to list public standalone files."""
+ frappe.set_user("test@example.com")
+ frappe.get_doc(
+ {
+ "doctype": "File",
+ "file_name": "test_public_single.txt",
+ "content": "Public single File",
+ "is_private": 0,
+ }
+ ).insert()
+
+ frappe.set_user("test4@example.com")
+ files = [file.file_name for file in get_files_in_folder("Home")["files"]]
+ self.assertIn("test_public_single.txt", files)
+
+ def test_list_public_attachment(self):
+ """Ensure that users are able to list public attachments."""
+ frappe.set_user("test@example.com")
+ self.attached_to_doctype, self.attached_to_docname = make_test_doc()
+ frappe.get_doc(
+ {
+ "doctype": "File",
+ "file_name": "test_public_attachment.txt",
+ "attached_to_doctype": self.attached_to_doctype,
+ "attached_to_name": self.attached_to_docname,
+ "content": "Public Attachment",
+ "is_private": 0,
+ }
+ ).insert()
+
+ frappe.set_user("test4@example.com")
+ files = [file.file_name for file in get_files_in_folder("Home/Attachments")["files"]]
+ self.assertIn("test_public_attachment.txt", files)
def tearDown(self) -> None:
frappe.set_user("Administrator")
From bcdc483a13ec61f83aac4230ce262bd42e6b8fff Mon Sep 17 00:00:00 2001
From: Corentin Flr <10946971+cogk@users.noreply.github.com>
Date: Thu, 15 Jun 2023 18:36:30 +0200
Subject: [PATCH 03/42] fix(test): Fix test_never_render to get path as string,
exclude PYC files from static downloads
This test code never actually tested the behaviour for two reasons:
- first, the page had an error which meant that a 500 Error page was returned (because `path` is not a string)
- second, every page contains the string "400" because it's contained in some of the icons.svg icons!
I also found a minor related bug in static_page.py, allowing people to download PYC files (pycache)
---
frappe/tests/test_website.py | 5 +++--
frappe/website/page_renderers/static_page.py | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py
index 6c319fff0a..1031a46c80 100644
--- a/frappe/tests/test_website.py
+++ b/frappe/tests/test_website.py
@@ -340,8 +340,9 @@ class TestWebsite(FrappeTestCase):
FILES_TO_SKIP = choices(list(WWW.glob("**/*.py*")), k=10)
for suffix in FILES_TO_SKIP:
- content = get_response_content(suffix.relative_to(WWW))
- self.assertIn("404", content)
+ path: str = suffix.relative_to(WWW).as_posix()
+ content = get_response_content(path)
+ self.assertIn("
Not Found", content)
def test_metatags(self):
content = get_response_content("/_test/_test_metatags")
diff --git a/frappe/website/page_renderers/static_page.py b/frappe/website/page_renderers/static_page.py
index 04e58ff217..d6de2f2991 100644
--- a/frappe/website/page_renderers/static_page.py
+++ b/frappe/website/page_renderers/static_page.py
@@ -8,7 +8,7 @@ import frappe
from frappe.website.page_renderers.base_renderer import BaseRenderer
from frappe.website.utils import is_binary_file
-UNSUPPORTED_STATIC_PAGE_TYPES = ("html", "md", "js", "xml", "css", "txt", "py", "json")
+UNSUPPORTED_STATIC_PAGE_TYPES = ("html", "md", "js", "xml", "css", "txt", "py", "pyc", "json")
class StaticPage(BaseRenderer):
From 9afedfae253896be463d52efe35eac035ac5f313 Mon Sep 17 00:00:00 2001
From: Corentin Flr <10946971+cogk@users.noreply.github.com>
Date: Fri, 16 Jun 2023 13:27:49 +0200
Subject: [PATCH 04/42] fix(test): Remove frappe.local.request between requests
`frappe.local.request` was not cleared between tests, which would not be a problem if all tests did set it to another Request object. But, some tests directly fetch the response content using get_response_content without first setting the frappe.local.request object (using set_request).
---
frappe/tests/test_website.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py
index 1031a46c80..16e82850de 100644
--- a/frappe/tests/test_website.py
+++ b/frappe/tests/test_website.py
@@ -11,10 +11,16 @@ from frappe.website.utils import build_response, clear_website_cache, get_home_p
class TestWebsite(FrappeTestCase):
def setUp(self):
frappe.set_user("Guest")
+ self._clearRequest()
def tearDown(self):
frappe.db.delete("Access Log")
frappe.set_user("Administrator")
+ self._clearRequest()
+
+ def _clearRequest(self):
+ if hasattr(frappe.local, "request"):
+ delattr(frappe.local, "request")
def test_home_page(self):
frappe.set_user("Administrator")
From 23846434ee03223f2460fa95b55974c06c51ab8a Mon Sep 17 00:00:00 2001
From: Corentin Flr <10946971+cogk@users.noreply.github.com>
Date: Fri, 16 Jun 2023 14:39:03 +0200
Subject: [PATCH 05/42] fix(path_resolver): Avoid 200 OK for NotFoundPage
renderer
---
frappe/website/path_resolver.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/frappe/website/path_resolver.py b/frappe/website/path_resolver.py
index 37bfb3ee56..50ef2afcb4 100644
--- a/frappe/website/path_resolver.py
+++ b/frappe/website/path_resolver.py
@@ -51,7 +51,6 @@ class PathResolver:
TemplatePage,
ListPage,
PrintPage,
- NotFoundPage,
]
for renderer in renderers:
From 85ac64ddd9794f3f77115b25e51f441041ad0acc Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Mon, 19 Jun 2023 13:18:25 +0530
Subject: [PATCH 06/42] fix: log errors while getting headers and data
---
.../integrations/doctype/webhook/webhook.py | 21 ++++++++++++-------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py
index 6fa24bfb67..711a565971 100644
--- a/frappe/integrations/doctype/webhook/webhook.py
+++ b/frappe/integrations/doctype/webhook/webhook.py
@@ -112,16 +112,21 @@ def get_context(doc):
def enqueue_webhook(doc, webhook) -> None:
- webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name"))
- headers = get_webhook_headers(doc, webhook)
- data = get_webhook_data(doc, webhook)
+ try:
+ webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name"))
+ headers = get_webhook_headers(doc, webhook)
+ data = get_webhook_data(doc, webhook)
- if webhook.is_dynamic_url:
- request_url = frappe.render_template(webhook.request_url, get_context(doc))
- else:
- request_url = webhook.request_url
+ if webhook.is_dynamic_url:
+ request_url = frappe.render_template(webhook.request_url, get_context(doc))
+ else:
+ request_url = webhook.request_url
+
+ r = None
+ except Exception as e:
+ frappe.logger().debug({"enqueue_webhook_error": e})
+ log_request(webhook.name, doc.name, request_url, headers, data, r)
- r = None
for i in range(3):
try:
r = requests.request(
From 74cc21013fbd954229d98cf8d802ad56706562d3 Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Mon, 19 Jun 2023 13:46:27 +0530
Subject: [PATCH 07/42] fix: validate webhook secret
---
frappe/integrations/doctype/webhook/webhook.py | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py
index 711a565971..2431fe2886 100644
--- a/frappe/integrations/doctype/webhook/webhook.py
+++ b/frappe/integrations/doctype/webhook/webhook.py
@@ -26,6 +26,7 @@ class Webhook(Document):
self.validate_request_url()
self.validate_request_body()
self.validate_repeating_fields()
+ self.validate_secret()
self.preview_document = None
def on_update(self):
@@ -74,6 +75,13 @@ class Webhook(Document):
if len(webhook_data) != len(set(webhook_data)):
frappe.throw(_("Same Field is entered more than once"))
+ def validate_secret(self):
+ if self.enable_security and self.webhook_secret:
+ try:
+ self.get_password("webhook_secret", False).encode("utf8")
+ except Exception:
+ frappe.throw(_("Invalid Webhook Secret"))
+
@frappe.whitelist()
def generate_preview(self):
# This function doesn't need to do anything specific as virtual fields
From 3f792a80b1036a7e2c1c28cdd9f1e3f817968f42 Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Fri, 23 Jun 2023 16:29:39 +0530
Subject: [PATCH 08/42] fix: return if exception occur before executing webhook
---
frappe/integrations/doctype/webhook/webhook.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py
index 2431fe2886..c798c64c2c 100644
--- a/frappe/integrations/doctype/webhook/webhook.py
+++ b/frappe/integrations/doctype/webhook/webhook.py
@@ -130,10 +130,10 @@ def enqueue_webhook(doc, webhook) -> None:
else:
request_url = webhook.request_url
- r = None
except Exception as e:
frappe.logger().debug({"enqueue_webhook_error": e})
- log_request(webhook.name, doc.name, request_url, headers, data, r)
+ log_request(webhook.name, doc.name, request_url, headers, data)
+ return
for i in range(3):
try:
From fe27b8c7e0c622365fe2b034ca5b7bf37bcd7464 Mon Sep 17 00:00:00 2001
From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com>
Date: Fri, 23 Jun 2023 16:31:55 +0530
Subject: [PATCH 09/42] fix: removed redundant condition
Co-authored-by: Ankush Menat
---
frappe/integrations/doctype/webhook/webhook.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py
index c798c64c2c..5b8c653198 100644
--- a/frappe/integrations/doctype/webhook/webhook.py
+++ b/frappe/integrations/doctype/webhook/webhook.py
@@ -76,7 +76,7 @@ class Webhook(Document):
frappe.throw(_("Same Field is entered more than once"))
def validate_secret(self):
- if self.enable_security and self.webhook_secret:
+ if self.enable_security:
try:
self.get_password("webhook_secret", False).encode("utf8")
except Exception:
From e26152f0dc3fed4ca797a12c7086b61a3dc82207 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 26 Jun 2023 13:28:57 +0530
Subject: [PATCH 10/42] chore: use node18 for github workflow
[skip ci]
---
frappe/utils/boilerplate.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py
index 0e646f9992..e2aaa26f22 100644
--- a/frappe/utils/boilerplate.py
+++ b/frappe/utils/boilerplate.py
@@ -593,7 +593,7 @@ jobs:
- name: Setup Node
uses: actions/setup-node@v3
with:
- node-version: 16
+ node-version: 18
check-latest: true
- name: Cache pip
From ee74830460a13e1042a9f587de423d61b0b3594b Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 26 Jun 2023 17:32:02 +0530
Subject: [PATCH 11/42] chore: add github star CTA on server script sidebar
[skip ci]
---
.../server_script/server_script_list.js | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 frappe/core/doctype/server_script/server_script_list.js
diff --git a/frappe/core/doctype/server_script/server_script_list.js b/frappe/core/doctype/server_script/server_script_list.js
new file mode 100644
index 0000000000..0df447a0eb
--- /dev/null
+++ b/frappe/core/doctype/server_script/server_script_list.js
@@ -0,0 +1,38 @@
+frappe.listview_settings["Server Script"] = {
+ onload: function (listview) {
+ add_github_star_cta(listview);
+ },
+};
+
+function add_github_star_cta(listview) {
+ try {
+ const key = "show_github_star_banner";
+ if (localStorage.getItem(key) == "false") {
+ return;
+ }
+
+ if (listview.github_star_banner) {
+ listview.github_star_banner.remove();
+ }
+
+ const message = "Loving Frappe Framework?";
+ const link = "https://github.com/frappe/frappe";
+ const cta = "Star us on GitHub";
+
+ listview.github_star_banner = $(`
+
+ `).appendTo(listview.page.sidebar);
+ } catch (error) {
+ console.error(error);
+ }
+}
From ca95b591ae8581a48dfc28f3560670da36f8da0e Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 26 Jun 2023 17:36:53 +0530
Subject: [PATCH 12/42] refactor: Pass redis connection directly
---
frappe/utils/background_jobs.py | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index 6b0249e720..b2a3fbab24 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -9,7 +9,7 @@ from uuid import uuid4
import redis
from redis.exceptions import BusyLoadingError, ConnectionError
-from rq import Connection, Queue, Worker
+from rq import Queue, Worker
from rq.exceptions import NoSuchJobError
from rq.job import Job, JobStatus
from rq.logutils import setup_loghandlers
@@ -253,17 +253,16 @@ def start_worker(
WorkerKlass = DEQUEUE_STRATEGIES.get(strategy, Worker)
- with Connection(redis_connection):
- logging_level = "INFO"
- if quiet:
- logging_level = "WARNING"
- worker = WorkerKlass(queues, name=get_worker_name(queue_name))
- worker.work(
- logging_level=logging_level,
- burst=burst,
- date_format="%Y-%m-%d %H:%M:%S",
- log_format="%(asctime)s,%(msecs)03d %(message)s",
- )
+ logging_level = "INFO"
+ if quiet:
+ logging_level = "WARNING"
+ worker = WorkerKlass(queues, name=get_worker_name(queue_name), connection=redis_connection)
+ worker.work(
+ logging_level=logging_level,
+ burst=burst,
+ date_format="%Y-%m-%d %H:%M:%S",
+ log_format="%(asctime)s,%(msecs)03d %(message)s",
+ )
def get_worker_name(queue):
From 7fbc6e8175da239a230428d936b2af7f58ceeff9 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 26 Jun 2023 17:42:18 +0530
Subject: [PATCH 13/42] refactor: Simplify dequeue_strategy selection
Classes arent required anymore, it can just be a parm to worker class
isntead.
---
frappe/utils/background_jobs.py | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index b2a3fbab24..a713163a7b 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -4,7 +4,7 @@ import socket
import time
from collections import defaultdict
from functools import lru_cache
-from typing import Any, Callable, Literal, NoReturn
+from typing import Any, Callable, NoReturn
from uuid import uuid4
import redis
@@ -13,7 +13,7 @@ from rq import Queue, Worker
from rq.exceptions import NoSuchJobError
from rq.job import Job, JobStatus
from rq.logutils import setup_loghandlers
-from rq.worker import RandomWorker, RoundRobinWorker
+from rq.worker import DequeueStrategy
from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
import frappe
@@ -230,10 +230,12 @@ def start_worker(
rq_username: str | None = None,
rq_password: str | None = None,
burst: bool = False,
- strategy: Literal["round_robin", "random"] | None = None,
+ strategy: DequeueStrategy | None = DequeueStrategy.DEFAULT,
) -> NoReturn | None: # pragma: no cover
"""Wrapper to start rq worker. Connects to redis and monitors these queues."""
- DEQUEUE_STRATEGIES = {"round_robin": RoundRobinWorker, "random": RandomWorker}
+
+ if not strategy:
+ strategy = DequeueStrategy.DEFAULT
if frappe._tune_gc:
gc.collect()
@@ -251,17 +253,17 @@ def start_worker(
if os.environ.get("CI"):
setup_loghandlers("ERROR")
- WorkerKlass = DEQUEUE_STRATEGIES.get(strategy, Worker)
-
logging_level = "INFO"
if quiet:
logging_level = "WARNING"
- worker = WorkerKlass(queues, name=get_worker_name(queue_name), connection=redis_connection)
+
+ worker = Worker(queues, name=get_worker_name(queue_name), connection=redis_connection)
worker.work(
logging_level=logging_level,
burst=burst,
date_format="%Y-%m-%d %H:%M:%S",
log_format="%(asctime)s,%(msecs)03d %(message)s",
+ dequeue_strategy=strategy,
)
From 73bca16d77acbdf9c78eb38f0e4132a0518094ff Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 26 Jun 2023 17:59:28 +0530
Subject: [PATCH 14/42] feat: RQ `WorkerPool`
RQ now has experimental support for workerpools.
When to use this?
Roughly when you have more than 2 workers a workerpool might make
sense, below 2 it's overhead as master "pool" process will need to run
to manager workerpool itself.
Why is it any better?
Currently we just let supervisor duplicate the worker process N number
of times. This is inefficient from shared memory POV. Forking the
original process to create workers enables sharing of more memory thus
leading upwards of 60-70% reduction in memory usage with pool size of 8
workers.
---
frappe/commands/scheduler.py | 22 +++++++++++
frappe/core/doctype/rq_job/test_rq_job.py | 11 ++++++
frappe/utils/background_jobs.py | 46 +++++++++++++++++++++--
3 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py
index 36fa81f8a5..6af3a2403e 100755
--- a/frappe/commands/scheduler.py
+++ b/frappe/commands/scheduler.py
@@ -215,6 +215,27 @@ def start_worker(
)
+@click.command("worker-pool")
+@click.option(
+ "--queue",
+ type=str,
+ help="Queue to consume from. Multiple queues can be specified using comma-separated string. If not specified all queues are consumed.",
+)
+@click.option("--num-workers", type=int, default=2, help="Number of workers to spawn in pool.")
+@click.option("--quiet", is_flag=True, default=False, help="Hide Log Outputs")
+@click.option("--burst", is_flag=True, default=False, help="Run Worker in Burst mode.")
+def start_worker_pool(queue, quiet=False, num_workers=2, burst=False):
+ """Start a backgrond worker"""
+ from frappe.utils.background_jobs import start_worker_pool
+
+ start_worker_pool(
+ queue=queue,
+ quiet=quiet,
+ burst=burst,
+ num_workers=num_workers,
+ )
+
+
@click.command("ready-for-migration")
@click.option("--site", help="site name")
@pass_context
@@ -251,5 +272,6 @@ commands = [
show_pending_jobs,
start_scheduler,
start_worker,
+ start_worker_pool,
trigger_scheduler_event,
]
diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py
index c39717cfd8..6512902fb3 100644
--- a/frappe/core/doctype/rq_job/test_rq_job.py
+++ b/frappe/core/doctype/rq_job/test_rq_job.py
@@ -96,6 +96,17 @@ class TestRQJob(FrappeTestCase):
_, stderr = execute_in_shell("bench worker --queue short,default --burst", check_exit_code=True)
self.assertIn("quitting", cstr(stderr))
+ @timeout(20)
+ def test_multi_queue_burst_consumption_worker_pool(self):
+ for _ in range(3):
+ for q in ["default", "short"]:
+ frappe.enqueue(self.BG_JOB, sleep=1, queue=q)
+
+ _, stderr = execute_in_shell(
+ "bench worker-pool --queue short,default --burst --num-workers=4", check_exit_code=True
+ )
+ self.assertIn("quitting", cstr(stderr))
+
@timeout(20)
def test_job_id_dedup(self):
job_id = "test_dedup"
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index a713163a7b..9008ba00ee 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -14,6 +14,7 @@ from rq.exceptions import NoSuchJobError
from rq.job import Job, JobStatus
from rq.logutils import setup_loghandlers
from rq.worker import DequeueStrategy
+from rq.worker_pool import WorkerPool
from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
import frappe
@@ -237,9 +238,7 @@ def start_worker(
if not strategy:
strategy = DequeueStrategy.DEFAULT
- if frappe._tune_gc:
- gc.collect()
- gc.freeze()
+ _freeze_gc()
with frappe.init_site():
# empty init is required to get redis_queue from common_site_config.json
@@ -267,6 +266,47 @@ def start_worker(
)
+def start_worker_pool(
+ queue: str | None = None,
+ num_workers: int = 1,
+ quiet: bool = False,
+ burst: bool = False,
+) -> NoReturn:
+ """Start worker pool with specified number of workers.
+
+ WARNING: This feature is considered "EXPERIMENTAL".
+ """
+
+ _freeze_gc()
+
+ with frappe.init_site():
+ redis_connection = get_redis_conn()
+
+ if queue:
+ queue = [q.strip() for q in queue.split(",")]
+ queues = get_queue_list(queue, build_queue_name=True)
+
+ if os.environ.get("CI"):
+ setup_loghandlers("ERROR")
+
+ logging_level = "INFO"
+ if quiet:
+ logging_level = "WARNING"
+
+ pool = WorkerPool(
+ queues=queues,
+ connection=redis_connection,
+ num_workers=num_workers,
+ )
+ pool.start(logging_level=logging_level, burst=burst)
+
+
+def _freeze_gc():
+ if frappe._tune_gc:
+ gc.collect()
+ gc.freeze()
+
+
def get_worker_name(queue):
"""When limiting worker to a specific queue, also append queue name to default worker name"""
name = None
From 4b046f0b1116830ba2326ba36fea06465782c6fb Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 26 Jun 2023 15:53:15 +0200
Subject: [PATCH 15/42] refactor: use new_doc instead of get_doc
---
frappe/core/doctype/file/test_file.py | 84 ++++++++++++---------------
1 file changed, 36 insertions(+), 48 deletions(-)
diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py
index b07e344dc0..1e7e698062 100644
--- a/frappe/core/doctype/file/test_file.py
+++ b/frappe/core/doctype/file/test_file.py
@@ -615,46 +615,38 @@ class TestAttachmentsAccess(FrappeTestCase):
frappe.set_user("test4@example.com")
self.attached_to_doctype, self.attached_to_docname = make_test_doc()
- frappe.get_doc(
- {
- "doctype": "File",
- "file_name": "test_user_attachment.txt",
- "attached_to_doctype": self.attached_to_doctype,
- "attached_to_name": self.attached_to_docname,
- "content": "Testing User",
- "is_private": 1,
- }
+ frappe.new_doc(
+ "File",
+ file_name="test_user_attachment.txt",
+ attached_to_doctype=self.attached_to_doctype,
+ attached_to_name=self.attached_to_docname,
+ content="Testing User",
+ is_private=1,
).insert()
- frappe.get_doc(
- {
- "doctype": "File",
- "file_name": "test_user_standalone.txt",
- "content": "User Home",
- "is_private": 1,
- }
+ frappe.new_doc(
+ "File",
+ file_name="test_user_standalone.txt",
+ content="User Home",
+ is_private=1,
).insert()
frappe.set_user("test@example.com")
- frappe.get_doc(
- {
- "doctype": "File",
- "file_name": "test_sm_attachment.txt",
- "attached_to_doctype": self.attached_to_doctype,
- "attached_to_name": self.attached_to_docname,
- "content": "Testing System Manager",
- "is_private": 1,
- }
+ frappe.new_doc(
+ "File",
+ file_name="test_sm_attachment.txt",
+ attached_to_doctype=self.attached_to_doctype,
+ attached_to_name=self.attached_to_docname,
+ content="Testing System Manager",
+ is_private=1,
).insert()
- frappe.get_doc(
- {
- "doctype": "File",
- "file_name": "test_sm_standalone.txt",
- "content": "System Manager Home",
- "is_private": 1,
- }
+ frappe.new_doc(
+ "File",
+ file_name="test_sm_standalone.txt",
+ content="System Manager Home",
+ is_private=1,
).insert()
system_manager_files = [file.file_name for file in get_files_in_folder("Home")["files"]]
@@ -682,13 +674,11 @@ class TestAttachmentsAccess(FrappeTestCase):
def test_list_public_single_file(self):
"""Ensure that users are able to list public standalone files."""
frappe.set_user("test@example.com")
- frappe.get_doc(
- {
- "doctype": "File",
- "file_name": "test_public_single.txt",
- "content": "Public single File",
- "is_private": 0,
- }
+ frappe.new_doc(
+ "File",
+ file_name="test_public_single.txt",
+ content="Public single File",
+ is_private=0,
).insert()
frappe.set_user("test4@example.com")
@@ -699,15 +689,13 @@ class TestAttachmentsAccess(FrappeTestCase):
"""Ensure that users are able to list public attachments."""
frappe.set_user("test@example.com")
self.attached_to_doctype, self.attached_to_docname = make_test_doc()
- frappe.get_doc(
- {
- "doctype": "File",
- "file_name": "test_public_attachment.txt",
- "attached_to_doctype": self.attached_to_doctype,
- "attached_to_name": self.attached_to_docname,
- "content": "Public Attachment",
- "is_private": 0,
- }
+ frappe.new_doc(
+ "File",
+ file_name="test_public_attachment.txt",
+ attached_to_doctype=self.attached_to_doctype,
+ attached_to_name=self.attached_to_docname,
+ content="Public Attachment",
+ is_private=0,
).insert()
frappe.set_user("test4@example.com")
From e2c468df6026992a706116c35074c3bcd6153a2f Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 26 Jun 2023 19:56:04 +0530
Subject: [PATCH 16/42] fix: set prepared report in background (#21485)
---
frappe/core/doctype/report/report.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py
index 8cdbc24074..dba82bada5 100644
--- a/frappe/core/doctype/report/report.py
+++ b/frappe/core/doctype/report/report.py
@@ -135,7 +135,7 @@ class Report(Document):
# automatically set as prepared
execution_time = (datetime.datetime.now() - start_time).total_seconds()
if execution_time > threshold and not self.prepared_report:
- self.db_set("prepared_report", 1)
+ frappe.enqueue(enable_prepared_report, report=self.name)
frappe.cache.hset("report_execution_time", self.name, execution_time)
@@ -382,3 +382,7 @@ def get_group_by_column_label(args, meta):
function=sql_fn_map[args.aggregate_function], fieldlabel=aggregate_on_label
)
return label
+
+
+def enable_prepared_report(report: str):
+ frappe.db.set_value("Report", report, "prepared_report", 1)
From b9bd05581323085d44b365b2027ad011e180bc04 Mon Sep 17 00:00:00 2001
From: Hussain Nagaria
Date: Tue, 27 Jun 2023 17:10:46 +0530
Subject: [PATCH 17/42] fix(WebForm): auto-increment link field
---
frappe/public/js/frappe/form/controls/autocomplete.js | 9 +++++++++
frappe/website/doctype/web_form/web_form.py | 2 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/frappe/public/js/frappe/form/controls/autocomplete.js b/frappe/public/js/frappe/form/controls/autocomplete.js
index 27bf75e807..c0674956e9 100644
--- a/frappe/public/js/frappe/form/controls/autocomplete.js
+++ b/frappe/public/js/frappe/form/controls/autocomplete.js
@@ -174,6 +174,15 @@ frappe.ui.form.ControlAutocomplete = class ControlAutoComplete extends frappe.ui
if (typeof options[0] === "string") {
options = options.map((o) => ({ label: o, value: o }));
}
+
+ options = options.map((o) => {
+ if (typeof o !== "string") {
+ o.label = o.label.toString();
+ o.value = o.value.toString();
+ }
+ return o;
+ });
+
return options;
}
diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py
index fd9949c45f..75f8793b4a 100644
--- a/frappe/website/doctype/web_form/web_form.py
+++ b/frappe/website/doctype/web_form/web_form.py
@@ -631,7 +631,7 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals
if title_field and show_title_field_in_link:
return json.dumps(link_options, default=str)
else:
- return "\n".join([doc.value for doc in link_options])
+ return "\n".join([str(doc.value) for doc in link_options])
else:
raise frappe.PermissionError(
From b9f000e1f9f460e1e800a6bef381883c3694d9e4 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 10:07:14 +0530
Subject: [PATCH 18/42] refactor!: Log 5xx error to error log instead of error
snapshot
Also move log_error function to right location
---
frappe/__init__.py | 37 +-----
frappe/app.py | 4 +-
frappe/hooks.py | 1 -
frappe/utils/error.py | 275 ++++++------------------------------------
4 files changed, 43 insertions(+), 274 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 998d881a13..13e9448109 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -2213,41 +2213,6 @@ def logger(
)
-def log_error(title=None, message=None, reference_doctype=None, reference_name=None):
- """Log error to Error Log"""
- # Parameter ALERT:
- # the title and message may be swapped
- # the better API for this is log_error(title, message), and used in many cases this way
- # this hack tries to be smart about whats a title (single line ;-)) and fixes it
-
- traceback = None
- if message:
- if "\n" in title: # traceback sent as title
- traceback, title = title, message
- else:
- traceback = message
-
- title = title or "Error"
- traceback = as_unicode(traceback or get_traceback(with_context=True))
-
- if not db:
- print(f"Failed to log error in db: {title}")
- return
-
- error_log = get_doc(
- doctype="Error Log",
- error=traceback,
- method=title,
- reference_doctype=reference_doctype,
- reference_name=reference_name,
- )
-
- if flags.read_only:
- error_log.deferred_insert()
- else:
- return error_log.insert(ignore_permissions=True)
-
-
def get_desk_link(doctype, name):
html = (
'{doctype_local} {name}'
@@ -2439,6 +2404,8 @@ def validate_and_sanitize_search_inputs(fn):
return wrapper
+from frappe.utils.error import log_error # noqa: backward compatibility
+
if _tune_gc:
# generational GC gets triggered after certain allocs (g0) which is 700 by default.
# This number is quite small for frappe where a single query can potentially create 700+
diff --git a/frappe/app.py b/frappe/app.py
index 5113c858a5..1cbdca1361 100644
--- a/frappe/app.py
+++ b/frappe/app.py
@@ -22,7 +22,7 @@ from frappe import _
from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest
from frappe.middlewares import StaticDataMiddleware
from frappe.utils import cint, get_site_name, sanitize_html
-from frappe.utils.error import make_error_snapshot
+from frappe.utils.error import log_error_snapshot
from frappe.website.serve import get_response
local_manager = LocalManager(frappe.local)
@@ -346,7 +346,7 @@ def handle_exception(e):
frappe.local.login_manager.clear_cookies()
if http_status_code >= 500:
- make_error_snapshot(e)
+ log_error_snapshot(e)
if return_as_message:
response = get_response("message", http_status_code=http_status_code)
diff --git a/frappe/hooks.py b/frappe/hooks.py
index f160d93ecc..85a28feb39 100644
--- a/frappe/hooks.py
+++ b/frappe/hooks.py
@@ -214,7 +214,6 @@ scheduler_events = {
"hourly": [
"frappe.model.utils.link_count.update_link_count",
"frappe.model.utils.user_settings.sync_user_settings",
- "frappe.utils.error.collect_error_snapshots",
"frappe.desk.page.backups.backups.delete_downloadable_backups",
"frappe.deferred_insert.save_to_db",
"frappe.desk.form.document_follow.send_hourly_updates",
diff --git a/frappe/utils/error.py b/frappe/utils/error.py
index a9891cb532..d44bdef47f 100644
--- a/frappe/utils/error.py
+++ b/frappe/utils/error.py
@@ -1,17 +1,10 @@
# Copyright (c) 2015, Maxwell Morais and contributors
# License: MIT. See LICENSE
-import datetime
import functools
import inspect
-import json
-import linecache
-import os
-import sys
-import traceback
import frappe
-from frappe.utils import cstr, encode
EXCLUDE_EXCEPTIONS = (
frappe.AuthenticationError,
@@ -37,194 +30,57 @@ def _is_ldap_exception(e):
return False
-def make_error_snapshot(exception):
- if frappe.conf.disable_error_snapshot:
+def log_error(
+ title=None, message=None, reference_doctype=None, reference_name=None, *, defer_insert=False
+):
+ """Log error to Error Log"""
+ # Parameter ALERT:
+ # the title and message may be swapped
+ # the better API for this is log_error(title, message), and used in many cases this way
+ # this hack tries to be smart about whats a title (single line ;-)) and fixes it
+
+ traceback = None
+ if message:
+ if "\n" in title: # traceback sent as title
+ traceback, title = title, message
+ else:
+ traceback = message
+
+ title = title or "Error"
+ traceback = frappe.as_unicode(traceback or frappe.get_traceback(with_context=True))
+
+ if not frappe.db:
+ print(f"Failed to log error in db: {title}")
return
+ error_log = frappe.get_doc(
+ doctype="Error Log",
+ error=traceback,
+ method=title,
+ reference_doctype=reference_doctype,
+ reference_name=reference_name,
+ )
+
+ if frappe.flags.read_only or defer_insert:
+ error_log.deferred_insert()
+ else:
+ return error_log.insert(ignore_permissions=True)
+
+
+def log_error_snapshot(exception: Exception):
+
if isinstance(exception, EXCLUDE_EXCEPTIONS) or _is_ldap_exception(exception):
return
logger = frappe.logger(with_more_info=True)
try:
- error_id = "{timestamp:s}-{ip:s}-{hash:s}".format(
- timestamp=cstr(datetime.datetime.now()),
- ip=frappe.local.request_ip or "127.0.0.1",
- hash=frappe.generate_hash(length=3),
- )
- snapshot_folder = get_error_snapshot_path()
- frappe.create_folder(snapshot_folder)
-
- snapshot_file_path = os.path.join(snapshot_folder, f"{error_id}.json")
- snapshot = get_snapshot(exception)
-
- with open(encode(snapshot_file_path), "wb") as error_file:
- error_file.write(encode(frappe.as_json(snapshot)))
-
- logger.error(f"New Exception collected with id: {error_id}")
-
+ log_error(title=str(exception), defer_insert=True)
+ logger.error("New Exception collected in error log")
except Exception as e:
logger.error(f"Could not take error snapshot: {e}", exc_info=True)
-def get_snapshot(exception, context=10):
- import pydoc
-
- """
- Return a dict describing a given traceback (based on cgitb.text)
- """
-
- etype, evalue, etb = sys.exc_info()
- if isinstance(etype, type):
- etype = etype.__name__
-
- # creates a snapshot dict with some basic information
-
- s = {
- "pyver": "Python {version:s}: {executable:s} (prefix: {prefix:s})".format(
- version=sys.version.split(maxsplit=1)[0], executable=sys.executable, prefix=sys.prefix
- ),
- "timestamp": cstr(datetime.datetime.now()),
- "traceback": traceback.format_exc(),
- "frames": [],
- "etype": cstr(etype),
- "evalue": cstr(repr(evalue)),
- "exception": {},
- "locals": {},
- }
-
- # start to process frames
- records = inspect.getinnerframes(etb, 5)
-
- for frame, file, lnum, func, lines, index in records:
- file = file and os.path.abspath(file) or "?"
- args, varargs, varkw, locals = inspect.getargvalues(frame)
- call = ""
-
- if func != "?":
- call = inspect.formatargvalues(
- args, varargs, varkw, locals, formatvalue=lambda value: f"={pydoc.text.repr(value)}"
- )
-
- # basic frame information
- f = {"file": file, "func": func, "call": call, "lines": {}, "lnum": lnum}
-
- def reader(lnum=[lnum]): # noqa
- try:
- # B023: function is evaluated immediately, binding not necessary
- return linecache.getline(file, lnum[0]) # noqa: B023
- finally:
- lnum[0] += 1
-
- vars = _scanvars(reader, frame, locals)
-
- # if it is a view, replace with generated code
- # if file.endswith('html'):
- # lmin = lnum > context and (lnum - context) or 0
- # lmax = lnum + context
- # lines = code.split("\n")[lmin:lmax]
- # index = min(context, lnum) - 1
-
- if index is not None:
- i = lnum - index
- for line in lines:
- f["lines"][i] = line.rstrip()
- i += 1
-
- # dump local variable (referenced in current line only)
- f["dump"] = {}
- for name, where, value in vars:
- if name in f["dump"]:
- continue
- if value is not __UNDEF__:
- if where == "global":
- name = f"global {name:s}"
- elif where != "local":
- name = where + " " + name.split(".")[-1]
- f["dump"][name] = pydoc.text.repr(value)
- else:
- f["dump"][name] = "undefined"
-
- s["frames"].append(f)
-
- # add exception type, value and attributes
- if isinstance(evalue, BaseException):
- for name in dir(evalue):
- if name != "messages" and not name.startswith("__"):
- value = pydoc.text.repr(getattr(evalue, name))
- s["exception"][name] = encode(value)
-
- # add all local values (of last frame) to the snapshot
- for name, value in locals.items():
- s["locals"][name] = value if isinstance(value, str) else pydoc.text.repr(value)
-
- return s
-
-
-def collect_error_snapshots():
- """Scheduled task to collect error snapshots from files and push into Error Snapshot table"""
- if frappe.conf.disable_error_snapshot:
- return
-
- try:
- path = get_error_snapshot_path()
- if not os.path.exists(path):
- return
-
- for fname in os.listdir(path):
- fullpath = os.path.join(path, fname)
-
- try:
- with open(fullpath) as filedata:
- data = json.load(filedata)
-
- except ValueError:
- # empty file
- os.remove(fullpath)
- continue
-
- for field in ["locals", "exception", "frames"]:
- data[field] = frappe.as_json(data[field])
-
- doc = frappe.new_doc("Error Snapshot")
- doc.update(data)
- doc.save()
-
- frappe.db.commit()
-
- os.remove(fullpath)
-
- clear_old_snapshots()
-
- except Exception as e:
- make_error_snapshot(e)
-
- # prevent creation of unlimited error snapshots
- raise
-
-
-def clear_old_snapshots():
- """Clear snapshots that are older than a month"""
- from frappe.query_builder import DocType, Interval
- from frappe.query_builder.functions import Now
-
- ErrorSnapshot = DocType("Error Snapshot")
- frappe.db.delete(ErrorSnapshot, filters=(ErrorSnapshot.creation < (Now() - Interval(months=1))))
-
- path = get_error_snapshot_path()
- today = datetime.datetime.now()
-
- for file in os.listdir(path):
- p = os.path.join(path, file)
- ctime = datetime.datetime.fromtimestamp(os.path.getctime(p))
- if (today - ctime).days > 31:
- os.remove(os.path.join(path, p))
-
-
-def get_error_snapshot_path():
- return frappe.get_site_path("error-snapshots")
-
-
def get_default_args(func):
"""Get default arguments of a function from its signature."""
signature = inspect.signature(func)
@@ -270,56 +126,3 @@ def raise_error_on_no_output(error_message, error_type=None, keep_quiet=None):
return wrapper_raise_error_on_no_output
return decorator_raise_error_on_no_output
-
-
-# Vendored from cgitb standard library reused under PSF License:
-# https://github.com/python/cpython/blob/main/LICENSE
-
-
-import keyword
-import tokenize
-
-__UNDEF__ = [] # a special sentinel object
-
-
-def _scanvars(reader, frame, locals):
- """Scan one logical line of Python and look up values of variables used."""
- vars, lasttoken, parent, prefix, value = [], None, None, "", __UNDEF__
- for ttype, token, start, end, line in tokenize.generate_tokens(reader):
- if ttype == tokenize.NEWLINE:
- break
- if ttype == tokenize.NAME and token not in keyword.kwlist:
- if lasttoken == ".":
- if parent is not __UNDEF__:
- value = getattr(parent, token, __UNDEF__)
- vars.append((prefix + token, prefix, value))
- else:
- where, value = _lookup(token, frame, locals)
- vars.append((token, where, value))
- elif token == ".":
- prefix += lasttoken + "."
- parent = value
- else:
- parent, prefix = None, ""
- lasttoken = token
- return vars
-
-
-def _lookup(name, frame, locals):
- """Find the value for a given name in the given environment."""
- if name in locals:
- return "local", locals[name]
- if name in frame.f_globals:
- return "global", frame.f_globals[name]
- if "__builtins__" in frame.f_globals:
- builtins = frame.f_globals["__builtins__"]
- if type(builtins) is type({}): # noqa
- if name in builtins:
- return "builtin", builtins[name]
- else:
- if hasattr(builtins, name):
- return "builtin", getattr(builtins, name)
- return None, __UNDEF__
-
-
-# end: vendored code
From ae8ee5064c972f66411030bcf7e9d6f9a4236743 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 10:25:26 +0530
Subject: [PATCH 19/42] refactor!: Remove error snapshot
---
.../core/doctype/error_snapshot/__init__.py | 0
.../doctype/error_snapshot/error_object.html | 12 --
.../error_snapshot/error_snapshot.html | 77 -----------
.../doctype/error_snapshot/error_snapshot.js | 20 ---
.../error_snapshot/error_snapshot.json | 130 ------------------
.../doctype/error_snapshot/error_snapshot.py | 40 ------
.../error_snapshot/error_snapshot_list.js | 19 ---
.../error_snapshot/test_error_snapshot.py | 11 --
.../core/doctype/log_settings/log_settings.py | 2 -
.../doctype/log_settings/test_log_settings.py | 1 -
frappe/core/notifications.py | 1 -
frappe/core/workspace/build/build.json | 128 ++++++++---------
frappe/installer.py | 1 -
frappe/patches.txt | 3 +-
.../v14_0/clear_long_pending_stale_logs.py | 1 -
15 files changed, 60 insertions(+), 386 deletions(-)
delete mode 100644 frappe/core/doctype/error_snapshot/__init__.py
delete mode 100644 frappe/core/doctype/error_snapshot/error_object.html
delete mode 100644 frappe/core/doctype/error_snapshot/error_snapshot.html
delete mode 100644 frappe/core/doctype/error_snapshot/error_snapshot.js
delete mode 100644 frappe/core/doctype/error_snapshot/error_snapshot.json
delete mode 100644 frappe/core/doctype/error_snapshot/error_snapshot.py
delete mode 100644 frappe/core/doctype/error_snapshot/error_snapshot_list.js
delete mode 100644 frappe/core/doctype/error_snapshot/test_error_snapshot.py
diff --git a/frappe/core/doctype/error_snapshot/__init__.py b/frappe/core/doctype/error_snapshot/__init__.py
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/frappe/core/doctype/error_snapshot/error_object.html b/frappe/core/doctype/error_snapshot/error_object.html
deleted file mode 100644
index 450bfacfc6..0000000000
--- a/frappe/core/doctype/error_snapshot/error_object.html
+++ /dev/null
@@ -1,12 +0,0 @@
-{% if (Object.prototype.toString.call(x) === "[object Object]") { %}
-
- {% for (var key in x) { %}
-
- {{ key }} |
- {{ x[key] }} |
-
- {% } %}
-
-{% } else { %}
- {{ x }}
-{% } %}
diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.html b/frappe/core/doctype/error_snapshot/error_snapshot.html
deleted file mode 100644
index 6f449e0fe9..0000000000
--- a/frappe/core/doctype/error_snapshot/error_snapshot.html
+++ /dev/null
@@ -1,77 +0,0 @@
-
-{% function id(){ return id._old_id++; }; id._old_id = 0; %}
-{{ __("Error Report") }}
-{{ doc.pyver }}
-
- - {{ __("Timestamp") }}:
- - {{ doc.timestamp }}
- - {{ __("Relapsed") }}
- {{ doc.relapses }}
-
-
-{{ __("Exception") }}
-{{ frappe.render_template("error_object", {x: JSON.parse(doc.exception)}) }}
-
-{{ __("Locals") }}
-{{ frappe.render_template("error_object", {x: JSON.parse(doc.locals)} )}}
-
-{{ __("Traceback") }}
-{% var frames = JSON.parse(doc.frames); %}
-{% for (var i in frames) { %}
- {% var frameid = id(), frame = frames[i] %}
- {{ frame.file }}: {{ frame.lnum }}
-
-
-
- {% for (var index in frame.lines) { %}
- {% var line = frame.lines[index] %}
-
- {{ index }}
- {{ line }}
-
- {% } %}
-
-
-
- {{ __("Locals") }}
-
-
-
-
-
-
-
{{ __("Locals") }}
- {{ frappe.render_template("error_object", {x: frame.dump }) }}
-
-
-
-{% } %}
diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.js b/frappe/core/doctype/error_snapshot/error_snapshot.js
deleted file mode 100644
index f8a7e3ded5..0000000000
--- a/frappe/core/doctype/error_snapshot/error_snapshot.js
+++ /dev/null
@@ -1,20 +0,0 @@
-frappe.ui.form.on("Error Snapshot", "load", function (frm) {
- frm.set_read_only(true);
-});
-
-frappe.ui.form.on("Error Snapshot", "refresh", function (frm) {
- frm.set_df_property(
- "view",
- "options",
- frappe.render_template("error_snapshot", { doc: frm.doc })
- );
-
- if (frm.doc.relapses) {
- frm.add_custom_button(__("Show Relapses"), function () {
- frappe.route_options = {
- parent_error_snapshot: frm.doc.name,
- };
- frappe.set_route("List", "Error Snapshot");
- });
- }
-});
diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.json b/frappe/core/doctype/error_snapshot/error_snapshot.json
deleted file mode 100644
index b92db8f99a..0000000000
--- a/frappe/core/doctype/error_snapshot/error_snapshot.json
+++ /dev/null
@@ -1,130 +0,0 @@
-{
- "actions": [],
- "creation": "2015-11-28 00:57:39.766888",
- "doctype": "DocType",
- "document_type": "System",
- "engine": "InnoDB",
- "field_order": [
- "view",
- "seen",
- "evalue",
- "timestamp",
- "relapses",
- "etype",
- "traceback",
- "parent_error_snapshot",
- "pyver",
- "exception",
- "locals",
- "frames"
- ],
- "fields": [
- {
- "fieldname": "view",
- "fieldtype": "HTML",
- "label": "Snapshot View"
- },
- {
- "default": "0",
- "fieldname": "seen",
- "fieldtype": "Check",
- "hidden": 1,
- "in_filter": 1,
- "label": "Seen"
- },
- {
- "fieldname": "evalue",
- "fieldtype": "Code",
- "hidden": 1,
- "in_list_view": 1,
- "label": "Friendly Title",
- "read_only": 1
- },
- {
- "fieldname": "timestamp",
- "fieldtype": "Datetime",
- "hidden": 1,
- "label": "Timestamp",
- "read_only": 1
- },
- {
- "default": "1",
- "fieldname": "relapses",
- "fieldtype": "Int",
- "hidden": 1,
- "in_list_view": 1,
- "label": "Relapses",
- "read_only": 1
- },
- {
- "fieldname": "etype",
- "fieldtype": "Data",
- "hidden": 1,
- "label": "Exception Type",
- "read_only": 1
- },
- {
- "fieldname": "traceback",
- "fieldtype": "Code",
- "hidden": 1,
- "label": "Traceback",
- "read_only": 1
- },
- {
- "fieldname": "parent_error_snapshot",
- "fieldtype": "Data",
- "hidden": 1,
- "label": "Parent Error Snapshot"
- },
- {
- "fieldname": "pyver",
- "fieldtype": "Code",
- "hidden": 1,
- "label": "Pyver",
- "read_only": 1
- },
- {
- "fieldname": "exception",
- "fieldtype": "Code",
- "hidden": 1,
- "label": "Exception"
- },
- {
- "fieldname": "locals",
- "fieldtype": "Code",
- "hidden": 1,
- "label": "Locals"
- },
- {
- "fieldname": "frames",
- "fieldtype": "Code",
- "hidden": 1,
- "label": "Frames"
- }
- ],
- "in_create": 1,
- "links": [],
- "modified": "2022-08-03 12:20:53.504160",
- "modified_by": "Administrator",
- "module": "Core",
- "name": "Error Snapshot",
- "owner": "Administrator",
- "permissions": [
- {
- "create": 1,
- "delete": 1,
- "email": 1,
- "export": 1,
- "print": 1,
- "read": 1,
- "report": 1,
- "role": "Administrator",
- "share": 1,
- "write": 1
- }
- ],
- "sort_field": "timestamp",
- "sort_order": "DESC",
- "states": [],
- "title_field": "evalue"
-}
\ No newline at end of file
diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.py b/frappe/core/doctype/error_snapshot/error_snapshot.py
deleted file mode 100644
index acc49c78cd..0000000000
--- a/frappe/core/doctype/error_snapshot/error_snapshot.py
+++ /dev/null
@@ -1,40 +0,0 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and contributors
-# License: MIT. See LICENSE
-
-import frappe
-from frappe.model.document import Document
-from frappe.query_builder import Interval
-from frappe.query_builder.functions import Now
-
-
-class ErrorSnapshot(Document):
- no_feed_on_delete = True
-
- def onload(self):
- if not self.parent_error_snapshot:
- self.db_set("seen", 1, update_modified=False)
-
- for relapsed in frappe.get_all("Error Snapshot", filters={"parent_error_snapshot": self.name}):
- frappe.db.set_value("Error Snapshot", relapsed.name, "seen", 1, update_modified=False)
-
- frappe.local.flags.commit = True
-
- def validate(self):
- parent = frappe.get_all(
- "Error Snapshot",
- filters={"evalue": self.evalue, "parent_error_snapshot": ""},
- fields=["name", "relapses", "seen"],
- limit_page_length=1,
- )
-
- if parent:
- parent = parent[0]
- self.update({"parent_error_snapshot": parent["name"]})
- frappe.db.set_value("Error Snapshot", parent["name"], "relapses", parent["relapses"] + 1)
- if parent["seen"]:
- frappe.db.set_value("Error Snapshot", parent["name"], "seen", 0)
-
- @staticmethod
- def clear_old_logs(days=30):
- table = frappe.qb.DocType("Error Snapshot")
- frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days))))
diff --git a/frappe/core/doctype/error_snapshot/error_snapshot_list.js b/frappe/core/doctype/error_snapshot/error_snapshot_list.js
deleted file mode 100644
index b331788852..0000000000
--- a/frappe/core/doctype/error_snapshot/error_snapshot_list.js
+++ /dev/null
@@ -1,19 +0,0 @@
-frappe.listview_settings["Error Snapshot"] = {
- add_fields: ["parent_error_snapshot", "relapses", "seen"],
- filters: [
- ["parent_error_snapshot", "=", null],
- ["seen", "=", false],
- ],
- get_indicator: function (doc) {
- if (doc.parent_error_snapshot && doc.parent_error_snapshot.length) {
- return [__("Relapsed"), !doc.seen ? "orange" : "blue", "parent_error_snapshot,!=,"];
- } else {
- return [__("First Level"), !doc.seen ? "red" : "green", "parent_error_snapshot,=,"];
- }
- },
- onload: function (listview) {
- frappe.require("logtypes.bundle.js", () => {
- frappe.utils.logtypes.show_log_retention_message(cur_list.doctype);
- });
- },
-};
diff --git a/frappe/core/doctype/error_snapshot/test_error_snapshot.py b/frappe/core/doctype/error_snapshot/test_error_snapshot.py
deleted file mode 100644
index 4779d56c7b..0000000000
--- a/frappe/core/doctype/error_snapshot/test_error_snapshot.py
+++ /dev/null
@@ -1,11 +0,0 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
-# License: MIT. See LICENSE
-from frappe.tests.utils import FrappeTestCase
-from frappe.utils.logger import sanitized_dict
-
-# test_records = frappe.get_test_records('Error Snapshot')
-
-
-class TestErrorSnapshot(FrappeTestCase):
- def test_form_dict_sanitization(self):
- self.assertNotEqual(sanitized_dict({"pwd": "SECRET", "usr": "WHAT"}).get("pwd"), "SECRET")
diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py
index 832be49f3c..c4d311cb3d 100644
--- a/frappe/core/doctype/log_settings/log_settings.py
+++ b/frappe/core/doctype/log_settings/log_settings.py
@@ -14,7 +14,6 @@ DEFAULT_LOGTYPES_RETENTION = {
"Error Log": 30,
"Activity Log": 90,
"Email Queue": 30,
- "Error Snapshot": 30,
"Scheduled Job Log": 90,
"Route History": 90,
"Submission Queue": 30,
@@ -156,7 +155,6 @@ LOG_DOCTYPES = [
"Route History",
"Email Queue",
"Email Queue Recipient",
- "Error Snapshot",
"Error Log",
]
diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py
index d7f43a181d..edee098553 100644
--- a/frappe/core/doctype/log_settings/test_log_settings.py
+++ b/frappe/core/doctype/log_settings/test_log_settings.py
@@ -62,7 +62,6 @@ class TestLogSettings(FrappeTestCase):
"Activity Log",
"Email Queue",
"Route History",
- "Error Snapshot",
"Scheduled Job Log",
]
diff --git a/frappe/core/notifications.py b/frappe/core/notifications.py
index 093418e345..26e920bca9 100644
--- a/frappe/core/notifications.py
+++ b/frappe/core/notifications.py
@@ -11,7 +11,6 @@ def get_notification_config():
"Communication": {"status": "Open", "communication_type": "Communication"},
"ToDo": "frappe.core.notifications.get_things_todo",
"Event": "frappe.core.notifications.get_todays_events",
- "Error Snapshot": {"seen": 0, "parent_error_snapshot": None},
"Workflow Action": {"status": "Open"},
},
}
diff --git a/frappe/core/workspace/build/build.json b/frappe/core/workspace/build/build.json
index b917f88e27..12bef0ed89 100644
--- a/frappe/core/workspace/build/build.json
+++ b/frappe/core/workspace/build/build.json
@@ -155,74 +155,6 @@
"onboard": 0,
"type": "Link"
},
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "System Logs",
- "link_count": 6,
- "onboard": 0,
- "type": "Card Break"
- },
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "Background Jobs",
- "link_count": 0,
- "link_to": "RQ Job",
- "link_type": "DocType",
- "onboard": 0,
- "type": "Link"
- },
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "Scheduled Jobs Logs",
- "link_count": 0,
- "link_to": "Scheduled Job Log",
- "link_type": "DocType",
- "onboard": 0,
- "type": "Link"
- },
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "Error Logs",
- "link_count": 0,
- "link_to": "Error Log",
- "link_type": "DocType",
- "onboard": 0,
- "type": "Link"
- },
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "Error Snapshot",
- "link_count": 0,
- "link_to": "Error Snapshot",
- "link_type": "DocType",
- "onboard": 0,
- "type": "Link"
- },
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "Communication Logs",
- "link_count": 0,
- "link_to": "Communication",
- "link_type": "DocType",
- "onboard": 0,
- "type": "Link"
- },
- {
- "hidden": 0,
- "is_query_report": 0,
- "label": "Activity Log",
- "link_count": 0,
- "link_to": "Activity Log",
- "link_type": "DocType",
- "onboard": 0,
- "type": "Link"
- },
{
"hidden": 0,
"is_query_report": 0,
@@ -331,9 +263,67 @@
"link_type": "DocType",
"onboard": 0,
"type": "Link"
+ },
+ {
+ "hidden": 0,
+ "is_query_report": 0,
+ "label": "System Logs",
+ "link_count": 5,
+ "onboard": 0,
+ "type": "Card Break"
+ },
+ {
+ "hidden": 0,
+ "is_query_report": 0,
+ "label": "Background Jobs",
+ "link_count": 0,
+ "link_to": "RQ Job",
+ "link_type": "DocType",
+ "onboard": 0,
+ "type": "Link"
+ },
+ {
+ "hidden": 0,
+ "is_query_report": 0,
+ "label": "Scheduled Jobs Logs",
+ "link_count": 0,
+ "link_to": "Scheduled Job Log",
+ "link_type": "DocType",
+ "onboard": 0,
+ "type": "Link"
+ },
+ {
+ "hidden": 0,
+ "is_query_report": 0,
+ "label": "Error Logs",
+ "link_count": 0,
+ "link_to": "Error Log",
+ "link_type": "DocType",
+ "onboard": 0,
+ "type": "Link"
+ },
+ {
+ "hidden": 0,
+ "is_query_report": 0,
+ "label": "Communication Logs",
+ "link_count": 0,
+ "link_to": "Communication",
+ "link_type": "DocType",
+ "onboard": 0,
+ "type": "Link"
+ },
+ {
+ "hidden": 0,
+ "is_query_report": 0,
+ "label": "Activity Log",
+ "link_count": 0,
+ "link_to": "Activity Log",
+ "link_type": "DocType",
+ "onboard": 0,
+ "type": "Link"
}
],
- "modified": "2023-05-24 14:47:24.395259",
+ "modified": "2023-06-28 10:30:17.228167",
"modified_by": "Administrator",
"module": "Core",
"name": "Build",
diff --git a/frappe/installer.py b/frappe/installer.py
index 4f02e207bd..775e5b9b02 100644
--- a/frappe/installer.py
+++ b/frappe/installer.py
@@ -617,7 +617,6 @@ def make_site_dirs():
os.path.join("public", "files"),
os.path.join("private", "backups"),
os.path.join("private", "files"),
- "error-snapshots",
"locks",
"logs",
]:
diff --git a/frappe/patches.txt b/frappe/patches.txt
index c26b1a74d7..ebdda9b220 100644
--- a/frappe/patches.txt
+++ b/frappe/patches.txt
@@ -31,7 +31,6 @@ execute:frappe.reload_doc('core', 'doctype', 'user') #2017-10-27
execute:frappe.reload_doc('core', 'doctype', 'report_column')
execute:frappe.reload_doc('core', 'doctype', 'report_filter')
execute:frappe.reload_doc('core', 'doctype', 'report') #2020-08-25
-execute:frappe.reload_doc('core', 'doctype', 'error_snapshot')
execute:frappe.get_doc("User", "Guest").save()
execute:frappe.delete_doc("DocType", "Control Panel", force=1)
execute:frappe.delete_doc("DocType", "Tag")
@@ -42,7 +41,6 @@ execute:frappe.db.sql("delete from `tabProperty Setter` where `property` = 'idx'
execute:frappe.db.sql("delete from tabSessions where user is null")
execute:frappe.delete_doc("DocType", "Backup Manager")
execute:frappe.permissions.reset_perms("Web Page")
-execute:frappe.permissions.reset_perms("Error Snapshot")
execute:frappe.db.sql("delete from `tabWeb Page` where ifnull(template_path, '')!=''")
execute:frappe.core.doctype.language.language.update_language_names() # 2017-04-12
execute:frappe.db.set_value("Print Settings", "Print Settings", "add_draft_heading", 1)
@@ -227,3 +225,4 @@ frappe.patches.v15_0.remove_background_jobs_from_dropdown
frappe.desk.doctype.form_tour.patches.introduce_ui_tours
execute:frappe.delete_doc_if_exists("Workspace", "Customization")
execute:frappe.db.set_single_value("Document Naming Settings", "default_amend_naming", "Amend Counter")
+execute:frappe.delete_doc_if_exists("DocType", "Error Snapshot")
diff --git a/frappe/patches/v14_0/clear_long_pending_stale_logs.py b/frappe/patches/v14_0/clear_long_pending_stale_logs.py
index 53127cb197..e419b1e562 100644
--- a/frappe/patches/v14_0/clear_long_pending_stale_logs.py
+++ b/frappe/patches/v14_0/clear_long_pending_stale_logs.py
@@ -15,7 +15,6 @@ def execute():
"Email Queue": get_current_setting("clear_email_queue_after") or 30,
# child table on email queue
"Email Queue Recipient": get_current_setting("clear_email_queue_after") or 30,
- "Error Snapshot": get_current_setting("clear_error_log_after") or 90,
# newly added
"Scheduled Job Log": 90,
}
From 37577b752ef025f9815e0996ca88417ca608d7a1 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 10:57:54 +0530
Subject: [PATCH 20/42] fix: log settings should delete delted doctypes
[skip ci]
---
frappe/core/doctype/log_settings/log_settings.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py
index c4d311cb3d..623e358b6c 100644
--- a/frappe/core/doctype/log_settings/log_settings.py
+++ b/frappe/core/doctype/log_settings/log_settings.py
@@ -44,11 +44,11 @@ def _supports_log_clearing(doctype: str) -> bool:
class LogSettings(Document):
def validate(self):
- self._remove_unsupported_doctypes()
+ self.remove_unsupported_doctypes()
self._deduplicate_entries()
self.add_default_logtypes()
- def _remove_unsupported_doctypes(self):
+ def remove_unsupported_doctypes(self):
for entry in list(self.logs_to_clear):
if _supports_log_clearing(entry.ref_doctype):
continue
@@ -113,6 +113,7 @@ class LogSettings(Document):
def run_log_clean_up():
doc = frappe.get_doc("Log Settings")
+ doc.remove_unsupported_doctypes()
doc.add_default_logtypes()
doc.save()
doc.clear_logs()
From 537d5511127235b41b44f7c4fc15df340f89d5de Mon Sep 17 00:00:00 2001
From: Suraj Shetty
Date: Wed, 28 Jun 2023 11:17:17 +0530
Subject: [PATCH 21/42] refactor: Deprecate broken-img mixin
---
frappe/public/scss/common/mixins.scss | 41 ++-----------------------
frappe/public/scss/desk/file_view.scss | 5 ---
frappe/public/scss/desk/image_view.scss | 5 ---
frappe/public/scss/desk/kanban.scss | 5 ---
4 files changed, 2 insertions(+), 54 deletions(-)
diff --git a/frappe/public/scss/common/mixins.scss b/frappe/public/scss/common/mixins.scss
index 6d71ea9d6f..55b54d9de3 100644
--- a/frappe/public/scss/common/mixins.scss
+++ b/frappe/public/scss/common/mixins.scss
@@ -45,42 +45,5 @@
$background-color: var(--bg-color),
$border-radius: var(--border-radius),
) {
-
- @if $content {
- img:after {
- content: url($content);
- }
- } @else {
- img:after {
- content: url("data:image/svg+xml;utf8,");
- }
- }
-
- img[alt]:after {
- height: $height;
- top: $top;
- left: $left;
- background-color: $background-color;
- border-radius: $border-radius;
- width: 100%;
- position: absolute;
- @include flex();
- z-index: 1;
- }
-}
-
-// @mixin img-foreground() {
-// content: "\f1c5";
-// display: block;
-// font-style: normal;
-// font-family: FontAwesome;
-// font-size: 32px;
-// color: var(--text-muted);
-
-// position: absolute;
-// top: 50%;
-// transform: translateY(-50%);
-// left: 0;
-// width: 100%;
-// text-align: center;
-// }
\ No newline at end of file
+ // Deprecated: Does not work as expected anymore. Also, this never worked in Safari.
+}
\ No newline at end of file
diff --git a/frappe/public/scss/desk/file_view.scss b/frappe/public/scss/desk/file_view.scss
index d074a7efdd..29e49ab65e 100644
--- a/frappe/public/scss/desk/file_view.scss
+++ b/frappe/public/scss/desk/file_view.scss
@@ -97,11 +97,6 @@
color: transparent;
position: relative;
}
-
- @include broken-img(
- $height: 70px,
- $top: -15px,
- );
}
}
diff --git a/frappe/public/scss/desk/image_view.scss b/frappe/public/scss/desk/image_view.scss
index 3b9d033406..063af27923 100644
--- a/frappe/public/scss/desk/image_view.scss
+++ b/frappe/public/scss/desk/image_view.scss
@@ -153,11 +153,6 @@
position: relative;
width: 100%;
}
-
- @include broken-img(
- $height: 175px,
- $border-radius: 0
- );
}
.image-title {
diff --git a/frappe/public/scss/desk/kanban.scss b/frappe/public/scss/desk/kanban.scss
index 8f286b7b35..5a748c581c 100644
--- a/frappe/public/scss/desk/kanban.scss
+++ b/frappe/public/scss/desk/kanban.scss
@@ -181,11 +181,6 @@
color: transparent;
position: relative;
}
-
- @include broken-img(
- $height: 125px,
- $top: -4px,
- )
}
.kanban-card-body {
From f3c876e43e0c8182220e6d5a0e6365efcfa4fd97 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 11:43:23 +0530
Subject: [PATCH 22/42] chore: ignore pyo files too
---
frappe/website/page_renderers/static_page.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/website/page_renderers/static_page.py b/frappe/website/page_renderers/static_page.py
index d6de2f2991..f992743c6a 100644
--- a/frappe/website/page_renderers/static_page.py
+++ b/frappe/website/page_renderers/static_page.py
@@ -8,7 +8,7 @@ import frappe
from frappe.website.page_renderers.base_renderer import BaseRenderer
from frappe.website.utils import is_binary_file
-UNSUPPORTED_STATIC_PAGE_TYPES = ("html", "md", "js", "xml", "css", "txt", "py", "pyc", "json")
+UNSUPPORTED_STATIC_PAGE_TYPES = ("html", "md", "js", "xml", "css", "txt", "py", "pyc", "json", "pyo")
class StaticPage(BaseRenderer):
From aaf1bd7f4bcd21db2e36888ac48d91d2260f772f Mon Sep 17 00:00:00 2001
From: Hussain Nagaria
Date: Wed, 28 Jun 2023 11:46:17 +0530
Subject: [PATCH 23/42] style: fix linter whitespace issue
---
frappe/public/js/frappe/form/controls/autocomplete.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/public/js/frappe/form/controls/autocomplete.js b/frappe/public/js/frappe/form/controls/autocomplete.js
index c0674956e9..7b91a2031c 100644
--- a/frappe/public/js/frappe/form/controls/autocomplete.js
+++ b/frappe/public/js/frappe/form/controls/autocomplete.js
@@ -182,7 +182,7 @@ frappe.ui.form.ControlAutocomplete = class ControlAutoComplete extends frappe.ui
}
return o;
});
-
+
return options;
}
From 814265d24581ca6024ebdf8c4e53df95ac060726 Mon Sep 17 00:00:00 2001
From: Maharshi Patel <39730881+maharshivpatel@users.noreply.github.com>
Date: Wed, 28 Jun 2023 13:34:36 +0530
Subject: [PATCH 24/42] fix(minor): workflow state indicator (#21508)
In List view only `Status` type was considered for indicator. Added fields with `Workflow State` as options
---
frappe/public/js/frappe/list/list_view.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js
index c0eef27b9d..1ca72a4e45 100644
--- a/frappe/public/js/frappe/list/list_view.js
+++ b/frappe/public/js/frappe/list/list_view.js
@@ -711,7 +711,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
}
get_column_html(col, doc) {
- if (col.type === "Status") {
+ if (col.type === "Status" || col.df?.options == "Workflow State") {
return `
${this.get_indicator_html(doc)}
From 95e49193c838564de080379eab804b586a85277f Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 15:21:04 +0530
Subject: [PATCH 25/42] fix: dont retry if redis not available for realtime
(#21517)
* fix: reduce retries for rq connection
10 seconds of retries when connection isn't available is too much, just
failing might be beneficial.
- BusyLoadingError only occurs when redis is restarting
- ConnectionError mostly means redis is dead, no amount of retries will
bring it back.
* fix: dont retry if redis is down for realtime
---
frappe/realtime.py | 4 ++--
frappe/utils/background_jobs.py | 16 +++++++++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/frappe/realtime.py b/frappe/realtime.py
index 410112b164..47b3d8a95c 100644
--- a/frappe/realtime.py
+++ b/frappe/realtime.py
@@ -100,10 +100,10 @@ def emit_via_redis(event, message, room):
:param event: Event name, like `task_progress` etc.
:param message: JSON message object. For async must contain `task_id`
:param room: name of the room"""
- from frappe.utils.background_jobs import get_redis_conn
+ from frappe.utils.background_jobs import get_redis_connection_without_auth
with suppress(redis.exceptions.ConnectionError):
- r = get_redis_conn()
+ r = get_redis_connection_without_auth()
r.publish("events", frappe.as_json({"event": event, "message": message, "room": room}))
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index 9008ba00ee..7de7ef6692 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -394,8 +394,8 @@ def validate_queue(queue, default_queue_list=None):
@retry(
- retry=retry_if_exception_type(BusyLoadingError) | retry_if_exception_type(ConnectionError),
- stop=stop_after_attempt(10),
+ retry=retry_if_exception_type((BusyLoadingError, ConnectionError)),
+ stop=stop_after_attempt(5),
wait=wait_fixed(1),
reraise=True,
)
@@ -423,9 +423,7 @@ def get_redis_conn(username=None, password=None):
try:
if not cred:
- if not _redis_queue_conn:
- _redis_queue_conn = RedisQueue.get_connection()
- return _redis_queue_conn
+ return get_redis_connection_without_auth()
else:
return RedisQueue.get_connection(**cred)
except (redis.exceptions.AuthenticationError, redis.exceptions.ResponseError):
@@ -440,6 +438,14 @@ def get_redis_conn(username=None, password=None):
raise
+def get_redis_connection_without_auth():
+ global _redis_queue_conn
+
+ if not _redis_queue_conn:
+ _redis_queue_conn = RedisQueue.get_connection()
+ return _redis_queue_conn
+
+
def get_queues() -> list[Queue]:
"""Get all the queues linked to the current bench."""
queues = Queue.all(connection=get_redis_conn())
From 564b960678d7ff31da1cb527f689a39a935a4016 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 13:20:49 +0530
Subject: [PATCH 26/42] fix: correct last update value
`NOW()` evalautes to server's time we should use system time instead.
---
frappe/sessions.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/frappe/sessions.py b/frappe/sessions.py
index 64a1a6b663..8682300845 100644
--- a/frappe/sessions.py
+++ b/frappe/sessions.py
@@ -386,8 +386,8 @@ class Session:
# update sessions table
frappe.db.sql(
"""update `tabSessions` set sessiondata=%s,
- lastupdate=NOW() where sid=%s""",
- (str(self.data["data"]), self.data["sid"]),
+ lastupdate=%s where sid=%s""",
+ (str(self.data["data"]), now, self.data["sid"]),
)
# update last active in user table
From 7c4009fde98244459be25e1f6d15699f87f5f02c Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 13:26:39 +0530
Subject: [PATCH 27/42] refactor: use QB
---
frappe/sessions.py | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/frappe/sessions.py b/frappe/sessions.py
index 8682300845..bcacc79ce7 100644
--- a/frappe/sessions.py
+++ b/frappe/sessions.py
@@ -373,6 +373,8 @@ class Session:
now = frappe.utils.now()
+ Sessions = frappe.qb.DocType("Sessions")
+
self.data["data"]["last_updated"] = now
self.data["data"]["lang"] = str(frappe.lang)
@@ -384,17 +386,14 @@ class Session:
updated_in_db = False
if (force or (time_diff is None) or (time_diff > 600)) and not frappe.flags.read_only:
# update sessions table
- frappe.db.sql(
- """update `tabSessions` set sessiondata=%s,
- lastupdate=%s where sid=%s""",
- (str(self.data["data"]), now, self.data["sid"]),
- )
+ (
+ frappe.qb.update(Sessions)
+ .where(Sessions.sid == self.data["sid"])
+ .set(Sessions.sessiondata, str(self.data["data"]))
+ .set(Sessions.lastupdate, now)
+ ).run()
- # update last active in user table
- frappe.db.sql(
- """update `tabUser` set last_active=%(now)s where name=%(name)s""",
- {"now": now, "name": frappe.session.user},
- )
+ frappe.db.set_value("User", frappe.session.user, "last_active", now, update_modified=False)
frappe.db.commit()
frappe.cache.hset("last_db_session_update", self.sid, now)
From 60efb7c2ff7350f52a9da1f878ba14ae76d2230e Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 15:58:41 +0530
Subject: [PATCH 28/42] fix: incorrect session expiry datediff
Datediff doesn't work like this in MYSQL, mysql just treats the
timestamp as flat timestamp.
---
frappe/sessions.py | 45 +++++++++++++++++++++------------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/frappe/sessions.py b/frappe/sessions.py
index bcacc79ce7..94ad991c3a 100644
--- a/frappe/sessions.py
+++ b/frappe/sessions.py
@@ -19,8 +19,7 @@ import frappe.utils
from frappe import _
from frappe.cache_manager import clear_user_cache
from frappe.query_builder import DocType, Order
-from frappe.query_builder.functions import Now
-from frappe.query_builder.utils import PseudoColumn
+from frappe.query_builder.functions import UnixTimestamp
from frappe.utils import cint, cstr, get_assets_json
@@ -112,17 +111,14 @@ def clear_all_sessions(reason=None):
def get_expired_sessions():
"""Returns list of expired sessions"""
- sessions = DocType("Sessions")
+ sessions = frappe.qb.DocType("Sessions")
+ now = frappe.utils.now()
- return frappe.db.get_values(
- sessions,
- filters=(
- PseudoColumn(f"({Now()} - {sessions.lastupdate.get_sql()})") > get_expiry_period_for_query()
- ),
- fieldname="sid",
- order_by=None,
- pluck=True,
- )
+ return (
+ frappe.qb.from_(sessions)
+ .select(sessions.sid)
+ .where((UnixTimestamp(now) - UnixTimestamp(sessions.lastupdate)) > get_expiry_period_for_query())
+ ).run(pluck=True)
def clear_expired_sessions():
@@ -339,19 +335,20 @@ class Session:
def get_session_data_from_db(self):
sessions = DocType("Sessions")
- rec = frappe.db.get_values(
- sessions,
- filters=(sessions.sid == self.sid)
- & (
- PseudoColumn(f"({Now()} - {sessions.lastupdate.get_sql()})") < get_expiry_period_for_query()
- ),
- fieldname=["user", "sessiondata"],
- order_by=None,
- )
+ now = frappe.utils.now()
- if rec:
- data = frappe._dict(frappe.safe_eval(rec and rec[0][1] or "{}"))
- data.user = rec[0][0]
+ record = (
+ frappe.qb.from_(sessions)
+ .select(sessions.user, sessions.sessiondata)
+ .where(sessions.sid == self.sid)
+ .where(
+ (UnixTimestamp(now) - UnixTimestamp(sessions.lastupdate)) < get_expiry_period_for_query()
+ )
+ ).run()
+
+ if record:
+ data = frappe._dict(frappe.safe_eval(record and record[0][1] or "{}"))
+ data.user = record[0][0]
else:
self._delete_session()
data = None
From d353662b53aea7e4232de456bbcdf2de83f0fc99 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 16:21:00 +0530
Subject: [PATCH 29/42] fix: Session insert using system time
NOW() is server time not system time.
---
frappe/sessions.py | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/frappe/sessions.py b/frappe/sessions.py
index 94ad991c3a..d14a4211af 100644
--- a/frappe/sessions.py
+++ b/frappe/sessions.py
@@ -18,7 +18,7 @@ import frappe.translate
import frappe.utils
from frappe import _
from frappe.cache_manager import clear_user_cache
-from frappe.query_builder import DocType, Order
+from frappe.query_builder import Order
from frappe.query_builder.functions import UnixTimestamp
from frappe.utils import cint, cstr, get_assets_json
@@ -61,7 +61,7 @@ def get_sessions_to_clear(user=None, keep_current=False):
simultaneous_sessions = frappe.db.get_value("User", user, "simultaneous_sessions") or 1
offset = simultaneous_sessions - 1
- session = DocType("Sessions")
+ session = frappe.qb.DocType("Sessions")
session_id = frappe.qb.from_(session).where(session.user == user)
if keep_current:
session_id = session_id.where(session.sid != frappe.session.sid)
@@ -87,7 +87,7 @@ 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:
- table = DocType("Sessions")
+ table = frappe.qb.DocType("Sessions")
user_details = (
frappe.qb.from_(table).where(table.sid == sid).select(table.user).run(as_dict=True)
)
@@ -264,14 +264,17 @@ class Session:
frappe.db.commit()
def insert_session_record(self):
- frappe.db.sql(
- """insert into `tabSessions`
- (`sessiondata`, `user`, `lastupdate`, `sid`, `status`)
- values (%s , %s, NOW(), %s, 'Active')""",
- (str(self.data["data"]), self.data["user"], self.data["sid"]),
- )
- # also add to memcache
+ Sessions = frappe.qb.DocType("Sessions")
+ now = frappe.utils.now()
+
+ (
+ frappe.qb.into(Sessions)
+ .columns(
+ Sessions.sessiondata, Sessions.user, Sessions.lastupdate, Sessions.sid, Sessions.status
+ )
+ .insert((str(self.data["data"]), self.data["user"], now, self.data["sid"], "Active"))
+ ).run()
frappe.cache.hset("session", self.data.sid, self.data)
def resume(self):
@@ -334,7 +337,7 @@ class Session:
return data and data.data
def get_session_data_from_db(self):
- sessions = DocType("Sessions")
+ sessions = frappe.qb.DocType("Sessions")
now = frappe.utils.now()
record = (
From 0e1236b6bef8109c561e524d73b51c3be92c9048 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 17:23:27 +0530
Subject: [PATCH 30/42] refactor: Simplify expiry queries.
Dont rely on mysql dateutils, simply compare dates with a cutoff.
---
frappe/sessions.py | 22 +++++++++++++---------
frappe/tests/test_auth.py | 34 ++++++++++++++++++++++++++++++----
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/frappe/sessions.py b/frappe/sessions.py
index d14a4211af..2709de8fab 100644
--- a/frappe/sessions.py
+++ b/frappe/sessions.py
@@ -19,8 +19,8 @@ import frappe.utils
from frappe import _
from frappe.cache_manager import clear_user_cache
from frappe.query_builder import Order
-from frappe.query_builder.functions import UnixTimestamp
from frappe.utils import cint, cstr, get_assets_json
+from frappe.utils.data import add_to_date
@frappe.whitelist()
@@ -112,12 +112,10 @@ def get_expired_sessions():
"""Returns list of expired sessions"""
sessions = frappe.qb.DocType("Sessions")
- now = frappe.utils.now()
-
return (
frappe.qb.from_(sessions)
.select(sessions.sid)
- .where((UnixTimestamp(now) - UnixTimestamp(sessions.lastupdate)) > get_expiry_period_for_query())
+ .where(sessions.lastupdate < get_expired_threshold())
).run(pluck=True)
@@ -228,7 +226,7 @@ class Session:
sid = frappe.generate_hash()
self.data.user = self.user
- self.data.sid = sid
+ self.sid = self.data.sid = sid
self.data.data.user = self.user
self.data.data.session_ip = frappe.local.request_ip
if self.user != "Guest":
@@ -338,15 +336,12 @@ class Session:
def get_session_data_from_db(self):
sessions = frappe.qb.DocType("Sessions")
- now = frappe.utils.now()
record = (
frappe.qb.from_(sessions)
.select(sessions.user, sessions.sessiondata)
.where(sessions.sid == self.sid)
- .where(
- (UnixTimestamp(now) - UnixTimestamp(sessions.lastupdate)) < get_expiry_period_for_query()
- )
+ .where(sessions.lastupdate > get_expired_threshold())
).run()
if record:
@@ -420,6 +415,15 @@ def get_expiry_in_seconds(expiry=None):
return (cint(parts[0]) * 3600) + (cint(parts[1]) * 60) + cint(parts[2])
+def get_expired_threshold():
+ """Get cutoff time before which all sessions are considered expired."""
+
+ now = frappe.utils.now()
+ expiry_in_seconds = get_expiry_in_seconds()
+
+ return add_to_date(now, seconds=-expiry_in_seconds, as_string=True)
+
+
def get_expiry_period():
exp_sec = frappe.defaults.get_global_default("session_expiry") or "06:00:00"
diff --git a/frappe/tests/test_auth.py b/frappe/tests/test_auth.py
index d4cfe4451e..6aa8d2cc2f 100644
--- a/frappe/tests/test_auth.py
+++ b/frappe/tests/test_auth.py
@@ -1,14 +1,18 @@
# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
# License: MIT. See LICENSE
import time
+from unittest.mock import patch
import requests
import frappe
-import frappe.utils
from frappe.auth import LoginAttemptTracker
from frappe.frappeclient import AuthError, FrappeClient
+from frappe.sessions import Session, get_expired_sessions, get_expiry_in_seconds
+from frappe.tests.test_api import FrappeAPITestCase
from frappe.tests.utils import FrappeTestCase
+from frappe.utils import get_site_url, now
+from frappe.utils.data import add_to_date
from frappe.www.login import _generate_temporary_login_link
@@ -26,9 +30,7 @@ class TestAuth(FrappeTestCase):
@classmethod
def setUpClass(cls):
super().setUpClass()
- cls.HOST_NAME = frappe.get_site_config().host_name or frappe.utils.get_site_url(
- frappe.local.site
- )
+ cls.HOST_NAME = frappe.get_site_config().host_name or get_site_url(frappe.local.site)
cls.test_user_email = "test_auth@test.com"
cls.test_user_name = "test_auth_user"
cls.test_user_mobile = "+911234567890"
@@ -197,3 +199,27 @@ class TestLoginAttemptTracker(FrappeTestCase):
tracker.add_failure_attempt()
self.assertTrue(tracker.is_user_allowed())
+
+
+class TestSessionExpirty(FrappeAPITestCase):
+ def test_session_expires(self):
+ sid = self.sid # triggers login for test case login
+ s: Session = frappe.local.session_obj
+
+ expiry_in = get_expiry_in_seconds()
+ session_created = now()
+
+ # Try with 1% increments of times, it should always work
+ for step in range(0, 100, 1):
+ seconds_elapsed = expiry_in * step / 100
+
+ time_now = add_to_date(session_created, seconds=seconds_elapsed, as_string=True)
+ with patch("frappe.utils.now", return_value=time_now):
+ data = s.get_session_data_from_db()
+ self.assertEqual(data.user, "Administrator")
+
+ # 1% higher should immediately expire
+ time_now = add_to_date(session_created, seconds=expiry_in * 1.01, as_string=True)
+ with patch("frappe.utils.now", return_value=time_now):
+ self.assertIn(sid, get_expired_sessions())
+ self.assertFalse(s.get_session_data_from_db())
From c73d9fb7838178bb3d01db61ef3c76c38157b0dd Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 19:47:04 +0530
Subject: [PATCH 31/42] test: add perf test helper for counting rows read
---
frappe/tests/test_db.py | 21 ---------------------
frappe/tests/test_perf.py | 20 ++++++++++++++++++++
frappe/tests/utils.py | 20 ++++++++++++++++++++
3 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py
index f026726cfa..e8afba5d96 100644
--- a/frappe/tests/test_db.py
+++ b/frappe/tests/test_db.py
@@ -140,27 +140,6 @@ class TestDB(FrappeTestCase):
frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0),
)
- def test_get_value_limits(self):
- # check both dict and list style filters
- filters = [{"enabled": 1}, [["enabled", "=", 1]]]
- for filter in filters:
- self.assertEqual(1, len(frappe.db.get_values("User", filters=filter, limit=1)))
- # count of last touched rows as per DB-API 2.0 https://peps.python.org/pep-0249/#rowcount
- self.assertGreaterEqual(1, cint(frappe.db._cursor.rowcount))
- self.assertEqual(2, len(frappe.db.get_values("User", filters=filter, limit=2)))
- self.assertGreaterEqual(2, cint(frappe.db._cursor.rowcount))
-
- # without limits length == count
- self.assertEqual(
- len(frappe.db.get_values("User", filters=filter)), frappe.db.count("User", filter)
- )
-
- frappe.db.get_value("User", filters=filter)
- self.assertGreaterEqual(1, cint(frappe.db._cursor.rowcount))
-
- frappe.db.exists("User", filter)
- self.assertGreaterEqual(1, cint(frappe.db._cursor.rowcount))
-
def test_escape(self):
frappe.db.escape("香港濟生堂製藥有限公司 - IT".encode())
diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py
index 65f03c2f13..cc7d0b031d 100644
--- a/frappe/tests/test_perf.py
+++ b/frappe/tests/test_perf.py
@@ -27,6 +27,7 @@ from frappe.model.base_document import get_controller
from frappe.query_builder.utils import db_type_is
from frappe.tests.test_query_builder import run_only_if
from frappe.tests.utils import FrappeTestCase
+from frappe.utils import cint
from frappe.website.path_resolver import PathResolver
@@ -70,6 +71,25 @@ class TestPerformance(FrappeTestCase):
with self.assertQueryCount(0):
get_controller("User")
+ def test_get_value_limits(self):
+ # check both dict and list style filters
+ filters = [{"enabled": 1}, [["enabled", "=", 1]]]
+ for filter in filters:
+ with self.assertRowsRead(1):
+ self.assertEqual(1, len(frappe.db.get_values("User", filters=filter, limit=1)))
+ with self.assertRowsRead(2):
+ self.assertEqual(2, len(frappe.db.get_values("User", filters=filter, limit=2)))
+
+ self.assertEqual(
+ len(frappe.db.get_values("User", filters=filter)), frappe.db.count("User", filter)
+ )
+
+ with self.assertRowsRead(1):
+ frappe.db.get_value("User", filters=filter)
+
+ with self.assertRowsRead(1):
+ frappe.db.exists("User", filter)
+
def test_db_value_cache(self):
"""Link validation if repeated should just use db.value_cache, hence no extra queries"""
doc = frappe.get_last_doc("User")
diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py
index 6e6c72052a..762cd885b0 100644
--- a/frappe/tests/utils.py
+++ b/frappe/tests/utils.py
@@ -92,6 +92,26 @@ class FrappeTestCase(unittest.TestCase):
finally:
frappe.db.sql = orig_sql
+ @contextmanager
+ def assertRowsRead(self, count):
+ rows_read = 0
+
+ def _sql_with_count(*args, **kwargs):
+ nonlocal rows_read
+
+ ret = orig_sql(*args, **kwargs)
+ # count of last touched rows as per DB-API 2.0 https://peps.python.org/pep-0249/#rowcount
+ rows_read += cint(frappe.db._cursor.rowcount)
+ return ret
+
+ try:
+ orig_sql = frappe.db.sql
+ frappe.db.sql = _sql_with_count
+ yield
+ self.assertLessEqual(rows_read, count, msg="Queries read more rows than expected")
+ finally:
+ frappe.db.sql = orig_sql
+
class MockedRequestTestCase(FrappeTestCase):
def setUp(self):
From e4bae5c8315c25da3402739dfa2c615ba68667f1 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Wed, 28 Jun 2023 19:39:37 +0530
Subject: [PATCH 32/42] perf: faster doc shared checks
- If document, explicitly query document
- If checking doctype then put limit and only see if 1 record is
returned.
---
frappe/core/doctype/docshare/test_docshare.py | 17 ++++++++++++++++-
frappe/permissions.py | 17 ++++++++++++-----
frappe/share.py | 16 ++++++++++++----
3 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/frappe/core/doctype/docshare/test_docshare.py b/frappe/core/doctype/docshare/test_docshare.py
index e080b0d4ff..125e829d9b 100644
--- a/frappe/core/doctype/docshare/test_docshare.py
+++ b/frappe/core/doctype/docshare/test_docshare.py
@@ -33,13 +33,28 @@ class TestDocShare(FrappeTestCase):
def test_doc_permission(self):
frappe.set_user(self.user)
+
self.assertFalse(self.event.has_permission())
frappe.set_user("Administrator")
frappe.share.add("Event", self.event.name, self.user)
frappe.set_user(self.user)
- self.assertTrue(self.event.has_permission())
+ # PERF: All share permission check should happen with maximum 1 query.
+ with self.assertRowsRead(1):
+ self.assertTrue(self.event.has_permission())
+
+ second_event = frappe.get_doc(
+ {
+ "doctype": "Event",
+ "subject": "test share event 2",
+ "starts_on": "2015-01-01 10:00:00",
+ "event_type": "Private",
+ }
+ ).insert()
+ frappe.share.add("Event", second_event.name, self.user)
+ with self.assertRowsRead(1):
+ self.assertTrue(self.event.has_permission())
def test_share_permission(self):
frappe.share.add("Event", self.event.name, self.user, write=1, share=1)
diff --git a/frappe/permissions.py b/frappe/permissions.py
index 633d0e278d..e8ca0ecb3c 100644
--- a/frappe/permissions.py
+++ b/frappe/permissions.py
@@ -118,17 +118,24 @@ def has_permission(
def false_if_not_shared():
if ptype in ("read", "write", "share", "submit", "email", "print"):
- shared = frappe.share.get_shared(
- doctype, user, ["read" if ptype in ("email", "print") else ptype]
- )
+
+ rights = ["read" if ptype in ("email", "print") else ptype]
if doc:
doc_name = get_doc_name(doc)
- if doc_name in shared:
+ shared = frappe.share.get_shared(
+ doctype,
+ user,
+ rights=rights,
+ filters=[["share_name", "=", doc_name]],
+ limit=1,
+ )
+
+ if shared:
if ptype in ("read", "write", "share", "submit") or meta.permissions[0].get(ptype):
return True
- elif shared:
+ elif frappe.share.get_shared(doctype, user, rights=rights, limit=1):
# if atleast one shared doc of that type, then return True
# this is used in db_query to check if permission on DocType
return True
diff --git a/frappe/share.py b/frappe/share.py
index 6c2fb356a6..c068e063b2 100644
--- a/frappe/share.py
+++ b/frappe/share.py
@@ -141,7 +141,7 @@ def get_users(doctype, name):
)
-def get_shared(doctype, user=None, rights=None):
+def get_shared(doctype, user=None, rights=None, *, filters=None, limit=None):
"""Get list of shared document names for given user and DocType.
:param doctype: DocType of which shared names are queried.
@@ -154,14 +154,22 @@ def get_shared(doctype, user=None, rights=None):
if not rights:
rights = ["read"]
- filters = [[right, "=", 1] for right in rights]
- filters += [["share_doctype", "=", doctype]]
+ share_filters = [[right, "=", 1] for right in rights]
+ share_filters += [["share_doctype", "=", doctype]]
+ if filters:
+ share_filters += filters
+
or_filters = [["user", "=", user]]
if user != "Guest":
or_filters += [["everyone", "=", 1]]
shared_docs = frappe.get_all(
- "DocShare", fields=["share_name"], filters=filters, or_filters=or_filters, order_by=None
+ "DocShare",
+ fields=["share_name"],
+ filters=share_filters,
+ or_filters=or_filters,
+ order_by=None,
+ limit_page_length=limit,
)
return [doc.share_name for doc in shared_docs]
From 85145c2c1189a538280c9cc4d4cf78acece2e629 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Wed, 28 Jun 2023 17:22:40 +0200
Subject: [PATCH 33/42] fix: styling of group by button
---
frappe/public/scss/desk/report.scss | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/public/scss/desk/report.scss b/frappe/public/scss/desk/report.scss
index 03e2763cd7..1db38529ed 100644
--- a/frappe/public/scss/desk/report.scss
+++ b/frappe/public/scss/desk/report.scss
@@ -100,11 +100,11 @@
.group-by-button {
margin: 5px;
- max-width: 125px;
}
.group-by-button.btn-primary-light {
color: var(--text-on-blue);
+ outline: 1px solid var(--bg-dark-blue);
}
.group-by-icon.active {
From 1668ba7d9eb7aa4dcff3233e95195a8720cec162 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Thu, 29 Jun 2023 16:35:10 +0530
Subject: [PATCH 34/42] feat: Namespace all RQ jobs to site
---
frappe/utils/background_jobs.py | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index 7de7ef6692..8254a6fb87 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -84,9 +84,8 @@ def enqueue(
# To handle older implementations
is_async = kwargs.pop("async", is_async)
- if job_id:
- # namespace job ids to sites
- job_id = create_job_id(job_id)
+ # namespace job ids to sites
+ job_id = create_job_id(job_id)
if job_name:
deprecation_warning("Using enqueue with `job_name` is deprecated, use `job_id` instead.")
@@ -481,6 +480,9 @@ def test_job(s):
def create_job_id(job_id: str) -> str:
"""Generate unique job id for deduplication"""
+
+ if not job_id:
+ job_id = str(uuid4())
return f"{frappe.local.site}::{job_id}"
From 1092eef7bd8b71e741109db6f835af30af22a6c0 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Thu, 29 Jun 2023 16:42:37 +0530
Subject: [PATCH 35/42] perf: faster pending jobs check
---
frappe/commands/scheduler.py | 4 ++--
frappe/tests/test_commands.py | 15 +++++++++++++++
frappe/utils/doctor.py | 9 +++++++++
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py
index 6af3a2403e..5d453e3568 100755
--- a/frappe/commands/scheduler.py
+++ b/frappe/commands/scheduler.py
@@ -240,14 +240,14 @@ def start_worker_pool(queue, quiet=False, num_workers=2, burst=False):
@click.option("--site", help="site name")
@pass_context
def ready_for_migration(context, site=None):
- from frappe.utils.doctor import get_pending_jobs
+ from frappe.utils.doctor import any_job_pending
if not site:
site = get_site(context)
try:
frappe.init(site=site)
- pending_jobs = get_pending_jobs(site=site)
+ pending_jobs = any_job_pending(site=site)
if pending_jobs:
print(f"NOT READY for migration: site {site} has pending background jobs")
diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py
index 7bda577bb6..cdf93a1870 100644
--- a/frappe/tests/test_commands.py
+++ b/frappe/tests/test_commands.py
@@ -20,9 +20,11 @@ from unittest.mock import patch
import click
from click import Command
from click.testing import CliRunner, Result
+from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
# imports - module imports
import frappe
+import frappe.commands.scheduler
import frappe.commands.site
import frappe.commands.utils
import frappe.recorder
@@ -760,6 +762,19 @@ class TestBenchBuild(BaseTestCommands):
)
+class TestSchedulerUtils(BaseTestCommands):
+ # Retry just in case there are stuck queued jobs
+ @retry(
+ retry=retry_if_exception_type(AssertionError),
+ stop=stop_after_attempt(3),
+ wait=wait_fixed(3),
+ reraise=True,
+ )
+ def test_ready_for_migrate(self):
+ with cli(frappe.commands.scheduler.ready_for_migration) as result:
+ self.assertEqual(result.exit_code, 0)
+
+
class TestCommandUtils(FrappeTestCase):
def test_bench_helper(self):
from frappe.utils.bench_helper import get_app_groups
diff --git a/frappe/utils/doctor.py b/frappe/utils/doctor.py
index 5b12a01990..002fd8b154 100644
--- a/frappe/utils/doctor.py
+++ b/frappe/utils/doctor.py
@@ -79,6 +79,15 @@ def get_pending_jobs(site=None):
return jobs_per_queue
+def any_job_pending(site: str) -> bool:
+ for queue in get_queue_list():
+ q = get_queue(queue)
+ for job_id in q.get_job_ids():
+ if job_id.startswith(site):
+ return True
+ return False
+
+
def check_number_of_workers():
return len(get_workers())
From 3ae2d19073598afad874344d35813b9937916889 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Thu, 29 Jun 2023 16:56:59 +0530
Subject: [PATCH 36/42] perf: efficient RQ jobs filters
Assuming site's job start with site prefix it's much easier to filter
jobs by looking at job IDs instead of fetching entire job in memory.
---
frappe/core/doctype/rq_job/rq_job.py | 37 ++++++++++++++--------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/frappe/core/doctype/rq_job/rq_job.py b/frappe/core/doctype/rq_job/rq_job.py
index 391ccd8dfd..9a67fae586 100644
--- a/frappe/core/doctype/rq_job/rq_job.py
+++ b/frappe/core/doctype/rq_job/rq_job.py
@@ -63,23 +63,15 @@ class RQJob(Document):
order_desc = "desc" in args.get("order_by", "")
- matched_job_ids = RQJob.get_matching_job_ids(args)
+ matched_job_ids = RQJob.get_matching_job_ids(args)[start : start + page_length]
- jobs = []
- for job_ids in create_batch(matched_job_ids, 100):
- jobs.extend(
- serialize_job(job)
- for job in Job.fetch_many(job_ids=job_ids, connection=get_redis_conn())
- if job and for_current_site(job)
- )
- if len(jobs) > start + page_length:
- # we have fetched enough. This is inefficient but because of site filtering TINA
- break
+ conn = get_redis_conn()
+ jobs = [serialize_job(job) for job in Job.fetch_many(job_ids=matched_job_ids, connection=conn)]
- return sorted(jobs, key=lambda j: j.modified, reverse=order_desc)[start : start + page_length]
+ return sorted(jobs, key=lambda j: j.modified, reverse=order_desc)
@staticmethod
- def get_matching_job_ids(args):
+ def get_matching_job_ids(args) -> list[str]:
filters = make_filter_dict(args.get("filters"))
queues = _eval_filters(filters.get("queue"), QUEUES)
@@ -92,7 +84,7 @@ class RQJob(Document):
for status in statuses:
matched_job_ids.extend(fetch_job_ids(queue, status))
- return matched_job_ids
+ return filter_current_site_jobs(matched_job_ids)
@check_permissions
def delete(self):
@@ -155,6 +147,12 @@ def for_current_site(job: Job) -> bool:
return job.kwargs.get("site") == frappe.local.site
+def filter_current_site_jobs(job_ids: list[str]) -> list[str]:
+ site = frappe.local.site
+
+ return [j for j in job_ids if j.startswith(site)]
+
+
def _eval_filters(filter, values: list[str]) -> list[str]:
if filter:
operator, operand = filter
@@ -186,10 +184,13 @@ def remove_failed_jobs():
frappe.only_for("System Manager")
for queue in get_queues():
fail_registry = queue.failed_job_registry
- for job_ids in create_batch(fail_registry.get_job_ids(), 100):
- for job in Job.fetch_many(job_ids=job_ids, connection=get_redis_conn()):
- if job and for_current_site(job):
- fail_registry.remove(job, delete_job=True)
+ failed_jobs = filter_current_site_jobs(fail_registry.get_job_ids())
+
+ # Delete in batches to avoid loading too many things in memory
+ conn = get_redis_conn()
+ for job_ids in create_batch(failed_jobs, 100):
+ for job in Job.fetch_many(job_ids=job_ids, connection=conn):
+ job and fail_registry.remove(job, delete_job=True)
def get_all_queued_jobs():
From d57c552e26aba55e26701b20e52677e757e33811 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Thu, 29 Jun 2023 17:20:48 +0530
Subject: [PATCH 37/42] feat: frappe.enqueue with deduplication
use deduplicate=True and set job_id for automatic and mostly sane job deduplication.
---
frappe/core/doctype/rq_job/test_rq_job.py | 16 +++++++++++++-
frappe/utils/background_jobs.py | 26 ++++++++++++++++++++---
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py
index 6512902fb3..f5d5f89ed4 100644
--- a/frappe/core/doctype/rq_job/test_rq_job.py
+++ b/frappe/core/doctype/rq_job/test_rq_job.py
@@ -108,13 +108,27 @@ class TestRQJob(FrappeTestCase):
self.assertIn("quitting", cstr(stderr))
@timeout(20)
- def test_job_id_dedup(self):
+ def test_job_id_manual_dedup(self):
job_id = "test_dedup"
job = frappe.enqueue(self.BG_JOB, sleep=5, job_id=job_id)
self.assertTrue(is_job_enqueued(job_id))
self.check_status(job, "finished")
self.assertFalse(is_job_enqueued(job_id))
+ @timeout(20)
+ def test_auto_job_dedup(self):
+ job_id = "test_dedup"
+ job1 = frappe.enqueue(self.BG_JOB, sleep=2, job_id=job_id, deduplicate=True)
+ job2 = frappe.enqueue(self.BG_JOB, sleep=5, job_id=job_id, deduplicate=True)
+ self.assertIsNone(job2)
+ self.check_status(job1, "finished") # wait
+
+ # Failed jobs last longer, subsequent job should still pass with same ID.
+ job3 = frappe.enqueue(self.BG_JOB, fail=True, job_id=job_id, deduplicate=True)
+ self.check_status(job3, "failed")
+ job4 = frappe.enqueue(self.BG_JOB, sleep=1, job_id=job_id, deduplicate=True)
+ self.check_status(job4, "finished")
+
@timeout(20)
def test_enqueue_after_commit(self):
job_id = frappe.generate_hash()
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index 8254a6fb87..be5291b771 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -66,6 +66,7 @@ def enqueue(
on_failure: Callable = None,
at_front: bool = False,
job_id: str = None,
+ deduplicate=False,
**kwargs,
) -> Job | Any:
"""
@@ -79,11 +80,26 @@ def enqueue(
:param job_name: [DEPRECATED] can be used to name an enqueue call, which can be used to prevent duplicate calls
:param now: if now=True, the method is executed via frappe.call
:param kwargs: keyword arguments to be passed to the method
+ :param deduplicate: do not re-queue job if it's already queued, requires job_id.
:param job_id: Assigning unique job id, which can be checked using `is_job_enqueued`
"""
# To handle older implementations
is_async = kwargs.pop("async", is_async)
+ if deduplicate:
+ if not job_id:
+ frappe.throw(_("`job_id` paramater is required for deduplication."))
+ job = get_job(job_id)
+ if job and job.get_status() in (JobStatus.QUEUED, JobStatus.STARTED):
+ frappe.logger().debug(f"Not queueing job {job.id} because it is in queue already")
+ return
+ elif job:
+ # delete job to avoid argument issues related to job args
+ # https://github.com/rq/rq/issues/793
+ job.delete()
+
+ # If job exists and is completed then delete it before re-queue
+
# namespace job ids to sites
job_id = create_job_id(job_id)
@@ -492,9 +508,13 @@ def is_job_enqueued(job_id: str) -> bool:
def get_job_status(job_id: str) -> JobStatus | None:
"""Get RQ job status, returns None if job is not found."""
+ job = get_job(job_id)
+ if job:
+ return job.get_status()
+
+
+def get_job(job_id: str) -> Job:
try:
- job = Job.fetch(create_job_id(job_id), connection=get_redis_conn())
+ return Job.fetch(create_job_id(job_id), connection=get_redis_conn())
except NoSuchJobError:
return None
-
- return job.get_status()
From 31d05b466a0b0cf0ae0b0bc4a107c1ca97e84395 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Thu, 29 Jun 2023 17:14:53 +0530
Subject: [PATCH 38/42] perf: Email queue dedup using job id instead of name
---
frappe/email/queue.py | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/frappe/email/queue.py b/frappe/email/queue.py
index 0df88ebd5c..500a126f72 100755
--- a/frappe/email/queue.py
+++ b/frappe/email/queue.py
@@ -132,7 +132,6 @@ def return_unsubscribed_page(email, doctype, name):
def flush(from_test=False):
"""flush email queue, every time: called from scheduler"""
from frappe.email.doctype.email_queue.email_queue import send_mail
- from frappe.utils.background_jobs import get_jobs
# To avoid running jobs inside unit tests
if frappe.are_emails_muted():
@@ -142,24 +141,16 @@ def flush(from_test=False):
if cint(frappe.db.get_default("suspend_email_queue")) == 1:
return
- try:
- queued_jobs = set(get_jobs(site=frappe.local.site, key="job_name")[frappe.local.site])
- except Exception:
- queued_jobs = set()
-
for row in get_queue():
try:
- job_name = f"email_queue_sendmail_{row.name}"
- if job_name not in queued_jobs:
- frappe.enqueue(
- method=send_mail,
- email_queue_name=row.name,
- now=from_test,
- job_name=job_name,
- queue="short",
- )
- else:
- frappe.logger().debug(f"Not queueing job {job_name} because it is in queue already")
+ frappe.enqueue(
+ method=send_mail,
+ email_queue_name=row.name,
+ now=from_test,
+ job_id=f"email_queue_sendmail_{row.name}",
+ queue="short",
+ dedupicate=True,
+ )
except Exception:
frappe.get_doc("Email Queue", row.name).log_error()
From a52485cc53269e0c5d73b1c3d174e6dad4fcb6fe Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Thu, 29 Jun 2023 17:34:29 +0530
Subject: [PATCH 39/42] feat: RQ jobs can show count
---
frappe/core/doctype/rq_job/rq_job.py | 3 +--
frappe/core/doctype/rq_job/rq_job_list.js | 3 +--
frappe/tests/test_background_jobs.py | 6 ++++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/frappe/core/doctype/rq_job/rq_job.py b/frappe/core/doctype/rq_job/rq_job.py
index 9a67fae586..659c81e5c4 100644
--- a/frappe/core/doctype/rq_job/rq_job.py
+++ b/frappe/core/doctype/rq_job/rq_job.py
@@ -99,8 +99,7 @@ class RQJob(Document):
@staticmethod
def get_count(args) -> int:
- # Can not be implemented efficiently due to site filtering hence ignored.
- return 0
+ return len(RQJob.get_matching_job_ids(args))
# None of these methods apply to virtual job doctype, overriden for sanity.
@staticmethod
diff --git a/frappe/core/doctype/rq_job/rq_job_list.js b/frappe/core/doctype/rq_job/rq_job_list.js
index aa05b411ba..7d140d668f 100644
--- a/frappe/core/doctype/rq_job/rq_job_list.js
+++ b/frappe/core/doctype/rq_job/rq_job_list.js
@@ -15,7 +15,6 @@ frappe.listview_settings["RQ Job"] = {
);
if (listview.list_view_settings) {
- listview.list_view_settings.disable_count = 1;
listview.list_view_settings.disable_sidebar_stats = 1;
}
@@ -57,6 +56,6 @@ frappe.listview_settings["RQ Job"] = {
}
listview.refresh();
- }, 5000);
+ }, 15000);
},
};
diff --git a/frappe/tests/test_background_jobs.py b/frappe/tests/test_background_jobs.py
index b6c1a0d694..99373a84a6 100644
--- a/frappe/tests/test_background_jobs.py
+++ b/frappe/tests/test_background_jobs.py
@@ -10,6 +10,7 @@ from frappe.tests.utils import FrappeTestCase
from frappe.utils.background_jobs import (
RQ_JOB_FAILURE_TTL,
RQ_RESULTS_TTL,
+ create_job_id,
execute_job,
generate_qname,
get_redis_conn,
@@ -54,11 +55,12 @@ class TestBackgroundJobs(FrappeTestCase):
def test_enqueue_call(self):
with patch.object(Queue, "enqueue_call") as mock_enqueue_call:
- frappe.enqueue(
+ job = frappe.enqueue(
"frappe.handler.ping",
queue="short",
timeout=300,
kwargs={"site": frappe.local.site},
+ job_id="test",
)
mock_enqueue_call.assert_called_once_with(
@@ -78,7 +80,7 @@ class TestBackgroundJobs(FrappeTestCase):
at_front=False,
failure_ttl=RQ_JOB_FAILURE_TTL,
result_ttl=RQ_RESULTS_TTL,
- job_id=None,
+ job_id=create_job_id("test"),
)
def test_job_hooks(self):
From c6a46e68122a9f0bc7309f6c76082e3ba6f424fd Mon Sep 17 00:00:00 2001
From: Smit Vora
Date: Fri, 30 Jun 2023 13:35:01 +0530
Subject: [PATCH 40/42] fix: correct condition check for dynamic filters
(#21530)
Co-authored-by: Sagar Vora
---
frappe/public/js/frappe/utils/dashboard_utils.js | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/frappe/public/js/frappe/utils/dashboard_utils.js b/frappe/public/js/frappe/utils/dashboard_utils.js
index 488aa20742..1058101fff 100644
--- a/frappe/public/js/frappe/utils/dashboard_utils.js
+++ b/frappe/public/js/frappe/utils/dashboard_utils.js
@@ -202,14 +202,16 @@ frappe.dashboard_utils = {
},
get_all_filters(doc) {
- let filters = JSON.parse(doc.filters_json || "null");
- let dynamic_filters = JSON.parse(doc.dynamic_filters_json || "null");
+ let filters = doc.filters_json ? JSON.parse(doc.filters_json) : null;
+ let dynamic_filters = doc.dynamic_filters_json
+ ? JSON.parse(doc.dynamic_filters_json)
+ : null;
- if (!dynamic_filters) {
+ if (!dynamic_filters || !Object.keys(dynamic_filters).length) {
return filters;
}
- if ($.isArray(dynamic_filters)) {
+ if (Array.isArray(dynamic_filters)) {
dynamic_filters.forEach((f) => {
try {
f[3] = eval(f[3]);
From 69d0060bdf631ed9888eed17eeb372636f566f4f Mon Sep 17 00:00:00 2001
From: Corentin Flr <10946971+cogk@users.noreply.github.com>
Date: Fri, 30 Jun 2023 11:52:29 +0200
Subject: [PATCH 41/42] chore: format code
---
frappe/website/page_renderers/static_page.py | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/frappe/website/page_renderers/static_page.py b/frappe/website/page_renderers/static_page.py
index f992743c6a..00162c0772 100644
--- a/frappe/website/page_renderers/static_page.py
+++ b/frappe/website/page_renderers/static_page.py
@@ -8,7 +8,18 @@ import frappe
from frappe.website.page_renderers.base_renderer import BaseRenderer
from frappe.website.utils import is_binary_file
-UNSUPPORTED_STATIC_PAGE_TYPES = ("html", "md", "js", "xml", "css", "txt", "py", "pyc", "json", "pyo")
+UNSUPPORTED_STATIC_PAGE_TYPES = (
+ "css",
+ "html",
+ "js",
+ "json",
+ "md",
+ "py",
+ "pyc",
+ "pyo",
+ "txt",
+ "xml",
+)
class StaticPage(BaseRenderer):
From 441495b561ed6df9a05ce6716c6439d58654ac6d Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 30 Jun 2023 17:57:40 +0530
Subject: [PATCH 42/42] refactor!: Drop support for currentsite.txt (#21536)
* refactor!: Drop currentsite.txt
- `bench use` will continue to work.
- Instead of txt file use common_site_config to set default site using `default_site` key.
- `FRAPPE_SITE` environment variable also works
* fix(DX): warn if non-empty currentsite.txt is present
---
frappe/commands/site.py | 7 +++++--
frappe/utils/bench_helper.py | 18 ++++++++++++++----
node_utils.js | 4 ----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/frappe/commands/site.py b/frappe/commands/site.py
index d606bb78cf..01b3be9590 100644
--- a/frappe/commands/site.py
+++ b/frappe/commands/site.py
@@ -700,9 +700,12 @@ def _use(site, sites_path="."):
def use(site, sites_path="."):
+ from frappe.installer import update_site_config
+
if os.path.exists(os.path.join(sites_path, site)):
- with open(os.path.join(sites_path, "currentsite.txt"), "w") as sitefile:
- sitefile.write(site)
+ sites_path = os.getcwd()
+ conifg = os.path.join(sites_path, "common_site_config.json")
+ update_site_config("default_site", site, validate=False, site_config_path=conifg)
print(f"Current Site set to {site}")
else:
print(f"Site {site} does not exist")
diff --git a/frappe/utils/bench_helper.py b/frappe/utils/bench_helper.py
index 01e611f021..14db5c8449 100644
--- a/frappe/utils/bench_helper.py
+++ b/frappe/utils/bench_helper.py
@@ -4,6 +4,7 @@ import os
import traceback
import warnings
from pathlib import Path
+from textwrap import dedent
import click
@@ -52,10 +53,19 @@ def get_sites(site_arg: str) -> list[str]:
return [site_arg]
elif os.environ.get("FRAPPE_SITE"):
return [os.environ.get("FRAPPE_SITE")]
- elif os.path.exists("currentsite.txt"):
- with open("currentsite.txt") as f:
- if site := f.read().strip():
- return [site]
+ elif default_site := frappe.get_conf().default_site:
+ return [default_site]
+ # This is not supported, just added here for warning.
+ elif (site := frappe.read_file("currentsite.txt")) and site.strip():
+ click.secho(
+ dedent(
+ f"""
+ WARNING: currentsite.txt is not supported anymore for setting default site. Use following command to set it as default site.
+ $ bench use {site}"""
+ ),
+ fg="red",
+ )
+
return []
diff --git a/node_utils.js b/node_utils.js
index 0b8c455875..4835a84433 100644
--- a/node_utils.js
+++ b/node_utils.js
@@ -31,10 +31,6 @@ function get_conf() {
if (process.env.FRAPPE_SITE) {
conf.default_site = process.env.FRAPPE_SITE;
}
- if (fs.existsSync("sites/currentsite.txt")) {
- conf.default_site = fs.readFileSync("sites/currentsite.txt").toString().trim();
- }
-
return conf;
}