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/commands/scheduler.py b/frappe/commands/scheduler.py index 36fa81f8a5..5d453e3568 100755 --- a/frappe/commands/scheduler.py +++ b/frappe/commands/scheduler.py @@ -215,18 +215,39 @@ 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 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") @@ -251,5 +272,6 @@ commands = [ show_pending_jobs, start_scheduler, start_worker, + start_worker_pool, trigger_scheduler_event, ] 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/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/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/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 7770226e79..8778826b56 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 @@ -718,40 +719,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/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index bbe8bb6d1a..1e7e698062 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -611,46 +611,42 @@ 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", - "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname, - "content": "Testing User", - } + 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_home.txt", - "content": "User Home", - } + 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_system_manager.txt", - "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname, - "content": "Testing System Manager", - } + 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_home.txt", - "content": "System Manager Home", - } + 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"]] @@ -664,15 +660,47 @@ 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.new_doc( + "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.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") + 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") diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 832be49f3c..623e358b6c 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, @@ -45,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 @@ -114,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() @@ -156,7 +156,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/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) diff --git a/frappe/core/doctype/rq_job/rq_job.py b/frappe/core/doctype/rq_job/rq_job.py index 391ccd8dfd..659c81e5c4 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): @@ -107,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 @@ -155,6 +146,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 +183,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(): 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/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py index c39717cfd8..f5d5f89ed4 100644 --- a/frappe/core/doctype/rq_job/test_rq_job.py +++ b/frappe/core/doctype/rq_job/test_rq_job.py @@ -97,13 +97,38 @@ class TestRQJob(FrappeTestCase): self.assertIn("quitting", cstr(stderr)) @timeout(20) - def test_job_id_dedup(self): + 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_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/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 = $(` +
+
+ ${message}
${cta} → +
+
+ + + +
+
+ `).appendTo(listview.page.sidebar); + } catch (error) { + console.error(error); + } +} 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/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() diff --git a/frappe/hooks.py b/frappe/hooks.py index ed2b25bc1b..85a28feb39 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 = { @@ -213,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/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/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 6fa24bfb67..5b8c653198 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: + 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 @@ -112,16 +120,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 + + except Exception as e: + frappe.logger().debug({"enqueue_webhook_error": e}) + log_request(webhook.name, doc.name, request_url, headers, data) + return - r = None for i in range(3): try: r = requests.request( 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, } 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/public/js/frappe/form/controls/autocomplete.js b/frappe/public/js/frappe/form/controls/autocomplete.js index 27bf75e807..7b91a2031c 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/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 `