From 01d69b67c72e04d5ce1d57d86946e12fdc8e208f Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 17 Sep 2021 11:59:05 +0530 Subject: [PATCH 01/24] feat: automatically reload web pages whenever JS/CSS assets are rebuilt --- esbuild/esbuild.js | 10 ++++++++-- frappe/build.py | 3 +++ .../js/frappe/build_events/BuildSuccess.vue | 17 +++++++++++++---- .../frappe/build_events/build_events.bundle.js | 5 +++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index 9074beae06..b0ed92f9ec 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("auto-reload", { + type: "boolean", + description: `Automatically reload webpages when assets are rebuilt. + Can only be used with the --watch flag.` + }) .option("production", { type: "boolean", description: "Run build in production mode" @@ -475,7 +480,8 @@ async function notify_redis({ error, success }) { } if (success) { payload = { - success: true + success: true, + autoreload: argv["auto-reload"] }; } @@ -528,4 +534,4 @@ function log_rebuilt_assets(prev_assets, new_assets) { log(" " + filename); } log(); -} \ No newline at end of file +} diff --git a/frappe/build.py b/frappe/build.py index dfbe20f31e..ec092d4b57 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -235,6 +235,9 @@ def watch(apps=None): if apps: command += " --apps {apps}".format(apps=apps) + if frappe.conf.autoreload_on_build: + command += " --auto-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/public/js/frappe/build_events/BuildSuccess.vue b/frappe/public/js/frappe/build_events/BuildSuccess.vue index 75a365fdc2..d489c4a1a3 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" > -
Compiled successfully
- + Compiled successfully + Refresh @@ -14,11 +17,17 @@ export default { name: "BuildSuccess", data() { return { - is_shown: false + is_shown: false, + autoreload: false, }; }, methods: { - show() { + show(data) { + if (data.autoreload) { + this.autoreload = true; + this.reload(); + } + this.is_shown = true; if (this.timeout) { clearTimeout(this.timeout); diff --git a/frappe/public/js/frappe/build_events/build_events.bundle.js b/frappe/public/js/frappe/build_events/build_events.bundle.js index 6c8986af3f..13b9c7a334 100644 --- a/frappe/public/js/frappe/build_events/build_events.bundle.js +++ b/frappe/public/js/frappe/build_events/build_events.bundle.js @@ -13,10 +13,11 @@ frappe.realtime.on("build_event", data => { } }); -function show_build_success() { +function show_build_success(data) { if (error) { error.hide(); } + if (!success) { let target = $('
') .appendTo($container) @@ -27,7 +28,7 @@ function show_build_success() { }); success = vm.$children[0]; } - success.show(); + success.show(data); } function show_build_error(data) { From 096700a77aaf74e66c7e795a42ba69869d41bf96 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 12 Oct 2021 19:11:25 +0530 Subject: [PATCH 02/24] fix: Loading state for query reports --- .../js/frappe/views/reports/query_report.js | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 7d68919821..8a50a516af 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -107,7 +107,6 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { } if (this.report_name !== frappe.get_route()[1]) { - // this.toggle_loading(true); // different report this.load_report(); } @@ -556,6 +555,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { refresh() { this.toggle_message(true); this.toggle_report(false); + this.show_loading_indicator(); let filters = this.get_filter_values(true); // only one refresh at a time @@ -645,6 +645,8 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.show_footer_message(); frappe.hide_progress(); + }).finally(() => { + this.hide_loading_indicator(); }); } @@ -869,6 +871,22 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { } } + show_loading_indicator() { + this.$loading.find('div').html(`
+
+ Generic Empty State +
+

${__('Loading')}...

+
`); + this.$report.hide(); + this.$loading.show(); + } + + hide_loading_indicator() { + this.$loading.hide(); + this.$report.show(); + } + get_chart_options(data) { let options = this.report_settings.get_chart_data ? this.report_settings.get_chart_data(data.columns, data.result) @@ -1679,6 +1697,9 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { .hide().appendTo(this.page.main); this.$chart = $('
').hide().appendTo(this.page.main); + + this.$loading = $(this.message_div('')).hide().appendTo(this.page.main); + this.$report = $('
').appendTo(this.page.main); this.$message = $(this.message_div('')).hide().appendTo(this.page.main); } @@ -1738,11 +1759,6 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.refresh(); } - toggle_loading(flag) { - this.toggle_message(flag, __('Loading') + '...'); - } - - toggle_nothing_to_show(flag) { let message = this.prepared_report ? __('This is a background report. Please set the appropriate filters and then generate a new one.') From 2bebf4b2c0e73f117664e7673d63928cac9bff35 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Sep 2021 15:10:21 +0530 Subject: [PATCH 03/24] refactor: convert core communication db.sql calls --- .../doctype/communication/communication.py | 2 +- frappe/core/doctype/communication/mixins.py | 24 ++----------------- 2 files changed, 3 insertions(+), 23 deletions(-) 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): From a81a0d16c27db7fcb9276cfa925a25867e75beb1 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Sep 2021 15:11:17 +0530 Subject: [PATCH 04/24] refactor: convert doctype data_export db.sql calls --- frappe/core/doctype/data_export/exporter.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) 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: From ac54e24fc4e8339c350cda3073ab4c03ee11d9e9 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Sep 2021 15:12:06 +0530 Subject: [PATCH 05/24] refactor: convert doctype db.sql calls --- frappe/core/doctype/doctype/doctype.py | 38 ++++++++++++++------------ 1 file changed, 20 insertions(+), 18 deletions(-) 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])) From ab976aaaae255e3606c0eebde8e217846acb265b Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Sep 2021 15:13:10 +0530 Subject: [PATCH 06/24] refactor: convert test_file db.sql calls --- frappe/core/doctype/file/test_file.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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({ From b0421ab7cca69862bcacf7922cd1d37276dfdbe3 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Sep 2021 15:16:46 +0530 Subject: [PATCH 07/24] refactor: convert doctype log settings db.sql calls --- frappe/core/doctype/log_settings/log_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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: From 3cc2202ce9bd82cc04d04b839c8e85b76bf4a7f2 Mon Sep 17 00:00:00 2001 From: abhishek Date: Wed, 13 Oct 2021 11:59:01 +0530 Subject: [PATCH 08/24] refactor: convert doctype transaction_log db.sql calls --- frappe/core/doctype/transaction_log/transaction_log.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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: From d257adc1302b720016cc3ed830ede379aee4403e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sat, 16 Oct 2021 16:48:33 +0530 Subject: [PATCH 09/24] fix: Rename loader functions --- .../js/frappe/views/reports/query_report.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 8a50a516af..4a3a953a01 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -555,7 +555,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { refresh() { this.toggle_message(true); this.toggle_report(false); - this.show_loading_indicator(); + this.show_loading_screen(); let filters = this.get_filter_values(true); // only one refresh at a time @@ -646,7 +646,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.show_footer_message(); frappe.hide_progress(); }).finally(() => { - this.hide_loading_indicator(); + this.hide_loading_screen(); }); } @@ -871,18 +871,20 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { } } - show_loading_indicator() { - this.$loading.find('div').html(`
+ show_loading_screen() { + const loading_state = `
Generic Empty State

${__('Loading')}...

-
`); +
`; + + this.$loading.find('div').html(loading_state); this.$report.hide(); this.$loading.show(); } - hide_loading_indicator() { + hide_loading_screen() { this.$loading.hide(); this.$report.show(); } @@ -1699,7 +1701,6 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { this.$chart = $('
').hide().appendTo(this.page.main); this.$loading = $(this.message_div('')).hide().appendTo(this.page.main); - this.$report = $('
').appendTo(this.page.main); this.$message = $(this.message_div('')).hide().appendTo(this.page.main); } From 236c84ab8bbba439057149c9a20eb289f7b85b52 Mon Sep 17 00:00:00 2001 From: abhishek Date: Mon, 18 Oct 2021 16:35:13 +0530 Subject: [PATCH 10/24] feat: semgrep rule for db.sql --- .github/helper/semgrep_rules/frappe_correctness.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index d9603e89aa..04341b3430 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -131,3 +131,10 @@ rules: key `$X` is uselessly assigned twice. This could be a potential bug. languages: [python] severity: ERROR + +- id: frappe-using-db.sql + pattern-regex: \.sql.*\( + 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 \ No newline at end of file From c560d67a5709320054b5b265f8f32ece5800ae6e Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 18 Oct 2021 16:47:11 +0530 Subject: [PATCH 11/24] fix: remove/update older whitelist calls --- frappe/core/doctype/navbar_settings/navbar_settings.py | 1 - frappe/core/doctype/user_permission/user_permission.py | 2 +- frappe/desk/form/load.py | 4 ++-- frappe/desk/listview.py | 2 +- frappe/desk/reportview.py | 2 +- .../integrations/doctype/dropbox_settings/dropbox_settings.py | 1 - frappe/sessions.py | 4 ++-- 7 files changed, 7 insertions(+), 9 deletions(-) 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/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/sessions.py b/frappe/sessions.py index ce104968ad..bdf18f8d82 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -17,8 +17,8 @@ import redis from urllib.parse import unquote from frappe.cache_manager import clear_user_cache -@frappe.whitelist(allow_guest=True) -def clear(user=None): +@frappe.whitelist() +def clear(): frappe.local.session_obj.update(force=True) frappe.local.db.commit() clear_user_cache(frappe.session.user) From d99403393acfbdefefce406a0025a7811306e4d3 Mon Sep 17 00:00:00 2001 From: Pruthvi Patel Date: Mon, 18 Oct 2021 17:52:33 +0530 Subject: [PATCH 12/24] fix: rename `auto-reload` to `live-reload` --- esbuild/esbuild.js | 4 ++-- frappe/build.py | 8 ++++++-- frappe/public/js/frappe/build_events/BuildSuccess.vue | 8 ++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index b0ed92f9ec..c71fbc752c 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -44,7 +44,7 @@ let argv = yargs type: "boolean", description: "Run in watch mode and rebuild on file changes" }) - .option("auto-reload", { + .option("live-reload", { type: "boolean", description: `Automatically reload webpages when assets are rebuilt. Can only be used with the --watch flag.` @@ -481,7 +481,7 @@ async function notify_redis({ error, success }) { if (success) { payload = { success: true, - autoreload: argv["auto-reload"] + live_reload: argv["live-reload"] }; } diff --git a/frappe/build.py b/frappe/build.py index ec092d4b57..4472049ab7 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -235,8 +235,12 @@ def watch(apps=None): if apps: command += " --apps {apps}".format(apps=apps) - if frappe.conf.autoreload_on_build: - command += " --auto-reload" + live_reload = frappe.conf.live_reload + if "LIVE_RELOAD" in os.environ: + live_reload = os.environ["LIVE_RELOAD"] + + if live_reload: + command += " --live-reload" check_node_executable() frappe_app_path = frappe.get_app_path("frappe", "..") diff --git a/frappe/public/js/frappe/build_events/BuildSuccess.vue b/frappe/public/js/frappe/build_events/BuildSuccess.vue index d489c4a1a3..5ab40271bb 100644 --- a/frappe/public/js/frappe/build_events/BuildSuccess.vue +++ b/frappe/public/js/frappe/build_events/BuildSuccess.vue @@ -5,7 +5,7 @@ > Compiled successfully Refresh @@ -18,13 +18,13 @@ export default { data() { return { is_shown: false, - autoreload: false, + live_reload: false, }; }, methods: { show(data) { - if (data.autoreload) { - this.autoreload = true; + if (data.live_reload) { + this.live_reload = true; this.reload(); } From 3cec476cbc2f62d2ce4bd010657a7ec7455a7bc6 Mon Sep 17 00:00:00 2001 From: Pruthvi Patel Date: Mon, 18 Oct 2021 19:07:13 +0530 Subject: [PATCH 13/24] fix: add whitespace --- esbuild/esbuild.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index c71fbc752c..faccb36698 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -46,7 +46,7 @@ let argv = yargs }) .option("live-reload", { type: "boolean", - description: `Automatically reload webpages when assets are rebuilt. + description: `Automatically reload web pages when assets are rebuilt. Can only be used with the --watch flag.` }) .option("production", { From 059e5441c27baf0a81b2356359eabd1e3eb354ab Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 18 Oct 2021 21:33:06 +0530 Subject: [PATCH 14/24] ci: use semgrep pattern instead of regex --- .github/helper/semgrep_rules/frappe_correctness.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 04341b3430..662e6e8022 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -132,8 +132,8 @@ rules: languages: [python] severity: ERROR -- id: frappe-using-db.sql - pattern-regex: \.sql.*\( +- id: frappe-using-db-sql + pattern: frappe.db.sql(...) 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] From ac1bb636cf86490702dc6330d4c7e01937bb5d13 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 18 Oct 2021 22:11:38 +0530 Subject: [PATCH 15/24] ci: flag new instances of sql_ddl and sql_list --- .github/helper/semgrep_rules/frappe_correctness.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 662e6e8022..1297d7fa67 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -133,8 +133,11 @@ rules: severity: ERROR - id: frappe-using-db-sql - pattern: frappe.db.sql(...) + pattern-either: + - pattern: frappe.db.sql(...) + - pattern: frappe.db.sql_ddl(...) + - pattern: frappe.db.sql_list(...) 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 \ No newline at end of file + severity: ERROR From 586ef77cd713a2919b8848b114ed7d4ed5e9d1d3 Mon Sep 17 00:00:00 2001 From: hrwx Date: Mon, 18 Oct 2021 13:13:52 +0100 Subject: [PATCH 16/24] feat: Add enabled to Language --- frappe/core/doctype/language/language.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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": [ { From 770bda8cc624b9dc6a97113186701e8156b85034 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 19 Oct 2021 04:04:00 +0530 Subject: [PATCH 17/24] fix: use `cint`, as `os.environ` only contains string values --- frappe/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index 4472049ab7..ddce94ac2f 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -235,9 +235,9 @@ def watch(apps=None): if apps: command += " --apps {apps}".format(apps=apps) - live_reload = frappe.conf.live_reload - if "LIVE_RELOAD" in os.environ: - live_reload = os.environ["LIVE_RELOAD"] + live_reload = frappe.utils.cint( + os.environ.get("LIVE_RELOAD", frappe.conf.live_reload) + ) if live_reload: command += " --live-reload" From 2e4f5e0796a202d5100c5de69ac9e11772029726 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Tue, 19 Oct 2021 01:41:02 +0200 Subject: [PATCH 18/24] fix: handle negative duration --- frappe/public/js/frappe/utils/utils.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 831538d255..065ae415f5 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1049,16 +1049,16 @@ Object.assign(frappe.utils, { return duration; }, - seconds_to_duration(value, duration_options) { - let secs = value; + seconds_to_duration(secs, duration_options) { + let round = secs > 0 ? Math.floor : Math.ceil; let total_duration = { - days: Math.floor(secs / (3600 * 24)), - hours: Math.floor(secs % (3600 * 24) / 3600), - minutes: Math.floor(secs % 3600 / 60), - seconds: Math.floor(secs % 60) + days: round(secs / (3600 * 24)), + hours: round(secs % (3600 * 24) / 3600), + minutes: round(secs % 3600 / 60), + seconds: round(secs % 60) }; if (duration_options.hide_days) { - total_duration.hours = Math.floor(secs / 3600); + total_duration.hours = round(secs / 3600); total_duration.days = 0; } return total_duration; From a575cff8a7f55fc5550bd6a519e3e6288b9200ee Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 19 Oct 2021 12:35:55 +0530 Subject: [PATCH 19/24] fix: Retry flaky test_printview_page if errored --- frappe/tests/test_website.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 818dc8bce6..688679a80f 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -4,6 +4,7 @@ import frappe from frappe.utils import set_request from frappe.website.serve import get_response, get_response_content from frappe.website.utils import (build_response, clear_website_cache, get_home_page) +from tenacity import retry, stop_after_attempt, retry_if_exception_type class TestWebsite(unittest.TestCase): @@ -196,6 +197,11 @@ class TestWebsite(unittest.TestCase): delattr(frappe.hooks, 'page_renderer') frappe.cache().delete_key('app_hooks') + # TODO: Get rid of this retry logic + # Added since test is flaky and we can't figure out why at this point + @retry( + stop=stop_after_attempt(5), retry=retry_if_exception_type(AssertionError), + ) def test_printview_page(self): content = get_response_content('/Language/en') self.assertIn('
', content) From 4a5bb2873dc900f7c4394f6cb5ecf6d5170cfb18 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 19 Oct 2021 13:25:37 +0530 Subject: [PATCH 20/24] ci: Ignore sql=>qb rule for test files --- .github/helper/semgrep_rules/frappe_correctness.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 1297d7fa67..33a22fba6a 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -137,6 +137,9 @@ rules: - 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] From 7649cdff4cb21ee62b2c0cc254d7b722d6d11c3b Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 19 Oct 2021 14:00:19 +0530 Subject: [PATCH 21/24] fix: minor cleanup --- frappe/public/js/frappe/utils/utils.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/frappe/public/js/frappe/utils/utils.js b/frappe/public/js/frappe/utils/utils.js index 065ae415f5..b49dfa0280 100644 --- a/frappe/public/js/frappe/utils/utils.js +++ b/frappe/public/js/frappe/utils/utils.js @@ -1049,18 +1049,20 @@ Object.assign(frappe.utils, { return duration; }, - seconds_to_duration(secs, duration_options) { - let round = secs > 0 ? Math.floor : Math.ceil; - let total_duration = { - days: round(secs / (3600 * 24)), - hours: round(secs % (3600 * 24) / 3600), - minutes: round(secs % 3600 / 60), - seconds: round(secs % 60) + seconds_to_duration(seconds, duration_options) { + const round = seconds > 0 ? Math.floor : Math.ceil; + const total_duration = { + days: round(seconds / 86400), // 60 * 60 * 24 + hours: round(seconds % 86400 / 3600), + minutes: round(seconds % 3600 / 60), + seconds: round(seconds % 60) }; + if (duration_options.hide_days) { - total_duration.hours = round(secs / 3600); + total_duration.hours = round(seconds / 3600); total_duration.days = 0; } + return total_duration; }, From 4f94f19940c23835bff26911d89f5d50e746d691 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Tue, 19 Oct 2021 14:46:01 +0530 Subject: [PATCH 22/24] ci: Remove Github status for UI coverage --- codecov.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/codecov.yml b/codecov.yml index eeba1ff381..90fec8fbcb 100644 --- a/codecov.yml +++ b/codecov.yml @@ -10,11 +10,6 @@ coverage: threshold: 0.5% flags: - server - ui-tests: - target: auto - threshold: 0.5% - flags: - - ui-tests comment: layout: "diff, flags" @@ -28,4 +23,4 @@ flags: ui-tests: paths: - ".*\\.js" - carryforward: true \ No newline at end of file + carryforward: true From 5c86dc80c7905896b9b308754c012afc4576f1c5 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 19 Oct 2021 15:03:17 +0530 Subject: [PATCH 23/24] fix: `if_owner` constraint being applied wrongly (#14471) * fix: `if_owner` constraint being applied wrongly * test: check if owner on `frappe.get_list` Co-authored-by: Pruthvi Patel --- frappe/model/db_query.py | 23 +++++++++++++++++++++-- frappe/permissions.py | 21 +++++++++------------ frappe/tests/test_permissions.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 14 deletions(-) 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/tests/test_permissions.py b/frappe/tests/test_permissions.py index 48510f55f6..f4276b2d59 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -493,6 +493,34 @@ class TestPermissions(unittest.TestCase): frappe.set_user("test2@example.com") self.assertRaises(frappe.PermissionError, getdoc, 'Blog Post', doc.name) + def test_if_owner_permission_on_get_list(self): + doc = frappe.get_doc({ + "doctype": "Blog Post", + "blog_category": "-test-blog-category", + "blogger": "_Test Blogger 1", + "title": "_Test If Owner Permissions on Get List", + "content": "_Test Blog Post Content" + }) + + doc.insert(ignore_if_duplicate=True) + + update('Blog Post', 'Blogger', 0, 'if_owner', 1) + update('Blog Post', 'Blogger', 0, 'read', 1) + user = frappe.get_doc("User", "test2@example.com") + user.add_roles("Website Manager") + frappe.clear_cache(doctype="Blog Post") + + frappe.set_user("test2@example.com") + self.assertIn(doc.name, frappe.get_list("Blog Post", pluck="name")) + + # Become system manager to remove role + frappe.set_user("test1@example.com") + user.remove_roles("Website Manager") + frappe.clear_cache(doctype="Blog Post") + + frappe.set_user("test2@example.com") + self.assertNotIn(doc.name, frappe.get_list("Blog Post", pluck="name")) + def test_if_owner_permission_on_delete(self): update('Blog Post', 'Blogger', 0, 'if_owner', 1) update('Blog Post', 'Blogger', 0, 'read', 1) From 50e9930ca5d630ec9366b2ec87c5a785230783b5 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Tue, 19 Oct 2021 18:53:43 +0530 Subject: [PATCH 24/24] ci: Disable patch status Can be enabled later --- codecov.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/codecov.yml b/codecov.yml index 90fec8fbcb..a9f6df0296 100644 --- a/codecov.yml +++ b/codecov.yml @@ -3,6 +3,7 @@ codecov: coverage: status: + patch: off project: default: false server: