From 5bd3fd8d6515b45df11295dad8eaed64fffa3e56 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 May 2022 18:18:03 +0530 Subject: [PATCH 01/62] fix: prioritize report custom roles --- frappe/core/doctype/report/report.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 9e6cc73f11..efb45f41c8 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -89,7 +89,9 @@ class Report(Document): ] custom_roles = get_custom_allowed_roles("report", self.name) - allowed.extend(custom_roles) + + if custom_roles: + allowed = custom_roles if not allowed: return True From a3d9d8028f03009ecc6ceab490b4967d310232e0 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 May 2022 20:14:05 +0530 Subject: [PATCH 02/62] test: added unit test for report custom role permissions --- frappe/core/doctype/report/test_report.py | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/frappe/core/doctype/report/test_report.py b/frappe/core/doctype/report/test_report.py index 7b17a5a8d5..e490dd5c01 100644 --- a/frappe/core/doctype/report/test_report.py +++ b/frappe/core/doctype/report/test_report.py @@ -186,6 +186,38 @@ class TestReport(FrappeTestCase): self.assertNotEqual(report.is_permitted(), True) frappe.set_user("Administrator") + def test_report_custom_permissions(self): + frappe.set_user("test@example.com") + frappe.db.delete("Custom Role", {"report": "Test Custom Role Report"}) + frappe.db.commit() + if not frappe.db.exists("Report", "Test Custom Role Report"): + report = frappe.get_doc( + { + "doctype": "Report", + "ref_doctype": "User", + "report_name": "Test Custom Role Report", + "report_type": "Query Report", + "is_standard": "No", + "roles": [{"role": "_Test Role"}, {"role": "System Manager"}], + } + ).insert(ignore_permissions=True) + else: + report = frappe.get_doc("Report", "Test Custom Role Report") + + self.assertEqual(report.is_permitted(), True) + + custom_role = frappe.get_doc( + { + "doctype": "Custom Role", + "report": "Test Custom Role Report", + "roles": [{"role": "_Test Role 2"}], + "ref_doctype": "User", + } + ).insert(ignore_permissions=True) + + self.assertNotEqual(report.is_permitted(), True) + frappe.set_user("Administrator") + # test for the `_format` method if report data doesn't have sort_by parameter def test_format_method(self): if frappe.db.exists("Report", "User Activity Report Without Sort"): From 95921c870762a9ef7fbff0a7f9f184df2e223fd5 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 24 May 2022 21:49:49 +0530 Subject: [PATCH 03/62] chore: removed unused variable & added nosemgrep on db commit --- frappe/core/doctype/report/test_report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/report/test_report.py b/frappe/core/doctype/report/test_report.py index e490dd5c01..bbae616e93 100644 --- a/frappe/core/doctype/report/test_report.py +++ b/frappe/core/doctype/report/test_report.py @@ -189,7 +189,7 @@ class TestReport(FrappeTestCase): def test_report_custom_permissions(self): frappe.set_user("test@example.com") frappe.db.delete("Custom Role", {"report": "Test Custom Role Report"}) - frappe.db.commit() + frappe.db.commit() # nosemgrep if not frappe.db.exists("Report", "Test Custom Role Report"): report = frappe.get_doc( { @@ -206,7 +206,7 @@ class TestReport(FrappeTestCase): self.assertEqual(report.is_permitted(), True) - custom_role = frappe.get_doc( + frappe.get_doc( { "doctype": "Custom Role", "report": "Test Custom Role Report", From a0e8983199cf9c4f176377458025dafb6929b0e6 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 24 May 2022 19:23:45 +0200 Subject: [PATCH 04/62] feat: delete User Permission when User is deleted --- frappe/core/doctype/user/user.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 1ff5c98a91..1413e0449f 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -424,6 +424,9 @@ class User(Document): frappe.cache().delete_key("enabled_users") + # delete user permissions + frappe.db.delete("User Permission", {"user": self.name}) + def before_rename(self, old_name, new_name, merge=False): frappe.clear_cache(user=old_name) self.validate_rename(old_name, new_name) From 9ffb0db80c0680ebd35a566bd21e7c19483dd732 Mon Sep 17 00:00:00 2001 From: Byju Abraham <65185610+polycurve@users.noreply.github.com> Date: Wed, 25 May 2022 15:14:31 +0530 Subject: [PATCH 05/62] Fix: Change currency fraction for Singapore from Sen to Cent (#16987) --- frappe/geo/country_info.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/geo/country_info.json b/frappe/geo/country_info.json index 735dcddac3..94d1f3ed37 100644 --- a/frappe/geo/country_info.json +++ b/frappe/geo/country_info.json @@ -2412,7 +2412,7 @@ "Singapore": { "code": "sg", "currency": "SGD", - "currency_fraction": "Sen", + "currency_fraction": "Cent", "currency_fraction_units": 100, "currency_name": "Singapore Dollar", "currency_symbol": "$", From e42fa8de056acb3c76b3f2096ee02758ea131f52 Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Wed, 25 May 2022 15:18:08 +0530 Subject: [PATCH 06/62] test: fix newsletter tests setup due to duplicate entry of email group member for postgres (#16989) --- frappe/email/doctype/newsletter/test_newsletter.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index 9ec61194ef..0cf388564f 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/frappe/email/doctype/newsletter/test_newsletter.py @@ -1,7 +1,6 @@ # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See LICENSE -import unittest from random import choice from typing import Union from unittest.mock import MagicMock, PropertyMock, patch @@ -17,9 +16,9 @@ from frappe.email.doctype.newsletter.newsletter import ( send_scheduled_email, ) from frappe.email.queue import flush +from frappe.tests.utils import FrappeTestCase from frappe.utils import add_days, getdate -test_dependencies = ["Email Group"] emails = [ "test_subscriber1@example.com", "test_subscriber2@example.com", @@ -63,6 +62,10 @@ class TestNewsletterMixin: for email in emails: doctype = "Email Group Member" email_filters = {"email": email, "email_group": "_Test Email Group"} + + savepoint = "setup_email_group" + frappe.db.savepoint(savepoint) + try: frappe.get_doc( { @@ -71,8 +74,11 @@ class TestNewsletterMixin: } ).insert() except Exception: + frappe.db.rollback(save_point=savepoint) frappe.db.update(doctype, email_filters, "unsubscribed", 0) + frappe.db.release_savepoint(savepoint) + def send_newsletter(self, published=0, schedule_send=None) -> Union[str, None]: frappe.db.delete("Email Queue") frappe.db.delete("Email Queue Recipient") @@ -127,7 +133,7 @@ class TestNewsletterMixin: return newsletter -class TestNewsletter(TestNewsletterMixin, unittest.TestCase): +class TestNewsletter(TestNewsletterMixin, FrappeTestCase): def test_send(self): self.send_newsletter() From c38219627c99a71e3627426e0622eaf0e26f5c2b Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Wed, 25 May 2022 11:50:28 +0200 Subject: [PATCH 07/62] feat: Display button "Create User Email" only if current user can create an Email Account (#16979) --- frappe/core/doctype/user/user.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index 77c199cdd4..6bbab0fdb3 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -194,14 +194,14 @@ frappe.ui.form.on('User', { } } } - if (frm.doc.user_emails){ - var found =0; - for (var i = 0;i Date: Wed, 25 May 2022 15:22:30 +0530 Subject: [PATCH 08/62] test: flaky test due to stale translation cache (#16985) --- frappe/tests/utils.py | 2 ++ .../test_personal_data_download_request.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index 7d00a0c1f9..fc26694d46 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -82,6 +82,8 @@ def _restore_thread_locals(flags): frappe.local.realtime_log = [] frappe.local.conf = frappe._dict(frappe.get_site_config()) frappe.local.cache = {} + frappe.local.lang = "en" + frappe.local.lang_full_dict = None @contextmanager diff --git a/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py b/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py index 4f115325df..6518cda5ed 100644 --- a/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py +++ b/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py @@ -2,17 +2,17 @@ # Copyright (c) 2019, Frappe Technologies and Contributors # License: MIT. See LICENSE import json -import unittest import frappe from frappe.contacts.doctype.contact.contact import get_contact_name from frappe.core.doctype.user.user import create_contact +from frappe.tests.utils import FrappeTestCase from frappe.website.doctype.personal_data_download_request.personal_data_download_request import ( get_user_data, ) -class TestRequestPersonalData(unittest.TestCase): +class TestRequestPersonalData(FrappeTestCase): def setUp(self): create_user_if_not_exists(email="test_privacy@example.com") @@ -48,7 +48,7 @@ class TestRequestPersonalData(unittest.TestCase): email_queue = frappe.get_all( "Email Queue", fields=["message"], order_by="creation DESC", limit=1 ) - self.assertTrue("Subject: Download Your Data" in email_queue[0].message) + self.assertIn(frappe._("Download Your Data"), email_queue[0].message) frappe.db.delete("Email Queue") From 8557cff2bb45b1c20a6beb4b36f6ff83e73fbc03 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 22 May 2022 22:30:29 +0530 Subject: [PATCH 09/62] perf: faster auth ~ validate_ip_address from redis --- frappe/auth.py | 14 ++++++++++---- frappe/core/doctype/user/user.py | 12 ++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index dc53c20f28..9bab8b8bf3 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -412,10 +412,16 @@ def clear_cookies(): def validate_ip_address(user): """check if IP Address is valid""" - user = ( - frappe.get_cached_doc("User", user) if not frappe.flags.in_test else frappe.get_doc("User", user) + from frappe.core.doctype.user.user import get_restricted_ip_list + + # Only fetch required fields - for perf + user_fields = ["restrict_ip", "bypass_restrict_ip_check_if_2fa_enabled"] + user_info = ( + frappe.get_cached_value("User", user, user_fields, as_dict=True) + if not frappe.flags.in_test + else frappe.db.get_value("User", user, user_fields, as_dict=True) ) - ip_list = user.get_restricted_ip_list() + ip_list = get_restricted_ip_list(user_info) if not ip_list: return @@ -430,7 +436,7 @@ def validate_ip_address(user): # check if two factor auth is enabled if system_settings.enable_two_factor_auth and not bypass_restrict_ip_check: # check if bypass restrict ip is enabled for login user - bypass_restrict_ip_check = user.bypass_restrict_ip_check_if_2fa_enabled + bypass_restrict_ip_check = user_info.bypass_restrict_ip_check_if_2fa_enabled for ip in ip_list: if frappe.local.request_ip.startswith(ip) or bypass_restrict_ip_check: diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 1ff5c98a91..d9c67aaaaf 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -586,10 +586,7 @@ class User(Document): self.append("social_logins", social_logins) def get_restricted_ip_list(self): - if not self.restrict_ip: - return - - return [i.strip() for i in self.restrict_ip.split(",")] + return get_restricted_ip_list(self) @classmethod def find_by_credentials(cls, user_name: str, password: str, validate_password: bool = True): @@ -1156,6 +1153,13 @@ def create_contact(user, ignore_links=False, ignore_mandatory=False): contact.save(ignore_permissions=True) +def get_restricted_ip_list(user): + if not user.restrict_ip: + return + + return [i.strip() for i in user.restrict_ip.split(",")] + + @frappe.whitelist() def generate_keys(user): """ From 5c9421b750e9baf7f192ceb48763ab658c663ca2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 23 May 2022 11:26:39 +0530 Subject: [PATCH 10/62] perf: use redis cache for user_info --- frappe/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/auth.py b/frappe/auth.py index 9bab8b8bf3..80141d1d6c 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -165,7 +165,7 @@ class LoginManager: self.set_user_info() def get_user_info(self): - self.info = frappe.db.get_value( + self.info = frappe.get_cached_value( "User", self.user, ["user_type", "first_name", "last_name", "user_image"], as_dict=1 ) From 96b30e714c19fa935187f344a46fd306cdcb21ce Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 27 May 2022 16:04:22 +0530 Subject: [PATCH 11/62] feat: table_field.fieldname field syntax for db_query --- frappe/desk/reportview.py | 13 +++++++++++-- frappe/model/db_query.py | 15 +++++++++++++++ frappe/tests/test_db_query.py | 16 ++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index b45f80f6ff..cf34e1e986 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -171,7 +171,7 @@ def raise_invalid_field(fieldname): def is_standard(fieldname): if "." in fieldname: - parenttype, fieldname = get_parenttype_and_fieldname(fieldname, None) + fieldname = fieldname.split(".")[1].strip("`") return ( fieldname in default_fields or fieldname in optional_fields or fieldname in child_table_fields ) @@ -235,7 +235,16 @@ def parse_json(data): def get_parenttype_and_fieldname(field, data): if "." in field: - parenttype, fieldname = field.split(".")[0][4:-1], field.split(".")[1].strip("`") + parts = field.split(".") + parenttype = parts[0] + fieldname = parts[1] + if parenttype.startswith("`tab"): + # `tabChild DocType`.`fieldname` + parenttype = parenttype[4:-1] + fieldname = fieldname.strip("`") + else: + # tablefield.fieldname + parenttype = frappe.get_meta(data.doctype).get_field(parenttype).options else: parenttype = data.doctype fieldname = field.strip("`") diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 54331f5124..8b2475de52 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -287,6 +287,21 @@ class DatabaseQuery(object): # remove empty strings / nulls in fields self.fields = [f for f in self.fields if f] + # convert child_table.fieldname to `tabChild DocType`.`fieldname` + for field in self.fields: + if "." in field and "tab" not in field: + original_field = field + alias = None + if " as " in field: + field, alias = field.split(" as ") + tablefield, fieldname = field.split(".") + child_doctype = frappe.get_meta(self.doctype).get_field(tablefield).options + field = field.replace(tablefield, f"`tab{child_doctype}`") + field = field.replace(fieldname, f"`{fieldname}`") + if alias: + field = f"{field} as {alias}" + self.fields[self.fields.index(original_field)] = field + for filter_name in ["filters", "or_filters"]: filters = getattr(self, filter_name) if isinstance(filters, str): diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index dd67d68cd2..e2a32a020c 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -35,6 +35,22 @@ class TestReportview(unittest.TestCase): clear_custom_fields("DocType") + def test_child_table_field_syntax(self): + note = frappe.get_doc( + doctype="Note", + title=f"Test {frappe.utils.random_string(8)}", + content="test", + seen_by=[{"user": "Administrator"}], + ).insert() + result = frappe.db.get_all( + "Note", + filters={"name": note.name}, + fields=["name", "seen_by.user as seen_by"], + limit=1, + ) + self.assertEqual(result[0].seen_by, "Administrator") + note.delete() + def test_build_match_conditions(self): clear_user_permissions_for_doctype("Blog Post", "test2@example.com") From f416e2a1d2ddeaa314cc6b2ec31c1e5ec727e1dc Mon Sep 17 00:00:00 2001 From: gavin Date: Fri, 27 May 2022 16:36:37 +0530 Subject: [PATCH 12/62] feat(website-settings): Configurable Splash Image Configurable Splash Image option which will override app's splash_image hook set. --- .../doctype/website_settings/website_settings.json | 10 ++++++++-- .../doctype/website_settings/website_settings.py | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/frappe/website/doctype/website_settings/website_settings.json b/frappe/website/doctype/website_settings/website_settings.json index b628437315..aa17fa261f 100644 --- a/frappe/website/doctype/website_settings/website_settings.json +++ b/frappe/website/doctype/website_settings/website_settings.json @@ -21,6 +21,7 @@ "website_theme_image_link", "brand", "banner_image", + "splash_image", "brand_html", "set_banner_from_image", "favicon", @@ -413,6 +414,11 @@ "fieldname": "footer_powered", "fieldtype": "Small Text", "label": "Footer \"Powered By\"" + }, + { + "fieldname": "splash_image", + "fieldtype": "Attach Image", + "label": "Splash Image" } ], "icon": "fa fa-cog", @@ -420,7 +426,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-03-09 01:47:31.094462", + "modified": "2022-05-27 12:33:29.019998", "modified_by": "Administrator", "module": "Website", "name": "Website Settings", @@ -445,4 +451,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} +} \ No newline at end of file diff --git a/frappe/website/doctype/website_settings/website_settings.py b/frappe/website/doctype/website_settings/website_settings.py index e8f15290c4..1042b97efa 100644 --- a/frappe/website/doctype/website_settings/website_settings.py +++ b/frappe/website/doctype/website_settings/website_settings.py @@ -136,7 +136,7 @@ def get_website_settings(context=None): } ) - settings = frappe.get_single("Website Settings") + settings: "WebsiteSettings" = frappe.get_single("Website Settings") for k in [ "banner_html", "banner_image", @@ -203,6 +203,8 @@ def get_website_settings(context=None): context["hide_login"] = settings.hide_login + context["splash_image"] = settings.splash_image or context["splash_image"] + return context From 6db103850105206e7272cfba877b90f900c77933 Mon Sep 17 00:00:00 2001 From: gavin Date: Fri, 27 May 2022 16:55:54 +0530 Subject: [PATCH 13/62] refactor(qb): Rename term subqry to SubQuery "SubQuery" conforms with terms naming in PyPika. Kept subqry that points to same def for maintaining APIs. --- frappe/query_builder/terms.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py index 8d64d2ddcd..64a4707983 100644 --- a/frappe/query_builder/terms.py +++ b/frappe/query_builder/terms.py @@ -100,7 +100,7 @@ class ParameterizedFunction(Function): return function_sql -class subqry(Criterion): +class SubQuery(Criterion): def __init__( self, subq: QueryBuilder, @@ -112,3 +112,6 @@ class subqry(Criterion): def get_sql(self, **kwg: Any) -> str: kwg["subquery"] = True return self.subq.get_sql(**kwg) + + +subqry = SubQuery From e92ead1257245290774b74851de9012505b32fdc Mon Sep 17 00:00:00 2001 From: gavin Date: Fri, 27 May 2022 17:24:40 +0530 Subject: [PATCH 14/62] refactor: Re-write queries using SubQuery --- frappe/boot.py | 8 ++++---- frappe/utils/nestedset.py | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/frappe/boot.py b/frappe/boot.py index a23a7e6ac3..6cd86dc4fc 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -15,7 +15,7 @@ from frappe.geo.country_info import get_all from frappe.model.base_document import get_controller from frappe.query_builder import DocType from frappe.query_builder.functions import Count -from frappe.query_builder.terms import subqry +from frappe.query_builder.terms import SubQuery from frappe.social.doctype.energy_point_log.energy_point_log import get_energy_points from frappe.social.doctype.energy_point_settings.energy_point_settings import ( is_energy_point_enabled, @@ -211,7 +211,7 @@ def get_user_pages_or_reports(parent, cache=False): if parent == "Report": has_role[p.name].update({"ref_doctype": p.ref_doctype}) - no_of_roles = ( + no_of_roles = SubQuery( frappe.qb.from_(hasRole).select(Count("*")).where(hasRole.parent == parentTable.name) ) @@ -221,7 +221,7 @@ def get_user_pages_or_reports(parent, cache=False): pages_with_no_roles = ( frappe.qb.from_(parentTable) .select(parentTable.name, parentTable.modified, *columns) - .where(subqry(no_of_roles) == 0) + .where(no_of_roles == 0) ).run(as_dict=True) for p in pages_with_no_roles: @@ -327,7 +327,7 @@ def get_unseen_notes(): (note.notify_on_every_login == 1) & (note.expire_notification_on > frappe.utils.now()) & ( - subqry(frappe.qb.from_(nsb).select(nsb.user).where(nsb.parent == note.name)).notin( + SubQuery(frappe.qb.from_(nsb).select(nsb.user).where(nsb.parent == note.name)).notin( [frappe.session.user] ) ) diff --git a/frappe/utils/nestedset.py b/frappe/utils/nestedset.py index d3067973ef..681cd6439d 100644 --- a/frappe/utils/nestedset.py +++ b/frappe/utils/nestedset.py @@ -17,6 +17,7 @@ from frappe import _ from frappe.model.document import Document from frappe.query_builder import Order from frappe.query_builder.functions import Coalesce, Max +from frappe.query_builder.terms import SubQuery from frappe.query_builder.utils import DocType @@ -336,14 +337,15 @@ class NestedSet(Document): def get_root_of(doctype): """Get root element of a DocType with a tree structure""" from frappe.query_builder.functions import Count - from frappe.query_builder.terms import subqry Table = DocType(doctype) t1 = Table.as_("t1") t2 = Table.as_("t2") - subq = frappe.qb.from_(t2).select(Count("*")).where((t2.lft < t1.lft) & (t2.rgt > t1.rgt)) - result = frappe.qb.from_(t1).select(t1.name).where((subqry(subq) == 0) & (t1.rgt > t1.lft)).run() + node_query = SubQuery( + frappe.qb.from_(t2).select(Count("*")).where((t2.lft < t1.lft) & (t2.rgt > t1.rgt)) + ) + result = frappe.qb.from_(t1).select(t1.name).where((node_query == 0) & (t1.rgt > t1.lft)).run() return result[0][0] if result else None From 8f245be46025360a19f952b22119220ab0321075 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Fri, 27 May 2022 21:06:07 +0530 Subject: [PATCH 15/62] fix: Strip all spacing characters from Message-ID & In-Reply-To (#16999) --- frappe/core/doctype/communication/email.py | 3 ++- .../doctype/email_account/test_email_account.py | 2 +- frappe/email/doctype/email_queue/email_queue.py | 12 ++++++++++-- frappe/email/receive.py | 8 ++++++-- frappe/utils/data.py | 10 ++++++++++ 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 464bc35a1c..887037dca1 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -12,6 +12,7 @@ from frappe.utils import ( cint, get_datetime, get_formatted_email, + get_string_between, list_to_str, split_emails, validate_email_address, @@ -152,7 +153,7 @@ def _make( "reference_doctype": doctype, "reference_name": name, "email_template": email_template, - "message_id": get_message_id().strip(" <>"), + "message_id": get_string_between("<", get_message_id(), ">"), "read_receipt": read_receipt, "has_attachment": 1 if attachments else 0, "communication_type": communication_type, diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index a027a81bd7..537bf9eb7f 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -284,7 +284,7 @@ class TestEmailAccount(unittest.TestCase): messages = { # append_to = ToDo '"INBOX"': { - "latest_messages": [f.read().replace("{{ message_id }}", last_mail.message_id)], + "latest_messages": [f.read().replace("{{ message_id }}", "<" + last_mail.message_id + ">")], "seen_status": {2: "UNSEEN"}, "uid_list": [2], } diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index db2ca9e32b..662ba1b2ed 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -19,7 +19,15 @@ from frappe.email.email_body import add_attachment, get_email, get_formatted_htm from frappe.email.queue import get_unsubcribed_url, get_unsubscribe_message from frappe.model.document import Document from frappe.query_builder.utils import DocType -from frappe.utils import add_days, cint, cstr, get_hook_method, nowdate, split_emails +from frappe.utils import ( + add_days, + cint, + cstr, + get_hook_method, + get_string_between, + nowdate, + split_emails, +) MAX_RETRY_COUNT = 3 @@ -635,7 +643,7 @@ class QueueBuilder: d = { "priority": self.send_priority, "attachments": json.dumps(self.get_attachments()), - "message_id": mail.msg_root["Message-Id"].strip(" <>"), + "message_id": get_string_between("<", mail.msg_root["Message-Id"], ">"), "message": mail_to_string, "sender": self.sender, "reference_doctype": self.reference_doctype, diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 80c413faa1..d39eaa5213 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -25,6 +25,7 @@ from frappe.utils import ( cstr, extract_email_id, get_datetime, + get_string_between, markdown, now, parse_addr, @@ -425,7 +426,9 @@ class Email: self.set_content_and_type() self.set_subject() self.set_from() - self.message_id = (self.mail.get("Message-ID") or "").strip(" <>") + + message_id = self.mail.get("Message-ID") or "" + self.message_id = get_string_between("<", message_id, ">") if self.mail["Date"]: try: @@ -441,7 +444,8 @@ class Email: @property def in_reply_to(self): - return (self.mail.get("In-Reply-To") or "").strip(" <>") + in_reply_to = self.mail.get("In-Reply-To") or "" + return get_string_between("<", in_reply_to, ">") def parse(self): """Walk and process multi-part email.""" diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 6a9ffc81a6..bf030020ac 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1887,6 +1887,16 @@ def strip(val: str, chars: Optional[str] = None) -> str: return (val or "").replace("\ufeff", "").replace("\u200b", "").strip(chars) +def get_string_between(start: str, string: str, end: str) -> str: + if not string: + return "" + + regex = "{0}(.*){1}".format(start, end) + out = re.search(regex, string) + + return out.group(1) if out else string + + def to_markdown(html: str) -> str: from html.parser import HTMLParser From 507a45d8f2ce20e57c1c25f16f312b9f8b0a43d3 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sat, 28 May 2022 13:57:44 +0530 Subject: [PATCH 16/62] fix: dot syntax support for link fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✅ test --- frappe/model/db_query.py | 25 ++++++++++++++++++++----- frappe/tests/test_db_query.py | 13 +++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 8b2475de52..dd7227df47 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -131,6 +131,7 @@ class DatabaseQuery(object): self.run = run self.strict = strict self.ignore_ddl = ignore_ddl + self.link_tables = [] # for contextual user permission check # to determine which user permission is applicable on link field of specific doctype @@ -216,6 +217,10 @@ class DatabaseQuery(object): parent_name = cast_name(f"{self.tables[0]}.name") args.tables += f" {self.join} {child} on ({child}.parent = {parent_name})" + # left join link tables + for link in self.link_tables: + args.tables += f" {self.join} `tab{link.doctype}` on (`tab{link.doctype}`.`name` = {self.tables[0]}.`{link.fieldname}`)" + if self.grouped_or_conditions: self.conditions.append(f"({' or '.join(self.grouped_or_conditions)})") @@ -294,9 +299,17 @@ class DatabaseQuery(object): alias = None if " as " in field: field, alias = field.split(" as ") - tablefield, fieldname = field.split(".") - child_doctype = frappe.get_meta(self.doctype).get_field(tablefield).options - field = field.replace(tablefield, f"`tab{child_doctype}`") + linked_fieldname, fieldname = field.split(".") + linked_field = frappe.get_meta(self.doctype).get_field(linked_fieldname) + linked_doctype = linked_field.options + if linked_field.fieldtype == "Link": + self.link_tables.append( + frappe._dict( + doctype=linked_doctype, fieldname=linked_fieldname, table_name=f"`tab{linked_doctype}`" + ) + ) + + field = field.replace(linked_fieldname, f"`tab{linked_doctype}`") field = field.replace(fieldname, f"`{fieldname}`") if alias: field = f"{field} as {alias}" @@ -411,7 +424,9 @@ class DatabaseQuery(object): table_name = table_name[13:] if not table_name[0] == "`": table_name = f"`{table_name}`" - if table_name not in self.tables: + if table_name not in self.tables and table_name not in ( + d.table_name for d in self.link_tables + ): self.append_table(table_name) def append_table(self, table_name): @@ -433,7 +448,7 @@ class DatabaseQuery(object): methods = ("count(", "avg(", "sum(", "extract(", "dayofyear(") return field.lower().startswith(methods) - if len(self.tables) > 1: + if len(self.tables) > 1 or len(self.link_tables) > 0: for idx, field in enumerate(self.fields): if "." not in field and not _in_standard_sql_methods(field): self.fields[idx] = f"{self.tables[0]}.{field}" diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index e2a32a020c..df518ce3cb 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -51,6 +51,19 @@ class TestReportview(unittest.TestCase): self.assertEqual(result[0].seen_by, "Administrator") note.delete() + def test_link_field_syntax(self): + todo = frappe.get_doc( + doctype="ToDo", description=f"Test ToDo", allocated_to="Administrator" + ).insert() + result = frappe.db.get_all( + "ToDo", + filters={"name": todo.name}, + fields=["name", "allocated_to.email as allocated_user_email"], + limit=1, + ) + self.assertEqual(result[0].allocated_user_email, "admin@example.com") + todo.delete() + def test_build_match_conditions(self): clear_user_permissions_for_doctype("Blog Post", "test2@example.com") From a0ecb912db1432faa5e1b8ddd99636cd6c2a574e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 27 May 2022 19:17:12 +0530 Subject: [PATCH 17/62] fix!: dont delete customizations when doctypes are deleted If someone deletes doctype and restores it back all customization are lost, there's no "real" reason to delete all these customization. They are only ever active if the doctype is being used. Explanations: - Custom field: is used by meta when doctype meta is requested, if meta isn't requested then custom field is effectively inactive. - Client script: loaded by meta when doctype is requested by desk. So inactive in deleted state. - Property setter: loaded by meta, so inactive when doctype isn't present. - Report: will break doctype isn't present, but user should delete them manually to avoid loss of "scripts" or anything special they might have done. Also report's doctype don't 100% indicate that it's based solely on that doctype. --- frappe/core/doctype/doctype/test_doctype.py | 55 +++++++++++++++++++-- frappe/model/delete_doc.py | 4 -- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 11f5ef8a69..0bcd972c68 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -1,10 +1,14 @@ # -*- coding: utf-8 -*- # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import random +import string import unittest from typing import Dict, List, Optional +from unittest.mock import patch import frappe +from frappe.cache_manager import clear_doctype_cache from frappe.core.doctype.doctype.doctype import ( CannotIndexedError, DoctypeLinkError, @@ -15,8 +19,8 @@ from frappe.core.doctype.doctype.doctype import ( WrongOptionsDoctypeLinkError, validate_links_table_fieldnames, ) - -# test_records = frappe.get_test_records('DocType') +from frappe.custom.doctype.custom_field.custom_field import create_custom_fields +from frappe.desk.form.load import getdoc class TestDocType(unittest.TestCase): @@ -628,10 +632,55 @@ class TestDocType(unittest.TestCase): self.assertEqual(test_json.test_json_field["hello"], "world") + @patch.dict(frappe.conf, {"developer_mode": 1}) + def test_delete_doctype_with_customization(self): + from frappe.custom.doctype.property_setter.property_setter import make_property_setter + + custom_field = "customfield" + + doctype = new_doctype(custom=0).insert().name + + # Create property setter and custom field + field = "some_fieldname" + make_property_setter(doctype, field, "default", "DELETETHIS", "Data") + create_custom_fields({doctype: [{"fieldname": custom_field, "fieldtype": "Data"}]}) + + # Create 1 record + original_doc = frappe.get_doc(doctype=doctype, custom_field_name="wat").insert() + self.assertEqual(original_doc.some_fieldname, "DELETETHIS") + + # delete doctype + frappe.delete_doc("DocType", doctype) + clear_doctype_cache(doctype) + + # "restore" doctype by inserting doctype with same schema again + new_doctype(doctype, custom=0).insert() + + # Ensure basically same doctype getting "restored" + restored_doc = frappe.get_last_doc(doctype) + verify_fields = ["doctype", field, custom_field] + for f in verify_fields: + self.assertEqual(original_doc.get(f), restored_doc.get(f)) + + # Check form load of restored doctype + getdoc(doctype, restored_doc.name) + + # ensure meta - property setter + self.assertEqual(frappe.get_meta(doctype).get_field(field).default, "DELETETHIS") + frappe.delete_doc("DocType", doctype) + def new_doctype( - name, unique: bool = False, depends_on: str = "", fields: Optional[List[Dict]] = None, **kwargs + name: Optional[str] = None, + unique: bool = False, + depends_on: str = "", + fields: Optional[List[Dict]] = None, + **kwargs, ): + if not name: + # Test prefix is required to avoid coverage + name = "Test " + "".join(random.sample(string.ascii_lowercase, 10)) + doc = frappe.get_doc( { "doctype": "DocType", diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 2eccc1e717..b53455134d 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -88,10 +88,6 @@ def delete_doc( update_flags(doc, flags, ignore_permissions) check_permission_and_not_submitted(doc) - frappe.db.delete("Custom Field", {"dt": name}) - frappe.db.delete("Client Script", {"dt": name}) - frappe.db.delete("Property Setter", {"doc_type": name}) - frappe.db.delete("Report", {"ref_doctype": name}) frappe.db.delete("Custom DocPerm", {"parent": name}) frappe.db.delete("__global_search", {"doctype": name}) From 076eb593e320a585a5cdf0880ff81c64d7833b6c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 27 May 2022 19:07:56 +0530 Subject: [PATCH 18/62] refactor: delete_from_table Whole lot of unnecessary complexity, closure, multiple function calls, comprehensions all that can be replaced with single `get_all` --- frappe/model/delete_doc.py | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index b53455134d..a6d29ed190 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -3,6 +3,7 @@ import os import shutil +from typing import List import frappe import frappe.defaults @@ -188,39 +189,24 @@ def update_naming_series(doc): revert_series_if_last(doc.meta.autoname, doc.name, doc) -def delete_from_table(doctype, name, ignore_doctypes, doc): +def delete_from_table(doctype: str, name: str, ignore_doctypes: List[str], doc): if doctype != "DocType" and doctype == name: frappe.db.delete("Singles", {"doctype": name}) else: frappe.db.delete(doctype, {"name": name}) - # get child tables if doc: - tables = [d.options for d in doc.meta.get_table_fields()] - + child_doctypes = [d.options for d in doc.meta.get_table_fields()] else: + child_doctypes = frappe.get_all( + "DocField", + fields="options", + filters={"fieldtype": ["in", frappe.model.table_fields], "parent": doctype}, + pluck="options", + ) - def get_table_fields(field_doctype): - if field_doctype == "Custom Field": - return [] - - return [ - r[0] - for r in frappe.get_all( - field_doctype, - fields="options", - filters={"fieldtype": ["in", frappe.model.table_fields], "parent": doctype}, - as_list=1, - ) - ] - - tables = get_table_fields("DocField") - if not frappe.flags.in_install == "frappe": - tables += get_table_fields("Custom Field") - - # delete from child tables - for t in list(set(tables)): - if t not in ignore_doctypes: - frappe.db.delete(t, {"parenttype": doctype, "parent": name}) + child_doctypes_to_delete = set(child_doctypes) - set(ignore_doctypes) + for child_doctype in child_doctypes_to_delete: + frappe.db.delete(child_doctype, {"parenttype": doctype, "parent": name}) def update_flags(doc, flags=None, ignore_permissions=False): From 1e64abe9a56fdd8a33e478e5a2e641d97717640e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 11:27:12 +0530 Subject: [PATCH 19/62] fix!: meaningful error messages over `KeyError` --- frappe/__init__.py | 6 ++++-- frappe/modules/utils.py | 28 ++++++++++++++++++---------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 17a945c875..a5f4aafce4 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1258,8 +1258,10 @@ def get_module_path(module, *joins): :param module: Module name. :param *joins: Join additional path elements using `os.path.join`.""" - module = scrub(module) - return get_pymodule_path(local.module_app[module] + "." + module, *joins) + from frappe.modules.utils import get_module_app + + app = get_module_app(module) + return get_pymodule_path(app + "." + scrub(module), *joins) def get_app_path(app_name, *joins): diff --git a/frappe/modules/utils.py b/frappe/modules/utils.py index 60eaefddc5..ad6a75e900 100644 --- a/frappe/modules/utils.py +++ b/frappe/modules/utils.py @@ -207,13 +207,18 @@ def export_doc(doctype, name, module=None): write_document_file(frappe.get_doc(doctype, name), module) -def get_doctype_module(doctype): +def get_doctype_module(doctype) -> str: """Returns **Module Def** name of given doctype.""" def make_modules_dict(): return dict(frappe.db.sql("select name, module from tabDocType")) - return frappe.cache().get_value("doctype_modules", make_modules_dict)[doctype] + doctype_module_map = frappe.cache().get_value("doctype_modules", make_modules_dict) + + if module_name := doctype_module_map.get(doctype): + return module_name + else: + frappe.throw(_("DocType {} not found").format(doctype), exc=frappe.DoesNotExistError) doctype_python_modules = {} @@ -234,9 +239,9 @@ def load_doctype_module(doctype, module=None, prefix="", suffix=""): if key not in doctype_python_modules: doctype_python_modules[key] = frappe.get_module(module_name) except ImportError as e: - raise ImportError( - "Module import failed for {0} ({1})".format(doctype, module_name + " Error: " + str(e)) - ) + msg = f"Module import failed for {doctype}, the DocType you're trying to open might be deleted." + msg += f"
Error: {e}" + raise ImportError(msg) from e return doctype_python_modules[key] @@ -251,12 +256,15 @@ def get_module_name(doctype, module, prefix="", suffix="", app=None): ) -def get_module_app(module): - return frappe.local.module_app[scrub(module)] +def get_module_app(module: str) -> str: + app = frappe.local.module_app.get(scrub(module)) + if app is None: + frappe.throw(_("Module {} not found").format(module), exc=frappe.DoesNotExistError) + return app -def get_app_publisher(module): - app = frappe.local.module_app[scrub(module)] +def get_app_publisher(module: str) -> str: + app = get_module_app(module) if not app: frappe.throw(_("App not found")) app_publisher = frappe.get_hooks(hook="app_publisher", app_name=app)[0] @@ -321,7 +329,7 @@ def make_boilerplate(template, doc, opts=None): base_class=base_class, doctype=doc.name, **opts, - custom_controller=custom_controller + custom_controller=custom_controller, ) ) ) From 82cc98366d80fda6cd727a7fc4b51c66f1eab6ad Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 12:57:39 +0530 Subject: [PATCH 20/62] fix: dont attempt to init singles that dont exist This is done during app install stage and causes unnecessary failures where stale doctypes are left in db. --- frappe/installer.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 5cd46e618d..c8373ff06f 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -473,13 +473,20 @@ def set_all_patches_as_completed(app): def init_singles(): - singles = [single["name"] for single in frappe.get_all("DocType", filters={"issingle": True})] + singles = frappe.get_all("DocType", filters={"issingle": True}, pluck="name") for single in singles: - if not frappe.db.get_singles_dict(single): + if frappe.db.get_singles_dict(single): + continue + + try: doc = frappe.new_doc(single) doc.flags.ignore_mandatory = True doc.flags.ignore_validate = True doc.save() + except ImportError: + # The doctype exists, but controller is deleted, + # no need to attempt to init such single, ref: #16917 + continue def make_conf( From c3918e8e347669fc9f2d4773304cec4594026dd6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 16:11:07 +0530 Subject: [PATCH 21/62] test: test get_newargs --- frappe/tests/test_utils.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 04f9d16fd1..f4636fc026 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -615,3 +615,18 @@ class TestAppParser(unittest.TestCase): self.assertEqual("healthcare", parse_app_name("https://github.com/frappe/healthcare.git")) self.assertEqual("healthcare", parse_app_name("git@github.com:frappe/healthcare.git")) self.assertEqual("healthcare", parse_app_name("frappe/healthcare@develop")) + + +class TestIntrospectionMagic(unittest.TestCase): + """Test utils that inspect live objects""" + + def test_get_newargs(self): + def f(a, b=2, **args): + pass + + safe_kwargs = {"company": "Wind Power", "b": 1} + self.assertEqual(frappe.get_newargs(f, safe_kwargs), safe_kwargs) + + unsafe_args = dict(safe_kwargs) + unsafe_args.update({"ignore_permissions": True, "flags": {"ignore_mandatory": True}}) + self.assertEqual(frappe.get_newargs(f, unsafe_args), safe_kwargs) From 4085646495acd559ab16225cf998c7363f80d277 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 16:39:25 +0530 Subject: [PATCH 22/62] fix: identify varkw in get_newargs --- frappe/__init__.py | 16 ++++++++++++---- frappe/tests/test_utils.py | 11 +++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 17a945c875..a172c7412d 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1501,18 +1501,26 @@ def call(fn, *args, **kwargs): def get_newargs(fn, kwargs): + + # if function has any **kwargs parameter that capture arbitrary keyword arguments + # Ref: https://docs.python.org/3/library/inspect.html#inspect.Parameter.kind + varkw_exist = False + if hasattr(fn, "fnargs"): fnargs = fn.fnargs else: signature = inspect.signature(fn) fnargs = list(signature.parameters) - varkw = "kwargs" in fnargs - if varkw: - fnargs.pop(-1) + + for param_name, parameter in signature.parameters.items(): + if parameter.kind == inspect.Parameter.VAR_KEYWORD: + varkw_exist = True + fnargs.remove(param_name) + break newargs = {} for a in kwargs: - if (a in fnargs) or varkw: + if (a in fnargs) or varkw_exist: newargs[a] = kwargs.get(a) newargs.pop("ignore_permissions", None) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index f4636fc026..4f4fca8bbf 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -621,6 +621,7 @@ class TestIntrospectionMagic(unittest.TestCase): """Test utils that inspect live objects""" def test_get_newargs(self): + # `kwargs` is just convention any **varname should work. def f(a, b=2, **args): pass @@ -630,3 +631,13 @@ class TestIntrospectionMagic(unittest.TestCase): unsafe_args = dict(safe_kwargs) unsafe_args.update({"ignore_permissions": True, "flags": {"ignore_mandatory": True}}) self.assertEqual(frappe.get_newargs(f, unsafe_args), safe_kwargs) + + def test_strip_off_kwargs_when_not_supported(self): + def f(a, b=2): + pass + + args = {"company": "Wind Power", "b": 1} + self.assertEqual(frappe.get_newargs(f, args), {"b": 1}) + + # No args + self.assertEqual(frappe.get_newargs(lambda: None, args), {}) From 87ec6d4fb9c56f11795cbd9b7e5c4334a3dda7b6 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 30 May 2022 10:41:08 +0530 Subject: [PATCH 23/62] fix: initialize link_tables in constructor --- frappe/model/db_query.py | 2 +- frappe/tests/test_db_query.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index dd7227df47..3013483ca9 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -34,6 +34,7 @@ class DatabaseQuery(object): def __init__(self, doctype, user=None): self.doctype = doctype self.tables = [] + self.link_tables = [] self.conditions = [] self.or_conditions = [] self.fields = None @@ -131,7 +132,6 @@ class DatabaseQuery(object): self.run = run self.strict = strict self.ignore_ddl = ignore_ddl - self.link_tables = [] # for contextual user permission check # to determine which user permission is applicable on link field of specific doctype diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index df518ce3cb..c1b2e05266 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -53,7 +53,7 @@ class TestReportview(unittest.TestCase): def test_link_field_syntax(self): todo = frappe.get_doc( - doctype="ToDo", description=f"Test ToDo", allocated_to="Administrator" + doctype="ToDo", description="Test ToDo", allocated_to="Administrator" ).insert() result = frappe.db.get_all( "ToDo", From 1df1bc5232e9dd7f459deb5e6febaefb84920af4 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 May 2022 17:15:44 +0530 Subject: [PATCH 24/62] fix: Don't update modified time on set_notifications_as_unseen --- frappe/desk/doctype/notification_log/notification_log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index c4a082ff11..def626513c 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -148,6 +148,6 @@ def trigger_indicator_hide(): def set_notifications_as_unseen(user): try: - frappe.db.set_value("Notification Settings", user, "seen", 0) + frappe.db.set_value("Notification Settings", user, "seen", 0, update_modified=False) except frappe.DoesNotExistError: return From dcff9a5d75cfa3c1d610eab9ecb74866ed558130 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 17:20:53 +0530 Subject: [PATCH 25/62] chore: bump pypika functions.Replace is used but it's only available in latest version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 88ad0562b7..bf798fe747 100644 --- a/requirements.txt +++ b/requirements.txt @@ -44,7 +44,7 @@ PyMySQL~=1.0.2 pyOpenSSL~=20.0.1 pyotp~=2.6.0 PyPDF2~=1.26.0 -PyPika~=0.48.6 +PyPika~=0.48.9 pypng~=0.0.20 PyQRCode~=1.2.1 python-dateutil~=2.8.1 From 3c7dd75047ac190c1a00b2953c40cb9fd98ddf2b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 May 2022 17:28:12 +0530 Subject: [PATCH 26/62] fix: Update Notification Settings via doc.save toggle_notifications is called via User doc. Changes made through another doc should leave some trace. Using doc.save in order to maintain versions. --- .../notification_settings/notification_settings.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index ee425e154b..1436ecc1cd 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -48,9 +48,15 @@ def create_notification_settings(user): _doc.insert(ignore_permissions=True) -def toggle_notifications(user, enable=False): - if frappe.db.exists("Notification Settings", user): - frappe.db.set_value("Notification Settings", user, "enabled", enable) +def toggle_notifications(user: str, enable: bool = False): + try: + settings = frappe.get_doc("Notification Settings", user) + except frappe.DoesNotExistError: + return + + if settings.enabled != enable: + settings.enabled = enable + settings.save() @frappe.whitelist() From 9bc85402f67044ebce46ebd1ba1391053ec9837d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 30 May 2022 18:56:17 +0530 Subject: [PATCH 27/62] fix: Add splash_image to context if exists --- 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 1042b97efa..7fb5d191ab 100644 --- a/frappe/website/doctype/website_settings/website_settings.py +++ b/frappe/website/doctype/website_settings/website_settings.py @@ -203,7 +203,8 @@ def get_website_settings(context=None): context["hide_login"] = settings.hide_login - context["splash_image"] = settings.splash_image or context["splash_image"] + if splash_image := settings.splash_image or context.get("splash_image"): + context["splash_image"] = splash_image return context From 4c691d5da3388aa1bba5b3f4a2c586ae761537e5 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 16:54:58 +0530 Subject: [PATCH 28/62] chore: Naming Series -> Document Naming Settings Migrate old code and format JS code --- .../document_naming_settings/__init__.py | 0 .../document_naming_settings.js | 94 ++++++ .../document_naming_settings.json | 129 ++++++++ .../document_naming_settings.py | 302 ++++++++++++++++++ .../test_document_naming_settings.py | 36 +++ 5 files changed, 561 insertions(+) create mode 100644 frappe/core/doctype/document_naming_settings/__init__.py create mode 100644 frappe/core/doctype/document_naming_settings/document_naming_settings.js create mode 100644 frappe/core/doctype/document_naming_settings/document_naming_settings.json create mode 100644 frappe/core/doctype/document_naming_settings/document_naming_settings.py create mode 100644 frappe/core/doctype/document_naming_settings/test_document_naming_settings.py diff --git a/frappe/core/doctype/document_naming_settings/__init__.py b/frappe/core/doctype/document_naming_settings/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.js b/frappe/core/doctype/document_naming_settings/document_naming_settings.js new file mode 100644 index 0000000000..7fa2808b6e --- /dev/null +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -0,0 +1,94 @@ +// Copyright (c) 2022, Frappe Technologies and contributors +// For license information, please see license.txt + +frappe.ui.form.on("Document Naming Settings", { + onload: function(frm) { + frm.events.get_doc_and_prefix(frm); + }, + + refresh: function(frm) { + frm.disable_save(); + }, + + get_doc_and_prefix: function(frm) { + frappe.call({ + method: "get_transactions", + doc: frm.doc, + callback: function(r) { + frm.set_df_property( + "select_doc_for_series", + "options", + r.message.transactions + ); + frm.set_df_property("prefix", "options", r.message.prefixes); + }, + }); + }, + + select_doc_for_series: function(frm) { + frm.set_value("user_must_always_select", 0); + frappe.call({ + method: "get_options", + doc: frm.doc, + callback: function(r) { + frm.set_value("set_options", r.message); + if (r.message && r.message.split("\n")[0] == "") + frm.set_value("user_must_always_select", 1); + frm.refresh(); + }, + }); + }, + + prefix: function(frm) { + frappe.call({ + method: "get_current", + doc: frm.doc, + callback: function(r) { + frm.refresh_field("current_value"); + }, + }); + }, + + update: function(frm) { + frappe.call({ + method: "update_series", + doc: frm.doc, + callback: function(r) { + frm.events.get_doc_and_prefix(frm); + }, + }); + }, + + naming_series_to_check(frm) { + frappe.call({ + method: "preview_series", + doc: frm.doc, + callback: function(r) { + if (!r.exc) { + frm.set_value("preview", r.message); + } else { + frm.set_value( + "preview", + __("Failed to generate preview of series") + ); + } + }, + }); + }, + + add_series(frm) { + const series = frm.doc.naming_series_to_check; + + if (!series) { + frappe.show_alert(__("Please type a valid series.")); + return; + } + + if (!frm.doc.set_options.includes(series)) { + const current_series = frm.doc.set_options; + frm.set_value("set_options", `${current_series}\n${series}`); + } else { + frappe.show_alert(__("Series already added to transaction.")); + } + }, +}); diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.json b/frappe/core/doctype/document_naming_settings/document_naming_settings.json new file mode 100644 index 0000000000..a0fdae06dc --- /dev/null +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -0,0 +1,129 @@ +{ + "actions": [], + "creation": "2022-05-30 07:24:07.736646", + "description": "Set prefix for numbering series on your transactions", + "doctype": "DocType", + "engine": "InnoDB", + "field_order": [ + "setup_series", + "select_doc_for_series", + "help_html", + "naming_series_to_check", + "preview", + "add_series", + "set_options", + "user_must_always_select", + "update", + "column_break_13", + "update_series", + "prefix", + "current_value", + "update_series_start" + ], + "fields": [ + { + "description": "Set prefix for numbering series on your transactions", + "fieldname": "setup_series", + "fieldtype": "Section Break", + "label": "Setup Series" + }, + { + "fieldname": "select_doc_for_series", + "fieldtype": "Select", + "label": "Select Transaction" + }, + { + "depends_on": "select_doc_for_series", + "fieldname": "help_html", + "fieldtype": "HTML", + "label": "Help HTML", + "options": "
\n Edit list of Series in the box below. Rules:\n
    \n
  • Each Series Prefix on a new line.
    • \n
  • Allowed special characters are \"/\" and \"-\"
    • \n
  • \n Optionally, set the number of digits in the series using dot (.)\n followed by hashes (#). For example, \".####\" means that the series\n will have four digits. Default is five digits.\n
    • \n
  • \n You can also use variables in the series name by putting them\n between (.) dots\n
    \n Support Variables:\n
      \n
    • .YYYY. - Year in 4 digits
      • \n
    • .YY. - Year in 2 digits
      • \n
    • .MM. - Month
      • \n
    • .DD. - Day of month
      • \n
    • .WW. - Week of the year
      • \n
    • .FY. - Fiscal Year
      • \n
    • \n .{fieldname}. - fieldname on the document e.g.\n branch\n
      • \n
    \n
    • \n
\n Examples:\n
    \n
  • INV-
    • \n
  • INV-10-
    • \n
  • INVK-
    • \n
  • INV-.YYYY.-.{branch}.-.MM.-.####
    • \n
\n
\n
\n" + }, + { + "fieldname": "naming_series_to_check", + "fieldtype": "Data", + "label": "Try a naming Series" + }, + { + "default": " ", + "fieldname": "preview", + "fieldtype": "Text", + "label": "Preview of generated names", + "read_only": 1 + }, + { + "fieldname": "add_series", + "fieldtype": "Button", + "label": "Add this Series" + }, + { + "depends_on": "select_doc_for_series", + "fieldname": "set_options", + "fieldtype": "Text", + "label": "Series List for this Transaction" + }, + { + "default": "0", + "depends_on": "select_doc_for_series", + "description": "Check this if you want to force the user to select a series before saving. There will be no default if you check this.", + "fieldname": "user_must_always_select", + "fieldtype": "Check", + "label": "User must always select" + }, + { + "depends_on": "select_doc_for_series", + "fieldname": "update", + "fieldtype": "Button", + "label": "Update" + }, + { + "fieldname": "column_break_13", + "fieldtype": "Column Break" + }, + { + "description": "Change the starting / current sequence number of an existing series.", + "fieldname": "update_series", + "fieldtype": "Section Break", + "label": "Update Series" + }, + { + "fieldname": "prefix", + "fieldtype": "Select", + "label": "Prefix" + }, + { + "description": "This is the number of the last created transaction with this prefix", + "fieldname": "current_value", + "fieldtype": "Int", + "label": "Current Value" + }, + { + "fieldname": "update_series_start", + "fieldtype": "Button", + "label": "Update Series Number" + } + ], + "hide_toolbar": 1, + "icon": "fa fa-sort-by-order", + "issingle": 1, + "links": [], + "modified": "2022-05-30 08:00:20.236345", + "modified_by": "Administrator", + "module": "Core", + "name": "Document Naming Settings", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "email": 1, + "print": 1, + "read": 1, + "role": "System Manager", + "share": 1, + "write": 1 + } + ], + "sort_field": "modified", + "sort_order": "DESC", + "states": [] +} \ No newline at end of file diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.py b/frappe/core/doctype/document_naming_settings/document_naming_settings.py new file mode 100644 index 0000000000..800822e7ac --- /dev/null +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -0,0 +1,302 @@ +# Copyright (c) 2022, Frappe Technologies and contributors +# For license information, please see license.txt + +import frappe +from frappe import _, msgprint, throw +from frappe.core.doctype.doctype.doctype import validate_series +from frappe.model.document import Document +from frappe.model.naming import make_autoname, parse_naming_series +from frappe.permissions import get_doctypes_with_read +from frappe.utils import cint, cstr + + +class NamingSeriesNotSetError(frappe.ValidationError): + pass + + +class DocumentNamingSettings(Document): + @frappe.whitelist() + def get_transactions(self, arg=None): + doctypes = list( + set( + frappe.db.sql_list( + """select parent + from `tabDocField` df where fieldname='naming_series'""" + ) + + frappe.db.sql_list( + """select dt from `tabCustom Field` + where fieldname='naming_series'""" + ) + ) + ) + + doctypes = list(set(get_doctypes_with_read()).intersection(set(doctypes))) + prefixes = "" + for d in doctypes: + options = "" + try: + options = self.get_options(d) + except frappe.DoesNotExistError: + frappe.msgprint(_("Unable to find DocType {0}").format(d)) + # frappe.pass_does_not_exist_error() + continue + + if options: + prefixes = prefixes + "\n" + options + prefixes.replace("\n\n", "\n") + prefixes = prefixes.split("\n") + + custom_prefixes = frappe.get_all( + "DocType", + fields=["autoname"], + filters={ + "name": ("not in", doctypes), + "autoname": ("like", "%.#%"), + "module": ("not in", ["Core"]), + }, + ) + if custom_prefixes: + prefixes = prefixes + [d.autoname.rsplit(".", 1)[0] for d in custom_prefixes] + + prefixes = "\n".join(sorted(prefixes)) + + return {"transactions": "\n".join([""] + sorted(doctypes)), "prefixes": prefixes} + + def scrub_options_list(self, ol): + options = list(filter(lambda x: x, [cstr(n).strip() for n in ol])) + return options + + @frappe.whitelist() + def update_series(self, arg=None): + """update series list""" + self.validate_series_set() + self.check_duplicate() + series_list = self.set_options.split("\n") + + # set in doctype + self.set_series_for(self.select_doc_for_series, series_list) + + # create series + map(self.insert_series, [d.split(".")[0] for d in series_list if d.strip()]) + + msgprint(_("Series Updated")) + + return self.get_transactions() + + def validate_series_set(self): + if self.select_doc_for_series and not self.set_options: + frappe.throw(_("Please set the series to be used.")) + + def set_series_for(self, doctype, ol): + options = self.scrub_options_list(ol) + + # validate names + for i in options: + self.validate_series_name(i) + + if options and self.user_must_always_select: + options = [""] + options + + default = options[0] if options else "" + + # update in property setter + prop_dict = {"options": "\n".join(options), "default": default} + + for prop in prop_dict: + ps_exists = frappe.db.get_value( + "Property Setter", {"field_name": "naming_series", "doc_type": doctype, "property": prop} + ) + + if ps_exists: + ps = frappe.get_doc("Property Setter", ps_exists) + ps.value = prop_dict[prop] + ps.save() + else: + ps = frappe.get_doc( + { + "doctype": "Property Setter", + "doctype_or_field": "DocField", + "doc_type": doctype, + "field_name": "naming_series", + "property": prop, + "value": prop_dict[prop], + "property_type": "Text", + "__islocal": 1, + } + ) + ps.save() + + self.set_options = "\n".join(options) + + frappe.clear_cache(doctype=doctype) + + def check_duplicate(self): + parent = list( + set( + frappe.db.sql_list( + """select dt.name + from `tabDocField` df, `tabDocType` dt + where dt.name = df.parent and df.fieldname='naming_series' and dt.name != %s""", + self.select_doc_for_series, + ) + + frappe.db.sql_list( + """select dt.name + from `tabCustom Field` df, `tabDocType` dt + where dt.name = df.dt and df.fieldname='naming_series' and dt.name != %s""", + self.select_doc_for_series, + ) + ) + ) + sr = [[frappe.get_meta(p).get_field("naming_series").options, p] for p in parent] + dt = frappe.get_doc("DocType", self.select_doc_for_series) + options = self.scrub_options_list(self.set_options.split("\n")) + for series in options: + validate_series(dt, series) + for i in sr: + if i[0]: + existing_series = [d.split(".")[0] for d in i[0].split("\n")] + if series.split(".")[0] in existing_series: + frappe.throw(_("Series {0} already used in {1}").format(series, i[1])) + + def validate_series_name(self, n): + import re + + if not re.match(r"^[\w\- \/.#{}]+$", n, re.UNICODE): + throw( + _('Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series') + ) + + @frappe.whitelist() + def get_options(self, arg=None): + if frappe.get_meta(arg or self.select_doc_for_series).get_field("naming_series"): + return frappe.get_meta(arg or self.select_doc_for_series).get_field("naming_series").options + + @frappe.whitelist() + def get_current(self, arg=None): + """get series current""" + if self.prefix: + prefix = self.parse_naming_series() + self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name") + + def insert_series(self, series): + """insert series if missing""" + if frappe.db.get_value("Series", series, "name", order_by="name") == None: + frappe.db.sql("insert into tabSeries (name, current) values (%s, 0)", (series)) + + @frappe.whitelist() + def update_series_start(self): + if self.prefix: + prefix = self.parse_naming_series() + self.insert_series(prefix) + frappe.db.sql( + "update `tabSeries` set current = %s where name = %s", (cint(self.current_value), prefix) + ) + msgprint(_("Series Updated Successfully")) + else: + msgprint(_("Please select prefix first")) + + def parse_naming_series(self): + parts = self.prefix.split(".") + + # Remove ### from the end of series + if parts[-1] == "#" * len(parts[-1]): + del parts[-1] + + prefix = parse_naming_series(parts) + return prefix + + @frappe.whitelist() + def preview_series(self) -> str: + """Preview what the naming series will generate.""" + + generated_names = [] + series = self.naming_series_to_check + if not series: + return "" + + try: + doc = self._fetch_last_doc_if_available() + for _count in range(3): + generated_names.append(make_autoname(series, doc=doc)) + except Exception as e: + if frappe.message_log: + frappe.message_log.pop() + return _("Failed to generate names from the series") + f"\n{str(e)}" + + # Explcitly rollback in case any changes were made to series table. + frappe.db.rollback() # nosemgrep + return "\n".join(generated_names) + + def _fetch_last_doc_if_available(self): + """Fetch last doc for evaluating naming series with fields.""" + try: + return frappe.get_last_doc(self.select_doc_for_series) + except Exception: + return None + + +def set_by_naming_series( + doctype, fieldname, naming_series, hide_name_field=True, make_mandatory=1 +): + from frappe.custom.doctype.property_setter.property_setter import make_property_setter + + if naming_series: + make_property_setter( + doctype, "naming_series", "hidden", 0, "Check", validate_fields_for_doctype=False + ) + make_property_setter( + doctype, "naming_series", "reqd", make_mandatory, "Check", validate_fields_for_doctype=False + ) + + # set values for mandatory + try: + frappe.db.sql( + """update `tab{doctype}` set naming_series={s} where + ifnull(naming_series, '')=''""".format( + doctype=doctype, s="%s" + ), + get_default_naming_series(doctype), + ) + except NamingSeriesNotSetError: + pass + + if hide_name_field: + make_property_setter(doctype, fieldname, "reqd", 0, "Check", validate_fields_for_doctype=False) + make_property_setter( + doctype, fieldname, "hidden", 1, "Check", validate_fields_for_doctype=False + ) + else: + make_property_setter( + doctype, "naming_series", "reqd", 0, "Check", validate_fields_for_doctype=False + ) + make_property_setter( + doctype, "naming_series", "hidden", 1, "Check", validate_fields_for_doctype=False + ) + + if hide_name_field: + make_property_setter( + doctype, fieldname, "hidden", 0, "Check", validate_fields_for_doctype=False + ) + make_property_setter(doctype, fieldname, "reqd", 1, "Check", validate_fields_for_doctype=False) + + # set values for mandatory + frappe.db.sql( + """update `tab{doctype}` set `{fieldname}`=`name` where + ifnull({fieldname}, '')=''""".format( + doctype=doctype, fieldname=fieldname + ) + ) + + +def get_default_naming_series(doctype): + naming_series = frappe.get_meta(doctype).get_field("naming_series").options or "" + naming_series = naming_series.split("\n") + out = naming_series[0] or (naming_series[1] if len(naming_series) > 1 else None) + + if not out: + frappe.throw( + _("Please set Naming Series for {0} via Setup > Settings > Naming Series").format(doctype), + NamingSeriesNotSetError, + ) + else: + return out 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 new file mode 100644 index 0000000000..3823c5f99c --- /dev/null +++ b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py @@ -0,0 +1,36 @@ +# Copyright (c) 2022, Frappe Technologies and Contributors +# See license.txt + +import frappe +from frappe.core.doctype.document_naming_settings.document_naming_settings import ( + DocumentNamingSettings, +) +from frappe.tests.utils import FrappeTestCase + + +class TestNamingSeries(FrappeTestCase): + def setUp(self): + self.ns: DocumentNamingSettings = frappe.get_doc("Naming Series Settings") + + def tearDown(self): + frappe.db.rollback() + + def test_naming_preview(self): + self.ns.select_doc_for_series = "Sales Invoice" + + self.ns.naming_series_to_check = "AXBZ.####" + serieses = self.ns.preview_series().split("\n") + self.assertEqual(["AXBZ0001", "AXBZ0002", "AXBZ0003"], serieses) + + self.ns.naming_series_to_check = "AXBZ-.{currency}.-" + serieses = self.ns.preview_series().split("\n") + + def test_get_transactions(self): + + naming_info = self.ns.get_transactions() + self.assertIn("Sales Invoice", naming_info["transactions"]) + + existing_naming_series = frappe.get_meta("Sales Invoice").get_field("naming_series").options + + for series in existing_naming_series.split("\n"): + self.assertIn(series, naming_info["prefixes"]) From 22f2c78400874839f3ac7034731366a7549c3c1a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 17:35:25 +0530 Subject: [PATCH 29/62] refactor: use sane-r fieldnames --- .../document_naming_settings.js | 16 +++---- .../document_naming_settings.json | 48 +++++++++---------- .../document_naming_settings.py | 24 +++++----- .../test_document_naming_settings.py | 6 +-- 4 files changed, 47 insertions(+), 47 deletions(-) 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 7fa2808b6e..a2a5e10834 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -16,7 +16,7 @@ frappe.ui.form.on("Document Naming Settings", { doc: frm.doc, callback: function(r) { frm.set_df_property( - "select_doc_for_series", + "transaction_type", "options", r.message.transactions ); @@ -25,13 +25,13 @@ frappe.ui.form.on("Document Naming Settings", { }); }, - select_doc_for_series: function(frm) { + transaction_type: function(frm) { frm.set_value("user_must_always_select", 0); frappe.call({ method: "get_options", doc: frm.doc, callback: function(r) { - frm.set_value("set_options", r.message); + frm.set_value("naming_series_options", r.message); if (r.message && r.message.split("\n")[0] == "") frm.set_value("user_must_always_select", 1); frm.refresh(); @@ -59,7 +59,7 @@ frappe.ui.form.on("Document Naming Settings", { }); }, - naming_series_to_check(frm) { + try_naming_series(frm) { frappe.call({ method: "preview_series", doc: frm.doc, @@ -77,16 +77,16 @@ frappe.ui.form.on("Document Naming Settings", { }, add_series(frm) { - const series = frm.doc.naming_series_to_check; + const series = frm.doc.try_naming_series; if (!series) { frappe.show_alert(__("Please type a valid series.")); return; } - if (!frm.doc.set_options.includes(series)) { - const current_series = frm.doc.set_options; - frm.set_value("set_options", `${current_series}\n${series}`); + if (!frm.doc.naming_series_options.includes(series)) { + const current_series = frm.doc.naming_series_options; + frm.set_value("naming_series_options", `${current_series}\n${series}`); } else { frappe.show_alert(__("Series already added to transaction.")); } 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 a0fdae06dc..98f7df87a8 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.json +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -6,12 +6,12 @@ "engine": "InnoDB", "field_order": [ "setup_series", - "select_doc_for_series", + "transaction_type", "help_html", - "naming_series_to_check", + "try_naming_series", "preview", "add_series", - "set_options", + "naming_series_options", "user_must_always_select", "update", "column_break_13", @@ -28,22 +28,12 @@ "label": "Setup Series" }, { - "fieldname": "select_doc_for_series", - "fieldtype": "Select", - "label": "Select Transaction" - }, - { - "depends_on": "select_doc_for_series", + "depends_on": "transaction_type", "fieldname": "help_html", "fieldtype": "HTML", "label": "Help HTML", "options": "
\n Edit list of Series in the box below. Rules:\n
    \n
  • Each Series Prefix on a new line.
    • \n
  • Allowed special characters are \"/\" and \"-\"
    • \n
  • \n Optionally, set the number of digits in the series using dot (.)\n followed by hashes (#). For example, \".####\" means that the series\n will have four digits. Default is five digits.\n
    • \n
  • \n You can also use variables in the series name by putting them\n between (.) dots\n
    \n Support Variables:\n
      \n
    • .YYYY. - Year in 4 digits
      • \n
    • .YY. - Year in 2 digits
      • \n
    • .MM. - Month
      • \n
    • .DD. - Day of month
      • \n
    • .WW. - Week of the year
      • \n
    • .FY. - Fiscal Year
      • \n
    • \n .{fieldname}. - fieldname on the document e.g.\n branch\n
      • \n
    \n
    • \n
\n Examples:\n
    \n
  • INV-
    • \n
  • INV-10-
    • \n
  • INVK-
    • \n
  • INV-.YYYY.-.{branch}.-.MM.-.####
    • \n
\n
\n
\n" }, - { - "fieldname": "naming_series_to_check", - "fieldtype": "Data", - "label": "Try a naming Series" - }, { "default": " ", "fieldname": "preview", @@ -56,22 +46,16 @@ "fieldtype": "Button", "label": "Add this Series" }, - { - "depends_on": "select_doc_for_series", - "fieldname": "set_options", - "fieldtype": "Text", - "label": "Series List for this Transaction" - }, { "default": "0", - "depends_on": "select_doc_for_series", + "depends_on": "transaction_type", "description": "Check this if you want to force the user to select a series before saving. There will be no default if you check this.", "fieldname": "user_must_always_select", "fieldtype": "Check", "label": "User must always select" }, { - "depends_on": "select_doc_for_series", + "depends_on": "transaction_type", "fieldname": "update", "fieldtype": "Button", "label": "Update" @@ -101,13 +85,29 @@ "fieldname": "update_series_start", "fieldtype": "Button", "label": "Update Series Number" + }, + { + "depends_on": "transaction_type", + "fieldname": "naming_series_options", + "fieldtype": "Text", + "label": "Series List for this Transaction" + }, + { + "fieldname": "try_naming_series", + "fieldtype": "Data", + "label": "Try a naming Series" + }, + { + "fieldname": "transaction_type", + "fieldtype": "Select", + "label": "Select Transaction" } ], "hide_toolbar": 1, "icon": "fa fa-sort-by-order", "issingle": 1, "links": [], - "modified": "2022-05-30 08:00:20.236345", + "modified": "2022-05-30 08:07:28.047633", "modified_by": "Administrator", "module": "Core", "name": "Document Naming Settings", @@ -126,4 +126,4 @@ "sort_field": "modified", "sort_order": "DESC", "states": [] -} \ No newline at end of file +} 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 800822e7ac..13e1e4aeba 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -71,10 +71,10 @@ class DocumentNamingSettings(Document): """update series list""" self.validate_series_set() self.check_duplicate() - series_list = self.set_options.split("\n") + series_list = self.naming_series_options.split("\n") # set in doctype - self.set_series_for(self.select_doc_for_series, series_list) + self.set_series_for(self.transaction_type, series_list) # create series map(self.insert_series, [d.split(".")[0] for d in series_list if d.strip()]) @@ -84,7 +84,7 @@ class DocumentNamingSettings(Document): return self.get_transactions() def validate_series_set(self): - if self.select_doc_for_series and not self.set_options: + if self.transaction_type and not self.naming_series_options: frappe.throw(_("Please set the series to be used.")) def set_series_for(self, doctype, ol): @@ -126,7 +126,7 @@ class DocumentNamingSettings(Document): ) ps.save() - self.set_options = "\n".join(options) + self.naming_series_options = "\n".join(options) frappe.clear_cache(doctype=doctype) @@ -137,19 +137,19 @@ class DocumentNamingSettings(Document): """select dt.name from `tabDocField` df, `tabDocType` dt where dt.name = df.parent and df.fieldname='naming_series' and dt.name != %s""", - self.select_doc_for_series, + self.transaction_type, ) + frappe.db.sql_list( """select dt.name from `tabCustom Field` df, `tabDocType` dt where dt.name = df.dt and df.fieldname='naming_series' and dt.name != %s""", - self.select_doc_for_series, + self.transaction_type, ) ) ) sr = [[frappe.get_meta(p).get_field("naming_series").options, p] for p in parent] - dt = frappe.get_doc("DocType", self.select_doc_for_series) - options = self.scrub_options_list(self.set_options.split("\n")) + dt = frappe.get_doc("DocType", self.transaction_type) + options = self.scrub_options_list(self.naming_series_options.split("\n")) for series in options: validate_series(dt, series) for i in sr: @@ -168,8 +168,8 @@ class DocumentNamingSettings(Document): @frappe.whitelist() def get_options(self, arg=None): - if frappe.get_meta(arg or self.select_doc_for_series).get_field("naming_series"): - return frappe.get_meta(arg or self.select_doc_for_series).get_field("naming_series").options + if frappe.get_meta(arg or self.transaction_type).get_field("naming_series"): + return frappe.get_meta(arg or self.transaction_type).get_field("naming_series").options @frappe.whitelist() def get_current(self, arg=None): @@ -210,7 +210,7 @@ class DocumentNamingSettings(Document): """Preview what the naming series will generate.""" generated_names = [] - series = self.naming_series_to_check + series = self.try_naming_series if not series: return "" @@ -230,7 +230,7 @@ class DocumentNamingSettings(Document): def _fetch_last_doc_if_available(self): """Fetch last doc for evaluating naming series with fields.""" try: - return frappe.get_last_doc(self.select_doc_for_series) + return frappe.get_last_doc(self.transaction_type) except Exception: return None 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 3823c5f99c..13da12fa19 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 @@ -16,13 +16,13 @@ class TestNamingSeries(FrappeTestCase): frappe.db.rollback() def test_naming_preview(self): - self.ns.select_doc_for_series = "Sales Invoice" + self.ns.transaction_type = "Sales Invoice" - self.ns.naming_series_to_check = "AXBZ.####" + self.ns.try_naming_series = "AXBZ.####" serieses = self.ns.preview_series().split("\n") self.assertEqual(["AXBZ0001", "AXBZ0002", "AXBZ0003"], serieses) - self.ns.naming_series_to_check = "AXBZ-.{currency}.-" + self.ns.try_naming_series = "AXBZ-.{currency}.-" serieses = self.ns.preview_series().split("\n") def test_get_transactions(self): From b0a7c47de7cccc496cf3eef124b4f96cee7c2e1f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 17:50:57 +0530 Subject: [PATCH 30/62] fix(UX): dont query options for doctype that dont exist --- .../document_naming_settings.py | 10 +++++++--- .../test_document_naming_settings.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) 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 13e1e4aeba..b3f1ad0e97 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -167,9 +167,13 @@ class DocumentNamingSettings(Document): ) @frappe.whitelist() - def get_options(self, arg=None): - if frappe.get_meta(arg or self.transaction_type).get_field("naming_series"): - return frappe.get_meta(arg or self.transaction_type).get_field("naming_series").options + def get_options(self, doctype=None): + doctype = doctype or self.transaction_type + if not doctype: + return + + if frappe.get_meta(doctype or self.transaction_type).get_field("naming_series"): + return frappe.get_meta(doctype or self.transaction_type).get_field("naming_series").options @frappe.whitelist() def get_current(self, arg=None): 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 13da12fa19..f57d7b06aa 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 @@ -10,7 +10,7 @@ from frappe.tests.utils import FrappeTestCase class TestNamingSeries(FrappeTestCase): def setUp(self): - self.ns: DocumentNamingSettings = frappe.get_doc("Naming Series Settings") + self.ns: DocumentNamingSettings = frappe.get_doc("Document Naming Settings") def tearDown(self): frappe.db.rollback() From c75cfcc659eb216786a95636937e245c30b6da4d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 17:55:20 +0530 Subject: [PATCH 31/62] fix(UX): tab layout + misc - Use tab layout - Use second column for instructions / help - Avoid huge content slides while selecting transactions --- .../document_naming_settings.js | 17 --------- .../document_naming_settings.json | 35 ++++++++++++------- 2 files changed, 22 insertions(+), 30 deletions(-) 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 a2a5e10834..39a4854288 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -34,7 +34,6 @@ frappe.ui.form.on("Document Naming Settings", { frm.set_value("naming_series_options", r.message); if (r.message && r.message.split("\n")[0] == "") frm.set_value("user_must_always_select", 1); - frm.refresh(); }, }); }, @@ -75,20 +74,4 @@ frappe.ui.form.on("Document Naming Settings", { }, }); }, - - add_series(frm) { - const series = frm.doc.try_naming_series; - - if (!series) { - frappe.show_alert(__("Please type a valid series.")); - return; - } - - if (!frm.doc.naming_series_options.includes(series)) { - const current_series = frm.doc.naming_series_options; - frm.set_value("naming_series_options", `${current_series}\n${series}`); - } else { - frappe.show_alert(__("Series already added to transaction.")); - } - }, }); 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 98f7df87a8..dccace9947 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.json +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -1,20 +1,21 @@ { "actions": [], "creation": "2022-05-30 07:24:07.736646", - "description": "Set prefix for numbering series on your transactions", + "description": "Configure various aspects of how document naming works like naming series, current counter.", "doctype": "DocType", "engine": "InnoDB", "field_order": [ + "naming_series_tab", "setup_series", "transaction_type", - "help_html", "try_naming_series", "preview", "add_series", "naming_series_options", "user_must_always_select", "update", - "column_break_13", + "column_break_9", + "help_html", "update_series", "prefix", "current_value", @@ -22,10 +23,11 @@ ], "fields": [ { - "description": "Set prefix for numbering series on your transactions", + "collapsible": 1, + "description": "Set Naming Series options on your transactions.", "fieldname": "setup_series", "fieldtype": "Section Break", - "label": "Setup Series" + "label": "Setup Series for transactions" }, { "depends_on": "transaction_type", @@ -61,14 +63,11 @@ "label": "Update" }, { - "fieldname": "column_break_13", - "fieldtype": "Column Break" - }, - { - "description": "Change the starting / current sequence number of an existing series.", + "collapsible": 1, + "description": "Change the starting / current sequence number of an existing series.
\n\nWarning: Incorrectly updating counters can prevent documents from getting created. ", "fieldname": "update_series", "fieldtype": "Section Break", - "label": "Update Series" + "label": "Update Series Counter" }, { "fieldname": "prefix", @@ -84,7 +83,8 @@ { "fieldname": "update_series_start", "fieldtype": "Button", - "label": "Update Series Number" + "label": "Update Series Number", + "options": "update_series_start" }, { "depends_on": "transaction_type", @@ -101,13 +101,22 @@ "fieldname": "transaction_type", "fieldtype": "Select", "label": "Select Transaction" + }, + { + "fieldname": "column_break_9", + "fieldtype": "Column Break" + }, + { + "fieldname": "naming_series_tab", + "fieldtype": "Tab Break", + "label": "Naming Series" } ], "hide_toolbar": 1, "icon": "fa fa-sort-by-order", "issingle": 1, "links": [], - "modified": "2022-05-30 08:07:28.047633", + "modified": "2022-05-30 23:51:36.136535", "modified_by": "Administrator", "module": "Core", "name": "Document Naming Settings", From 28721be87582ce0dceb16929ebfb38ca4eac35fe Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 18:24:11 +0530 Subject: [PATCH 32/62] fix(UX): auto complete for selecting transaction Giant select fields are bad for UX, many users dont know they can type prefix to select value which leads to slow "visual search" --- .../document_naming_settings.js | 17 +++++------------ .../document_naming_settings.json | 17 ++++++----------- .../document_naming_settings.py | 4 +--- 3 files changed, 12 insertions(+), 26 deletions(-) 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 39a4854288..15b0339f7a 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -2,25 +2,18 @@ // For license information, please see license.txt frappe.ui.form.on("Document Naming Settings", { - onload: function(frm) { - frm.events.get_doc_and_prefix(frm); - }, - refresh: function(frm) { + frm.trigger("setup_transaction_autocomplete"); frm.disable_save(); }, - get_doc_and_prefix: function(frm) { + setup_transaction_autocomplete: function(frm) { frappe.call({ method: "get_transactions", doc: frm.doc, callback: function(r) { - frm.set_df_property( - "transaction_type", - "options", - r.message.transactions - ); - frm.set_df_property("prefix", "options", r.message.prefixes); + frm.fields_dict.transaction_type.set_data(r.message.transactions); + frm.fields_dict.prefix.set_data(r.message.prefixes); }, }); }, @@ -53,7 +46,7 @@ frappe.ui.form.on("Document Naming Settings", { method: "update_series", doc: frm.doc, callback: function(r) { - frm.events.get_doc_and_prefix(frm); + frm.trigger("setup_transaction_autocomplete"); }, }); }, 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 dccace9947..1dd84735b6 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.json +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -8,13 +8,12 @@ "naming_series_tab", "setup_series", "transaction_type", - "try_naming_series", - "preview", - "add_series", "naming_series_options", "user_must_always_select", "update", "column_break_9", + "try_naming_series", + "preview", "help_html", "update_series", "prefix", @@ -34,7 +33,7 @@ "fieldname": "help_html", "fieldtype": "HTML", "label": "Help HTML", - "options": "
\n Edit list of Series in the box below. Rules:\n
    \n
  • Each Series Prefix on a new line.
    • \n
  • Allowed special characters are \"/\" and \"-\"
    • \n
  • \n Optionally, set the number of digits in the series using dot (.)\n followed by hashes (#). For example, \".####\" means that the series\n will have four digits. Default is five digits.\n
    • \n
  • \n You can also use variables in the series name by putting them\n between (.) dots\n
    \n Support Variables:\n
      \n
    • .YYYY. - Year in 4 digits
      • \n
    • .YY. - Year in 2 digits
      • \n
    • .MM. - Month
      • \n
    • .DD. - Day of month
      • \n
    • .WW. - Week of the year
      • \n
    • .FY. - Fiscal Year
      • \n
    • \n .{fieldname}. - fieldname on the document e.g.\n branch\n
      • \n
    \n
    • \n
\n Examples:\n
    \n
  • INV-
    • \n
  • INV-10-
    • \n
  • INVK-
    • \n
  • INV-.YYYY.-.{branch}.-.MM.-.####
    • \n
\n
\n
\n" + "options": "
\n Edit list of Series in the box. Rules:\n
    \n
  • Each Series Prefix on a new line.
    • \n
  • Allowed special characters are \"/\" and \"-\"
    • \n
  • \n Optionally, set the number of digits in the series using dot (.)\n followed by hashes (#). For example, \".####\" means that the series\n will have four digits. Default is five digits.\n
    • \n
  • \n You can also use variables in the series name by putting them\n between (.) dots\n
    \n Supported Variables:\n
      \n
    • .YYYY. - Year in 4 digits
      • \n
    • .YY. - Year in 2 digits
      • \n
    • .MM. - Month
      • \n
    • .DD. - Day of month
      • \n
    • .WW. - Week of the year
      • \n
    • .FY. - Fiscal Year
      • \n
    • \n .{fieldname}. - fieldname on the document e.g.\n branch\n
      • \n
    \n
    • \n
\n Examples:\n
    \n
  • INV-
    • \n
  • INV-10-
    • \n
  • INVK-
    • \n
  • INV-.YYYY.-.{branch}.-.MM.-.####
    • \n
\n
\n
\n" }, { "default": " ", @@ -43,11 +42,6 @@ "label": "Preview of generated names", "read_only": 1 }, - { - "fieldname": "add_series", - "fieldtype": "Button", - "label": "Add this Series" - }, { "default": "0", "depends_on": "transaction_type", @@ -71,7 +65,7 @@ }, { "fieldname": "prefix", - "fieldtype": "Select", + "fieldtype": "Autocomplete", "label": "Prefix" }, { @@ -93,13 +87,14 @@ "label": "Series List for this Transaction" }, { + "depends_on": "transaction_type", "fieldname": "try_naming_series", "fieldtype": "Data", "label": "Try a naming Series" }, { "fieldname": "transaction_type", - "fieldtype": "Select", + "fieldtype": "Autocomplete", "label": "Select Transaction" }, { 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 b3f1ad0e97..1dd73062d3 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -58,9 +58,7 @@ class DocumentNamingSettings(Document): if custom_prefixes: prefixes = prefixes + [d.autoname.rsplit(".", 1)[0] for d in custom_prefixes] - prefixes = "\n".join(sorted(prefixes)) - - return {"transactions": "\n".join([""] + sorted(doctypes)), "prefixes": prefixes} + return {"transactions": sorted(doctypes), "prefixes": sorted(prefixes)} def scrub_options_list(self, ol): options = list(filter(lambda x: x, [cstr(n).strip() for n in ol])) From 0fca966a8f5d3180669bbf64406ad95bce97b87a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 18:45:50 +0530 Subject: [PATCH 33/62] refactor: rename preview field `preview` is very common and might conflict with other settings in future --- .../document_naming_settings.js | 4 ++-- .../document_naming_settings.json | 16 ++++++++-------- .../document_naming_settings.py | 1 - 3 files changed, 10 insertions(+), 11 deletions(-) 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 15b0339f7a..15080b8498 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -57,10 +57,10 @@ frappe.ui.form.on("Document Naming Settings", { doc: frm.doc, callback: function(r) { if (!r.exc) { - frm.set_value("preview", r.message); + frm.set_value("series_preview", r.message); } else { frm.set_value( - "preview", + "series_preview", __("Failed to generate preview of series") ); } 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 1dd84735b6..4c86b2ec1d 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.json +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -13,7 +13,7 @@ "update", "column_break_9", "try_naming_series", - "preview", + "series_preview", "help_html", "update_series", "prefix", @@ -35,13 +35,6 @@ "label": "Help HTML", "options": "
\n Edit list of Series in the box. Rules:\n
    \n
  • Each Series Prefix on a new line.
    • \n
  • Allowed special characters are \"/\" and \"-\"
    • \n
  • \n Optionally, set the number of digits in the series using dot (.)\n followed by hashes (#). For example, \".####\" means that the series\n will have four digits. Default is five digits.\n
    • \n
  • \n You can also use variables in the series name by putting them\n between (.) dots\n
    \n Supported Variables:\n
      \n
    • .YYYY. - Year in 4 digits
      • \n
    • .YY. - Year in 2 digits
      • \n
    • .MM. - Month
      • \n
    • .DD. - Day of month
      • \n
    • .WW. - Week of the year
      • \n
    • .FY. - Fiscal Year
      • \n
    • \n .{fieldname}. - fieldname on the document e.g.\n branch\n
      • \n
    \n
    • \n
\n Examples:\n
    \n
  • INV-
    • \n
  • INV-10-
    • \n
  • INVK-
    • \n
  • INV-.YYYY.-.{branch}.-.MM.-.####
    • \n
\n
\n
\n" }, - { - "default": " ", - "fieldname": "preview", - "fieldtype": "Text", - "label": "Preview of generated names", - "read_only": 1 - }, { "default": "0", "depends_on": "transaction_type", @@ -88,6 +81,7 @@ }, { "depends_on": "transaction_type", + "description": "Generate 3 preview of names generate by any valid series.", "fieldname": "try_naming_series", "fieldtype": "Data", "label": "Try a naming Series" @@ -105,6 +99,12 @@ "fieldname": "naming_series_tab", "fieldtype": "Tab Break", "label": "Naming Series" + }, + { + "fieldname": "series_preview", + "fieldtype": "Text", + "label": "Preview of generated names", + "read_only": 1 } ], "hide_toolbar": 1, 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 1dd73062d3..ea45860500 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -38,7 +38,6 @@ class DocumentNamingSettings(Document): options = self.get_options(d) except frappe.DoesNotExistError: frappe.msgprint(_("Unable to find DocType {0}").format(d)) - # frappe.pass_does_not_exist_error() continue if options: From 15dbcacd564de96544ebb99a544ef109306fe8db Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 18:58:59 +0530 Subject: [PATCH 34/62] refactor: split transaction and prefix functions refactor: fetching transaction list refactor: get default naming series refactor: simplify naming series option scrubbing fix: remove dead code related to insert series This code is dead in many ways - Creates map but it's never evaluated, so it doesn't actually run :D - Just randomly splits naming series parts to create prefix... makes no sense. fix: deduplicate prefix list --- .../document_naming_settings.js | 2 +- .../document_naming_settings.py | 136 +++++------------- .../test_document_naming_settings.py | 13 +- frappe/model/meta.py | 11 ++ frappe/model/naming.py | 15 +- 5 files changed, 62 insertions(+), 115 deletions(-) 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 15080b8498..578a9cb2ca 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -9,7 +9,7 @@ frappe.ui.form.on("Document Naming Settings", { setup_transaction_autocomplete: function(frm) { frappe.call({ - method: "get_transactions", + method: "get_transactions_and_prefixes", doc: frm.doc, callback: function(r) { frm.fields_dict.transaction_type.set_data(r.message.transactions); 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 ea45860500..71b5afc4f6 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -1,8 +1,10 @@ # Copyright (c) 2022, Frappe Technologies and contributors # For license information, please see license.txt +from typing import List + import frappe -from frappe import _, msgprint, throw +from frappe import _ from frappe.core.doctype.doctype.doctype import validate_series from frappe.model.document import Document from frappe.model.naming import make_autoname, parse_naming_series @@ -16,21 +18,23 @@ class NamingSeriesNotSetError(frappe.ValidationError): class DocumentNamingSettings(Document): @frappe.whitelist() - def get_transactions(self, arg=None): - doctypes = list( - set( - frappe.db.sql_list( - """select parent - from `tabDocField` df where fieldname='naming_series'""" - ) - + frappe.db.sql_list( - """select dt from `tabCustom Field` - where fieldname='naming_series'""" - ) - ) - ) + def get_transactions_and_prefixes(self, arg=None): - doctypes = list(set(get_doctypes_with_read()).intersection(set(doctypes))) + transactions = self._get_transactions() + prefixes = self._get_prefixes(transactions) + + return {"transactions": transactions, "prefixes": prefixes} + + def _get_transactions(self) -> List[str]: + + readable_doctypes = set(get_doctypes_with_read()) + + standard = frappe.get_all("DocField", {"fieldname": "naming_series"}, "parent", pluck="parent") + custom = frappe.get_all("Custom Field", {"fieldname": "naming_series"}, "dt", pluck="dt") + + return sorted(readable_doctypes.intersection(standard + custom)) + + def _get_prefixes(self, doctypes) -> List[str]: prefixes = "" for d in doctypes: options = "" @@ -57,35 +61,28 @@ class DocumentNamingSettings(Document): if custom_prefixes: prefixes = prefixes + [d.autoname.rsplit(".", 1)[0] for d in custom_prefixes] - return {"transactions": sorted(doctypes), "prefixes": sorted(prefixes)} + return sorted(set(prefixes)) - def scrub_options_list(self, ol): - options = list(filter(lambda x: x, [cstr(n).strip() for n in ol])) - return options + def get_options_list(self, options: str) -> List[str]: + return [op.strip() for op in options.split("\n") if op.strip()] @frappe.whitelist() - def update_series(self, arg=None): + def update_series(self): """update series list""" self.validate_series_set() self.check_duplicate() - series_list = self.naming_series_options.split("\n") + self.set_series_options_in_meta(self.transaction_type, self.naming_series_options) - # set in doctype - self.set_series_for(self.transaction_type, series_list) - - # create series - map(self.insert_series, [d.split(".")[0] for d in series_list if d.strip()]) - - msgprint(_("Series Updated")) - - return self.get_transactions() + frappe.msgprint( + _("Series Updated for {}").format(self.transaction_type), alert=True, indicator="green" + ) def validate_series_set(self): if self.transaction_type and not self.naming_series_options: frappe.throw(_("Please set the series to be used.")) - def set_series_for(self, doctype, ol): - options = self.scrub_options_list(ol) + def set_series_options_in_meta(self, doctype: str, options: str) -> None: + options = self.get_options_list(options) # validate names for i in options: @@ -146,7 +143,7 @@ class DocumentNamingSettings(Document): ) sr = [[frappe.get_meta(p).get_field("naming_series").options, p] for p in parent] dt = frappe.get_doc("DocType", self.transaction_type) - options = self.scrub_options_list(self.naming_series_options.split("\n")) + options = self.get_options_list(self.naming_series_options) for series in options: validate_series(dt, series) for i in sr: @@ -159,7 +156,7 @@ class DocumentNamingSettings(Document): import re if not re.match(r"^[\w\- \/.#{}]+$", n, re.UNICODE): - throw( + frappe.throw( _('Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series') ) @@ -192,9 +189,9 @@ class DocumentNamingSettings(Document): frappe.db.sql( "update `tabSeries` set current = %s where name = %s", (cint(self.current_value), prefix) ) - msgprint(_("Series Updated Successfully")) + frappe.msgprint(_("Series Updated Successfully")) else: - msgprint(_("Please select prefix first")) + frappe.msgprint(_("Please select prefix first")) def parse_naming_series(self): parts = self.prefix.split(".") @@ -234,70 +231,3 @@ class DocumentNamingSettings(Document): return frappe.get_last_doc(self.transaction_type) except Exception: return None - - -def set_by_naming_series( - doctype, fieldname, naming_series, hide_name_field=True, make_mandatory=1 -): - from frappe.custom.doctype.property_setter.property_setter import make_property_setter - - if naming_series: - make_property_setter( - doctype, "naming_series", "hidden", 0, "Check", validate_fields_for_doctype=False - ) - make_property_setter( - doctype, "naming_series", "reqd", make_mandatory, "Check", validate_fields_for_doctype=False - ) - - # set values for mandatory - try: - frappe.db.sql( - """update `tab{doctype}` set naming_series={s} where - ifnull(naming_series, '')=''""".format( - doctype=doctype, s="%s" - ), - get_default_naming_series(doctype), - ) - except NamingSeriesNotSetError: - pass - - if hide_name_field: - make_property_setter(doctype, fieldname, "reqd", 0, "Check", validate_fields_for_doctype=False) - make_property_setter( - doctype, fieldname, "hidden", 1, "Check", validate_fields_for_doctype=False - ) - else: - make_property_setter( - doctype, "naming_series", "reqd", 0, "Check", validate_fields_for_doctype=False - ) - make_property_setter( - doctype, "naming_series", "hidden", 1, "Check", validate_fields_for_doctype=False - ) - - if hide_name_field: - make_property_setter( - doctype, fieldname, "hidden", 0, "Check", validate_fields_for_doctype=False - ) - make_property_setter(doctype, fieldname, "reqd", 1, "Check", validate_fields_for_doctype=False) - - # set values for mandatory - frappe.db.sql( - """update `tab{doctype}` set `{fieldname}`=`name` where - ifnull({fieldname}, '')=''""".format( - doctype=doctype, fieldname=fieldname - ) - ) - - -def get_default_naming_series(doctype): - naming_series = frappe.get_meta(doctype).get_field("naming_series").options or "" - naming_series = naming_series.split("\n") - out = naming_series[0] or (naming_series[1] if len(naming_series) > 1 else None) - - if not out: - frappe.throw( - _("Please set Naming Series for {0} via Setup > Settings > Naming Series").format(doctype), - NamingSeriesNotSetError, - ) - else: - return out 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 f57d7b06aa..f9e687e10d 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 @@ -5,6 +5,7 @@ import frappe from frappe.core.doctype.document_naming_settings.document_naming_settings import ( DocumentNamingSettings, ) +from frappe.model.naming import get_default_naming_series from frappe.tests.utils import FrappeTestCase @@ -16,7 +17,7 @@ class TestNamingSeries(FrappeTestCase): frappe.db.rollback() def test_naming_preview(self): - self.ns.transaction_type = "Sales Invoice" + self.ns.transaction_type = "Webhook" self.ns.try_naming_series = "AXBZ.####" serieses = self.ns.preview_series().split("\n") @@ -27,10 +28,14 @@ class TestNamingSeries(FrappeTestCase): def test_get_transactions(self): - naming_info = self.ns.get_transactions() - self.assertIn("Sales Invoice", naming_info["transactions"]) + naming_info = self.ns.get_transactions_and_prefixes() + self.assertIn("Webhook", naming_info["transactions"]) - existing_naming_series = frappe.get_meta("Sales Invoice").get_field("naming_series").options + existing_naming_series = frappe.get_meta("Webhook").get_field("naming_series").options for series in existing_naming_series.split("\n"): self.assertIn(series, naming_info["prefixes"]) + + def test_default_naming_series(self): + self.assertIn("HOOK", get_default_naming_series("Webhook")) + self.assertIsNone(get_default_naming_series("DocType")) diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 4f7dc01ea4..65ab7d39c2 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -17,6 +17,7 @@ Example: import json import os from datetime import datetime +from typing import List import click @@ -341,6 +342,16 @@ class Meta(Document): def get_workflow(self): return get_workflow_name(self.name) + def get_naming_series_options(self) -> List[str]: + """Get list naming series options.""" + + field = self.get_field("naming_series") + if field: + options = field.options or "" + + return options.split("\n") + return [] + def add_custom_fields(self): if not frappe.db.table_exists("Custom Field"): return diff --git a/frappe/model/naming.py b/frappe/model/naming.py index bb93244a66..ff7448e069 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -311,14 +311,15 @@ def revert_series_if_last(key, name, doc=None): frappe.db.sql("UPDATE `tabSeries` SET `current` = `current` - 1 WHERE `name`=%s", prefix) -def get_default_naming_series(doctype): +def get_default_naming_series(doctype: str) -> Optional[str]: """get default value for `naming_series` property""" - naming_series = frappe.get_meta(doctype).get_field("naming_series").options or "" - if naming_series: - naming_series = naming_series.split("\n") - return naming_series[0] or naming_series[1] - else: - return None + naming_series_options = frappe.get_meta(doctype).get_naming_series_options() + + # Return first truthy options + # Empty strings are used to avoid populating forms by default + for option in naming_series_options: + if option: + return option def validate_name(doctype: str, name: Union[int, str], case: Optional[str] = None): From ebcb568e9d1bde1422bae8dfb650d133017065bf Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 21:19:03 +0530 Subject: [PATCH 35/62] test: making naming tests re-runnable These were modifying TODO schema ._. WAD --- frappe/tests/test_naming.py | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index e217f24154..89266d92ea 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -11,10 +11,11 @@ from frappe.model.naming import ( getseries, revert_series_if_last, ) +from frappe.tests.utils import FrappeTestCase from frappe.utils import now_datetime -class TestNaming(unittest.TestCase): +class TestNaming(FrappeTestCase): def setUp(self): frappe.db.delete("Note") @@ -52,16 +53,13 @@ class TestNaming(unittest.TestCase): self.assertEqual(country.name, country.country_name) def test_child_table_naming(self): - child_dt_with_naming = new_doctype( - "childtable_with_autonaming", istable=1, autoname="field:some_fieldname" - ).insert() + child_dt_with_naming = new_doctype(istable=1, autoname="field:some_fieldname").insert() dt_with_child_autoname = new_doctype( - "dt_with_childtable_naming", fields=[ { "label": "table with naming", "fieldname": "table_with_naming", - "options": "childtable_with_autonaming", + "options": child_dt_with_naming.name, "fieldtype": "Table", } ], @@ -69,7 +67,7 @@ class TestNaming(unittest.TestCase): name = frappe.generate_hash(length=10) - doc = frappe.new_doc("dt_with_childtable_naming") + doc = frappe.new_doc(dt_with_child_autoname.name) doc.append("table_with_naming", {"some_fieldname": name}) doc.save() self.assertEqual(doc.table_with_naming[0].name, name) @@ -89,31 +87,18 @@ class TestNaming(unittest.TestCase): """ Test if braced params are replaced in format autoname """ - doctype = "ToDo" - - todo_doctype = frappe.get_doc("DocType", doctype) - todo_doctype.autoname = "format:TODO-{MM}-{status}-{##}" - todo_doctype.save() + doctype = new_doctype(autoname="format:TODO-{MM}-{some_fieldname}-{##}").insert() description = "Format" - todo = frappe.new_doc(doctype) - todo.description = description - todo.insert() + doc = frappe.new_doc(doctype.name) + doc.some_fieldname = description + doc.insert() series = getseries("", 2) + series = int(series) - 1 - series = str(int(series) - 1) - - if len(series) < 2: - series = "0" + series - - self.assertEqual( - todo.name, - "TODO-{month}-{status}-{series}".format( - month=now_datetime().strftime("%m"), status=todo.status, series=series - ), - ) + self.assertEqual(doc.name, f"TODO-{now_datetime().strftime('%m')}-{description}-{series:02}") def test_format_autoname_for_consecutive_week_number(self): """ From efd083223510492ae1dfe514cfb71631dfe6c517 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 22:15:55 +0530 Subject: [PATCH 36/62] refactor: property setters for setting naming series --- .../document_naming_settings.py | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) 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 71b5afc4f6..35f16f282e 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -9,7 +9,7 @@ from frappe.core.doctype.doctype.doctype import validate_series from frappe.model.document import Document from frappe.model.naming import make_autoname, parse_naming_series from frappe.permissions import get_doctypes_with_read -from frappe.utils import cint, cstr +from frappe.utils import cint class NamingSeriesNotSetError(frappe.ValidationError): @@ -93,37 +93,20 @@ class DocumentNamingSettings(Document): default = options[0] if options else "" - # update in property setter - prop_dict = {"options": "\n".join(options), "default": default} + option_string = "\n".join(options) - for prop in prop_dict: - ps_exists = frappe.db.get_value( - "Property Setter", {"field_name": "naming_series", "doc_type": doctype, "property": prop} - ) + self.update_naming_series_property_setter(doctype, "options", option_string) + self.update_naming_series_property_setter(doctype, "default", default) - if ps_exists: - ps = frappe.get_doc("Property Setter", ps_exists) - ps.value = prop_dict[prop] - ps.save() - else: - ps = frappe.get_doc( - { - "doctype": "Property Setter", - "doctype_or_field": "DocField", - "doc_type": doctype, - "field_name": "naming_series", - "property": prop, - "value": prop_dict[prop], - "property_type": "Text", - "__islocal": 1, - } - ) - ps.save() - - self.naming_series_options = "\n".join(options) + self.naming_series_options = option_string frappe.clear_cache(doctype=doctype) + def update_naming_series_property_setter(self, doctype, property, value): + from frappe.custom.doctype.property_setter.property_setter import make_property_setter + + make_property_setter(doctype, "naming_series", property, value, "Text") + def check_duplicate(self): parent = list( set( From 952a59a04861ea94fc612912a4b6fcb351206486 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 22:28:27 +0530 Subject: [PATCH 37/62] fix: freeze dom while updating series --- .../document_naming_settings/document_naming_settings.js | 3 +++ 1 file changed, 3 insertions(+) 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 578a9cb2ca..2dc5fc4d58 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.js +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.js @@ -45,8 +45,11 @@ frappe.ui.form.on("Document Naming Settings", { frappe.call({ method: "update_series", doc: frm.doc, + freeze: true, + freeze_msg: __("Updating naming series options"), callback: function(r) { frm.trigger("setup_transaction_autocomplete"); + frm.trigger("transaction_type"); }, }); }, From 5c35aae8767a0eb34da44f9a0e1b2a4ec2935f3b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 09:54:21 +0530 Subject: [PATCH 38/62] fix: accurate prefix parsing Previous version of prefix parsing relied on partial reimplemntation of naming series logic which was outdated and often incorrect. --- .../document_naming_settings.py | 45 +++++++------- frappe/model/naming.py | 59 +++++++++++++++++-- frappe/tests/test_naming.py | 18 +++++- 3 files changed, 90 insertions(+), 32 deletions(-) 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 35f16f282e..1f6a71e567 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -7,7 +7,7 @@ import frappe from frappe import _ from frappe.core.doctype.doctype.doctype import validate_series from frappe.model.document import Document -from frappe.model.naming import make_autoname, parse_naming_series +from frappe.model.naming import get_naming_series_prefix, make_autoname from frappe.permissions import get_doctypes_with_read from frappe.utils import cint @@ -153,38 +153,35 @@ class DocumentNamingSettings(Document): return frappe.get_meta(doctype or self.transaction_type).get_field("naming_series").options @frappe.whitelist() - def get_current(self, arg=None): + def get_current(self): """get series current""" if self.prefix: - prefix = self.parse_naming_series() + prefix = get_naming_series_prefix(self.prefix) self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name") - def insert_series(self, series): - """insert series if missing""" - if frappe.db.get_value("Series", series, "name", order_by="name") == None: - frappe.db.sql("insert into tabSeries (name, current) values (%s, 0)", (series)) - @frappe.whitelist() def update_series_start(self): - if self.prefix: - prefix = self.parse_naming_series() - self.insert_series(prefix) - frappe.db.sql( - "update `tabSeries` set current = %s where name = %s", (cint(self.current_value), prefix) - ) - frappe.msgprint(_("Series Updated Successfully")) - else: - frappe.msgprint(_("Please select prefix first")) + if not self.prefix: + frappe.throw(_("Please select prefix first")) - def parse_naming_series(self): - parts = self.prefix.split(".") + series = frappe.qb.DocType("Series") - # Remove ### from the end of series - if parts[-1] == "#" * len(parts[-1]): - del parts[-1] + db_prefix = get_naming_series_prefix(self.prefix) - prefix = parse_naming_series(parts) - return prefix + if frappe.db.get_value("Series", db_prefix, "name", order_by="name") is None: + series.insert(db_prefix, 0).columns(series.name, series.current).run() + + ( + frappe.qb.update(series) + .set(series.current, cint(self.current_value)) + .where(series.name == db_prefix) + ).run() + + frappe.msgprint( + _("Series counter for {} updated to {} successfully").format(self.prefix, self.current_value), + alert=True, + indicator="green", + ) @frappe.whitelist() def preview_series(self) -> str: diff --git a/frappe/model/naming.py b/frappe/model/naming.py index ff7448e069..9ffb72b09c 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -2,7 +2,7 @@ # License: MIT. See LICENSE import re -from typing import TYPE_CHECKING, Optional, Union +from typing import TYPE_CHECKING, Callable, List, Optional, Union import frappe from frappe import _ @@ -11,6 +11,7 @@ from frappe.query_builder import DocType from frappe.utils import cint, cstr, now_datetime if TYPE_CHECKING: + from frappe.model.document import Document from frappe.model.meta import Meta @@ -189,10 +190,28 @@ def make_autoname(key="", doctype="", doc=""): return n -def parse_naming_series(parts, doctype="", doc=""): - n = "" +def parse_naming_series( + parts: Union[List[str], str], + doctype=None, + doc: Optional["Document"] = None, + number_generator: Optional[Callable[[str, int], str]] = None, +) -> str: + + """Parse the naming series and get next name. + + args: + parts: naming series parts (split by `.`) + doc: document to use for series that have parts using fieldnames + number_generator: Use different counter backend other than `tabSeries`. Primarily used for testing. + """ + + name = "" if isinstance(parts, str): parts = parts.split(".") + + if not number_generator: + number_generator = getseries + series_set = False today = now_datetime() for e in parts: @@ -200,7 +219,7 @@ def parse_naming_series(parts, doctype="", doc=""): if e.startswith("#"): if not series_set: digits = len(e) - part = getseries(n, digits) + part = number_generator(name, digits) series_set = True elif e == "YY": part = today.strftime("%y") @@ -225,9 +244,37 @@ def parse_naming_series(parts, doctype="", doc=""): part = e if isinstance(part, str): - n += part + name += part - return n + return name + + +def get_naming_series_prefix(series: str) -> str: + """Naming series stores prefix to maintain a counter in DB. This prefix can be used to update counter and/or other validations. + + e.g. `SINV-.YY.-.####` has prefix of `SINV-22-` in database for year 2022. + """ + + prefix = None + + if "#" not in series: + series += ".#####" + + def fake_counter_backend(partial_series, digits): + nonlocal prefix + prefix = partial_series + return "#" * digits + + # This function evaluates all parts till we hit numerical parts and then + # sends prefix + digits to DB to find next number. + # Instead of reimplemnted the whole parsing logic in multiple places we can + # just ask this function to give us the prefix. + parse_naming_series(series, number_generator=fake_counter_backend) + + if prefix is None: + frappe.throw(_("Invalid Naming Series")) + + return prefix def determine_consecutive_week_number(datetime): diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index 89266d92ea..cad91c11a3 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -1,13 +1,12 @@ # Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -import unittest - import frappe from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.model.naming import ( append_number_if_name_exists, determine_consecutive_week_number, + get_naming_series_prefix, getseries, revert_series_if_last, ) @@ -288,6 +287,21 @@ class TestNaming(FrappeTestCase): dt.delete(ignore_permissions=True) + def test_naming_series_prefix(self): + today = now_datetime() + year = today.strftime("%y") + month = today.strftime("%m") + + prefix_test_cases = { + "SINV-.YY.-.####": f"SINV-{year}-", + "SINV-.YY.-.MM.-.####": f"SINV-{year}-{month}-", + "SINV": "SINV", + "SINV-.": "SINV-", + } + + for series, prefix in prefix_test_cases.items(): + self.assertEqual(prefix, get_naming_series_prefix(series)) + def make_invalid_todo(): frappe.get_doc({"doctype": "ToDo", "description": "Test"}).insert(set_name="ToDo") From ae678cca7846b0b127b91b51289528052b1453b2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 10:44:52 +0530 Subject: [PATCH 39/62] refactor: simplify duplicate naming series check --- .../document_naming_settings.py | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) 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 1f6a71e567..c44b276075 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -69,7 +69,7 @@ class DocumentNamingSettings(Document): @frappe.whitelist() def update_series(self): """update series list""" - self.validate_series_set() + self.validate_set_series() self.check_duplicate() self.set_series_options_in_meta(self.transaction_type, self.naming_series_options) @@ -77,7 +77,7 @@ class DocumentNamingSettings(Document): _("Series Updated for {}").format(self.transaction_type), alert=True, indicator="green" ) - def validate_series_set(self): + def validate_set_series(self): if self.transaction_type and not self.naming_series_options: frappe.throw(_("Please set the series to be used.")) @@ -108,32 +108,27 @@ class DocumentNamingSettings(Document): make_property_setter(doctype, "naming_series", property, value, "Text") def check_duplicate(self): - parent = list( - set( - frappe.db.sql_list( - """select dt.name - from `tabDocField` df, `tabDocType` dt - where dt.name = df.parent and df.fieldname='naming_series' and dt.name != %s""", - self.transaction_type, - ) - + frappe.db.sql_list( - """select dt.name - from `tabCustom Field` df, `tabDocType` dt - where dt.name = df.dt and df.fieldname='naming_series' and dt.name != %s""", - self.transaction_type, - ) - ) - ) - sr = [[frappe.get_meta(p).get_field("naming_series").options, p] for p in parent] + def stripped_series(s: str) -> str: + return s.strip().rstrip("#") + + standard = frappe.get_all("DocField", {"fieldname": "naming_series"}, "parent", pluck="parent") + custom = frappe.get_all("Custom Field", {"fieldname": "naming_series"}, "dt", pluck="dt") + + all_doctypes_with_naming_series = set(standard + custom) + all_doctypes_with_naming_series.remove(self.transaction_type) + + existing_series = {} + for doctype in all_doctypes_with_naming_series: + for series in frappe.get_meta(doctype).get_naming_series_options(): + existing_series[stripped_series(series)] = doctype + dt = frappe.get_doc("DocType", self.transaction_type) + options = self.get_options_list(self.naming_series_options) for series in options: + if stripped_series(series) in existing_series: + frappe.throw(_("Series {0} already used in {1}").format(series, existing_series[series])) validate_series(dt, series) - for i in sr: - if i[0]: - existing_series = [d.split(".")[0] for d in i[0].split("\n")] - if series.split(".")[0] in existing_series: - frappe.throw(_("Series {0} already used in {1}").format(series, i[1])) def validate_series_name(self, n): import re From 6930822a207f581f861065f75177f10239ff7dc9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 10:56:02 +0530 Subject: [PATCH 40/62] refactor: simplify _get_prefixes --- .../document_naming_settings.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) 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 c44b276075..0402ea90ea 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -18,7 +18,7 @@ class NamingSeriesNotSetError(frappe.ValidationError): class DocumentNamingSettings(Document): @frappe.whitelist() - def get_transactions_and_prefixes(self, arg=None): + def get_transactions_and_prefixes(self): transactions = self._get_transactions() prefixes = self._get_prefixes(transactions) @@ -35,20 +35,15 @@ class DocumentNamingSettings(Document): return sorted(readable_doctypes.intersection(standard + custom)) def _get_prefixes(self, doctypes) -> List[str]: - prefixes = "" + prefixes = set() for d in doctypes: - options = "" try: - options = self.get_options(d) + options = frappe.get_meta(d).get_naming_series_options() + prefixes.update(options) except frappe.DoesNotExistError: frappe.msgprint(_("Unable to find DocType {0}").format(d)) continue - if options: - prefixes = prefixes + "\n" + options - prefixes.replace("\n\n", "\n") - prefixes = prefixes.split("\n") - custom_prefixes = frappe.get_all( "DocType", fields=["autoname"], @@ -59,9 +54,9 @@ class DocumentNamingSettings(Document): }, ) if custom_prefixes: - prefixes = prefixes + [d.autoname.rsplit(".", 1)[0] for d in custom_prefixes] + prefixes.update([d.autoname.rsplit(".", 1)[0] for d in custom_prefixes]) - return sorted(set(prefixes)) + return sorted(prefixes) def get_options_list(self, options: str) -> List[str]: return [op.strip() for op in options.split("\n") if op.strip()] From 5590cb0be83c4878f473ec4bb50fce82a0afa45e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 10:19:56 +0530 Subject: [PATCH 41/62] feat: NamingSeries class Single class to group together everything required related to naming series --- .../document_naming_settings.py | 20 ++-- .../test_document_naming_settings.py | 15 +-- frappe/model/naming.py | 103 +++++++++++------- frappe/tests/test_naming.py | 21 +++- 4 files changed, 98 insertions(+), 61 deletions(-) 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 0402ea90ea..01a230b833 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -1,13 +1,14 @@ # Copyright (c) 2022, Frappe Technologies and contributors # For license information, please see license.txt +import re from typing import List import frappe from frappe import _ from frappe.core.doctype.doctype.doctype import validate_series from frappe.model.document import Document -from frappe.model.naming import get_naming_series_prefix, make_autoname +from frappe.model.naming import NamingSeries, make_autoname from frappe.permissions import get_doctypes_with_read from frappe.utils import cint @@ -80,8 +81,8 @@ class DocumentNamingSettings(Document): options = self.get_options_list(options) # validate names - for i in options: - self.validate_series_name(i) + for series in options: + self.validate_series_name(series) if options and self.user_must_always_select: options = [""] + options @@ -125,13 +126,8 @@ class DocumentNamingSettings(Document): frappe.throw(_("Series {0} already used in {1}").format(series, existing_series[series])) validate_series(dt, series) - def validate_series_name(self, n): - import re - - if not re.match(r"^[\w\- \/.#{}]+$", n, re.UNICODE): - frappe.throw( - _('Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series') - ) + def validate_series_name(self, series): + NamingSeries(series).validate() @frappe.whitelist() def get_options(self, doctype=None): @@ -146,7 +142,7 @@ class DocumentNamingSettings(Document): def get_current(self): """get series current""" if self.prefix: - prefix = get_naming_series_prefix(self.prefix) + prefix = NamingSeries(self.prefix).get_prefix() self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name") @frappe.whitelist() @@ -156,7 +152,7 @@ class DocumentNamingSettings(Document): series = frappe.qb.DocType("Series") - db_prefix = get_naming_series_prefix(self.prefix) + db_prefix = NamingSeries(self.prefix).get_prefix() if frappe.db.get_value("Series", db_prefix, "name", order_by="name") is None: series.insert(db_prefix, 0).columns(series.name, series.current).run() 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 f9e687e10d..85ecc3b5d9 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 @@ -3,6 +3,7 @@ import frappe from frappe.core.doctype.document_naming_settings.document_naming_settings import ( + NAMING_SERIES_PATTERN, DocumentNamingSettings, ) from frappe.model.naming import get_default_naming_series @@ -11,24 +12,24 @@ from frappe.tests.utils import FrappeTestCase class TestNamingSeries(FrappeTestCase): def setUp(self): - self.ns: DocumentNamingSettings = frappe.get_doc("Document Naming Settings") + self.dns: DocumentNamingSettings = frappe.get_doc("Document Naming Settings") def tearDown(self): frappe.db.rollback() def test_naming_preview(self): - self.ns.transaction_type = "Webhook" + self.dns.transaction_type = "Webhook" - self.ns.try_naming_series = "AXBZ.####" - serieses = self.ns.preview_series().split("\n") + self.dns.try_naming_series = "AXBZ.####" + serieses = self.dns.preview_series().split("\n") self.assertEqual(["AXBZ0001", "AXBZ0002", "AXBZ0003"], serieses) - self.ns.try_naming_series = "AXBZ-.{currency}.-" - serieses = self.ns.preview_series().split("\n") + self.dns.try_naming_series = "AXBZ-.{currency}.-" + serieses = self.dns.preview_series().split("\n") def test_get_transactions(self): - naming_info = self.ns.get_transactions_and_prefixes() + naming_info = self.dns.get_transactions_and_prefixes() self.assertIn("Webhook", naming_info["transactions"]) existing_naming_series = frappe.get_meta("Webhook").get_field("naming_series").options diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 9ffb72b09c..50535f0a99 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -19,6 +19,67 @@ if TYPE_CHECKING: # whether `log_types` have autoincremented naming set for the site or not. autoincremented_site_status_map = {} +NAMING_SERIES_PATTERN = re.compile(r"^[\w\- \/.#{}]+$", re.UNICODE) + + +class InvalidNamingSeriesError(frappe.ValidationError): + pass + + +class NamingSeries: + __slots__ = ("series",) + + def __init__(self, series: str): + self.series = series + + # Add default number part if missing + if "#" not in self.series: + self.series += ".#####" + + def validate(self): + if "." not in self.series: + frappe.throw( + _("Invalid naming series {}: dot (.) missing").format(frappe.bold(self.series)), + exc=InvalidNamingSeriesError, + ) + + if not NAMING_SERIES_PATTERN.match(self.series): + frappe.throw( + _( + 'Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series', + ), + exc=InvalidNamingSeriesError, + ) + + def generate_next_name(self, doc: "Document") -> str: + self.validate() + parts = self.series.split(".") + return parse_naming_series(parts, doc) + + def get_prefix(self) -> str: + """Naming series stores prefix to maintain a counter in DB. This prefix can be used to update counter or validations. + + e.g. `SINV-.YY.-.####` has prefix of `SINV-22-` in database for year 2022. + """ + + prefix = None + + def fake_counter_backend(partial_series, digits): + nonlocal prefix + prefix = partial_series + return "#" * digits + + # This function evaluates all parts till we hit numerical parts and then + # sends prefix + digits to DB to find next number. + # Instead of reimplementing the whole parsing logic in multiple places we + # can just ask this function to give us the prefix. + parse_naming_series(self.series, number_generator=fake_counter_backend) + + if prefix is None: + frappe.throw(_("Invalid Naming Series")) + + return prefix + def set_new_name(doc): """ @@ -176,18 +237,8 @@ def make_autoname(key="", doctype="", doc=""): if key == "hash": return frappe.generate_hash(doctype, 10) - if "#" not in key: - key = key + ".#####" - elif "." not in key: - error_message = _("Invalid naming series (. missing)") - if doctype: - error_message = _("Invalid naming series (. missing) for {0}").format(doctype) - - frappe.throw(error_message) - - parts = key.split(".") - n = parse_naming_series(parts, doctype, doc) - return n + series = NamingSeries(key) + return series.generate_next_name(doc) def parse_naming_series( @@ -249,34 +300,6 @@ def parse_naming_series( return name -def get_naming_series_prefix(series: str) -> str: - """Naming series stores prefix to maintain a counter in DB. This prefix can be used to update counter and/or other validations. - - e.g. `SINV-.YY.-.####` has prefix of `SINV-22-` in database for year 2022. - """ - - prefix = None - - if "#" not in series: - series += ".#####" - - def fake_counter_backend(partial_series, digits): - nonlocal prefix - prefix = partial_series - return "#" * digits - - # This function evaluates all parts till we hit numerical parts and then - # sends prefix + digits to DB to find next number. - # Instead of reimplemnted the whole parsing logic in multiple places we can - # just ask this function to give us the prefix. - parse_naming_series(series, number_generator=fake_counter_backend) - - if prefix is None: - frappe.throw(_("Invalid Naming Series")) - - return prefix - - def determine_consecutive_week_number(datetime): """Determines the consecutive calendar week""" m = datetime.month diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index cad91c11a3..a1e023461a 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -4,9 +4,10 @@ import frappe from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.model.naming import ( + InvalidNamingSeriesError, + NamingSeries, append_number_if_name_exists, determine_consecutive_week_number, - get_naming_series_prefix, getseries, revert_series_if_last, ) @@ -300,7 +301,23 @@ class TestNaming(FrappeTestCase): } for series, prefix in prefix_test_cases.items(): - self.assertEqual(prefix, get_naming_series_prefix(series)) + self.assertEqual(prefix, NamingSeries(series).get_prefix()) + + def test_naming_series_validation(self): + dns = frappe.get_doc("Document Naming Settings") + exisiting_series = dns.get_transactions_and_prefixes()["prefixes"] + valid = ["SINV-", "SI-.{field}.", "SI-#.###", ""] + exisiting_series + invalid = ["$INV-", r"WINDOWS\NAMING"] + + for series in valid: + if series.strip(): + try: + NamingSeries(series).validate() + except Exception as e: + self.fail(f"{series} should be valid\n{e}") + + for series in invalid: + self.assertRaises(InvalidNamingSeriesError, NamingSeries(series).validate) def make_invalid_todo(): From fdff13cb290ce1bc454bd703b28ffd4d90270244 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Tue, 31 May 2022 12:51:02 +0530 Subject: [PATCH 42/62] fix: pass currency to razorpay checkout --- frappe/templates/includes/integrations/razorpay_checkout.js | 1 + frappe/templates/pages/integrations/razorpay_checkout.py | 1 + 2 files changed, 2 insertions(+) diff --git a/frappe/templates/includes/integrations/razorpay_checkout.js b/frappe/templates/includes/integrations/razorpay_checkout.js index 2986fcb0fc..3df6ed68ea 100644 --- a/frappe/templates/includes/integrations/razorpay_checkout.js +++ b/frappe/templates/includes/integrations/razorpay_checkout.js @@ -3,6 +3,7 @@ $(document).ready(function(){ var options = { "key": "{{ api_key }}", "amount": cint({{ amount }} * 100), // 2000 paise = INR 20 + "currency": "{{ currency }}", "name": "{{ title }}", "description": "{{ description }}", "subscription_id": "{{ subscription_id }}", diff --git a/frappe/templates/pages/integrations/razorpay_checkout.py b/frappe/templates/pages/integrations/razorpay_checkout.py index aed832119b..942cf6ce5c 100644 --- a/frappe/templates/pages/integrations/razorpay_checkout.py +++ b/frappe/templates/pages/integrations/razorpay_checkout.py @@ -17,6 +17,7 @@ expected_keys = ( "payer_name", "payer_email", "order_id", + "currency" ) From b7a032e7e8f2e5d3785de2355227b6f601e4b6c2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 11:23:44 +0530 Subject: [PATCH 43/62] refactor: generate naming series preview without DB calls --- .../document_naming_settings.py | 11 ++--------- .../test_document_naming_settings.py | 1 - frappe/model/naming.py | 11 +++++++++++ 3 files changed, 13 insertions(+), 10 deletions(-) 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 01a230b833..e340f554f8 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -8,7 +8,7 @@ import frappe from frappe import _ from frappe.core.doctype.doctype.doctype import validate_series from frappe.model.document import Document -from frappe.model.naming import NamingSeries, make_autoname +from frappe.model.naming import NamingSeries, parse_naming_series from frappe.permissions import get_doctypes_with_read from frappe.utils import cint @@ -173,24 +173,17 @@ class DocumentNamingSettings(Document): def preview_series(self) -> str: """Preview what the naming series will generate.""" - generated_names = [] series = self.try_naming_series if not series: return "" - try: doc = self._fetch_last_doc_if_available() - for _count in range(3): - generated_names.append(make_autoname(series, doc=doc)) + return "\n".join(NamingSeries(series).get_preview(doc=doc)) except Exception as e: if frappe.message_log: frappe.message_log.pop() return _("Failed to generate names from the series") + f"\n{str(e)}" - # Explcitly rollback in case any changes were made to series table. - frappe.db.rollback() # nosemgrep - return "\n".join(generated_names) - def _fetch_last_doc_if_available(self): """Fetch last doc for evaluating naming series with fields.""" try: 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 85ecc3b5d9..2d6878e604 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 @@ -3,7 +3,6 @@ import frappe from frappe.core.doctype.document_naming_settings.document_naming_settings import ( - NAMING_SERIES_PATTERN, DocumentNamingSettings, ) from frappe.model.naming import get_default_naming_series diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 50535f0a99..0af8a886cd 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -80,6 +80,17 @@ class NamingSeries: return prefix + def get_preview(self, doc=None) -> List[str]: + """Generate preview of naming series without using DB counters""" + generated_names = [] + for count in range(1, 4): + + def fake_counter(_prefix, digits): + return str(count).zfill(digits) + + generated_names.append(parse_naming_series(self.series, doc=doc, number_generator=fake_counter)) + return generated_names + def set_new_name(doc): """ From 7739da536df26c76d54180eaae5a6d24ea2acce8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 11:49:28 +0530 Subject: [PATCH 44/62] test: add missing tests --- .../document_naming_settings.py | 3 ++- .../test_document_naming_settings.py | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) 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 e340f554f8..ba74b78732 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -144,6 +144,7 @@ class DocumentNamingSettings(Document): if self.prefix: prefix = NamingSeries(self.prefix).get_prefix() self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name") + return self.current_value @frappe.whitelist() def update_series_start(self): @@ -155,7 +156,7 @@ class DocumentNamingSettings(Document): db_prefix = NamingSeries(self.prefix).get_prefix() if frappe.db.get_value("Series", db_prefix, "name", order_by="name") is None: - series.insert(db_prefix, 0).columns(series.name, series.current).run() + frappe.db.sql("insert into `tabSeries` (`name`, `current`) values (%s, 0)", (db_prefix)) ( frappe.qb.update(series) 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 2d6878e604..ce65478339 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 @@ -7,6 +7,7 @@ from frappe.core.doctype.document_naming_settings.document_naming_settings impor ) from frappe.model.naming import get_default_naming_series from frappe.tests.utils import FrappeTestCase +from frappe.utils import cint class TestNamingSeries(FrappeTestCase): @@ -16,6 +17,11 @@ class TestNamingSeries(FrappeTestCase): def tearDown(self): frappe.db.rollback() + def get_valid_serieses(self): + VALID_SERIES = ["SINV-", "SI-.{field}.", "SI-#.###", ""] + exisiting_series = self.dns.get_transactions_and_prefixes()["prefixes"] + return VALID_SERIES + exisiting_series + def test_naming_preview(self): self.dns.transaction_type = "Webhook" @@ -39,3 +45,21 @@ class TestNamingSeries(FrappeTestCase): def test_default_naming_series(self): self.assertIn("HOOK", get_default_naming_series("Webhook")) self.assertIsNone(get_default_naming_series("DocType")) + + def test_updates_naming_options(self): + self.dns.transaction_type = "Webhook" + test_series = "KOOHBEW.###" + self.dns.naming_series_options = self.dns.get_options() + "\n" + test_series + self.dns.update_series() + self.assertIn(test_series, frappe.get_meta("Webhook").get_naming_series_options()) + + def test_update_series_counter(self): + for series in self.get_valid_serieses(): + if not series: + continue + self.dns.prefix = series + current_count = cint(self.dns.get_current()) + new_count = self.dns.current_value = current_count + 1 + self.dns.update_series_start() + + self.assertEqual(self.dns.get_current(), new_count) From b04e4cfeccaae770c71e13999764db34ae08d086 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 12:28:58 +0530 Subject: [PATCH 45/62] feat: update series counter for non-meta naming series Previously only naming serieses that were part of meta could be updated. This change permits updating and prefix that exists in DB and also evalautes templates on the fly before sending to users. --- .../document_naming_settings.py | 38 +++++++++++++++---- .../test_document_naming_settings.py | 4 +- 2 files changed, 32 insertions(+), 10 deletions(-) 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 ba74b78732..5273cb861c 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -1,8 +1,7 @@ # Copyright (c) 2022, Frappe Technologies and contributors # For license information, please see license.txt -import re -from typing import List +from typing import List, Set import frappe from frappe import _ @@ -36,16 +35,21 @@ class DocumentNamingSettings(Document): return sorted(readable_doctypes.intersection(standard + custom)) def _get_prefixes(self, doctypes) -> List[str]: - prefixes = set() + """Get all prefixes for naming series. + + - For all templates prefix is evaluated considering today's date + - All existing prefix in DB are shared as is. + """ + series_templates = set() for d in doctypes: try: options = frappe.get_meta(d).get_naming_series_options() - prefixes.update(options) + series_templates.update(options) except frappe.DoesNotExistError: frappe.msgprint(_("Unable to find DocType {0}").format(d)) continue - custom_prefixes = frappe.get_all( + custom_templates = frappe.get_all( "DocType", fields=["autoname"], filters={ @@ -54,10 +58,26 @@ class DocumentNamingSettings(Document): "module": ("not in", ["Core"]), }, ) - if custom_prefixes: - prefixes.update([d.autoname.rsplit(".", 1)[0] for d in custom_prefixes]) + if custom_templates: + series_templates.update([d.autoname.rsplit(".", 1)[0] for d in custom_templates]) - return sorted(prefixes) + return self._evaluate_and_clean_templates(series_templates) + + def _evaluate_and_clean_templates(self, series_templates: Set[str]) -> List[str]: + evalauted_prefix = set() + + series = frappe.qb.DocType("Series") + prefixes_from_db = frappe.qb.from_(series).select(series.name).run(pluck=True) + evalauted_prefix.update(prefixes_from_db) + + for series_template in series_templates: + prefix = NamingSeries(series_template).get_prefix() + if "{" in prefix: + # fieldnames can't be evalauted, rely on data in DB instead + continue + evalauted_prefix.add(prefix) + + return sorted(evalauted_prefix) def get_options_list(self, options: str) -> List[str]: return [op.strip() for op in options.split("\n") if op.strip()] @@ -148,6 +168,8 @@ class DocumentNamingSettings(Document): @frappe.whitelist() def update_series_start(self): + frappe.only_for("System Manager") + if not self.prefix: frappe.throw(_("Please select prefix first")) 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 ce65478339..917ed976b7 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 @@ -5,7 +5,7 @@ import frappe from frappe.core.doctype.document_naming_settings.document_naming_settings import ( DocumentNamingSettings, ) -from frappe.model.naming import get_default_naming_series +from frappe.model.naming import NamingSeries, get_default_naming_series from frappe.tests.utils import FrappeTestCase from frappe.utils import cint @@ -40,7 +40,7 @@ class TestNamingSeries(FrappeTestCase): existing_naming_series = frappe.get_meta("Webhook").get_field("naming_series").options for series in existing_naming_series.split("\n"): - self.assertIn(series, naming_info["prefixes"]) + self.assertIn(NamingSeries(series).get_prefix(), naming_info["prefixes"]) def test_default_naming_series(self): self.assertIn("HOOK", get_default_naming_series("Webhook")) From 6e47db7c1d37e2ac91a5cc03445d8f241bacd5c7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 12:48:55 +0530 Subject: [PATCH 46/62] feat: keep version log of counter changes --- .../document_naming_settings.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 5273cb861c..533f43bfa4 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -180,18 +180,31 @@ class DocumentNamingSettings(Document): if frappe.db.get_value("Series", db_prefix, "name", order_by="name") is None: frappe.db.sql("insert into `tabSeries` (`name`, `current`) values (%s, 0)", (db_prefix)) + previous_value = frappe.db.get_value("Series", db_prefix, "current", order_by="name") + ( frappe.qb.update(series) .set(series.current, cint(self.current_value)) .where(series.name == db_prefix) ).run() + self.create_version_log_for_change(db_prefix, previous_value, cint(self.current_value)) + frappe.msgprint( _("Series counter for {} updated to {} successfully").format(self.prefix, self.current_value), alert=True, indicator="green", ) + def create_version_log_for_change(self, series, old, new): + version = frappe.new_doc("Version") + version.ref_doctype = "Series" + version.docname = series + version.data = frappe.as_json({"changed": [["current", old, new]]}) + version.flags.ignore_links = True # series is not a "real" doctype + version.flags.ignore_permissions = True + version.insert() + @frappe.whitelist() def preview_series(self) -> str: """Preview what the naming series will generate.""" From 1826c9e9afd6311dc92e830a700a52f9fb487540 Mon Sep 17 00:00:00 2001 From: HENRY Florian Date: Tue, 31 May 2022 10:45:21 +0200 Subject: [PATCH 47/62] fix: translation context for print format align values (#17026) --- .../page/print_format_builder/print_format_builder.js | 5 +++-- .../public/js/print_format_builder/PrintFormatControls.vue | 4 ++-- frappe/translations/fr.csv | 3 +++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/frappe/printing/page/print_format_builder/print_format_builder.js b/frappe/printing/page/print_format_builder/print_format_builder.js index 313e8da539..d12a7cc96c 100644 --- a/frappe/printing/page/print_format_builder/print_format_builder.js +++ b/frappe/printing/page/print_format_builder/print_format_builder.js @@ -441,7 +441,7 @@ frappe.PrintFormatBuilder = class PrintFormatBuilder { const field = $(e.currentTarget).parent(); // new dialog var d = new frappe.ui.Dialog({ - title: "Set Properties", + title: __("Set Properties"), fields: [ { label: __("Label"), @@ -452,7 +452,8 @@ frappe.PrintFormatBuilder = class PrintFormatBuilder { label: __("Align Value"), fieldname: "align", fieldtype: "Select", - options: [{'label': __('Left'), 'value': 'left'}, {'label': __('Right'), 'value': 'right'}] + options: [{'label': __('Left', null, 'alignment'), 'value': 'left'}, + {'label': __('Right', null, 'alignment'), 'value': 'right'}] }, { label: __("Remove Field"), diff --git a/frappe/public/js/print_format_builder/PrintFormatControls.vue b/frappe/public/js/print_format_builder/PrintFormatControls.vue index 2eefc22409..7a4e9c81e7 100644 --- a/frappe/public/js/print_format_builder/PrintFormatControls.vue +++ b/frappe/public/js/print_format_builder/PrintFormatControls.vue @@ -181,8 +181,8 @@ export default { return [ { label: __("Top"), fieldname: "margin_top" }, { label: __("Bottom"), fieldname: "margin_bottom" }, - { label: __("Left"), fieldname: "margin_left" }, - { label: __("Right"), fieldname: "margin_right" } + { label: __("Left", null, 'alignment'), fieldname: "margin_left" }, + { label: __("Right", null, 'alignment'), fieldname: "margin_right" } ]; }, fields() { diff --git a/frappe/translations/fr.csv b/frappe/translations/fr.csv index d1dba64a44..f17585fe88 100644 --- a/frappe/translations/fr.csv +++ b/frappe/translations/fr.csv @@ -4717,3 +4717,6 @@ Document has been cancelled,Document annulé Document is in draft state,Document au statut brouillon Copy to Clipboard,Copier vers le presse-papiers Don't have an account?,Vous n'avez pas de compte? +Left:alignment,Gauche +Right:alignment,Droite +Set Properties,Gérer les proriétés From 7fa451333a115bb9516c1b244381dbda4b848f7b Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Tue, 31 May 2022 14:16:16 +0530 Subject: [PATCH 48/62] fix: formatting --- frappe/templates/pages/integrations/razorpay_checkout.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/templates/pages/integrations/razorpay_checkout.py b/frappe/templates/pages/integrations/razorpay_checkout.py index 942cf6ce5c..b4f9e74a03 100644 --- a/frappe/templates/pages/integrations/razorpay_checkout.py +++ b/frappe/templates/pages/integrations/razorpay_checkout.py @@ -17,7 +17,7 @@ expected_keys = ( "payer_name", "payer_email", "order_id", - "currency" + "currency", ) From eaef59e361d221bc473de7b8e7ad4c838a4f0a2f Mon Sep 17 00:00:00 2001 From: Tom-Finke Date: Tue, 31 May 2022 10:51:51 +0200 Subject: [PATCH 49/62] Fix: Delete Share in Frontend when read is de-selected (#17020) --- frappe/share.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/share.py b/frappe/share.py index 01d1412b8d..3edcb1be38 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -93,7 +93,7 @@ def set_permission(doctype, name, user, permission_to, value=1, everyone=0): if not (share.read or share.write or share.submit or share.share): share.delete() - share = {} + share = None return share From 45604a7f3f170405b1bd26490ee36e2455deb0f4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 14:39:27 +0530 Subject: [PATCH 50/62] refactor: move all naming series related logic --- .../document_naming_settings.py | 26 ++++++------------- .../test_document_naming_settings.py | 2 +- frappe/model/naming.py | 17 ++++++++++++ 3 files changed, 26 insertions(+), 19 deletions(-) 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 533f43bfa4..46def88b65 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.py +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.py @@ -7,7 +7,7 @@ import frappe from frappe import _ from frappe.core.doctype.doctype.doctype import validate_series from frappe.model.document import Document -from frappe.model.naming import NamingSeries, parse_naming_series +from frappe.model.naming import NamingSeries from frappe.permissions import get_doctypes_with_read from frappe.utils import cint @@ -162,8 +162,7 @@ class DocumentNamingSettings(Document): def get_current(self): """get series current""" if self.prefix: - prefix = NamingSeries(self.prefix).get_prefix() - self.current_value = frappe.db.get_value("Series", prefix, "current", order_by="name") + self.current_value = NamingSeries(self.prefix).get_current_value() return self.current_value @frappe.whitelist() @@ -173,22 +172,13 @@ class DocumentNamingSettings(Document): if not self.prefix: frappe.throw(_("Please select prefix first")) - series = frappe.qb.DocType("Series") + naming_series = NamingSeries(self.prefix) + previous_value = naming_series.get_current_value() + naming_series.update_counter(self.current_value) - db_prefix = NamingSeries(self.prefix).get_prefix() - - if frappe.db.get_value("Series", db_prefix, "name", order_by="name") is None: - frappe.db.sql("insert into `tabSeries` (`name`, `current`) values (%s, 0)", (db_prefix)) - - previous_value = frappe.db.get_value("Series", db_prefix, "current", order_by="name") - - ( - frappe.qb.update(series) - .set(series.current, cint(self.current_value)) - .where(series.name == db_prefix) - ).run() - - self.create_version_log_for_change(db_prefix, previous_value, cint(self.current_value)) + self.create_version_log_for_change( + naming_series.get_prefix(), previous_value, self.current_value + ) frappe.msgprint( _("Series counter for {} updated to {} successfully").format(self.prefix, self.current_value), 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 917ed976b7..98ce9e738b 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 @@ -62,4 +62,4 @@ class TestNamingSeries(FrappeTestCase): new_count = self.dns.current_value = current_count + 1 self.dns.update_series_start() - self.assertEqual(self.dns.get_current(), new_count) + self.assertEqual(self.dns.get_current(), new_count, f"Incorrect update for {series}") diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 0af8a886cd..2029fa084c 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -91,6 +91,23 @@ class NamingSeries: generated_names.append(parse_naming_series(self.series, doc=doc, number_generator=fake_counter)) return generated_names + def update_counter(self, new_count: int) -> None: + """Warning: Incorrectly updating series can result in unusable transactions""" + Series = frappe.qb.DocType("Series") + prefix = self.get_prefix() + + # Initialize if not present in DB + if frappe.db.get_value("Series", prefix, "name", order_by="name") is None: + frappe.qb.into(Series).insert(prefix, 0).columns("name", "current").run() + + ( + frappe.qb.update(Series).set(Series.current, cint(new_count)).where(Series.name == prefix) + ).run() + + def get_current_value(self) -> int: + prefix = self.get_prefix() + return cint(frappe.db.get_value("Series", prefix, "current", order_by="name")) + def set_new_name(doc): """ From 890f5dd5975b28db8b225e75a6da24b58ad22555 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 14:49:12 +0530 Subject: [PATCH 51/62] fix: misc client side errors (#17018) * fix: group by button color consistency with filter * fix: return promise from HTML.set_value * fix: only call set_input if control supports input --- frappe/public/js/frappe/form/controls/html.js | 1 + frappe/public/js/frappe/ui/field_group.js | 2 +- frappe/public/scss/desk/report.scss | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/html.js b/frappe/public/js/frappe/form/controls/html.js index b2f18d4ccc..4cc0e4ab50 100644 --- a/frappe/public/js/frappe/form/controls/html.js +++ b/frappe/public/js/frappe/form/controls/html.js @@ -28,5 +28,6 @@ frappe.ui.form.ControlHTML = class ControlHTML extends frappe.ui.form.Control { this.df.options = html; this.html(html); } + return Promise.resolve(); } }; diff --git a/frappe/public/js/frappe/ui/field_group.js b/frappe/public/js/frappe/ui/field_group.js index cba702407a..c226c4cbfb 100644 --- a/frappe/public/js/frappe/ui/field_group.js +++ b/frappe/public/js/frappe/ui/field_group.js @@ -132,7 +132,7 @@ frappe.ui.FieldGroup = class FieldGroup extends frappe.ui.form.Layout { var f = this.fields_dict[key]; if (f) { f.set_value(val).then(() => { - f.set_input(val); + f.set_input?.(val); this.refresh_dependency(); resolve(); }); diff --git a/frappe/public/scss/desk/report.scss b/frappe/public/scss/desk/report.scss index f8666602ff..8ed0fb740c 100644 --- a/frappe/public/scss/desk/report.scss +++ b/frappe/public/scss/desk/report.scss @@ -104,12 +104,12 @@ } .group-by-button.btn-primary-light { - color: var(--blue-500); + color: var(--text-on-blue); } .group-by-icon.active { use { - stroke: var(--blue-500); + stroke: var(--text-on-blue); } } From ef3565383229272fd719e69a3141004d93ad683f Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 31 May 2022 18:32:06 +0530 Subject: [PATCH 52/62] chore: Enable semantic release for version-14-beta branch --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9c7ecf989e..434f784461 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -2,7 +2,7 @@ name: Generate Semantic Release on: push: branches: - - version-13 + - [version-13, version-14-beta] jobs: release: name: Release From 462aa2038692e62063c965680b0d3301c9e130d1 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 19:55:34 +0530 Subject: [PATCH 53/62] fix: naming using doc fields (#17040) --- frappe/model/naming.py | 2 +- frappe/tests/test_naming.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 2029fa084c..d63466e556 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -54,7 +54,7 @@ class NamingSeries: def generate_next_name(self, doc: "Document") -> str: self.validate() parts = self.series.split(".") - return parse_naming_series(parts, doc) + return parse_naming_series(parts, doc=doc) def get_prefix(self) -> str: """Naming series stores prefix to maintain a counter in DB. This prefix can be used to update counter or validations. diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index a1e023461a..d966fd5ce8 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -319,6 +319,15 @@ class TestNaming(FrappeTestCase): for series in invalid: self.assertRaises(InvalidNamingSeriesError, NamingSeries(series).validate) + def test_naming_using_fields(self): + + webhook = frappe.new_doc("Webhook") + webhook.webhook_docevent = "on_update" + name = NamingSeries("KOOH-.{webhook_docevent}.").generate_next_name(webhook) + self.assertTrue( + name.startswith("KOOH-on_update"), f"incorrect name generated {name}, missing field value" + ) + def make_invalid_todo(): frappe.get_doc({"doctype": "ToDo", "description": "Test"}).insert(set_name="ToDo") From f8f29e284fd885b7ff23a050a24894b5a17bd711 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 31 May 2022 22:40:07 +0530 Subject: [PATCH 54/62] perf: store compact JSON in version diffs (#17042) --- frappe/__init__.py | 9 ++++++--- frappe/core/doctype/version/version.py | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index ecd79fc26f..11acdc55f4 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1818,18 +1818,21 @@ def get_value(*args, **kwargs): return db.get_value(*args, **kwargs) -def as_json(obj: Union[Dict, List], indent=1) -> str: +def as_json(obj: Union[Dict, List], indent=1, separators=None) -> str: from frappe.utils.response import json_handler + if separators is None: + separators = (",", ": ") + try: return json.dumps( - obj, indent=indent, sort_keys=True, default=json_handler, separators=(",", ": ") + obj, indent=indent, sort_keys=True, default=json_handler, separators=separators ) except TypeError: # this would break in case the keys are not all os "str" type - as defined in the JSON # adding this to ensure keys are sorted (expected behaviour) sorted_obj = dict(sorted(obj.items(), key=lambda kv: str(kv[0]))) - return json.dumps(sorted_obj, indent=indent, default=json_handler, separators=(",", ": ")) + return json.dumps(sorted_obj, indent=indent, default=json_handler, separators=separators) def are_emails_muted(): diff --git a/frappe/core/doctype/version/version.py b/frappe/core/doctype/version/version.py index 540f8c7a02..863885e85c 100644 --- a/frappe/core/doctype/version/version.py +++ b/frappe/core/doctype/version/version.py @@ -15,7 +15,7 @@ class Version(Document): if diff: self.ref_doctype = new.doctype self.docname = new.name - self.data = frappe.as_json(diff) + self.data = frappe.as_json(diff, indent=None, separators=(",", ":")) return True else: return False From 1431cb26d3f753fddf1acace46d49214d5853e0c Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 1 Jun 2022 08:08:52 +0530 Subject: [PATCH 55/62] fix: Catch any expection and show proper error page - Also, show title & message if it is defined in the exception class --- frappe/templates/web.html | 2 +- frappe/website/page_renderers/error_page.py | 8 +++++++- frappe/website/page_renderers/template_page.py | 18 ++++++------------ frappe/www/error.html | 6 +++--- frappe/www/error.py | 6 ++++-- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/frappe/templates/web.html b/frappe/templates/web.html index 48fbce7cd1..df5ee9aeef 100644 --- a/frappe/templates/web.html +++ b/frappe/templates/web.html @@ -26,7 +26,7 @@ {% endif %} -
+
{%- block page_content -%}{%- endblock -%}
diff --git a/frappe/website/page_renderers/error_page.py b/frappe/website/page_renderers/error_page.py index 613809bfdc..6a3925967c 100644 --- a/frappe/website/page_renderers/error_page.py +++ b/frappe/website/page_renderers/error_page.py @@ -5,7 +5,13 @@ class ErrorPage(TemplatePage): def __init__(self, path=None, http_status_code=None, exception=None): path = "error" super().__init__(path=path, http_status_code=http_status_code) - self.http_status_code = getattr(exception, "http_status_code", None) or http_status_code or 500 + self.exception = exception def can_render(self): return True + + def init_context(self): + super().init_context() + self.context.http_status_code = getattr(self.exception, "http_status_code", None) or 500 + self.context.error_title = getattr(self.exception, "title", None) + self.context.error_message = getattr(self.exception, "message", None) diff --git a/frappe/website/page_renderers/template_page.py b/frappe/website/page_renderers/template_page.py index 2ed8a62119..83f68d3716 100644 --- a/frappe/website/page_renderers/template_page.py +++ b/frappe/website/page_renderers/template_page.py @@ -212,19 +212,13 @@ class TemplatePage(BaseTemplatePage): def run_pymodule_method(self, method_name): if hasattr(self.pymodule, method_name): - try: - import inspect + import inspect - method = getattr(self.pymodule, method_name) - if inspect.getfullargspec(method).args: - return method(self.context) - else: - return method() - except (frappe.PermissionError, frappe.DoesNotExistError, frappe.Redirect): - raise - except Exception: - if not frappe.flags.in_migrate: - frappe.errprint(frappe.utils.get_traceback()) + method = getattr(self.pymodule, method_name) + if inspect.getfullargspec(method).args: + return method(self.context) + else: + return method() def render_template(self): if self.template_path.endswith("min.js"): diff --git a/frappe/www/error.html b/frappe/www/error.html index d63daec759..142897c35a 100644 --- a/frappe/www/error.html +++ b/frappe/www/error.html @@ -23,15 +23,15 @@
- {{_("Uncaught Server Exception")}} + {{ error_title }}
-

{{_("There was an error building this page")}}

+

{{ error_message }}

- {{ _("Error Code: {0}").format('500') }} + {{ _("Error Code: {0}").format(http_status_code) }}

-
+
{%- block page_content -%}{%- endblock -%}