diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index d9603e89aa..33a22fba6a 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -131,3 +131,16 @@ rules: key `$X` is uselessly assigned twice. This could be a potential bug. languages: [python] severity: ERROR + +- id: frappe-using-db-sql + pattern-either: + - pattern: frappe.db.sql(...) + - pattern: frappe.db.sql_ddl(...) + - pattern: frappe.db.sql_list(...) + paths: + exclude: + - "test_*.py" + message: | + The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database) + languages: [python] + severity: ERROR diff --git a/codecov.yml b/codecov.yml index eeba1ff381..a9f6df0296 100644 --- a/codecov.yml +++ b/codecov.yml @@ -3,6 +3,7 @@ codecov: coverage: status: + patch: off project: default: false server: @@ -10,11 +11,6 @@ coverage: threshold: 0.5% flags: - server - ui-tests: - target: auto - threshold: 0.5% - flags: - - ui-tests comment: layout: "diff, flags" @@ -28,4 +24,4 @@ flags: ui-tests: paths: - ".*\\.js" - carryforward: true \ No newline at end of file + carryforward: true diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index bf4436358e..af2ffd3fc5 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -44,6 +44,11 @@ let argv = yargs type: "boolean", description: "Run in watch mode and rebuild on file changes" }) + .option("live-reload", { + type: "boolean", + description: `Automatically reload web pages when assets are rebuilt. + Can only be used with the --watch flag.` + }) .option("production", { type: "boolean", description: "Run build in production mode" @@ -478,7 +483,8 @@ async function notify_redis({ error, success }) { } if (success) { payload = { - success: true + success: true, + live_reload: argv["live-reload"] }; } diff --git a/frappe/build.py b/frappe/build.py index 05fa213018..8b32b03d60 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -257,6 +257,13 @@ def watch(apps=None): if apps: command += " --apps {apps}".format(apps=apps) + live_reload = frappe.utils.cint( + os.environ.get("LIVE_RELOAD", frappe.conf.live_reload) + ) + + if live_reload: + command += " --live-reload" + check_node_executable() frappe_app_path = frappe.get_app_path("frappe", "..") frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env()) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 66bb3909da..bd33189d58 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -255,7 +255,7 @@ class Communication(Document, CommunicationEmailMixin): def set_delivery_status(self, commit=False): '''Look into the status of Email Queue linked to this Communication and set the Delivery Status of this Communication''' delivery_status = None - status_counts = Counter(frappe.db.sql_list('''select status from `tabEmail Queue` where communication=%s''', self.name)) + status_counts = Counter(frappe.get_all("Email Queue", pluck="status", filters={"communication": self.name})) if self.sent_or_received == "Received": return diff --git a/frappe/core/doctype/communication/mixins.py b/frappe/core/doctype/communication/mixins.py index 52cd370890..b6d8070d00 100644 --- a/frappe/core/doctype/communication/mixins.py +++ b/frappe/core/doctype/communication/mixins.py @@ -217,17 +217,7 @@ class CommunicationEmailMixin: if not emails: return [] - disabled_users = frappe.db.sql_list(""" - SELECT - email - FROM - `tabUser` - where - email in %(emails)s - and - thread_notify=0 - """, {'emails': tuple(emails)}) - return disabled_users + return frappe.get_all("User", pluck="email", filters={"email": ["in", emails], "thread_notify": 0}) @staticmethod def filter_disabled_users(emails): @@ -236,17 +226,7 @@ class CommunicationEmailMixin: if not emails: return [] - disabled_users = frappe.db.sql_list(""" - SELECT - email - FROM - `tabUser` - where - email in %(emails)s - and - enabled=0 - """, {'emails': tuple(emails)}) - return disabled_users + return frappe.get_all("User", pluck="email", filters={"email": ["in", emails], "enabled": 0}) def sendmail_input_dict(self, print_html=None, print_format=None, send_me_a_copy=None, print_letterhead=None, is_inbound_mail_communcation=None): diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py index 7c660c7180..c5cf67ba57 100644 --- a/frappe/core/doctype/data_export/exporter.py +++ b/frappe/core/doctype/data_export/exporter.py @@ -261,6 +261,7 @@ class DataExporter: self.writer.writerow([self.data_keys.data_separator]) def add_data(self): + from frappe.query_builder import DocType if self.template and not self.with_data: return @@ -305,9 +306,15 @@ class DataExporter: if self.all_doctypes: # add child tables for c in self.child_doctypes: - for ci, child in enumerate(frappe.db.sql("""select * from `tab{0}` - where parent=%s and parentfield=%s order by idx""".format(c['doctype']), - (doc.name, c['parentfield']), as_dict=1)): + child_doctype_table = DocType(c["doctype"]) + data_row = ( + frappe.qb.from_(child_doctype_table) + .select("*") + .where(child_doctype_table.parent == doc.name) + .where(child_doctype_table.parentfield == c["parentfield"]) + .orderby(child_doctype_table.idx) + ) + for ci, child in enumerate(data_row.run()): self.add_data_row(rows, c['doctype'], c['parentfield'], child, ci) for row in rows: diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 8f8a8ed287..5a91016e32 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -23,6 +23,7 @@ from frappe.modules.import_file import get_file_path from frappe.model.meta import Meta from frappe.desk.utils import validate_route_conflict from frappe.website.utils import clear_cache +from frappe.query_builder.functions import Concat class InvalidFieldNameError(frappe.ValidationError): pass class UniqueFieldnameError(frappe.ValidationError): pass @@ -465,7 +466,7 @@ class DocType(Document): return # check if atleast 1 record exists - if not (frappe.db.table_exists(self.name) and frappe.db.sql("select name from `tab{}` limit 1".format(self.name))): + if not (frappe.db.table_exists(self.name) and frappe.get_all(self.name, fields=["name"], limit=1, as_list=True)): return existing_property_setter = frappe.db.get_value("Property Setter", {"doc_type": self.name, @@ -571,17 +572,17 @@ class DocType(Document): def make_amendable(self): """If is_submittable is set, add amended_from docfields.""" if self.is_submittable: - if not frappe.db.sql("""select name from tabDocField - where fieldname = 'amended_from' and parent = %s""", self.name): - self.append("fields", { - "label": "Amended From", - "fieldtype": "Link", - "fieldname": "amended_from", - "options": self.name, - "read_only": 1, - "print_hide": 1, - "no_copy": 1 - }) + docfield_exists = frappe.get_all("DocField", filters={"fieldname": "amended_from", "parent": self.name}, pluck="name", limit=1) + if not docfield_exists: + self.append("fields", { + "label": "Amended From", + "fieldtype": "Link", + "fieldname": "amended_from", + "options": self.name, + "read_only": 1, + "print_hide": 1, + "no_copy": 1 + }) def make_repeatable(self): """If allow_auto_repeat is set, add auto_repeat custom field.""" @@ -706,12 +707,13 @@ def validate_series(dt, autoname=None, name=None): and (not autoname.startswith('format:')): prefix = autoname.split('.')[0] - used_in = frappe.db.sql(""" - SELECT `name` - FROM `tabDocType` - WHERE `autoname` LIKE CONCAT(%s, '.%%') - AND `name`!=%s - """, (prefix, name)) + doctype = frappe.qb.DocType("DocType") + used_in = (frappe.qb + .from_(doctype) + .select(doctype.name) + .where(doctype.autoname.like(Concat(prefix,".%"))) + .where(doctype.name != name) + ).run() if used_in: frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 4538ffb6bb..9a758b53f5 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -204,10 +204,14 @@ class TestFile(unittest.TestCase): def delete_test_data(self): - for f in frappe.db.sql('''select name, file_name from tabFile where - is_home_folder = 0 and is_attachments_folder = 0 order by creation desc'''): - frappe.delete_doc("File", f[0]) - + test_file_data = frappe.db.get_all( + "File", + pluck="name", + filters={"is_home_folder": 0, "is_attachments_folder": 0}, + order_by="creation desc", + ) + for f in test_file_data: + frappe.delete_doc("File", f) def upload_file(self): _file = frappe.get_doc({ diff --git a/frappe/core/doctype/language/language.json b/frappe/core/doctype/language/language.json index eed29883c1..9ab8f55f6b 100644 --- a/frappe/core/doctype/language/language.json +++ b/frappe/core/doctype/language/language.json @@ -7,6 +7,7 @@ "document_type": "Setup", "engine": "InnoDB", "field_order": [ + "enabled", "language_code", "language_name", "flag", @@ -39,15 +40,22 @@ "fieldtype": "Link", "label": "Based On", "options": "Language" + }, + { + "default": "1", + "fieldname": "enabled", + "fieldtype": "Check", + "label": "Enabled" } ], "icon": "fa fa-globe", "in_create": 1, "links": [], - "modified": "2020-04-16 22:11:33.066852", + "modified": "2021-10-18 14:02:06.818219", "modified_by": "Administrator", "module": "Core", "name": "Language", + "naming_rule": "By fieldname", "owner": "Administrator", "permissions": [ { diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 8a471b9173..c505302c52 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -38,7 +38,7 @@ def has_unseen_error_log(user): 'message': _("You have unseen {0}").format(' Error Logs ') } - if frappe.db.sql_list("select name from `tabError Log` where seen = 0 limit 1"): + if frappe.get_all("Error Log", filters={"seen": 0}, limit=1): log_settings = frappe.get_cached_doc('Log Settings') if log_settings.users_to_notify: diff --git a/frappe/core/doctype/navbar_settings/navbar_settings.py b/frappe/core/doctype/navbar_settings/navbar_settings.py index fd8db31d10..46eb5c3e7a 100644 --- a/frappe/core/doctype/navbar_settings/navbar_settings.py +++ b/frappe/core/doctype/navbar_settings/navbar_settings.py @@ -22,7 +22,6 @@ class NavbarSettings(Document): if not frappe.flags.in_patch and (len(before_save_items) > len(after_save_items)): frappe.throw(_("Please hide the standard navbar items instead of deleting them")) -@frappe.whitelist(allow_guest=True) def get_app_logo(): app_logo = frappe.db.get_single_value('Navbar Settings', 'app_logo', cache=True) if not app_logo: diff --git a/frappe/core/doctype/transaction_log/transaction_log.py b/frappe/core/doctype/transaction_log/transaction_log.py index bb94642f48..e2e75b130c 100644 --- a/frappe/core/doctype/transaction_log/transaction_log.py +++ b/frappe/core/doctype/transaction_log/transaction_log.py @@ -14,10 +14,9 @@ class TransactionLog(Document): self.row_index = index self.timestamp = now_datetime() if index != 1: - prev_hash = frappe.db.sql( - "SELECT `chaining_hash` FROM `tabTransaction Log` WHERE `row_index` = '{0}'".format(index - 1)) + prev_hash = frappe.get_all("Transaction Log", filters={"row_index":str(index-1)}, pluck="chaining_hash", limit=1) if prev_hash: - self.previous_hash = prev_hash[0][0] + self.previous_hash = prev_hash[0] else: self.previous_hash = "Indexing broken" else: diff --git a/frappe/core/doctype/user_permission/user_permission.py b/frappe/core/doctype/user_permission/user_permission.py index 66ffd48822..1366ace115 100644 --- a/frappe/core/doctype/user_permission/user_permission.py +++ b/frappe/core/doctype/user_permission/user_permission.py @@ -54,7 +54,7 @@ class UserPermission(Document): ref_link = frappe.get_desk_link(self.doctype, overlap_exists[0].name) frappe.throw(_("{0} has already assigned default value for {1}.").format(ref_link, self.allow)) -@frappe.whitelist(allow_guest=True) +@frappe.whitelist() def get_user_permissions(user=None): '''Get all users permissions for the user as a dict of doctype''' # if this is called from client-side, diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index d276a9707f..89e6598859 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -13,7 +13,7 @@ from frappe.desk.form.document_follow import is_document_followed from frappe import _ from urllib.parse import quote -@frappe.whitelist(allow_guest=True) +@frappe.whitelist() def getdoc(doctype, name, user=None): """ Loads a doclist for a given document. This method is called directly from the client. @@ -52,7 +52,7 @@ def getdoc(doctype, name, user=None): frappe.response.docs.append(doc) -@frappe.whitelist(allow_guest=True) +@frappe.whitelist() def getdoctype(doctype, with_parent=False, cached_timestamp=None): """load doctype""" diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index f079205cb0..e733adf868 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -2,7 +2,7 @@ # License: MIT. See LICENSE import frappe -@frappe.whitelist(allow_guest=True) +@frappe.whitelist() def get_list_settings(doctype): try: return frappe.get_cached_doc("List View Settings", doctype) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 31eb224652..6c9fa2e937 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -14,7 +14,7 @@ from frappe.utils import cstr, format_duration from frappe.model.base_document import get_controller -@frappe.whitelist(allow_guest=True) +@frappe.whitelist() @frappe.read_only() def get(): args = get_form_params() diff --git a/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py b/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py index 90927e13f8..9ccd1c0210 100644 --- a/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py +++ b/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py @@ -336,7 +336,6 @@ def dropbox_auth_finish(return_access_token=False): _("Dropbox access is approved!") + close, indicator_color='green') -@frappe.whitelist(allow_guest=True) def set_dropbox_access_token(access_token): frappe.db.set_value("Dropbox Settings", None, 'dropbox_access_token', access_token) frappe.db.commit() diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 978f3062c5..8f0e0aaefc 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -597,8 +597,8 @@ class DatabaseQuery(object): self.conditions.append(self.get_share_condition()) else: - #if has if_owner permission skip user perm check - if role_permissions.get("has_if_owner_enabled") and role_permissions.get("if_owner", {}): + # skip user perm check if owner constraint is required + if requires_owner_constraint(role_permissions): self.match_conditions.append( f"`tab{self.doctype}`.`owner` = {frappe.db.escape(self.user, percent=False)}" ) @@ -895,3 +895,22 @@ def get_date_range(operator, value): timespan = period_map[operator] + ' ' + timespan_map[value] if operator != 'timespan' else value return get_timespan_date_range(timespan) + +def requires_owner_constraint(role_permissions): + """Returns True if "select" or "read" isn't available without being creator.""" + + if not role_permissions.get("has_if_owner_enabled"): + return + + if_owner_perms = role_permissions.get("if_owner") + if not if_owner_perms: + return + + # has select or read without if owner, no need for constraint + for perm_type in ("select", "read"): + if role_permissions.get(perm_type) and perm_type not in if_owner_perms: + return + + # not checking if either select or read if present in if_owner_perms + # because either of those is required to perform a query + return True diff --git a/frappe/permissions.py b/frappe/permissions.py index 7ee1119ebb..a086c73920 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -107,13 +107,9 @@ def get_doc_permissions(doc, user=None, ptype=None): meta = frappe.get_meta(doc.doctype) def is_user_owner(): - doc_owner = doc.get('owner') or '' - doc_owner = doc_owner.lower() - session_user = frappe.session.user.lower() - return doc_owner == session_user + return (doc.get("owner") or "").lower() == frappe.session.user.lower() - - if has_controller_permissions(doc, ptype, user=user) == False : + if has_controller_permissions(doc, ptype, user=user) is False: push_perm_check_log('Not allowed via controller permission check') return {ptype: 0} @@ -182,22 +178,23 @@ def get_role_permissions(doctype_meta, user=None, is_owner=None): applicable_permissions = list(filter(is_perm_applicable, getattr(doctype_meta, 'permissions', []))) has_if_owner_enabled = any(p.get('if_owner', 0) for p in applicable_permissions) - perms['has_if_owner_enabled'] = has_if_owner_enabled for ptype in rights: pvalue = any(p.get(ptype, 0) for p in applicable_permissions) # check if any perm object allows perm type perms[ptype] = cint(pvalue) - if (pvalue - and has_if_owner_enabled - and not has_permission_without_if_owner_enabled(ptype) - and ptype != 'create'): + if ( + pvalue + and has_if_owner_enabled + and not has_permission_without_if_owner_enabled(ptype) + and ptype != 'create' + ): perms['if_owner'][ptype] = cint(pvalue and is_owner) # has no access if not owner # only provide select or read access so that user is able to at-least access list # (and the documents will be filtered based on owner sin further checks) - perms[ptype] = 1 if ptype in ['select', 'read'] else 0 + perms[ptype] = 1 if ptype in ('select', 'read') else 0 frappe.local.role_permissions[cache_key] = perms diff --git a/frappe/public/js/frappe/build_events/BuildSuccess.vue b/frappe/public/js/frappe/build_events/BuildSuccess.vue index 75a365fdc2..5ab40271bb 100644 --- a/frappe/public/js/frappe/build_events/BuildSuccess.vue +++ b/frappe/public/js/frappe/build_events/BuildSuccess.vue @@ -3,8 +3,11 @@ v-if="is_shown" class="flex justify-between build-success-message align-center" > -
${__('Loading')}...
+