From f89dc40b94e30e3a00b1c54434df2f3a05d4c610 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Mon, 27 Jun 2022 14:46:06 +0530 Subject: [PATCH 01/10] fix: Allow permitted number card of type report (#17316) --- frappe/desk/doctype/number_card/number_card.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 2290e49656..74c8e9eb99 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -4,6 +4,7 @@ import frappe from frappe import _ +from frappe.boot import get_allowed_reports from frappe.config import get_modules_from_all_apps_for_user from frappe.model.document import Document from frappe.model.naming import append_number_if_name_exists @@ -90,9 +91,16 @@ def has_permission(doc, ptype, user): if "System Manager" in roles: return True - allowed_doctypes = tuple(frappe.permissions.get_doctypes_with_read()) - if doc.document_type in allowed_doctypes: - return True + if doc.type == "Report": + allowed_reports = [ + key if type(key) == str else key.encode("UTF8") for key in get_allowed_reports() + ] + if doc.report_name in allowed_reports: + return True + else: + allowed_doctypes = tuple(frappe.permissions.get_doctypes_with_read()) + if doc.document_type in allowed_doctypes: + return True return False From 084a1e6c31520de09d639dbb5db129fb7e757d4b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 27 Jun 2022 15:18:06 +0530 Subject: [PATCH 02/10] refactor: get_permissions * Show page even if dangling Custom DocPerm records encountered * Add typing hints * Cleanup APIs --- .../permission_manager/permission_manager.py | 25 +++++++++++++------ frappe/permissions.py | 23 ++++++++--------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index ad12e0fd4c..e2d08488c0 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -1,6 +1,8 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +from typing import Optional + import frappe import frappe.defaults from frappe import _ @@ -8,6 +10,7 @@ from frappe.core.doctype.doctype.doctype import ( clear_permissions_cache, validate_permissions_for_doctype, ) +from frappe.exceptions import DoesNotExistError from frappe.modules.import_file import get_file_path, read_doc_from_file from frappe.permissions import ( add_permission, @@ -68,17 +71,19 @@ def get_roles_and_doctypes(): @frappe.whitelist() -def get_permissions(doctype=None, role=None): +def get_permissions(doctype: Optional[str] = None, role: Optional[str] = None): frappe.only_for("System Manager") + if role: out = get_all_perms(role) if doctype: out = [p for p in out if p.parent == doctype] + else: - filters = dict(parent=doctype) + filters = {"parent": doctype} if frappe.session.user != "Administrator": - custom_roles = frappe.get_all("Role", filters={"is_custom": 1}) - filters["role"] = ["not in", [row.name for row in custom_roles]] + custom_roles = frappe.get_all("Role", filters={"is_custom": 1}, pluck="name") + filters["role"] = ["not in", custom_roles] out = frappe.get_all("Custom DocPerm", fields="*", filters=filters, order_by="permlevel") if not out: @@ -86,11 +91,15 @@ def get_permissions(doctype=None, role=None): linked_doctypes = {} for d in out: - if not d.parent in linked_doctypes: - linked_doctypes[d.parent] = get_linked_doctypes(d.parent) + if d.parent not in linked_doctypes: + try: + linked_doctypes[d.parent] = get_linked_doctypes(d.parent) + except DoesNotExistError: + # exclude & continue if linked doctype is not found + frappe.clear_last_message() + continue d.linked_doctypes = linked_doctypes[d.parent] - meta = frappe.get_meta(d.parent) - if meta: + if meta := frappe.get_meta(d.parent): d.is_submittable = meta.is_submittable d.in_create = meta.in_create diff --git a/frappe/permissions.py b/frappe/permissions.py index 8980d2e63e..55a7c0e5f3 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import copy +from typing import List import frappe import frappe.share @@ -605,19 +606,17 @@ def reset_perms(doctype): frappe.db.delete("Custom DocPerm", {"parent": doctype}) -def get_linked_doctypes(dt): - return list( - set( - [dt] - + [ - d.options - for d in frappe.get_meta(dt).get( - "fields", - {"fieldtype": "Link", "ignore_user_permissions": ("!=", 1), "options": ("!=", "[Select]")}, - ) - ] +def get_linked_doctypes(dt: str) -> List: + meta = frappe.get_meta(dt) + linked_doctypes = [dt] + [ + d.options + for d in meta.get( + "fields", + {"fieldtype": "Link", "ignore_user_permissions": ("!=", 1), "options": ("!=", "[Select]")}, ) - ) + ] + + return list(set(linked_doctypes)) def get_doc_name(doc): From fba75c35951982631322a17b3af1778ed2d129aa Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 27 Jun 2022 15:55:54 +0530 Subject: [PATCH 03/10] fix: use console.error for logging errors (#17318) Currently if uncaught exception occurs on server side, there's no simple way to find it out. Converting these console.log to console.error simplifies this for UI tests. Also converted `console.trace to` `console.error`; Both are practically same and give stack trace too, `trace` expands tracebacks by default. This is done to make spying on window.console.error easier. --- frappe/public/js/frappe/request.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/public/js/frappe/request.js b/frappe/public/js/frappe/request.js index 87ab06ce18..dd4c352f41 100644 --- a/frappe/public/js/frappe/request.js +++ b/frappe/public/js/frappe/request.js @@ -280,7 +280,7 @@ frappe.request.call = function(opts) { } } catch(e) { console.log("Unable to handle success response", data); // eslint-disable-line - console.trace(e); // eslint-disable-line + console.error(e); // eslint-disable-line } }) @@ -332,7 +332,7 @@ frappe.request.call = function(opts) { opts.error_callback && opts.error_callback(xhr); } catch(e) { console.log("Unable to handle failed response"); // eslint-disable-line - console.trace(e); // eslint-disable-line + console.error(e); // eslint-disable-line } }); } @@ -433,13 +433,13 @@ frappe.request.cleanup = function(opts, r) { if(r.exc) { r.exc = JSON.parse(r.exc); if(r.exc instanceof Array) { - $.each(r.exc, function(i, v) { - if(v) { - console.log(v); + r.exc.forEach(exc => { + if(exc) { + console.error(exc); } - }) + }); } else { - console.log(r.exc); + console.error(r.exc); } } From 112f11359847cd286752177b42a357d4b6910cc3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 27 Jun 2022 17:04:34 +0530 Subject: [PATCH 04/10] fix!: remove dangerous "rollback_on_exception" flag (#17321) --- frappe/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index ee199c8aed..9104cd5109 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -432,9 +432,6 @@ def msgprint( def _raise_exception(): if raise_exception: - if flags.rollback_on_exception: - db.rollback() - if inspect.isclass(raise_exception) and issubclass(raise_exception, Exception): raise raise_exception(msg) else: From b7514a05ca9039bf9336ece3938b69bdeecb6585 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 28 Jun 2022 00:37:31 +0530 Subject: [PATCH 05/10] fix(redis): pass shared param when setting value based on generator --- frappe/utils/redis_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 178c3d4416..c29836af32 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -199,7 +199,7 @@ class RedisWrapper(redis.Redis): frappe.local.cache[_name][key] = value elif generator: value = generator() - self.hset(name, key, value) + self.hset(name, key, value, shared=shared) return value def hdel(self, name, key, shared=False): From c6fa8ab090fdc0d07dbd295b5a262ed2a5acf456 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Tue, 28 Jun 2022 11:50:57 +0530 Subject: [PATCH 06/10] fix: email not sent if contain file with current site url (#17250) --- frappe/core/doctype/file/file.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index e8b8da76ab..bb4b441680 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -16,7 +16,7 @@ from requests.exceptions import HTTPError, SSLError import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import call_hook_method, cint, get_files_path, get_hook_method +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 @@ -61,7 +61,12 @@ class File(Document): self.set_file_name() self.validate_attachment_limit() - if not self.is_folder and not self.is_remote_file: + if self.is_folder: + return + + if self.is_remote_file: + self.validate_remote_file() + else: self.save_file(content=self.get_content()) self.flags.new_file = True frappe.local.rollback_observers.append(self) @@ -255,6 +260,12 @@ class File(Document): title=_("Attachment Limit Reached"), ) + def validate_remote_file(self): + """Validates if file uploaded using URL already exist""" + site_url = get_url() + if "/files/" in self.file_url and self.file_url.startswith(site_url): + self.file_url = self.file_url.split(site_url, 1)[1] + def set_folder_name(self): """Make parent folders if not exists based on reference doctype and name""" if self.folder: @@ -445,6 +456,10 @@ class File(Document): file_path = self.file_url or self.file_name + site_url = get_url() + if "/files/" in file_path and file_path.startswith(site_url): + file_path = file_path.split(site_url, 1)[1] + if "/" not in file_path: if self.is_private: file_path = f"/private/files/{file_path}" From 9bd753551b6af9cb25a445526969f563bb738e5a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 28 Jun 2022 12:07:34 +0530 Subject: [PATCH 07/10] ci: temp fix for semgrep (#17228) * ci: respekt my authoritah * ci: use pip semgrep --- .github/helper/roulette.py | 8 ++++---- .github/workflows/linters.yml | 16 +++++++--------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/.github/helper/roulette.py b/.github/helper/roulette.py index 9165198012..b859b87047 100644 --- a/.github/helper/roulette.py +++ b/.github/helper/roulette.py @@ -77,13 +77,13 @@ if __name__ == "__main__": updated_py_file_count = len(list(filter(is_py, files_list))) only_py_changed = updated_py_file_count == len(files_list) - if ci_files_changed: - print("CI related files were updated, running all build processes.") - - elif has_skip_ci_label(pr_number, repo): + if has_skip_ci_label(pr_number, repo): print("Found `Skip CI` label on pr, stopping build process.") sys.exit(0) + elif ci_files_changed: + print("CI related files were updated, running all build processes.") + elif only_docs_changed: print("Only docs were updated, stopping build process.") sys.exit(0) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index 4c845dba8c..6d1029d51d 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -11,10 +11,10 @@ jobs: steps: - uses: actions/checkout@v3 - - name: Set up Python 3.8 + - name: Set up Python uses: actions/setup-python@v4 with: - python-version: 3.8 + python-version: '3.10' - name: Install and Run Pre-commit uses: pre-commit/action@v3.0.0 @@ -22,10 +22,8 @@ jobs: - name: Download Semgrep rules run: git clone --depth 1 https://github.com/frappe/semgrep-rules.git frappe-semgrep-rules - - uses: returntocorp/semgrep-action@v1 - env: - SEMGREP_TIMEOUT: 120 - with: - config: >- - r/python.lang.correctness - ./frappe-semgrep-rules/rules + - name: Download semgrep + run: pip install semgrep==0.97.0 + + - name: Run Semgrep rules + run: semgrep ci --config ./frappe-semgrep-rules/rules --config r/python.lang.correctness From 4bf7cd8210d92e89e9ad818fc9c72e317688d8d7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 28 Jun 2022 12:19:52 +0530 Subject: [PATCH 08/10] fix: ignore virtual docfield property conflicts --- frappe/core/doctype/doctype/doctype.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index b4cbbd6233..e56803acb7 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -171,7 +171,7 @@ class DocType(Document): if docfield.fieldname in method_set: conflict_type = "controller method" - if docfield.fieldname in property_set: + if docfield.fieldname in property_set and not docfield.is_virtual: conflict_type = "class property" if conflict_type: From 600186488866709b2099630fc3a402b6d0e5bb30 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 28 Jun 2022 11:38:40 +0530 Subject: [PATCH 09/10] fix(UX): show next execution time on scheduled job --- .../scheduled_job_type.json | 25 +++++++++++++++++-- .../scheduled_job_type/scheduled_job_type.py | 4 +++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/scheduled_job_type/scheduled_job_type.json b/frappe/core/doctype/scheduled_job_type/scheduled_job_type.json index d4d79b21fb..cc2a0e870a 100644 --- a/frappe/core/doctype/scheduled_job_type/scheduled_job_type.json +++ b/frappe/core/doctype/scheduled_job_type/scheduled_job_type.json @@ -16,8 +16,11 @@ "server_script", "frequency", "cron_format", + "create_log", + "status_section", "last_execution", - "create_log" + "column_break_9", + "next_execution" ], "fields": [ { @@ -72,6 +75,22 @@ "options": "Server Script", "read_only": 1, "search_index": 1 + }, + { + "fieldname": "next_execution", + "fieldtype": "Datetime", + "is_virtual": 1, + "label": "Next Execution", + "read_only": 1 + }, + { + "fieldname": "status_section", + "fieldtype": "Section Break", + "label": "Status" + }, + { + "fieldname": "column_break_9", + "fieldtype": "Column Break" } ], "in_create": 1, @@ -81,7 +100,7 @@ "link_fieldname": "scheduled_job_type" } ], - "modified": "2020-10-07 10:39:24.519460", + "modified": "2022-06-28 02:55:12.470915", "modified_by": "Administrator", "module": "Core", "name": "Scheduled Job Type", @@ -103,5 +122,7 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], + "title_field": "method", "track_changes": 1 } \ No newline at end of file diff --git a/frappe/core/doctype/scheduled_job_type/scheduled_job_type.py b/frappe/core/doctype/scheduled_job_type/scheduled_job_type.py index 318b156dcd..673805ae8b 100644 --- a/frappe/core/doctype/scheduled_job_type/scheduled_job_type.py +++ b/frappe/core/doctype/scheduled_job_type/scheduled_job_type.py @@ -50,6 +50,10 @@ class ScheduledJobType(Document): queued_jobs = get_jobs(site=frappe.local.site, key="job_type")[frappe.local.site] return self.method in queued_jobs + @property + def next_execution(self): + return self.get_next_execution() + def get_next_execution(self): CRON_MAP = { "Yearly": "0 0 1 1 *", From 971b8160a33fab9ce939e9c751fa45da7499e387 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Tue, 28 Jun 2022 12:39:45 +0530 Subject: [PATCH 10/10] fix: extra column in excel after exporting report with group by (#17126) Co-authored-by: gavin --- frappe/desk/reportview.py | 22 ++++++++++++++++------ frappe/model/db_query.py | 5 +++++ frappe/tests/test_db_query.py | 4 +--- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index d6dce68399..893c2a6606 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -156,8 +156,6 @@ def setup_group_by(data): **data ) ) - if data.aggregate_on_field: - data.fields.append(f"`tab{data.aggregate_on_doctype}`.`{data.aggregate_on_field}`") else: raise_invalid_field(data.aggregate_on_field) @@ -435,11 +433,20 @@ def append_totals_row(data): def get_labels(fields, doctype): """get column labels based on column names""" labels = [] + doctype = doctype.lower() for key in fields: - key = key.split(" as ")[0] + aggregate_function = "" + + key = key.casefold().split(" as ", maxsplit=1)[0] if key.startswith(("count(", "sum(", "avg(")): - continue + # Get aggregate function and _aggregate_column + # key = 'sum(`tabDocType`.`fieldname`)' + if not key.rstrip().endswith(")"): + continue + _agg_fn, _key = key.split("(", maxsplit=1) + aggregate_function = _agg_fn.lower() # aggregate_function = 'sum' + key = _key[:-1] # key = `tabDocType`.`fieldname` if "." in key: parenttype, fieldname = key.split(".")[0][4:-1], key.split(".")[1].strip("`") @@ -455,7 +462,10 @@ def get_labels(fields, doctype): if parenttype != doctype: # If the column is from a child table, append the child doctype. # For example, "Item Code (Sales Invoice Item)". - label += f" ({ _(parenttype) })" + label += f" ({ _(parenttype.title()) })" + + if aggregate_function: + label = _("{0} of {1}").format(aggregate_function.capitalize(), label) labels.append(label) @@ -464,7 +474,7 @@ def get_labels(fields, doctype): def handle_duration_fieldtype_values(doctype, data, fields): for field in fields: - key = field.split(" as ")[0] + key = field.casefold().split(" as ", maxsplit=1)[0] if key.startswith(("count(", "sum(", "avg(")): continue diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 7fb38848e2..0b8f790246 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -417,6 +417,8 @@ class DatabaseQuery(object): "extract(", "locate(", "strpos(", + ] + aggregate_functions = [ "count(", "sum(", "avg(", @@ -427,6 +429,9 @@ class DatabaseQuery(object): if not ("tab" in field and "." in field) or any(x for x in sql_functions if x in field): continue + if any(x for x in aggregate_functions if x in field): + field = field.split("(", 1)[1][:-1] + table_name = field.split(".")[0] if table_name.lower().startswith("group_concat("): diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index c1b2e05266..2dd1743767 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -643,9 +643,7 @@ class TestReportview(unittest.TestCase): ) response = execute_cmd("frappe.desk.reportview.get") - self.assertListEqual( - response["keys"], ["field_label", "field_name", "_aggregate_column", "columns"] - ) + self.assertListEqual(response["keys"], ["field_label", "field_name", "_aggregate_column"]) def test_cast_name(self): from frappe.core.doctype.doctype.test_doctype import new_doctype