From 01f4ba206188c747a1eebf42fe8d5dee950b272e Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 22 Dec 2021 12:58:06 +0530 Subject: [PATCH 01/12] feat: `frappe.enqueue` and `frappe.call` for server scripts --- .../server_script/server_script_utils.py | 7 -- frappe/handler.py | 20 +++++- frappe/tests/test_safe_exec.py | 25 ++++++- frappe/utils/safe_exec.py | 67 +++++++++++++++++-- 4 files changed, 101 insertions(+), 18 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script_utils.py b/frappe/core/doctype/server_script/server_script_utils.py index b4cfdf0a17..d8c945fb6d 100644 --- a/frappe/core/doctype/server_script/server_script_utils.py +++ b/frappe/core/doctype/server_script/server_script_utils.py @@ -19,13 +19,6 @@ EVENT_MAP = { 'on_update_after_submit': 'After Save (Submitted Document)' } -def run_server_script_api(method): - # called via handler, execute an API script - script_name = get_server_script_map().get('_api', {}).get(method) - if script_name: - frappe.get_doc('Server Script', script_name).execute_method() - return True - def run_server_script_for_doc_event(doc, event): # run document event method if not event in EVENT_MAP: diff --git a/frappe/handler.py b/frappe/handler.py index 2c2b5723fa..3fd1c096e4 100755 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -12,7 +12,7 @@ from frappe.utils.response import build_response from frappe.utils.csvutils import build_csv_response from frappe.utils.image import optimize_image from mimetypes import guess_type -from frappe.core.doctype.server_script.server_script_utils import run_server_script_api +from frappe.core.doctype.server_script.server_script_utils import get_server_script_map ALLOWED_MIMETYPES = ('image/png', 'image/jpeg', 'application/pdf', 'application/msword', @@ -49,8 +49,9 @@ def execute_cmd(cmd, from_async=False): break # via server script - if run_server_script_api(cmd): - return None + server_script = get_server_script_map().get('_api', {}).get(cmd) + if server_script: + return run_server_script(server_script) try: method = get_attr(cmd) @@ -66,7 +67,20 @@ def execute_cmd(cmd, from_async=False): return frappe.call(method, **frappe.form_dict) + +def run_server_script(server_script): + response = frappe.get_doc('Server Script', server_script).execute_method() + + # some server scripts return output using flags (empty dict by default), + # while others directly modify frappe.response + # return flags if not empty dict (this overwrites frappe.response.message) + if response != {}: + return response + def is_valid_http_method(method): + if frappe.flags.in_safe_exec: + return + http_method = frappe.local.request.method if http_method not in frappe.allowed_http_methods_for_whitelisted_func[method]: diff --git a/frappe/tests/test_safe_exec.py b/frappe/tests/test_safe_exec.py index 783d05ff37..7fec292c49 100644 --- a/frappe/tests/test_safe_exec.py +++ b/frappe/tests/test_safe_exec.py @@ -31,4 +31,27 @@ class TestSafeExec(unittest.TestCase): self.assertEqual(frappe.db.sql("SELECT Max(name) FROM tabUser"), _locals["out"]) def test_safe_query_builder(self): - self.assertRaises(frappe.PermissionError, safe_exec, '''frappe.qb.from_("User").delete().run()''') \ No newline at end of file + self.assertRaises(frappe.PermissionError, safe_exec, '''frappe.qb.from_("User").delete().run()''') + + def test_call(self): + # call non whitelisted method + self.assertRaises( + frappe.PermissionError, + safe_exec, + """frappe.call("frappe.get_user")""" + ) + + # call whitelisted method + safe_exec("""frappe.call("ping")""") + + + def test_enqueue(self): + # enqueue non whitelisted method + self.assertRaises( + frappe.PermissionError, + safe_exec, + """frappe.enqueue("frappe.get_user", now=True)""" + ) + + # enqueue whitelisted method + safe_exec("""frappe.enqueue("ping", now=True)""") diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index f51efefc85..2042c1f2ce 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -14,11 +14,12 @@ import frappe.integrations.utils import frappe.utils import frappe.utils.data from frappe import _ +from frappe.handler import execute_cmd from frappe.frappeclient import FrappeClient from frappe.modules import scrub from frappe.website.utils import get_next_link, get_shade, get_toc from frappe.www.printview import get_visible_columns - +from frappe.utils.background_jobs import enqueue, get_jobs class ServerScriptNotEnabled(frappe.PermissionError): pass @@ -74,7 +75,9 @@ def get_safe_globals(): add_data_utils(datautils) - if "_" in getattr(frappe.local, 'form_dict', {}): + form_dict = getattr(frappe.local, 'form_dict', frappe._dict()) + + if "_" in form_dict: del frappe.local.form_dict["_"] user = getattr(frappe.local, "session", None) and frappe.local.session.user or "Guest" @@ -89,14 +92,16 @@ def get_safe_globals(): dict=dict, log=frappe.log, _dict=frappe._dict, + args=form_dict, frappe=NamespaceDict( + call=call_whitelisted_function, flags=frappe._dict(), format=frappe.format_value, format_value=frappe.format_value, date_format=date_format, time_format=time_format, format_date=frappe.utils.data.global_date_format, - form_dict=getattr(frappe.local, 'form_dict', {}), + form_dict=form_dict, bold=frappe.bold, copy_doc=frappe.copy_doc, errprint=frappe.errprint, @@ -132,6 +137,7 @@ def get_safe_globals(): make_post_request=frappe.integrations.utils.make_post_request, socketio_port=frappe.conf.socketio_port, get_hooks=get_hooks, + enqueue=safe_enqueue, sanitize_html=frappe.utils.sanitize_html, log_error=frappe.log_error ), @@ -147,7 +153,8 @@ def get_safe_globals(): guess_mimetype=mimetypes.guess_type, html2text=html2text, dev_server=1 if frappe._dev_server else 0, - run_script=run_script + run_script=run_script, + is_job_queued=is_job_queued, ) add_module_properties(frappe.exceptions, out.frappe, lambda obj: inspect.isclass(obj) and issubclass(obj, Exception)) @@ -190,6 +197,55 @@ def get_safe_globals(): return out +def is_job_queued(job_name, queue="default"): + ''' + :param job_name: used to identify a queued job, usually dotted path to function + :param queue: should be either long, default or short + ''' + + site = frappe.local.site + queued_jobs = get_jobs(site=site, queue=queue, key='job_name').get(site) + return queued_jobs and job_name in queued_jobs + +def safe_enqueue(function, **kwargs): + ''' + Enqueue function to be executed using a background worker + Accepts frappe.enqueue params like job_name, queue, timeout, etc. + in addition to params to be passed to function + + :param function: whitelised function or API Method set in Server Script + ''' + + return enqueue( + 'frappe.utils.safe_exec.call_whitelisted_function', + function=function, + **kwargs + ) + +def call_whitelisted_function(function, **kwargs): + '''Executes a whitelisted function or Server Script of type API''' + + return call_with_form_dict(lambda: execute_cmd(function), kwargs) + +def run_script(script, **kwargs): + '''run another server script''' + + return call_with_form_dict( + lambda: frappe.get_doc('Server Script', script).execute_method(), + kwargs + ) + +def call_with_form_dict(function, kwargs): + # temporarily update form_dict, to use inside below call + form_dict = getattr(frappe.local, 'form_dict', frappe._dict()) + if kwargs: + frappe.local.form_dict = form_dict.copy().update(kwargs) + + try: + return function() + finally: + frappe.local.form_dict = form_dict + def get_python_builtins(): return { 'abs': abs, @@ -221,9 +277,6 @@ def read_sql(query, *args, **kwargs): raise frappe.PermissionError('Only SELECT SQL allowed in scripting') return frappe.db.sql(query, *args, **kwargs) -def run_script(script): - '''run another server script''' - return frappe.get_doc('Server Script', script).execute_method() def _getitem(obj, key): # guard function for RestrictedPython From 016bda51aad5dddb705f9ba1931eacd4b156188f Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 23 Dec 2021 21:29:12 +0530 Subject: [PATCH 02/12] test: import doctype fixtures --- frappe/custom/fixtures/temp_doctype.json | 168 +++++++++++++++++++++++ frappe/tests/test_fixture_import.py | 55 ++++++++ 2 files changed, 223 insertions(+) create mode 100644 frappe/custom/fixtures/temp_doctype.json create mode 100644 frappe/tests/test_fixture_import.py diff --git a/frappe/custom/fixtures/temp_doctype.json b/frappe/custom/fixtures/temp_doctype.json new file mode 100644 index 0000000000..343aa2cb37 --- /dev/null +++ b/frappe/custom/fixtures/temp_doctype.json @@ -0,0 +1,168 @@ +{ + "docstatus": 0, + "doctype": "DocType", + "name": "new-doctype-2", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "is_submittable": 0, + "istable": 0, + "issingle": 0, + "is_tree": 0, + "editable_grid": 1, + "quick_entry": 1, + "track_changes": 1, + "track_seen": 0, + "track_views": 0, + "custom": 1, + "beta": 0, + "is_virtual": 0, + "naming_rule": "", + "name_case": "", + "allow_rename": 1, + "hide_toolbar": 0, + "allow_copy": 0, + "allow_import": 0, + "allow_events_in_timeline": 0, + "allow_auto_repeat": 0, + "sort_field": "modified", + "sort_order": "DESC", + "document_type": "", + "show_preview_popup": 0, + "show_name_in_global_search": 0, + "email_append_to": 0, + "read_only": 0, + "in_create": 0, + "has_web_view": 0, + "allow_guest_to_view": 0, + "index_web_pages_for_search": 1, + "engine": "InnoDB", + "permissions": [ + { + "docstatus": 0, + "doctype": "DocPerm", + "name": "new-docperm-2", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "if_owner": 0, + "permlevel": 0, + "select": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + "submit": 0, + "cancel": 0, + "amend": 0, + "report": 1, + "export": 1, + "import": 0, + "set_user_permissions": 0, + "share": 1, + "print": 1, + "email": 1, + "parent": "new-doctype-2", + "parentfield": "permissions", + "parenttype": "DocType", + "idx": 1, + "role": "System Manager" + } + ], + "__newname": "temp_doctype", + "module": "Custom", + "fields": [ + { + "docstatus": 0, + "doctype": "DocField", + "name": "new-docfield-1", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "fieldtype": "Data", + "precision": "", + "non_negative": 0, + "hide_days": 0, + "hide_seconds": 0, + "reqd": 1, + "search_index": 0, + "fetch_if_empty": 0, + "hidden": 0, + "bold": 0, + "allow_in_quick_entry": 0, + "translatable": 0, + "print_hide": 0, + "print_hide_if_no_value": 0, + "report_hide": 0, + "collapsible": 0, + "hide_border": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "in_preview": 0, + "in_filter": 0, + "in_global_search": 0, + "read_only": 0, + "allow_on_submit": 0, + "ignore_user_permissions": 0, + "allow_bulk_edit": 0, + "permlevel": 0, + "ignore_xss_filter": 0, + "unique": 0, + "no_copy": 0, + "set_only_once": 0, + "remember_last_selected_value": 0, + "parent": "new-doctype-2", + "parentfield": "fields", + "parenttype": "DocType", + "idx": 1, + "__unedited": false, + "label": "member_name" + }, + { + "docstatus": 0, + "doctype": "DocField", + "name": "new-docfield-2", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "fieldtype": "Data", + "precision": "", + "non_negative": 0, + "hide_days": 0, + "hide_seconds": 0, + "reqd": 0, + "search_index": 0, + "fetch_if_empty": 0, + "hidden": 0, + "bold": 0, + "allow_in_quick_entry": 0, + "translatable": 0, + "print_hide": 0, + "print_hide_if_no_value": 0, + "report_hide": 0, + "collapsible": 0, + "hide_border": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "in_preview": 0, + "in_filter": 0, + "in_global_search": 0, + "read_only": 0, + "allow_on_submit": 0, + "ignore_user_permissions": 0, + "allow_bulk_edit": 0, + "permlevel": 0, + "ignore_xss_filter": 0, + "unique": 0, + "no_copy": 0, + "set_only_once": 0, + "remember_last_selected_value": 0, + "parent": "new-doctype-2", + "parentfield": "fields", + "parenttype": "DocType", + "idx": 2, + "__unedited": false, + "label": "email" + } + ] +} diff --git a/frappe/tests/test_fixture_import.py b/frappe/tests/test_fixture_import.py new file mode 100644 index 0000000000..49a357e18c --- /dev/null +++ b/frappe/tests/test_fixture_import.py @@ -0,0 +1,55 @@ +import os +import unittest +from typing import List + +import frappe +from frappe.core.doctype.data_import.data_import import export_json, import_doc +from frappe.desk.form.save import savedocs +from frappe.model.delete_doc import delete_doc + + +class TestFixtureImport(unittest.TestCase): + def create_new_doctype(self, DocType: str) -> None: + file = frappe.get_app_path("frappe", "custom", "fixtures", f"{DocType}.json") + + file = open(file, "r") + doc = file.read() + file.close() + + savedocs(doc, "Save") + + def insert_dummy_data_and_export(self, DocType: str, dummy_name_list: List[str]) -> str: + for name in dummy_name_list: + doc = frappe.get_doc({"doctype": DocType, "member_name": name}) + doc.insert() + + path_to_exported_fixtures = os.path.join(os.getcwd(), f"{DocType}_data.json") + + export_json(DocType, path_to_exported_fixtures) + + return path_to_exported_fixtures + + def test_fixtures_import(self): + self.assertFalse(frappe.db.exists("DocType", "temp_doctype")) + + self.create_new_doctype("temp_doctype") + + dummy_name_list = ["jhon", "jane"] + path_to_exported_fixtures = self.insert_dummy_data_and_export("temp_doctype", dummy_name_list) + frappe.db.truncate("temp_doctype") + + import_doc(path_to_exported_fixtures) + + delete_doc("DocType", "temp_doctype", delete_permanently=True) + os.remove(path_to_exported_fixtures) + + self.assertEqual(frappe.db.count("temp_doctype"), len(dummy_name_list)) + + data = frappe.get_all("temp_doctype", "member_name") + frappe.db.truncate("temp_doctype") + + imported_data = set() + for item in data: + imported_data.add(item["member_name"]) + + self.assertEqual(set(dummy_name_list), imported_data) From ed9dad8f302e622e0a039bba186670d17fd1f502 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 23 Dec 2021 22:13:51 +0530 Subject: [PATCH 03/12] test: import single doctype --- frappe/custom/fixtures/temp_singles.json | 168 +++++++++++++++++++++++ frappe/tests/test_fixture_import.py | 27 ++++ 2 files changed, 195 insertions(+) create mode 100644 frappe/custom/fixtures/temp_singles.json diff --git a/frappe/custom/fixtures/temp_singles.json b/frappe/custom/fixtures/temp_singles.json new file mode 100644 index 0000000000..b7e2536f25 --- /dev/null +++ b/frappe/custom/fixtures/temp_singles.json @@ -0,0 +1,168 @@ +{ + "docstatus": 0, + "doctype": "DocType", + "name": "new-doctype-1", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "is_submittable": 0, + "istable": 0, + "issingle": 1, + "is_tree": 0, + "editable_grid": 1, + "quick_entry": 0, + "track_changes": 1, + "track_seen": 0, + "track_views": 0, + "custom": 1, + "beta": 0, + "is_virtual": 0, + "naming_rule": "", + "name_case": "", + "allow_rename": 1, + "hide_toolbar": 0, + "allow_copy": 0, + "allow_import": 0, + "allow_events_in_timeline": 0, + "allow_auto_repeat": 0, + "sort_field": "modified", + "sort_order": "DESC", + "document_type": "", + "show_preview_popup": 0, + "show_name_in_global_search": 0, + "email_append_to": 0, + "read_only": 0, + "in_create": 0, + "has_web_view": 0, + "allow_guest_to_view": 0, + "index_web_pages_for_search": 1, + "engine": "InnoDB", + "permissions": [ + { + "docstatus": 0, + "doctype": "DocPerm", + "name": "new-docperm-1", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "if_owner": 0, + "permlevel": 0, + "select": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + "submit": 0, + "cancel": 0, + "amend": 0, + "report": 1, + "export": 1, + "import": 0, + "set_user_permissions": 0, + "share": 1, + "print": 1, + "email": 1, + "parent": "new-doctype-1", + "parentfield": "permissions", + "parenttype": "DocType", + "idx": 1, + "role": "System Manager" + } + ], + "__newname": "temp_singles", + "module": "Custom", + "fields": [ + { + "docstatus": 0, + "doctype": "DocField", + "name": "new-docfield-1", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "fieldtype": "Data", + "precision": "", + "non_negative": 0, + "hide_days": 0, + "hide_seconds": 0, + "reqd": 0, + "search_index": 0, + "fetch_if_empty": 0, + "hidden": 0, + "bold": 0, + "allow_in_quick_entry": 0, + "translatable": 0, + "print_hide": 0, + "print_hide_if_no_value": 0, + "report_hide": 0, + "collapsible": 0, + "hide_border": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "in_preview": 0, + "in_filter": 0, + "in_global_search": 0, + "read_only": 0, + "allow_on_submit": 0, + "ignore_user_permissions": 0, + "allow_bulk_edit": 0, + "permlevel": 0, + "ignore_xss_filter": 0, + "unique": 0, + "no_copy": 0, + "set_only_once": 0, + "remember_last_selected_value": 0, + "parent": "new-doctype-1", + "parentfield": "fields", + "parenttype": "DocType", + "idx": 1, + "__unedited": false, + "label": "member_name" + }, + { + "docstatus": 0, + "doctype": "DocField", + "name": "new-docfield-2", + "__islocal": 1, + "__unsaved": 1, + "owner": "Administrator", + "fieldtype": "Data", + "precision": "", + "non_negative": 0, + "hide_days": 0, + "hide_seconds": 0, + "reqd": 0, + "search_index": 0, + "fetch_if_empty": 0, + "hidden": 0, + "bold": 0, + "allow_in_quick_entry": 0, + "translatable": 0, + "print_hide": 0, + "print_hide_if_no_value": 0, + "report_hide": 0, + "collapsible": 0, + "hide_border": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "in_preview": 0, + "in_filter": 0, + "in_global_search": 0, + "read_only": 0, + "allow_on_submit": 0, + "ignore_user_permissions": 0, + "allow_bulk_edit": 0, + "permlevel": 0, + "ignore_xss_filter": 0, + "unique": 0, + "no_copy": 0, + "set_only_once": 0, + "remember_last_selected_value": 0, + "parent": "new-doctype-1", + "parentfield": "fields", + "parenttype": "DocType", + "idx": 2, + "__unedited": false, + "label": "email" + } + ] +} diff --git a/frappe/tests/test_fixture_import.py b/frappe/tests/test_fixture_import.py index 49a357e18c..c9467b5de2 100644 --- a/frappe/tests/test_fixture_import.py +++ b/frappe/tests/test_fixture_import.py @@ -53,3 +53,30 @@ class TestFixtureImport(unittest.TestCase): imported_data.add(item["member_name"]) self.assertEqual(set(dummy_name_list), imported_data) + + def test_singles_fixtures_import(self): + self.assertFalse(frappe.db.exists("DocType", "temp_singles")) + + self.create_new_doctype("temp_singles") + + dummy_name_list = ["Phoebe"] + path_to_exported_fixtures = self.insert_dummy_data_and_export("temp_singles", dummy_name_list) + + singles_doctype = frappe.qb.DocType("Singles") + truncate_query = ( + frappe.qb.from_(singles_doctype) + .delete() + .where( + singles_doctype.doctype == "temp_singles") + ) + truncate_query.run() + + import_doc(path_to_exported_fixtures) + + delete_doc("DocType", "temp_singles", delete_permanently=True) + os.remove(path_to_exported_fixtures) + + data = frappe.db.get_single_value("temp_singles", "member_name") + truncate_query.run() + + self.assertEqual(data, dummy_name_list[0]) From 33b7d7d74f0c97dc43e297ae260d7fd30cfc7959 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 24 Dec 2021 13:28:40 +0530 Subject: [PATCH 04/12] fix: Handle custom child tables via check_parent_permission Check `tabCustom Field` and `tabDocfield` for matching parent existence. --- frappe/model/db_query.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 16c0d18d9f..6d261dd4fd 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -787,12 +787,15 @@ class DatabaseQuery(object): def check_parent_permission(parent, child_doctype): if parent: # User may pass fake parent and get the information from the child table - if child_doctype and not frappe.db.exists('DocField', - {'parent': parent, 'options': child_doctype}): + if child_doctype and not ( + frappe.db.exists('DocField', {'parent': parent, 'options': child_doctype}) + or frappe.db.exists('Custom Field', {'dt': parent, 'options': child_doctype}) + ): raise frappe.PermissionError if frappe.permissions.has_permission(parent): return + # Either parent not passed or the user doesn't have permission on parent doctype of child table! raise frappe.PermissionError From 3b7b555148d8ffa9a4f10ffcb52855a28b3879ae Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 24 Dec 2021 13:57:07 +0530 Subject: [PATCH 05/12] fix: Pass parent_doctype required for DatabaseQuery perm checks has_child_table_permission would throw 'Parent DocType Required: Please specify a valid parent DocType for {TEST DOCTYPE}' after the previous commit 33b7d7d74f0c97dc43e297ae260d7fd30cfc7959. Passing parent_doctype solves this --- frappe/client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/client.py b/frappe/client.py index 6641e471af..e835e7fee7 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -32,6 +32,7 @@ def get_list(doctype, fields=None, filters=None, order_by=None, args = frappe._dict( doctype=doctype, + parent_doctype=parent, fields=fields, filters=filters, or_filters=or_filters, From 84ebdabe49fc9447acead8300808b4af3387974a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 24 Dec 2021 13:59:57 +0530 Subject: [PATCH 06/12] refactor(minor): frappe.has_permission The throw block was very clearly broken. Referencing frappe.throw inside __init__.py rip. Added drop in replacement msgprint call --- frappe/__init__.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 96aa2b1b5f..08c0f794b3 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -740,17 +740,26 @@ def has_permission(doctype=None, ptype="read", doc=None, user=None, verbose=Fals :param doc: [optional] Checks User permissions for given doc. :param user: [optional] Check for given user. Default: current user. :param parent_doctype: Required when checking permission for a child DocType (unless doc is specified).""" + import frappe.permissions + if not doctype and doc: doctype = doc.doctype - import frappe.permissions out = frappe.permissions.has_permission(doctype, ptype, doc=doc, verbose=verbose, user=user, raise_exception=throw, parent_doctype=parent_doctype) + if throw and not out: - if doc: - frappe.throw(_("No permission for {0}").format(doc.doctype + " " + doc.name)) - else: - frappe.throw(_("No permission for {0}").format(doctype)) + # mimics frappe.throw + document_label = f"{doc.doctype} {doc.name}" if doc else doctype + msgprint( + _("No permission for {0}").format(document_label), + raise_exception=ValidationError, + title=None, + indicator='red', + is_minimizable=None, + wide=None, + as_list=False + ) return out From 0f71dd411bbbb3a3eaa30150f4d0a7acc4971e55 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 24 Dec 2021 14:01:26 +0530 Subject: [PATCH 07/12] style: DatabaseQuery.execute's permission cond block --- frappe/model/db_query.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 6d261dd4fd..cb2c2af898 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -36,10 +36,12 @@ class DatabaseQuery(object): ignore_ifnull=False, save_user_settings=False, save_user_settings_fields=False, update=None, add_total_row=None, user_settings=None, reference_doctype=None, run=True, strict=True, pluck=None, ignore_ddl=False, parent_doctype=None) -> List: - if not ignore_permissions and \ - not frappe.has_permission(self.doctype, "select", user=user, parent_doctype=parent_doctype) and \ - not frappe.has_permission(self.doctype, "read", user=user, parent_doctype=parent_doctype): + if ( + not ignore_permissions + and not frappe.has_permission(self.doctype, "select", user=user, parent_doctype=parent_doctype) + and not frappe.has_permission(self.doctype, "read", user=user, parent_doctype=parent_doctype) + ): frappe.flags.error_message = _('Insufficient Permission for {0}').format(frappe.bold(self.doctype)) raise frappe.PermissionError(self.doctype) From f19c67314dbd412ff3c476db2078e6a37c093ba5 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Fri, 24 Dec 2021 19:19:22 +0530 Subject: [PATCH 08/12] style: Fixed indentation --- frappe/tests/test_fixture_import.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frappe/tests/test_fixture_import.py b/frappe/tests/test_fixture_import.py index c9467b5de2..2fe7e40d0d 100644 --- a/frappe/tests/test_fixture_import.py +++ b/frappe/tests/test_fixture_import.py @@ -64,11 +64,10 @@ class TestFixtureImport(unittest.TestCase): singles_doctype = frappe.qb.DocType("Singles") truncate_query = ( - frappe.qb.from_(singles_doctype) - .delete() - .where( - singles_doctype.doctype == "temp_singles") - ) + frappe.qb.from_(singles_doctype) + .delete() + .where(singles_doctype.doctype == "temp_singles") + ) truncate_query.run() import_doc(path_to_exported_fixtures) From 16435de3d6f2c87133790cd25f3ec6cbc8c8262c Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sat, 25 Dec 2021 21:38:58 +0530 Subject: [PATCH 09/12] fix: Null state style --- frappe/public/scss/desk/page.scss | 35 +++++++++++++++++-------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/frappe/public/scss/desk/page.scss b/frappe/public/scss/desk/page.scss index 339dd755d0..91bd942889 100644 --- a/frappe/public/scss/desk/page.scss +++ b/frappe/public/scss/desk/page.scss @@ -156,26 +156,29 @@ .result, .no-result, .freeze { min-height: #{"calc(100vh - 284px)"}; } + } - .msg-box { - margin-bottom: 4em; - font-size: var(--text-sm); + .msg-box { + margin-bottom: 4em; + font-size: var(--text-sm); - // To compensate for perceived centering - .null-state { - height: 85px; - width: auto; - margin-bottom: var(--margin-md); - img { - fill: var(--fg-color); - } + // To compensate for perceived centering + .null-state { + height: 85px; + width: auto; + margin-bottom: var(--margin-md); + img { + fill: var(--fg-color); } + } + p { + font-size: var(--text-md); + } - .meta-description { - width: 45%; - margin-right: auto; - margin-left: auto; - } + .meta-description { + width: 45%; + margin-right: auto; + margin-left: auto; } } } From 8dce649933212445a8210ed9d53a64b972ad4b9e Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sat, 25 Dec 2021 22:50:34 +0530 Subject: [PATCH 10/12] fix: Use last_value as a fallback if model value isn't set - Did this to avoid repeated value set of datetime value which causes browser to freeze. This happens only with the datetime control is used in filters (with a default value) and is not linked with a form. --- frappe/public/js/frappe/form/controls/datetime.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/public/js/frappe/form/controls/datetime.js b/frappe/public/js/frappe/form/controls/datetime.js index 5d0ecb9fe7..d1a06a6ac6 100644 --- a/frappe/public/js/frappe/form/controls/datetime.js +++ b/frappe/public/js/frappe/form/controls/datetime.js @@ -81,6 +81,9 @@ frappe.ui.form.ControlDatetime = class ControlDatetime extends frappe.ui.form.Co get_model_value() { let value = super.get_model_value(); + if (!value && !this.doc) { + value = this.last_value; + } return frappe.datetime.get_datetime_as_string(value); } }; From 829475a98d21a5eb3ba4fee8af387ff3453d1ff8 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Sat, 25 Dec 2021 22:53:17 +0100 Subject: [PATCH 11/12] fix: don't run webhook in migrate --- frappe/integrations/doctype/webhook/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/integrations/doctype/webhook/__init__.py b/frappe/integrations/doctype/webhook/__init__.py index 6dcc0218a3..cb7a9963b7 100644 --- a/frappe/integrations/doctype/webhook/__init__.py +++ b/frappe/integrations/doctype/webhook/__init__.py @@ -7,7 +7,7 @@ import frappe def run_webhooks(doc, method): '''Run webhooks for this method''' - if frappe.flags.in_import or frappe.flags.in_patch or frappe.flags.in_install: + if frappe.flags.in_import or frappe.flags.in_patch or frappe.flags.in_install or frappe.flags.in_migrate: return if frappe.flags.webhooks_executed is None: From f0e15338fee4112d361220c1ddb5218c48a59767 Mon Sep 17 00:00:00 2001 From: Wolfram Schmidt Date: Mon, 27 Dec 2021 06:53:30 +0100 Subject: [PATCH 12/12] fix: Grammar of error statement in 404 page (#15447) Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/www/404.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/www/404.html b/frappe/www/404.html index c03b5d3e96..2596741c7a 100644 --- a/frappe/www/404.html +++ b/frappe/www/404.html @@ -15,10 +15,10 @@ {{ _("There's nothing here") }}
- {{ _("The page you are looking for have gone missing.") }} + {{ _("The page you are looking for has gone missing.") }}
{{ _("Back to Home") }}
-{% endblock %} \ No newline at end of file +{% endblock %}