feat(sla): filter service level agreement link by document type (backport #56954) (#56955)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

erpnext/patches.txt

@@ -438,3 +438,5 @@ erpnext.patches.v16_0.migrate_address_contact_custom_fields
 erpnext.patches.v15_0.set_main_item_code_in_material_request_plan_item
 erpnext.patches.v16_0.set_posting_datetime_for_sabb_and_drop_indexes
 execute:frappe.db.set_single_value("Accounts Settings", "pcv_job_timeout", 3600)
+erpnext.patches.v15_0.backfill_sla_link_filters_on_custom_field
+erpnext.patches.v15_0.backfill_sla_link_filters_on_docfield

erpnext/patches/v15_0/backfill_sla_link_filters_on_custom_field.py

@@ -0,0 +1,21 @@
+import frappe
+
+
+def execute():
+	for custom_field in frappe.get_all(
+		"Custom Field",
+		filters={
+			"fieldname": "service_level_agreement",
+			"fieldtype": "Link",
+			"options": "Service Level Agreement",
+			"link_filters": ("is", "not set"),
+		},
+		fields=["name", "dt"],
+	):
+		link_filters = frappe.as_json(
+			[["Service Level Agreement", "document_type", "=", custom_field.dt]], indent=None
+		)
+		frappe.db.set_value(
+			"Custom Field", custom_field.name, "link_filters", link_filters, update_modified=False
+		)
+		frappe.clear_cache(doctype=custom_field.dt)

erpnext/patches/v15_0/backfill_sla_link_filters_on_docfield.py

@@ -0,0 +1,20 @@
+import frappe
+
+
+def execute():
+	for docfield in frappe.get_all(
+		"DocField",
+		filters={
+			"parenttype": "DocType",
+			"fieldname": "service_level_agreement",
+			"fieldtype": "Link",
+			"options": "Service Level Agreement",
+			"link_filters": ("is", "not set"),
+		},
+		fields=["name", "parent"],
+	):
+		link_filters = frappe.as_json(
+			[["Service Level Agreement", "document_type", "=", docfield.parent]], indent=None
+		)
+		frappe.db.set_value("DocField", docfield.name, "link_filters", link_filters, update_modified=False)
+		frappe.clear_cache(doctype=docfield.parent)

erpnext/support/doctype/service_level_agreement/service_level_agreement.py

@@ -232,7 +232,7 @@ class ServiceLevelAgreement(Document):
 		if self.document_type == "Issue":
 			return
 
-		service_level_agreement_fields = get_service_level_agreement_fields()
+		service_level_agreement_fields = get_service_level_agreement_fields(self.document_type)
 		meta = frappe.get_meta(self.document_type, cached=False)
 
 		if meta.custom:
@@ -276,6 +276,7 @@ class ServiceLevelAgreement(Document):
 						"hidden": field.get("hidden"),
 						"description": field.get("description"),
 						"default": field.get("default"),
+						"link_filters": field.get("link_filters"),
 					}
 				).insert(ignore_permissions=True)
 			else:
@@ -302,6 +303,7 @@ class ServiceLevelAgreement(Document):
 						"hidden": field.get("hidden"),
 						"description": field.get("description"),
 						"default": field.get("default"),
+						"link_filters": field.get("link_filters"),
 					}
 				).insert(ignore_permissions=True)
 			else:
@@ -309,7 +311,7 @@ class ServiceLevelAgreement(Document):
 				self.reset_field_properties(existing_field, "Custom Field", field)
 
 	def reset_field_properties(self, field, field_dt, sla_field):
-		field = frappe.get_doc(field_dt, {"fieldname": field.fieldname})
+		field = frappe.get_doc(field_dt, field.name)
 		field.label = sla_field.get("label")
 		field.fieldname = sla_field.get("fieldname")
 		field.fieldtype = sla_field.get("fieldtype")
@@ -320,6 +322,7 @@ class ServiceLevelAgreement(Document):
 		field.hidden = sla_field.get("hidden")
 		field.description = sla_field.get("description")
 		field.default = sla_field.get("default")
+		field.link_filters = sla_field.get("link_filters")
 		field.save(ignore_permissions=True)
 
 
@@ -909,7 +912,7 @@ def record_assigned_users_on_failure(doc):
 		doc.add_comment(comment_type="Assigned", text=message)
 
 
-def get_service_level_agreement_fields():
+def get_service_level_agreement_fields(doctype: str):
 	return [
 		{
 			"collapsible": 1,
@@ -922,6 +925,9 @@ def get_service_level_agreement_fields():
 			"fieldtype": "Link",
 			"label": "Service Level Agreement",
 			"options": "Service Level Agreement",
+			"link_filters": frappe.as_json(
+				[["Service Level Agreement", "document_type", "=", doctype]], indent=None
+			),
 		},
 		{"fieldname": "priority", "fieldtype": "Link", "label": "Priority", "options": "Issue Priority"},
 		{"fieldname": "response_by", "fieldtype": "Datetime", "label": "Response By", "read_only": 1},

erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py

@@ -2,6 +2,7 @@
 # See license.txt
 
 import datetime
+import json
 import unittest
 
 import frappe
@@ -176,11 +177,14 @@ class TestServiceLevelAgreement(unittest.TestCase):
 		self.assertEqual(lead_sla.name, default_sla.name)
 
 		# check SLA custom fields created for leads
-		sla_fields = get_service_level_agreement_fields()
+		sla_fields = get_service_level_agreement_fields(doctype)
 
 		for field in sla_fields:
-			self.assertTrue(
+			filters = {"dt": doctype, "fieldname": field.get("fieldname")}
-				frappe.db.exists("Custom Field", {"dt": doctype, "fieldname": field.get("fieldname")})
+			self.assertTrue(frappe.db.exists("Custom Field", filters))
+			self.assertEqual(
+				get_link_filters("Custom Field", filters),
+				json.loads(field["link_filters"]) if field.get("link_filters") else None,
 			)
 
 	def test_docfield_creation_for_sla_on_custom_dt(self):
@@ -200,13 +204,66 @@ class TestServiceLevelAgreement(unittest.TestCase):
 		self.assertEqual(sla.name, default_sla.name)
 
 		# check SLA docfields created
-		sla_fields = get_service_level_agreement_fields()
+		sla_fields = get_service_level_agreement_fields(doctype.name)
 
 		for field in sla_fields:
-			self.assertTrue(
+			filters = {"fieldname": field.get("fieldname"), "parent": doctype.name}
-				frappe.db.exists("DocField", {"fieldname": field.get("fieldname"), "parent": doctype.name})
+			self.assertTrue(frappe.db.exists("DocField", filters))
+			self.assertEqual(
+				get_link_filters("DocField", filters),
+				json.loads(field["link_filters"]) if field.get("link_filters") else None,
 			)
 
+	def test_reset_field_properties_does_not_clobber_other_doctypes_field(self):
+		"""Two doctypes each get their own "service_level_agreement" custom field
+		(same fieldname, different owning doctype). Updating the field on one of
+		them must not clobber the other's, even though both share the fieldname
+		(regression test for the fix in reset_field_properties, see PR #56954)."""
+		doctype_a = create_custom_doctype("Test SLA Dt A")
+		doctype_b = create_custom_doctype("Test SLA Dt B")
+
+		for doctype in (doctype_a.name, doctype_b.name):
+			create_service_level_agreement(
+				default_service_level_agreement=1,
+				holiday_list="__Test Holiday List",
+				entity_type=None,
+				entity=None,
+				response_time=14400,
+				resolution_time=21600,
+				doctype=doctype,
+			)
+
+		def get_sla_field_link_filters(doctype):
+			return get_link_filters("DocField", {"parent": doctype, "fieldname": "service_level_agreement"})
+
+		self.assertEqual(
+			get_sla_field_link_filters(doctype_a.name),
+			[["Service Level Agreement", "document_type", "=", doctype_a.name]],
+		)
+
+		# The field on doctype_b already exists, so creating another, entity-specific
+		# SLA for doctype_b takes the "update existing field" branch (reset_field_properties)
+		# instead of creating a new field.
+		customer = create_customer()
+		create_service_level_agreement(
+			default_service_level_agreement=0,
+			holiday_list="__Test Holiday List",
+			entity_type="Customer",
+			entity=customer,
+			response_time=7200,
+			resolution_time=10800,
+			doctype=doctype_b.name,
+		)
+
+		self.assertEqual(
+			get_sla_field_link_filters(doctype_a.name),
+			[["Service Level Agreement", "document_type", "=", doctype_a.name]],
+		)
+		self.assertEqual(
+			get_sla_field_link_filters(doctype_b.name),
+			[["Service Level Agreement", "document_type", "=", doctype_b.name]],
+		)
+
 	def test_sla_application(self):
 		# Default Service Level Agreement
 		doctype = "Lead"
@@ -362,6 +419,11 @@ class TestServiceLevelAgreement(unittest.TestCase):
 			frappe.delete_doc("Service Level Agreement", d.name, force=1)
 
 
+def get_link_filters(field_doctype, filters):
+	value = frappe.db.get_value(field_doctype, filters, "link_filters")
+	return json.loads(value) if value else None
+
+
 def get_service_level_agreement(
 	default_service_level_agreement=None, entity_type=None, entity=None, doctype="Issue"
 ):
@@ -602,8 +664,8 @@ def make_holiday_list():
 		).insert()
 
 
-def create_custom_doctype():
+def create_custom_doctype(name="Test SLA on Custom Dt"):
-	if not frappe.db.exists("DocType", "Test SLA on Custom Dt"):
+	if not frappe.db.exists("DocType", name):
 		doc = frappe.get_doc(
 			{
 				"doctype": "DocType",
@@ -626,13 +688,13 @@ def create_custom_doctype():
 					},
 				],
 				"permissions": [{"role": "System Manager", "read": 1, "write": 1}],
-				"name": "Test SLA on Custom Dt",
+				"name": name,
 			}
 		)
 		doc.insert()
 		return doc
 	else:
-		return frappe.get_doc("DocType", "Test SLA on Custom Dt")
+		return frappe.get_doc("DocType", name)
 
 
 def make_lead(creation=None, index=0, company=None):