diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 06ebcc7d42..6bd7f2306f 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1018,12 +1018,13 @@ def validate_fields(meta): validate_column_name(fieldname) def check_invalid_fieldnames(docname, fieldname): - invalid_fields = ("doctype",) - if fieldname in invalid_fields: + if fieldname in Document._reserved_keywords: frappe.throw( - _("{0}: Fieldname cannot be one of {1}").format( - docname, ", ".join(frappe.bold(d) for d in invalid_fields) - ) + _("{0}: fieldname cannot be set to reserved keyword {1}").format( + frappe.bold(docname), + frappe.bold(fieldname), + ), + title=_("Invalid Fieldname"), ) def check_unique_fieldname(docname, fieldname): diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index f5edd3fcc6..dc26dfe5b0 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -55,12 +55,6 @@ class FormMeta(Meta): super(FormMeta, self).__init__(doctype) self.load_assets() - def set(self, key, value, *args, **kwargs): - if key in ASSET_KEYS: - self.__dict__[key] = value - else: - super(FormMeta, self).set(key, value, *args, **kwargs) - def load_assets(self): if self.get("__assets_loaded", False): return diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index a272dedd02..22f8cc15df 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -5,7 +5,7 @@ import json from typing import Dict, List import frappe -from frappe import _ +from frappe import _, _dict from frappe.model import ( child_table_fields, datetime_fields, @@ -23,7 +23,16 @@ from frappe.utils.html_utils import unescape_html max_positive_value = {"smallint": 2**15, "int": 2**31, "bigint": 2**63} -DOCTYPES_FOR_DOCTYPE = ("DocType", "DocField", "DocPerm", "DocType Action", "DocType Link") +DOCTYPE_TABLE_FIELDS = [ + _dict(fieldname="fields", options="DocField"), + _dict(fieldname="permissions", options="DocPerm"), + _dict(fieldname="actions", options="DocType Action"), + _dict(fieldname="links", options="DocType Link"), + _dict(fieldname="states", options="DocType State"), +] + +TABLE_DOCTYPES_FOR_DOCTYPE = {df["fieldname"]: df["options"] for df in DOCTYPE_TABLE_FIELDS} +DOCTYPES_FOR_DOCTYPE = {"DocType", *TABLE_DOCTYPES_FOR_DOCTYPE.values()} def get_controller(doctype): @@ -78,12 +87,28 @@ def get_controller(doctype): class BaseDocument(object): - ignore_in_setter = ("doctype", "_meta", "meta", "_table_fields", "_valid_columns") + _reserved_keywords = { + "doctype", + "meta", + "_meta", + "flags", + "_table_fields", + "_valid_columns", + "_table_fieldnames", + "_reserved_keywords", + "dont_update_if_missing", + } def __init__(self, d): if d.get("doctype"): self.doctype = d["doctype"] + self._table_fieldnames = ( + d["_table_fieldnames"] # from cache + if "_table_fieldnames" in d + else {df.fieldname for df in self._get_table_fields()} + ) + self.update(d) self.dont_update_if_missing = [] @@ -92,10 +117,10 @@ class BaseDocument(object): @property def meta(self): - if not getattr(self, "_meta", None): - self._meta = frappe.get_meta(self.doctype) + if not (meta := getattr(self, "_meta", None)): + self._meta = meta = frappe.get_meta(self.doctype) - return self._meta + return meta def __getstate__(self): self._meta = None @@ -144,17 +169,12 @@ class BaseDocument(object): if filters: if isinstance(filters, dict): - value = _filter(self.__dict__.get(key, []), filters, limit=limit) - else: - default = filters - filters = None - value = self.__dict__.get(key, default) - else: - value = self.__dict__.get(key, default) + return _filter(self.__dict__.get(key, []), filters, limit=limit) - if value is None and key in (d.fieldname for d in self.meta.get_table_fields()): - value = [] - self.set(key, value) + # perhaps you wanted to set a default instead + default = filters + + value = self.__dict__.get(key, default) if limit and isinstance(value, (list, tuple)) and len(value) > limit: value = value[:limit] @@ -165,14 +185,19 @@ class BaseDocument(object): return self.get(key, filters=filters, limit=1)[0] def set(self, key, value, as_value=False): - if key in self.ignore_in_setter: + if key in self._reserved_keywords: return - if isinstance(value, list) and not as_value: + if not as_value and key in self._table_fieldnames: self.__dict__[key] = [] - self.extend(key, value) - else: - self.__dict__[key] = value + + # if value is falsy, just init to an empty list + if value: + self.extend(key, value) + + return + + self.__dict__[key] = value def delete_key(self, key): if key in self.__dict__: @@ -190,41 +215,27 @@ class BaseDocument(object): """ if value is None: value = {} - if isinstance(value, (dict, BaseDocument)): - if not self.__dict__.get(key): - self.__dict__[key] = [] - value = self._init_child(value, key) - self.__dict__[key].append(value) + if (table := self.__dict__.get(key)) is None: + self.__dict__[key] = table = [] - # reference parent document - value.parent_doc = self + value = self._init_child(value, key) + table.append(value) - return value - else: + # reference parent document + value.parent_doc = self - # metaclasses may have arbitrary lists - # which we can ignore - if getattr(self, "_metaclass", None) or self.__class__.__name__ in ( - "Meta", - "FormMeta", - "DocField", - ): - return value - - raise ValueError( - 'Document for field "{0}" attached to child table of "{1}" must be a dict or BaseDocument, not {2} ({3})'.format( - key, self.name, str(type(value))[1:-1], value - ) - ) + return value def extend(self, key, value): - if isinstance(value, list): - for v in value: - self.append(key, v) - else: + try: + value = iter(value) + except TypeError: raise ValueError + for v in value: + self.append(key, v) + def remove(self, doc): # Usage: from the parent doc, pass the child table doc # to remove that child doc from the child table, thus removing it from the parent doc @@ -232,15 +243,12 @@ class BaseDocument(object): self.get(doc.parentfield).remove(doc) def _init_child(self, value, key): - if not self.doctype: - return value - if not isinstance(value, BaseDocument): - value["doctype"] = self.get_table_field_doctype(key) - if not value["doctype"]: + if not (doctype := self.get_table_field_doctype(key)): raise AttributeError(key) - value = get_controller(value["doctype"])(value) + value["doctype"] = doctype + value = get_controller(doctype)(value) value.parent = self.name value.parenttype = self.doctype @@ -250,17 +258,35 @@ class BaseDocument(object): value.docstatus = DocStatus.draft() if not getattr(value, "idx", None): - value.idx = len(self.get(key) or []) + 1 + if table := getattr(self, key, None): + value.idx = len(table) + 1 + else: + value.idx = 1 if not getattr(value, "name", None): value.__dict__["__islocal"] = 1 return value + def _get_table_fields(self): + """ + To get table fields during Document init + Meta.get_table_fields goes into recursion for special doctypes + """ + + if self.doctype == "DocType": + return DOCTYPE_TABLE_FIELDS + + # child tables don't have child tables + if self.doctype in DOCTYPES_FOR_DOCTYPE or getattr(self, "parentfield", None): + return () + + return self.meta.get_table_fields() + def get_valid_dict( self, sanitize=True, convert_dates_to_str=False, ignore_nulls=False, ignore_virtual=False ) -> Dict: - d = frappe._dict() + d = _dict() for fieldname in self.meta.get_valid_columns(): # column is valid, we can use getattr d[fieldname] = getattr(self, fieldname, None) @@ -316,6 +342,16 @@ class BaseDocument(object): return d + def init_child_tables(self): + """ + This is needed so that one can loop over child table properties + without worrying about whether or not they have values + """ + + for fieldname in self._table_fieldnames: + if self.__dict__.get(fieldname) is None: + self.__dict__[fieldname] = [] + def init_valid_columns(self): for key in default_fields: if key not in self.__dict__: @@ -365,9 +401,9 @@ class BaseDocument(object): doc = self.get_valid_dict(convert_dates_to_str=convert_dates_to_str, ignore_nulls=no_nulls) doc["doctype"] = self.doctype - for df in self.meta.get_table_fields(): - children = self.get(df.fieldname) or [] - doc[df.fieldname] = [ + for fieldname in self._table_fieldnames: + children = self.get(fieldname) or [] + doc[fieldname] = [ d.as_dict( convert_dates_to_str=convert_dates_to_str, no_nulls=no_nulls, @@ -407,10 +443,9 @@ class BaseDocument(object): try: return self.meta.get_field(fieldname).options except AttributeError: - if self.doctype == "DocType": - return dict(links="DocType Link", actions="DocType Action", states="DocType State").get( - fieldname - ) + if self.doctype == "DocType" and (table_doctype := TABLE_DOCTYPES_FOR_DOCTYPE.get(fieldname)): + return table_doctype + raise def get_parentfield_of_doctype(self, doctype): @@ -519,8 +554,8 @@ class BaseDocument(object): """Raw update parent + children DOES NOT VALIDATE AND CALL TRIGGERS""" self.db_update() - for df in self.meta.get_table_fields(): - for doc in self.get(df.fieldname): + for fieldname in self._table_fieldnames: + for doc in self.get(fieldname): doc.db_update() def show_unique_validation_message(self, e): @@ -632,7 +667,7 @@ class BaseDocument(object): if self.meta.istable: for fieldname in ("parent", "parenttype"): if not self.get(fieldname): - missing.append((fieldname, get_msg(frappe._dict(label=fieldname)))) + missing.append((fieldname, get_msg(_dict(label=fieldname)))) return missing @@ -679,7 +714,7 @@ class BaseDocument(object): if not frappe.get_meta(doctype).get("is_virtual"): if not fields_to_fetch: # cache a single value type - values = frappe._dict(name=frappe.db.get_value(doctype, docname, "name", cache=True)) + values = _dict(name=frappe.db.get_value(doctype, docname, "name", cache=True)) else: values_to_fetch = ["name"] + [_df.fetch_from.split(".")[-1] for _df in fields_to_fetch] @@ -1009,10 +1044,10 @@ class BaseDocument(object): cache_key = parentfield or "main" if not hasattr(self, "_precision"): - self._precision = frappe._dict() + self._precision = _dict() if cache_key not in self._precision: - self._precision[cache_key] = frappe._dict() + self._precision[cache_key] = _dict() if fieldname not in self._precision[cache_key]: self._precision[cache_key][fieldname] = None diff --git a/frappe/model/document.py b/frappe/model/document.py index c5e61563f8..f59fb81350 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -113,6 +113,7 @@ class Document(BaseDocument): if kwargs: # init base document super(Document, self).__init__(kwargs) + self.init_child_tables() self.init_valid_columns() else: @@ -150,25 +151,19 @@ class Document(BaseDocument): super(Document, self).__init__(d) - if self.name == "DocType" and self.doctype == "DocType": - from frappe.model.meta import DOCTYPE_TABLE_FIELDS - - table_fields = DOCTYPE_TABLE_FIELDS - else: - table_fields = self.meta.get_table_fields() - - for df in table_fields: - children = frappe.db.get_values( - df.options, - {"parent": self.name, "parenttype": self.doctype, "parentfield": df.fieldname}, - "*", - as_dict=True, - order_by="idx asc", + for df in self._get_table_fields(): + children = ( + frappe.db.get_values( + df.options, + {"parent": self.name, "parenttype": self.doctype, "parentfield": df.fieldname}, + "*", + as_dict=True, + order_by="idx asc", + ) + or [] ) - if children: - self.set(df.fieldname, children) - else: - self.set(df.fieldname, []) + + self.set(df.fieldname, children) # sometimes __setup__ can depend on child values, hence calling again at the end if hasattr(self, "__setup__"): @@ -897,8 +892,7 @@ class Document(BaseDocument): if parenttype and df.options != parenttype: continue - value = self.get(df.fieldname) - if isinstance(value, list): + if value := self.get(df.fieldname): children.extend(value) return children diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 232d0a8d58..aeb12136ef 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -30,7 +30,11 @@ from frappe.model import ( optional_fields, table_fields, ) -from frappe.model.base_document import BaseDocument +from frappe.model.base_document import ( + DOCTYPE_TABLE_FIELDS, + TABLE_DOCTYPES_FOR_DOCTYPE, + BaseDocument, +) from frappe.model.document import Document from frappe.model.workflow import get_workflow_name from frappe.modules import load_doctype_module @@ -148,9 +152,9 @@ class Meta(Document): out[key] = value # set empty lists for unset table fields - for table_field in DOCTYPE_TABLE_FIELDS: - if out.get(table_field.fieldname) is None: - out[table_field.fieldname] = [] + for fieldname in TABLE_DOCTYPES_FOR_DOCTYPE.keys(): + if out.get(fieldname) is None: + out[fieldname] = [] return out @@ -225,13 +229,7 @@ class Meta(Document): return self._valid_columns def get_table_field_doctype(self, fieldname): - return { - "fields": "DocField", - "permissions": "DocPerm", - "actions": "DocType Action", - "links": "DocType Link", - "states": "DocType State", - }.get(fieldname) + return TABLE_DOCTYPES_FOR_DOCTYPE.get(fieldname) def get_field(self, fieldname): """Return docfield from meta""" @@ -642,14 +640,6 @@ class Meta(Document): return self.has_field("lft") and self.has_field("rgt") -DOCTYPE_TABLE_FIELDS = [ - frappe._dict({"fieldname": "fields", "options": "DocField"}), - frappe._dict({"fieldname": "permissions", "options": "DocPerm"}), - frappe._dict({"fieldname": "actions", "options": "DocType Action"}), - frappe._dict({"fieldname": "links", "options": "DocType Link"}), - frappe._dict({"fieldname": "states", "options": "DocType State"}), -] - ####### diff --git a/frappe/tests/test_base_document.py b/frappe/tests/test_base_document.py index fda795b5b6..a861b3454b 100644 --- a/frappe/tests/test_base_document.py +++ b/frappe/tests/test_base_document.py @@ -5,7 +5,7 @@ from frappe.model.base_document import BaseDocument class TestBaseDocument(unittest.TestCase): def test_docstatus(self): - doc = BaseDocument({"docstatus": 0}) + doc = BaseDocument({"docstatus": 0, "doctype": "ToDo"}) self.assertTrue(doc.docstatus.is_draft()) self.assertEqual(doc.docstatus, 0) diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index 40bc8e2d45..5bda6a1d9d 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -341,3 +341,19 @@ class TestDocument(unittest.TestCase): # run_method should get overridden self.assertEqual(doc.run_method("as_dict"), "success") + + def test_extend(self): + doc = frappe.get_last_doc("User") + self.assertRaises(ValueError, doc.extend, "user_emails", None) + + # allow calling doc.extend with iterable objects + doc.extend("user_emails", ()) + doc.extend("user_emails", []) + doc.extend("user_emails", (x for x in ())) + + def test_set(self): + doc = frappe.get_last_doc("User") + + # setting None should init a table field to empty list + doc.set("user_emails", None) + self.assertEqual(doc.user_emails, [])