From 983a9a6ef9ad20604569a14fb3f887b9f34030c0 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 9 Jun 2023 12:20:10 +0200 Subject: [PATCH 01/72] fix(Data Import): cast selected value to string --- frappe/core/doctype/data_import/importer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 0b983d0be9..e1fc02d072 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -690,7 +690,7 @@ class Row: df = col.df if df.fieldtype == "Select": select_options = get_select_options(df) - if select_options and value not in select_options: + if select_options and cstr(value) not in select_options: options_string = ", ".join(frappe.bold(d) for d in select_options) msg = _("Value must be one of {0}").format(options_string) self.warnings.append( From b0e8a75ee6de72034db83e224cd381584be6edc9 Mon Sep 17 00:00:00 2001 From: Marco Fonseca <62901164+treasuryesc@users.noreply.github.com> Date: Tue, 13 Jun 2023 22:59:40 -0300 Subject: [PATCH 02/72] fix: "Remember Last Selected Value" not working (frappe#21364) --- frappe/public/js/frappe/model/create_new.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/model/create_new.js b/frappe/public/js/frappe/model/create_new.js index 523cb3b7a6..e313f76113 100644 --- a/frappe/public/js/frappe/model/create_new.js +++ b/frappe/public/js/frappe/model/create_new.js @@ -163,7 +163,8 @@ $.extend(frappe.model, { if (!user_default) { user_default = frappe.defaults.get_user_default(df.fieldname); - } else if ( + } + if ( !user_default && df.remember_last_selected_value && frappe.boot.user.last_selected_values From d1aea9981aa723c2a14314c35f9d7884dc931222 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Wed, 14 Jun 2023 12:10:52 +0530 Subject: [PATCH 03/72] fix: re-delete doc if dependent doc is deleted --- frappe/desk/reportview.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 326e9bb864..290ad8466c 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -479,6 +479,7 @@ def delete_items(): def delete_bulk(doctype, items): + undeleted_items = [] for i, d in enumerate(items): try: frappe.delete_doc(doctype, d) @@ -493,7 +494,10 @@ def delete_bulk(doctype, items): except Exception: # rollback if any record failed to delete # if not rollbacked, queries get committed on after_request method in app.py + undeleted_items.append(d) frappe.db.rollback() + if undeleted_items and len(items) != len(undeleted_items): + delete_bulk(doctype, undeleted_items) @frappe.whitelist() From fd65fe8625a52b92039e31fae56ba24f8082f0d8 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Wed, 14 Jun 2023 12:11:21 +0530 Subject: [PATCH 04/72] fix: clear throw message --- frappe/desk/reportview.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 290ad8466c..071b6e7e61 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -497,6 +497,7 @@ def delete_bulk(doctype, items): undeleted_items.append(d) frappe.db.rollback() if undeleted_items and len(items) != len(undeleted_items): + frappe.clear_messages() delete_bulk(doctype, undeleted_items) From f0cf70f9ebac9de1454d51b8bfddbb1ae515eb9b Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Wed, 14 Jun 2023 13:24:20 +0530 Subject: [PATCH 05/72] fix: bring progress dailog above freeze screen --- frappe/public/js/frappe/ui/messages.js | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/public/js/frappe/ui/messages.js b/frappe/public/js/frappe/ui/messages.js index c2d74ee7a3..16096e1845 100644 --- a/frappe/public/js/frappe/ui/messages.js +++ b/frappe/public/js/frappe/ui/messages.js @@ -376,6 +376,7 @@ frappe.show_progress = (title, count, total = 100, description, hide_on_completi // timeout to avoid abrupt hide setTimeout(frappe.hide_progress, 500); } + frappe.cur_progress.$wrapper.css("z-index", 2000); return dialog; }; From fcce713b14367ec8c080ea7594df6032137c2920 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 14 Jun 2023 21:24:54 +0530 Subject: [PATCH 06/72] perf: index for_user in Notification log (#21379) Missed out before somehow: https://github.com/frappe/frappe/commit/3a5a45d8af6e86742b466e06702d74cc8bade7e3 --- .../desk/doctype/notification_log/notification_log.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/desk/doctype/notification_log/notification_log.json b/frappe/desk/doctype/notification_log/notification_log.json index f24a6447b4..bafe28faf8 100644 --- a/frappe/desk/doctype/notification_log/notification_log.json +++ b/frappe/desk/doctype/notification_log/notification_log.json @@ -29,7 +29,8 @@ "fieldtype": "Link", "hidden": 1, "label": "For User", - "options": "User" + "options": "User", + "search_index": 1 }, { "fieldname": "type", @@ -64,8 +65,7 @@ "fieldtype": "Link", "hidden": 1, "label": "From User", - "options": "User", - "search_index": 1 + "options": "User" }, { "default": "0", @@ -96,7 +96,7 @@ "hide_toolbar": 1, "in_create": 1, "links": [], - "modified": "2022-09-13 16:08:48.153934", + "modified": "2023-06-14 21:20:51.197943", "modified_by": "Administrator", "module": "Desk", "name": "Notification Log", From 4ec2d5f00d5879699fc37f91b60cc0f52874d404 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 15 Jun 2023 12:48:55 +0530 Subject: [PATCH 07/72] fix: commit after successfull deletion of all tasks --- frappe/desk/reportview.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 071b6e7e61..6efd7d8658 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -489,8 +489,6 @@ def delete_bulk(doctype, items): dict(progress=[i + 1, len(items)], title=_("Deleting {0}").format(doctype), description=d), user=frappe.session.user, ) - # Commit after successful deletion - frappe.db.commit() except Exception: # rollback if any record failed to delete # if not rollbacked, queries get committed on after_request method in app.py @@ -499,6 +497,9 @@ def delete_bulk(doctype, items): if undeleted_items and len(items) != len(undeleted_items): frappe.clear_messages() delete_bulk(doctype, undeleted_items) + else: + # Commit after successful deletion + frappe.db.commit() @frappe.whitelist() From f6799307903a652895ff62824907b3c364bf075d Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 15 Jun 2023 14:07:53 +0530 Subject: [PATCH 08/72] revert: last commit --- frappe/desk/reportview.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 6efd7d8658..071b6e7e61 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -489,6 +489,8 @@ def delete_bulk(doctype, items): dict(progress=[i + 1, len(items)], title=_("Deleting {0}").format(doctype), description=d), user=frappe.session.user, ) + # Commit after successful deletion + frappe.db.commit() except Exception: # rollback if any record failed to delete # if not rollbacked, queries get committed on after_request method in app.py @@ -497,9 +499,6 @@ def delete_bulk(doctype, items): if undeleted_items and len(items) != len(undeleted_items): frappe.clear_messages() delete_bulk(doctype, undeleted_items) - else: - # Commit after successful deletion - frappe.db.commit() @frappe.whitelist() From 09501da12f9f6c9654e44ebdd84038123e6c39f3 Mon Sep 17 00:00:00 2001 From: gavin Date: Thu, 15 Jun 2023 17:45:10 +0530 Subject: [PATCH 09/72] fix(meta): Revert to using conf over _dev_server for cache (#21391) --- frappe/desk/form/meta.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index 4f664658ad..3ad65f7b13 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -36,7 +36,7 @@ ASSET_KEYS = ( def get_meta(doctype, cached=True) -> "FormMeta": # don't cache for developer mode as js files, templates may be edited - cached = cached and not frappe._dev_server + cached = cached and not frappe.conf.developer_mode if cached: meta = frappe.cache.hget("doctype_form_meta", doctype) if not meta: From 85cef317f2bea7ecf81805309d6d0399cdd357bd Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 15 Jun 2023 18:03:09 +0530 Subject: [PATCH 10/72] perf: Index `everyone` in DocShare A typical share query looks like this. select `share_name` from `tabDocShare` where `tabDocShare`.`read` = 1.0 and `tabDocShare`.`share_doctype` = 'Pick List' and (`tabDocShare`.`user` = 'username' or `tabDocShare`.`everyone` = 1.0) order by `tabDocShare`.`modified` DESC; None of existing indexes provide `everyone` values quickly, so `OR` clause effectively clauses full table scan ALL THE TIME. --- frappe/core/doctype/docshare/docshare.json | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/docshare/docshare.json b/frappe/core/doctype/docshare/docshare.json index ca10b05dac..e3581f516e 100644 --- a/frappe/core/doctype/docshare/docshare.json +++ b/frappe/core/doctype/docshare/docshare.json @@ -67,7 +67,8 @@ "default": "0", "fieldname": "everyone", "fieldtype": "Check", - "label": "Everyone" + "label": "Everyone", + "search_index": 1 }, { "default": "1", @@ -85,10 +86,11 @@ ], "in_create": 1, "links": [], - "modified": "2021-04-04 11:38:50.813312", + "modified": "2023-06-15 18:02:51.877533", "modified_by": "Administrator", "module": "Core", "name": "DocShare", + "naming_rule": "Random", "owner": "Administrator", "permissions": [ { @@ -106,5 +108,6 @@ "read_only": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file From e9645b759fe32a6aee732b1eb9c7569fefe22422 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 15 Jun 2023 18:07:06 +0530 Subject: [PATCH 11/72] perf: Dont order_by while fetching shares Unnecessary confusion to query planner --- frappe/share.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/share.py b/frappe/share.py index adae95ea23..6c2fb356a6 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -161,7 +161,7 @@ def get_shared(doctype, user=None, rights=None): or_filters += [["everyone", "=", 1]] shared_docs = frappe.get_all( - "DocShare", fields=["share_name"], filters=filters, or_filters=or_filters + "DocShare", fields=["share_name"], filters=filters, or_filters=or_filters, order_by=None ) return [doc.share_name for doc in shared_docs] From 3e9a2d01e3e76b416456b2816682631e1bb0e37e Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 15 Jun 2023 16:15:55 +0200 Subject: [PATCH 12/72] fix: sort options in group by field --- frappe/public/js/frappe/ui/group_by/group_by.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/ui/group_by/group_by.js b/frappe/public/js/frappe/ui/group_by/group_by.js index f48aba1b7f..0430915782 100644 --- a/frappe/public/js/frappe/ui/group_by/group_by.js +++ b/frappe/public/js/frappe/ui/group_by/group_by.js @@ -367,7 +367,9 @@ frappe.ui.GroupBy = class { ["Select", "Link", "Data", "Int", "Check"].includes(f.fieldtype) ); const tag_field = { fieldname: "_user_tags", fieldtype: "Data", label: __("Tags") }; - this.group_by_fields[this.doctype] = fields.concat(tag_field); + this.group_by_fields[this.doctype] = fields + .concat(tag_field) + .sort((a, b) => __(a.label).localeCompare(__(b.label))); this.all_fields[this.doctype] = this.report_view.meta.fields; const standard_fields_filter = (df) => @@ -379,7 +381,8 @@ frappe.ui.GroupBy = class { const cdt = df.options; const child_table_fields = frappe.meta .get_docfields(cdt) - .filter(standard_fields_filter); + .filter(standard_fields_filter) + .sort((a, b) => __(a.label).localeCompare(__(b.label))); this.group_by_fields[cdt] = child_table_fields; this.all_fields[cdt] = child_table_fields; }); From a2a6eaa15f1da35b69e3025417ce6bb569f27768 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 15 Jun 2023 16:16:54 +0200 Subject: [PATCH 13/72] fix: german translations --- frappe/translations/de.csv | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index d1f503be7c..f8bbd8e22e 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -1208,6 +1208,7 @@ Google Calendar ID,Google Kalender-ID, Google Font,Google Font, Google Services,Google-Dienste, Grant Type,Grant Typ, +Group By {0},Gruppieren nach {0}, Group Name,Gruppenname, Group name cannot be empty.,Der Gruppenname darf nicht leer sein., Groups of DocTypes,Gruppen von DocTypes, @@ -3238,6 +3239,7 @@ Change User,Benutzer wechseln, Check the Error Log for more information: {0},Überprüfen Sie das Fehlerprotokoll auf weitere Informationen: {0}, Clear Cache and Reload,Cache leeren und neu laden, Clear Filters,Filter löschen, +Clear all filters,Alle Filter löschen, Click on Authorize Google Drive Access to authorize Google Drive Access.,"Klicken Sie auf Google Drive Access autorisieren, um Google Drive Access zu autorisieren.", Click on a file to select it.,"Klicken Sie auf eine Datei, um sie auszuwählen.", Click on the link below to approve the request,"Klicken Sie auf den folgenden Link, um die Anfrage zu genehmigen", @@ -3559,7 +3561,7 @@ Select Field...,Feld auswählen ..., Select Filters,Wählen Sie Filter, Select Google Calendar to which event should be synced.,"Wählen Sie Google Kalender aus, mit dem das Ereignis synchronisiert werden soll.", Select Google Contacts to which contact should be synced.,"Wählen Sie Google-Kontakte aus, mit denen der Kontakt synchronisiert werden soll.", -Select Group By...,Wählen Sie Gruppieren nach ..., +Select Group By...,Gruppieren nach ..., Select Mandatory,Verpflichtende auswählen, Select atleast 2 actions,Wählen Sie mindestens 2 Aktionen aus, Select list item,Listenelement auswählen, From d35099320f389112295162182e4c74bbd38d6ac4 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 15 Jun 2023 16:27:22 +0200 Subject: [PATCH 14/72] fix: make field label translatable --- frappe/public/js/frappe/ui/group_by/group_by.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/ui/group_by/group_by.js b/frappe/public/js/frappe/ui/group_by/group_by.js index 0430915782..dcfdd91d40 100644 --- a/frappe/public/js/frappe/ui/group_by/group_by.js +++ b/frappe/public/js/frappe/ui/group_by/group_by.js @@ -324,9 +324,9 @@ frappe.ui.GroupBy = class { ); if (this.aggregate_function === "sum") { - docfield.label = __("Sum of {0}", [docfield.label]); + docfield.label = __("Sum of {0}", [__(docfield.label)]); } else { - docfield.label = __("Average of {0}", [docfield.label]); + docfield.label = __("Average of {0}", [__(docfield.label)]); } } From 7710dbb8bbae8a65b569dcf34cdfe26c68d57991 Mon Sep 17 00:00:00 2001 From: Corentin Flr <10946971+cogk@users.noreply.github.com> Date: Fri, 16 Jun 2023 00:28:34 +0200 Subject: [PATCH 15/72] feat(icons): Add new icon arrow-down-right for Number Card --- frappe/public/icons/timeless/icons.svg | 5 +++++ frappe/public/js/frappe/widgets/number_card_widget.js | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/public/icons/timeless/icons.svg b/frappe/public/icons/timeless/icons.svg index f5a34a14c9..bd1125a7a2 100644 --- a/frappe/public/icons/timeless/icons.svg +++ b/frappe/public/icons/timeless/icons.svg @@ -93,6 +93,11 @@ + + + + + diff --git a/frappe/public/js/frappe/widgets/number_card_widget.js b/frappe/public/js/frappe/widgets/number_card_widget.js index af89f95197..26b1dabd35 100644 --- a/frappe/public/js/frappe/widgets/number_card_widget.js +++ b/frappe/public/js/frappe/widgets/number_card_widget.js @@ -238,7 +238,7 @@ export default class NumberCardWidget extends Widget { color_class = "green-stat"; } else { caret_html = ` - ${frappe.utils.icon("arrow-down-left", "xs")} + ${frappe.utils.icon("arrow-down-right", "xs")} `; color_class = "red-stat"; } From e586a1e5cfc690bc4833f7eeaeac182fa5fa29e1 Mon Sep 17 00:00:00 2001 From: Corentin Flr <10946971+cogk@users.noreply.github.com> Date: Fri, 16 Jun 2023 00:50:11 +0200 Subject: [PATCH 16/72] perf(icons): Remove uneeded code on some arrow icons --- frappe/public/icons/timeless/icons.svg | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/frappe/public/icons/timeless/icons.svg b/frappe/public/icons/timeless/icons.svg index bd1125a7a2..29aa428d99 100644 --- a/frappe/public/icons/timeless/icons.svg +++ b/frappe/public/icons/timeless/icons.svg @@ -84,18 +84,15 @@ - - + - - + - - + From 9a963bedb5c3c99feb73366f959762aba6c4c4a9 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 10:23:05 +0530 Subject: [PATCH 17/72] test: Fix failing test for icon & phone - for some reason findBYRole("searchbox") stopped working - there are no changes to related DOM recently - There's pending issue related to this https://github.com/testing-library/cypress-testing-library/issues/205#issuecomment-1572230102 --- cypress/integration/control_icon.js | 4 ++-- cypress/integration/control_phone.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cypress/integration/control_icon.js b/cypress/integration/control_icon.js index a965ed0f9e..406e9f1162 100644 --- a/cypress/integration/control_icon.js +++ b/cypress/integration/control_icon.js @@ -42,14 +42,14 @@ context("Control Icon", () => { it("search for icon and clear search input", () => { let search_text = "ed"; - cy.get(".icon-picker").findByRole("searchbox").click().type(search_text); + cy.get(".icon-picker").get(".search-icons > input").click().type(search_text); cy.get(".icon-section .icon-wrapper:not(.hidden)").then((i) => { cy.get(`.icon-section .icon-wrapper[id*='${search_text}']`).then((icons) => { expect(i.length).to.equal(icons.length); }); }); - cy.get(".icon-picker").findByRole("searchbox").clear().blur(); + cy.get(".icon-picker").get(".search-icons > input").clear().blur(); cy.get(".icon-section .icon-wrapper").should("not.have.class", "hidden"); }); }); diff --git a/cypress/integration/control_phone.js b/cypress/integration/control_phone.js index b56343c2d8..b5b29fe758 100644 --- a/cypress/integration/control_phone.js +++ b/cypress/integration/control_phone.js @@ -47,7 +47,7 @@ context("Control Phone", () => { it("case insensitive search for country and clear search", () => { let search_text = "india"; cy.get(".selected-phone").click().first(); - cy.get(".phone-picker").findByRole("searchbox").click().type(search_text); + cy.get(".phone-picker").get(".search-phones").click().type(search_text); cy.get(".phone-section .phone-wrapper:not(.hidden)").then((i) => { cy.get(`.phone-section .phone-wrapper[id*="${search_text.toLowerCase()}"]`).then( (countries) => { @@ -56,7 +56,7 @@ context("Control Phone", () => { ); }); - cy.get(".phone-picker").findByRole("searchbox").clear().blur(); + cy.get(".phone-picker").get(".search-phones").clear(); cy.get(".phone-section .phone-wrapper").should("not.have.class", "hidden"); }); From e8ebf19308310b08f9c09a6b05dd4dfa0eb72ab0 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 10:30:02 +0530 Subject: [PATCH 18/72] style: Fix formatting issue --- frappe/public/js/frappe/model/create_new.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/model/create_new.js b/frappe/public/js/frappe/model/create_new.js index e313f76113..5deccaeb9b 100644 --- a/frappe/public/js/frappe/model/create_new.js +++ b/frappe/public/js/frappe/model/create_new.js @@ -163,7 +163,7 @@ $.extend(frappe.model, { if (!user_default) { user_default = frappe.defaults.get_user_default(df.fieldname); - } + } if ( !user_default && df.remember_last_selected_value && From 91afe51c7c67d59235bbbd5bad8679dbc1c346ea Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 10:58:27 +0530 Subject: [PATCH 19/72] test: Make folder navigation test less flaky --- cypress/integration/folder_navigation.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cypress/integration/folder_navigation.js b/cypress/integration/folder_navigation.js index ba65454ef6..4a32c16516 100644 --- a/cypress/integration/folder_navigation.js +++ b/cypress/integration/folder_navigation.js @@ -11,9 +11,9 @@ context("Folder Navigation", () => { cy.click_filter_button(); cy.get(".filter-action-buttons > .text-muted").findByText("+ Add a Filter").click(); cy.get(".fieldname-select-area > .awesomplete > .form-control:last").type("Fol{enter}"); - cy.get( - ".filter-field > .form-group > .link-field > .awesomplete > .input-with-feedback" - ).type("Home{enter}"); + cy.get(".filter-field > .form-group > .link-field > .awesomplete > .input-with-feedback") + .first() + .type("Home{enter}"); cy.get(".filter-action-buttons > div > .btn-primary").findByText("Apply Filters").click(); //Adding folder (Test Folder) @@ -24,6 +24,7 @@ context("Folder Navigation", () => { it("Navigating the nested folders, checking if the URL formed is correct, checking if the added content in the child folder is correct", () => { //Navigating inside the Attachments folder + cy.clear_filters(); cy.wait(500); cy.get('[title="Attachments"] > span').click(); From 6c3957aa302f4b2ea42ae6757f577015bda7a616 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 11:01:21 +0530 Subject: [PATCH 20/72] test: Return cy.request promise from login - to make cypress wait for the promise. --- cypress/support/commands.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 4b44a24598..3ad9aa090a 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -34,7 +34,7 @@ Cypress.Commands.add("login", (email, password) => { if (!password) { password = Cypress.env("adminPassword"); } - cy.request({ + return cy.request({ url: "/api/method/login", method: "POST", body: { From 49d7f132dad3e99e4bfff6c773bcf05d0f6f9a9a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 11:32:55 +0530 Subject: [PATCH 21/72] test: Fix control link flaky test --- cypress/integration/control_link.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cypress/integration/control_link.js b/cypress/integration/control_link.js index d3462492f6..0746f4460e 100644 --- a/cypress/integration/control_link.js +++ b/cypress/integration/control_link.js @@ -133,8 +133,7 @@ context("Control Link", () => { true ); - cy.clear_cache(); - cy.wait(500); + cy.reload(); get_dialog_with_link().as("dialog"); cy.window() @@ -177,7 +176,7 @@ context("Control Link", () => { cy.intercept("POST", "/api/method/frappe.client.validate_link").as("validate_link"); cy.get(".frappe-control[data-fieldname=assigned_by] input").focus().as("input"); - cy.get("@input").type(cy.config("testUser"), { delay: 100 }).blur(); + cy.get("@input").clear().type(cy.config("testUser"), { delay: 300 }).blur(); cy.wait("@validate_link"); cy.get(".frappe-control[data-fieldname=assigned_by_full_name] .control-value").should( "contain", From 8008afaf779482ed94b11bfdb5f6d7615b16dedc Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 11:53:35 +0530 Subject: [PATCH 22/72] test: Fix Kanban test --- cypress/support/commands.js | 1 + 1 file changed, 1 insertion(+) diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 3ad9aa090a..319d6d5840 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -374,6 +374,7 @@ Cypress.Commands.add("update_doc", (doctype, docname, args) => { Cypress.Commands.add("switch_to_user", (user) => { cy.call("logout"); cy.login(user); + cy.reload(); }); Cypress.Commands.add("add_role", (user, role) => { From 5b52350ae0b8acd75ee3d611ef1d287b7b5a496a Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Fri, 16 Jun 2023 12:27:30 +0530 Subject: [PATCH 23/72] fix(build): Exit build process instead of throwing an exception --- esbuild/esbuild.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index 8c386c86e4..e1594aa651 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -89,7 +89,7 @@ execute() .then(() => RUN_BUILD_COMMAND && run_build_command_for_apps(APPS)) .catch((e) => { console.error(e); - throw e; + process.exit(1); }); if (WATCH_MODE) { From 821f75c3ad43fab3e79cfed6c46b8d60add74bc6 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 16 Jun 2023 12:47:39 +0530 Subject: [PATCH 24/72] fix: Try waiting for logout --- cypress/integration/kanban.js | 10 +++++----- cypress/support/commands.js | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cypress/integration/kanban.js b/cypress/integration/kanban.js index f14c991c7c..12c5e7e8bd 100644 --- a/cypress/integration/kanban.js +++ b/cypress/integration/kanban.js @@ -100,15 +100,15 @@ context("Kanban Board", () => { it("Checks if Kanban Board edits are blocked for non-System Manager and non-owner of the Board", () => { cy.switch_to_user("Administrator"); - const noSystemManager = "nosysmanager@example.com"; + const not_system_manager = "nosysmanager@example.com"; cy.call("frappe.tests.ui_test_helpers.create_test_user", { - username: noSystemManager, + username: not_system_manager, }); - cy.remove_role(noSystemManager, "System Manager"); + cy.remove_role(not_system_manager, "System Manager"); cy.call("frappe.tests.ui_test_helpers.create_todo", { description: "Frappe User ToDo" }); cy.call("frappe.tests.ui_test_helpers.create_admin_kanban"); - cy.switch_to_user(noSystemManager); + cy.switch_to_user(not_system_manager); cy.visit("/app/todo/view/kanban/Admin Kanban"); @@ -125,7 +125,7 @@ context("Kanban Board", () => { cy.get(".kanban .column-options").should("have.length", 0); cy.switch_to_user("Administrator"); - cy.call("frappe.client.delete", { doctype: "User", name: noSystemManager }); + cy.call("frappe.client.delete", { doctype: "User", name: not_system_manager }); }); after(() => { diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 319d6d5840..23b03549fa 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -373,6 +373,7 @@ Cypress.Commands.add("update_doc", (doctype, docname, args) => { Cypress.Commands.add("switch_to_user", (user) => { cy.call("logout"); + cy.wait(200); cy.login(user); cy.reload(); }); From 6303e53edadfcd4085ff0c1e64bf58207b7e09cd Mon Sep 17 00:00:00 2001 From: gavin Date: Fri, 16 Jun 2023 12:54:05 +0530 Subject: [PATCH 25/72] fix(desk): Allow setting filters_description via df API (#21382) --- frappe/public/js/frappe/form/controls/link.js | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index b187135181..cb6beadb7a 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -267,15 +267,17 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat r.results = me.merge_duplicates(r.results); // show filter description in awesomplete - if (args.filters) { - let filter_string = me.get_filter_description(args.filters); - if (filter_string) { - r.results.push({ - html: `${filter_string}`, - value: "", - action: () => {}, - }); - } + let filter_string = me.df.filter_description + ? me.df.filter_description + : args.filters + ? me.get_filter_description(args.filters) + : null; + if (filter_string) { + r.results.push({ + html: `${filter_string}`, + value: "", + action: () => {}, + }); } if (!me.df.only_select) { From ae0edd85fe42162e4810aa5abf7f5181c88e8125 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 17 Jun 2023 20:32:13 +0530 Subject: [PATCH 26/72] perf: Faster report exporting logic (#21415) * perf: Faster report exports to Excel Try exporting report with 100K rows, most likely it fails or takes a really long time. Root causes: - visible_idx was a list, lookups are SLOW AF when lists grow to 100k+ - visible_idx check is not required when I am exporting entire report. Test with 85,000 rows. | | Before | After | Improvement | | --- | --- | --- | --- | | Export | 25.99 | 0.77 | ~33x faster | --- frappe/desk/query_report.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 5b7c450ae9..3d54520356 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -349,6 +349,13 @@ def build_xlsx_data(data, visible_idx, include_indentation, ignore_visible_idx=F datetime.timedelta, ) + if len(visible_idx) == len(data.result): + # It's not possible to have same length and different content. + ignore_visible_idx = True + else: + # Note: converted for faster lookups + visible_idx = set(visible_idx) + result = [[]] column_widths = [] From e94cdc05152c76f3c6c92980528aa0d2b5acd555 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sat, 17 Jun 2023 20:48:00 -0500 Subject: [PATCH 27/72] fix: section fields_dict the previous code was an oversight as fieldobj.fieldname is undefined --- frappe/public/js/frappe/form/section.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js index b4908e749a..5acae84f46 100644 --- a/frappe/public/js/frappe/form/section.js +++ b/frappe/public/js/frappe/form/section.js @@ -84,7 +84,7 @@ export default class Section { add_field(fieldobj) { this.fields_list.push(fieldobj); - this.fields_dict[fieldobj.fieldname] = fieldobj; + this.fields_dict[fieldobj.df.fieldname] = fieldobj; fieldobj.section = this; } From 62a3a70bf8d81445648928003f335e532e92ad7c Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Sun, 18 Jun 2023 11:23:03 -0400 Subject: [PATCH 28/72] feat: webhook timeout (#21410) * feat: webhook timeout * fix: ensure default timeout 5 seconds Co-authored-by: Ankush Menat --- frappe/integrations/doctype/webhook/webhook.json | 11 ++++++++++- frappe/integrations/doctype/webhook/webhook.py | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json index 404e0be944..1728da97d7 100644 --- a/frappe/integrations/doctype/webhook/webhook.json +++ b/frappe/integrations/doctype/webhook/webhook.json @@ -17,6 +17,7 @@ "html_condition", "sb_webhook", "request_url", + "timeout", "is_dynamic_url", "cb_webhook", "request_method", @@ -204,6 +205,14 @@ "fieldname": "is_dynamic_url", "fieldtype": "Check", "label": "Is Dynamic URL?" + }, + { + "default": "5", + "description": "The number of seconds until the request expires", + "fieldname": "timeout", + "fieldtype": "Int", + "label": "Request Timeout", + "reqd": 1 } ], "links": [ @@ -212,7 +221,7 @@ "link_fieldname": "webhook" } ], - "modified": "2023-06-02 17:25:12.598232", + "modified": "2023-06-16 10:21:00.971833", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook", diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index a4d198a118..6fa24bfb67 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -129,7 +129,7 @@ def enqueue_webhook(doc, webhook) -> None: url=request_url, data=json.dumps(data, default=str), headers=headers, - timeout=5, + timeout=webhook.timeout or 5, ) r.raise_for_status() frappe.logger().debug({"webhook_success": r.text}) From 727059a2d115bc37ac3732df0de01ef8e6076a1f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 19 Jun 2023 16:21:58 +0530 Subject: [PATCH 29/72] fix(UX): Indicate that list view is empty because of filters (#21427) --- frappe/public/js/frappe/list/list_view.js | 30 +++++++++++------------ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index f009593f6f..c0eef27b9d 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -456,22 +456,20 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { get_no_result_message() { let help_link = this.get_documentation_link(); let filters = this.filter_area && this.filter_area.get(); - let no_result_message = - filters && filters.length - ? __("No {0} found", [__(this.doctype)]) - : __("You haven't created a {0} yet", [__(this.doctype)]); - let new_button_label = - filters && filters.length - ? __( - "Create a new {0}", - [__(this.doctype)], - "Create a new document from list view" - ) - : __( - "Create your first {0}", - [__(this.doctype)], - "Create a new document from list view" - ); + + let has_filters_set = filters && filters.length; + let no_result_message = has_filters_set + ? __("No {0} found with matching filters. Clear filters to see all {0}.", [ + __(this.doctype), + ]) + : __("You haven't created a {0} yet", [__(this.doctype)]); + let new_button_label = has_filters_set + ? __("Create a new {0}", [__(this.doctype)], "Create a new document from list view") + : __( + "Create your first {0}", + [__(this.doctype)], + "Create a new document from list view" + ); let empty_state_image = this.settings.empty_state_image || "/assets/frappe/images/ui-states/list-empty-state.svg"; From 0e92fc9bf58f3b9f88df0cd30c46527102a9015d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 19 Jun 2023 16:29:05 +0530 Subject: [PATCH 30/72] fix: Clear cache after role perm manager --- frappe/core/page/permission_manager/permission_manager.py | 7 +++++++ frappe/permissions.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index 5ed3014778..fd879095c0 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -123,8 +123,15 @@ def update(doctype, role, permlevel, ptype, value=None): Returns: str: Refresh flag is permission is updated successfully """ + + def clear_cache(): + frappe.clear_cache(doctype=doctype) + frappe.only_for("System Manager") out = update_permission_property(doctype, role, permlevel, ptype, value) + + frappe.db.after_commit.add(clear_cache) + return "refresh" if out else None diff --git a/frappe/permissions.py b/frappe/permissions.py index 4cd0d369e2..633d0e278d 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -532,7 +532,7 @@ def update_permission_property(doctype, role, permlevel, ptype, value=None, vali out = setup_custom_perms(doctype) - name = frappe.get_value("Custom DocPerm", dict(parent=doctype, role=role, permlevel=permlevel)) + name = frappe.db.get_value("Custom DocPerm", dict(parent=doctype, role=role, permlevel=permlevel)) table = DocType("Custom DocPerm") frappe.qb.update(table).set(ptype, value).where(table.name == name).run() From 276b11d331add07217ededf9cf92be1d4bddf403 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 19 Jun 2023 18:07:23 +0530 Subject: [PATCH 31/72] fix: ignore undefined fields on new doctype form --- frappe/public/js/frappe/model/model.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index 6deddacd19..355e0a9c5d 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -836,9 +836,9 @@ $.extend(frappe.model, { } if ( - (frm.doc.fields.find((i) => i.fieldname === "latitude") && - frm.doc.fields.find((i) => i.fieldname === "longitude")) || - frm.doc.fields.find( + (frm.doc.fields?.find((i) => i.fieldname === "latitude") && + frm.doc.fields?.find((i) => i.fieldname === "longitude")) || + frm.doc.fields?.find( (i) => i.fieldname === "location" && i.fieldtype == "Geolocation" ) ) { From 38960f42192e1ea94f4364b2dc8dbe833b9ad40f Mon Sep 17 00:00:00 2001 From: Anand Baburajan Date: Mon, 19 Jun 2023 18:19:07 +0530 Subject: [PATCH 32/72] feat: migrate translations command (#21362) * feat: migrate translations command * chore: formatting --- frappe/commands/translate.py | 18 ++++++++++++++ frappe/translate.py | 47 ++++++++++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/frappe/commands/translate.py b/frappe/commands/translate.py index 69970d8d97..5042843405 100644 --- a/frappe/commands/translate.py +++ b/frappe/commands/translate.py @@ -102,10 +102,28 @@ def import_translations(context, lang, path): frappe.destroy() +@click.command("migrate-translations") +@click.argument("source-app") +@click.argument("target-app") +@pass_context +def migrate_translations(context, source_app, target_app): + "Migrate target-app-specific translations from source-app to target-app" + import frappe.translate + + site = get_site(context) + try: + frappe.init(site=site) + frappe.connect() + frappe.translate.migrate_translations(source_app, target_app) + finally: + frappe.destroy() + + commands = [ build_message_files, get_untranslated, import_translations, new_language, update_translations, + migrate_translations, ] diff --git a/frappe/translate.py b/frappe/translate.py index f35a4b7ec3..7ad5ac80f8 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -15,7 +15,7 @@ import operator import os import re from contextlib import contextmanager -from csv import reader +from csv import reader, writer from babel.messages.extract import extract_python from babel.messages.jslexer import Token, tokenize, unquote_string @@ -997,7 +997,6 @@ def write_csv_file(path, app_messages, lang_dict): :param lang_dict: Full translated dict. """ app_messages.sort(key=lambda x: x[1]) - from csv import writer with open(path, "w", newline="") as msgfile: w = writer(msgfile, lineterminator="\n") @@ -1118,6 +1117,50 @@ def import_translations(lang, path): write_translations_file(app, lang, full_dict) +def migrate_translations(source_app, target_app): + """Migrate target-app-specific translations from source-app to target-app""" + clear_cache() + strings_in_source_app = [m[1] for m in frappe.translate.get_messages_for_app(source_app)] + strings_in_target_app = [m[1] for m in frappe.translate.get_messages_for_app(target_app)] + + strings_in_target_app_but_not_in_source_app = list( + set(strings_in_target_app) - set(strings_in_source_app) + ) + + languages = frappe.translate.get_all_languages() + + source_app_translations_dir = os.path.join(frappe.get_pymodule_path(source_app), "translations") + target_app_translations_dir = os.path.join(frappe.get_pymodule_path(target_app), "translations") + + if not os.path.exists(target_app_translations_dir): + os.makedirs(target_app_translations_dir) + + for lang in languages: + source_csv = os.path.join(source_app_translations_dir, lang + ".csv") + + if not os.path.exists(source_csv): + continue + + target_csv = os.path.join(target_app_translations_dir, lang + ".csv") + temp_csv = os.path.join(source_app_translations_dir, "_temp.csv") + + with open(source_csv) as s, open(target_csv, "a+") as t, open(temp_csv, "a+") as temp: + source_reader = reader(s, lineterminator="\n") + target_writer = writer(t, lineterminator="\n") + temp_writer = writer(temp, lineterminator="\n") + + for row in source_reader: + if row[0] in strings_in_target_app_but_not_in_source_app: + target_writer.writerow(row) + else: + temp_writer.writerow(row) + + if not os.path.getsize(target_csv): + os.remove(target_csv) + os.remove(source_csv) + os.rename(temp_csv, source_csv) + + def rebuild_all_translation_files(): """Rebuild all translation files: `[app]/translations/[lang].csv`.""" for lang in get_all_languages(): From d0ba31c911d3f32531dc1fdd62fe787f6c0794ca Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 19 Jun 2023 18:22:36 +0530 Subject: [PATCH 33/72] refactor!: Prefix all custom fieldnames created from Desk (#21355) * fix: actualy remove special characterso `or "_"` is always truthy, this code didn't do anything * refactor!: Prefix all custom fieldnames created from Desk Why? - Custom and standard field clashes are frequent. This will prevent it from happening. E.g. recent `incoterm` field addition in ERPNext. - Apps/fixtures can still specify exact fieldnames * test: custom field naming --- .semgrepignore | 0 .../doctype/custom_field/custom_field.py | 3 +- .../customize_form/test_customize_form.py | 45 +++++++++---------- 3 files changed, 23 insertions(+), 25 deletions(-) create mode 100644 .semgrepignore diff --git a/.semgrepignore b/.semgrepignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index 8953153be6..ed6296b6f2 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -40,8 +40,9 @@ class CustomField(Document): # remove special characters from fieldname self.fieldname = "".join( - filter(lambda x: x.isdigit() or x.isalpha() or "_", cstr(label).replace(" ", "_")) + [c for c in cstr(label).replace(" ", "_") if c.isdigit() or c.isalpha() or c == "_"] ) + self.fieldname = f"custom_{self.fieldname}" # fieldnames should be lowercase self.fieldname = self.fieldname.lower() diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index 149ef85e28..8a62d331be 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -14,10 +14,11 @@ test_dependencies = ["Custom Field", "Property Setter"] class TestCustomizeForm(FrappeTestCase): def insert_custom_field(self): - frappe.delete_doc_if_exists("Custom Field", "Event-test_custom_field") - frappe.get_doc( + frappe.delete_doc_if_exists("Custom Field", "Event-custom_test_field") + self.field = frappe.get_doc( { "doctype": "Custom Field", + "fieldname": "custom_test_field", "dt": "Event", "label": "Test Custom Field", "description": "A Custom Field for Testing", @@ -36,7 +37,7 @@ class TestCustomizeForm(FrappeTestCase): frappe.clear_cache(doctype="Event") def tearDown(self): - frappe.delete_doc("Custom Field", "Event-test_custom_field") + frappe.delete_doc("Custom Field", self.field.name) frappe.db.commit() frappe.clear_cache(doctype="Event") @@ -60,7 +61,7 @@ class TestCustomizeForm(FrappeTestCase): self.assertEqual(d.doc_type, "Event") self.assertEqual(len(d.get("fields")), len(frappe.get_doc("DocType", d.doc_type).fields) + 1) - self.assertEqual(d.get("fields")[-1].fieldname, "test_custom_field") + self.assertEqual(d.get("fields")[-1].fieldname, self.field.fieldname) self.assertEqual(d.get("fields", {"fieldname": "event_type"})[0].in_list_view, 1) return d @@ -129,21 +130,21 @@ class TestCustomizeForm(FrappeTestCase): def test_save_customization_custom_field_property(self): d = self.get_customize_form("Event") - self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0) + self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "reqd"), 0) - custom_field = d.get("fields", {"fieldname": "test_custom_field"})[0] + custom_field = d.get("fields", {"fieldname": self.field.fieldname})[0] custom_field.reqd = 1 custom_field.no_copy = 1 d.run_method("save_customization") - self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 1) - self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "no_copy"), 1) + self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "reqd"), 1) + self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "no_copy"), 1) custom_field = d.get("fields", {"is_custom_field": True})[0] custom_field.reqd = 0 custom_field.no_copy = 0 d.run_method("save_customization") - self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0) - self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "no_copy"), 0) + self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "reqd"), 0) + self.assertEqual(frappe.db.get_value("Custom Field", self.field.name, "no_copy"), 0) def test_save_customization_new_field(self): d = self.get_customize_form("Event") @@ -157,28 +158,24 @@ class TestCustomizeForm(FrappeTestCase): }, ) d.run_method("save_customization") + + custom_field_name = "Event-custom_test_add_custom_field_via_customize_form" self.assertEqual( - frappe.db.get_value( - "Custom Field", "Event-test_add_custom_field_via_customize_form", "fieldtype" - ), + frappe.db.get_value("Custom Field", custom_field_name, "fieldtype"), "Data", ) self.assertEqual( - frappe.db.get_value( - "Custom Field", "Event-test_add_custom_field_via_customize_form", "insert_after" - ), + frappe.db.get_value("Custom Field", custom_field_name, "insert_after"), last_fieldname, ) - frappe.delete_doc("Custom Field", "Event-test_add_custom_field_via_customize_form") - self.assertEqual( - frappe.db.get_value("Custom Field", "Event-test_add_custom_field_via_customize_form"), None - ) + frappe.delete_doc("Custom Field", custom_field_name) + self.assertEqual(frappe.db.get_value("Custom Field", custom_field_name), None) def test_save_customization_remove_field(self): d = self.get_customize_form("Event") - custom_field = d.get("fields", {"fieldname": "test_custom_field"})[0] + custom_field = d.get("fields", {"fieldname": self.field.fieldname})[0] d.get("fields").remove(custom_field) d.run_method("save_customization") @@ -200,7 +197,7 @@ class TestCustomizeForm(FrappeTestCase): def test_set_allow_on_submit(self): d = self.get_customize_form("Event") d.get("fields", {"fieldname": "subject"})[0].allow_on_submit = 1 - d.get("fields", {"fieldname": "test_custom_field"})[0].allow_on_submit = 1 + d.get("fields", {"fieldname": "custom_test_field"})[0].allow_on_submit = 1 d.run_method("save_customization") d = self.get_customize_form("Event") @@ -209,7 +206,7 @@ class TestCustomizeForm(FrappeTestCase): self.assertEqual(d.get("fields", {"fieldname": "subject"})[0].allow_on_submit or 0, 0) # allow for custom field - self.assertEqual(d.get("fields", {"fieldname": "test_custom_field"})[0].allow_on_submit, 1) + self.assertEqual(d.get("fields", {"fieldname": "custom_test_field"})[0].allow_on_submit, 1) def test_title_field_pattern(self): d = self.get_customize_form("Web Form") @@ -406,7 +403,7 @@ class TestCustomizeForm(FrappeTestCase): def test_system_generated_fields(self): doctype = "Event" - custom_field_name = "test_custom_field" + custom_field_name = "custom_test_field" custom_field = frappe.get_doc("Custom Field", {"dt": doctype, "fieldname": custom_field_name}) custom_field.is_system_generated = 1 From eb63faaf05fd0676e897bd07b41157703afc8fd9 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 20 Jun 2023 10:30:37 +0530 Subject: [PATCH 34/72] test: Change viewport to regular size --- cypress.config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cypress.config.js b/cypress.config.js index bfd0bc0025..2fdf10ca14 100644 --- a/cypress.config.js +++ b/cypress.config.js @@ -8,6 +8,8 @@ module.exports = defineConfig({ pageLoadTimeout: 15000, video: true, videoUploadOnPasses: false, + viewportHeight: 960, + viewportWidth: 1400, retries: { runMode: 2, openMode: 2, From 08bb03abd64c8cb8b4684d46a42cc95674317c58 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 20 Jun 2023 10:50:21 +0530 Subject: [PATCH 35/72] test(awesomebar): Remove unnecessary delay --- cypress/integration/awesome_bar.js | 39 +++++++++++------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/cypress/integration/awesome_bar.js b/cypress/integration/awesome_bar.js index 71e5e498cf..bb72ad1c59 100644 --- a/cypress/integration/awesome_bar.js +++ b/cypress/integration/awesome_bar.js @@ -7,50 +7,39 @@ context("Awesome Bar", () => { beforeEach(() => { cy.get(".navbar .navbar-home").click(); - cy.findByPlaceholderText("Search or type a command (Ctrl + G)").clear(); + cy.findByPlaceholderText("Search or type a command (Ctrl + G)").as("awesome_bar"); + cy.get("@awesome_bar").type("{selectall}"); }); it("navigates to doctype list", () => { - cy.findByPlaceholderText("Search or type a command (Ctrl + G)").type("todo", { - delay: 700, - }); + cy.get("@awesome_bar").type("todo"); + cy.wait(100); cy.get(".awesomplete").findByRole("listbox").should("be.visible"); - cy.findByPlaceholderText("Search or type a command (Ctrl + G)").type("{enter}", { - delay: 700, - }); - + cy.get("@awesome_bar").type("{enter}"); cy.get(".title-text").should("contain", "To Do"); - cy.location("pathname").should("eq", "/app/todo"); }); it("find text in doctype list", () => { - cy.findByPlaceholderText("Search or type a command (Ctrl + G)").type( - "test in todo{enter}", - { delay: 700 } - ); - + cy.get("@awesome_bar").type("test in todo"); + cy.wait(100); + cy.get("@awesome_bar").type("{enter}"); cy.get(".title-text").should("contain", "To Do"); - cy.findByPlaceholderText("ID").should("have.value", "%test%"); cy.clear_filters(); }); it("navigates to new form", () => { - cy.findByPlaceholderText("Search or type a command (Ctrl + G)").type( - "new blog post{enter}", - { delay: 700 } - ); - + cy.get("@awesome_bar").type("new blog post"); + cy.wait(100); + cy.get("@awesome_bar").type("{enter}"); cy.get(".title-text:visible").should("have.text", "New Blog Post"); }); it("calculates math expressions", () => { - cy.findByPlaceholderText("Search or type a command (Ctrl + G)").type( - "55 + 32{downarrow}{enter}", - { delay: 700 } - ); - + cy.get("@awesome_bar").type("55 + 32"); + cy.wait(100); + cy.get("@awesome_bar").type("{downarrow}{enter}"); cy.get(".modal-title").should("contain", "Result"); cy.get(".msgprint").should("contain", "55 + 32 = 87"); }); From ddadee3f3d4f3635c4a81f056de07c7782b9cec6 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 20 Jun 2023 10:50:50 +0530 Subject: [PATCH 36/72] test: Fix navigation test --- cypress/integration/navigation.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/cypress/integration/navigation.js b/cypress/integration/navigation.js index cf1b5dc89d..5961702ba5 100644 --- a/cypress/integration/navigation.js +++ b/cypress/integration/navigation.js @@ -1,21 +1,26 @@ context("Navigation", () => { before(() => { + cy.visit("/login"); cy.login(); + cy.visit("/app/website"); }); it("Navigate to route with hash in document name", () => { - cy.insert_doc("ToDo", { - __newname: "ABC#123", - description: "Test this", - ignore_duplicate: true, - }); - cy.visit("/app/todo/ABC#123"); + cy.insert_doc( + "ToDo", + { + __newname: "ABC#123", + description: "Test this", + }, + true + ); + cy.visit(`/app/todo/${encodeURIComponent("ABC#123")}`); cy.title().should("eq", "Test this - ABC#123"); cy.get_field("description", "Text Editor").contains("Test this"); cy.go("back"); cy.title().should("eq", "Website"); }); - it.only("Navigate to previous page after login", () => { + it("Navigate to previous page after login", () => { cy.visit("/app/todo"); cy.get(".page-head").findByTitle("To Do").should("be.visible"); cy.clear_filters(); From 4da6796b2db5c56d7b3ea586cf4cc38305bb422c Mon Sep 17 00:00:00 2001 From: David Arnold Date: Tue, 20 Jun 2023 01:24:22 -0500 Subject: [PATCH 37/72] feat: add style & onEachFeature function for geojson in geolocation controller (#21322) This no-op function can be overriden / inherited in custom controllers to craft richer map experiences --- .../js/frappe/form/controls/geolocation.js | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index ada729fd66..84b1989400 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -60,7 +60,11 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f this.clear_editable_layers(); const data_layers = new L.FeatureGroup().addLayer( - L.geoJson(JSON.parse(value), { pointToLayer: this.point_to_layer }) + L.geoJson(JSON.parse(value), { + pointToLayer: this.point_to_layer, + style: this.set_style, + onEachFeature: this.on_each_feature, + }) ); this.add_non_group_layers(data_layers, this.editableLayers); this.editableLayers.addTo(this.map); @@ -70,6 +74,8 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f /** * Defines custom rules for how geoJSON data is rendered on the map. * + * Can be inherited in custom map controllers. + * * @param {Object} geoJsonPoint - The geoJSON object to be rendered on the map. * @param {Object} latlng - The latitude and longitude where the geoJSON data should be rendered on the map. * @returns {Object} - Returns the Leaflet layer object to be rendered on the map. @@ -85,6 +91,28 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f } } + /** + * Defines custom styles for how geoJSON Line and LineString data is rendered on the map. + * + * Can be inherited in custom map controllers. + * + * @param {Object} geoJsonFeature - The geoJSON object to be rendered on the map. + * @returns {Object} - Returns the style object for the geoJSON object. + */ + set_style(geoJsonFeature) { + return {}; + } + + /** + * Is called after each feature is rendered and styles, can be used to attache popups, tooltips and other events + * + * Can be inherited in custom map controllers. + * + * @param {Object} feature - The leaflet object representing a geojson feature. + * @param {Object} layer - The leaflet layer object. + */ + on_each_feature(feature, layer) {} + bind_leaflet_map() { const circleToGeoJSON = L.Circle.prototype.toGeoJSON; L.Circle.include({ From db6a06d2042fc6e1e512700ca6d9bd6e1f38bf72 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Tue, 20 Jun 2023 12:00:22 +0530 Subject: [PATCH 38/72] fix: make sure number is not zero for `bankers_rounding` (#21431) * fix: make sure num is not zero for bankers_rounding * test: rounding near zero --------- Co-authored-by: Ankush Menat --- frappe/tests/test_utils.py | 2 ++ frappe/utils/data.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 4b362e7b47..142fbe7fa1 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -1096,6 +1096,8 @@ class TestRounding(FrappeTestCase): rounding_method = "Banker's Rounding" self.assertEqual(rounded(0, 0, rounding_method=rounding_method), 0) + self.assertEqual(rounded(5.551115123125783e-17, 2, rounding_method=rounding_method), 0.0) + self.assertEqual(flt("0.5", 0, rounding_method=rounding_method), 0) self.assertEqual(flt("0.3", rounding_method=rounding_method), 0.3) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index d59a6c16e6..bf999825f4 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1116,12 +1116,12 @@ def _round_away_from_zero(num, precision): def _bankers_rounding(num, precision): - if num == 0: - return 0.0 - multiplier = 10**precision num = round(num * multiplier, 12) + if num == 0: + return 0.0 + floor_num = math.floor(num) decimal_part = num - floor_num From 9c06d864c149dd4873048aff5915e36e5792ec79 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 20 Jun 2023 15:08:44 +0530 Subject: [PATCH 39/72] test: Add wait before checking for value --- cypress/integration/awesome_bar.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cypress/integration/awesome_bar.js b/cypress/integration/awesome_bar.js index bb72ad1c59..ecf8dcc718 100644 --- a/cypress/integration/awesome_bar.js +++ b/cypress/integration/awesome_bar.js @@ -25,7 +25,9 @@ context("Awesome Bar", () => { cy.wait(100); cy.get("@awesome_bar").type("{enter}"); cy.get(".title-text").should("contain", "To Do"); - cy.findByPlaceholderText("ID").should("have.value", "%test%"); + cy.wait(200); + const name_filter = cy.findByPlaceholderText("ID"); + name_filter.should("have.value", "%test%"); cy.clear_filters(); }); From c6b1b026c5a4fd0fcc60d314988092dbbff27aae Mon Sep 17 00:00:00 2001 From: Alfredo Altamirano <8353891+Ahuahuachi@users.noreply.github.com> Date: Tue, 20 Jun 2023 06:39:23 -0600 Subject: [PATCH 40/72] fix: data-import command fails with local files (#20521) (#21381) * fix: data is retrieved from db instead of local file * fix: data import command only accepts absolute path * fix: revert expanduser - Shells expand it. Works fine with bash/zsh - Doesn't work anyway as click wont let you init a path that doesn't exist * Revert "fix: data is retrieved from db instead of local file" This reverts commit cbe50a26da61e01b5a9a7c51f8632defb0913aab. * fix: allow local import if from console --------- Co-authored-by: Alfredo Altamirano Co-authored-by: Ankush Menat --- frappe/commands/utils.py | 9 ++++++++- frappe/core/doctype/data_import/importer.py | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 3f507514d4..7e4b1e8e46 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -386,7 +386,14 @@ def import_doc(context, path, force=False): @click.command("data-import") @click.option( - "--file", "file_path", type=click.Path(), required=True, help="Path to import file (.csv, .xlsx)" + "--file", + "file_path", + type=click.Path(exists=True, dir_okay=False, resolve_path=True), + required=True, + help=( + "Path to import file (.csv, .xlsx)." + "Consider that relative paths will resolve from 'sites' directory" + ), ) @click.option("--doctype", type=str, required=True) @click.option( diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index e1fc02d072..6d6e34d97d 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -579,6 +579,10 @@ class ImportFile: file_content = None + if self.console: + file_content = frappe.read_file(file_path, True) + return file_content, extn + file_name = frappe.db.get_value("File", {"file_url": file_path}) if file_name: file = frappe.get_doc("File", file_name) From cbe8a41cff31a3eff38551296ad7265e2d4e496a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 20 Jun 2023 19:50:51 +0530 Subject: [PATCH 41/72] build: Add responses as developer dependency (#21440) Useful for mocking HTTP responses in tests --- .../doctype/webhook/test_webhook.py | 54 +++++++++++++++---- pyproject.toml | 1 + 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/frappe/integrations/doctype/webhook/test_webhook.py b/frappe/integrations/doctype/webhook/test_webhook.py index 1701c418f7..d308ec95ab 100644 --- a/frappe/integrations/doctype/webhook/test_webhook.py +++ b/frappe/integrations/doctype/webhook/test_webhook.py @@ -3,6 +3,9 @@ import json from contextlib import contextmanager +import responses +from responses.matchers import json_params_matcher + import frappe from frappe.integrations.doctype.webhook.webhook import ( enqueue_webhook, @@ -94,9 +97,15 @@ class TestWebhook(FrappeTestCase): self.test_user.email = "user1@integration.webhooks.test.com" self.test_user.first_name = "user1" + self.responses = responses.RequestsMock() + self.responses.start() + def tearDown(self) -> None: self.user.delete() self.test_user.delete() + + self.responses.stop() + self.responses.reset() super().tearDown() def test_webhook_trigger_with_enabled_webhooks(self): @@ -172,6 +181,13 @@ class TestWebhook(FrappeTestCase): self.assertEqual(data, {"name": self.user.name}) def test_webhook_req_log_creation(self): + self.responses.add( + responses.POST, + "https://httpbin.org/post", + status=200, + json={}, + ) + if not frappe.db.get_value("User", "user2@integration.webhooks.test.com"): user = frappe.get_doc( {"doctype": "User", "email": "user2@integration.webhooks.test.com", "first_name": "user2"} @@ -185,6 +201,7 @@ class TestWebhook(FrappeTestCase): self.assertTrue(frappe.get_all("Webhook Request Log", pluck="name")) def test_webhook_with_array_body(self): + """Check if array request body are supported.""" wh_config = { "doctype": "Webhook", @@ -194,7 +211,7 @@ class TestWebhook(FrappeTestCase): "request_url": "https://httpbin.org/post", "request_method": "POST", "request_structure": "JSON", - "webhook_json": '[\r\n{% for n in range(3) %}\r\n {\r\n "title": "{{ doc.title }}",\r\n "n": {{ n }}\r\n }\r\n {%- if not loop.last -%}\r\n , \r\n {%endif%}\r\n{%endfor%}\r\n]', + "webhook_json": '[\r\n{% for n in range(3) %}\r\n {\r\n "title": "{{ doc.title }}" }\r\n {%- if not loop.last -%}\r\n , \r\n {%endif%}\r\n{%endfor%}\r\n]', "meets_condition": "Yes", "webhook_headers": [ { @@ -204,13 +221,22 @@ class TestWebhook(FrappeTestCase): ], } - with get_test_webhook(wh_config) as wh: - doc = frappe.new_doc("Note") - doc.title = "Test Webhook Note" + doc = frappe.new_doc("Note") + doc.title = "Test Webhook Note" + expected_req = [{"title": doc.title} for _ in range(3)] + self.responses.add( + responses.POST, + "https://httpbin.org/post", + status=200, + json=expected_req, + match=[json_params_matcher(expected_req)], + ) + + with get_test_webhook(wh_config) as wh: enqueue_webhook(doc, wh) log = frappe.get_last_doc("Webhook Request Log") - self.assertEqual(len(json.loads(log.response)["json"]), 3) + self.assertEqual(len(json.loads(log.response)), 3) def test_webhook_with_dynamic_url_enabled(self): wh_config = { @@ -232,12 +258,16 @@ class TestWebhook(FrappeTestCase): ], } + self.responses.add( + responses.POST, + "https://httpbin.org/anything/Note", + status=200, + ) + 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 = { @@ -259,11 +289,13 @@ class TestWebhook(FrappeTestCase): ], } + self.responses.add( + responses.POST, + "https://httpbin.org/anything/{{doc.doctype}}", + status=200, + ) + 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}}" - ) diff --git a/pyproject.toml b/pyproject.toml index 687a08ac0f..b9c1823794 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -105,3 +105,4 @@ pyngrok = "~=6.0.0" unittest-xml-reporting = "~=3.2.0" watchdog = "~=3.0.0" hypothesis = "~=6.77.0" +responses = "~=0.23.1" From 4f3c0f6e99462ea3d98378e692bfb003f95d5e90 Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Tue, 20 Jun 2023 21:37:08 +0530 Subject: [PATCH 42/72] feat: configurable amended document naming (#21414) * feat: configurable amendment naming * patch: set default amend naming * chore: linters and document filter * test: amended document naming * refactor: use set_single_value * chore: typo, fix copy * fix(UX): move action button below table * refactor: Series Counter -> Default Naming The behaviour in this PR doesn't necessarily mean to apply naming series it can be: - Naming Series - hash - Naming Rule - Some code So the name was misleading. --------- Co-authored-by: Ankush Menat --- .../__init__.py | 0 .../amended_document_naming_settings.json | 44 +++++++++++++++++++ .../amended_document_naming_settings.py | 9 ++++ .../document_naming_settings.js | 10 +++++ .../document_naming_settings.json | 36 ++++++++++++++- .../document_naming_settings.py | 17 +++++++ .../test_document_naming_settings.py | 34 ++++++++++++++ frappe/model/naming.py | 14 +++++- frappe/patches.txt | 1 + 9 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 frappe/core/doctype/amended_document_naming_settings/__init__.py create mode 100644 frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.json create mode 100644 frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.py diff --git a/frappe/core/doctype/amended_document_naming_settings/__init__.py b/frappe/core/doctype/amended_document_naming_settings/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.json b/frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.json new file mode 100644 index 0000000000..2892cc6091 --- /dev/null +++ b/frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.json @@ -0,0 +1,44 @@ +{ + "actions": [], + "allow_rename": 1, + "creation": "2023-06-16 17:57:36.604672", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "document_type", + "action" + ], + "fields": [ + { + "default": "Amend Counter", + "fieldname": "action", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Action", + "options": "Amend Counter\nDefault Naming", + "reqd": 1 + }, + { + "fieldname": "document_type", + "fieldtype": "Link", + "in_list_view": 1, + "label": "DocType", + "options": "DocType", + "reqd": 1, + "unique": 1 + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2023-06-16 18:26:16.247475", + "modified_by": "Administrator", + "module": "Core", + "name": "Amended Document Naming Settings", + "owner": "Administrator", + "permissions": [], + "sort_field": "modified", + "sort_order": "DESC", + "states": [] +} \ No newline at end of file diff --git a/frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.py b/frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.py new file mode 100644 index 0000000000..91b31350b0 --- /dev/null +++ b/frappe/core/doctype/amended_document_naming_settings/amended_document_naming_settings.py @@ -0,0 +1,9 @@ +# Copyright (c) 2023, Frappe Technologies and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + + +class AmendedDocumentNamingSettings(Document): + pass diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.js b/frappe/core/doctype/document_naming_settings/document_naming_settings.js index 2a9ec4aae5..f19e197249 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -2,6 +2,16 @@ // For license information, please see license.txt frappe.ui.form.on("Document Naming Settings", { + setup: function (frm) { + frm.set_query("document_type", "amend_naming_override", () => { + return { + filters: { + is_submittable: 1, + }, + }; + }); + }, + refresh: function (frm) { frm.trigger("setup_transaction_autocomplete"); frm.disable_save(); diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.json b/frappe/core/doctype/document_naming_settings/document_naming_settings.json index 9a12f3f77e..5a1991c14b 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.json +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -18,7 +18,11 @@ "update_series", "prefix", "current_value", - "update_series_start" + "update_series_start", + "amended_documents_section", + "default_amend_naming", + "amend_naming_override", + "update_amendment_naming" ], "fields": [ { @@ -105,13 +109,41 @@ "fieldtype": "Text", "label": "Preview of generated names", "read_only": 1 + }, + { + "collapsible": 1, + "description": "Configure how amended documents will be named.
\n\nDefault behaviour is to follow an amend counter which adds a number to the end of the original name indicating the amended version.
\n\nDefault Naming will make the amended document to behave same as new documents.", + "fieldname": "amended_documents_section", + "fieldtype": "Section Break", + "label": "Amended Documents" + }, + { + "default": "Amend Counter", + "fieldname": "default_amend_naming", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Default Amendment Naming", + "options": "Amend Counter\nDefault Naming", + "reqd": 1 + }, + { + "fieldname": "amend_naming_override", + "fieldtype": "Table", + "label": "Amendment Naming Override", + "options": "Amended Document Naming Settings" + }, + { + "fieldname": "update_amendment_naming", + "fieldtype": "Button", + "label": "Update Amendment Naming", + "options": "update_amendment_rule" } ], "hide_toolbar": 1, "icon": "fa fa-sort-by-order", "issingle": 1, "links": [], - "modified": "2023-02-20 13:11:56.662100", + "modified": "2023-06-20 17:47:52.204139", "modified_by": "Administrator", "module": "Core", "name": "Document Naming Settings", diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.py b/frappe/core/doctype/document_naming_settings/document_naming_settings.py index f8647bd74a..625b7cdd50 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -169,6 +169,23 @@ class DocumentNamingSettings(Document): self.current_value = NamingSeries(self.prefix).get_current_value() return self.current_value + @frappe.whitelist() + def update_amendment_rule(self): + self.db_set("default_amend_naming", self.default_amend_naming) + + existing_overrides = frappe.db.get_all( + "Amended Document Naming Settings", + filters={"name": ["not in", [d.name for d in self.amend_naming_override]]}, + pluck="name", + ) + for override in existing_overrides: + frappe.delete_doc("Amended Document Naming Settings", override) + + for row in self.amend_naming_override: + row.save() + + frappe.msgprint(_("Amendment naming rules updated."), indicator="green", alert=True) + @frappe.whitelist() def update_series_start(self): frappe.only_for("System Manager") diff --git a/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py index bcd3197112..d1a6fbe90d 100644 --- a/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py @@ -26,6 +26,7 @@ class TestNamingSeries(FrappeTestCase): } ], autoname="naming_series:", + is_submittable=1, ) .insert() .name @@ -82,3 +83,36 @@ class TestNamingSeries(FrappeTestCase): self.dns.update_series_start() self.assertEqual(self.dns.get_current(), new_count, f"Incorrect update for {series}") + + def test_amended_naming(self): + self.dns.amend_naming_override = [] + self.dns.default_amend_naming = "Amend Counter" + self.dns.update_amendment_rule() + + submittable_doc = frappe.get_doc( + dict(doctype=self.ns_doctype, some_fieldname="test doc with submit") + ).submit() + submittable_doc.cancel() + + amended_doc = frappe.get_doc( + dict( + doctype=self.ns_doctype, + some_fieldname="test doc with submit", + amended_from=submittable_doc.name, + ) + ).insert() + + self.assertIn(submittable_doc.name, amended_doc.name) + amended_doc.delete() + + self.dns.default_amend_naming = "Default Naming" + self.dns.update_amendment_rule() + + new_amended_doc = frappe.get_doc( + dict( + doctype=self.ns_doctype, + some_fieldname="test doc with submit", + amended_from=submittable_doc.name, + ) + ).insert() + self.assertNotIn(submittable_doc.name, new_amended_doc.name) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 73b5930563..0b76d18cff 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -151,7 +151,8 @@ def set_new_name(doc): if getattr(doc, "amended_from", None): _set_amended_name(doc) - return + if doc.name: + return elif getattr(doc.meta, "issingle", False): doc.name = doc.doctype @@ -506,6 +507,17 @@ def append_number_if_name_exists(doctype, value, fieldname="name", separator="-" def _set_amended_name(doc): + amend_naming_rule = frappe.db.get_value( + "Amended Document Naming Settings", {"document_type": doc.doctype}, "action", cache=True + ) + if not amend_naming_rule: + amend_naming_rule = frappe.db.get_single_value( + "Document Naming Settings", "default_amend_naming", cache=True + ) + + if amend_naming_rule == "Default Naming": + return + am_id = 1 am_prefix = doc.amended_from if frappe.db.get_value(doc.doctype, doc.amended_from, "amended_from"): diff --git a/frappe/patches.txt b/frappe/patches.txt index 436701e7bf..c26b1a74d7 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -226,3 +226,4 @@ frappe.patches.v14_0.remove_manage_subscriptions_from_navbar frappe.patches.v15_0.remove_background_jobs_from_dropdown frappe.desk.doctype.form_tour.patches.introduce_ui_tours execute:frappe.delete_doc_if_exists("Workspace", "Customization") +execute:frappe.db.set_single_value("Document Naming Settings", "default_amend_naming", "Amend Counter") From 05893bef4232cbc22c719aa2eee947ee0badce53 Mon Sep 17 00:00:00 2001 From: gavin Date: Wed, 21 Jun 2023 15:46:09 +0530 Subject: [PATCH 43/72] fix: Rename document/update title via toolbar (update_document_title API) (#21404) * fix: update_document_title * Fix broken socket event * Fix broken title + name doc update * fix: if only title updated then dont enqueue --------- Co-authored-by: Ankush Menat --- frappe/model/rename_doc.py | 36 ++++++++++++++++--------- frappe/public/js/frappe/form/toolbar.js | 6 ++--- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index e3ca6de739..e8f5626af4 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -53,6 +53,7 @@ def update_document_title( # handle bad API usages merge = sbool(merge) enqueue = sbool(enqueue) + action_enqueued = enqueue and not is_scheduler_inactive() doc = frappe.get_doc(doctype, docname) doc.check_permission(permtype="write") @@ -65,7 +66,7 @@ def update_document_title( name_updated = updated_name and (updated_name != doc.name) if name_updated: - if enqueue and not is_scheduler_inactive(): + if action_enqueued: current_name = doc.name # before_name hook may have DocType specific validations or transformations @@ -90,18 +91,27 @@ def update_document_title( doc.rename(updated_name, merge=merge) if title_updated: - try: - setattr(doc, title_field, updated_title) - doc.save() - frappe.msgprint(_("Saved"), alert=True, indicator="green") - except Exception as e: - if frappe.db.is_duplicate_entry(e): - frappe.throw( - _("{0} {1} already exists").format(doctype, frappe.bold(docname)), - title=_("Duplicate Name"), - exc=frappe.DuplicateEntryError, - ) - raise + if action_enqueued and name_updated: + frappe.enqueue( + "frappe.client.set_value", + doctype=doc.doctype, + name=updated_name, + fieldname=title_field, + value=updated_title, + ) + else: + try: + setattr(doc, title_field, updated_title) + doc.save() + frappe.msgprint(_("Saved"), alert=True, indicator="green") + except Exception as e: + if frappe.db.is_duplicate_entry(e): + frappe.throw( + _("{0} {1} already exists").format(doctype, frappe.bold(docname)), + title=_("Duplicate Name"), + exc=frappe.DuplicateEntryError, + ) + raise return doc.name diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index dbe88f2306..6d9a3365c4 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -109,6 +109,7 @@ frappe.ui.form.Toolbar = class Toolbar { } let rename_document = () => { + if (input_name != docname) frappe.socketio.doctype_subscribe(doctype, input_name); return frappe .xcall("frappe.model.rename_doc.update_document_title", { doctype, @@ -129,9 +130,8 @@ frappe.ui.form.Toolbar = class Toolbar { }; // handle document renaming queued action - if (input_name && new_docname == docname) { - frappe.socketio.doc_subscribe(doctype, input_name); - frappe.realtime.on("doc_update", (data) => { + if (input_name != docname) { + frappe.realtime.on("list_update", (data) => { if (data.doctype == doctype && data.name == input_name) { reload_form(input_name); frappe.show_alert({ From 7d352d7c345132d10858480fa9e81036a9f326c3 Mon Sep 17 00:00:00 2001 From: Ernesto Ruiz Date: Wed, 21 Jun 2023 04:23:17 -0600 Subject: [PATCH 44/72] chore: add translation to label on new doc in Update quick_entry.js (#21444) [skip ci] --- frappe/public/js/frappe/form/quick_entry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/quick_entry.js b/frappe/public/js/frappe/form/quick_entry.js index aea8d07f57..51e2f1317b 100644 --- a/frappe/public/js/frappe/form/quick_entry.js +++ b/frappe/public/js/frappe/form/quick_entry.js @@ -113,7 +113,7 @@ frappe.ui.form.QuickEntryForm = class QuickEntryForm { this.mandatory = [ { fieldname: "__newname", - label: __("{0} Name", [this.meta.name]), + label: __("{0} Name", [__(this.meta.name)]), reqd: 1, fieldtype: "Data", }, From b3840596fcb08b9e17b82daa6b3efb3fc812c4a9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 21 Jun 2023 16:33:09 +0530 Subject: [PATCH 45/72] test: mock github API calls (#21450) [skip ci] --- frappe/tests/test_utils.py | 10 ++++++++-- frappe/tests/utils.py | 13 +++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 142fbe7fa1..9997522692 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -20,7 +20,7 @@ from PIL import Image import frappe from frappe.installer import parse_app_name from frappe.model.document import Document -from frappe.tests.utils import FrappeTestCase, change_settings +from frappe.tests.utils import FrappeTestCase, MockedRequestTestCase, change_settings from frappe.utils import ( ceil, dict_to_str, @@ -815,8 +815,14 @@ class TestLinkTitle(FrappeTestCase): prop_setter.delete() -class TestAppParser(FrappeTestCase): +class TestAppParser(MockedRequestTestCase): def test_app_name_parser(self): + self.responses.add( + "HEAD", + "https://api.github.com/repos/frappe/healthcare", + status=200, + json={}, + ) bench_path = get_bench_path() frappe_app = os.path.join(bench_path, "apps", "frappe") self.assertEqual("frappe", parse_app_name(frappe_app)) diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index 07003d3b8c..6e6c72052a 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -93,6 +93,19 @@ class FrappeTestCase(unittest.TestCase): frappe.db.sql = orig_sql +class MockedRequestTestCase(FrappeTestCase): + def setUp(self): + import responses + + self.responses = responses.RequestsMock() + self.responses.start() + + self.addCleanup(self.responses.stop) + self.addCleanup(self.responses.reset) + + return super().setUp() + + def _commit_watcher(): import traceback From 378c97871b7b61ae54aaab4fe77e2e1c7aeef1a4 Mon Sep 17 00:00:00 2001 From: Ernesto Ruiz Date: Wed, 21 Jun 2023 23:51:30 -0600 Subject: [PATCH 46/72] chore: Add translation to confirmation text on Update kanban_view.js (#21453) [skip ci] --- frappe/public/js/frappe/views/kanban/kanban_view.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index fb15131394..25b875c2f9 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -102,7 +102,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { this.menu_items.push({ label: __("Delete Kanban Board"), action: () => { - frappe.confirm("Are you sure you want to proceed?", () => { + frappe.confirm(__("Are you sure you want to proceed?"), () => { frappe.db.delete_doc("Kanban Board", this.board_name).then(() => { frappe.show_alert(`Kanban Board ${this.board_name} deleted.`); frappe.set_route("List", this.doctype, "List"); From 69220a9a9e63458ed9e66736b4da30658a6072db Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 22 Jun 2023 11:53:22 +0530 Subject: [PATCH 47/72] fix(dx): Reset module onboarding progress Useful while working on form tours to quickly reset progress [skip ci] --- .../doctype/module_onboarding/module_onboarding.js | 4 ++++ .../doctype/module_onboarding/module_onboarding.py | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/frappe/desk/doctype/module_onboarding/module_onboarding.js b/frappe/desk/doctype/module_onboarding/module_onboarding.js index 0e312025bf..831b29a660 100644 --- a/frappe/desk/doctype/module_onboarding/module_onboarding.js +++ b/frappe/desk/doctype/module_onboarding/module_onboarding.js @@ -13,6 +13,10 @@ frappe.ui.form.on("Module Onboarding", { if (!frappe.boot.developer_mode) { frm.trigger("disable_form"); } + + frm.add_custom_button(__("Reset"), () => { + frm.call("reset_progress"); + }); }, disable_form: function (frm) { diff --git a/frappe/desk/doctype/module_onboarding/module_onboarding.py b/frappe/desk/doctype/module_onboarding/module_onboarding.py index ea02f5911d..94805d05b6 100644 --- a/frappe/desk/doctype/module_onboarding/module_onboarding.py +++ b/frappe/desk/doctype/module_onboarding/module_onboarding.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import frappe +from frappe import _ from frappe.model.document import Document from frappe.modules.export_file import export_to_files @@ -37,6 +38,16 @@ class ModuleOnboarding(Document): return False + @frappe.whitelist() + def reset_progress(self): + self.db_set("is_complete", 0) + + for step in self.get_steps(): + step.db_set("is_complete", 0) + step.db_set("is_skipped", 0) + + frappe.msgprint(_("Module onboarding progress reset"), alert=True) + def before_export(self, doc): doc.is_complete = 0 From 4f70200bd5a259095b34e255ed664d58dfdea924 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 22 Jun 2023 12:05:31 +0530 Subject: [PATCH 48/72] chore: track update actions per doctype [skip ci] --- frappe/desk/form/save.py | 4 +++- frappe/utils/telemetry.py | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/frappe/desk/form/save.py b/frappe/desk/form/save.py index 75335cb1ce..180717da40 100644 --- a/frappe/desk/form/save.py +++ b/frappe/desk/form/save.py @@ -16,7 +16,7 @@ from frappe.utils.telemetry import capture_doc def savedocs(doc, action): """save / submit / update doclist""" doc = frappe.get_doc(json.loads(doc)) - capture_doc(doc) + capture_doc(doc, action) set_local_name(doc) # action @@ -47,6 +47,8 @@ def savedocs(doc, action): def cancel(doctype=None, name=None, workflow_state_fieldname=None, workflow_state=None): """cancel a doclist""" doc = frappe.get_doc(doctype, name) + capture_doc(doc, "Cancel") + if workflow_state_fieldname and workflow_state: doc.set(workflow_state_fieldname, workflow_state) doc.cancel() diff --git a/frappe/utils/telemetry.py b/frappe/utils/telemetry.py index e15146c71d..5f3a7ee0c9 100644 --- a/frappe/utils/telemetry.py +++ b/frappe/utils/telemetry.py @@ -5,11 +5,10 @@ removed without any warning. """ from contextlib import suppress -from posthog import Posthog - import frappe from frappe.utils import getdate from frappe.utils.caching import site_cache +from posthog import Posthog POSTHOG_PROJECT_FIELD = "posthog_project_id" POSTHOG_HOST_FIELD = "posthog_host" @@ -59,11 +58,13 @@ def capture(event, app, **kwargs): ph and ph.capture(distinct_id=frappe.local.site, event=f"{app}_{event}", **kwargs) -def capture_doc(doc): +def capture_doc(doc, action): with suppress(Exception): age = site_age() if not age or age > 15: return if doc.get("__islocal") or not doc.get("name"): - capture("document_created", "frappe", properties={"doctype": doc.doctype}) + capture("document_created", "frappe", properties={"doctype": doc.doctype, "action": "Insert"}) + else: + capture("document_modified", "frappe", properties={"doctype": doc.doctype, "action": action}) From 085eb4124588382ffc4a97f2a78499f3b94d32e3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 22 Jun 2023 13:48:20 +0530 Subject: [PATCH 49/72] ci: use multiple python version in patch test (#21443) * ci: bump pyenv action * ci: set default python version in patch test * ci: setup pyenv manually * ci: replace pyenv with setup-action * debug * use multiple python versions directly * Revert "debug" This reverts commit 692cbc0c9a407b86647ee09079fdde1510f300d7. * setup py with --python flag --- .github/workflows/patch-mariadb-tests.yml | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/patch-mariadb-tests.yml b/.github/workflows/patch-mariadb-tests.yml index 864661ef54..3311f5235e 100644 --- a/.github/workflows/patch-mariadb-tests.yml +++ b/.github/workflows/patch-mariadb-tests.yml @@ -62,9 +62,11 @@ jobs: fi - name: Setup Python - uses: "gabrielfalcao/pyenv-action@v10" + uses: actions/setup-python@v4 with: - versions: 3.10:latest, 3.7:latest + python-version: | + 3.7 + 3.10 - name: Setup Node uses: actions/setup-node@v3 @@ -100,7 +102,6 @@ jobs: run: | bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh pip install frappe-bench - pyenv global $(pyenv versions | grep '3.10') bash ${GITHUB_WORKSPACE}/.github/helper/install.sh env: BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} @@ -120,25 +121,25 @@ jobs: function update_to_version() { version=$1 + py=$2 + branch_name="version-$version-hotfix" echo "Updating to v$version" git fetch --depth 1 upstream $branch_name:$branch_name git checkout -q -f $branch_name - pip install -U frappe-bench pgrep honcho | xargs kill rm -rf ~/frappe-bench/env - bench -v setup env + bench -v setup env --python $py bench start &> ~/frappe-bench/bench_start.log & + bench --site test_site migrate } - pyenv global $(pyenv versions | grep '3.7') - update_to_version 12 - update_to_version 13 + update_to_version 12 python3.7 + update_to_version 13 python3.7 - pyenv global $(pyenv versions | grep '3.10') - update_to_version 14 + update_to_version 14 python3.10 echo "Updating to last commit" rm -rf ~/frappe-bench/env From d044b305225459c5b2a795b25ab419fc9cb7bb48 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 22 Jun 2023 15:47:34 +0200 Subject: [PATCH 50/72] fix: remove `app_version` from hooks boilerplate It's not used by any frappe/erpnext code and linters complain about the unused import --- frappe/utils/boilerplate.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index 2cb1a6ab80..eb764f495c 100644 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -298,9 +298,7 @@ __version__ = '0.0.1' """ -hooks_template = """from . import __version__ as app_version - -app_name = "{app_name}" +hooks_template = """app_name = "{app_name}" app_title = "{app_title}" app_publisher = "{app_publisher}" app_description = "{app_description}" From 15396ceb6052051b8f8daae1bc9124fa187f91fd Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 22 Jun 2023 15:48:04 +0200 Subject: [PATCH 51/72] fix: add `required_apps` to hooks boilerplate --- frappe/utils/boilerplate.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index eb764f495c..7c79d3b2dd 100644 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -304,6 +304,7 @@ app_publisher = "{app_publisher}" app_description = "{app_description}" app_email = "{app_email}" app_license = "{app_license}" +# required_apps = [] # Includes in # ------------------ From 4ad98ecb7f7eb29daafc0af1f1e13a4c70a93fc3 Mon Sep 17 00:00:00 2001 From: Sayed Ayman Date: Thu, 22 Jun 2023 23:26:52 +0300 Subject: [PATCH 52/72] fix: use correct property name for filters in get_report_filters (#21465) --- frappe/public/js/frappe/views/reports/report_utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/reports/report_utils.js b/frappe/public/js/frappe/views/reports/report_utils.js index d75716541b..16b02c28a1 100644 --- a/frappe/public/js/frappe/views/reports/report_utils.js +++ b/frappe/public/js/frappe/views/reports/report_utils.js @@ -128,7 +128,7 @@ frappe.report_utils = { return frappe.after_ajax(() => { if ( frappe.query_reports[report_name] && - !frappe.query_reports[report_name].filter && + !frappe.query_reports[report_name].filters && r.filters ) { return (frappe.query_reports[report_name].filters = r.filters); From fb11b5a1b088628a7bebe5bc933be8e26068a73c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 23 Jun 2023 12:40:42 +0530 Subject: [PATCH 53/72] fix(UX): better error message for Encryption key --- frappe/utils/password.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/utils/password.py b/frappe/utils/password.py index 29a8c12994..fe3ef27d6b 100644 --- a/frappe/utils/password.py +++ b/frappe/utils/password.py @@ -195,7 +195,13 @@ def decrypt(txt, encryption_key=None): return cstr(cipher_suite.decrypt(encode(txt))) except InvalidToken: # encryption_key in site_config is changed and not valid - frappe.throw(_("Encryption key is invalid! Please check site_config.json")) + frappe.throw( + _("Encryption key is invalid! Please check site_config.json") + + "
" + + _( + "If you have recently restored the site you may need to copy the site config contaning original Encryption Key." + ) + ) def get_encryption_key(): From 8a37d6d2786a35c4b6231aa029ce338790f1ecfe Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 23 Jun 2023 12:51:45 +0530 Subject: [PATCH 54/72] perf: reduce memory usage of background processes (#21467) * perf: defer translation.py imports This indirectly imports babel which isn't really required most of the time. * perf: defer gzip import * perf: move validate_and_sanitize_search_inputs This causes all sorts of indirect imports and increases memory usage * perf: defer requests module imports * perf: defer system settings import * perf: defer LOG_DOCTYPES import Causes many indirect imports * perf: defer update_site_config * perf: defer notifications import * perf: remove unused import * perf: defer safe exec import * test: memory usage overhead --- frappe/__init__.py | 18 ++++++++- frappe/boot.py | 5 ++- frappe/build.py | 8 ++-- frappe/cache_manager.py | 7 ++-- frappe/commands/redis_utils.py | 3 +- frappe/commands/site.py | 5 ++- frappe/core/doctype/rq_job/test_rq_job.py | 14 +++++++ .../system_settings/system_settings.py | 9 +++-- frappe/database/database.py | 1 - frappe/defaults.py | 1 - frappe/desk/form/meta.py | 3 +- frappe/desk/search.py | 20 ++-------- frappe/query_builder/utils.py | 37 ++++++++++--------- frappe/utils/__init__.py | 5 ++- frappe/utils/change_log.py | 2 +- frappe/utils/data.py | 8 ++++ frappe/utils/scheduler.py | 3 +- 17 files changed, 93 insertions(+), 56 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 0f85c8a11b..a4ac8111dc 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -2418,4 +2418,20 @@ def mock(type, size=1, locale="en"): return squashify(results) -from frappe.desk.search import validate_and_sanitize_search_inputs # noqa +def validate_and_sanitize_search_inputs(fn): + @functools.wraps(fn) + def wrapper(*args, **kwargs): + from frappe.desk.search import sanitize_searchfield + from frappe.utils import cint + + kwargs.update(dict(zip(fn.__code__.co_varnames, args))) + sanitize_searchfield(kwargs["searchfield"]) + kwargs["start"] = cint(kwargs["start"]) + kwargs["page_len"] = cint(kwargs["page_len"]) + + if kwargs["doctype"] and not db.exists("DocType", kwargs["doctype"]): + return [] + + return fn(**kwargs) + + return wrapper diff --git a/frappe/boot.py b/frappe/boot.py index 8881d25bd6..fb1bd3d7a2 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -21,7 +21,6 @@ from frappe.social.doctype.energy_point_log.energy_point_log import get_energy_p from frappe.social.doctype.energy_point_settings.energy_point_settings import ( is_energy_point_enabled, ) -from frappe.translate import get_lang_dict, get_messages_for_boot, get_translated_doctypes from frappe.utils import add_user_info, cstr, get_system_timezone from frappe.utils.change_log import get_versions from frappe.website.doctype.web_page_view.web_page_view import is_tracking_enabled @@ -29,6 +28,8 @@ from frappe.website.doctype.web_page_view.web_page_view import is_tracking_enabl def get_bootinfo(): """build and return boot info""" + from frappe.translate import get_lang_dict, get_translated_doctypes + frappe.set_user_lang(frappe.session.user) bootinfo = frappe._dict() hooks = frappe.get_hooks() @@ -257,6 +258,8 @@ def get_user_pages_or_reports(parent, cache=False): def load_translations(bootinfo): + from frappe.translate import get_messages_for_boot + bootinfo["lang"] = frappe.lang bootinfo["__messages"] = get_messages_for_boot() diff --git a/frappe/build.py b/frappe/build.py index 46301dadaf..66b3eabd5e 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -10,8 +10,6 @@ from urllib.parse import urlparse import click import psutil -from requests import head -from requests.exceptions import HTTPError from semantic_version import Version import frappe @@ -27,7 +25,7 @@ class AssetsNotDownloadedError(Exception): pass -class AssetsDontExistError(HTTPError): +class AssetsDontExistError(Exception): pass @@ -78,6 +76,8 @@ def build_missing_files(): def get_assets_link(frappe_head) -> str: + import requests + tag = getoutput( r"cd ../apps/frappe && git show-ref --tags -d | grep %s | sed -e 's,.*" r" refs/tags/,,' -e 's/\^{}//'" % frappe_head @@ -89,7 +89,7 @@ def get_assets_link(frappe_head) -> str: else: url = f"http://assets.frappeframework.com/{frappe_head}.tar.gz" - if not head(url): + if not requests.head(url): reference = f"Release {tag}" if tag else f"Commit {frappe_head}" raise AssetsDontExistError(f"Assets for {reference} don't exist") diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py index 6ee88d9d37..f47478d871 100644 --- a/frappe/cache_manager.py +++ b/frappe/cache_manager.py @@ -1,10 +1,7 @@ # Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -import json - import frappe -from frappe.desk.notifications import clear_notifications, delete_notification_count_for common_default_keys = ["__default", "__global"] @@ -79,6 +76,8 @@ doctype_cache_keys = ( def clear_user_cache(user=None): + from frappe.desk.notifications import clear_notifications + # this will automatically reload the global cache # so it is important to clear this first clear_notifications(user) @@ -128,6 +127,8 @@ def clear_doctype_cache(doctype=None): def _clear_doctype_cache_form_redis(doctype: str | None = None): + from frappe.desk.notifications import delete_notification_count_for + for key in ("is_table", "doctype_modules"): frappe.cache.delete_value(key) diff --git a/frappe/commands/redis_utils.py b/frappe/commands/redis_utils.py index 1c3292b7fa..07ad8715c4 100644 --- a/frappe/commands/redis_utils.py +++ b/frappe/commands/redis_utils.py @@ -3,7 +3,6 @@ import os import click import frappe -from frappe.installer import update_site_config from frappe.utils.redis_queue import RedisQueue @@ -23,6 +22,8 @@ def create_rq_users(set_admin_password=False, use_rq_auth=False): acl config file will be used by redis server while starting the server and app config is used by app while connecting to redis server. """ + from frappe.installer import update_site_config + acl_file_path = os.path.abspath("../config/redis_queue.acl") with frappe.init_site(): diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 25c8c3159d..d606bb78cf 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -9,7 +9,6 @@ import click # imports - module imports import frappe from frappe.commands import get_site, pass_context -from frappe.core.doctype.log_settings.log_settings import LOG_DOCTYPES from frappe.exceptions import SiteNotSpecifiedError @@ -1199,11 +1198,12 @@ def build_search_index(context): @click.command("clear-log-table") -@click.option("--doctype", required=True, type=click.Choice(LOG_DOCTYPES), help="Log DocType") +@click.option("--doctype", required=True, type=str, help="Log DocType") @click.option("--days", type=int, help="Keep records for days") @click.option("--no-backup", is_flag=True, default=False, help="Do not backup the table") @pass_context def clear_log_table(context, doctype, days, no_backup): + """If any logtype table grows too large then clearing it with DELETE query is not feasible in reasonable time. This command copies recent data to new table and replaces current table with new smaller table. @@ -1211,6 +1211,7 @@ def clear_log_table(context, doctype, days, no_backup): ref: https://mariadb.com/kb/en/big-deletes/#deleting-more-than-half-a-table """ + from frappe.core.doctype.log_settings.log_settings import LOG_DOCTYPES from frappe.core.doctype.log_settings.log_settings import clear_log_table as clear_logs from frappe.utils.backups import scheduled_backup diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py index 09a90f7445..c39717cfd8 100644 --- a/frappe/core/doctype/rq_job/test_rq_job.py +++ b/frappe/core/doctype/rq_job/test_rq_job.py @@ -124,6 +124,20 @@ class TestRQJob(FrappeTestCase): frappe.db.commit() self.assertIsNone(get_job_status(job_id)) + @timeout(20) + def test_memory_usage(self): + job = frappe.enqueue("frappe.utils.data._get_rss_memory_usage") + self.check_status(job, "finished") + + rss = job.latest_result().return_value + msg = """Memory usage of simple background job increased. Potential root cause can be a newly added python module import. Check and move them to approriate file/function to avoid loading the module by default.""" + + # If this starts failing analyze memory usage using memray or some equivalent tool to find + # offending imports/function calls. + # Refer this PR: https://github.com/frappe/frappe/pull/21467 + LAST_MEASURED_USAGE = 40 + self.assertLessEqual(rss, LAST_MEASURED_USAGE * 1.05, msg) + def test_func(fail=False, sleep=0): if fail: diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index 2fec4e87af..0c842f9c7d 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -5,14 +5,13 @@ import frappe from frappe import _ from frappe.model import no_value_fields from frappe.model.document import Document -from frappe.translate import set_default_language -from frappe.twofactor import toggle_two_factor_auth from frappe.utils import cint, today -from frappe.utils.momentjs import get_all_timezones class SystemSettings(Document): def validate(self): + from frappe.twofactor import toggle_two_factor_auth + enable_password_policy = cint(self.enable_password_policy) and True or False minimum_password_score = cint(getattr(self, "minimum_password_score", 0)) or 0 if enable_password_policy and minimum_password_score <= 0: @@ -71,6 +70,8 @@ class SystemSettings(Document): update_last_reset_password_date() def set_defaults(self): + from frappe.translate import set_default_language + for df in self.meta.get("fields"): if df.fieldtype not in no_value_fields and self.has_value_changed(df.fieldname): frappe.db.set_default(df.fieldname, self.get(df.fieldname)) @@ -92,6 +93,8 @@ def update_last_reset_password_date(): @frappe.whitelist() def load(): + from frappe.utils.momentjs import get_all_timezones + if not "System Manager" in frappe.get_roles(): frappe.throw(_("Not permitted"), frappe.PermissionError) diff --git a/frappe/database/database.py b/frappe/database/database.py index 1f2a1eeba9..a264f39d47 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -17,7 +17,6 @@ from pypika.terms import Criterion, NullValue import frappe import frappe.defaults -import frappe.model.meta from frappe import _ from frappe.database.utils import ( DefaultOrderBy, diff --git a/frappe/defaults.py b/frappe/defaults.py index 0b86e99efa..3bcfbec1ce 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -3,7 +3,6 @@ import frappe from frappe.cache_manager import clear_defaults_cache, common_default_keys -from frappe.desk.notifications import clear_notifications from frappe.query_builder import DocType # Note: DefaultValue records are identified by parent (e.g. __default, __global) diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index 3ad65f7b13..6c338dbbbc 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -9,7 +9,6 @@ from frappe.build import scrub_html_template from frappe.model.meta import Meta from frappe.model.utils import render_include from frappe.modules import get_module_path, load_doctype_module, scrub -from frappe.translate import extract_messages_from_code, make_dict_from_messages from frappe.utils import get_html_format from frappe.utils.data import get_link_to_form @@ -260,6 +259,8 @@ class FormMeta(Meta): self.set("__form_grid_templates", templates) def set_translations(self, lang): + from frappe.translate import extract_messages_from_code, make_dict_from_messages + self.set("__messages", frappe.get_lang_dict("doctype", self.name)) # set translations for grid templates diff --git a/frappe/desk/search.py b/frappe/desk/search.py index d347cc188c..c4c11558dd 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -6,7 +6,9 @@ import json import re import frappe -from frappe import _, is_whitelisted + +# Backward compatbility +from frappe import _, is_whitelisted, validate_and_sanitize_search_inputs from frappe.database.schema import SPECIAL_CHAR_PATTERN from frappe.permissions import has_permission from frappe.utils import cint, cstr, unique @@ -293,22 +295,6 @@ def relevance_sorter(key, query, as_dict): return (cstr(value).casefold().startswith(query.casefold()) is not True, value) -def validate_and_sanitize_search_inputs(fn): - @functools.wraps(fn) - def wrapper(*args, **kwargs): - kwargs.update(dict(zip(fn.__code__.co_varnames, args))) - sanitize_searchfield(kwargs["searchfield"]) - kwargs["start"] = cint(kwargs["start"]) - kwargs["page_len"] = cint(kwargs["page_len"]) - - if kwargs["doctype"] and not frappe.db.exists("DocType", kwargs["doctype"]): - return [] - - return fn(**kwargs) - - return wrapper - - @frappe.whitelist() def get_names_for_mentions(search_term): users_for_mentions = frappe.cache.get_value("users_for_mentions", get_users_for_mentions) diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index af2c871e83..f2651904fb 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -106,27 +106,28 @@ def patch_query_execute(): def prepare_query(query): import inspect - from frappe.utils.safe_exec import check_safe_sql_query - param_collector = NamedParameterWrapper() query = query.get_sql(param_wrapper=param_collector) - if frappe.flags.in_safe_exec and not check_safe_sql_query(query, throw=False): - callstack = inspect.stack() - if len(callstack) >= 3 and ".py" in callstack[2].filename: - # ignore any query builder methods called from python files - # assumption is that those functions are whitelisted already. + if frappe.flags.in_safe_exec: + from frappe.utils.safe_exec import check_safe_sql_query - # since query objects are patched everywhere any query.run() - # will have callstack like this: - # frame0: this function prepare_query() - # frame1: execute_query() - # frame2: frame that called `query.run()` - # - # if frame2 is server script is set as the filename - # it shouldn't be allowed. - pass - else: - raise frappe.PermissionError("Only SELECT SQL allowed in scripting") + if not check_safe_sql_query(query, throw=False): + callstack = inspect.stack() + if len(callstack) >= 3 and ".py" in callstack[2].filename: + # ignore any query builder methods called from python files + # assumption is that those functions are whitelisted already. + + # since query objects are patched everywhere any query.run() + # will have callstack like this: + # frame0: this function prepare_query() + # frame1: execute_query() + # frame2: frame that called `query.run()` + # + # if frame2 is server script is set as the filename + # it shouldn't be allowed. + pass + else: + raise frappe.PermissionError("Only SELECT SQL allowed in scripting") return query, param_collector.get_parameters() builder_class = frappe.qb._BuilderClasss diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index d6b8186a2f..6ffa011ed9 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -20,7 +20,6 @@ from collections.abc import ( ) from email.header import decode_header, make_header from email.utils import formataddr, parseaddr -from gzip import GzipFile from typing import Any, Callable, Literal from urllib.parse import quote, urlparse @@ -873,6 +872,8 @@ def gzip_compress(data, compresslevel=9): """Compress data in one shot and return the compressed string. Optional argument is the compression level, in range of 0-9. """ + from gzip import GzipFile + buf = io.BytesIO() with GzipFile(fileobj=buf, mode="wb", compresslevel=compresslevel) as f: f.write(data) @@ -883,6 +884,8 @@ def gzip_decompress(data): """Decompress a gzip compressed string in one shot. Return the decompressed string. """ + from gzip import GzipFile + with GzipFile(fileobj=io.BytesIO(data)) as f: return f.read() diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py index 586024f931..3da9c5d757 100644 --- a/frappe/utils/change_log.py +++ b/frappe/utils/change_log.py @@ -5,7 +5,6 @@ import json import os import subprocess # nosec -import requests from semantic_version import Version import frappe @@ -231,6 +230,7 @@ def check_release_on_github(app: str): organization name, if the application exists, otherwise None. """ + import requests from giturlparse import parse from giturlparse.parser import ParserError diff --git a/frappe/utils/data.py b/frappe/utils/data.py index bf999825f4..eeabfad559 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -2234,3 +2234,11 @@ def add_trackers_to_url(url: str, source: str, campaign: str, medium: str = "ema url_parts[4] = urlencode(query) return urlunparse(url_parts) + + +# This is used in test to count memory overhead of default imports. +def _get_rss_memory_usage(): + import psutil + + rss = psutil.Process().memory_info().rss // (1024 * 1024) + return rss diff --git a/frappe/utils/scheduler.py b/frappe/utils/scheduler.py index 529a3c7bf7..903a8dd081 100755 --- a/frappe/utils/scheduler.py +++ b/frappe/utils/scheduler.py @@ -15,7 +15,6 @@ from typing import NoReturn # imports - module imports import frappe -from frappe.installer import update_site_config from frappe.utils import cint, get_datetime, get_sites, now_datetime from frappe.utils.background_jobs import get_jobs @@ -176,6 +175,8 @@ def _get_last_modified_timestamp(doctype): @frappe.whitelist() def activate_scheduler(): + from frappe.installer import update_site_config + frappe.only_for("Administrator") if frappe.local.conf.maintenance_mode: From c6419f52e2458a756efce4562dbe0340c3ee1760 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 23 Jun 2023 14:13:21 +0530 Subject: [PATCH 55/72] perf: Move babel import to extract function This is never used in production. [skip ci] --- frappe/translate.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/translate.py b/frappe/translate.py index 7ad5ac80f8..806ec8e66f 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -17,8 +17,6 @@ import re from contextlib import contextmanager from csv import reader, writer -from babel.messages.extract import extract_python -from babel.messages.jslexer import Token, tokenize, unquote_string from pypika.terms import PseudoColumn import frappe @@ -737,6 +735,7 @@ def get_messages_from_file(path: str) -> list[tuple[str, str, str | None, int]]: def extract_messages_from_python_code(code: str) -> list[tuple[int, str, str | None]]: """Extracts translatable strings from Python code using babel.""" + from babel.messages.extract import extract_python messages = [] @@ -809,6 +808,8 @@ def extract_javascript(code, keywords=("__",), options=None): * `template_string` -- set to false to disable ES6 template string support. """ + from babel.messages.jslexer import Token, tokenize, unquote_string + if options is None: options = {} From 54be05c18e3820943b211102415b56ad01751902 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 23 Jun 2023 21:01:22 +0530 Subject: [PATCH 56/72] fix(Workspace): ignore `is_hidden` when importing standard workspaces (#21470) --- frappe/modules/import_file.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/modules/import_file.py b/frappe/modules/import_file.py index 36e329409a..8c9a209501 100644 --- a/frappe/modules/import_file.py +++ b/frappe/modules/import_file.py @@ -34,6 +34,7 @@ ignore_values = { "Print Style": ["disabled"], "Module Onboarding": ["is_complete"], "Onboarding Step": ["is_complete", "is_skipped"], + "Workspace": ["is_hidden"], } ignore_doctypes = [""] From 4aa20c72c19e197f322a176e9571f72282088cbc Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 11:54:20 +0530 Subject: [PATCH 57/72] refactor: Simplify FrappeOAuth2Client No need to override anything, use request.session in way it is intended. --- frappe/frappeclient.py | 39 +++++---------------------------------- 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/frappe/frappeclient.py b/frappe/frappeclient.py index 3f7577fac6..7ad016828a 100644 --- a/frappe/frappeclient.py +++ b/frappe/frappeclient.py @@ -4,8 +4,6 @@ FrappeClient is a library that helps you connect with other frappe systems import base64 import json -import requests - import frappe from frappe.utils.data import cstr @@ -37,6 +35,8 @@ class FrappeClient: api_secret=None, frappe_authorization_source=None, ): + import requests + self.headers = { "Accept": "application/json", "content-type": "application/x-www-form-urlencoded", @@ -390,42 +390,13 @@ class FrappeClient: class FrappeOAuth2Client(FrappeClient): def __init__(self, url, access_token, verify=True): + import requests + self.access_token = access_token self.headers = { "Authorization": "Bearer " + access_token, "content-type": "application/x-www-form-urlencoded", } self.verify = verify - self.session = OAuth2Session(self.headers) + self.session = requests.session() self.url = url - - def get_request(self, params): - res = requests.get( - self.url, params=self.preprocess(params), headers=self.headers, verify=self.verify - ) - res = self.post_process(res) - return res - - def post_request(self, data): - res = requests.post( - self.url, data=self.preprocess(data), headers=self.headers, verify=self.verify - ) - res = self.post_process(res) - return res - - -class OAuth2Session: - def __init__(self, headers): - self.headers = headers - - def get(self, url, params, verify): - res = requests.get(url, params=params, headers=self.headers, verify=verify) - return res - - def post(self, url, data, verify): - res = requests.post(url, data=data, headers=self.headers, verify=verify) - return res - - def put(self, url, data, verify): - res = requests.put(url, data=data, headers=self.headers, verify=verify) - return res From 71b44efcac4af3c7381ebe817694939e03bef8a4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 11:42:08 +0530 Subject: [PATCH 58/72] perf: defer many requests imports --- frappe/core/doctype/file/utils.py | 5 +++-- .../doctype/slack_webhook_url/slack_webhook_url.py | 4 ++-- frappe/integrations/doctype/webhook/webhook.py | 10 +++++++--- frappe/integrations/google_oauth.py | 12 ++++++++---- frappe/utils/csvutils.py | 4 ++-- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/frappe/core/doctype/file/utils.py b/frappe/core/doctype/file/utils.py index 1d0d145303..7b44e94271 100644 --- a/frappe/core/doctype/file/utils.py +++ b/frappe/core/doctype/file/utils.py @@ -7,8 +7,6 @@ from typing import TYPE_CHECKING, Optional from urllib.parse import unquote import filetype -import requests -import requests.exceptions from PIL import Image import frappe @@ -116,6 +114,9 @@ def get_local_image(file_url: str) -> tuple["ImageFile", str, str]: def get_web_image(file_url: str) -> tuple["ImageFile", str, str]: + import requests + import requests.exceptions + # download file_url = frappe.utils.get_url(file_url) r = requests.get(file_url, stream=True) diff --git a/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py b/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py index d71d7075a6..4ea11aaccb 100644 --- a/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py +++ b/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py @@ -3,8 +3,6 @@ import json -import requests - import frappe from frappe import _ from frappe.model.document import Document @@ -24,6 +22,8 @@ class SlackWebhookURL(Document): def send_slack_message(webhook_url, message, reference_doctype, reference_name): + import requests + data = {"text": message, "attachments": []} slack_url, show_link = frappe.db.get_value( diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 6fa24bfb67..9e96870857 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -5,11 +5,10 @@ import base64 import hashlib import hmac import json +import typing from time import sleep from urllib.parse import urlparse -import requests - import frappe from frappe import _ from frappe.model.document import Document @@ -18,6 +17,9 @@ from frappe.utils.safe_exec import get_safe_globals WEBHOOK_SECRET_HEADER = "X-Frappe-Webhook-Signature" +if typing.TYPE_CHECKING: + import requests + class Webhook(Document): def validate(self): @@ -112,6 +114,8 @@ def get_context(doc): def enqueue_webhook(doc, webhook) -> None: + import requests + webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name")) headers = get_webhook_headers(doc, webhook) data = get_webhook_data(doc, webhook) @@ -154,7 +158,7 @@ def log_request( url: str, headers: dict, data: dict, - res: requests.Response | None = None, + res: typing.Optional["requests.Response"] = None, ): request_log = frappe.get_doc( { diff --git a/frappe/integrations/google_oauth.py b/frappe/integrations/google_oauth.py index 8bc54e0b1d..4a6f108150 100644 --- a/frappe/integrations/google_oauth.py +++ b/frappe/integrations/google_oauth.py @@ -2,7 +2,6 @@ import json from google.oauth2.credentials import Credentials from googleapiclient.discovery import build -from requests import get, post import frappe from frappe.utils import get_request_site_address @@ -56,6 +55,8 @@ class GoogleOAuth: frappe.throw(frappe._("Please update {} before continuing.").format(google_settings)) def authorize(self, oauth_code: str) -> dict[str, str | int]: + import requests + """Returns a dict with access and refresh token. :param oauth_code: code got back from google upon successful auhtorization @@ -73,13 +74,14 @@ class GoogleOAuth: } return handle_response( - post(self.OAUTH_URL, data=data).json(), + requests.post(self.OAUTH_URL, data=data).json(), "Google Oauth Authorization Error", "Something went wrong during the authorization.", ) def refresh_access_token(self, refresh_token: str) -> dict[str, str | int]: """Refreshes google access token using refresh token""" + import requests data = { "client_id": self.google_settings.client_id, @@ -92,7 +94,7 @@ class GoogleOAuth: } return handle_response( - post(self.OAUTH_URL, data=data).json(), + requests.post(self.OAUTH_URL, data=data).json(), "Google Oauth Access Token Refresh Error", "Something went wrong during the access token generation.", raise_err=True, @@ -158,7 +160,9 @@ def handle_response( def is_valid_access_token(access_token: str) -> bool: - response = get( + import requests + + response = requests.get( "https://oauth2.googleapis.com/tokeninfo", params={"access_token": access_token} ).json() diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index 4840c806bb..86a0e9776f 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -4,8 +4,6 @@ import csv import json from io import StringIO -import requests - import frappe from frappe import _, msgprint from frappe.utils import cint, comma_or, cstr, flt @@ -178,6 +176,8 @@ def getlink(doctype, name): def get_csv_content_from_google_sheets(url): + import requests + # https://docs.google.com/spreadsheets/d/{sheetid}}/edit#gid={gid} validate_google_sheets_url(url) # get gid, defaults to first sheet From 8a83226c608a8d5fdf66d7a499af6b0715c9c996 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 12:17:19 +0530 Subject: [PATCH 59/72] perf: defer GoogleOAuth import --- frappe/website/doctype/website_settings/website_settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/website_settings/website_settings.py b/frappe/website/doctype/website_settings/website_settings.py index ef48203cda..0295ac03e4 100644 --- a/frappe/website/doctype/website_settings/website_settings.py +++ b/frappe/website/doctype/website_settings/website_settings.py @@ -5,7 +5,6 @@ from urllib.parse import quote import frappe from frappe import _ -from frappe.integrations.google_oauth import GoogleOAuth from frappe.model.document import Document from frappe.utils import encode, get_request_site_address from frappe.website.utils import get_boot_data @@ -100,6 +99,8 @@ class WebsiteSettings(Document): frappe.clear_cache() def get_access_token(self): + from frappe.integrations.google_oauth import GoogleOAuth + if not self.indexing_refresh_token: button_label = frappe.bold(_("Allow API Indexing Access")) raise frappe.ValidationError(_("Click on {0} to generate Refresh Token.").format(button_label)) From 45f8aff909c8ebc257a100dada6f53f211f4d79a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 12:28:20 +0530 Subject: [PATCH 60/72] perf: defer LDAP import --- .../core/doctype/error_log/test_error_log.py | 9 ++++++++ frappe/utils/error.py | 21 +++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/error_log/test_error_log.py b/frappe/core/doctype/error_log/test_error_log.py index 59670de8d2..3f19a6dd0c 100644 --- a/frappe/core/doctype/error_log/test_error_log.py +++ b/frappe/core/doctype/error_log/test_error_log.py @@ -1,7 +1,10 @@ # Copyright (c) 2015, Frappe Technologies and Contributors # License: MIT. See LICENSE +from ldap3.core.exceptions import LDAPException, LDAPInappropriateAuthenticationResult + import frappe from frappe.tests.utils import FrappeTestCase +from frappe.utils.error import _is_ldap_exception # test_records = frappe.get_test_records('Error Log') @@ -12,3 +15,9 @@ class TestErrorLog(FrappeTestCase): doc = frappe.new_doc("Error Log") error = doc.log_error("This is an error") self.assertEqual(error.doctype, "Error Log") + + def test_ldap_exceptions(self): + exc = [LDAPException, LDAPInappropriateAuthenticationResult] + + for e in exc: + self.assertTrue(_is_ldap_exception(e())) diff --git a/frappe/utils/error.py b/frappe/utils/error.py index 47c5b055a8..323b0e2746 100644 --- a/frappe/utils/error.py +++ b/frappe/utils/error.py @@ -11,8 +11,6 @@ import pydoc import sys import traceback -from ldap3.core.exceptions import LDAPException - import frappe from frappe.utils import cstr, encode @@ -20,16 +18,31 @@ EXCLUDE_EXCEPTIONS = ( frappe.AuthenticationError, frappe.CSRFTokenError, # CSRF covers OAuth too frappe.SecurityException, - LDAPException, frappe.InReadOnlyMode, ) +LDAP_BASE_EXCEPTION = "LDAPException" + + +def _is_ldap_exception(e): + """Check if exception is from LDAP library. + + This is a hack but ensures that LDAP is not imported unless it's required. This is tested in + unittests in case the exception changes in future. + """ + + for t in type(e).__mro__: + if t.__name__ == LDAP_BASE_EXCEPTION: + return True + + return False + def make_error_snapshot(exception): if frappe.conf.disable_error_snapshot: return - if isinstance(exception, EXCLUDE_EXCEPTIONS): + if isinstance(exception, EXCLUDE_EXCEPTIONS) or _is_ldap_exception(exception): return logger = frappe.logger(with_more_info=True) From 4c08689744a2628695a1240648e6ae4439ef6c20 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 12:32:25 +0530 Subject: [PATCH 61/72] perf: defer psutil import --- frappe/build.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/build.py b/frappe/build.py index 66b3eabd5e..18ceff1d7a 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -9,7 +9,6 @@ from tempfile import mkdtemp, mktemp from urllib.parse import urlparse import click -import psutil from semantic_version import Version import frappe @@ -288,6 +287,8 @@ def get_node_env(): def get_safe_max_old_space_size(): + import psutil + safe_max_old_space_size = 0 try: total_memory = psutil.virtual_memory().total / (1024 * 1024) From 6a9c9bd89db0132290726b525d3ab2ade217a78d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 12:36:19 +0530 Subject: [PATCH 62/72] perf: defer pydoc import --- frappe/utils/error.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/utils/error.py b/frappe/utils/error.py index 323b0e2746..a9891cb532 100644 --- a/frappe/utils/error.py +++ b/frappe/utils/error.py @@ -7,7 +7,6 @@ import inspect import json import linecache import os -import pydoc import sys import traceback @@ -69,6 +68,8 @@ def make_error_snapshot(exception): def get_snapshot(exception, context=10): + import pydoc + """ Return a dict describing a given traceback (based on cgitb.text) """ From 278b6fc85af5766d9b7f0e013fdd23bb22f8f5f2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 12:46:52 +0530 Subject: [PATCH 63/72] perf: defer QR code import --- frappe/twofactor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/twofactor.py b/frappe/twofactor.py index 65f94cae90..bb37c2a593 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -5,7 +5,6 @@ from base64 import b32encode, b64encode from io import BytesIO import pyotp -from pyqrcode import create as qrcreate import frappe import frappe.defaults @@ -387,6 +386,8 @@ def send_token_via_email(user, token, otp_secret, otp_issuer, subject=None, mess def get_qr_svg_code(totp_uri): """Get SVG code to display Qrcode for OTP.""" + from pyqrcode import create as qrcreate + url = qrcreate(totp_uri) svg = "" stream = BytesIO() From 423e78132657b7731d272ed22903c57e061de6bf Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 14:29:55 +0530 Subject: [PATCH 64/72] perf: Freeze GC before forking Gunicorn workers --- frappe/app.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/frappe/app.py b/frappe/app.py index ddde313ace..6883b42c51 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import gc import logging import os @@ -394,3 +395,15 @@ def serve( use_evalex=not in_test_env, threaded=not no_threading, ) + + +# Both Gunicorn and RQ use forking to spawn workers. In an ideal world, the fork should be sharing +# most of the memory if there are no writes made to data because of Copy on Write, however, +# python's GC is not CoW friendly and writes to data even if user-code doesn't. Specifically, the +# generational GC which stores and mutates every python object: `PyGC_Head` +# +# Calling gc.freeze() moves all the objects imported so far into permanant generation and hence +# doesn't mutate `PyGC_Head` +# +# Refer to issue for more info: https://github.com/frappe/frappe/issues/18927 +gc.freeze() From cc43af984ed82976a681641cdad96e309af66976 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 16:05:45 +0530 Subject: [PATCH 65/72] fix: Only import LDAP if it's enabled This is disabled on 99%+ sites, still incurs 3-4MB of memory hit on import of ldap3. --- frappe/www/login.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/www/login.py b/frappe/www/login.py index 0aa1eba0cd..7cf89e7c3c 100644 --- a/frappe/www/login.py +++ b/frappe/www/login.py @@ -5,7 +5,6 @@ import frappe import frappe.utils from frappe import _ from frappe.auth import LoginManager -from frappe.integrations.doctype.ldap_settings.ldap_settings import LDAPSettings from frappe.rate_limiter import rate_limit from frappe.utils import cint, get_url from frappe.utils.data import escape_html @@ -85,7 +84,10 @@ def get_context(context): ) context["social_login"] = True - context["ldap_settings"] = LDAPSettings.get_ldap_client_settings() + if cint(frappe.db.get_value("LDAP Settings", "LDAP Settings", "enabled")): + from frappe.integrations.doctype.ldap_settings.ldap_settings import LDAPSettings + + context["ldap_settings"] = LDAPSettings.get_ldap_client_settings() login_label = [_("Email")] From 32bd5d3e2c5fb4970293e3247e1f8d919ab1c34d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 16:26:22 +0530 Subject: [PATCH 66/72] Revert "perf: defer many requests imports" This reverts commit 71b44efcac4af3c7381ebe817694939e03bef8a4. This gets frequently imported from one place or another. Since with gc.freeze we can mostly reuse the import from parent, let's just leave it here. --- frappe/core/doctype/file/utils.py | 5 ++--- .../doctype/slack_webhook_url/slack_webhook_url.py | 4 ++-- frappe/integrations/doctype/webhook/webhook.py | 10 +++------- frappe/integrations/google_oauth.py | 12 ++++-------- frappe/utils/csvutils.py | 4 ++-- 5 files changed, 13 insertions(+), 22 deletions(-) diff --git a/frappe/core/doctype/file/utils.py b/frappe/core/doctype/file/utils.py index 7b44e94271..1d0d145303 100644 --- a/frappe/core/doctype/file/utils.py +++ b/frappe/core/doctype/file/utils.py @@ -7,6 +7,8 @@ from typing import TYPE_CHECKING, Optional from urllib.parse import unquote import filetype +import requests +import requests.exceptions from PIL import Image import frappe @@ -114,9 +116,6 @@ def get_local_image(file_url: str) -> tuple["ImageFile", str, str]: def get_web_image(file_url: str) -> tuple["ImageFile", str, str]: - import requests - import requests.exceptions - # download file_url = frappe.utils.get_url(file_url) r = requests.get(file_url, stream=True) diff --git a/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py b/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py index 4ea11aaccb..d71d7075a6 100644 --- a/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py +++ b/frappe/integrations/doctype/slack_webhook_url/slack_webhook_url.py @@ -3,6 +3,8 @@ import json +import requests + import frappe from frappe import _ from frappe.model.document import Document @@ -22,8 +24,6 @@ class SlackWebhookURL(Document): def send_slack_message(webhook_url, message, reference_doctype, reference_name): - import requests - data = {"text": message, "attachments": []} slack_url, show_link = frappe.db.get_value( diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 9e96870857..6fa24bfb67 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -5,10 +5,11 @@ import base64 import hashlib import hmac import json -import typing from time import sleep from urllib.parse import urlparse +import requests + import frappe from frappe import _ from frappe.model.document import Document @@ -17,9 +18,6 @@ from frappe.utils.safe_exec import get_safe_globals WEBHOOK_SECRET_HEADER = "X-Frappe-Webhook-Signature" -if typing.TYPE_CHECKING: - import requests - class Webhook(Document): def validate(self): @@ -114,8 +112,6 @@ def get_context(doc): def enqueue_webhook(doc, webhook) -> None: - import requests - webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name")) headers = get_webhook_headers(doc, webhook) data = get_webhook_data(doc, webhook) @@ -158,7 +154,7 @@ def log_request( url: str, headers: dict, data: dict, - res: typing.Optional["requests.Response"] = None, + res: requests.Response | None = None, ): request_log = frappe.get_doc( { diff --git a/frappe/integrations/google_oauth.py b/frappe/integrations/google_oauth.py index 4a6f108150..8bc54e0b1d 100644 --- a/frappe/integrations/google_oauth.py +++ b/frappe/integrations/google_oauth.py @@ -2,6 +2,7 @@ import json from google.oauth2.credentials import Credentials from googleapiclient.discovery import build +from requests import get, post import frappe from frappe.utils import get_request_site_address @@ -55,8 +56,6 @@ class GoogleOAuth: frappe.throw(frappe._("Please update {} before continuing.").format(google_settings)) def authorize(self, oauth_code: str) -> dict[str, str | int]: - import requests - """Returns a dict with access and refresh token. :param oauth_code: code got back from google upon successful auhtorization @@ -74,14 +73,13 @@ class GoogleOAuth: } return handle_response( - requests.post(self.OAUTH_URL, data=data).json(), + post(self.OAUTH_URL, data=data).json(), "Google Oauth Authorization Error", "Something went wrong during the authorization.", ) def refresh_access_token(self, refresh_token: str) -> dict[str, str | int]: """Refreshes google access token using refresh token""" - import requests data = { "client_id": self.google_settings.client_id, @@ -94,7 +92,7 @@ class GoogleOAuth: } return handle_response( - requests.post(self.OAUTH_URL, data=data).json(), + post(self.OAUTH_URL, data=data).json(), "Google Oauth Access Token Refresh Error", "Something went wrong during the access token generation.", raise_err=True, @@ -160,9 +158,7 @@ def handle_response( def is_valid_access_token(access_token: str) -> bool: - import requests - - response = requests.get( + response = get( "https://oauth2.googleapis.com/tokeninfo", params={"access_token": access_token} ).json() diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index 86a0e9776f..4840c806bb 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -4,6 +4,8 @@ import csv import json from io import StringIO +import requests + import frappe from frappe import _, msgprint from frappe.utils import cint, comma_or, cstr, flt @@ -176,8 +178,6 @@ def getlink(doctype, name): def get_csv_content_from_google_sheets(url): - import requests - # https://docs.google.com/spreadsheets/d/{sheetid}}/edit#gid={gid} validate_google_sheets_url(url) # get gid, defaults to first sheet From 4f0a2e6e9c293417be9e17711d1c4985db6c684b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 16:33:32 +0530 Subject: [PATCH 67/72] fix: move gc.freeze behind environ variable --- frappe/app.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/app.py b/frappe/app.py index 6883b42c51..af3cb89bfa 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -406,4 +406,5 @@ def serve( # doesn't mutate `PyGC_Head` # # Refer to issue for more info: https://github.com/frappe/frappe/issues/18927 -gc.freeze() +if bool(os.environ.get("FRAPPE_TUNE_GC", False)): + gc.freeze() From fbe3174914e21dffa6dc403cdec2026ed8119076 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 16:40:49 +0530 Subject: [PATCH 68/72] perf: Bump alloc count to 7,000 for generation 0 This has overall 1-2% CPU usage reduction for little to no costs. Benefits increase when doing bulk processing with lots of objects. --- frappe/__init__.py | 12 ++++++++++++ frappe/app.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index a4ac8111dc..998d881a13 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -11,6 +11,7 @@ be used to build database driven apps. Read the documentation: https://frappeframework.com/docs """ import functools +import gc import importlib import inspect import json @@ -57,6 +58,7 @@ re._MAXCACHE = ( 50 # reduced from default 512 given we are already maintaining this on parent worker ) +_tune_gc = bool(os.environ.get("FRAPPE_TUNE_GC", False)) if _dev_server: warnings.simplefilter("always", DeprecationWarning) @@ -2435,3 +2437,13 @@ def validate_and_sanitize_search_inputs(fn): return fn(**kwargs) return wrapper + + +if _tune_gc: + # generational GC gets triggered after certain allocs (g0) which is 700 by default. + # This number is quite small for frappe where a single query can potentially create 700+ + # objects easily. + # Bump this number higher, this will make GC less aggressive but that improves performance of + # everything else. + g0, g1, g2 = gc.get_threshold() # defaults are 700, 10, 10. + gc.set_threshold(g0 * 10, g1 * 2, g2 * 2) diff --git a/frappe/app.py b/frappe/app.py index af3cb89bfa..e031d6e84b 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -406,5 +406,5 @@ def serve( # doesn't mutate `PyGC_Head` # # Refer to issue for more info: https://github.com/frappe/frappe/issues/18927 -if bool(os.environ.get("FRAPPE_TUNE_GC", False)): +if frappe._tune_gc: gc.freeze() From 29d28a460f1e7bb14595e827b43c0ba9c57b8fe9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 17:06:23 +0530 Subject: [PATCH 69/72] perf: Freeze GC right before starting background worker BG worker forks are not CoW friendly. Freezing right before we start worker should lessen overall memory usage. Though this isn't useful much because at max you're sharing with 2 processes - master and horse. WorkerPool can improve this benefit a lot by forking each worker from master process and horse from forked processes. TBD when WorkerPool is out of beta. --- frappe/utils/background_jobs.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index ea3ea4c191..9b2ff329df 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -1,3 +1,4 @@ +import gc import os import socket import time @@ -234,6 +235,9 @@ def start_worker( """Wrapper to start rq worker. Connects to redis and monitors these queues.""" DEQUEUE_STRATEGIES = {"round_robin": RoundRobinWorker, "random": RandomWorker} + if frappe._tune_gc: + gc.freeze() + with frappe.init_site(): # empty init is required to get redis_queue from common_site_config.json redis_connection = get_redis_conn(username=rq_username, password=rq_password) From 150c36c74d99e533673c1725f6e766ecf266bcf6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 17:36:10 +0530 Subject: [PATCH 70/72] fix: collect before freezing --- frappe/app.py | 1 + frappe/utils/background_jobs.py | 1 + 2 files changed, 2 insertions(+) diff --git a/frappe/app.py b/frappe/app.py index e031d6e84b..a698331e8e 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -407,4 +407,5 @@ def serve( # # Refer to issue for more info: https://github.com/frappe/frappe/issues/18927 if frappe._tune_gc: + gc.collect() # clean up any garbage created so far before freeze gc.freeze() diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index 9b2ff329df..6b0249e720 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -236,6 +236,7 @@ def start_worker( DEQUEUE_STRATEGIES = {"round_robin": RoundRobinWorker, "random": RandomWorker} if frappe._tune_gc: + gc.collect() gc.freeze() with frappe.init_site(): From 793f4ebba3a4d11dc1a74130234e825fcb6aaa42 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 19:37:24 +0530 Subject: [PATCH 71/72] perf: defer loading JWT --- frappe/tests/test_oauth20.py | 3 ++- frappe/utils/oauth.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_oauth20.py b/frappe/tests/test_oauth20.py index 52e9f1b0c5..fb064eb82d 100644 --- a/frappe/tests/test_oauth20.py +++ b/frappe/tests/test_oauth20.py @@ -4,7 +4,6 @@ from typing import TYPE_CHECKING from urllib.parse import parse_qs, urljoin, urlparse -import jwt import requests from werkzeug.test import TestResponse @@ -362,6 +361,8 @@ class TestOAuth20(FrappeRequestTestCase): self.assertTrue(payload.get("nonce") == nonce) def decode_id_token(self, id_token): + import jwt + return jwt.decode( id_token, audience=self.client_id, diff --git a/frappe/utils/oauth.py b/frappe/utils/oauth.py index 6a2b87d29f..611ea2967c 100644 --- a/frappe/utils/oauth.py +++ b/frappe/utils/oauth.py @@ -5,8 +5,6 @@ import base64 import json from typing import TYPE_CHECKING, Callable -import jwt - import frappe import frappe.utils from frappe import _ @@ -126,6 +124,9 @@ def login_via_oauth2_id_token( def get_info_via_oauth( provider: str, code: str, decoder: Callable | None = None, id_token: bool = False ): + + import jwt + flow = get_oauth2_flow(provider) oauth2_providers = get_oauth2_providers() From af03b76c883aec62eac5567f6652e50ce992aa26 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 24 Jun 2023 18:28:04 +0530 Subject: [PATCH 72/72] perf: Preload and share common python modules --- .github/helper/install.sh | 2 ++ frappe/app.py | 24 ++++++++++++++++++++++++ frappe/utils/data.py | 18 +++--------------- frappe/utils/telemetry.py | 4 +++- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/.github/helper/install.sh b/.github/helper/install.sh index 5cdcbebe1a..5a4d341a9b 100644 --- a/.github/helper/install.sh +++ b/.github/helper/install.sh @@ -54,6 +54,8 @@ fi echo "Starting Bench..." +export FRAPPE_TUNE_GC=True + bench start &> ~/frappe-bench/bench_start.log & if [ "$TYPE" == "server" ] diff --git a/frappe/app.py b/frappe/app.py index a698331e8e..5113c858a5 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -31,6 +31,30 @@ _site = None _sites_path = os.environ.get("SITES_PATH", ".") +# If gc.freeze is done then importing modules before forking allows us to share the memory +if frappe._tune_gc: + import frappe.boot + import frappe.client + import frappe.core.doctype.user.user + import frappe.database.mariadb.database # Load database related utils + import frappe.database.query + import frappe.desk.desktop # workspace + import frappe.model.db_query + import frappe.query_builder + import frappe.utils.background_jobs # Enqueue is very common + import frappe.utils.data # common utils + import frappe.utils.jinja # web page rendering + import frappe.utils.jinja_globals + import frappe.utils.redis_wrapper # Exact redis_wrapper + import frappe.utils.safe_exec + import frappe.utils.typing_validations # any whitelisted method uses this + import frappe.website.path_resolver # all the page types and resolver + import frappe.website.router # Website router + import frappe.website.website_generator # web page doctypes + +# end: module pre-loading + + @local_manager.middleware @Request.application def application(request: Request): diff --git a/frappe/utils/data.py b/frappe/utils/data.py index eeabfad559..c42a741017 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -15,6 +15,9 @@ from typing import Any, Literal, Optional, TypeVar, Union from urllib.parse import parse_qsl, quote, urlencode, urljoin, urlparse, urlunparse from click import secho +from dateutil import parser +from dateutil.parser import ParserError +from dateutil.relativedelta import relativedelta import frappe from frappe.desk.utils import slug @@ -80,9 +83,6 @@ def getdate( Converts string date (yyyy-mm-dd) to datetime.date object. If no input is provided, current date is returned. """ - from dateutil import parser - from dateutil.parser._parser import ParserError - if not string_date: return get_datetime().date() if isinstance(string_date, datetime.datetime): @@ -105,7 +105,6 @@ def getdate( def get_datetime( datetime_str: Optional["DateTimeLikeObject"] = None, ) -> datetime.datetime | None: - from dateutil import parser if datetime_str is None: return now_datetime() @@ -141,9 +140,6 @@ def get_timedelta(time: str | None = None) -> datetime.timedelta | None: Returns: datetime.timedelta: Timedelta object equivalent of the passed `time` string """ - from dateutil import parser - from dateutil.parser import ParserError - time = time or "0:0:0" try: @@ -161,8 +157,6 @@ def get_timedelta(time: str | None = None) -> datetime.timedelta | None: def to_timedelta(time_str: str | datetime.time) -> datetime.timedelta: - from dateutil import parser - if isinstance(time_str, datetime.time): time_str = str(time_str) @@ -237,9 +231,6 @@ def add_to_date( as_datetime=False, ) -> DateTimeLikeObject: """Adds `days` to the given date""" - from dateutil import parser - from dateutil.parser._parser import ParserError - from dateutil.relativedelta import relativedelta if date is None: date = now_datetime() @@ -500,9 +491,6 @@ def get_year_ending(date) -> datetime.date: def get_time(time_str: str) -> datetime.time: - from dateutil import parser - from dateutil.parser import ParserError - if isinstance(time_str, datetime.datetime): return time_str.time() elif isinstance(time_str, datetime.time): diff --git a/frappe/utils/telemetry.py b/frappe/utils/telemetry.py index 5f3a7ee0c9..965b26ca21 100644 --- a/frappe/utils/telemetry.py +++ b/frappe/utils/telemetry.py @@ -8,7 +8,9 @@ from contextlib import suppress import frappe from frappe.utils import getdate from frappe.utils.caching import site_cache -from posthog import Posthog + +from posthog import Posthog # isort: skip + POSTHOG_PROJECT_FIELD = "posthog_project_id" POSTHOG_HOST_FIELD = "posthog_host"