From 31440f0538a767b8ee72e0c7ede8db2c359ed532 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 13 Apr 2022 13:37:20 +0530 Subject: [PATCH 01/18] 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 | 47 ++- .../v14_0/update_integration_request.py | 17 + 8 files changed, 155 insertions(+), 290 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) +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 - if isinstance(error, str): - error = json.loads(error) + 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 6d96f11dd961d7f71cd54cae421793209c86f921 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 29 Apr 2022 14:38:35 +0530 Subject: [PATCH 02/18] fix: Use BlogPosting schema for Article Fixes https://github.com/frappe/frappe/issues/1730 --- frappe/website/doctype/help_article/templates/help_article.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/website/doctype/help_article/templates/help_article.html b/frappe/website/doctype/help_article/templates/help_article.html index 9ef9a398f5..105b63e651 100644 --- a/frappe/website/doctype/help_article/templates/help_article.html +++ b/frappe/website/doctype/help_article/templates/help_article.html @@ -5,7 +5,7 @@ {% endblock %} {% block page_content %} -
+
By {{ author }} on {{ frappe.format_date(creation) }}
{{ level }} From b41379c78ba527350b0118863448fed4ee11ba15 Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Fri, 29 Apr 2022 15:06:03 +0530 Subject: [PATCH 03/18] fix: misc fixes (integer primary keys) (#16307) * fix: misc fixes local.x gets resetted on every request so switched to a simple dict simplified is_val_used in set_next_val function for sequences * chore: use multisql for sequence methods * fix: fields not updating on form * minor(base_input): removed unnecessary branching in update_input * chore: remove prints and rename autoincremented_status_map * chore: added proper type hint + comment + formatting * fix: added searching in cast_name rather than handling it manually * fix: share condition query + test_build_match_conditions * fix: add cast_name to more places * test: test for sequence * fix: sequence functions * fix: inherit frappetestcase * minor: attach sequence methods to db context local * chore: update sequence function names in Database use frappe.db for sequences in naming.py * fix: convert filename to str (for autoincremented doctypes) * chore: better regex for modifying values for postgres * minor: allow changing name column type (if no data is present in the doctype) * refactor: validate_autoname converted it to a simple function enabled changing autoincrement autoname from customize form * fix: use sql_ddl for change_column_type in postgres * fix: use not null constraint in postgres when changing name type * fix(test): updated test_autoincremented_doctype_transition with transitioning when no data is present * fix(test): updated test_cast_name probably messed up during rebase * fix(test): used rollback upon error in transaction for postgres * chore: use frappe.db.x methods for sequences * minor: use temporary sequences in test * minor: use generate_hash for sequence naming in sequence tests * chore: replace sequence imports with frappe.db.x * chore: move out casting name fields to a separate method * refactor: cast_name more explicit cases for casts and added docstring * fix: added space in test_cast_name * chore: fix linter * chore: better naming for can_change_name_column_type * chore: add comment for autoincremented_site_status_map * chore: update/add docstrings --- frappe/__init__.py | 3 +- frappe/core/doctype/doctype/doctype.py | 70 +++++++++--- frappe/core/doctype/doctype/test_doctype.py | 21 +++- .../customize_form/customize_form.json | 4 +- .../doctype/customize_form/customize_form.py | 10 ++ frappe/database/database.py | 15 +++ frappe/database/mariadb/database.py | 2 +- frappe/database/mariadb/schema.py | 3 +- frappe/database/postgres/database.py | 12 ++- frappe/database/postgres/schema.py | 3 +- frappe/database/sequence.py | 53 ++++----- frappe/model/db_query.py | 101 +++++++++--------- frappe/model/naming.py | 37 +++---- .../js/frappe/form/controls/base_input.js | 6 +- frappe/public/js/frappe/form/form.js | 6 +- frappe/public/js/frappe/model/model.js | 2 +- frappe/tests/test_db_query.py | 17 +-- frappe/tests/test_sequence.py | 54 ++++++++++ 18 files changed, 279 insertions(+), 140 deletions(-) create mode 100644 frappe/tests/test_sequence.py diff --git a/frappe/__init__.py b/frappe/__init__.py index d54686304f..8bd7783283 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -232,7 +232,6 @@ def init(site, sites_path=None, new_site=False): local.cache = {} local.document_cache = {} local.meta_cache = {} - local.autoincremented_status_map = {site: -1} local.form_dict = _dict() local.session = _dict() local.dev_server = _dev_server @@ -1926,7 +1925,7 @@ def attach_print( if not file_name: file_name = name - file_name = file_name.replace(" ", "").replace("/", "-") + file_name = cstr(file_name).replace(" ", "").replace("/", "-") print_settings = db.get_singles_dict("Print Settings") diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 2bb971bac8..06ebcc7d42 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -92,10 +92,10 @@ class DocType(Document): self.check_developer_mode() - self.validate_autoname() self.validate_name() self.set_defaults_for_single_and_table() + self.set_defaults_for_autoincremented() self.scrub_field_names() self.set_default_in_list_view() self.set_default_translatable() @@ -124,6 +124,12 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) + if check_if_can_change_name_type(self): + change_name_column_type( + self.name, + "bigint" if self.autoname == "autoincrement" else f"varchar({frappe.db.VARCHAR_LEN})", + ) + def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -184,6 +190,10 @@ class DocType(Document): self.allow_import = 0 self.permissions = [] + def set_defaults_for_autoincremented(self): + if self.autoname and self.autoname == "autoincrement": + self.allow_rename = 0 + def set_default_in_list_view(self): """Set default in-list-view for first 4 mandatory fields""" if not [d.fieldname for d in self.fields if d.in_list_view]: @@ -809,17 +819,6 @@ class DocType(Document): max_idx = frappe.db.sql("""select max(idx) from `tabDocField` where parent = %s""", self.name) return max_idx and max_idx[0][0] or 0 - def validate_autoname(self): - if not self.is_new(): - doc_before_save = self.get_doc_before_save() - if doc_before_save: - if (self.autoname == "autoincrement" and doc_before_save.autoname != "autoincrement") or ( - self.autoname != "autoincrement" and doc_before_save.autoname == "autoincrement" - ): - frappe.throw(_("Cannot change to/from Autoincrement naming rule")) - if self.autoname == "autoincrement": - self.allow_rename = 0 - def validate_name(self, name=None): if not name: name = self.name @@ -887,7 +886,7 @@ def validate_series(dt, autoname=None, name=None): autoname and (not autoname.startswith("field:")) and (not autoname.startswith("eval:")) - and (not autoname.lower() in ("prompt", "hash")) + and (autoname.lower() not in ("prompt", "hash")) and (not autoname.startswith("naming_series:")) and (not autoname.startswith("format:")) ): @@ -904,6 +903,51 @@ def validate_series(dt, autoname=None, name=None): frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) +def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: + def get_autoname_before_save(doctype: str, to_be_customized_dt: str) -> str: + if doctype == "Customize Form": + property_value = frappe.db.get_value( + "Property Setter", {"doc_type": to_be_customized_dt, "property": "autoname"}, "value" + ) + + # initially no property setter is set, + # hence getting autoname value from the doctype itself + if not property_value: + return frappe.db.get_value("DocType", to_be_customized_dt, "autoname") or "" + + return property_value + + return getattr(dt.get_doc_before_save(), "autoname", "") + + doctype_name = dt.doc_type if dt.doctype == "Customize Form" else dt.name + + if not dt.is_new(): + autoname_before_save = get_autoname_before_save(dt.doctype, doctype_name) + is_autoname_autoincrement = dt.autoname == "autoincrement" + + if ( + is_autoname_autoincrement + and autoname_before_save != "autoincrement" + or (not is_autoname_autoincrement and autoname_before_save == "autoincrement") + ): + if not frappe.get_all(doctype_name, limit=1): + # allow changing the column type if there is no data + return True + + if raise_err: + frappe.throw( + _("Can only change to/from Autoincrement naming rule when there is no data in the doctype") + ) + + return False + + +def change_name_column_type(doctype_name: str, type: str) -> None: + return frappe.db.change_column_type( + doctype_name, "name", type, True if frappe.db.db_type == "mariadb" else False + ) + + def validate_links_table_fieldnames(meta): """Validate fieldnames in Links table""" if not meta.links or frappe.flags.in_patch or frappe.flags.in_fixtures: diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 7b4806da59..59475a95a7 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -524,18 +524,33 @@ class TestDocType(unittest.TestCase): dt.delete() def test_autoincremented_doctype_transition(self): - frappe.delete_doc("testy_autoinc_dt") + frappe.delete_doc_if_exists("DocType", "testy_autoinc_dt") dt = new_doctype("testy_autoinc_dt", autoname="autoincrement").insert(ignore_permissions=True) dt.autoname = "hash" + dt.save(ignore_permissions=True) + + dt_data = frappe.get_doc({"doctype": dt.name, "some_fieldname": "test data"}).insert( + ignore_permissions=True + ) + + dt.autoname = "autoincrement" + try: dt.save(ignore_permissions=True) except frappe.ValidationError as e: - self.assertEqual(e.args[0], "Cannot change to/from Autoincrement naming rule") + self.assertEqual( + e.args[0], + "Can only change to/from Autoincrement naming rule when there is no data in the doctype", + ) else: - self.fail("Shouldnt be possible to transition autoincremented doctype to any other naming rule") + self.fail( + """Shouldn't be possible to transition to/from autoincremented doctype + when data is present in doctype""" + ) finally: # cleanup + dt_data.delete(ignore_permissions=True) dt.delete(ignore_permissions=True) def test_json_field(self): diff --git a/frappe/custom/doctype/customize_form/customize_form.json b/frappe/custom/doctype/customize_form/customize_form.json index 6563cae94c..a0bc994c45 100644 --- a/frappe/custom/doctype/customize_form/customize_form.json +++ b/frappe/custom/doctype/customize_form/customize_form.json @@ -287,7 +287,7 @@ "label": "Naming" }, { - "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. naming_series: - By Naming Series (field called naming_series must be present)
  3. Prompt - Prompt user for a name
  4. [series] - Series by prefix (separated by a dot); for example PRE.#####
    5. \n
  5. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", + "description": "Naming Options:\n
  1. field:[fieldname] - By Field
    2. \n
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", "fieldname": "autoname", "fieldtype": "Data", "label": "Auto Name" @@ -319,7 +319,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-01-07 16:07:06.196534", + "modified": "2022-04-21 15:36:16.772277", "modified_by": "Administrator", "module": "Custom", "name": "Customize Form", diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index e7187db0d6..262542fd4b 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -11,7 +11,9 @@ import frappe import frappe.translate from frappe import _ from frappe.core.doctype.doctype.doctype import ( + change_name_column_type, check_email_append_to, + check_if_can_change_name_type, validate_fields_for_doctype, validate_series, ) @@ -159,7 +161,9 @@ class CustomizeForm(Document): def save_customization(self): if not self.doc_type: return + validate_series(self, self.autoname, self.doc_type) + can_change_name_type = check_if_can_change_name_type(self) self.flags.update_db = False self.flags.rebuild_doctype_for_global_search = False self.set_property_setters() @@ -168,6 +172,12 @@ class CustomizeForm(Document): validate_fields_for_doctype(self.doc_type) check_email_append_to(self) + if can_change_name_type: + change_name_column_type( + self.doc_type, + "bigint" if self.autoname == "autoincrement" else f"varchar({frappe.db.VARCHAR_LEN})", + ) + if self.flags.update_db: frappe.db.updatedb(self.doc_type) diff --git a/frappe/database/database.py b/frappe/database/database.py index fb631951fa..ca4b5a5310 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1246,6 +1246,21 @@ class Database(object): values_to_insert = values[start_index : start_index + chunk_size] query.columns(fields).insert(*values_to_insert).run() + def create_sequence(self, *args, **kwargs): + from frappe.database.sequence import create_sequence + + return create_sequence(*args, **kwargs) + + def set_next_sequence_val(self, *args, **kwargs): + from frappe.database.sequence import set_next_val + + set_next_val(*args, **kwargs) + + def get_next_sequence_val(self, *args, **kwargs): + from frappe.database.sequence import get_next_val + + return get_next_val(*args, **kwargs) + def enqueue_jobs_after_commit(): from frappe.utils.background_jobs import execute_job, get_queue diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 0f5410a403..28d78471d2 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -149,7 +149,7 @@ class MariaDBDatabase(Database): ) -> Union[List, Tuple]: table_name = get_table_name(doctype) null_constraint = "NOT NULL" if not nullable else "" - return self.sql(f"ALTER TABLE `{table_name}` MODIFY `{column}` {type} {null_constraint}") + return self.sql_ddl(f"ALTER TABLE `{table_name}` MODIFY `{column}` {type} {null_constraint}") # exception types @staticmethod diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 7c95e9ffcb..784fa23c13 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -1,7 +1,6 @@ import frappe from frappe import _ from frappe.database.schema import DBTable -from frappe.database.sequence import create_sequence from frappe.model import log_types @@ -48,7 +47,7 @@ class MariaDBTable(DBTable): # By default the cache is 1000 which will mess up the sequence when # using the system after a restore. # issue link: https://jira.mariadb.org/browse/MDEV-21786 - create_sequence(self.doctype, check_not_exists=True, cache=50) + frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=50) # NOTE: not used nextval func as default as the ability to restore # database with sequences has bugs in mariadb and gives a scary error. diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 4cd6ab9873..d69e0bea94 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -213,7 +213,11 @@ class PostgresDatabase(Database): ) -> Union[List, Tuple]: table_name = get_table_name(doctype) null_constraint = "SET NOT NULL" if not nullable else "DROP NOT NULL" - return self.sql( + + # postgres allows ddl in transactions but since we've currently made + # things same as mariadb (raising exception on ddl commands if the transaction has any writes), + # hence using sql_ddl here for committing and then moving forward. + return self.sql_ddl( f"""ALTER TABLE "{table_name}" ALTER COLUMN "{column}" TYPE {type}, ALTER COLUMN "{column}" {null_constraint}""" @@ -382,12 +386,10 @@ def modify_query(query): # drop .0 from decimals and add quotes around them # # >>> query = "c='abcd' , a >= 45, b = -45.0, c = 40, d=4500.0, e=3500.53, f=40psdfsd, g=9092094312, h=12.00023" - # >>> re.sub(r"([=><]+)\s*(?!\d+[a-zA-Z])(?![+-]?\d+\.\d\d+)([+-]?\d+)(\.0)?", r"\1 '\2'", query) + # >>> re.sub(r"([=><]+)\s*([+-]?\d+)(\.0)?(?![a-zA-Z\.\d])", r"\1 '\2'", query) # "c='abcd' , a >= '45', b = '-45', c = '40', d= '4500', e=3500.53, f=40psdfsd, g= '9092094312', h=12.00023 - query = re.sub( - r"([=><]+)\s*(?!\d+[a-zA-Z])(?![+-]?\d+\.\d\d+)([+-]?\d+)(\.0)?", r"\1 '\2'", query - ) + query = re.sub(r"([=><]+)\s*([+-]?\d+)(\.0)?(?![a-zA-Z\.\d])", r"\1 '\2'", query) return query diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index 3432c8b548..2abd5f37c7 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -1,7 +1,6 @@ import frappe from frappe import _ from frappe.database.schema import DBTable, get_definition -from frappe.database.sequence import create_sequence from frappe.model import log_types from frappe.utils import cint, flt @@ -39,7 +38,7 @@ class PostgresTable(DBTable): # Since we're opening and closing connections for every transaction this results in skipping the cache # to the next non-cached value hence not using cache in postgres. # ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers - create_sequence(self.doctype, check_not_exists=True) + frappe.db.create_sequence(self.doctype, check_not_exists=True) name_column = "name bigint primary key" # TODO: set docstatus length diff --git a/frappe/database/sequence.py b/frappe/database/sequence.py index c4789dbdaf..ede4689485 100644 --- a/frappe/database/sequence.py +++ b/frappe/database/sequence.py @@ -5,6 +5,7 @@ def create_sequence( doctype_name: str, *, slug: str = "_id_seq", + temporary=False, check_not_exists: bool = False, cycle: bool = False, cache: int = 0, @@ -14,7 +15,7 @@ def create_sequence( max_value: int = 0, ) -> str: - query = "create sequence" + query = "create sequence" if not temporary else "create temporary sequence" sequence_name = scrub(doctype_name + slug) if check_not_exists: @@ -22,29 +23,29 @@ def create_sequence( query += f" {sequence_name}" - if cache: - query += f" cache {cache}" - else: - # in postgres, the default is cache 1 - if db.db_type == "mariadb": - query += " nocache" - - if start_value: - # default is 1 - query += f" start with {start_value}" - if increment_by: # default is 1 query += f" increment by {increment_by}" if min_value: # default is 1 - query += f" min value {min_value}" + query += f" minvalue {min_value}" if max_value: - query += f" max value {max_value}" + query += f" maxvalue {max_value}" + + if start_value: + # default is 1 + query += f" start {start_value}" + + # in postgres, the default is cache 1 / no cache + if cache: + query += f" cache {cache}" + elif db.db_type == "mariadb": + query += " nocache" if not cycle: + # in postgres, default is no cycle if db.db_type == "mariadb": query += " nocycle" else: @@ -56,21 +57,23 @@ def create_sequence( def get_next_val(doctype_name: str, slug: str = "_id_seq") -> int: - if db.db_type == "postgres": - return db.sql(f"select nextval('\"{scrub(doctype_name + slug)}\"')")[0][0] - return db.sql(f"select nextval(`{scrub(doctype_name + slug)}`)")[0][0] + return db.multisql( + { + "postgres": f"select nextval('\"{scrub(doctype_name + slug)}\"')", + "mariadb": f"select nextval(`{scrub(doctype_name + slug)}`)", + } + )[0][0] def set_next_val( doctype_name: str, next_val: int, *, slug: str = "_id_seq", is_val_used: bool = False ) -> None: - if not is_val_used: - is_val_used = 0 if db.db_type == "mariadb" else "f" - else: - is_val_used = 1 if db.db_type == "mariadb" else "t" + is_val_used = "false" if not is_val_used else "true" - if db.db_type == "postgres": - db.sql(f"SELECT SETVAL('\"{scrub(doctype_name + slug)}\"', {next_val}, '{is_val_used}')") - else: - db.sql(f"SELECT SETVAL(`{scrub(doctype_name + slug)}`, {next_val}, {is_val_used})") + db.multisql( + { + "postgres": f"SELECT SETVAL('\"{scrub(doctype_name + slug)}\"', {next_val}, {is_val_used})", + "mariadb": f"SELECT SETVAL(`{scrub(doctype_name + slug)}`, {next_val}, {is_val_used})", + } + ) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index a7d9536ebc..acb63b5bfa 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -213,7 +213,7 @@ class DatabaseQuery(object): # left join parent, child tables for child in self.tables[1:]: - parent_name = self.cast_name(f"{self.tables[0]}.name") + parent_name = cast_name(f"{self.tables[0]}.name") args.tables += f" {self.join} {child} on ({child}.parent = {parent_name})" if self.grouped_or_conditions: @@ -225,6 +225,7 @@ class DatabaseQuery(object): args.conditions += (" or " if args.conditions else "") + " or ".join(self.or_conditions) self.set_field_tables() + self.cast_name_fields() fields = [] @@ -385,16 +386,8 @@ class DatabaseQuery(object): ] # add tables from fields if self.fields: - for i, field in enumerate(self.fields): - # add cast in locate/strpos - func_found = False - for func in sql_functions: - if func in field.lower(): - self.fields[i] = self.cast_name(field, func) - func_found = True - break - - if func_found or not ("tab" in field and "." in field): + for field in self.fields: + if not ("tab" in field and "." in field) or any(x for x in sql_functions if x in field): continue table_name = field.split(".")[0] @@ -406,38 +399,6 @@ class DatabaseQuery(object): if table_name not in self.tables: self.append_table(table_name) - def cast_name( - self, - column: str, - sql_function: str = "", - ) -> str: - if frappe.db.db_type == "postgres": - if "name" in column.lower(): - if "cast(" not in column.lower() or "::" not in column: - if not sql_function: - return f"cast({column} as varchar)" - - elif sql_function == "locate(": - return re.sub( - r"locate\(([^,]+),([^)]+)\)", - r"locate(\1, cast(\2 as varchar))", - column, - flags=re.IGNORECASE, - ) - - elif sql_function == "strpos(": - return re.sub( - r"strpos\(([^,]+),([^)]+)\)", - r"strpos(cast(\1 as varchar), \2)", - column, - flags=re.IGNORECASE, - ) - - elif sql_function == "ifnull(": - return re.sub(r"ifnull\(([^,]+)", r"ifnull(cast(\1 as varchar)", column, flags=re.IGNORECASE) - - return column - def append_table(self, table_name): self.tables.append(table_name) doctype = table_name[4:-1] @@ -462,6 +423,10 @@ class DatabaseQuery(object): if "." not in field and not _in_standard_sql_methods(field): self.fields[idx] = f"{self.tables[0]}.{field}" + def cast_name_fields(self): + for i, field in enumerate(self.fields): + self.fields[i] = cast_name(field) + def get_table_columns(self): try: return get_table_columns(self.doctype) @@ -541,10 +506,7 @@ class DatabaseQuery(object): if tname not in self.tables: self.append_table(tname) - if "ifnull(" in f.fieldname: - column_name = self.cast_name(f.fieldname, "ifnull(") - else: - column_name = self.cast_name(f"{tname}.`{f.fieldname}`") + column_name = cast_name(f.fieldname if "ifnull(" in f.fieldname else f"{tname}.`{f.fieldname}`") if f.operator.lower() in additional_filters_config: f.update(get_additional_filter_field(additional_filters_config, f, f.value)) @@ -766,7 +728,10 @@ class DatabaseQuery(object): return self.match_filters def get_share_condition(self): - return f"`tab{self.doctype}`.name in ({', '.join(frappe.db.escape(s, percent=False) for s in self.shared)})" + return ( + cast_name(f"`tab{self.doctype}`.name") + + f" in ({', '.join(frappe.db.escape(s, percent=False) for s in self.shared)})" + ) def add_user_permissions(self, user_permissions): meta = frappe.get_meta(self.doctype) @@ -794,7 +759,9 @@ class DatabaseQuery(object): if frappe.get_system_settings("apply_strict_user_permissions"): condition = "" else: - empty_value_condition = f"ifnull(`tab{self.doctype}`.`{df.get('fieldname')}`, '')=''" + empty_value_condition = cast_name( + f"ifnull(`tab{self.doctype}`.`{df.get('fieldname')}`, '')=''" + ) condition = empty_value_condition + " or " for permission in user_permission_values: @@ -815,7 +782,7 @@ class DatabaseQuery(object): if docs: values = ", ".join(frappe.db.escape(doc, percent=False) for doc in docs) - condition += f"`tab{self.doctype}`.`{df.get('fieldname')}` in ({values})" + condition += cast_name(f"`tab{self.doctype}`.`{df.get('fieldname')}`") + f" in ({values})" match_conditions.append(f"({condition})") match_filters[df.get("options")] = docs @@ -933,6 +900,40 @@ class DatabaseQuery(object): update_user_settings(self.doctype, user_settings) +def cast_name(column: str) -> str: + """Casts name field to varchar for postgres + + Handles majorly 4 cases: + 1. locate + 2. strpos + 3. ifnull + 4. coalesce + + Uses regex substitution. + + Example: + input - "ifnull(`tabBlog Post`.`name`, '')=''" + output - "ifnull(cast(`tabBlog Post`.`name` as varchar), '')=''" """ + + if frappe.db.db_type == "mariadb": + return column + + kwargs = {"string": column, "flags": re.IGNORECASE} + if "cast(" not in column.lower() and "::" not in column: + if re.search(r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", **kwargs): + return re.sub( + r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\)", r"locate(\1, cast(\2 as varchar))", **kwargs + ) + + elif match := re.search(r"(strpos|ifnull|coalesce)\(\s*([`\"]?name[`\"]?)\s*,", **kwargs): + func = match.groups()[0] + return re.sub(rf"{func}\(\s*([`\"]?name[`\"]?)\s*,", rf"{func}(cast(\1 as varchar),", **kwargs) + + return re.sub(r"([`\"]?tab[\w`\" -]+\.[`\"]?name[`\"]?)(?!\w)", r"cast(\1 as varchar)", **kwargs) + + return column + + def check_parent_permission(parent, child_doctype): if parent: # User may pass fake parent and get the information from the child table diff --git a/frappe/model/naming.py b/frappe/model/naming.py index aa502f5a4c..bb93244a66 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -14,6 +14,11 @@ if TYPE_CHECKING: from frappe.model.meta import Meta +# NOTE: This is used to keep track of status of sites +# whether `log_types` have autoincremented naming set for the site or not. +autoincremented_site_status_map = {} + + def set_new_name(doc): """ Sets the `name` property for the document based on various rules. @@ -35,9 +40,7 @@ def set_new_name(doc): doc.name = None if is_autoincremented(doc.doctype, meta): - from frappe.database.sequence import get_next_val - - doc.name = get_next_val(doc.doctype) + doc.name = frappe.db.get_next_sequence_val(doc.doctype) return if getattr(doc, "amended_from", None): @@ -72,12 +75,11 @@ def set_new_name(doc): doc.name = validate_name(doc.doctype, doc.name, meta.get_field("name_case")) -def is_autoincremented(doctype: str, meta: "Meta" = None): +def is_autoincremented(doctype: str, meta: Optional["Meta"] = None) -> bool: + """Checks if the doctype has autoincrement autoname set""" + if doctype in log_types: - if ( - frappe.local.autoincremented_status_map.get(frappe.local.site) is None - or frappe.local.autoincremented_status_map[frappe.local.site] == -1 - ): + if autoincremented_site_status_map.get(frappe.local.site) is None: if ( frappe.db.sql( f"""select data_type FROM information_schema.columns @@ -85,22 +87,19 @@ def is_autoincremented(doctype: str, meta: "Meta" = None): )[0][0] == "bigint" ): - frappe.local.autoincremented_status_map[frappe.local.site] = 1 + autoincremented_site_status_map[frappe.local.site] = 1 return True else: - frappe.local.autoincremented_status_map[frappe.local.site] = 0 + autoincremented_site_status_map[frappe.local.site] = 0 - elif frappe.local.autoincremented_status_map[frappe.local.site]: + elif autoincremented_site_status_map[frappe.local.site]: return True else: if not meta: meta = frappe.get_meta(doctype) - if getattr(meta, "issingle", False): - return False - - if meta.autoname == "autoincrement": + if not getattr(meta, "issingle", False) and meta.autoname == "autoincrement": return True return False @@ -329,11 +328,9 @@ def validate_name(doctype: str, name: Union[int, str], case: Optional[str] = Non if isinstance(name, int): if is_autoincremented(doctype): - from frappe.database.sequence import set_next_val - - # this will set the sequence val to be the provided name and set it to be used - # so that the sequence will start from the next val of the setted val(name) - set_next_val(doctype, name, is_val_used=True) + # this will set the sequence value to be the provided name/value and set it to be used + # so that the sequence will start from the next value + frappe.db.set_next_sequence_val(doctype, name, is_val_used=True) return name frappe.throw(_("Invalid name type (integer) for varchar name column"), frappe.NameError) diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index b7fe61b385..5a5af389ee 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -65,11 +65,7 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control }; var update_input = function() { - if (me.doctype && me.docname) { - me.set_input(me.value); - } else { - me.set_input(me.value || null); - } + me.set_input(me.value); }; if (me.disp_status != "None") { diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index c56ffc592d..2572f7b2e3 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -248,7 +248,7 @@ frappe.ui.form.Form = class FrappeForm { // on main doc frappe.model.on(me.doctype, "*", function(fieldname, value, doc, skip_dirty_trigger=false) { // set input - if (cstr(doc.name) === me.docname) { + if (doc.name == me.docname) { if (!skip_dirty_trigger) { me.dirty(); } @@ -273,7 +273,7 @@ frappe.ui.form.Form = class FrappeForm { // using $.each to preserve df via closure $.each(table_fields, function(i, df) { frappe.model.on(df.options, "*", function(fieldname, value, doc) { - if(doc.parent===me.docname && doc.parentfield===df.fieldname) { + if (doc.parent == me.docname && doc.parentfield === df.fieldname) { me.dirty(); me.fields_dict[df.fieldname].grid.set_value(fieldname, value, doc); return me.script_manager.trigger(fieldname, doc.doctype, doc.name); @@ -356,7 +356,7 @@ frappe.ui.form.Form = class FrappeForm { // check permissions if (!this.has_read_permission()) { - frappe.show_not_permitted(__(this.doctype) + " " + __(this.docname)); + frappe.show_not_permitted(__(this.doctype) + " " + __(cstr(this.docname))); return; } diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index 3b95a4b3f1..ad7a1181f6 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -403,7 +403,7 @@ $.extend(frappe.model, { } }); } else { - if(typeof filters==="string" && locals[doctype] && locals[doctype][filters]) { + if (["number", "string"].includes(typeof filters) && locals[doctype] && locals[doctype][filters]) { return locals[doctype][filters][fieldname]; } else { var l = frappe.get_list(doctype, filters); diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index c1ff1a1a15..19a8c445f8 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -61,10 +61,12 @@ class TestReportview(unittest.TestCase): in build_match_conditions(as_condition=False) ) # get as conditions - self.assertEqual( - build_match_conditions(as_condition=True), - """(((ifnull(`tabBlog Post`.`name`, '')='' or `tabBlog Post`.`name` in ('-test-blog-post-1', '-test-blog-post'))))""", - ) + if frappe.db.db_type == "mariadb": + assertion_string = """(((ifnull(`tabBlog Post`.`name`, '')='' or `tabBlog Post`.`name` in ('-test-blog-post-1', '-test-blog-post'))))""" + else: + assertion_string = """(((ifnull(cast(`tabBlog Post`.`name` as varchar), '')='' or cast(`tabBlog Post`.`name` as varchar) in ('-test-blog-post-1', '-test-blog-post'))))""" + + self.assertEqual(build_match_conditions(as_condition=True), assertion_string) frappe.set_user("Administrator") @@ -619,19 +621,22 @@ class TestReportview(unittest.TestCase): def test_cast_name(self): from frappe.core.doctype.doctype.test_doctype import new_doctype + frappe.delete_doc_if_exists("DocType", "autoinc_dt_test") dt = new_doctype("autoinc_dt_test", autoname="autoincrement").insert(ignore_permissions=True) query = DatabaseQuery("autoinc_dt_test").execute( - fields=["locate('1', `tabautoinc_dt_test`.`name`)", "`tabautoinc_dt_test`.`name`"], + fields=["locate('1', `tabautoinc_dt_test`.`name`)", "name", "locate('1', name)"], filters={"name": 1}, run=False, ) if frappe.db.db_type == "postgres": - self.assertTrue('strpos( cast( "tabautoinc_dt_test"."name" as varchar), \'1\')' in query) + self.assertTrue('strpos( cast("tabautoinc_dt_test"."name" as varchar), \'1\')' in query) + self.assertTrue("strpos( cast(name as varchar), '1')" in query) self.assertTrue('where cast("tabautoinc_dt_test"."name" as varchar) = \'1\'' in query) else: self.assertTrue("locate('1', `tabautoinc_dt_test`.`name`)" in query) + self.assertTrue("locate('1', name)" in query) self.assertTrue("where `tabautoinc_dt_test`.`name` = 1" in query) dt.delete(ignore_permissions=True) diff --git a/frappe/tests/test_sequence.py b/frappe/tests/test_sequence.py new file mode 100644 index 0000000000..a60e4b1ac9 --- /dev/null +++ b/frappe/tests/test_sequence.py @@ -0,0 +1,54 @@ +import psycopg2 +import pymysql + +import frappe +from frappe.tests.utils import FrappeTestCase + + +class TestSequence(FrappeTestCase): + def generate_sequence_name(self) -> str: + return self._testMethodName + "_" + frappe.generate_hash(length=5) + + def test_set_next_val(self): + seq_name = self.generate_sequence_name() + frappe.db.create_sequence(seq_name, check_not_exists=True, temporary=True) + + next_val = frappe.db.get_next_sequence_val(seq_name) + frappe.db.set_next_sequence_val(seq_name, next_val + 1) + self.assertEqual(next_val + 1, frappe.db.get_next_sequence_val(seq_name)) + + next_val = frappe.db.get_next_sequence_val(seq_name) + frappe.db.set_next_sequence_val(seq_name, next_val + 1, is_val_used=True) + self.assertEqual(next_val + 2, frappe.db.get_next_sequence_val(seq_name)) + + def test_create_sequence(self): + seq_name = self.generate_sequence_name() + frappe.db.create_sequence(seq_name, max_value=2, cycle=True, temporary=True) + frappe.db.get_next_sequence_val(seq_name) + frappe.db.get_next_sequence_val(seq_name) + self.assertEqual(1, frappe.db.get_next_sequence_val(seq_name)) + + seq_name = self.generate_sequence_name() + frappe.db.create_sequence(seq_name, max_value=2, temporary=True) + frappe.db.get_next_sequence_val(seq_name) + frappe.db.get_next_sequence_val(seq_name) + + try: + frappe.db.get_next_sequence_val(seq_name) + except pymysql.err.OperationalError as e: + self.assertEqual(e.args[0], 4084) + except psycopg2.errors.SequenceGeneratorLimitExceeded: + pass + else: + self.fail("NEXTVAL didn't raise any error upon sequence's end") + + # without this, we're not able to move further + # as postgres doesn't allow moving further in a transaction + # when an error occurs + frappe.db.rollback() + + seq_name = self.generate_sequence_name() + frappe.db.create_sequence(seq_name, min_value=10, max_value=20, increment_by=5, temporary=True) + self.assertEqual(10, frappe.db.get_next_sequence_val(seq_name)) + self.assertEqual(15, frappe.db.get_next_sequence_val(seq_name)) + self.assertEqual(20, frappe.db.get_next_sequence_val(seq_name)) From 76a6c282b81dfdc81ebf64a1e5cb9bdc0a5b15d6 Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Fri, 29 Apr 2022 16:07:40 +0530 Subject: [PATCH 04/18] fix: use proper validations for number card validation method (#16581) --- .../desk/doctype/number_card/number_card.js | 5 +++- .../desk/doctype/number_card/number_card.py | 25 +++++++++++++------ frappe/desk/query_report.py | 4 ++- .../public/js/frappe/utils/dashboard_utils.js | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/frappe/desk/doctype/number_card/number_card.js b/frappe/desk/doctype/number_card/number_card.js index f548388a99..79ddb71187 100644 --- a/frappe/desk/doctype/number_card/number_card.js +++ b/frappe/desk/doctype/number_card/number_card.js @@ -27,8 +27,11 @@ frappe.ui.form.on('Number Card', { frm.trigger('set_method_description'); frm.trigger('render_filters_table'); } - frm.trigger('create_add_to_dashboard_button'); frm.trigger('set_parent_document_type'); + + if (!frm.is_new()) { + frm.trigger('create_add_to_dashboard_button'); + } }, create_add_to_dashboard_button: function(frm) { diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 370b187ffe..e1b2b19026 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -20,15 +20,24 @@ class NumberCard(Document): self.name = append_number_if_name_exists("Number Card", self.name) def validate(self): - if not self.document_type: - frappe.throw(_("Document type is required to create a number card")) + if self.type == "Document Type": + if not (self.document_type and self.function): + frappe.throw(_("Document Type and Function are required to create a number card")) - if ( - self.document_type - and frappe.get_meta(self.document_type).istable - and not self.parent_document_type - ): - frappe.throw(_("Parent document type is required to create a number card")) + if ( + self.document_type + and frappe.get_meta(self.document_type).istable + and not self.parent_document_type + ): + frappe.throw(_("Parent Document Type is required to create a number card")) + + elif self.type == "Report": + if not (self.report_name and self.report_field and self.function): + frappe.throw(_("Report Name, Report Field and Fucntion are required to create a number card")) + + elif self.type == "Custom": + if not self.method: + frappe.throw(_("Method is required to create a number card")) def on_update(self): if frappe.conf.developer_mode and self.is_standard: diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 894e82d117..a51fd8b1e3 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -58,6 +58,8 @@ def get_report_doc(report_name): def get_report_result(report, filters): + res = None + if report.report_type == "Query Report": res = report.execute_query_report(filters) @@ -84,7 +86,7 @@ def generate_report_result( res = get_report_result(report, filters) or [] columns, result, message, chart, report_summary, skip_total_row = ljust_list(res, 6) - columns = [get_column_as_dict(col) for col in columns] + columns = [get_column_as_dict(col) for col in (columns or [])] report_column_names = [col["fieldname"] for col in columns] # convert to list of dicts diff --git a/frappe/public/js/frappe/utils/dashboard_utils.js b/frappe/public/js/frappe/utils/dashboard_utils.js index 85a4048a47..c4b094b216 100644 --- a/frappe/public/js/frappe/utils/dashboard_utils.js +++ b/frappe/public/js/frappe/utils/dashboard_utils.js @@ -249,7 +249,7 @@ frappe.dashboard_utils = { {args: values} ).then(()=> { let dashboard_route_html = - `${values.dashboard}`; + `${values.dashboard}`; let message = __("{0} {1} added to Dashboard {2}", [doctype, values.name, dashboard_route_html]); 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 05/18] 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 06/18] 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 07/18] 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 | 82 ++++++++----------- frappe/public/js/frappe/ui/slides.js | 4 +- 2 files changed, 35 insertions(+), 51 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: "country", label: __("Your Country"), reqd: 1, - fieldtype: "Autocomplete", - placeholder: __('Select Country') + fieldname: "language", + label: __("Your Language"), + fieldtype: "Select", + placeholder: __('Select Language'), + reqd: 1, + }, + { + fieldname: "country", + label: __("Your Country"), + fieldtype: "Autocomplete", + 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 08/18] 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 09/18] 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 10/18] 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 11/18] 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 12/18] 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 13/18] 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 14/18] 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 15/18] 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 | 38 ++++++------------- frappe/tests/test_db_query.py | 23 +++++++++++ .../workflow_action/workflow_action.py | 9 +++-- 9 files changed, 54 insertions(+), 35 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) + condition_query = frappe.db.query.build_conditions(doctype, filters) - 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, - ) + 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 16/18] 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: From de45cf9bbd20f7d07b47936cad61854408820eec Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 5 May 2022 13:13:40 +0200 Subject: [PATCH 17/18] feat: Use autocomplete attributes for login, signup and password reset --- frappe/templates/signup.html | 4 ++-- frappe/www/login.html | 4 ++-- frappe/www/update-password.html | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/templates/signup.html b/frappe/templates/signup.html index abc1328e25..7d93bdec85 100644 --- a/frappe/templates/signup.html +++ b/frappe/templates/signup.html @@ -3,12 +3,12 @@
+ required autofocus autocomplete="name">
+ placeholder="{{ _('jane@example.com') }}" required autocomplete="username">
diff --git a/frappe/www/login.html b/frappe/www/login.html index 927f451965..1aaaf85656 100644 --- a/frappe/www/login.html +++ b/frappe/www/login.html @@ -7,7 +7,7 @@