Merge pull request #16840 from phot0n/refactor-insert-many-client

refactor(minor): move out common functionality from insert and insert_many client functions to a separate function

frappe/client.py

@@ -2,6 +2,7 @@
 # License: MIT. See LICENSE
 import json
 import os
+from typing import TYPE_CHECKING
 
 import frappe
 import frappe.model
@@ -11,6 +12,9 @@ from frappe.desk.reportview import validate_args
 from frappe.model.db_query import check_parent_permission
 from frappe.utils import get_safe_filters
 
+if TYPE_CHECKING:
+	from frappe.model.document import Document
+
 """
 Handle RESTful requests that are mapped to the `/api/resource` route.
 
@@ -189,18 +193,7 @@ def insert(doc=None):
 	if isinstance(doc, str):
 		doc = json.loads(doc)
 
-	doc = frappe._dict(doc)
-	if frappe.is_table(doc.doctype):
-		if not (doc.parenttype and doc.parent and doc.parentfield):
-			frappe.throw(_("parenttype, parent and parentfield are required to insert a child record"))
-		# inserting a child record
-		parent = frappe.get_doc(doc.parenttype, doc.parent)
-		parent.append(doc.parentfield, doc)
-		parent.save()
-		return parent.as_dict()
-	else:
-		doc = frappe.get_doc(doc).insert()
-		return doc.as_dict()
+	return insert_doc(doc).as_dict()
 
 
 @frappe.whitelist(methods=["POST", "PUT"])
@@ -211,21 +204,12 @@ def insert_many(docs=None):
 	if isinstance(docs, str):
 		docs = json.loads(docs)
 
-	out = []
-
 	if len(docs) > 200:
 		frappe.throw(_("Only 200 inserts allowed in one request"))
 
+	out = set()
 	for doc in docs:
-		if doc.get("parenttype"):
-			# inserting a child record
-			parent = frappe.get_doc(doc.parenttype, doc.parent)
-			parent.append(doc.parentfield, doc)
-			parent.save()
-			out.append(parent.name)
-		else:
-			doc = frappe.get_doc(doc).insert()
-			out.append(doc.name)
+		out.add(insert_doc(doc).name)
 
 	return out
 
@@ -496,3 +480,23 @@ def validate_link(doctype: str, docname: str, fields=None):
 		)
 
 	return values
+
+
+def insert_doc(doc) -> "Document":
+	"""Inserts document and returns parent document object with appended child document
+	if `doc` is child document else returns the inserted document object
+
+	:param doc: doc to insert (dict)"""
+
+	doc = frappe._dict(doc)
+	if frappe.is_table(doc.doctype):
+		if not (doc.parenttype and doc.parent and doc.parentfield):
+			frappe.throw(_("Parenttype, Parent and Parentfield are required to insert a child record"))
+
+		# inserting a child record
+		parent = frappe.get_doc(doc.parenttype, doc.parent)
+		parent.append(doc.parentfield, doc)
+		parent.save()
+		return parent
+
+	return frappe.get_doc(doc).insert()

frappe/tests/test_client.py

@@ -178,3 +178,50 @@ class TestClient(unittest.TestCase):
 		# cleanup
 		frappe.delete_doc("Note", note1.name)
 		frappe.delete_doc("Note", note2.name)
+
+	def test_client_insert_many(self):
+		from frappe.client import insert, insert_many
+
+		def get_random_title():
+			return "test-{0}".format(frappe.generate_hash(length=5))
+
+		# insert a (parent) doc
+		note1 = {"doctype": "Note", "title": get_random_title(), "content": "test"}
+		note1 = insert(note1)
+
+		doc_list = [
+			{
+				"doctype": "Note Seen By",
+				"user": "Administrator",
+				"parenttype": "Note",
+				"parent": note1.name,
+				"parentfield": "seen_by",
+			},
+			{
+				"doctype": "Note Seen By",
+				"user": "Administrator",
+				"parenttype": "Note",
+				"parent": note1.name,
+				"parentfield": "seen_by",
+			},
+			{
+				"doctype": "Note Seen By",
+				"user": "Administrator",
+				"parenttype": "Note",
+				"parent": note1.name,
+				"parentfield": "seen_by",
+			},
+			{"doctype": "Note", "title": get_random_title(), "content": "test"},
+			{"doctype": "Note", "title": get_random_title(), "content": "test"},
+		]
+
+		# insert all docs
+		docs = insert_many(doc_list)
+
+		# make sure only 1 name is returned for the parent upon insertion of child docs
+		self.assertEqual(len(docs), 3)
+		self.assertIn(note1.name, docs)
+
+		# cleanup
+		for doc in docs:
+			frappe.delete_doc("Note", doc)