From 77e73ccea4ade94b6b37972328643f69394fba41 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Sun, 21 May 2023 16:35:57 +0530 Subject: [PATCH 01/32] feat: support for dynamic URL in webhook Closes #11921 --- frappe/integrations/doctype/webhook/webhook.json | 12 ++++++++++-- frappe/integrations/doctype/webhook/webhook.py | 15 ++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json index a21e460659..9b402c6a27 100644 --- a/frappe/integrations/doctype/webhook/webhook.json +++ b/frappe/integrations/doctype/webhook/webhook.json @@ -18,8 +18,9 @@ "html_condition", "sb_webhook", "request_url", - "request_method", + "is_dynamic_url", "cb_webhook", + "request_method", "request_structure", "sb_security", "enable_security", @@ -200,10 +201,17 @@ { "fieldname": "section_break_28", "fieldtype": "Section Break" + }, + { + "default": "0", + "description": "On checking this option, URL will be treated like a jinja template string", + "fieldname": "is_dynamic_url", + "fieldtype": "Check", + "label": "Is Dynamic URL?" } ], "links": [], - "modified": "2022-07-11 08:54:10.740512", + "modified": "2023-05-21 16:30:10.740512", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook", diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 7d168c659f..1b56a1b129 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -115,29 +115,34 @@ def enqueue_webhook(doc, webhook) -> None: webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name")) headers = get_webhook_headers(doc, webhook) data = get_webhook_data(doc, webhook) - r = None + if webhook.is_dynamic_url: + request_url = frappe.render_template(webhook.request_url, get_context(doc)) + else: + request_url = webhook.request_url + + r = None for i in range(3): try: r = requests.request( method=webhook.request_method, - url=webhook.request_url, + url=request_url, data=json.dumps(data, default=str), headers=headers, timeout=5, ) r.raise_for_status() frappe.logger().debug({"webhook_success": r.text}) - log_request(webhook.name, doc.name, webhook.request_url, headers, data, r) + log_request(webhook.name, doc.name, request_url, headers, data, r) break except requests.exceptions.ReadTimeout as e: frappe.logger().debug({"webhook_error": e, "try": i + 1}) - log_request(webhook.name, doc.name, webhook.request_url, headers, data) + log_request(webhook.name, doc.name, request_url, headers, data) except Exception as e: frappe.logger().debug({"webhook_error": e, "try": i + 1}) - log_request(webhook.name, doc.name, webhook.request_url, headers, data, r) + log_request(webhook.name, doc.name, request_url, headers, data, r) sleep(3 * i + 1) if i != 2: continue From 8f8c6b51e87f7659b87ac9c6af34932b35098da0 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Sun, 21 May 2023 16:36:08 +0530 Subject: [PATCH 02/32] test: tests for dynamic URL --- .../doctype/webhook/test_webhook.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/frappe/integrations/doctype/webhook/test_webhook.py b/frappe/integrations/doctype/webhook/test_webhook.py index 8284db7fd3..1092d2a253 100644 --- a/frappe/integrations/doctype/webhook/test_webhook.py +++ b/frappe/integrations/doctype/webhook/test_webhook.py @@ -206,3 +206,57 @@ class TestWebhook(FrappeTestCase): enqueue_webhook(doc, wh) log = frappe.get_last_doc("Webhook Request Log") self.assertEqual(len(json.loads(log.response)["json"]), 3) + + def test_webhook_with_dynamic_url_enabled(self): + wh_config = { + "doctype": "Webhook", + "webhook_doctype": "Note", + "webhook_docevent": "after_insert", + "enabled": 1, + "request_url": "https://httpbin.org/anything/{{ doc.doctype }}", + "is_dynamic_url": 1, + "request_method": "POST", + "request_structure": "JSON", + "webhook_json": '{}', + "meets_condition": "Yes", + "webhook_headers": [ + { + "key": "Content-Type", + "value": "application/json", + } + ], + } + + with get_test_webhook(wh_config) as wh: + doc = frappe.new_doc("Note") + doc.title = "Test Webhook Note" + enqueue_webhook(doc, wh) + log = frappe.get_last_doc("Webhook Request Log") + self.assertEqual(json.loads(log.response)["url"], "https://httpbin.org/anything/Note") + + def test_webhook_with_dynamic_url_disabled(self): + wh_config = { + "doctype": "Webhook", + "webhook_doctype": "Note", + "webhook_docevent": "after_insert", + "enabled": 1, + "request_url": "https://httpbin.org/anything/{{doc.doctype}}", + "is_dynamic_url": 0, + "request_method": "POST", + "request_structure": "JSON", + "webhook_json": '{}', + "meets_condition": "Yes", + "webhook_headers": [ + { + "key": "Content-Type", + "value": "application/json", + } + ], + } + + with get_test_webhook(wh_config) as wh: + doc = frappe.new_doc("Note") + doc.title = "Test Webhook Note" + enqueue_webhook(doc, wh) + log = frappe.get_last_doc("Webhook Request Log") + self.assertEqual(json.loads(log.response)["url"], "https://httpbin.org/anything/{{doc.doctype}}") From 3f08e80d888b279b3a7ad1ebdb852e1d7c8b340a Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Sun, 21 May 2023 16:44:34 +0530 Subject: [PATCH 03/32] style: lint test file --- frappe/integrations/doctype/webhook/test_webhook.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/integrations/doctype/webhook/test_webhook.py b/frappe/integrations/doctype/webhook/test_webhook.py index 1092d2a253..7235447662 100644 --- a/frappe/integrations/doctype/webhook/test_webhook.py +++ b/frappe/integrations/doctype/webhook/test_webhook.py @@ -217,7 +217,7 @@ class TestWebhook(FrappeTestCase): "is_dynamic_url": 1, "request_method": "POST", "request_structure": "JSON", - "webhook_json": '{}', + "webhook_json": "{}", "meets_condition": "Yes", "webhook_headers": [ { @@ -244,7 +244,7 @@ class TestWebhook(FrappeTestCase): "is_dynamic_url": 0, "request_method": "POST", "request_structure": "JSON", - "webhook_json": '{}', + "webhook_json": "{}", "meets_condition": "Yes", "webhook_headers": [ { @@ -259,4 +259,6 @@ class TestWebhook(FrappeTestCase): doc.title = "Test Webhook Note" enqueue_webhook(doc, wh) log = frappe.get_last_doc("Webhook Request Log") - self.assertEqual(json.loads(log.response)["url"], "https://httpbin.org/anything/{{doc.doctype}}") + self.assertEqual( + json.loads(log.response)["url"], "https://httpbin.org/anything/{{doc.doctype}}" + ) From 9fa2655bd995ffb75e9a244f7b4c6c4103b2287b Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Mon, 22 May 2023 23:11:21 +0530 Subject: [PATCH 04/32] fix: bring back modified field in json file --- frappe/integrations/doctype/webhook/webhook.json | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json index d30a605f06..cfb2a2e01c 100644 --- a/frappe/integrations/doctype/webhook/webhook.json +++ b/frappe/integrations/doctype/webhook/webhook.json @@ -218,6 +218,7 @@ "link_fieldname": "webhook" } ], + "modified": "2023-05-22 16:30:10.740512", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook", From 76d7e6e3791bcaeb8346576f105d12e47dd31bf5 Mon Sep 17 00:00:00 2001 From: Yash Jane Date: Thu, 25 May 2023 13:19:07 +0530 Subject: [PATCH 05/32] feat: added email template customization option for welcome and password reset emails --- .../system_settings/system_settings.json | 16 +++++++++++++- frappe/core/doctype/user/user.py | 21 +++++++++++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 091dc1df1e..5efe87da25 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -72,6 +72,8 @@ "disable_standard_email_footer", "hide_footer_in_auto_email_reports", "attach_view_link", + "welcome_email_template", + "reset_password_template", "prepared_report_section", "max_auto_email_report_per_user", "system_updates_section", @@ -549,12 +551,24 @@ "fieldname": "enable_telemetry", "fieldtype": "Check", "label": "Allow Sending Usage Data for Improving Applications" + }, + { + "fieldname": "welcome_email_template", + "fieldtype": "Link", + "label": "Welcome Email Template", + "options": "Email Template" + }, + { + "fieldname": "reset_password_template", + "fieldtype": "Link", + "label": "Reset Password Template", + "options": "Email Template" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2023-04-23 11:14:59.302851", + "modified": "2023-05-25 13:02:54.808773", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 14266e4cd8..de49b00bbd 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -325,7 +325,10 @@ class User(Document): return (self.first_name or "") + (self.first_name and " " or "") + (self.last_name or "") def password_reset_mail(self, link): - self.send_login_mail(_("Password Reset"), "password_reset", {"link": link}, now=True) + + reset_password_template = frappe.db.get_system_setting("reset_password_template") + + self.send_login_mail(_("Password Reset"), "password_reset", {"link": link}, now=True, custom_template=reset_password_template) def send_welcome_mail_to_user(self): from frappe.utils import get_url @@ -342,16 +345,19 @@ class User(Document): else: subject = _("Complete Registration") + welcome_email_template = frappe.db.get_system_setting("welcome_email_template") + self.send_login_mail( subject, "new_user", - dict( + dict( link=link, site_url=get_url(), ), + custom_template=welcome_email_template, ) - def send_login_mail(self, subject, template, add_args, now=None): + def send_login_mail(self, subject, template, add_args, now=None, custom_template=None): """send mail with login details""" from frappe.utils import get_url from frappe.utils.user import get_user_fullname @@ -374,11 +380,18 @@ class User(Document): frappe.session.user not in STANDARD_USERS and get_formatted_email(frappe.session.user) or None ) + if custom_template: + from frappe.email.doctype.email_template.email_template import get_email_template + email_template = get_email_template(custom_template, args) + subject = email_template.get("subject") + content = email_template.get("message") + frappe.sendmail( recipients=self.email, sender=sender, subject=subject, - template=template, + template=template if not custom_template else None, + content=content if custom_template else None, args=args, header=[subject, "green"], delayed=(not now) if now is not None else self.flags.delay_emails, From a13592a66a94b8bdd11f5c3d2c0cd9d69edc19f8 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 31 May 2023 08:55:59 +0530 Subject: [PATCH 06/32] fix: Warn users if "Repeat Header and Footer" is disabled - and if the user still sets Letterhead Also, add label to the print options --- frappe/printing/page/print/print.js | 23 ++++++++++++++++++---- frappe/public/scss/desk/print_preview.scss | 3 --- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/frappe/printing/page/print/print.js b/frappe/printing/page/print/print.js index 8e5e165c78..14770c2d26 100644 --- a/frappe/printing/page/print/print.js +++ b/frappe/printing/page/print/print.js @@ -91,7 +91,7 @@ frappe.ui.form.PrintView = class { fieldtype: "Link", fieldname: "print_format", options: "Print Format", - placeholder: __("Print Format"), + label: __("Print Format"), get_query: () => { return { filters: { doc_type: this.frm.doctype } }; }, @@ -101,7 +101,7 @@ frappe.ui.form.PrintView = class { this.language_selector = this.add_sidebar_item({ fieldtype: "Link", fieldname: "language", - placeholder: __("Language"), + label: __("Language"), options: "Language", change: () => { this.set_user_lang(); @@ -109,12 +109,27 @@ frappe.ui.form.PrintView = class { }, }).$input; + let description = ""; + if (!cint(this.print_settings.repeat_header_footer)) { + description = + "
" + + __("Footer might not be visible as {0} option is disabled
", [ + `${__( + "Repeat Header and Footer" + )}`, + ]); + } + const print_view = this; this.letterhead_selector = this.add_sidebar_item({ fieldtype: "Link", fieldname: "letterhead", options: "Letter Head", - placeholder: __("Letter Head"), - change: () => this.preview(), + label: __("Letter Head"), + description: description, + change: function () { + this.set_description(this.get_value() ? description : ""); + print_view.preview(); + }, }).$input; this.sidebar_dynamic_section = $(`
`).appendTo( this.sidebar diff --git a/frappe/public/scss/desk/print_preview.scss b/frappe/public/scss/desk/print_preview.scss index 468b37fe5a..ed85f8b933 100644 --- a/frappe/public/scss/desk/print_preview.scss +++ b/frappe/public/scss/desk/print_preview.scss @@ -45,9 +45,6 @@ .layout-side-section.print-preview-sidebar { padding-right: var(--padding-md); - .clearfix { - display: none; - } .label-area { white-space: nowrap; From 55455cef4e2b4bc73c80d588246d41013f396795 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 31 May 2023 09:03:07 +0530 Subject: [PATCH 07/32] fix: Update label "in PDF" is not required as it is already under PDF settings. --- frappe/printing/doctype/print_settings/print_settings.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/printing/doctype/print_settings/print_settings.json b/frappe/printing/doctype/print_settings/print_settings.json index f45de7637d..a67440b54e 100644 --- a/frappe/printing/doctype/print_settings/print_settings.json +++ b/frappe/printing/doctype/print_settings/print_settings.json @@ -47,7 +47,7 @@ "default": "1", "fieldname": "repeat_header_footer", "fieldtype": "Check", - "label": "Repeat Header and Footer in PDF" + "label": "Repeat Header and Footer" }, { "fieldname": "column_break_4", @@ -176,7 +176,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2021-09-17 12:59:14.783694", + "modified": "2023-05-30 14:55:25.740691", "modified_by": "Administrator", "module": "Printing", "name": "Print Settings", @@ -193,5 +193,6 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file From abb479f89fdca712170dccb20215febbbdb44e3c Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 31 May 2023 09:55:39 +0530 Subject: [PATCH 08/32] fix: Check if signature already exist --- frappe/public/js/frappe/views/communication.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index c3e998788d..6baf4893e9 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -747,7 +747,10 @@ frappe.views.CommunicationComposer = class { this.content_set = true; } - message += await this.get_signature(sender_email || null); + const signature = await this.get_signature(sender_email || ""); + if (!this.content_set || !strip_html(message).includes(strip_html(signature))) { + message += signature; + } if (this.is_a_reply && !this.reply_set) { message += this.get_earlier_reply(); From aabaab0fd21c402d713e246034e1a9d5d616addb Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 31 May 2023 11:15:33 +0530 Subject: [PATCH 09/32] feat: used cached version of document in mapper (#21186) * fix: in get_mapped_doc return cached version if available * feat: specify explicitly cached version of doc in doc_mapper [skip ci] --- frappe/model/mapper.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/model/mapper.py b/frappe/model/mapper.py index 9df79ef276..84d9c6c095 100644 --- a/frappe/model/mapper.py +++ b/frappe/model/mapper.py @@ -62,6 +62,7 @@ def get_mapped_doc( postprocess=None, ignore_permissions=False, ignore_child_tables=False, + cached=False, ): apply_strict_user_permissions = frappe.get_system_settings("apply_strict_user_permissions") @@ -79,7 +80,10 @@ def get_mapped_doc( ): target_doc.raise_no_permission_to("create") - source_doc = frappe.get_doc(from_doctype, from_docname) + if cached: + source_doc = frappe.get_cached_doc(from_doctype, from_docname) + else: + source_doc = frappe.get_doc(from_doctype, from_docname) if not ignore_permissions: if not source_doc.has_permission("read"): From 4a81d9f8e307261ca21a3f86e7efa4904db4d093 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 12:29:31 +0530 Subject: [PATCH 10/32] feat!: populate fields from kwargs in `frappe.new_doc` (#21190) This makes it similar to `get_doc` API BUT still signifies intent that it's a "NEW" document. Minor Breaking Change: positional arguments are now forcefully keyword arguments. Only seems to be used internally from what I can tell. https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/frappe/.*+/frappe.new_doc%5C%28.*%3F%2C.*%3F%5C%29/+lang:python+&patternType=regexp&case=yes&sm=0&groupBy=repo --- frappe/__init__.py | 12 ++++++++++-- frappe/model/mapper.py | 4 +++- .../v11_0/apply_customization_to_custom_doctype.py | 2 +- frappe/tests/test_document.py | 4 ++++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 5efdfd8ce9..e5a0b9c4aa 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1049,18 +1049,26 @@ def reset_metadata_version(): def new_doc( doctype: str, + *, parent_doc: Optional["Document"] = None, parentfield: str | None = None, as_dict: bool = False, + **kwargs, ) -> "Document": """Returns a new document of the given DocType with defaults set. :param doctype: DocType of the new document. :param parent_doc: [optional] add to parent document. - :param parentfield: [optional] add against this `parentfield`.""" + :param parentfield: [optional] add against this `parentfield`. + :param as_dict: [optional] return as dictionary instead of Document. + :param kwargs: [optional] You can specify fields as field=value pairs in function call. + """ + from frappe.model.create_new import get_new_doc - return get_new_doc(doctype, parent_doc, parentfield, as_dict=as_dict) + new_doc = get_new_doc(doctype, parent_doc, parentfield, as_dict=as_dict) + + return new_doc.update(kwargs) def set_value(doctype, docname, fieldname, value=None): diff --git a/frappe/model/mapper.py b/frappe/model/mapper.py index 84d9c6c095..4b9051f59c 100644 --- a/frappe/model/mapper.py +++ b/frappe/model/mapper.py @@ -259,7 +259,9 @@ def map_fetch_fields(target_doc, df, no_copy_fields): def map_child_doc(source_d, target_parent, table_map, source_parent=None): target_child_doctype = table_map["doctype"] target_parentfield = target_parent.get_parentfield_of_doctype(target_child_doctype) - target_d = frappe.new_doc(target_child_doctype, target_parent, target_parentfield) + target_d = frappe.new_doc( + target_child_doctype, parent_doc=target_parent, parentfield=target_parentfield + ) map_doc(source_d, target_d, table_map, source_parent) diff --git a/frappe/patches/v11_0/apply_customization_to_custom_doctype.py b/frappe/patches/v11_0/apply_customization_to_custom_doctype.py index d652efcef7..90986e065a 100644 --- a/frappe/patches/v11_0/apply_customization_to_custom_doctype.py +++ b/frappe/patches/v11_0/apply_customization_to_custom_doctype.py @@ -44,7 +44,7 @@ def execute(): if field: field.update(cf) else: - df = frappe.new_doc("DocField", meta, "fields") + df = frappe.new_doc("DocField", parent_doc=meta, parentfield="fields") df.update(cf) meta.fields.append(df) frappe.db.delete("Custom Field", {"name": cf.name}) diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index 474971c935..4e575528ab 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -169,6 +169,10 @@ class TestDocument(FrappeTestCase): with self.assertQueryCount(0): user.db_set("user_type", "Magical Wizard") + def test_new_doc_with_fields(self): + user = frappe.new_doc("User", first_name="wizard") + self.assertEqual(user.first_name, "wizard") + def test_update_after_submit(self): d = self.test_insert() d.starts_on = "2014-09-09" From fd232805fe27b5da95178f2b61668ef05d0c2251 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 12:34:06 +0530 Subject: [PATCH 11/32] chore: capture frappe version in heartbeat --- frappe/public/js/telemetry/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/telemetry/index.js b/frappe/public/js/telemetry/index.js index 48afaa5258..b9dee3be1c 100644 --- a/frappe/public/js/telemetry/index.js +++ b/frappe/public/js/telemetry/index.js @@ -32,9 +32,9 @@ class TelemetryManager { } } - capture(event, app) { + capture(event, app, props) { if (!this.enabled) return; - posthog.capture(`${app}_${event}`); + posthog.capture(`${app}_${event}`, props); } disable() { @@ -49,7 +49,7 @@ class TelemetryManager { if (!last || moment(now).diff(moment(last), "hours") > 12) { localStorage.setItem(KEY, now.toISOString()); - this.capture("heartbeat", "frappe"); + this.capture("heartbeat", "frappe", { frappe_version: frappe.boot?.versions?.frappe }); } } From 9f5a994f709898d157f55783cfa96c5702e0634a Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 30 May 2023 13:51:18 +0530 Subject: [PATCH 12/32] fix!: improved filter validation in `Engine.get_query` --- frappe/database/database.py | 26 +++++++++++++++---- frappe/database/query.py | 15 ++++++++--- .../desk/doctype/number_card/number_card.py | 6 ++++- frappe/desk/listview.py | 7 ++++- frappe/tests/test_query.py | 7 ----- frappe/utils/goal.py | 1 + 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 8b077ce4f7..aa30cd9ae9 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -812,6 +812,7 @@ class Database: fields=fields, distinct=distinct, limit=limit, + validate_filters=True, ) if isinstance(fields, str) and fields == "*": as_dict = True @@ -840,6 +841,7 @@ class Database: order_by=order_by, distinct=distinct, limit=limit, + validate_filters=True, ).run(debug=debug, run=run, as_dict=as_dict, pluck=pluck) return {} @@ -889,7 +891,12 @@ class Database: field, val, modified=modified, modified_by=modified_by, update_modified=update_modified ) - query = frappe.qb.get_query(table=dt, filters=dn, update=True) + query = frappe.qb.get_query( + table=dt, + filters=dn, + update=True, + validate_filters=True, + ) if isinstance(dn, str): frappe.clear_document_cache(dt, dn) @@ -1057,9 +1064,13 @@ class Database: cache_count = frappe.cache().get_value(f"doctype:count:{dt}") if cache_count is not None: return cache_count - count = frappe.qb.get_query(table=dt, filters=filters, fields=Count("*"), distinct=distinct).run( - debug=debug - )[0][0] + count = frappe.qb.get_query( + table=dt, + filters=filters, + fields=Count("*"), + distinct=distinct, + validate_filters=True, + ).run(debug=debug)[0][0] if not filters and cache: frappe.cache().set_value(f"doctype:count:{dt}", count, expires_in_sec=86400) return count @@ -1179,7 +1190,12 @@ class Database: Doctype name can be passed directly, it will be pre-pended with `tab`. """ filters = filters or kwargs.get("conditions") - query = frappe.qb.get_query(table=doctype, filters=filters, delete=True) + query = frappe.qb.get_query( + table=doctype, + filters=filters, + delete=True, + validate_filters=True, + ) if "debug" not in kwargs: kwargs["debug"] = debug return query.run(**kwargs) diff --git a/frappe/database/query.py b/frappe/database/query.py index 02beff9afc..eed52329d2 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -9,6 +9,7 @@ from pypika.queries import QueryBuilder, Table import frappe from frappe import _ from frappe.database.operator_map import OPERATOR_MAP +from frappe.database.schema import SPECIAL_CHAR_PATTERN from frappe.database.utils import DefaultOrderBy, get_doctype_name from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.functions import Function, SqlFunctions @@ -44,6 +45,8 @@ class Engine: update: bool = False, into: bool = False, delete: bool = False, + *, + validate_filters: bool = False, ) -> QueryBuilder: self.is_mariadb = frappe.db.db_type == "mariadb" self.is_postgres = frappe.db.db_type == "postgres" @@ -56,6 +59,8 @@ class Engine: self.validate_doctype() self.table = frappe.qb.DocType(table) + self.validate_filters = validate_filters + if update: self.query = frappe.qb.update(self.table) elif into: @@ -157,14 +162,16 @@ class Engine: _value = value _operator = operator - if isinstance(_field, Field): + if not isinstance(_field, str): pass - elif dynamic_field := DynamicTableField.parse(field, self.doctype): + elif not self.validate_filters and ( + dynamic_field := DynamicTableField.parse(field, self.doctype) + ): # apply implicit join if link field's field is referenced self.query = dynamic_field.apply_join(self.query) _field = dynamic_field.field - elif has_function(field): - _field = self.get_function_object(field) + elif self.validate_filters and SPECIAL_CHAR_PATTERN.search(_field): + frappe.throw(_("Invalid filter: {0}").format(_field)) elif not doctype or doctype == self.doctype: _field = self.table[field] elif doctype: diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 451dc699fe..a8e4841953 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -202,7 +202,11 @@ def get_cards_for_user(doctype, txt, searchfield, start, page_len, filters): if txt: search_conditions = [numberCard[field].like(f"%{txt}%") for field in searchfields] - condition_query = frappe.qb.get_query(doctype, filters=filters) + condition_query = frappe.qb.get_query( + doctype, + filters=filters, + validate_filters=True, + ) return ( condition_query.select(numberCard.name, numberCard.label, numberCard.document_type) diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index 05d45ad9ac..a1db82810e 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -36,7 +36,12 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d ToDo = DocType("ToDo") User = DocType("User") count = Count("*").as_("count") - filtered_records = frappe.qb.get_query(doctype, filters=current_filters, fields=["name"]) + filtered_records = frappe.qb.get_query( + doctype, + filters=current_filters, + fields=["name"], + validate_filters=True, + ) return ( frappe.qb.from_(ToDo) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index dfebf5e890..b3270c7c13 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -218,13 +218,6 @@ class TestQuery(FrappeTestCase): @run_only_if(db_type_is.MARIADB) def test_filters(self): - self.assertEqual( - frappe.qb.get_query( - "User", filters={"IfNull(name, " ")": ("<", Now())}, fields=["Max(name)"] - ).run(), - frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), - ) - self.assertEqual( frappe.qb.get_query( "DocType", diff --git a/frappe/utils/goal.py b/frappe/utils/goal.py index 709fdc1644..01cd9d835e 100644 --- a/frappe/utils/goal.py +++ b/frappe/utils/goal.py @@ -31,6 +31,7 @@ def get_monthly_results( Function(aggregation, goal_field), ], filters=filters, + validate_filters=True, ) .groupby("month_year") .run() From 81d5160ac1e61681e3c8a4c1737e036023919ec3 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 30 May 2023 17:49:51 +0530 Subject: [PATCH 13/32] test: ensure stricter filters when `validate_filters` is passed --- frappe/tests/test_query.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index b3270c7c13..9242630104 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -251,6 +251,17 @@ class TestQuery(FrappeTestCase): ), ) + self.assertRaisesRegex( + frappe.ValidationError, + "Invalid filter", + lambda: frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"permissions.role": "System Manager"}, + validate_filters=True, + ), + ) + self.assertEqual( frappe.qb.get_query( "DocType", From 1b2d1dd5678afca852661aa52406173925c73f7f Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 31 May 2023 14:20:26 +0530 Subject: [PATCH 14/32] chore: move statement to set `validate_filters` property --- frappe/database/query.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index eed52329d2..06295d33a6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -50,6 +50,7 @@ class Engine: ) -> QueryBuilder: self.is_mariadb = frappe.db.db_type == "mariadb" self.is_postgres = frappe.db.db_type == "postgres" + self.validate_filters = validate_filters if isinstance(table, Table): self.table = table @@ -59,8 +60,6 @@ class Engine: self.validate_doctype() self.table = frappe.qb.DocType(table) - self.validate_filters = validate_filters - if update: self.query = frappe.qb.update(self.table) elif into: From 1fd84f6b09efc1ed892a1716f5e4abccce0de7bd Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 31 May 2023 15:42:50 +0530 Subject: [PATCH 15/32] fix: Remove field_id from URL - scroll to field is not yet supported --- frappe/printing/page/print/print.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/printing/page/print/print.js b/frappe/printing/page/print/print.js index 14770c2d26..f930359b58 100644 --- a/frappe/printing/page/print/print.js +++ b/frappe/printing/page/print/print.js @@ -114,7 +114,7 @@ frappe.ui.form.PrintView = class { description = "
" + __("Footer might not be visible as {0} option is disabled
", [ - `${__( + `${__( "Repeat Header and Footer" )}`, ]); From eda8be74ca93a49b70a4d8af63f8bc6ba08343d4 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 31 May 2023 15:44:05 +0530 Subject: [PATCH 16/32] fix: Hide font-size from print format - Since it is only used in new print format builder and it can be set via new print format builder's interface --- frappe/printing/doctype/print_format/print_format.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/printing/doctype/print_format/print_format.json b/frappe/printing/doctype/print_format/print_format.json index 7f4408c950..664692ec45 100644 --- a/frappe/printing/doctype/print_format/print_format.json +++ b/frappe/printing/doctype/print_format/print_format.json @@ -245,6 +245,7 @@ "default": "14", "fieldname": "font_size", "fieldtype": "Int", + "hidden": 1, "label": "Font Size" }, { @@ -258,7 +259,7 @@ "icon": "fa fa-print", "idx": 1, "links": [], - "modified": "2022-11-09 15:29:46.709305", + "modified": "2023-05-31 15:40:52.919029", "modified_by": "Administrator", "module": "Printing", "name": "Print Format", From 48539dc0a338d619c69ded8c49da1c9e2c39903a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 16:00:59 +0530 Subject: [PATCH 17/32] feat: log all DDL queries (#21107) --- frappe/database/database.py | 21 ++++++++++++++++++--- frappe/database/mariadb/database.py | 4 ++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 8b077ce4f7..0a978b2d87 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -4,6 +4,7 @@ import datetime import itertools import json +import logging import random import re import string @@ -105,6 +106,8 @@ class Database: self.password = password or frappe.conf.db_password self.value_cache = {} + self.logger = frappe.logger("database") + self.logger.setLevel("INFO") # self.db_type: str # self.last_query (lazy) attribute of last sql query executed @@ -122,7 +125,7 @@ class Database: if execution_timeout := get_query_execution_timeout(): self.set_execution_timeout(execution_timeout) except Exception as e: - frappe.logger("database").warning(f"Couldn't set execution timeout {e}") + self.logger.warning(f"Couldn't set execution timeout {e}") def set_execution_timeout(self, seconds: int): """Set session speicifc timeout on exeuction of statements. @@ -285,7 +288,13 @@ class Database: return self.convert_to_lists(self.last_result) return self.last_result - def _log_query(self, mogrified_query: str, debug: bool = False, explain: bool = False) -> None: + def _log_query( + self, + mogrified_query: str, + debug: bool = False, + explain: bool = False, + unmogrified_query: str = "", + ) -> None: """Takes the query and logs it to various interfaces according to the settings.""" _query = None @@ -303,6 +312,12 @@ class Database: _query = _query or str(mogrified_query) frappe.log(f"<<<< query\n{_query}\n>>>>") + if unmogrified_query and is_query_type( + unmogrified_query, ("alter", "drop", "select", "create", "truncate", "rename") + ): + _query = _query or str(mogrified_query) + self.logger.info("DDL Query made:\n" + _query) + if frappe.flags.in_migrate: _query = _query or str(mogrified_query) self.log_touched_tables(_query) @@ -314,7 +329,7 @@ class Database: # like cursor._transformed_statement from the cursor object. We can also avoid setting # mogrified_query if we don't need to log it. mogrified_query = self.lazy_mogrify(query, values) - self._log_query(mogrified_query, debug, explain) + self._log_query(mogrified_query, debug, explain, unmogrified_query=query) return mogrified_query def mogrify(self, query: Query, values: QueryValues): diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 8e52cc7ffd..8465751ca4 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -200,8 +200,8 @@ class MariaDBDatabase(MariaDBConnectionUtil, MariaDBExceptionUtil, Database): return db_size[0].get("database_size") def log_query(self, query, values, debug, explain): - self.last_query = query = self._cursor._executed - self._log_query(query, debug, explain) + self.last_query = self._cursor._executed + self._log_query(self.last_query, debug, explain, query) return self.last_query @staticmethod From 950277a88de97590aa690dfa02e5511915e1cbd8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 16:04:56 +0530 Subject: [PATCH 18/32] chore: increase logging level --- frappe/database/database.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 0a978b2d87..faf65b02c1 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -4,7 +4,6 @@ import datetime import itertools import json -import logging import random import re import string @@ -107,7 +106,7 @@ class Database: self.password = password or frappe.conf.db_password self.value_cache = {} self.logger = frappe.logger("database") - self.logger.setLevel("INFO") + self.logger.setLevel("WARNING") # self.db_type: str # self.last_query (lazy) attribute of last sql query executed @@ -316,7 +315,7 @@ class Database: unmogrified_query, ("alter", "drop", "select", "create", "truncate", "rename") ): _query = _query or str(mogrified_query) - self.logger.info("DDL Query made:\n" + _query) + self.logger.warning("DDL Query made to DB:\n" + _query) if frappe.flags.in_migrate: _query = _query or str(mogrified_query) From 4104e7d733dd396def90267d3fb55cd9ad89c04d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 16:28:44 +0530 Subject: [PATCH 19/32] feat: show db table utilization on doctype (#21193) * feat: show db table utilization on doctype * fix: nicer error message with docs --- frappe/core/doctype/doctype/doctype.py | 13 ++++- .../doctype/customize_form/customize_form.py | 13 ++++- frappe/database/database.py | 4 ++ frappe/database/mariadb/database.py | 58 +++++++++++++++++++ frappe/database/postgres/database.py | 4 ++ frappe/public/js/frappe/doctype/index.js | 23 ++++++++ 6 files changed, 113 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 91a317dbff..12545adb4e 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -33,7 +33,7 @@ from frappe.model.meta import Meta from frappe.modules import get_doc_path, make_boilerplate from frappe.modules.import_file import get_file_path from frappe.query_builder.functions import Concat -from frappe.utils import cint, random_string +from frappe.utils import cint, flt, random_string from frappe.website.utils import clear_cache if TYPE_CHECKING: @@ -1751,3 +1751,14 @@ def get_field(doc, fieldname): for field in doc.fields: if field.fieldname == fieldname: return field + + +@frappe.whitelist() +def get_row_size_utilization(doctype: str) -> float: + """Get row size utilization in percentage""" + + frappe.has_permission("DocType", throw=True) + try: + return flt(frappe.db.get_row_size(doctype) / frappe.db.MAX_ROW_SIZE_LIMIT * 100, 2) + except Exception: + return 0.0 diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 42cbf33f4f..9e6b8990d5 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -172,7 +172,18 @@ class CustomizeForm(Document): check_email_append_to(self) if self.flags.update_db: - frappe.db.updatedb(self.doc_type) + try: + frappe.db.updatedb(self.doc_type) + except Exception as e: + if frappe.db.is_db_table_size_limit(e): + frappe.throw( + _("You have hit the row size limit on database table: {0}").format( + "" + "Maximum Number of Fields in a Form" + ), + title=_("Database Table Row Size Limit"), + ) + raise if not hasattr(self, "hide_success") or not self.hide_success: frappe.msgprint(_("{0} updated").format(_(self.doc_type)), alert=True) diff --git a/frappe/database/database.py b/frappe/database/database.py index faf65b02c1..aff9395846 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1283,6 +1283,10 @@ class Database: return get_next_val(*args, **kwargs) + def get_row_size(self, doctype: str) -> int: + """Get estimated max row size of any table in bytes.""" + raise NotImplementedError + def enqueue_jobs_after_commit(): from frappe.utils.background_jobs import ( diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 8465751ca4..f14fce2710 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -76,6 +76,10 @@ class MariaDBExceptionUtil: def is_data_too_long(e: pymysql.Error) -> bool: return e.args[0] == ER.DATA_TOO_LONG + @staticmethod + def is_db_table_size_limit(e: pymysql.Error) -> bool: + return e.args[0] == ER.TOO_BIG_ROWSIZE + @staticmethod def is_primary_key_violation(e: pymysql.Error) -> bool: return ( @@ -145,6 +149,7 @@ class MariaDBDatabase(MariaDBConnectionUtil, MariaDBExceptionUtil, Database): UnicodeWithAttrs: escape_string, } default_port = "3306" + MAX_ROW_SIZE_LIMIT = 65_535 # bytes def setup_type_map(self): self.db_type = "mariadb" @@ -445,3 +450,56 @@ class MariaDBDatabase(MariaDBConnectionUtil, MariaDBExceptionUtil, Database): frappe.cache().set_value("db_tables", tables) return tables + + def get_row_size(self, doctype: str) -> int: + """Get estimated max row size of any table in bytes.""" + + # Query reused from this answer: https://dba.stackexchange.com/a/313889/274503 + # Modification: get values for particular table instead of full summary. + # Reference: https://mariadb.com/kb/en/data-type-storage-requirements/ + + est_row_size = frappe.db.sql( + """ + SELECT SUM(col_sizes.col_size) AS EST_MAX_ROW_SIZE + FROM ( + SELECT + cols.COLUMN_NAME, + CASE cols.DATA_TYPE + WHEN 'tinyint' THEN 1 + WHEN 'smallint' THEN 2 + WHEN 'mediumint' THEN 3 + WHEN 'int' THEN 4 + WHEN 'bigint' THEN 8 + WHEN 'float' THEN IF(cols.NUMERIC_PRECISION > 24, 8, 4) + WHEN 'double' THEN 8 + WHEN 'decimal' THEN ((cols.NUMERIC_PRECISION - cols.NUMERIC_SCALE) DIV 9)*4 + (cols.NUMERIC_SCALE DIV 9)*4 + CEIL(MOD(cols.NUMERIC_PRECISION - cols.NUMERIC_SCALE,9)/2) + CEIL(MOD(cols.NUMERIC_SCALE,9)/2) + WHEN 'bit' THEN (cols.NUMERIC_PRECISION + 7) DIV 8 + WHEN 'year' THEN 1 + WHEN 'date' THEN 3 + WHEN 'time' THEN 3 + CEIL(cols.DATETIME_PRECISION /2) + WHEN 'datetime' THEN 5 + CEIL(cols.DATETIME_PRECISION /2) + WHEN 'timestamp' THEN 4 + CEIL(cols.DATETIME_PRECISION /2) + WHEN 'char' THEN cols.CHARACTER_OCTET_LENGTH + WHEN 'binary' THEN cols.CHARACTER_OCTET_LENGTH + WHEN 'varchar' THEN IF(cols.CHARACTER_OCTET_LENGTH > 255, 2, 1) + cols.CHARACTER_OCTET_LENGTH + WHEN 'varbinary' THEN IF(cols.CHARACTER_OCTET_LENGTH > 255, 2, 1) + cols.CHARACTER_OCTET_LENGTH + WHEN 'tinyblob' THEN 9 + WHEN 'tinytext' THEN 9 + WHEN 'blob' THEN 10 + WHEN 'text' THEN 10 + WHEN 'mediumblob' THEN 11 + WHEN 'mediumtext' THEN 11 + WHEN 'longblob' THEN 12 + WHEN 'longtext' THEN 12 + WHEN 'enum' THEN 2 + WHEN 'set' THEN 8 + ELSE 0 + END AS col_size + FROM INFORMATION_SCHEMA.COLUMNS cols + WHERE cols.TABLE_NAME = %s + ) AS col_sizes;""", + (get_table_name(doctype),), + ) + + if est_row_size: + return int(est_row_size[0][0]) diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 836a689251..2d5b3a893f 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -107,6 +107,10 @@ class PostgresExceptionUtil: def is_data_too_long(e): return getattr(e, "pgcode", None) == STRING_DATA_RIGHT_TRUNCATION + @staticmethod + def is_db_table_size_limit(e) -> bool: + return False + class PostgresDatabase(PostgresExceptionUtil, Database): REGEX_CHARACTER = "~" diff --git a/frappe/public/js/frappe/doctype/index.js b/frappe/public/js/frappe/doctype/index.js index 0dc5fd0a34..a0023164d7 100644 --- a/frappe/public/js/frappe/doctype/index.js +++ b/frappe/public/js/frappe/doctype/index.js @@ -22,6 +22,29 @@ frappe.model.DocTypeController = class DocTypeController extends frappe.ui.form. }; } + refresh() { + this.show_db_utilization(); + } + + show_db_utilization() { + const doctype = this.frm.doc.doc_type || this.frm.doc.name; + frappe + .xcall("frappe.core.doctype.doctype.doctype.get_row_size_utilization", { + doctype, + }) + .then((r) => { + if (r < 50.0) return; + this.frm.dashboard.show_progress( + __("Database Row Size Utilization"), + r, + __( + "Database Table Row Size Utilization: {0}%, this limits number of fields you can add.", + [r] + ) + ); + }); + } + max_attachments() { if (!this.frm.doc.max_attachments) { return; From 3bbe4498a005e38418e489c2a18bcef13d7b7e00 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 16:38:51 +0530 Subject: [PATCH 20/32] feat: allow re-running patches in developer mode Simpler debugging. --- frappe/core/doctype/patch_log/patch_log.js | 4 ++++ frappe/core/doctype/patch_log/patch_log.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/patch_log/patch_log.js b/frappe/core/doctype/patch_log/patch_log.js index 171a1d3a0f..78580a0cb0 100644 --- a/frappe/core/doctype/patch_log/patch_log.js +++ b/frappe/core/doctype/patch_log/patch_log.js @@ -4,5 +4,9 @@ frappe.ui.form.on("Patch Log", { refresh: function (frm) { frm.disable_save(); + + frm.add_custom_button(__("Re-Run Patch"), () => { + frm.call("rerun_patch"); + }); }, }); diff --git a/frappe/core/doctype/patch_log/patch_log.py b/frappe/core/doctype/patch_log/patch_log.py index c7d619017e..284a80df35 100644 --- a/frappe/core/doctype/patch_log/patch_log.py +++ b/frappe/core/doctype/patch_log/patch_log.py @@ -4,11 +4,20 @@ # License: MIT. See LICENSE import frappe +from frappe import _ from frappe.model.document import Document class PatchLog(Document): - pass + @frappe.whitelist() + def rerun_patch(self): + from frappe.modules.patch_handler import run_single + + if not frappe.conf.developer_mode: + frappe.throw(_("Re-running patch is only allowed in developer mode.")) + + run_single(self.patch, force=True) + frappe.msgprint(_("Successfully re-ran patch: {0}").format(self.patch), alert=True) def before_migrate(): From 83e3a20901db227bfa0f0122fa393e0f9a2f02cd Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 31 May 2023 17:50:10 +0530 Subject: [PATCH 21/32] feat: allow clearing web page views --- frappe/website/doctype/web_page_view/web_page_view.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_page_view/web_page_view.py b/frappe/website/doctype/web_page_view/web_page_view.py index bbf2a394a6..b284dc095c 100644 --- a/frappe/website/doctype/web_page_view/web_page_view.py +++ b/frappe/website/doctype/web_page_view/web_page_view.py @@ -9,7 +9,13 @@ from frappe.model.document import Document class WebPageView(Document): - pass + @staticmethod + def clear_old_logs(days=180): + from frappe.query_builder import Interval + from frappe.query_builder.functions import Now + + table = frappe.qb.DocType("Web Page View") + frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) @frappe.whitelist(allow_guest=True) From efff6ebba7a2c71c3339542ca65251a4d0e134fe Mon Sep 17 00:00:00 2001 From: Dhia' Alhaq Shalabi <30384731+dhiashalabi@users.noreply.github.com> Date: Thu, 1 Jun 2023 09:39:25 +0300 Subject: [PATCH 22/32] fix: doctype name localization (#21197) [skip ci] --- frappe/model/base_document.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 811ba5894c..63188e749d 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -530,7 +530,7 @@ class BaseDocument: if not ignore_if_duplicate: frappe.msgprint( - _("{0} {1} already exists").format(self.doctype, frappe.bold(self.name)), + _("{0} {1} already exists").format(_(self.doctype), frappe.bold(self.name)), title=_("Duplicate Name"), indicator="red", ) From f8edb3dc3daab5b5c36ba86d4933fb9821703f7e Mon Sep 17 00:00:00 2001 From: "Jeans K. Real" Date: Thu, 1 Jun 2023 19:10:29 -0600 Subject: [PATCH 23/32] Closing span tags on form_links.html and list_view status field indicator. Also Removed extra calls from multicheck.js --- frappe/public/js/frappe/form/controls/multicheck.js | 2 -- frappe/public/js/frappe/form/templates/form_links.html | 2 +- frappe/public/js/frappe/list/list_view.js | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/multicheck.js b/frappe/public/js/frappe/form/controls/multicheck.js index de4c330ff7..7b980299aa 100644 --- a/frappe/public/js/frappe/form/controls/multicheck.js +++ b/frappe/public/js/frappe/form/controls/multicheck.js @@ -18,13 +18,11 @@ frappe.ui.form.ControlMultiCheck = class ControlMultiCheck extends frappe.ui.for this.$checkbox_area = $(`
`).appendTo( this.wrapper ); - this.refresh(); } refresh() { this.set_options(); this.bind_checkboxes(); - this.refresh_input(); super.refresh(); } diff --git a/frappe/public/js/frappe/form/templates/form_links.html b/frappe/public/js/frappe/form/templates/form_links.html index 57edb69a15..cd423bb238 100644 --- a/frappe/public/js/frappe/form/templates/form_links.html +++ b/frappe/public/js/frappe/form/templates/form_links.html @@ -5,7 +5,7 @@ {% } %}
- {{ __(transactions[i].label) }} + {{ __(transactions[i].label) }}
{% for (let j=0; j < transactions[i].items.length; j++) { %} {% let doctype = transactions[i].items[j]; %} diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 7fd8d2c55c..f009593f6f 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -1029,7 +1029,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { return ` ${__(indicator[0])} - `; + `; } return ""; } From 6fd9c44391e9ec6a89cd6beb3e99d82836a8c123 Mon Sep 17 00:00:00 2001 From: Maharshi Patel <39730881+maharshivpatel@users.noreply.github.com> Date: Fri, 2 Jun 2023 10:53:05 +0530 Subject: [PATCH 24/32] fix: ui tour popover when outside viewport (#21164) * fix: ui tour always popover inside viewport There are times when popover will go outside viewport this updates the style on highlight to make sure it doesn't. * fix: don't run form ui tours on small screens. * Revert "fix: don't run form ui tours on small screens." This reverts commit b11aaf8d182fa07369f17b914e8a0cb3e7327a18. [skip ci] --- frappe/public/js/onboarding_tours/onboarding_tours.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/public/js/onboarding_tours/onboarding_tours.js b/frappe/public/js/onboarding_tours/onboarding_tours.js index df3e3c3894..29790ce5de 100644 --- a/frappe/public/js/onboarding_tours/onboarding_tours.js +++ b/frappe/public/js/onboarding_tours/onboarding_tours.js @@ -29,6 +29,13 @@ frappe.ui.OnboardingTour = class OnboardingTour { step.popover.node.offsetTop + step.options.step_info.offset_y }px`; } + if (step.popover.node.offsetLeft < 0) { + step.popover.node.style.minWidth = "200px"; + step.popover.node.style.maxWidth = `${ + 350 + step.popover.node.offsetLeft + }px`; + step.popover.node.style.left = "0px"; + } if (step.popover.closeBtnNode) { step.popover.closeBtnNode.onclick = () => { this.on_finish && this.on_finish(); From f223bc02490902dfcc32892058f13f343d51fbaf Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 2 Jun 2023 14:26:50 +0530 Subject: [PATCH 25/32] chore: fix formatting in `user.py` --- frappe/core/doctype/user/user.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index c670ec35a6..94ea8b16a0 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -332,7 +332,13 @@ class User(Document): reset_password_template = frappe.db.get_system_setting("reset_password_template") - self.send_login_mail(_("Password Reset"), "password_reset", {"link": link}, now=True, custom_template=reset_password_template) + self.send_login_mail( + _("Password Reset"), + "password_reset", + {"link": link}, + now=True, + custom_template=reset_password_template, + ) def send_welcome_mail_to_user(self): from frappe.utils import get_url @@ -354,7 +360,7 @@ class User(Document): self.send_login_mail( subject, "new_user", - dict( + dict( link=link, site_url=get_url(), ), @@ -386,6 +392,7 @@ class User(Document): if custom_template: from frappe.email.doctype.email_template.email_template import get_email_template + email_template = get_email_template(custom_template, args) subject = email_template.get("subject") content = email_template.get("message") From f65198fba3dd2bc38fb6fe02436fda89f42730c9 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 2 Jun 2023 14:28:04 +0530 Subject: [PATCH 26/32] chore: ignore formatting commit [skip ci] --- .git-blame-ignore-revs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 5a96c3fea8..03efd1d30d 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -34,3 +34,6 @@ c0c5b2ebdddbe8898ce2d5e5365f4931ff73b6bf # db.get_all -> get_all 2eec621e95564c359ad22da79501a855c1f32b03 + +# minor formatting fix in `user.py` +f223bc02490902dfcc32892058f13f343d51fbaf From cad6f938c164e437dd1d9edbeddc430cc33d658e Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 2 Jun 2023 14:09:48 +0530 Subject: [PATCH 27/32] feat: formatter for `Attach` and `Attach Image` fields --- frappe/public/js/frappe/form/formatters.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frappe/public/js/frappe/form/formatters.js b/frappe/public/js/frappe/form/formatters.js index f4371f901b..9739eed8bb 100644 --- a/frappe/public/js/frappe/form/formatters.js +++ b/frappe/public/js/frappe/form/formatters.js @@ -365,8 +365,14 @@ frappe.form.formatters = {
` : ""; }, + Attach: format_attachment_url, + AttachImage: format_attachment_url, }; +function format_attachment_url(url) { + return url ? `${url}` : ""; +} + frappe.form.get_formatter = function (fieldtype) { if (!fieldtype) fieldtype = "Data"; return frappe.form.formatters[fieldtype.replace(/ /g, "")] || frappe.form.formatters.Data; From ac95b7496be9f770481c0873785c3a224e1695e0 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Fri, 2 Jun 2023 16:16:39 +0530 Subject: [PATCH 28/32] fix: do not render custom cards if workspace does not contain content --- frappe/public/js/frappe/views/workspace/workspace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js index 7bb53c65cd..b5fb0e2e54 100644 --- a/frappe/public/js/frappe/views/workspace/workspace.js +++ b/frappe/public/js/frappe/views/workspace/workspace.js @@ -353,7 +353,7 @@ frappe.views.Workspace = class Workspace { let current_page = pages.filter((p) => p.title == page.name)[0]; this.content = current_page && JSON.parse(current_page.content); - this.add_custom_cards_in_content(); + this.content && this.add_custom_cards_in_content(); $(".item-anchor").addClass("disable-click"); From 691cbd6da74d6a17eec62eee42d35970935b0f0a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 2 Jun 2023 16:39:35 +0530 Subject: [PATCH 29/32] chore: remove select from ddl prefix --- frappe/database/database.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 3d2997a5a3..728d1e9584 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -312,7 +312,7 @@ class Database: frappe.log(f"<<<< query\n{_query}\n>>>>") if unmogrified_query and is_query_type( - unmogrified_query, ("alter", "drop", "select", "create", "truncate", "rename") + unmogrified_query, ("alter", "drop", "create", "truncate", "rename") ): _query = _query or str(mogrified_query) self.logger.warning("DDL Query made to DB:\n" + _query) From cb885a86ae3c341f9a66d7a4a4bd5229a5318c86 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 2 Jun 2023 17:32:33 +0530 Subject: [PATCH 30/32] feat: allow clearing view logs These dont need to exist for eternity. Let site admins decide when to drop them. --- frappe/core/doctype/view_log/view_log.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/view_log/view_log.py b/frappe/core/doctype/view_log/view_log.py index 8383af818e..5dde78d007 100644 --- a/frappe/core/doctype/view_log/view_log.py +++ b/frappe/core/doctype/view_log/view_log.py @@ -1,8 +1,15 @@ # Copyright (c) 2018, Frappe Technologies and contributors # License: MIT. See LICENSE +import frappe from frappe.model.document import Document class ViewLog(Document): - pass + @staticmethod + def clear_old_logs(days=180): + from frappe.query_builder import Interval + from frappe.query_builder.functions import Now + + table = frappe.qb.DocType("View Log") + frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) From 5185374f72681e9e7980f62bf3e6913d29a091ea Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 2 Jun 2023 21:57:58 +0530 Subject: [PATCH 31/32] refactor!: Remove dynamic addition of `_comments` (#21217) This isn't required anymore, was added to handle old sites. --- frappe/app.py | 3 --- frappe/core/doctype/comment/comment.py | 17 +---------------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index fab8facd3f..55855efaf9 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -19,7 +19,6 @@ import frappe.recorder import frappe.utils.response from frappe import _ from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest -from frappe.core.doctype.comment.comment import update_comments_in_parent_after_request from frappe.middlewares import StaticDataMiddleware from frappe.utils import cint, get_site_name, sanitize_html from frappe.utils.error import make_error_snapshot @@ -351,8 +350,6 @@ def sync_database(rollback: bool) -> bool: frappe.db.commit() rollback = False - update_comments_in_parent_after_request() - return rollback diff --git a/frappe/core/doctype/comment/comment.py b/frappe/core/doctype/comment/comment.py index dff13e1170..c86c7811ad 100644 --- a/frappe/core/doctype/comment/comment.py +++ b/frappe/core/doctype/comment/comment.py @@ -152,14 +152,9 @@ def update_comments_in_parent(reference_doctype, reference_name, _comments): except Exception as e: if frappe.db.is_column_missing(e) and getattr(frappe.local, "request", None): - # missing column and in request, add column and update after commit - frappe.local._comments = getattr(frappe.local, "_comments", []) + [ - (reference_doctype, reference_name, _comments) - ] - + pass elif frappe.db.is_data_too_long(e): raise frappe.DataTooLongException - else: raise else: @@ -169,13 +164,3 @@ def update_comments_in_parent(reference_doctype, reference_name, _comments): # Clear route cache if route := frappe.get_cached_value(reference_doctype, reference_name, "route"): clear_cache(route) - - -def update_comments_in_parent_after_request(): - """update _comments in parent if _comments column is missing""" - if hasattr(frappe.local, "_comments"): - for (reference_doctype, reference_name, _comments) in frappe.local._comments: - add_column(reference_doctype, "_comments", "Text") - update_comments_in_parent(reference_doctype, reference_name, _comments) - - frappe.db.commit() From 042595ca926afb523e6faf759af926e3411ceb47 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Sat, 3 Jun 2023 16:52:51 +0530 Subject: [PATCH 32/32] fix: handle multiple webform for same doctype --- frappe/website/doctype/web_form/web_form.json | 31 +++++++++++++++---- frappe/website/doctype/web_form/web_form.py | 14 ++++++--- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/frappe/website/doctype/web_form/web_form.json b/frappe/website/doctype/web_form/web_form.json index 96749e460d..e0883ba439 100644 --- a/frappe/website/doctype/web_form/web_form.json +++ b/frappe/website/doctype/web_form/web_form.json @@ -31,6 +31,10 @@ "allow_incomplete", "section_break_2", "max_attachment_size", + "section_break_xzqr", + "condition", + "column_break_tjgl", + "condition_description", "section_break_3", "list_setting_message", "show_list", @@ -279,10 +283,6 @@ "fieldtype": "Tab Break", "label": "Form" }, - { - "fieldname": "column_break_1", - "fieldtype": "Column Break" - }, { "fieldname": "section_break_1", "fieldtype": "Section Break" @@ -297,7 +297,6 @@ "fieldtype": "Column Break" }, { - "collapsible": 1, "fieldname": "section_break_2", "fieldtype": "Section Break" }, @@ -374,13 +373,33 @@ "fieldname": "anonymous", "fieldtype": "Check", "label": "Anonymous" + }, + { + "fieldname": "condition", + "fieldtype": "Code", + "label": "Condition", + "max_height": "150px" + }, + { + "fieldname": "section_break_xzqr", + "fieldtype": "Section Break" + }, + { + "fieldname": "column_break_tjgl", + "fieldtype": "Column Break" + }, + { + "fieldname": "condition_description", + "fieldtype": "HTML", + "label": "Condition Description", + "options": "

Multiple webforms can be created for a single doctype. Write a condition specific to this webform to display correct record after submission.

For Example:

\n

If you create a separate webform every year to capture feedback from employees add a \n field named year in doctype and add a condition doc.year==\"2023\"

\n" } ], "has_web_view": 1, "icon": "icon-edit", "is_published_field": "published", "links": [], - "modified": "2023-04-20 17:24:42.657731", + "modified": "2023-06-03 19:18:56.760479", "modified_by": "Administrator", "module": "Website", "name": "Web Form", diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 3e2705bdbe..81c6001558 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -153,10 +153,16 @@ def get_context(context): and not frappe.form_dict.name and not frappe.form_dict.is_list ): - name = frappe.db.get_value(self.doc_type, {"owner": frappe.session.user}, "name") - if name: - context.in_view_mode = True - frappe.redirect(f"/{self.route}/{name}") + names = frappe.db.get_values(self.doc_type, {"owner": frappe.session.user}, pluck="name") + for name in names: + if self.condition: + doc = frappe.get_doc(self.doc_type, name) + if frappe.safe_eval(self.condition, None, {"doc": doc.as_dict()}): + context.in_view_mode = True + frappe.redirect(f"/{self.route}/{name}") + else: + context.in_view_mode = True + frappe.redirect(f"/{self.route}/{name}") # Show new form when # - User is Guest