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 diff --git a/frappe/__init__.py b/frappe/__init__.py index 09248fa99f..86e8e6063f 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -435,9 +435,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: 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: 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}" 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 *", 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/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 diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index c7295730de..11a9ae936c 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 8a5becf3cc..2bb1a48819 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/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): 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); } } 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 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):