From 1154753a6fc8f269c9b0442ebfaa33dec2438866 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 20 Jan 2021 13:52:05 +0530 Subject: [PATCH 01/12] fix: Don't execute dynamic properties to check if conflicts exist (cherry picked from commit 9aa2f366dd6f51adde6fbd9be4fbc2b5c7a85ab9) --- frappe/core/doctype/doctype/doctype.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 3588cc553a..6f33221155 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1176,9 +1176,15 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): def check_if_fieldname_conflicts_with_methods(doctype, fieldname): doc = frappe.get_doc({"doctype": doctype}) - method_list = [method for method in dir(doc) if isinstance(method, str) and callable(getattr(doc, method))] - - if fieldname in method_list: + available_objects = [x for x in dir(doc) if isinstance(x, str)] + property_list = [ + x for x in available_objects if isinstance(getattr(type(doc), x, None), property) + ] + method_list = [ + x for x in available_objects if x not in property_list and callable(getattr(doc, x)) + ] + + if fieldname in method_list + property_list: frappe.throw(_("Fieldname {0} conflicting with meta object").format(fieldname)) def clear_linked_doctype_cache(): From f7f400985b8ca84e19a7d505b9a0341282faf6c8 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 4 Jan 2021 10:41:18 +0530 Subject: [PATCH 02/12] chore: Rename function to validate conflicting methods and properties (cherry picked from commit 255a959a3eb706ab25845aa94890708c37fe7573) --- frappe/core/doctype/doctype/doctype.py | 2 +- frappe/custom/doctype/custom_field/custom_field.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 6f33221155..d277a217d1 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1174,7 +1174,7 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): else: raise -def check_if_fieldname_conflicts_with_methods(doctype, fieldname): +def check_if_fieldname_conflicts_with_properties_and_methods(doctype, fieldname): doc = frappe.get_doc({"doctype": doctype}) available_objects = [x for x in dir(doc) if isinstance(x, str)] property_list = [ diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index fb49aa5da0..4bf492d1a1 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -64,8 +64,8 @@ class CustomField(Document): self.translatable = 0 if not self.flags.ignore_validate: - from frappe.core.doctype.doctype.doctype import check_if_fieldname_conflicts_with_methods - check_if_fieldname_conflicts_with_methods(self.dt, self.fieldname) + from frappe.core.doctype.doctype.doctype import check_if_fieldname_conflicts_with_properties_and_methods + check_if_fieldname_conflicts_with_properties_and_methods(self.dt, self.fieldname) def on_update(self): if not frappe.flags.in_setup_wizard: From 0b06a67aeec7fe2fbe78a7b8b96741fe7fb253e8 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 6 May 2021 11:58:17 +0530 Subject: [PATCH 03/12] feat: Validate field name conflicts in DocType.validate # Conflicts: # frappe/core/doctype/doctype/doctype.py (cherry picked from commit c652c7b7f52ad8d9a5b00b84af3f1ab33c3d1769) --- frappe/core/doctype/doctype/doctype.py | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index d277a217d1..13ebcd82ac 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -70,6 +70,7 @@ class DocType(Document): validate_series(self) self.validate_document_type() validate_fields(self) + self.validate_field_name_conflicts() if not self.istable: validate_permissions(self) @@ -89,6 +90,39 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_('Standard DocType cannot have default print format, use Customize Form')) + if frappe.conf.get('developer_mode'): + self.owner = 'Administrator' + self.modified_by = 'Administrator' + + def validate_field_name_conflicts(self): + """Check if field names dont conflict with controller properties and methods""" + from frappe.model.base_document import get_controller + + controller = get_controller(self.name) + available_objects = {x for x in dir(controller) if isinstance(x, str)} + property_set = { + x for x in available_objects if isinstance(getattr(controller, x, None), property) + } + method_set = { + x for x in available_objects if x not in property_set and callable(getattr(controller, x, None)) + } + + for docfield in self.get("fields") or []: + conflict_type = "" + field = docfield.fieldname + field_label = docfield.label or docfield.fieldname + + if docfield.fieldname in method_set: + conflict_type = "controller method" + if docfield.fieldname in property_set: + conflict_type = "class property" + + if conflict_type: + frappe.throw( + _("Fieldname '{0}' conflicting with a {1} of the name {2} in {3}") + .format(field_label, conflict_type, field, self.name) + ) + def after_insert(self): # clear user cache so that on the next reload this doctype is included in boot clear_user_cache(frappe.session.user) From b200b6dbb61d0c25898c979ef611264e25022a8e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 22 Jan 2021 18:08:18 +0530 Subject: [PATCH 04/12] chore: Drop dead file (cherry picked from commit 78e1297392706eaa209e53d3acb42faae66dfa65) --- frappe/email/doctype/newsletter/newsletter..json | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 frappe/email/doctype/newsletter/newsletter..json diff --git a/frappe/email/doctype/newsletter/newsletter..json b/frappe/email/doctype/newsletter/newsletter..json deleted file mode 100644 index e69de29bb2..0000000000 From 1e55ab34c3107ed2b8e5e8c4ee6547b1d08d3def Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 25 Jan 2021 13:55:10 +0530 Subject: [PATCH 05/12] fix: Ignore 'special' DocTypes to avoid breaking changes eg: Fieldname 'Delete' conflicting with a controller method of the name delete in Custom DocPerm Fieldname 'Delete' conflicting with a controller method of the name delete in DocPerm Fieldname 'Precision' conflicting with a controller method of the name precision in Custom Field Fieldname 'Precision' conflicting with a controller method of the name precision in Customize Form Field Fieldname 'Precision' conflicting with a controller method of the name precision in DocField (cherry picked from commit ce4253c6318546a7249f0edd518ad1f499f8fd88) --- frappe/core/doctype/doctype/doctype.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 13ebcd82ac..c1a8527512 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -96,6 +96,17 @@ class DocType(Document): def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" + core_doctypes = [ + "Custom DocPerm", + "DocPerm", + "Custom Field", + "Customize Form Field", + "DocField", + ] + + if self.name in core_doctypes: + return + from frappe.model.base_document import get_controller controller = get_controller(self.name) From 804e4e212dd30d3b5c63448bdbc27e70c22fb31d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 18 Jan 2020 04:11:19 +0530 Subject: [PATCH 06/12] fix: Use Document in case get_controller raises import errors (cherry picked from commit a3b79081d64e754d73fad46f26aa24ac42a32a2e) --- frappe/core/doctype/doctype/doctype.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index c1a8527512..d361bb8a98 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -109,7 +109,11 @@ class DocType(Document): from frappe.model.base_document import get_controller - controller = get_controller(self.name) + try: + controller = get_controller(self.name) + except ImportError: + controller = Document + available_objects = {x for x in dir(controller) if isinstance(x, str)} property_set = { x for x in available_objects if isinstance(getattr(controller, x, None), property) From 8d8a6cbd5d993847cd336cd45bbef06031bab289 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 18 Feb 2021 19:55:12 +0530 Subject: [PATCH 07/12] fix: Check for db value if cache doesn't exist in get_controller, if cached value doesn't exist for a DocType in the frappe.db.value_cache, then query the db as fallback before the final fallback of assuming module as Core and non custom (cherry picked from commit 05712abc60721f4633d6b0166648743154b56cca) --- frappe/model/base_document.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 983511f7a4..fbde35af86 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -34,8 +34,11 @@ def get_controller(doctype): from frappe.model.document import Document from frappe.utils.nestedset import NestedSet - module_name, custom = frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) \ + module_name, custom = ( + frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) + or frappe.db.get_value("DocType", doctype, ("module", "custom")) or ["Core", False] + ) if custom: if frappe.db.field_exists("DocType", "is_tree"): From 56606ebc44cb7f447e19c7ed5cd3bba51285dfbf Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 16 Mar 2021 12:23:10 +0530 Subject: [PATCH 08/12] refactor: Rename function and add docstring Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> (cherry picked from commit 843c544117e7f661cc97cc78c33349833b12df21) --- frappe/core/doctype/doctype/doctype.py | 4 +++- frappe/custom/doctype/custom_field/custom_field.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index d361bb8a98..2cdccda8e1 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1223,7 +1223,9 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): else: raise -def check_if_fieldname_conflicts_with_properties_and_methods(doctype, fieldname): +def check_fieldname_conflicts(doctype, fieldname): + """Checks if fieldname conflicts with methods or properties""" + doc = frappe.get_doc({"doctype": doctype}) available_objects = [x for x in dir(doc) if isinstance(x, str)] property_list = [ diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index 4bf492d1a1..39aff8b4a7 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -64,8 +64,8 @@ class CustomField(Document): self.translatable = 0 if not self.flags.ignore_validate: - from frappe.core.doctype.doctype.doctype import check_if_fieldname_conflicts_with_properties_and_methods - check_if_fieldname_conflicts_with_properties_and_methods(self.dt, self.fieldname) + from frappe.core.doctype.doctype.doctype import check_fieldname_conflicts + check_fieldname_conflicts(self.dt, self.fieldname) def on_update(self): if not frappe.flags.in_setup_wizard: From b343db84a07c52b7b358be10d24e833f351106d2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 16 Mar 2021 16:28:38 +0530 Subject: [PATCH 09/12] fix: Use fallback values if doctype values unset (cherry picked from commit 877f9d08df70a741aa493421a002651ef0098cbc) --- frappe/model/base_document.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index fbde35af86..445b92773f 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -34,11 +34,15 @@ def get_controller(doctype): from frappe.model.document import Document from frappe.utils.nestedset import NestedSet - module_name, custom = ( - frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) - or frappe.db.get_value("DocType", doctype, ("module", "custom")) - or ["Core", False] - ) + if frappe.flags.in_migrate or frappe.flags.in_install or frappe.flags.in_patch: + module_name, custom = ["Core", False] + else: + # this could be simplified in PY3.8 using walrus operators + result = frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) + if result: + module_name, custom = result + else: + module_name, custom = ["Core", bool(not frappe.db.exists(doctype))] if custom: if frappe.db.field_exists("DocType", "is_tree"): From 34372e278ebc0235cecef23bae463ca685315e42 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 23 Apr 2021 12:43:13 +0530 Subject: [PATCH 10/12] fix: Use older logic to set module_name and custom vars (cherry picked from commit 87ed7796ded4053d8c1f04924716b29bc4848a6d) --- frappe/model/base_document.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 445b92773f..b63ce9dd44 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -34,15 +34,9 @@ def get_controller(doctype): from frappe.model.document import Document from frappe.utils.nestedset import NestedSet - if frappe.flags.in_migrate or frappe.flags.in_install or frappe.flags.in_patch: - module_name, custom = ["Core", False] - else: - # this could be simplified in PY3.8 using walrus operators - result = frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) - if result: - module_name, custom = result - else: - module_name, custom = ["Core", bool(not frappe.db.exists(doctype))] + module_name, custom = frappe.db.get_value( + "DocType", doctype, ("module", "custom"), cache=True + ) or ["Core", False] if custom: if frappe.db.field_exists("DocType", "is_tree"): From 1eb84ad5e515eb52cab0d9b1769fd26b6cf4e45d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 18:02:25 +0530 Subject: [PATCH 11/12] fix: Skip field-method conflicts validation on new docs (cherry picked from commit 926d13e69ef88309f409446d2dc1c2e3f65ab0f3) --- frappe/core/doctype/doctype/doctype.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 2cdccda8e1..e0b9d15114 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -70,7 +70,6 @@ class DocType(Document): validate_series(self) self.validate_document_type() validate_fields(self) - self.validate_field_name_conflicts() if not self.istable: validate_permissions(self) @@ -84,6 +83,7 @@ class DocType(Document): if not self.is_new(): self.before_update = frappe.get_doc('DocType', self.name) self.setup_fields_to_fetch() + self.validate_field_name_conflicts() check_email_append_to(self) From 86d2989c52ddefb2cb970bbf718c3f892cae2225 Mon Sep 17 00:00:00 2001 From: gavin Date: Sat, 8 May 2021 12:39:27 +0530 Subject: [PATCH 12/12] fix: Avoid possible whitespace bug * Handles semgrep warning * Changed "" to None as a precaution against future whitespace bugs via human error Co-authored-by: Ankush Menat (cherry picked from commit c84feb16e9e456cdd040f20a6ac154d3a1d760c0) --- frappe/core/doctype/doctype/doctype.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index e0b9d15114..6eef5a4023 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -123,7 +123,7 @@ class DocType(Document): } for docfield in self.get("fields") or []: - conflict_type = "" + conflict_type = None field = docfield.fieldname field_label = docfield.label or docfield.fieldname