From 534911a1b4def6e2a973d8dcde188d3cf5023853 Mon Sep 17 00:00:00 2001 From: phot0n Date: Fri, 6 May 2022 00:27:09 +0530 Subject: [PATCH 1/4] refactor(minor): move out common functionality in insert and insert_many client functions to a separate function --- frappe/client.py | 49 +++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 1bad2bed2f..bb30248cf0 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import json import os +from typing import TYPE_CHECKING, Dict, List 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 = [] 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.append(insert_doc(doc).name) return out @@ -496,3 +480,22 @@ def validate_link(doctype: str, docname: str, fields=None): ) return values + + +def insert_doc(doc: Dict) -> "Document": + """Inserts document and returns parent document with appended child document + if `doc` is child document else just returns the inserted document""" + + 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 + else: + doc = frappe.get_doc(doc).insert() + return doc From 98c60dbb1f59be9122bca6d05be21a334c6e762a Mon Sep 17 00:00:00 2001 From: phot0n Date: Fri, 6 May 2022 22:56:07 +0530 Subject: [PATCH 2/4] chore: remove else condition, improve formatting & comment in insert_doc --- frappe/client.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index bb30248cf0..1e42d85dba 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -2,7 +2,7 @@ # License: MIT. See LICENSE import json import os -from typing import TYPE_CHECKING, Dict, List +from typing import TYPE_CHECKING import frappe import frappe.model @@ -482,20 +482,21 @@ def validate_link(doctype: str, docname: str, fields=None): return values -def insert_doc(doc: Dict) -> "Document": - """Inserts document and returns parent document with appended child document - if `doc` is child document else just returns the inserted document""" +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")) + 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 - else: - doc = frappe.get_doc(doc).insert() - return doc + + return frappe.get_doc(doc).insert() From b1a2007024f9eedeaa08fa864405a8c4112280d8 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 11 May 2022 23:41:47 +0530 Subject: [PATCH 3/4] fix: use set instead of list in insert_many client function --- frappe/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 1e42d85dba..f753da6f57 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -207,9 +207,9 @@ def insert_many(docs=None): if len(docs) > 200: frappe.throw(_("Only 200 inserts allowed in one request")) - out = [] + out = set() for doc in docs: - out.append(insert_doc(doc).name) + out.add(insert_doc(doc).name) return out From 0d9a6adcd9840f318b844c7451a8518e692ca11c Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 11 May 2022 23:59:21 +0530 Subject: [PATCH 4/4] test: test for insert_many client function --- frappe/tests/test_client.py | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/frappe/tests/test_client.py b/frappe/tests/test_client.py index 677f59a366..b5a1771800 100644 --- a/frappe/tests/test_client.py +++ b/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)