fix: incorrect response by variance & resolution by variance (#26173)

erpnext/patches.txt

@@ -290,5 +290,6 @@ erpnext.patches.v13_0.add_doctype_to_sla #14-06-2021
 erpnext.patches.v13_0.set_training_event_attendance
 erpnext.patches.v13_0.bill_for_rejected_quantity_in_purchase_invoice
 erpnext.patches.v13_0.rename_issue_status_hold_to_on_hold
+erpnext.patches.v13_0.update_response_by_variance
 erpnext.patches.v13_0.bill_for_rejected_quantity_in_purchase_invoice
 erpnext.patches.v13_0.update_job_card_details

erpnext/patches/v13_0/update_response_by_variance.py

@@ -0,0 +1,31 @@
+# Copyright (c) 2020, Frappe and Contributors
+# License: GNU General Public License v3. See license.txt
+
+from __future__ import unicode_literals
+import frappe
+
+def execute():
+	if frappe.db.exists('DocType', 'Issue') and frappe.db.count('Issue'):
+		invalid_issues = frappe.get_all('Issue', {
+			'first_responded_on': ['is', 'set'],
+			'response_by_variance': ['<', 0]
+		}, ["name", "response_by_variance", "timestampdiff(Second, `first_responded_on`, `response_by`) as variance"])
+
+		# issues which has response_by_variance set as -ve
+		# but diff between first_responded_on & response_by is +ve i.e SLA isn't failed
+		invalid_issues = [d for d in invalid_issues if d.get('variance') > 0]
+
+		for issue in invalid_issues:
+			frappe.db.set_value('Issue', issue.get('name'), 'response_by_variance', issue.get('variance'), update_modified=False)
+
+		invalid_issues = frappe.get_all('Issue', {
+			'resolution_date': ['is', 'set'],
+			'resolution_by_variance': ['<', 0]
+		}, ["name", "resolution_by_variance", "timestampdiff(Second, `resolution_date`, `resolution_by`) as variance"])
+
+		# issues which has resolution_by_variance set as -ve
+		# but diff between resolution_date & resolution_by is +ve i.e SLA isn't failed
+		invalid_issues = [d for d in invalid_issues if d.get('variance') > 0]
+
+		for issue in invalid_issues:
+			frappe.db.set_value('Issue', issue.get('name'), 'resolution_by_variance', issue.get('variance'), update_modified=False)

erpnext/support/doctype/service_level_agreement/service_level_agreement.js

@@ -5,15 +5,15 @@ frappe.ui.form.on('Service Level Agreement', {
 	setup: function(frm) {
 		if (cint(frm.doc.apply_sla_for_resolution) === 1) {
 			frm.get_field('priorities').grid.editable_fields = [
-				{fieldname: 'priority', columns: 1},
 				{fieldname: 'default_priority', columns: 1},
+				{fieldname: 'priority', columns: 2},
 				{fieldname: 'response_time', columns: 2},
 				{fieldname: 'resolution_time', columns: 2}
 			];
 		} else {
 			frm.get_field('priorities').grid.editable_fields = [
-				{fieldname: 'priority', columns: 1},
 				{fieldname: 'default_priority', columns: 1},
+				{fieldname: 'priority', columns: 2},
 				{fieldname: 'response_time', columns: 3},
 			];
 		}

erpnext/support/doctype/service_level_agreement/service_level_agreement.json

@@ -1,6 +1,6 @@
 {
  "actions": [],
- "autoname": "format:SLA-{document_type}-{service_level}-{####}",
+ "autoname": "format:SLA-{document_type}-{service_level}",
  "creation": "2018-12-26 21:08:15.448812",
  "doctype": "DocType",
  "editable_grid": 1,

erpnext/support/doctype/service_level_agreement/service_level_agreement.py

@@ -797,7 +797,7 @@ def set_response_by_and_variance(doc, meta, start_date_time, priority):
 	if meta.has_field("response_by"):
 		doc.response_by = get_expected_time_for(parameter="response", service_level=priority, start_date_time=start_date_time)
 
-	if meta.has_field("response_by_variance"):
+	if meta.has_field("response_by_variance") and not doc.get('first_responded_on'):
 		now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
 		doc.response_by_variance = round(time_diff_in_seconds(doc.response_by, now_time), 2)
 
@@ -805,7 +805,7 @@ def set_resolution_by_and_variance(doc, meta, start_date_time, priority):
 	if meta.has_field("resolution_by"):
 		doc.resolution_by = get_expected_time_for(parameter="resolution", service_level=priority, start_date_time=start_date_time)
 
-	if meta.has_field("resolution_by_variance"):
+	if meta.has_field("resolution_by_variance") and not doc.get("resolution_date"):
 		now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
 		doc.resolution_by_variance = round(time_diff_in_seconds(doc.resolution_by, now_time), 2)
 

erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py

@@ -217,6 +217,42 @@ class TestServiceLevelAgreement(unittest.TestCase):
 		lead.reload()
 		self.assertEqual(lead.agreement_status, 'Fulfilled')
 
+	def test_changing_of_variance_after_response(self):
+		# create lead
+		doctype = "Lead"
+		lead_sla = create_service_level_agreement(
+			default_service_level_agreement=1,
+			holiday_list="__Test Holiday List",
+			entity_type=None, entity=None,
+			response_time=14400,
+			doctype=doctype,
+			sla_fulfilled_on=[{"status": "Replied"}],
+			apply_sla_for_resolution=0
+		)
+		creation = datetime.datetime(2019, 3, 4, 12, 0)
+		lead = make_lead(creation=creation, index=2)
+		self.assertEqual(lead.service_level_agreement, lead_sla.name)
+
+		# set lead as replied to set first responded on
+		frappe.flags.current_time = datetime.datetime(2019, 3, 4, 15, 30)
+		lead.reload()
+		lead.status = 'Replied'
+		lead.save()
+		lead.reload()
+		self.assertEqual(lead.agreement_status, 'Fulfilled')
+
+		# check response_by_variance
+		self.assertEqual(lead.first_responded_on, frappe.flags.current_time)
+		self.assertEqual(lead.response_by_variance, 1800.0)
+
+		# make a change on the document &
+		# check response_by_variance is unchanged
+		frappe.flags.current_time = datetime.datetime(2019, 3, 4, 18, 30)
+		lead.status = 'Open'
+		lead.save()
+		lead.reload()
+		self.assertEqual(lead.response_by_variance, 1800.0)
+
 	def tearDown(self):
 		for d in frappe.get_all("Service Level Agreement"):
 			frappe.delete_doc("Service Level Agreement", d.name, force=1)
@@ -249,7 +285,7 @@ def create_service_level_agreement(default_service_level_agreement, holiday_list
 		"doctype": "Service Level Agreement",
 		"enabled": 1,
 		"document_type": doctype,
-		"service_level": "__Test Service Level",
+		"service_level": "__Test {} SLA".format(entity_type if entity_type else "Default"),
 		"default_service_level_agreement": default_service_level_agreement,
 		"default_priority": "Medium",
 		"holiday_list": holiday_list,

erpnext/support/doctype/service_level_priority/service_level_priority.json

@@ -5,9 +5,9 @@
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
-  "priority",
-  "cb_01",
   "default_priority",
+  "cb_01",
+  "priority",
   "sb_00",
   "response_time",
   "cb_00",
@@ -15,7 +15,7 @@
  ],
  "fields": [
   {
-   "columns": 1,
+   "columns": 2,
    "fieldname": "priority",
    "fieldtype": "Link",
    "in_list_view": 1,
@@ -64,7 +64,7 @@
  ],
  "istable": 1,
  "links": [],
- "modified": "2021-05-29 19:52:51.733248",
+ "modified": "2021-06-21 12:00:58.089962",
  "modified_by": "Administrator",
  "module": "Support",
  "name": "Service Level Priority",