fix: link between parent and child procedure (backport #37903)

erpnext/quality_management/doctype/quality_procedure/quality_procedure.js

@@ -3,18 +3,19 @@
 
 frappe.ui.form.on('Quality Procedure', {
 	refresh: function(frm) {
-		frm.set_query("procedure","processes", (frm) =>{
+		frm.set_query('procedure', 'processes', (frm) =>{
 			return {
 				filters: {
-					name: ["not in", [frm.parent_quality_procedure, frm.name]]
+					name: ['not in', [frm.parent_quality_procedure, frm.name]]
 				}
 			};
 		});
 
 		frm.set_query('parent_quality_procedure', function(){
 			return {
 				filters: {
-					is_group: 1
+					is_group: 1,
+					name: ['!=', frm.doc.name]
 				}
 			};
 		});

erpnext/quality_management/doctype/quality_procedure/quality_procedure.py

@@ -16,16 +16,13 @@ def before_save(self):
 	def on_update(self):
 		NestedSet.on_update(self)
 		self.set_parent()
+		self.remove_parent_from_old_child()
+		self.add_child_to_parent()
+		self.remove_child_from_old_parent()
 
 	def after_insert(self):
 		self.set_parent()
-
-		# add child to parent if missing
-		if self.parent_quality_procedure:
-			parent = frappe.get_doc("Quality Procedure", self.parent_quality_procedure)
-			if not [d for d in parent.processes if d.procedure == self.name]:
-				parent.append("processes", {"procedure": self.name, "process_description": self.name})
-				parent.save()
+		self.add_child_to_parent()
 
 	def on_trash(self):
 		# clear from child table (sub procedures)
@@ -36,15 +33,6 @@ def on_trash(self):
 		)
 		NestedSet.on_trash(self, allow_root_deletion=True)
 
-	def set_parent(self):
-		for process in self.processes:
-			# Set parent for only those children who don't have a parent
-			has_parent = frappe.db.get_value(
-				"Quality Procedure", process.procedure, "parent_quality_procedure"
-			)
-			if not has_parent and process.procedure:
-				frappe.db.set_value(self.doctype, process.procedure, "parent_quality_procedure", self.name)
-
 	def check_for_incorrect_child(self):
 		for process in self.processes:
 			if process.procedure:
@@ -61,6 +49,48 @@ def check_for_incorrect_child(self):
 						title=_("Invalid Child Procedure"),
 					)
 
+	def set_parent(self):
+		"""Set `Parent Procedure` in `Child Procedures`"""
+
+		for process in self.processes:
+			if process.procedure:
+				if not frappe.db.get_value("Quality Procedure", process.procedure, "parent_quality_procedure"):
+					frappe.db.set_value(
+						"Quality Procedure", process.procedure, "parent_quality_procedure", self.name
+					)
+
+	def remove_parent_from_old_child(self):
+		"""Remove `Parent Procedure` from `Old Child Procedures`"""
+
+		if old_doc := self.get_doc_before_save():
+			if old_child_procedures := set([d.procedure for d in old_doc.processes if d.procedure]):
+				current_child_procedures = set([d.procedure for d in self.processes if d.procedure])
+
+				if removed_child_procedures := list(old_child_procedures.difference(current_child_procedures)):
+					for child_procedure in removed_child_procedures:
+						frappe.db.set_value("Quality Procedure", child_procedure, "parent_quality_procedure", None)
+
+	def add_child_to_parent(self):
+		"""Add `Child Procedure` to `Parent Procedure`"""
+
+		if self.parent_quality_procedure:
+			parent = frappe.get_doc("Quality Procedure", self.parent_quality_procedure)
+			if not [d for d in parent.processes if d.procedure == self.name]:
+				parent.append("processes", {"procedure": self.name, "process_description": self.name})
+				parent.save()
+
+	def remove_child_from_old_parent(self):
+		"""Remove `Child Procedure` from `Old Parent Procedure`"""
+
+		if old_doc := self.get_doc_before_save():
+			if old_parent := old_doc.parent_quality_procedure:
+				if self.parent_quality_procedure != old_parent:
+					parent = frappe.get_doc("Quality Procedure", old_parent)
+					for process in parent.processes:
+						if process.procedure == self.name:
+							parent.remove(process)
+					parent.save()
+
 
 @frappe.whitelist()
 def get_children(doctype, parent=None, parent_quality_procedure=None, is_root=False):

erpnext/quality_management/doctype/quality_procedure/test_quality_procedure.py

@@ -1,56 +1,107 @@
 # Copyright (c) 2018, Frappe and Contributors
 # See license.txt
 
-import unittest
-
 import frappe
+from frappe.tests.utils import FrappeTestCase
 
 from .quality_procedure import add_node
 
 
-class TestQualityProcedure(unittest.TestCase):
+class TestQualityProcedure(FrappeTestCase):
 	def test_add_node(self):
-		try:
-			procedure = frappe.get_doc(
-				dict(
-					doctype="Quality Procedure",
-					quality_procedure_name="Test Procedure 1",
-					processes=[dict(process_description="Test Step 1")],
-				)
-			).insert()
+		procedure = create_procedure(
+			{
+				"quality_procedure_name": "Test Procedure 1",
+				"is_group": 1,
+				"processes": [dict(process_description="Test Step 1")],
+			}
+		)
+
+		frappe.local.form_dict = frappe._dict(
+			doctype="Quality Procedure",
+			quality_procedure_name="Test Child 1",
+			parent_quality_procedure=procedure.name,
+			cmd="test",
+			is_root="false",
+		)
+		node = add_node()
 
-			frappe.local.form_dict = frappe._dict(
-				doctype="Quality Procedure",
-				quality_procedure_name="Test Child 1",
-				parent_quality_procedure=procedure.name,
-				cmd="test",
-				is_root="false",
-			)
-			node = add_node()
+		procedure.reload()
 
-			procedure.reload()
+		self.assertEqual(procedure.is_group, 1)
 
-			self.assertEqual(procedure.is_group, 1)
+		# child row created
+		self.assertTrue([d for d in procedure.processes if d.procedure == node.name])
 
-			# child row created
-			self.assertTrue([d for d in procedure.processes if d.procedure == node.name])
+		node.delete()
+		procedure.reload()
 
-			node.delete()
-			procedure.reload()
+		# child unset
+		self.assertFalse([d for d in procedure.processes if d.name == node.name])
 
-			# child unset
-			self.assertFalse([d for d in procedure.processes if d.name == node.name])
+	def test_remove_parent_from_old_child(self):
+		child_qp = create_procedure(
+			{
+				"quality_procedure_name": "Test Child 1",
+				"is_group": 0,
+			}
+		)
+		group_qp = create_procedure(
+			{
+				"quality_procedure_name": "Test Group",
+				"is_group": 1,
+				"processes": [dict(procedure=child_qp.name)],
+			}
+		)
 
-		finally:
-			procedure.delete()
+		child_qp.reload()
+		self.assertEqual(child_qp.parent_quality_procedure, group_qp.name)
 
+		group_qp.reload()
+		del group_qp.processes[0]
+		group_qp.save()
 
-def create_procedure():
-	return frappe.get_doc(
-		dict(
-			doctype="Quality Procedure",
-			quality_procedure_name="Test Procedure 1",
-			is_group=1,
-			processes=[dict(process_description="Test Step 1")],
+		child_qp.reload()
+		self.assertEqual(child_qp.parent_quality_procedure, None)
+
+	def remove_child_from_old_parent(self):
+		child_qp = create_procedure(
+			{
+				"quality_procedure_name": "Test Child 1",
+				"is_group": 0,
+			}
+		)
+		group_qp = create_procedure(
+			{
+				"quality_procedure_name": "Test Group",
+				"is_group": 1,
+				"processes": [dict(procedure=child_qp.name)],
+			}
 		)
-	).insert()
+
+		group_qp.reload()
+		self.assertTrue([d for d in group_qp.processes if d.procedure == child_qp.name])
+
+		child_qp.reload()
+		self.assertEqual(child_qp.parent_quality_procedure, group_qp.name)
+
+		child_qp.parent_quality_procedure = None
+		child_qp.save()
+
+		group_qp.reload()
+		self.assertFalse([d for d in group_qp.processes if d.procedure == child_qp.name])
+
+
+def create_procedure(kwargs=None):
+	kwargs = frappe._dict(kwargs or {})
+
+	doc = frappe.new_doc("Quality Procedure")
+	doc.quality_procedure_name = kwargs.quality_procedure_name or "_Test Procedure"
+	doc.is_group = kwargs.is_group or 0
+
+	for process in kwargs.processes or []:
+		doc.append("processes", process)
+
+	doc.insert()
+
+	return doc