From 5554707148b4de05f81d2343df0313fd4c7319b3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 10 Aug 2023 06:29:18 +0000 Subject: [PATCH 1/4] refactor: Use single query to delete child rows --- frappe/model/document.py | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 9768200164..6b6f87be17 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -427,25 +427,20 @@ class Document(BaseDocument): # hack for docperm :( return - if rows: - # select rows that do not match the ones in the document - deleted_rows = frappe.db.sql( - """select name from `tab{}` where parent=%s - and parenttype=%s and parentfield=%s - and name not in ({})""".format( - df.options, ",".join(["%s"] * len(rows)) - ), - [self.name, self.doctype, fieldname] + rows, - ) - if len(deleted_rows) > 0: - # delete rows that do not match the ones in the document - frappe.db.delete(df.options, {"name": ("in", tuple(row[0] for row in deleted_rows))}) + # delete rows that do not match the ones in the document + tbl = frappe.qb.DocType(df.options) + qry = ( + frappe.qb.from_(tbl) + .where(tbl.parent == self.name) + .where(tbl.parenttype == self.doctype) + .where(tbl.parentfield == fieldname) + .delete() + ) - else: - # no rows found, delete all rows - frappe.db.delete( - df.options, {"parent": self.name, "parenttype": self.doctype, "parentfield": fieldname} - ) + if rows: + qry = qry.where(tbl.name.notin(rows)) + + qry.run() def get_doc_before_save(self) -> "Document": return getattr(self, "_doc_before_save", None) From 28e657b60399a68000503307ba893b9e3e336f8a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 10 Aug 2023 06:35:01 +0000 Subject: [PATCH 2/4] fix: Check if fieldtype in set rather than str comparison --- frappe/database/postgres/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index 5e28c81455..f260ad2016 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -88,7 +88,7 @@ class PostgresTable(DBTable): # involving the old values of the row # read more https://www.postgresql.org/docs/9.1/sql-altertable.html using_clause = f"USING {col.fieldname}::timestamp without time zone" - elif col.fieldtype in ("Check"): + elif col.fieldtype == "Check": using_clause = f"USING {col.fieldname}::smallint" query.append( From 84f134a683f21807782302887ec5ee4ae3331002 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 11 Aug 2023 11:44:00 +0530 Subject: [PATCH 3/4] fix: Add "better" typing hints --- .../doctype/custom_field/custom_field.py | 2 +- frappe/model/base_document.py | 19 +++++++++++++------ frappe/model/document.py | 11 +++++++---- frappe/model/naming.py | 2 +- pyproject.toml | 1 + 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index b73820a562..7342667668 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -293,7 +293,7 @@ def create_custom_field(doctype, df, ignore_validate=False, is_system_generated= return custom_field -def create_custom_fields(custom_fields, ignore_validate=False, update=True): +def create_custom_fields(custom_fields: dict, ignore_validate=False, update=True): """Add / update multiple custom fields :param custom_fields: example `{'Sales Invoice': [dict(fieldname='test')]}`""" diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 609bfa4b9e..87f491d942 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import datetime import json +from typing import TYPE_CHECKING, TypeVar import frappe from frappe import _, _dict @@ -21,6 +22,12 @@ from frappe.modules import load_doctype_module from frappe.utils import cast_fieldtype, cint, compare, cstr, flt, now, sanitize_html, strip_html from frappe.utils.html_utils import unescape_html +if TYPE_CHECKING: + from frappe.model.document import Document + +D = TypeVar("D", bound="Document") + + max_positive_value = {"smallint": 2**15 - 1, "int": 2**31 - 1, "bigint": 2**63 - 1} DOCTYPE_TABLE_FIELDS = [ @@ -220,7 +227,7 @@ class BaseDocument: if key in self.__dict__: del self.__dict__[key] - def append(self, key, value=None): + def append(self, key: str, value: D | dict | None = None): """Append an item to a child table. Example: @@ -236,13 +243,13 @@ class BaseDocument: if (table := self.__dict__.get(key)) is None: self.__dict__[key] = table = [] - value = self._init_child(value, key) - table.append(value) + ret_value = self._init_child(value, key) + table.append(ret_value) # reference parent document - value.parent_doc = self + ret_value.parent_doc = self - return value + return ret_value def extend(self, key, value): try: @@ -302,7 +309,7 @@ class BaseDocument: def get_valid_dict( self, sanitize=True, convert_dates_to_str=False, ignore_nulls=False, ignore_virtual=False - ) -> dict: + ) -> _dict: d = _dict() permitted_fields = get_permitted_fields( doctype=self.doctype, parenttype=getattr(self, "parenttype", None) diff --git a/frappe/model/document.py b/frappe/model/document.py index 6b6f87be17..f3aa77b4d2 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -4,7 +4,7 @@ import hashlib import json import time from collections.abc import Generator, Iterable -from typing import Any +from typing import TYPE_CHECKING, Any, Optional from werkzeug.exceptions import NotFound @@ -24,6 +24,9 @@ from frappe.utils import compare, cstr, date_diff, file_lock, flt, get_datetime_ from frappe.utils.data import get_absolute_url from frappe.utils.global_search import update_global_search +if TYPE_CHECKING: + from frappe.core.doctype.docfield.docfield import DocField + def get_doc(*args, **kwargs): """returns a frappe.model.Document object. @@ -409,13 +412,13 @@ class Document(BaseDocument): for df in self.meta.get_table_fields(): self.update_child_table(df.fieldname, df) - def update_child_table(self, fieldname, df=None): + def update_child_table(self, fieldname: str, df: Optional["DocField"] = None): """sync child table for given fieldname""" rows = [] - if not df: - df = self.meta.get_field(fieldname) + df: "DocField" = df or self.meta.get_field(fieldname) for d in self.get(df.fieldname): + d: Document d.db_update() rows.append(d.name) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index a202cba11f..c88f87bd5b 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -552,7 +552,7 @@ def _prompt_autoname(autoname, doc): frappe.throw(_("Please set the document name")) -def _format_autoname(autoname, doc): +def _format_autoname(autoname: str, doc): """ Generate autoname by replacing all instances of braced params (fields, date params ('DD', 'MM', 'YY'), series) Independent of remaining string or separators. diff --git a/pyproject.toml b/pyproject.toml index 2afef9902f..e281cdb581 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,6 +69,7 @@ dependencies = [ "tenacity~=8.2.2", "terminaltables~=3.1.10", "traceback-with-variables~=2.0.4", + "typing_extensions", # included by Pydantic, adding here since it's now being directly used in code "xlrd~=2.0.1", "zxcvbn~=4.4.28", "markdownify~=0.11.6", From 02e1311b3a5c12b1c08bbf282025b4f4a4bdf7b9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Aug 2023 11:50:59 +0530 Subject: [PATCH 4/4] build: pin typing_extensions to major version --- frappe/model/base_document.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 87f491d942..d19f28a475 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -227,7 +227,7 @@ class BaseDocument: if key in self.__dict__: del self.__dict__[key] - def append(self, key: str, value: D | dict | None = None): + def append(self, key: str, value: D | dict | None = None) -> D: """Append an item to a child table. Example: diff --git a/pyproject.toml b/pyproject.toml index e281cdb581..678be0bae7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,7 +69,7 @@ dependencies = [ "tenacity~=8.2.2", "terminaltables~=3.1.10", "traceback-with-variables~=2.0.4", - "typing_extensions", # included by Pydantic, adding here since it's now being directly used in code + "typing_extensions>=4.6.1,<5", "xlrd~=2.0.1", "zxcvbn~=4.4.28", "markdownify~=0.11.6",