From 31440f0538a767b8ee72e0c7ede8db2c359ed532 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 13 Apr 2022 13:37:20 +0530 Subject: [PATCH 01/13] refactor: Integration Request --- .../braintree_settings/braintree_settings.py | 2 +- .../integration_request.json | 360 +++++------------- .../paypal_settings/paypal_settings.py | 8 +- .../doctype/paytm_settings/paytm_settings.py | 2 +- .../razorpay_settings/razorpay_settings.py | 7 +- .../stripe_settings/stripe_settings.py | 2 +- frappe/integrations/utils.py | 49 ++- .../v14_0/update_integration_request.py | 17 + 8 files changed, 156 insertions(+), 291 deletions(-) create mode 100644 frappe/patches/v14_0/update_integration_request.py diff --git a/frappe/integrations/doctype/braintree_settings/braintree_settings.py b/frappe/integrations/doctype/braintree_settings/braintree_settings.py index ca7ab0cfdf..17330d7c84 100644 --- a/frappe/integrations/doctype/braintree_settings/braintree_settings.py +++ b/frappe/integrations/doctype/braintree_settings/braintree_settings.py @@ -190,7 +190,7 @@ class BraintreeSettings(Document): self.data = frappe._dict(data) try: - self.integration_request = create_request_log(self.data, "Host", "Braintree") + self.integration_request = create_request_log(self.data, service_name="Braintree") return self.create_charge_on_braintree() except Exception: diff --git a/frappe/integrations/doctype/integration_request/integration_request.json b/frappe/integrations/doctype/integration_request/integration_request.json index 8a3fbc41ba..98db8ea748 100644 --- a/frappe/integrations/doctype/integration_request/integration_request.json +++ b/frappe/integrations/doctype/integration_request/integration_request.json @@ -1,334 +1,154 @@ { - "allow_copy": 0, - "allow_events_in_timeline": 0, - "allow_guest_to_view": 0, - "allow_import": 0, - "allow_rename": 0, - "beta": 0, - "creation": "2016-08-04 04:58:40.457416", - "custom": 0, - "docstatus": 0, + "actions": [], + "creation": "2022-03-28 12:25:29.929952", "doctype": "DocType", - "document_type": "", "editable_grid": 1, "engine": "InnoDB", + "field_order": [ + "request_id", + "integration_request_service", + "is_remote_request", + "column_break_5", + "request_description", + "status", + "section_break_8", + "url", + "request_headers", + "data", + "response_section", + "output", + "error", + "reference_section", + "reference_doctype", + "column_break_16", + "reference_docname" + ], "fields": [ { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "integration_type", - "fieldtype": "Select", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Integration Type", - "length": 0, - "no_copy": 0, - "options": "\nHost\nRemote\nSubscription Notification", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, - { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, "fieldname": "integration_request_service", "fieldtype": "Data", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Integration Request Service", - "length": 0, - "no_copy": 0, - "options": "", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "label": "Service", + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "default": "Queued", - "fetch_if_empty": 0, "fieldname": "status", "fieldtype": "Select", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, "in_list_view": 1, "in_standard_filter": 1, "label": "Status", - "length": 0, - "no_copy": 0, "options": "\nQueued\nAuthorized\nCompleted\nCancelled\nFailed", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, "fieldname": "data", "fieldtype": "Code", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Data", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "label": "Request Data", + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, "fieldname": "output", "fieldtype": "Code", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, "label": "Output", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, "fieldname": "error", "fieldtype": "Code", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, "label": "Error", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, "fieldname": "reference_doctype", "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, "label": "Reference Document Type", - "length": 0, - "no_copy": 0, "options": "DocType", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, "fieldname": "reference_docname", "fieldtype": "Dynamic Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Reference Docname", - "length": 0, - "no_copy": 0, + "label": "Reference Document Name", "options": "reference_doctype", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 + }, + { + "default": "0", + "fieldname": "is_remote_request", + "fieldtype": "Check", + "label": "Is Remote Request?", + "read_only": 1 + }, + { + "fieldname": "request_description", + "fieldtype": "Data", + "label": "Request Description", + "read_only": 1 + }, + { + "fieldname": "request_id", + "fieldtype": "Data", + "label": "Request ID", + "read_only": 1 + }, + { + "fieldname": "column_break_5", + "fieldtype": "Column Break" + }, + { + "fieldname": "section_break_8", + "fieldtype": "Section Break" + }, + { + "fieldname": "url", + "fieldtype": "Data", + "label": "URL", + "read_only": 1 + }, + { + "fieldname": "response_section", + "fieldtype": "Section Break", + "label": "Response" + }, + { + "depends_on": "eval:doc.reference_doctype", + "fieldname": "reference_section", + "fieldtype": "Section Break", + "label": "Reference" + }, + { + "fieldname": "column_break_16", + "fieldtype": "Column Break" + }, + { + "fieldname": "request_headers", + "fieldtype": "Code", + "label": "Request Headers", + "read_only": 1 } ], - "has_web_view": 0, - "hide_heading": 0, - "hide_toolbar": 0, - "idx": 0, - "image_view": 0, "in_create": 1, - "is_submittable": 0, - "issingle": 0, - "istable": 0, - "max_attachments": 0, - "modified": "2019-09-05 14:22:27.664645", + "links": [], + "modified": "2022-04-07 11:32:27.557548", "modified_by": "Administrator", "module": "Integrations", "name": "Integration Request", - "name_case": "", "owner": "Administrator", "permissions": [ { - "amend": 0, - "cancel": 0, - "create": 1, "delete": 1, "email": 1, "export": 1, - "if_owner": 0, - "import": 0, - "permlevel": 0, "print": 1, "read": 1, "report": 1, "role": "System Manager", - "set_user_permissions": 0, - "share": 1, - "submit": 0, - "write": 1 + "share": 1 } ], - "quick_entry": 0, - "read_only": 0, - "read_only_onload": 0, - "show_name_in_global_search": 0, "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "integration_request_service", - "track_changes": 1, - "track_seen": 0, - "track_views": 0 + "track_changes": 1 } \ No newline at end of file diff --git a/frappe/integrations/doctype/paypal_settings/paypal_settings.py b/frappe/integrations/doctype/paypal_settings/paypal_settings.py index ab7512f403..3568b77baf 100644 --- a/frappe/integrations/doctype/paypal_settings/paypal_settings.py +++ b/frappe/integrations/doctype/paypal_settings/paypal_settings.py @@ -182,9 +182,8 @@ class PayPalSettings(Document): kwargs.update( {"token": response.get("TOKEN")[0], "correlation_id": response.get("CORRELATIONID")[0]} ) - self.integration_request = create_request_log( - kwargs, "Remote", "PayPal", response.get("TOKEN")[0] - ) + + create_request_log(kwargs, service_name="PayPal", name=kwargs["token"]) return return_url.format(kwargs["token"]) @@ -463,7 +462,8 @@ def ipn_handler(): { "data": json.dumps(frappe.local.form_dict), "doctype": "Integration Request", - "integration_type": "Subscription Notification", + "request_description": "Subscription Notification", + "is_remote_request": 1, "status": "Queued", } ).insert(ignore_permissions=True) diff --git a/frappe/integrations/doctype/paytm_settings/paytm_settings.py b/frappe/integrations/doctype/paytm_settings/paytm_settings.py index 0888fd35b7..d8c9159303 100644 --- a/frappe/integrations/doctype/paytm_settings/paytm_settings.py +++ b/frappe/integrations/doctype/paytm_settings/paytm_settings.py @@ -34,7 +34,7 @@ class PaytmSettings(Document): def get_payment_url(self, **kwargs): """Return payment url with several params""" # create unique order id by making it equal to the integration request - integration_request = create_request_log(kwargs, "Host", "Paytm") + integration_request = create_request_log(kwargs, service_name="Paytm") kwargs.update(dict(order_id=integration_request.name)) return get_url("./integrations/paytm_checkout?{0}".format(urlencode(kwargs))) diff --git a/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py b/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py index c4ffb74325..c2422d5bc3 100644 --- a/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py +++ b/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py @@ -196,7 +196,7 @@ class RazorpaySettings(Document): return kwargs def get_payment_url(self, **kwargs): - integration_request = create_request_log(kwargs, "Host", "Razorpay") + integration_request = create_request_log(kwargs, service_name="Razorpay") return get_url("./integrations/razorpay_checkout?token={0}".format(integration_request.name)) def create_order(self, **kwargs): @@ -206,7 +206,7 @@ class RazorpaySettings(Document): kwargs["amount"] *= 100 # Create integration log - integration_request = create_request_log(kwargs, "Host", "Razorpay") + integration_request = create_request_log(kwargs, service_name="Razorpay") # Setup payment options payment_options = { @@ -490,7 +490,8 @@ def razorpay_subscription_callback(): { "data": json.dumps(frappe.local.form_dict), "doctype": "Integration Request", - "integration_type": "Subscription Notification", + "request_description": "Subscription Notification", + "is_remote_request": 1, "status": "Queued", } ).insert(ignore_permissions=True) diff --git a/frappe/integrations/doctype/stripe_settings/stripe_settings.py b/frappe/integrations/doctype/stripe_settings/stripe_settings.py index 4ebf902e84..bc8b0dfa17 100644 --- a/frappe/integrations/doctype/stripe_settings/stripe_settings.py +++ b/frappe/integrations/doctype/stripe_settings/stripe_settings.py @@ -200,7 +200,7 @@ class StripeSettings(Document): stripe.default_http_client = stripe.http_client.RequestsClient() try: - self.integration_request = create_request_log(self.data, "Host", "Stripe") + self.integration_request = create_request_log(self.data, service_name="Stripe") return self.create_charge_on_stripe() except Exception: diff --git a/frappe/integrations/utils.py b/frappe/integrations/utils.py index 191cd1f23b..1440c80399 100644 --- a/frappe/integrations/utils.py +++ b/frappe/integrations/utils.py @@ -42,22 +42,45 @@ def make_put_request(url, **kwargs): return make_request("PUT", url, **kwargs) -def create_request_log(data, integration_type, service_name, name=None, error=None): - if isinstance(data, str): - data = json.loads(data) - - if isinstance(error, str): - error = json.loads(error) +def create_request_log( + data, + integration_type=None, + service_name=None, + name=None, + error=None, + request_headers=None, + output=None, + **kwargs +): + """ + DEPRECATED: The parameter integration_type will be removed in the next major release. + Use is_remote_request instead. + """ + if integration_type == "Remote": + kwargs["is_remote_request"] = 1 + + elif integration_type == "Subscription Notification": + kwargs["request_description"] = integration_type + + reference_doctype = reference_docname = None + if "reference_doctype" not in kwargs: + if isinstance(data, str): + data = json.loads(data) + + reference_doctype = data.get("reference_doctype") + reference_docname = data.get("reference_docname") integration_request = frappe.get_doc( { "doctype": "Integration Request", - "integration_type": integration_type, "integration_request_service": service_name, - "reference_doctype": data.get("reference_doctype"), - "reference_docname": data.get("reference_docname"), - "error": json.dumps(error, default=json_handler), - "data": json.dumps(data, default=json_handler), + "request_headers": get_json(request_headers), + "data": get_json(data), + "output": get_json(output), + "error": get_json(error), + "reference_doctype": reference_doctype, + "reference_docname": reference_docname, + **kwargs, } ) @@ -70,6 +93,10 @@ def create_request_log(data, integration_type, service_name, name=None, error=No return integration_request +def get_json(obj): + return obj if isinstance(obj, str) else frappe.as_json(obj, indent=1) + + def get_payment_gateway_controller(payment_gateway): """Return payment gateway controller""" gateway = frappe.get_doc("Payment Gateway", payment_gateway) diff --git a/frappe/patches/v14_0/update_integration_request.py b/frappe/patches/v14_0/update_integration_request.py new file mode 100644 index 0000000000..7d491461e3 --- /dev/null +++ b/frappe/patches/v14_0/update_integration_request.py @@ -0,0 +1,17 @@ +import frappe + + +def execute(): + doctype = "Integration Request" + frappe.db.set_value( + doctype, + {"integration_type": "Remote", "integration_request_service": ("!=", "PayPal")}, + "is_remote_request", + 1, + ) + frappe.db.set_value( + doctype, + {"integration_type": "Subscription Notification"}, + "request_description", + "Subscription Notification", + ) From 2e4953b8ecf4d4606d54b7cc6a8bfdea323f0dce Mon Sep 17 00:00:00 2001 From: KrutikaBhatt <65107474+KrutikaBhatt@users.noreply.github.com> Date: Sat, 30 Apr 2022 19:58:52 +0530 Subject: [PATCH 02/13] fix(bug): required meta field is used inplace of mandatory --- frappe/public/js/frappe/ui/sort_selector.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/ui/sort_selector.js b/frappe/public/js/frappe/ui/sort_selector.js index 879466e8f7..837454ed09 100644 --- a/frappe/public/js/frappe/ui/sort_selector.js +++ b/frappe/public/js/frappe/ui/sort_selector.js @@ -132,7 +132,7 @@ frappe.ui.SortSelector = class SortSelector { // bold, mandatory and fields that are available in list view meta.fields.forEach(function(df) { if ( - (df.mandatory || df.bold || df.in_list_view) + (df.mandatory || df.bold || df.in_list_view || df.reqd) && frappe.model.is_value_type(df.fieldtype) && frappe.perm.has_perm(me.doctype, df.permlevel, "read") ) { From 693a6a7789be4c578174f2a1d014f3158986c26f Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sun, 1 May 2022 11:34:08 +0530 Subject: [PATCH 03/13] fix: `frappe.log_error` arguments while capturing razorpay payment failures --- .../doctype/razorpay_settings/razorpay_settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py b/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py index c4ffb74325..cc620aa32c 100644 --- a/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py +++ b/frappe/integrations/doctype/razorpay_settings/razorpay_settings.py @@ -140,7 +140,7 @@ class RazorpaySettings(Document): headers={"content-type": "application/json"}, ) if not resp.get("id"): - frappe.log_error(str(resp), "Razorpay Failed while creating subscription") + frappe.log_error(message=str(resp), title="Razorpay Failed while creating subscription") except: frappe.log_error(frappe.get_traceback()) # failed @@ -179,7 +179,7 @@ class RazorpaySettings(Document): frappe.flags.status = "created" return kwargs else: - frappe.log_error(str(resp), "Razorpay Failed while creating subscription") + frappe.log_error(message=str(resp), title="Razorpay Failed while creating subscription") except: frappe.log_error(frappe.get_traceback()) @@ -281,7 +281,7 @@ class RazorpaySettings(Document): self.flags.status_changed_to = "Verified" else: - frappe.log_error(str(resp), "Razorpay Payment not authorized") + frappe.log_error(message=str(resp), title="Razorpay Payment not authorized") except: frappe.log_error(frappe.get_traceback()) From a688577a3fbf568e3d8aded29451aa2c793be91c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 2 May 2022 16:17:19 +0530 Subject: [PATCH 04/13] fix(setup_wizard): Merge language & country info slides Minimizing the number of slides required for setting up a new site. In developer mode, only one slide is visible now titled "Hello" or selected language equivalent. --- frappe/desk/page/setup_wizard/setup_wizard.js | 78 ++++++++----------- frappe/public/js/frappe/ui/slides.js | 4 +- 2 files changed, 33 insertions(+), 49 deletions(-) diff --git a/frappe/desk/page/setup_wizard/setup_wizard.js b/frappe/desk/page/setup_wizard/setup_wizard.js index cc91a16345..eee4394edd 100644 --- a/frappe/desk/page/setup_wizard/setup_wizard.js +++ b/frappe/desk/page/setup_wizard/setup_wizard.js @@ -49,7 +49,6 @@ frappe.pages['setup-wizard'].on_page_load = function (wrapper) { } frappe.wizard = new frappe.setup.SetupWizard(wizard_settings); frappe.setup.run_event("after_load"); - // frappe.wizard.values = test_values_edu; let route = frappe.get_route(); if (route) { frappe.wizard.show_slide(route[1]); @@ -145,7 +144,7 @@ frappe.setup.SetupWizard = class SetupWizard extends frappe.ui.Slides { refresh_slides() { // For Translations, etc. - if (this.in_refresh_slides || !this.current_slide.set_values()) { + if (this.in_refresh_slides || !this.current_slide.set_values(true)) { return; } this.in_refresh_slides = true; @@ -344,63 +343,39 @@ frappe.setup.slides_settings = [ // Welcome (language) slide name: "welcome", title: __("Hello!"), - icon: "fa fa-world", - // help: __("Let's prepare the system for first use."), fields: [ { - fieldname: "language", label: __("Your Language"), - fieldtype: "Select", reqd: 1 - } - ], - - onload: function (slide) { - this.setup_fields(slide); - let browser_language = frappe.setup.utils.get_language_name_from_code(navigator.language); - let language_field = slide.get_field("language"); - - language_field.set_input(browser_language || "English"); - - if (!frappe.setup._from_load_messages) { - language_field.$input.trigger("change"); - } - delete frappe.setup._from_load_messages; - moment.locale("en"); - }, - - setup_fields: function (slide) { - frappe.setup.utils.setup_language_field(slide); - frappe.setup.utils.bind_language_events(slide); - }, - }, - - { - // Region slide - name: 'region', - title: __("Select Your Region"), - icon: "fa fa-flag", - // help: __("Select your Country, Time Zone and Currency"), - fields: [ + fieldname: "language", + label: __("Your Language"), + fieldtype: "Select", + placeholder: __('Select Language'), + reqd: 1, + }, { - fieldname: "country", label: __("Your Country"), reqd: 1, + fieldname: "country", + label: __("Your Country"), fieldtype: "Autocomplete", - placeholder: __('Select Country') + placeholder: __('Select Country'), + reqd: 1, + }, + { + fieldtype: "Section Break" }, - { fieldtype: "Section Break" }, { fieldname: "timezone", label: __("Time Zone"), placeholder: __('Select Time Zone'), - reqd: 1, fieldtype: "Select", + reqd: 1, }, { fieldtype: "Column Break" }, { fieldname: "currency", label: __("Currency"), placeholder: __('Select Currency'), - reqd: 1, fieldtype: "Select", + reqd: 1, } ], @@ -410,14 +385,25 @@ frappe.setup.slides_settings = [ } else { frappe.setup.utils.load_regional_data(slide, this.setup_fields); } + if (!slide.get_value("language")) { + let session_language = frappe.setup.utils.get_language_name_from_code(frappe.boot.lang || navigator.language); + let language_field = slide.get_field("language"); + language_field.set_input(session_language || "English"); + if (!frappe.setup._from_load_messages) { + language_field.$input.trigger("change"); + } + delete frappe.setup._from_load_messages; + moment.locale("en"); + } + frappe.setup.utils.bind_region_events(slide); + frappe.setup.utils.bind_language_events(slide); }, setup_fields: function (slide) { frappe.setup.utils.setup_region_fields(slide); - frappe.setup.utils.bind_region_events(slide); - } + frappe.setup.utils.setup_language_field(slide); + }, }, - { // Profile slide name: 'user', @@ -438,7 +424,7 @@ frappe.setup.slides_settings = [ }, { "fieldname": "password", "label": __("Password"), "fieldtype": "Password" } ], - // help: __('The first user will become the System Manager (you can change this later).'), + onload: function (slide) { if (frappe.session.user !== "Administrator") { slide.form.fields_dict.email.$wrapper.toggle(false); @@ -555,9 +541,7 @@ frappe.setup.utils = { } slide.get_field("currency").set_input(frappe.wizard.values.currency); - slide.get_field("timezone").set_input(frappe.wizard.values.timezone); - }, bind_language_events: function (slide) { diff --git a/frappe/public/js/frappe/ui/slides.js b/frappe/public/js/frappe/ui/slides.js index f79f54b786..de24c1a4e6 100644 --- a/frappe/public/js/frappe/ui/slides.js +++ b/frappe/public/js/frappe/ui/slides.js @@ -105,8 +105,8 @@ frappe.ui.Slide = class Slide { }); } - set_values() { - this.values = this.form.get_values(); + set_values(ignore_errors) { + this.values = this.form.get_values(ignore_errors); if (this.values === null) { return false; } From 6cb8127e8d385960054ccaaea8035e80b41d0a01 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 2 May 2022 16:56:25 +0530 Subject: [PATCH 05/13] fix(setup_wizard): Make language field Autocomplete --- frappe/desk/page/setup_wizard/setup_wizard.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frappe/desk/page/setup_wizard/setup_wizard.js b/frappe/desk/page/setup_wizard/setup_wizard.js index eee4394edd..c683655dd5 100644 --- a/frappe/desk/page/setup_wizard/setup_wizard.js +++ b/frappe/desk/page/setup_wizard/setup_wizard.js @@ -348,8 +348,9 @@ frappe.setup.slides_settings = [ { fieldname: "language", label: __("Your Language"), - fieldtype: "Select", + fieldtype: "Autocomplete", placeholder: __('Select Language'), + default: "English", reqd: 1, }, { @@ -386,9 +387,10 @@ frappe.setup.slides_settings = [ frappe.setup.utils.load_regional_data(slide, this.setup_fields); } if (!slide.get_value("language")) { - let session_language = frappe.setup.utils.get_language_name_from_code(frappe.boot.lang || navigator.language); + let session_language = frappe.setup.utils.get_language_name_from_code(frappe.boot.lang || navigator.language) || "English"; let language_field = slide.get_field("language"); - language_field.set_input(session_language || "English"); + + language_field.set_input(session_language); if (!frappe.setup._from_load_messages) { language_field.$input.trigger("change"); } @@ -502,7 +504,7 @@ frappe.setup.utils = { setup_language_field: function (slide) { var language_field = slide.get_field("language"); language_field.df.options = frappe.setup.data.lang.languages; - language_field.refresh(); + language_field.set_options(); }, setup_region_fields: function (slide) { From 4310a2ecca213193a42065ccde0c7d2fbbd4a807 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 2 May 2022 17:30:21 +0530 Subject: [PATCH 06/13] fix(UX): suggest app-specific password for gmail logins (#16812) --- frappe/email/doctype/email_account/email_account.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/email/doctype/email_account/email_account.js b/frappe/email/doctype/email_account/email_account.js index 54f0d2372d..a357126a48 100644 --- a/frappe/email/doctype/email_account/email_account.js +++ b/frappe/email/doctype/email_account/email_account.js @@ -134,10 +134,11 @@ frappe.ui.form.on("Email Account", { show_gmail_message_for_less_secure_apps: function(frm) { frm.dashboard.clear_headline(); + let msg = __("GMail will only work if you enable 2-step authentication and use app-specific password."); + let cta = __("Read the step by step guide here."); + msg += ` ${cta}`; if (frm.doc.service==="GMail") { - frm.dashboard.set_headline_alert('Gmail will only work if you allow access for less secure \ - apps in Gmail settings. Read this for details'); + frm.dashboard.set_headline_alert(msg); } }, From d1938ee27198493c58df065a6111635a6340fad6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 May 2022 10:54:52 +0530 Subject: [PATCH 07/13] perf: remove naming series from log-like doctypes (#16823) - webhook request log - access log --- frappe/core/doctype/access_log/access_log.json | 8 ++++++-- .../doctype/webhook_request_log/webhook_request_log.json | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/access_log/access_log.json b/frappe/core/doctype/access_log/access_log.json index 6a3028303b..c5f1030266 100644 --- a/frappe/core/doctype/access_log/access_log.json +++ b/frappe/core/doctype/access_log/access_log.json @@ -1,5 +1,6 @@ { - "autoname": "format:AL-{#####}", + "actions": [], + "autoname": "hash", "creation": "2019-07-25 15:44:44.955496", "doctype": "DocType", "editable_grid": 1, @@ -127,10 +128,12 @@ "read_only": 1 } ], - "modified": "2019-08-05 19:00:13.839471", + "links": [], + "modified": "2022-05-03 09:34:19.337551", "modified_by": "Administrator", "module": "Core", "name": "Access Log", + "naming_rule": "Random", "owner": "Administrator", "permissions": [ { @@ -146,5 +149,6 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_seen": 1 } \ No newline at end of file diff --git a/frappe/integrations/doctype/webhook_request_log/webhook_request_log.json b/frappe/integrations/doctype/webhook_request_log/webhook_request_log.json index 96690f6e8c..d9410a2f82 100644 --- a/frappe/integrations/doctype/webhook_request_log/webhook_request_log.json +++ b/frappe/integrations/doctype/webhook_request_log/webhook_request_log.json @@ -1,6 +1,6 @@ { "actions": [], - "autoname": "WEBHOOK-REQ-.#####", + "autoname": "hash", "creation": "2021-05-24 21:35:59.104776", "doctype": "DocType", "editable_grid": 1, @@ -56,10 +56,11 @@ "in_create": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-05-26 23:57:58.495261", + "modified": "2022-05-03 09:33:49.240777", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook Request Log", + "naming_rule": "Random", "owner": "Administrator", "permissions": [ { @@ -77,5 +78,6 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file From 73c87c0e29aff67feda6d19da9a391427ef8ca34 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 4 May 2022 11:46:12 +0530 Subject: [PATCH 08/13] fix: Hide progress bar for less than 2 slides --- frappe/public/js/frappe/ui/slides.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/public/js/frappe/ui/slides.js b/frappe/public/js/frappe/ui/slides.js index de24c1a4e6..100dd13610 100644 --- a/frappe/public/js/frappe/ui/slides.js +++ b/frappe/public/js/frappe/ui/slides.js @@ -309,6 +309,8 @@ frappe.ui.Slides = class Slides { // Can be called by a slide to update states this.$slide_progress.empty(); + if (this.slides.length <= 1) return + this.slides.map((slide, id) => { let $dot = $(`
From ed78a2374855e0d9b7c97415338291e167369212 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 May 2022 14:22:14 +0530 Subject: [PATCH 09/13] fix: handle dict data in deferred_insert vanilla dict doesn't have attrs, setattr fails --- frappe/deferred_insert.py | 2 +- frappe/tests/test_deferred_insert.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 frappe/tests/test_deferred_insert.py diff --git a/frappe/deferred_insert.py b/frappe/deferred_insert.py index 28c77002f8..08c8c79eaa 100644 --- a/frappe/deferred_insert.py +++ b/frappe/deferred_insert.py @@ -46,8 +46,8 @@ def save_to_db(): def insert_record(record: Union[Dict, "Document"], doctype: str): - setattr(record, "doctype", doctype) try: + record.update({"doctype": doctype}) frappe.get_doc(record).insert() except Exception as e: frappe.logger().error(f"Error while inserting deferred {doctype} record: {e}") diff --git a/frappe/tests/test_deferred_insert.py b/frappe/tests/test_deferred_insert.py new file mode 100644 index 0000000000..4f27bef4f0 --- /dev/null +++ b/frappe/tests/test_deferred_insert.py @@ -0,0 +1,12 @@ +import frappe +from frappe.deferred_insert import deferred_insert, save_to_db +from frappe.tests.utils import FrappeTestCase + + +class TestDeferredInsert(FrappeTestCase): + def test_deferred_insert(self): + route_history = {"route": frappe.generate_hash(), "user": "Administrator"} + deferred_insert("Route History", [route_history]) + + save_to_db() + self.assertTrue(frappe.db.exists("Route History", route_history)) From 05f9201e07d5660acce117e155b134b5084cd5d5 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 May 2022 14:57:04 +0530 Subject: [PATCH 10/13] fix: dont manually commit after flushing deferred_insert this runs from scheduled job which commits after finishing. --- frappe/deferred_insert.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/deferred_insert.py b/frappe/deferred_insert.py index 08c8c79eaa..3b47d46cdf 100644 --- a/frappe/deferred_insert.py +++ b/frappe/deferred_insert.py @@ -42,8 +42,6 @@ def save_to_db(): record_count += 1 insert_record(record, doctype) - frappe.db.commit() - def insert_record(record: Union[Dict, "Document"], doctype: str): try: From a33c2e2abe8dd5fa8a8b87b334cf90832db2e717 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 4 May 2022 18:37:06 +0530 Subject: [PATCH 11/13] refactor(BaseDocument)!: improved `get`, `set` and `extend` methods (#16540) * perf!: 80% faster doc.get for fields with `None` as value * perf: quicker init child (#3) * refactor: avoid repitition and improve error message * test: `doc.extend` * fix: improve constant naming * fix: minor improvements and tests * refactor: improve naming --- frappe/core/doctype/doctype/doctype.py | 11 +- frappe/desk/form/meta.py | 6 - frappe/model/base_document.py | 173 +++++++++++++++---------- frappe/model/document.py | 34 ++--- frappe/model/meta.py | 28 ++-- frappe/tests/test_base_document.py | 2 +- frappe/tests/test_document.py | 16 +++ 7 files changed, 150 insertions(+), 120 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 06ebcc7d42..6bd7f2306f 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1018,12 +1018,13 @@ def validate_fields(meta): validate_column_name(fieldname) def check_invalid_fieldnames(docname, fieldname): - invalid_fields = ("doctype",) - if fieldname in invalid_fields: + if fieldname in Document._reserved_keywords: frappe.throw( - _("{0}: Fieldname cannot be one of {1}").format( - docname, ", ".join(frappe.bold(d) for d in invalid_fields) - ) + _("{0}: fieldname cannot be set to reserved keyword {1}").format( + frappe.bold(docname), + frappe.bold(fieldname), + ), + title=_("Invalid Fieldname"), ) def check_unique_fieldname(docname, fieldname): diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index f5edd3fcc6..dc26dfe5b0 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -55,12 +55,6 @@ class FormMeta(Meta): super(FormMeta, self).__init__(doctype) self.load_assets() - def set(self, key, value, *args, **kwargs): - if key in ASSET_KEYS: - self.__dict__[key] = value - else: - super(FormMeta, self).set(key, value, *args, **kwargs) - def load_assets(self): if self.get("__assets_loaded", False): return diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index a272dedd02..22f8cc15df 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -5,7 +5,7 @@ import json from typing import Dict, List import frappe -from frappe import _ +from frappe import _, _dict from frappe.model import ( child_table_fields, datetime_fields, @@ -23,7 +23,16 @@ from frappe.utils.html_utils import unescape_html max_positive_value = {"smallint": 2**15, "int": 2**31, "bigint": 2**63} -DOCTYPES_FOR_DOCTYPE = ("DocType", "DocField", "DocPerm", "DocType Action", "DocType Link") +DOCTYPE_TABLE_FIELDS = [ + _dict(fieldname="fields", options="DocField"), + _dict(fieldname="permissions", options="DocPerm"), + _dict(fieldname="actions", options="DocType Action"), + _dict(fieldname="links", options="DocType Link"), + _dict(fieldname="states", options="DocType State"), +] + +TABLE_DOCTYPES_FOR_DOCTYPE = {df["fieldname"]: df["options"] for df in DOCTYPE_TABLE_FIELDS} +DOCTYPES_FOR_DOCTYPE = {"DocType", *TABLE_DOCTYPES_FOR_DOCTYPE.values()} def get_controller(doctype): @@ -78,12 +87,28 @@ def get_controller(doctype): class BaseDocument(object): - ignore_in_setter = ("doctype", "_meta", "meta", "_table_fields", "_valid_columns") + _reserved_keywords = { + "doctype", + "meta", + "_meta", + "flags", + "_table_fields", + "_valid_columns", + "_table_fieldnames", + "_reserved_keywords", + "dont_update_if_missing", + } def __init__(self, d): if d.get("doctype"): self.doctype = d["doctype"] + self._table_fieldnames = ( + d["_table_fieldnames"] # from cache + if "_table_fieldnames" in d + else {df.fieldname for df in self._get_table_fields()} + ) + self.update(d) self.dont_update_if_missing = [] @@ -92,10 +117,10 @@ class BaseDocument(object): @property def meta(self): - if not getattr(self, "_meta", None): - self._meta = frappe.get_meta(self.doctype) + if not (meta := getattr(self, "_meta", None)): + self._meta = meta = frappe.get_meta(self.doctype) - return self._meta + return meta def __getstate__(self): self._meta = None @@ -144,17 +169,12 @@ class BaseDocument(object): if filters: if isinstance(filters, dict): - value = _filter(self.__dict__.get(key, []), filters, limit=limit) - else: - default = filters - filters = None - value = self.__dict__.get(key, default) - else: - value = self.__dict__.get(key, default) + return _filter(self.__dict__.get(key, []), filters, limit=limit) - if value is None and key in (d.fieldname for d in self.meta.get_table_fields()): - value = [] - self.set(key, value) + # perhaps you wanted to set a default instead + default = filters + + value = self.__dict__.get(key, default) if limit and isinstance(value, (list, tuple)) and len(value) > limit: value = value[:limit] @@ -165,14 +185,19 @@ class BaseDocument(object): return self.get(key, filters=filters, limit=1)[0] def set(self, key, value, as_value=False): - if key in self.ignore_in_setter: + if key in self._reserved_keywords: return - if isinstance(value, list) and not as_value: + if not as_value and key in self._table_fieldnames: self.__dict__[key] = [] - self.extend(key, value) - else: - self.__dict__[key] = value + + # if value is falsy, just init to an empty list + if value: + self.extend(key, value) + + return + + self.__dict__[key] = value def delete_key(self, key): if key in self.__dict__: @@ -190,41 +215,27 @@ class BaseDocument(object): """ if value is None: value = {} - if isinstance(value, (dict, BaseDocument)): - if not self.__dict__.get(key): - self.__dict__[key] = [] - value = self._init_child(value, key) - self.__dict__[key].append(value) + if (table := self.__dict__.get(key)) is None: + self.__dict__[key] = table = [] - # reference parent document - value.parent_doc = self + value = self._init_child(value, key) + table.append(value) - return value - else: + # reference parent document + value.parent_doc = self - # metaclasses may have arbitrary lists - # which we can ignore - if getattr(self, "_metaclass", None) or self.__class__.__name__ in ( - "Meta", - "FormMeta", - "DocField", - ): - return value - - raise ValueError( - 'Document for field "{0}" attached to child table of "{1}" must be a dict or BaseDocument, not {2} ({3})'.format( - key, self.name, str(type(value))[1:-1], value - ) - ) + return value def extend(self, key, value): - if isinstance(value, list): - for v in value: - self.append(key, v) - else: + try: + value = iter(value) + except TypeError: raise ValueError + for v in value: + self.append(key, v) + def remove(self, doc): # Usage: from the parent doc, pass the child table doc # to remove that child doc from the child table, thus removing it from the parent doc @@ -232,15 +243,12 @@ class BaseDocument(object): self.get(doc.parentfield).remove(doc) def _init_child(self, value, key): - if not self.doctype: - return value - if not isinstance(value, BaseDocument): - value["doctype"] = self.get_table_field_doctype(key) - if not value["doctype"]: + if not (doctype := self.get_table_field_doctype(key)): raise AttributeError(key) - value = get_controller(value["doctype"])(value) + value["doctype"] = doctype + value = get_controller(doctype)(value) value.parent = self.name value.parenttype = self.doctype @@ -250,17 +258,35 @@ class BaseDocument(object): value.docstatus = DocStatus.draft() if not getattr(value, "idx", None): - value.idx = len(self.get(key) or []) + 1 + if table := getattr(self, key, None): + value.idx = len(table) + 1 + else: + value.idx = 1 if not getattr(value, "name", None): value.__dict__["__islocal"] = 1 return value + def _get_table_fields(self): + """ + To get table fields during Document init + Meta.get_table_fields goes into recursion for special doctypes + """ + + if self.doctype == "DocType": + return DOCTYPE_TABLE_FIELDS + + # child tables don't have child tables + if self.doctype in DOCTYPES_FOR_DOCTYPE or getattr(self, "parentfield", None): + return () + + return self.meta.get_table_fields() + def get_valid_dict( self, sanitize=True, convert_dates_to_str=False, ignore_nulls=False, ignore_virtual=False ) -> Dict: - d = frappe._dict() + d = _dict() for fieldname in self.meta.get_valid_columns(): # column is valid, we can use getattr d[fieldname] = getattr(self, fieldname, None) @@ -316,6 +342,16 @@ class BaseDocument(object): return d + def init_child_tables(self): + """ + This is needed so that one can loop over child table properties + without worrying about whether or not they have values + """ + + for fieldname in self._table_fieldnames: + if self.__dict__.get(fieldname) is None: + self.__dict__[fieldname] = [] + def init_valid_columns(self): for key in default_fields: if key not in self.__dict__: @@ -365,9 +401,9 @@ class BaseDocument(object): doc = self.get_valid_dict(convert_dates_to_str=convert_dates_to_str, ignore_nulls=no_nulls) doc["doctype"] = self.doctype - for df in self.meta.get_table_fields(): - children = self.get(df.fieldname) or [] - doc[df.fieldname] = [ + for fieldname in self._table_fieldnames: + children = self.get(fieldname) or [] + doc[fieldname] = [ d.as_dict( convert_dates_to_str=convert_dates_to_str, no_nulls=no_nulls, @@ -407,10 +443,9 @@ class BaseDocument(object): try: return self.meta.get_field(fieldname).options except AttributeError: - if self.doctype == "DocType": - return dict(links="DocType Link", actions="DocType Action", states="DocType State").get( - fieldname - ) + if self.doctype == "DocType" and (table_doctype := TABLE_DOCTYPES_FOR_DOCTYPE.get(fieldname)): + return table_doctype + raise def get_parentfield_of_doctype(self, doctype): @@ -519,8 +554,8 @@ class BaseDocument(object): """Raw update parent + children DOES NOT VALIDATE AND CALL TRIGGERS""" self.db_update() - for df in self.meta.get_table_fields(): - for doc in self.get(df.fieldname): + for fieldname in self._table_fieldnames: + for doc in self.get(fieldname): doc.db_update() def show_unique_validation_message(self, e): @@ -632,7 +667,7 @@ class BaseDocument(object): if self.meta.istable: for fieldname in ("parent", "parenttype"): if not self.get(fieldname): - missing.append((fieldname, get_msg(frappe._dict(label=fieldname)))) + missing.append((fieldname, get_msg(_dict(label=fieldname)))) return missing @@ -679,7 +714,7 @@ class BaseDocument(object): if not frappe.get_meta(doctype).get("is_virtual"): if not fields_to_fetch: # cache a single value type - values = frappe._dict(name=frappe.db.get_value(doctype, docname, "name", cache=True)) + values = _dict(name=frappe.db.get_value(doctype, docname, "name", cache=True)) else: values_to_fetch = ["name"] + [_df.fetch_from.split(".")[-1] for _df in fields_to_fetch] @@ -1009,10 +1044,10 @@ class BaseDocument(object): cache_key = parentfield or "main" if not hasattr(self, "_precision"): - self._precision = frappe._dict() + self._precision = _dict() if cache_key not in self._precision: - self._precision[cache_key] = frappe._dict() + self._precision[cache_key] = _dict() if fieldname not in self._precision[cache_key]: self._precision[cache_key][fieldname] = None diff --git a/frappe/model/document.py b/frappe/model/document.py index c5e61563f8..f59fb81350 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -113,6 +113,7 @@ class Document(BaseDocument): if kwargs: # init base document super(Document, self).__init__(kwargs) + self.init_child_tables() self.init_valid_columns() else: @@ -150,25 +151,19 @@ class Document(BaseDocument): super(Document, self).__init__(d) - if self.name == "DocType" and self.doctype == "DocType": - from frappe.model.meta import DOCTYPE_TABLE_FIELDS - - table_fields = DOCTYPE_TABLE_FIELDS - else: - table_fields = self.meta.get_table_fields() - - for df in table_fields: - children = frappe.db.get_values( - df.options, - {"parent": self.name, "parenttype": self.doctype, "parentfield": df.fieldname}, - "*", - as_dict=True, - order_by="idx asc", + for df in self._get_table_fields(): + children = ( + frappe.db.get_values( + df.options, + {"parent": self.name, "parenttype": self.doctype, "parentfield": df.fieldname}, + "*", + as_dict=True, + order_by="idx asc", + ) + or [] ) - if children: - self.set(df.fieldname, children) - else: - self.set(df.fieldname, []) + + self.set(df.fieldname, children) # sometimes __setup__ can depend on child values, hence calling again at the end if hasattr(self, "__setup__"): @@ -897,8 +892,7 @@ class Document(BaseDocument): if parenttype and df.options != parenttype: continue - value = self.get(df.fieldname) - if isinstance(value, list): + if value := self.get(df.fieldname): children.extend(value) return children diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 232d0a8d58..aeb12136ef 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -30,7 +30,11 @@ from frappe.model import ( optional_fields, table_fields, ) -from frappe.model.base_document import BaseDocument +from frappe.model.base_document import ( + DOCTYPE_TABLE_FIELDS, + TABLE_DOCTYPES_FOR_DOCTYPE, + BaseDocument, +) from frappe.model.document import Document from frappe.model.workflow import get_workflow_name from frappe.modules import load_doctype_module @@ -148,9 +152,9 @@ class Meta(Document): out[key] = value # set empty lists for unset table fields - for table_field in DOCTYPE_TABLE_FIELDS: - if out.get(table_field.fieldname) is None: - out[table_field.fieldname] = [] + for fieldname in TABLE_DOCTYPES_FOR_DOCTYPE.keys(): + if out.get(fieldname) is None: + out[fieldname] = [] return out @@ -225,13 +229,7 @@ class Meta(Document): return self._valid_columns def get_table_field_doctype(self, fieldname): - return { - "fields": "DocField", - "permissions": "DocPerm", - "actions": "DocType Action", - "links": "DocType Link", - "states": "DocType State", - }.get(fieldname) + return TABLE_DOCTYPES_FOR_DOCTYPE.get(fieldname) def get_field(self, fieldname): """Return docfield from meta""" @@ -642,14 +640,6 @@ class Meta(Document): return self.has_field("lft") and self.has_field("rgt") -DOCTYPE_TABLE_FIELDS = [ - frappe._dict({"fieldname": "fields", "options": "DocField"}), - frappe._dict({"fieldname": "permissions", "options": "DocPerm"}), - frappe._dict({"fieldname": "actions", "options": "DocType Action"}), - frappe._dict({"fieldname": "links", "options": "DocType Link"}), - frappe._dict({"fieldname": "states", "options": "DocType State"}), -] - ####### diff --git a/frappe/tests/test_base_document.py b/frappe/tests/test_base_document.py index fda795b5b6..a861b3454b 100644 --- a/frappe/tests/test_base_document.py +++ b/frappe/tests/test_base_document.py @@ -5,7 +5,7 @@ from frappe.model.base_document import BaseDocument class TestBaseDocument(unittest.TestCase): def test_docstatus(self): - doc = BaseDocument({"docstatus": 0}) + doc = BaseDocument({"docstatus": 0, "doctype": "ToDo"}) self.assertTrue(doc.docstatus.is_draft()) self.assertEqual(doc.docstatus, 0) diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index 40bc8e2d45..5bda6a1d9d 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -341,3 +341,19 @@ class TestDocument(unittest.TestCase): # run_method should get overridden self.assertEqual(doc.run_method("as_dict"), "success") + + def test_extend(self): + doc = frappe.get_last_doc("User") + self.assertRaises(ValueError, doc.extend, "user_emails", None) + + # allow calling doc.extend with iterable objects + doc.extend("user_emails", ()) + doc.extend("user_emails", []) + doc.extend("user_emails", (x for x in ())) + + def test_set(self): + doc = frappe.get_last_doc("User") + + # setting None should init a table field to empty list + doc.set("user_emails", None) + self.assertEqual(doc.user_emails, []) From b4e43257c3e0e15bd171e080711927a8724b111e Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Wed, 4 May 2022 19:07:51 +0530 Subject: [PATCH 12/13] fix: bad query if user has ' in the email address (#16796) --- frappe/core/doctype/user/test_records.json | 7 ++++ .../dashboard_settings/dashboard_settings.py | 2 +- .../desk/doctype/kanban_board/kanban_board.py | 4 +- frappe/desk/doctype/note/note.py | 2 +- .../notification_log/notification_log.py | 2 +- .../notification_settings.py | 2 +- .../desk/doctype/number_card/number_card.py | 40 ++++++------------- frappe/tests/test_db_query.py | 23 +++++++++++ .../workflow_action/workflow_action.py | 9 +++-- 9 files changed, 55 insertions(+), 36 deletions(-) diff --git a/frappe/core/doctype/user/test_records.json b/frappe/core/doctype/user/test_records.json index 21fe3ff69d..9d1bf0efd4 100644 --- a/frappe/core/doctype/user/test_records.json +++ b/frappe/core/doctype/user/test_records.json @@ -45,6 +45,13 @@ "new_password": "Eastern_43A1W", "enabled": 1 }, + { + "doctype": "User", + "email": "test'5@example.com", + "first_name": "_Test'5", + "new_password": "Eastern_43A1W", + "enabled": 1 + }, { "doctype": "User", "email": "testperm@example.com", diff --git a/frappe/desk/doctype/dashboard_settings/dashboard_settings.py b/frappe/desk/doctype/dashboard_settings/dashboard_settings.py index d43476ad7d..01c3a87f20 100644 --- a/frappe/desk/doctype/dashboard_settings/dashboard_settings.py +++ b/frappe/desk/doctype/dashboard_settings/dashboard_settings.py @@ -28,7 +28,7 @@ def get_permission_query_conditions(user): if not user: user = frappe.session.user - return """(`tabDashboard Settings`.name = '{user}')""".format(user=user) + return """(`tabDashboard Settings`.name = {user})""".format(user=frappe.db.escape(user)) @frappe.whitelist() diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index 381f71438c..bc47bbbaf4 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -34,7 +34,9 @@ def get_permission_query_conditions(user): if user == "Administrator": return "" - return """(`tabKanban Board`.private=0 or `tabKanban Board`.owner='{user}')""".format(user=user) + return """(`tabKanban Board`.private=0 or `tabKanban Board`.owner={user})""".format( + user=frappe.db.escape(user) + ) def has_permission(doc, ptype, user): diff --git a/frappe/desk/doctype/note/note.py b/frappe/desk/doctype/note/note.py index de019d9898..d67ecda594 100644 --- a/frappe/desk/doctype/note/note.py +++ b/frappe/desk/doctype/note/note.py @@ -38,7 +38,7 @@ def get_permission_query_conditions(user): if user == "Administrator": return "" - return """(`tabNote`.public=1 or `tabNote`.owner="{user}")""".format(user=user) + return """(`tabNote`.public=1 or `tabNote`.owner={user})""".format(user=frappe.db.escape(user)) def has_permission(doc, ptype, user): diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index 011f3e22ff..9e7ed7d9fe 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -30,7 +30,7 @@ def get_permission_query_conditions(for_user): if for_user == "Administrator": return - return """(`tabNotification Log`.for_user = '{user}')""".format(user=for_user) + return """(`tabNotification Log`.for_user = {user})""".format(user=frappe.db.escape(for_user)) def get_title(doctype, docname, title_field=None): diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index bbb4a62154..ee425e154b 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -81,7 +81,7 @@ def get_permission_query_conditions(user): if "System Manager" in roles: return """(`tabNotification Settings`.name != 'Administrator')""" - return """(`tabNotification Settings`.name = '{user}')""".format(user=user) + return """(`tabNotification Settings`.name = {user})""".format(user=frappe.db.escape(user)) @frappe.whitelist() diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index e1b2b19026..d6d4f00b69 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -8,6 +8,8 @@ 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 from frappe.modules.export_file import export_to_files +from frappe.query_builder import Criterion +from frappe.query_builder.utils import DocType from frappe.utils import cint @@ -190,36 +192,18 @@ def get_cards_for_user(doctype, txt, searchfield, start, page_len, filters): if not frappe.db.exists("DocType", doctype): return + numberCard = DocType("Number Card") + if txt: - for field in searchfields: - search_conditions.append( - "`tab{doctype}`.`{field}` like %(txt)s".format(field=field, doctype=doctype, txt=txt) - ) + search_conditions = [numberCard[field].like("%{txt}%".format(txt=txt)) for field in searchfields] - search_conditions = " or ".join(search_conditions) - - search_conditions = "and (" + search_conditions + ")" if search_conditions else "" - conditions, values = frappe.db.build_conditions(filters) - values["txt"] = "%" + txt + "%" - - return frappe.db.sql( - """select - `tabNumber Card`.name, `tabNumber Card`.label, `tabNumber Card`.document_type - from - `tabNumber Card` - where - {conditions} and - (`tabNumber Card`.owner = '{user}' or - `tabNumber Card`.is_public = 1) - {search_conditions} - """.format( - filters=filters, - user=frappe.session.user, - search_conditions=search_conditions, - conditions=conditions, - ), - values, - ) + condition_query = frappe.db.query.build_conditions(doctype, filters) + + return ( + condition_query.select(numberCard.name, numberCard.label, numberCard.document_type) + .where((numberCard.owner == frappe.session.user) | (numberCard.is_public == 1)) + .where(Criterion.any(search_conditions)) + ).run() @frappe.whitelist() diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 19a8c445f8..dd67d68cd2 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -692,6 +692,29 @@ class TestReportview(unittest.TestCase): dt.delete() table_dt.delete() + def test_permission_query_condition(self): + from frappe.desk.doctype.dashboard_settings.dashboard_settings import create_dashboard_settings + + self.doctype = "Dashboard Settings" + self.user = "test'5@example.com" + + permission_query_conditions = DatabaseQuery.get_permission_query_conditions(self) + + create_dashboard_settings(self.user) + + dashboard_settings = frappe.db.sql( + """ + SELECT name + FROM `tabDashboard Settings` + WHERE {condition} + """.format( + condition=permission_query_conditions + ), + as_dict=1, + )[0] + + self.assertTrue(dashboard_settings) + def add_child_table_to_blog_post(): child_table = frappe.get_doc( diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index abbcda5c4c..7b54df2d51 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -53,9 +53,12 @@ def get_permission_query_conditions(user): .where(WorkflowActionPermittedRole.role.isin(roles)) ).get_sql() - return f"""(`tabWorkflow Action`.`name` in ({permitted_workflow_actions}) - or `tabWorkflow Action`.`user`='{user}') - and `tabWorkflow Action`.`status`='Open'""" + return """(`tabWorkflow Action`.`name` in ({permitted_workflow_actions}) + or `tabWorkflow Action`.`user`={user}) + and `tabWorkflow Action`.`status`='Open' + """.format( + permitted_workflow_actions=permitted_workflow_actions, user=frappe.db.escape(user) + ) def has_permission(doc, user): From c31eca3ba589164d21174be2d578d66221007cdc Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 May 2022 19:09:53 +0530 Subject: [PATCH 13/13] fix: circular imports (#16830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Circular imports issue when loading modules in background worker. Doesn't happen in web worker or console 🤷 --- frappe/model/rename_doc.py | 45 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index a0cd10f967..25e471d4b0 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -9,7 +9,6 @@ from frappe.model.dynamic_links import get_dynamic_link_map from frappe.model.naming import validate_name from frappe.model.utils.user_settings import sync_user_settings, update_user_settings_data from frappe.query_builder import Field -from frappe.query_builder.utils import DocType, Table from frappe.utils.data import sbool from frappe.utils.password import rename_password from frappe.utils.scheduler import is_scheduler_inactive @@ -198,7 +197,7 @@ def rename_doc( rename_password(doctype, old, new) # update user_permissions - DefaultValue = DocType("DefaultValue") + DefaultValue = frappe.qb.DocType("DefaultValue") frappe.qb.update(DefaultValue).set(DefaultValue.defvalue, new).where( (DefaultValue.parenttype == "User Permission") & (DefaultValue.defkey == doctype) @@ -267,7 +266,7 @@ def update_user_settings(old: str, new: str, link_fields: List[Dict]) -> None: # find the user settings for the linked doctypes linked_doctypes = {d.parent for d in link_fields if not d.issingle} - UserSettings = Table("__UserSettings") + UserSettings = frappe.qb.Table("__UserSettings") user_settings_details = ( frappe.qb.from_(UserSettings) @@ -299,7 +298,7 @@ def update_customizations(old: str, new: str) -> None: def update_attachments(doctype: str, old: str, new: str) -> None: if doctype != "DocType": - File = DocType("File") + File = frappe.qb.DocType("File") frappe.qb.update(File).set(File.attached_to_name, new).where( (File.attached_to_name == old) & (File.attached_to_doctype == doctype) @@ -307,7 +306,7 @@ def update_attachments(doctype: str, old: str, new: str) -> None: def rename_versions(doctype: str, old: str, new: str) -> None: - Version = DocType("Version") + Version = frappe.qb.DocType("Version") frappe.qb.update(Version).set(Version.docname, new).where( (Version.docname == old) & (Version.ref_doctype == doctype) @@ -315,7 +314,7 @@ def rename_versions(doctype: str, old: str, new: str) -> None: def rename_eps_records(doctype: str, old: str, new: str) -> None: - EPL = DocType("Energy Point Log") + EPL = frappe.qb.DocType("Energy Point Log") frappe.qb.update(EPL).set(EPL.reference_name, new).where( (EPL.reference_doctype == doctype) & (EPL.reference_name == old) @@ -455,10 +454,10 @@ def get_link_fields(doctype: str) -> List[Dict]: frappe.flags.link_fields = {} if doctype not in frappe.flags.link_fields: - dt = DocType("DocType") - df = DocType("DocField") - cf = DocType("Custom Field") - ps = DocType("Property Setter") + dt = frappe.qb.DocType("DocType") + df = frappe.qb.DocType("DocField") + cf = frappe.qb.DocType("Custom Field") + ps = frappe.qb.DocType("Property Setter") st_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == df.parent).as_("issingle") standard_fields = ( @@ -492,8 +491,8 @@ def get_link_fields(doctype: str) -> List[Dict]: def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: - CustomField = DocType("Custom Field") - PropertySetter = DocType("Property Setter") + CustomField = frappe.qb.DocType("Custom Field") + PropertySetter = frappe.qb.DocType("Property Setter") if frappe.conf.developer_mode: for name in frappe.get_all("DocField", filters={"options": old}, pluck="parent"): @@ -506,7 +505,7 @@ def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: if save: doctype.save() else: - DocField = DocType("DocField") + DocField = frappe.qb.DocType("DocField") frappe.qb.update(DocField).set(DocField.options, new).where( (DocField.fieldtype == fieldtype) & (DocField.options == old) ).run() @@ -525,10 +524,10 @@ def get_select_fields(old: str, new: str) -> List[Dict]: get select type fields where doctype's name is hardcoded as new line separated list """ - df = DocType("DocField") - dt = DocType("DocType") - cf = DocType("Custom Field") - ps = DocType("Property Setter") + df = frappe.qb.DocType("DocField") + dt = frappe.qb.DocType("DocType") + cf = frappe.qb.DocType("Custom Field") + ps = frappe.qb.DocType("Property Setter") # get link fields from tabDocField st_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == df.parent).as_("issingle") @@ -570,9 +569,9 @@ def get_select_fields(old: str, new: str) -> List[Dict]: def update_select_field_values(old: str, new: str): from frappe.query_builder.functions import Replace - DocField = DocType("DocField") - CustomField = DocType("Custom Field") - PropertySetter = DocType("Property Setter") + DocField = frappe.qb.DocType("DocField") + CustomField = frappe.qb.DocType("Custom Field") + PropertySetter = frappe.qb.DocType("Property Setter") frappe.qb.update(DocField).set(DocField.options, Replace(DocField.options, old, new)).where( (DocField.fieldtype == "Select") @@ -623,12 +622,12 @@ def update_parenttype_values(old: str, new: str): child_doctypes = set(list(d["options"] for d in child_doctypes) + property_setter_child_doctypes) for doctype in child_doctypes: - Table = DocType(doctype) - frappe.qb.update(Table).set(Table.parenttype, new).where(Table.parenttype == old).run() + table = frappe.qb.DocType(doctype) + frappe.qb.update(table).set(table.parenttype, new).where(table.parenttype == old).run() def rename_dynamic_links(doctype: str, old: str, new: str): - Singles = DocType("Singles") + Singles = frappe.qb.DocType("Singles") for df in get_dynamic_link_map().get(doctype, []): # dynamic link in single, just one value to check if frappe.get_meta(df.parent).issingle: