Merge pull request #11795 from gavindsouza/fix-broken-folders

fix: Rename Doc (and others)

.travis.yml

@@ -31,12 +31,12 @@ matrix:
   - name: "Python 3.7 MariaDB"
     python: 3.7
     env: DB=mariadb TYPE=server
-    script: bench --site test_site run-tests --coverage
+    script: bench --verbose --site test_site run-tests --coverage
 
   - name: "Python 3.7 PostgreSQL"
     python: 3.7
     env: DB=postgres TYPE=server
-    script: bench --site test_site run-tests --coverage
+    script: bench --verbose --site test_site run-tests --coverage
 
   - name: "Cypress"
     python: 3.7

frappe/__init__.py

@@ -326,7 +326,7 @@ def msgprint(msg, title=None, raise_exception=0, as_table=False, as_list=False,
 	:param is_minimizable: [optional] Allow users to minimize the modal
 	:param wide: [optional] Show wide modal
 	"""
-	from frappe.utils import encode
+	from frappe.utils import strip_html_tags
 
 	msg = safe_decode(msg)
 	out = _dict(message=msg)
@@ -353,7 +353,7 @@ def msgprint(msg, title=None, raise_exception=0, as_table=False, as_list=False,
 		out.as_list = 1
 
 	if flags.print_messages and out.message:
-		print(f"Message: {repr(out.message).encode('utf-8')}")
+		print(f"Message: {strip_html_tags(out.message)}")
 
 	if title:
 		out.title = title

frappe/core/doctype/doctype/doctype.py

@@ -290,9 +290,15 @@ class DocType(Document):
 
 		self.update_fields_to_fetch()
 
-		from frappe import conf
-		allow_doctype_export = frappe.flags.allow_doctype_export or (not frappe.flags.in_test and conf.get('developer_mode'))
-		if not self.custom and not frappe.flags.in_import and allow_doctype_export:
+		allow_doctype_export = (
+			not self.custom
+			and not frappe.flags.in_import
+			and (
+				frappe.conf.developer_mode
+				or frappe.flags.allow_doctype_export
+			)
+		)
+		if allow_doctype_export:
 			self.export_doc()
 			self.make_controller_template()
 
@@ -382,13 +388,10 @@ class DocType(Document):
 		if merge:
 			frappe.throw(_("DocType can not be merged"))
 
-		# Do not rename and move files and folders for custom doctype
-		if not self.custom and not frappe.flags.in_test and not frappe.flags.in_patch:
-			self.rename_files_and_folders(old, new)
-
 	def after_rename(self, old, new, merge=False):
 		"""Change table name using `RENAME TABLE` if table exists. Or update
 		`doctype` property for Single type."""
+
 		if self.issingle:
 			frappe.db.sql("""update tabSingles set doctype=%s where doctype=%s""", (new, old))
 			frappe.db.sql("""update tabSingles set value=%s
@@ -398,6 +401,20 @@ class DocType(Document):
 				"mariadb": f"RENAME TABLE `tab{old}` TO `tab{new}`",
 				"postgres": f"ALTER TABLE `tab{old}` RENAME TO `tab{new}`"
 			})
+			frappe.db.commit()
+
+		# Do not rename and move files and folders for custom doctype
+		if not self.custom:
+			if not frappe.flags.in_patch:
+				self.rename_files_and_folders(old, new)
+
+			for site in frappe.utils.get_sites():
+				frappe.cache().delete(f"{site}:doctype_classes", old)
+
+	def after_delete(self):
+		if not self.custom:
+			for site in frappe.utils.get_sites():
+				frappe.cache().delete(f"{site}:doctype_classes", self.name)
 
 	def rename_files_and_folders(self, old, new):
 		# move files

frappe/model/delete_doc.py

@@ -76,7 +76,12 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa
 
 			delete_from_table(doctype, name, ignore_doctypes, None)
 
-			if not (for_reload or frappe.flags.in_migrate or frappe.flags.in_install or frappe.flags.in_uninstall or frappe.flags.in_test):
+			if frappe.conf.developer_mode and not doc.custom and not (
+				for_reload
+				or frappe.flags.in_migrate
+				or frappe.flags.in_install
+				or frappe.flags.in_uninstall
+			):
 				try:
 					delete_controllers(name, doc.module)
 				except (FileNotFoundError, OSError, KeyError):

frappe/model/rename_doc.py

@@ -49,9 +49,7 @@ def rename_doc(doctype, old, new, force=False, merge=False, ignore_permissions=F
 	old_doc = frappe.get_doc(doctype, old)
 	out = old_doc.run_method("before_rename", old, new, merge) or {}
 	new = (out.get("new") or new) if isinstance(out, dict) else (out or new)
-
-	if doctype != "DocType":
-		new = validate_rename(doctype, new, meta, merge, force, ignore_permissions)
+	new = validate_rename(doctype, new, meta, merge, force, ignore_permissions)
 
 	if not merge:
 		rename_parent_and_child(doctype, old, new, meta)
@@ -250,6 +248,7 @@ def update_link_field_values(link_fields, old, new, doctype):
 				pass
 		else:
 			parent = field['parent']
+			docfield = field["fieldname"]
 
 			# Handles the case where one of the link fields belongs to
 			# the DocType being renamed.
@@ -261,11 +260,8 @@ def update_link_field_values(link_fields, old, new, doctype):
 			if parent == new and doctype == "DocType":
 				parent = old
 
-			frappe.db.sql("""
-				update `tab{table_name}` set `{fieldname}`=%s
-				where `{fieldname}`=%s""".format(
-					table_name=parent,
-					fieldname=field['fieldname']), (new, old))
+			frappe.db.set_value(parent, {docfield: old}, docfield, new)
+
 		# update cached link_fields as per new
 		if doctype=='DocType' and field['parent'] == old:
 			field['parent'] = new

frappe/tests/test_document.py

@@ -249,82 +249,6 @@ class TestDocument(unittest.TestCase):
 
 			self.assertEqual(cint(old_current) - 1, new_current)
 
-	def test_rename_doc(self):
-		from random import choice, sample
-
-		available_documents = []
-		doctype = "ToDo"
-
-		# data generation: 4 todo documents
-		for num in range(1, 5):
-			doc = frappe.get_doc({
-				"doctype": doctype,
-				"date": add_to_date(now(), days=num),
-				"description": "this is todo #{}".format(num)
-			}).insert()
-			available_documents.append(doc.name)
-
-		# test 1: document renaming
-		old_name = choice(available_documents)
-		new_name = old_name + '.new'
-		self.assertEqual(new_name, frappe.rename_doc(doctype, old_name, new_name, force=True))
-		available_documents.remove(old_name)
-		available_documents.append(new_name)
-
-		# test 2: merge documents
-		first_todo, second_todo = sample(available_documents, 2)
-
-		second_todo_doc = frappe.get_doc(doctype, second_todo)
-		second_todo_doc.priority = "High"
-		second_todo_doc.save()
-
-		merged_todo = frappe.rename_doc(doctype, first_todo, second_todo, merge=True, force=True)
-		merged_todo_doc = frappe.get_doc(doctype, merged_todo)
-		available_documents.remove(first_todo)
-
-		with self.assertRaises(DoesNotExistError):
-			frappe.get_doc(doctype, first_todo)
-
-		self.assertEqual(merged_todo_doc.priority, second_todo_doc.priority)
-
-		for docname in available_documents:
-			frappe.delete_doc(doctype, docname)
-
-	def test_rename_doctype(self):
-		from frappe.core.doctype.doctype.test_doctype import new_doctype
-
-		fields =[{
-			"label": "Linked To",
-			"fieldname": "linked_to_doctype",
-			"fieldtype": "Link",
-			"options": "DocType",
-			"unique": 0
-		}]
-		if not frappe.db.exists("DocType", "Rename This"):
-			new_doctype("Rename This", unique=0, fields=fields).insert()
-
-		to_rename_record = frappe.get_doc({
-			"doctype": "Rename This",
-			"linked_to_doctype": "Rename This"
-		})
-		to_rename_record.insert()
-
-		# Rename doctype
-		self.assertEqual("Renamed Doc", frappe.rename_doc("DocType", "Rename This", "Renamed Doc", force=True))
-
-		# Test if Doctype value has changed in Link field
-		renamed_doctype_record = frappe.get_doc("Renamed Doc", to_rename_record.name)
-		self.assertEqual(renamed_doctype_record.linked_to_doctype, "Renamed Doc")
-
-		# Test if there are conflicts between a record and a DocType
-		# having the same name
-		old_name = to_rename_record.name
-		new_name = "ToDo"
-		self.assertEqual(new_name, frappe.rename_doc("Renamed Doc", old_name, new_name, force=True))
-
-		frappe.delete_doc_if_exists("Renamed Doc", "ToDo")
-		frappe.delete_doc_if_exists("DocType", "Renamed Doc")
-
 	def test_non_negative_check(self):
 		frappe.delete_doc_if_exists("Currency", "Frappe Coin", 1)
 

frappe/tests/test_rename_doc.py

@@ -0,0 +1,159 @@
+import os
+import unittest
+
+import frappe
+from frappe.utils import add_to_date, now
+from frappe.exceptions import DoesNotExistError
+
+from random import choice, sample
+from frappe.model.base_document import get_controller
+from frappe.modules.utils import get_doc_path
+
+
+class TestRenameDoc(unittest.TestCase):
+	@classmethod
+	def setUpClass(self):
+		"""Setting Up data for the tests defined under TestRenameDoc"""
+		# set developer_mode to rename doc controllers
+		self._original_developer_flag = frappe.conf.developer_mode
+		frappe.conf.developer_mode = 1
+
+		# data generation: for base and merge tests
+		self.available_documents = []
+		self.test_doctype = "ToDo"
+
+		for num in range(1, 5):
+			doc = frappe.get_doc({
+				"doctype": self.test_doctype,
+				"date": add_to_date(now(), days=num),
+				"description": "this is todo #{}".format(num),
+			}).insert()
+			self.available_documents.append(doc.name)
+
+		#  data generation: for controllers tests
+		self.doctype = frappe._dict({
+			"old": "Test Rename Document Old",
+			"new": "Test Rename Document New",
+		})
+
+		frappe.get_doc({
+			"doctype": "DocType",
+			"module": "Custom",
+			"name": self.doctype.old,
+			"custom": 0,
+			"fields": [
+				{"label": "Some Field", "fieldname": "some_fieldname", "fieldtype": "Data"}
+			],
+			"permissions": [{"role": "System Manager", "read": 1}],
+		}).insert()
+
+	@classmethod
+	def tearDownClass(self):
+		"""Deleting data generated for the tests defined under TestRenameDoc"""
+		# delete the documents created
+		for docname in self.available_documents:
+			frappe.delete_doc(self.test_doctype, docname)
+
+		for dt in self.doctype.values():
+			if frappe.db.exists("DocType", dt):
+				frappe.delete_doc("DocType", dt)
+				frappe.db.sql_ddl(f"DROP TABLE IF EXISTS `tab{dt}`")
+
+		frappe.delete_doc_if_exists("Renamed Doc", "ToDo")
+
+		# reset original value of developer_mode conf
+		frappe.conf.developer_mode = self._original_developer_flag
+
+	def setUp(self):
+		frappe.flags.link_fields = {}
+		super().setUp()
+
+	def test_rename_doc(self):
+		"""Rename an existing document via frappe.rename_doc"""
+		old_name = choice(self.available_documents)
+		new_name = old_name + ".new"
+		self.assertEqual(new_name, frappe.rename_doc(self.test_doctype, old_name, new_name, force=True))
+		self.available_documents.remove(old_name)
+		self.available_documents.append(new_name)
+
+	def test_merging_docs(self):
+		"""Merge two documents via frappe.rename_doc"""
+		first_todo, second_todo = sample(self.available_documents, 2)
+
+		second_todo_doc = frappe.get_doc(self.test_doctype, second_todo)
+		second_todo_doc.priority = "High"
+		second_todo_doc.save()
+
+		merged_todo = frappe.rename_doc(
+			self.test_doctype, first_todo, second_todo, merge=True, force=True
+		)
+		merged_todo_doc = frappe.get_doc(self.test_doctype, merged_todo)
+		self.available_documents.remove(first_todo)
+
+		with self.assertRaises(DoesNotExistError):
+			frappe.get_doc(self.test_doctype, first_todo)
+
+		self.assertEqual(merged_todo_doc.priority, second_todo_doc.priority)
+
+	def test_rename_controllers(self):
+		"""Rename doctypes with controller code paths"""
+		# check if module exists exists;
+		# if custom, get_controller will return Document class
+		# if not custom, a different class will be returned
+		self.assertNotEqual(get_controller(self.doctype.old), frappe.model.document.Document)
+
+		old_doctype_path = get_doc_path("Custom", "DocType", self.doctype.old)
+
+		# rename doc via wrapper API accessible via /desk
+		frappe.rename_doc("DocType", self.doctype.old, self.doctype.new)
+
+		# check if database and controllers are updated
+		self.assertTrue(frappe.db.exists("DocType", self.doctype.new))
+		self.assertFalse(frappe.db.exists("DocType", self.doctype.old))
+		self.assertFalse(os.path.exists(old_doctype_path))
+
+	def test_rename_doctype(self):
+		"""Rename DocType via frappe.rename_doc"""
+		from frappe.core.doctype.doctype.test_doctype import new_doctype
+
+		if not frappe.db.exists("DocType", "Rename This"):
+			new_doctype(
+				"Rename This",
+				fields=[
+					{
+						"label": "Linked To",
+						"fieldname": "linked_to_doctype",
+						"fieldtype": "Link",
+						"options": "DocType",
+						"unique": 0,
+					}
+				],
+			).insert()
+
+		to_rename_record = frappe.get_doc(
+			{"doctype": "Rename This", "linked_to_doctype": "Rename This"}
+		).insert()
+
+		# Rename doctype
+		self.assertEqual(
+			"Renamed Doc", frappe.rename_doc("DocType", "Rename This", "Renamed Doc", force=True)
+		)
+
+		# Test if Doctype value has changed in Link field
+		linked_to_doctype = frappe.db.get_value(
+			"Renamed Doc", to_rename_record.name, "linked_to_doctype"
+		)
+		self.assertEqual(linked_to_doctype, "Renamed Doc")
+
+		# Test if there are conflicts between a record and a DocType
+		# having the same name
+		old_name = to_rename_record.name
+		new_name = "ToDo"
+		self.assertEqual(
+			new_name, frappe.rename_doc("Renamed Doc", old_name, new_name, force=True)
+		)
+
+		# delete_doc doesnt drop tables
+		# this is done to bypass inconsistencies in the db
+		frappe.delete_doc_if_exists("DocType", "Renamed Doc")
+		frappe.db.sql_ddl("drop table if exists `tabRenamed Doc`")