Merge pull request #22003 from gavindsouza/refactor-doc-bits

refactor: Use single query to delete child rows on doc.save

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')]}`"""

frappe/database/postgres/schema.py

@@ -85,7 +85,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(

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) -> D:
 		"""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)

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)
 
@@ -427,25 +430,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)

frappe/model/naming.py

@@ -551,7 +551,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.

pyproject.toml

@@ -69,6 +69,7 @@ dependencies = [
     "tenacity~=8.2.2",
     "terminaltables~=3.1.10",
     "traceback-with-variables~=2.0.4",
+    "typing_extensions>=4.6.1,<5",
     "xlrd~=2.0.1",
     "zxcvbn~=4.4.28",
     "markdownify~=0.11.6",