fix: review changes

erpnext/patches/v12_0/set_priority_for_support.py

@@ -10,6 +10,7 @@ def execute():
 	set_priorities_service_level_agreement()
 
 def set_issue_priority():
+	# Adds priority from issue to Issue Priority DocType as Priority is a new DocType.
 	for priority in frappe.get_meta("Issue").get_field("priority").options.split("\n"):
 		if not frappe.db.exists("Issue Priority", priority):
 			frappe.get_doc({
@@ -18,6 +19,7 @@ def set_issue_priority():
 			}).insert(ignore_permissions=True)
 
 def set_priority_for_issue():
+	# Sets priority for Issues as Select field is changed to Link field.
 	issue_priority = frappe.get_list("Issue", fields=["name", "priority"])
 	frappe.reload_doc("support", "doctype", "issue")
 
@@ -25,6 +27,8 @@ def set_priority_for_issue():
 		frappe.db.set_value("Issue", issue.name, "priority", issue.priority)
 
 def set_priorities_service_level():
+	# Migrates "priority", "response_time", "response_time_period", "resolution_time", "resolution_time_period" to Child Table
+	# as a Service Level can have multiple priorities
 	try:
 		service_level_priorities = frappe.get_list("Service Level", fields=["name", "priority", "response_time", "response_time_period", "resolution_time", "resolution_time_period"])
 
@@ -45,6 +49,8 @@ def set_priorities_service_level():
 		frappe.reload_doc("support", "doctype", "service_level")
 
 def set_priorities_service_level_agreement():
+	# Migrates "priority", "response_time", "response_time_period", "resolution_time", "resolution_time_period" to Child Table
+	# as a Service Level Agreement can have multiple priorities
 	try:
 		service_level_agreement_priorities = frappe.get_list("Service Level Agreement", fields=["name", "priority", "response_time", "response_time_period", "resolution_time", "resolution_time_period"])
 

erpnext/support/doctype/issue/issue.js

@@ -4,34 +4,24 @@ frappe.ui.form.on("Issue", {
 
 		if (frm.doc.service_level_agreement) {
 			frappe.call({
-				method: "erpnext.support.doctype.service_level_agreement.service_level_agreement.get_service_level_agreement_priorities",
+				method: "erpnext.support.doctype.service_level_agreement.service_level_agreement.get_service_level_agreement_filters",
 				args: {
 					name: frm.doc.service_level_agreement,
+					customer: frm.doc.customer
 				},
 				callback: function (r) {
 					if (r && r.message) {
 						frm.set_query('priority', function() {
 							return {
 								filters: {
-									"name": ["in", r.message],
+									"name": ["in", r.message.priority],
 								}
 							};
 						});
-					}
-				}
-			});
-
-			frappe.call({
-				method: "erpnext.support.doctype.service_level_agreement.service_level_agreement.get_valid_service_level_agreements",
-				args: {
-					customer: frm.doc.customer,
-				},
-				callback: function (r) {
-					if (r && r.message) {
 						frm.set_query('service_level_agreement', function() {
 							return {
 								filters: {
-									"name": ["in", r.message],
+									"name": ["in", r.message.service_level_agreements],
 								}
 							};
 						});
@@ -85,10 +75,6 @@ frappe.ui.form.on("Issue", {
 
 	priority: function(frm) {
 		if (frm.doc.service_level_agreement) {
-			frappe.show_alert({
-				indicator: 'green',
-				message: __('Changing Priority.')
-			});
 			frm.call('change_service_level_agreement_and_priority', {
 				"priority": frm.doc.priority,
 				"service_level_agreement": frm.doc.service_level_agreement
@@ -100,10 +86,6 @@ frappe.ui.form.on("Issue", {
 	},
 
 	service_level_agreement: function(frm) {
-		frappe.show_alert({
-			indicator: 'green',
-			message: __('Changing Service Level Agreement.')
-		});
 		frm.call('change_service_level_agreement_and_priority', {
 			"service_level_agreement": frm.doc.service_level_agreement
 		}).then(() => {

erpnext/support/doctype/issue/test_issue.py

@@ -21,62 +21,50 @@ class TestIssue(unittest.TestCase):
 		create_service_level_agreements_for_issues()
 
 		creation = datetime.datetime(2019, 3, 4, 12, 0)
-		"""
+
-			make issue with customer specific SLA
+		# make issue with customer specific SLA
-		"""
 		customer = create_customer("_Test Customer", "__Test SLA Customer Group", "__Test SLA Territory")
 		issue = make_issue(creation, "_Test Customer", 1)
 
 		self.assertEquals(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
 		self.assertEquals(issue.resolution_by, datetime.datetime(2019, 3, 4, 15, 0))
 
-		"""
+		# make issue with customer_group specific SLA
-			make issue with customer_group specific SLA
-		"""
 		customer = create_customer("__Test Customer", "_Test SLA Customer Group", "__Test SLA Territory")
 		issue = make_issue(creation, "__Test Customer", 2)
 
 		self.assertEquals(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
 		self.assertEquals(issue.resolution_by, datetime.datetime(2019, 3, 4, 15, 0))
 
-		"""
+
-			make issue with territory specific SLA
+		# make issue with territory specific SLA
-		"""
 		customer = create_customer("___Test Customer", "__Test SLA Customer Group", "_Test SLA Territory")
 		issue = make_issue(creation, "___Test Customer", 3)
 
 		self.assertEquals(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
 		self.assertEquals(issue.resolution_by, datetime.datetime(2019, 3, 4, 15, 0))
 
-		"""
+		# make issue with default SLA
-			make issue with default SLA
-		"""
 		issue = make_issue(creation=creation, index=4)
 
 		self.assertEquals(issue.response_by, datetime.datetime(2019, 3, 4, 16, 0))
 		self.assertEquals(issue.resolution_by, datetime.datetime(2019, 3, 4, 18, 0))
 
-		"""
+		# make issue with default SLA before working hours
-			make issue with default SLA before working hours
-		"""
 		creation = datetime.datetime(2019, 3, 4, 7, 0)
 		issue = make_issue(creation=creation, index=5)
 
 		self.assertEquals(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
 		self.assertEquals(issue.resolution_by, datetime.datetime(2019, 3, 4, 16, 0))
 
-		"""
+		# make issue with default SLA after working hours
-			make issue with default SLA after working hours
-		"""
 		creation = datetime.datetime(2019, 3, 4, 20, 0)
 		issue = make_issue(creation, index=6)
 
 		self.assertEquals(issue.response_by, datetime.datetime(2019, 3, 6, 14, 0))
 		self.assertEquals(issue.resolution_by, datetime.datetime(2019, 3, 6, 16, 0))
 
-		"""
+		# make issue with default SLA next day
-			make issue with default SLA next day
-		"""
 		creation = datetime.datetime(2019, 3, 4, 14, 0)
 		issue = make_issue(creation=creation, index=7)
 

erpnext/support/doctype/service_level_agreement/service_level_agreement.py

@@ -70,12 +70,16 @@ def get_active_service_level_agreement_for(priority, customer=None, service_leve
 
 	return agreement[0] if agreement else None
 
-@frappe.whitelist()
+def get_customer_group(customer):
-def get_service_level_agreement_priorities(name):
+	if customer:
-	return [priority.priority for priority in frappe.get_list("Service Level Priority", filters={"parent": name}, fields=["priority"])]
+		return frappe.db.get_value("Customer", customer, "customer_group")
+
+def get_customer_territory(customer):
+	if customer:
+		return frappe.db.get_value("Customer", customer, "territory")
 
 @frappe.whitelist()
-def get_valid_service_level_agreements(customer=None):
+def get_service_level_agreement_filters(name, customer=None):
 	if not customer:
 		or_filters = [
 			["Service Level Agreement", "default_service_level_agreement", "=", 1]
@@ -86,12 +90,7 @@ def get_valid_service_level_agreements(customer=None):
 			["Service Level Agreement", "default_service_level_agreement", "=", 1]
 		]
 
-	return [d.name for d in frappe.get_list("Service Level Agreement", or_filters=or_filters)]
+	return {
-
+		"priority": [priority.priority for priority in frappe.get_list("Service Level Priority", filters={"parent": name}, fields=["priority"])],
-def get_customer_group(customer):
+		"service_level_agreements": [d.name for d in frappe.get_list("Service Level Agreement", or_filters=or_filters)]
-	if customer:
+	}
-		return frappe.db.get_value("Customer", customer, "customer_group")
-
-def get_customer_territory(customer):
-	if customer:
-		return frappe.db.get_value("Customer", customer, "territory")