refactor(SLA): remove response_by_variance & resolution_by_variance

erpnext/support/doctype/issue/issue.json

@@ -24,12 +24,10 @@
   "service_level_section",
   "service_level_agreement",
   "response_by",
-  "response_by_variance",
   "reset_service_level_agreement",
   "cb",
   "agreement_status",
   "resolution_by",
-  "resolution_by_variance",
   "service_level_agreement_creation",
   "on_hold_since",
   "total_hold_time",
@@ -44,8 +42,6 @@
   "opening_date",
   "opening_time",
   "resolution_date",
-  "resolution_time",
-  "user_resolution_time",
   "additional_info",
   "lead",
   "contact",
@@ -317,22 +313,6 @@
    "fieldtype": "Check",
    "label": "Via Customer Portal"
   },
-  {
-   "depends_on": "eval: doc.service_level_agreement && doc.status != 'Replied';",
-   "fieldname": "response_by_variance",
-   "fieldtype": "Duration",
-   "hide_seconds": 1,
-   "label": "Response By Variance",
-   "read_only": 1
-  },
-  {
-   "depends_on": "eval: doc.service_level_agreement && doc.status != 'Replied';",
-   "fieldname": "resolution_by_variance",
-   "fieldtype": "Duration",
-   "hide_seconds": 1,
-   "label": "Resolution By Variance",
-   "read_only": 1
-  },
   {
    "fieldname": "service_level_agreement_creation",
    "fieldtype": "Datetime",
@@ -395,7 +375,7 @@
    "fieldname": "agreement_status",
    "fieldtype": "Select",
    "label": "Service Level Agreement Status",
-   "options": "Ongoing\nFulfilled\nFailed",
+   "options": "First Response Due\nResolution Due\nFulfilled\nFailed",
    "read_only": 1
   },
   {

erpnext/support/doctype/issue/issue.py

@@ -87,11 +87,9 @@ class Issue(Document):
 		if replicated_issue.service_level_agreement:
 			replicated_issue.service_level_agreement_creation = now_datetime()
 			replicated_issue.service_level_agreement = None
-			replicated_issue.agreement_status = "Ongoing"
+			replicated_issue.agreement_status = "First Response Due"
 			replicated_issue.response_by = None
-			replicated_issue.response_by_variance = None
 			replicated_issue.resolution_by = None
-			replicated_issue.resolution_by_variance = None
 			replicated_issue.reset_issue_metrics()
 
 		frappe.get_doc(replicated_issue).insert()

erpnext/support/doctype/issue/test_issue.py

@@ -83,30 +83,6 @@ class TestIssue(TestSetUp):
 
 		self.assertEqual(issue.agreement_status, 'Fulfilled')
 
-	def test_issue_metrics(self):
-		creation = get_datetime("2020-03-04 4:00")
-
-		issue = make_issue(creation, index=1)
-		create_communication(issue.name, "test@example.com", "Received", creation)
-
-		creation = get_datetime("2020-03-04 4:15")
-		create_communication(issue.name, "test@admin.com", "Sent", creation)
-
-		creation = get_datetime("2020-03-04 5:00")
-		create_communication(issue.name, "test@example.com", "Received", creation)
-
-		creation = get_datetime("2020-03-04 5:05")
-		create_communication(issue.name, "test@admin.com", "Sent", creation)
-
-		frappe.flags.current_time = get_datetime("2020-03-04 5:05")
-		issue.reload()
-		issue.status = 'Closed'
-		issue.save()
-
-		self.assertEqual(issue.avg_response_time, 600)
-		self.assertEqual(issue.resolution_time, 3900)
-		self.assertEqual(issue.user_resolution_time, 1200)
-
 	def test_hold_time_on_replied(self):
 		creation = get_datetime("2020-03-04 4:00")
 
@@ -143,16 +119,15 @@ class TestIssue(TestSetUp):
 		self.assertEqual(flt(issue.total_hold_time, 2), 2700)
 
 	def test_issue_close_after_on_hold(self):
-		creation = get_datetime("2021-11-01 19:00")
+		frappe.flags.current_time = get_datetime("2021-11-01 19:00")
 
-		issue = make_issue(creation, index=1)
+		issue = make_issue(frappe.flags.current_time, index=1)
-		create_communication(issue.name, "test@example.com", "Received", creation)
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
 
 		# send a reply within SLA
-		creation = get_datetime("2021-11-02 11:00")
+		frappe.flags.current_time = get_datetime("2021-11-02 11:00")
-		create_communication(issue.name, "test@admin.com", "Sent", creation)
+		create_communication(issue.name, "test@admin.com", "Sent", frappe.flags.current_time)
 
-		frappe.flags.current_time = creation
 		issue.reload()
 		issue.status = 'Replied'
 		issue.save()
@@ -168,6 +143,75 @@ class TestIssue(TestSetUp):
 		self.assertEqual(issue.resolution_date, get_datetime('2021-11-22 01:00:00'))
 		self.assertEqual(issue.agreement_status, 'Fulfilled')
 
+	def test_issue_open_after_closed(self):
+
+		# Created on -> 1 pm, Response Time -> 4 hrs, Resolution Time -> 6 hrs
+		frappe.flags.current_time = get_datetime("2021-11-01 13:00")
+		issue = make_issue(frappe.flags.current_time, index=1, issue_type='Critical') # Applies 24hr working time SLA
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
+		self.assertEquals(issue.agreement_status, 'First Response Due')
+		self.assertEquals(issue.response_by, get_datetime("2021-11-01 17:00"))
+		self.assertEquals(issue.resolution_by, get_datetime("2021-11-01 19:00"))
+
+		# Replied on → 2 pm
+		frappe.flags.current_time = get_datetime("2021-11-01 14:00")
+		create_communication(issue.name, "test@admin.com", "Sent", frappe.flags.current_time)
+		issue.reload()
+		issue.status = 'Replied'
+		issue.save()
+		self.assertEquals(issue.agreement_status, 'Resolution Due')
+		self.assertEquals(issue.on_hold_since, frappe.flags.current_time)
+		self.assertEquals(issue.first_responded_on, frappe.flags.current_time)
+
+		# Customer Replied → 3 pm
+		frappe.flags.current_time = get_datetime("2021-11-01 15:00")
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
+		issue.reload()
+		self.assertEquals(issue.status, 'Open')
+		# Hold Time + 1 Hrs
+		self.assertEquals(issue.total_hold_time, 3600)
+		# Resolution By should increase by one hrs
+		self.assertEquals(issue.resolution_by, get_datetime("2021-11-01 20:00"))
+
+		# Replied on → 4 pm, Open → 1 hr, Resolution Due → 8 pm
+		frappe.flags.current_time = get_datetime("2021-11-01 16:00")
+		create_communication(issue.name, "test@admin.com", "Sent", frappe.flags.current_time)
+		issue.reload()
+		issue.status = 'Replied'
+		issue.save()
+		self.assertEquals(issue.agreement_status, 'Resolution Due')
+
+		# Customer Closed → 10 pm
+		frappe.flags.current_time = get_datetime("2021-11-01 22:00")
+		issue.status = 'Closed'
+		issue.save()
+		# Hold Time + 6 Hrs
+		self.assertEquals(issue.total_hold_time, 3600 + 21600)
+		# Resolution By should increase by 6 hrs
+		self.assertEquals(issue.resolution_by, get_datetime("2021-11-02 02:00"))
+		self.assertEquals(issue.agreement_status, 'Fulfilled')
+		self.assertEquals(issue.resolution_date, frappe.flags.current_time)
+
+		# Customer Open → 3 am i.e after resolution by is crossed
+		frappe.flags.current_time = get_datetime("2021-11-02 03:00")
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
+		issue.reload()
+		# Since issue was Resolved, Resolution By should be increased by 5 hrs (3am - 10pm)
+		self.assertEquals(issue.total_hold_time, 3600 + 21600 + 18000)
+		# Resolution By should increase by 5 hrs
+		self.assertEquals(issue.resolution_by, get_datetime("2021-11-02 07:00"))
+		self.assertEquals(issue.agreement_status, 'Resolution Due')
+		self.assertFalse(issue.resolution_date)
+
+		# We Closed → 4 am, SLA should be Fulfilled
+		frappe.flags.current_time = get_datetime("2021-11-02 04:00")
+		issue.status = 'Closed'
+		issue.save()
+		self.assertEquals(issue.resolution_by, get_datetime("2021-11-02 07:00"))
+		self.assertEquals(issue.agreement_status, 'Fulfilled')
+		self.assertEquals(issue.resolution_date, frappe.flags.current_time)
+
+
 class TestFirstResponseTime(TestSetUp):
 	# working hours used in all cases: Mon-Fri, 10am to 6pm
 	# all dates are in the mm-dd-yyyy format
@@ -386,7 +430,10 @@ def create_issue_and_communication(issue_creation, first_responded_on):
 
 	return issue
 
-def make_issue(creation=None, customer=None, index=0, priority=None, issue_type=None):
+def make_issue(creation=None, customer=None, index=0, priority=None, issue_type=None, do_not_insert=False):
+	if not frappe.db.exists('Issue Type', issue_type):
+		frappe.get_doc(dict(doctype='Issue Type', name=issue_type)).insert()
+
 	issue = frappe.get_doc({
 		"doctype": "Issue",
 		"subject": "Service Level Agreement Issue {0}".format(index),

erpnext/support/doctype/service_level_agreement/service_level_agreement.py

@@ -358,36 +358,102 @@ def apply(doc, method=None):
 	):
 		return
 
-	service_level_agreement = get_active_service_level_agreement_for(doc)
+	sla = get_active_service_level_agreement_for(doc)
 
-	if not service_level_agreement:
+	if not sla:
 		return
 
-	process_sla(doc, service_level_agreement)
+	process_sla(doc, sla)
 
 
-def process_sla(doc, service_level_agreement):
+def process_sla(doc, sla):
 
 	if not doc.creation:
 		doc.creation = now_datetime(doc.get("owner"))
 		if doc.meta.has_field("service_level_agreement_creation"):
 			doc.service_level_agreement_creation = now_datetime(doc.get("owner"))
 
-	doc.service_level_agreement = service_level_agreement.name
+	doc.service_level_agreement = sla.name
-	doc.priority = doc.get("priority") or service_level_agreement.default_priority
+	doc.priority = doc.get("priority") or sla.default_priority
 
 	prev_status = frappe.db.get_value(doc.doctype, doc.name, 'status')
-	handle_status_change(doc, prev_status, service_level_agreement.apply_sla_for_resolution)
+	handle_status_change(doc, prev_status, sla.apply_sla_for_resolution)
-	update_response_and_resolution_metrics(doc, service_level_agreement.apply_sla_for_resolution)
+	update_response_and_resolution_metrics(doc, sla.apply_sla_for_resolution)
-	update_agreement_status(doc, service_level_agreement.apply_sla_for_resolution)
+	update_agreement_status(doc, sla.apply_sla_for_resolution)
 
 
-def update_response_and_resolution_metrics(doc, apply_sla_for_resolution):
+def handle_status_change(doc, prev_status, apply_sla_for_resolution):
-	priority = get_response_and_resolution_duration(doc)
+	now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
-	start_date_time = get_datetime(doc.get("service_level_agreement_creation") or doc.creation)
+
-	set_response_by_and_variance(doc, start_date_time, priority)
+	hold_statuses = get_hold_statuses(doc.service_level_agreement)
+	fulfillment_statuses = get_fulfillment_statuses(doc.service_level_agreement)
+
+	def is_hold_status(status):
+		return status in hold_statuses
+
+	def is_fulfilled_status(status):
+		return status in fulfillment_statuses
+
+	def is_open_status(status):
+		return status not in hold_statuses and status not in fulfillment_statuses
+
+	def calculate_hold_hours():
+		# In case issue was closed and after few days it has been opened
+		# The hold time should be calculated from resolution_date
+
+		on_hold_since = doc.resolution_date or doc.on_hold_since
+		if on_hold_since:
+			current_hold_hours = time_diff_in_seconds(now_time, on_hold_since)
+			doc.total_hold_time = (doc.total_hold_time or 0) + current_hold_hours
+		doc.on_hold_since = None
+
+	if is_open_status(prev_status) and not is_open_status(doc.status):
+		# status changed from Open to something else
+		if doc.meta.has_field("first_responded_on") and not doc.first_responded_on:
+			doc.first_responded_on = now_time
+
+	# Open to Replied
+	if is_open_status(prev_status) and is_hold_status(doc.status):
+		# Issue is on hold -> Set on_hold_since
+		doc.on_hold_since = now_time
+
+	# Replied to Open
+	if is_hold_status(prev_status) and is_open_status(doc.status):
+		# Issue was on hold -> Calculate Total Hold Time
+		calculate_hold_hours()
+		# Issue is open -> reset resolution_date
+		reset_expected_response_and_resolution(doc)
+		reset_resolution_metrics(doc)
+
+	# Open to Closed
+	if is_open_status(prev_status) and is_fulfilled_status(doc.status):
+		# Issue is closed -> Set resolution_date
+		doc.resolution_date = now_time
+		set_resolution_time(doc)
+
+	# Closed to Open
+	if is_fulfilled_status(prev_status) and is_open_status(doc.status):
+		# Issue was closed -> Calculate Total Hold Time from resolution_date
+		calculate_hold_hours()
+		# Issue is open -> reset resolution_date
+		reset_expected_response_and_resolution(doc)
+		reset_resolution_metrics(doc)
+
+	# Closed to Replied
+	if is_fulfilled_status(prev_status) and is_hold_status(doc.status):
+		# Issue was closed -> Calculate Total Hold Time from resolution_date
+		calculate_hold_hours()
+		# Issue is on hold -> Set on_hold_since
+		doc.on_hold_since = now_time
+
+	# Replied to Closed
+	if is_hold_status(prev_status) and is_fulfilled_status(doc.status):
+		# Issue was on hold -> Calculate Total Hold Time
+		calculate_hold_hours()
+		# Issue is closed -> Set resolution_date
 		if apply_sla_for_resolution:
-		set_resolution_by_and_variance(doc, start_date_time, priority)
+			doc.resolution_date = now_time
+			set_resolution_time(doc)
 
 
 def get_fulfillment_statuses(service_level_agreement):
@@ -402,60 +468,12 @@ def get_hold_statuses(service_level_agreement):
 	}, fields=["status"])]
 
 
-def handle_status_change(doc, prev_status, apply_sla_for_resolution):
+def update_response_and_resolution_metrics(doc, apply_sla_for_resolution):
-
+	priority = get_response_and_resolution_duration(doc)
-	if doc.status != "Open" and prev_status == "Open":
+	start_date_time = get_datetime(doc.get("service_level_agreement_creation") or doc.creation)
-		# status changed from Open to something else
+	set_response_by(doc, start_date_time, priority)
-		if doc.meta.has_field("first_responded_on") and not doc.first_responded_on:
+	if apply_sla_for_resolution:
-			# status changed to something other than Open
+		set_resolution_by(doc, start_date_time, priority)
-			doc.first_responded_on = frappe.flags.current_time or now_datetime(doc.get("owner"))
-
-	if doc.status == "Open" and prev_status != "Open":
-		# status changed from something else to Open
-		reset_resolution_metrics(doc)
-
-	handle_fulfillment_status(doc, prev_status, apply_sla_for_resolution)
-	handle_hold_status(doc, prev_status)
-
-
-def handle_fulfillment_status(doc, prev_status, apply_sla_for_resolution):
-	fulfillment_statuses = get_fulfillment_statuses(doc.service_level_agreement)
-	if (
-		doc.status in fulfillment_statuses
-		and prev_status not in fulfillment_statuses
-		and apply_sla_for_resolution
-	):
-		# status changed to any fulfillment_statuses
-		if doc.meta.has_field("resolution_date"):
-			doc.resolution_date = frappe.flags.current_time or now_datetime(doc.get("owner"))
-		if doc.meta.has_field("resolution_time"):
-			doc.resolution_time = time_diff_in_seconds(doc.resolution_date, doc.creation)
-		set_user_resolution_time(doc)
-
-
-def handle_hold_status(doc, prev_status):
-	hold_statuses = get_hold_statuses(doc.service_level_agreement)
-	if doc.status in hold_statuses:
-		# reset if status is a hold status, regardless of previous status
-		reset_expected_response_and_resolution(doc)
-		if prev_status not in hold_statuses:
-			# set on_hold_since status changed from any non-hold status
-			# for eg. doc.status changed from Open to Replied
-			if doc.meta.has_field("on_hold_since"):
-				doc.on_hold_since = frappe.flags.current_time or now_datetime(doc.get("owner"))
-
-	if doc.status not in hold_statuses and prev_status in hold_statuses:
-		# status changed to any non-hold status
-		# for eg. doc.status changed from Replied to Closed
-		if doc.meta.has_field("on_hold_since") and doc.on_hold_since:
-			cumulate_hold_time(doc)
-			doc.on_hold_since = None
-
-
-def cumulate_hold_time(doc):
-	now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
-	on_hold_duration = time_diff_in_seconds(now_time, doc.on_hold_since)
-	doc.total_hold_time = (doc.total_hold_time or 0) + on_hold_duration
 
 
 def get_expected_time_for(parameter, service_level, start_date_time):
@@ -526,7 +544,11 @@ def get_support_days(service_level):
 	return support_days
 
 
-def set_user_resolution_time(doc):
+def set_resolution_time(doc):
+	start_date_time = get_datetime(doc.get("service_level_agreement_creation") or doc.creation)
+	if doc.meta.has_field("resolution_time"):
+		doc.resolution_time = time_diff_in_seconds(doc.resolution_date, start_date_time)
+
 	# total time taken by a user to close the issue apart from wait_time
 	if not doc.meta.has_field("user_resolution_time"):
 		return
@@ -544,7 +566,7 @@ def set_user_resolution_time(doc):
 				pending_time.append(wait_time)
 
 	total_pending_time = sum(pending_time)
-	resolution_time_in_secs = time_diff_in_seconds(doc.resolution_date, doc.creation)
+	resolution_time_in_secs = time_diff_in_seconds(doc.resolution_date, start_date_time)
 	doc.user_resolution_time = resolution_time_in_secs - total_pending_time
 
 
@@ -562,11 +584,11 @@ def change_service_level_agreement_and_priority(self):
 
 
 def get_response_and_resolution_duration(doc):
-	service_level_agreement = frappe.get_doc("Service Level Agreement", doc.service_level_agreement)
+	sla = frappe.get_doc("Service Level Agreement", doc.service_level_agreement)
-	priority = service_level_agreement.get_service_level_agreement_priority(doc.priority)
+	priority = sla.get_service_level_agreement_priority(doc.priority)
 	priority.update({
-		"support_and_resolution": service_level_agreement.support_and_resolution,
+		"support_and_resolution": sla.support_and_resolution,
-		"holiday_list": service_level_agreement.holiday_list
+		"holiday_list": sla.holiday_list
 	})
 	return priority
 
@@ -585,8 +607,6 @@ def reset_service_level_agreement(doc, reason, user):
 	}).insert(ignore_permissions=True)
 
 	doc.service_level_agreement_creation = now_datetime(doc.get("owner"))
-	doc.set_response_and_resolution_time(priority=doc.priority, service_level_agreement=doc.service_level_agreement)
-	doc.agreement_status = "Ongoing"
 	doc.save()
 
 
@@ -616,56 +636,37 @@ def update_hold_time(doc, status):
 	if not parent.meta.has_field('service_level_agreement'):
 		return
 
-	apply_sla_for_resolution = frappe.db.get_value('Service Level Agreement', parent.service_level_agreement, 'apply_sla_for_resolution')
+	for_resolution = frappe.db.get_value('Service Level Agreement', parent.service_level_agreement, 'apply_sla_for_resolution')
 
-	handle_status_change(parent, 'Replied', apply_sla_for_resolution)
+	handle_status_change(parent, 'Replied', for_resolution)
-	update_response_and_resolution_metrics(parent, apply_sla_for_resolution)
+	update_response_and_resolution_metrics(parent, for_resolution)
-	update_agreement_status(parent, apply_sla_for_resolution)
+	update_agreement_status(parent, for_resolution)
 
 	parent.save()
 
 
 def reset_expected_response_and_resolution(doc):
 	update_values = {}
-
 	if doc.meta.has_field("first_responded_on") and not doc.first_responded_on:
 		update_values['response_by'] = None
-		update_values['response_by_variance'] = 0
-
 	if doc.meta.has_field("resolution_by") and not doc.resolution_date:
 		update_values['resolution_by'] = None
-		update_values['resolution_by_variance'] = 0
-
 	doc.db_set(update_values)
 
 
-def set_response_by_and_variance(doc, start_date_time, priority):
+def set_response_by(doc, start_date_time, priority):
 	if doc.meta.has_field("response_by"):
 		doc.response_by = get_expected_time_for(parameter="response", service_level=priority, start_date_time=start_date_time)
 		if doc.meta.has_field("total_hold_time") and doc.get('total_hold_time') and not doc.get('first_responded_on'):
 			doc.response_by = add_to_date(doc.response_by, seconds=round(doc.get('total_hold_time')))
 
-	if doc.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)
 
-	if doc.meta.has_field("response_by_variance") and doc.get('first_responded_on'):
+def set_resolution_by(doc, start_date_time, priority):
-		doc.response_by_variance = round(time_diff_in_seconds(doc.response_by, doc.get('first_responded_on')), 2)
-
-
-def set_resolution_by_and_variance(doc, start_date_time, priority):
 	if doc.meta.has_field("resolution_by"):
 		doc.resolution_by = get_expected_time_for(parameter="resolution", service_level=priority, start_date_time=start_date_time)
 		if doc.meta.has_field("total_hold_time") and doc.get('total_hold_time'):
 			doc.resolution_by = add_to_date(doc.resolution_by, seconds=round(doc.get('total_hold_time')))
 
-	if doc.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)
-
-	if doc.meta.has_field("resolution_by_variance") and doc.get('resolution_date'):
-		doc.resolution_by_variance = round(time_diff_in_seconds(doc.resolution_by, doc.get('resolution_date')), 2)
-
 
 def get_service_level_agreement_fields():
 	return [
@@ -693,17 +694,11 @@ def get_service_level_agreement_fields():
 			"label": "Response By",
 			"read_only": 1
 		},
-		{
-			"fieldname": "response_by_variance",
-			"fieldtype": "Duration",
-			"hide_seconds": 1,
-			"label": "Response By Variance",
-			"read_only": 1
-		},
 		{
 			"fieldname": "first_responded_on",
 			"fieldtype": "Datetime",
 			"label": "First Responded On",
+			"no_copy": 1,
 			"read_only": 1
 		},
 		{
@@ -725,11 +720,11 @@ def get_service_level_agreement_fields():
 			"read_only": 1
 		},
 		{
-			"default": "Ongoing",
+			"default": "First Response Due",
 			"fieldname": "agreement_status",
 			"fieldtype": "Select",
 			"label": "Service Level Agreement Status",
-			"options": "Ongoing\nFulfilled\nFailed",
+			"options": "First Response Due\nResolution Due\nFulfilled\nFailed",
 			"read_only": 1
 		},
 		{
@@ -738,13 +733,6 @@ def get_service_level_agreement_fields():
 			"label": "Resolution By",
 			"read_only": 1
 		},
-		{
-			"fieldname": "resolution_by_variance",
-			"fieldtype": "Duration",
-			"hide_seconds": 1,
-			"label": "Resolution By Variance",
-			"read_only": 1
-		},
 		{
 			"fieldname": "service_level_agreement_creation",
 			"fieldtype": "Datetime",
@@ -765,43 +753,28 @@ def get_service_level_agreement_fields():
 
 def update_agreement_status_on_custom_status(doc):
 	# Update Agreement Fulfilled status using Custom Scripts for Custom Status
-
+	update_agreement_status(doc)
-	meta = frappe.get_meta(doc.doctype)
-	now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
-	if doc.meta.has_field("first_responded_on") and not doc.first_responded_on:
-		# first_responded_on set when first reply is sent to customer
-		doc.response_by_variance = round(time_diff_in_seconds(doc.response_by, now_time), 2)
-
-	if doc.meta.has_field("first_responded_on") and doc.first_responded_on:
-		# first_responded_on set when first reply is sent to customer
-		doc.response_by_variance = round(time_diff_in_seconds(doc.response_by, doc.first_responded_on), 2)
-
-	if doc.meta.has_field("resolution_date") and not doc.resolution_date:
-		# resolution_date set when issue has been closed
-		doc.resolution_by_variance = round(time_diff_in_seconds(doc.resolution_by, now_time), 2)
-
-	if doc.meta.has_field("resolution_date") and doc.resolution_date:
-		# resolution_date set when issue has been closed
-		doc.resolution_by_variance = round(time_diff_in_seconds(doc.resolution_by, doc.resolution_date), 2)
-
-	if doc.meta.has_field("agreement_status"):
-		doc.agreement_status = "Fulfilled" if doc.response_by_variance > 0 and doc.resolution_by_variance > 0 else "Failed"
 
 
 def update_agreement_status(doc, apply_sla_for_resolution):
 	if (doc.meta.has_field("agreement_status")):
 		# if SLA is applied for resolution check for response and resolution, else only response
 		if apply_sla_for_resolution:
-			if doc.meta.has_field("response_by_variance") and doc.meta.has_field("resolution_by_variance"):
+			if not doc.first_responded_on:
-				if doc.response_by_variance < 0 or doc.resolution_by_variance < 0:
+				doc.agreement_status = "First Response Due"
-					doc.agreement_status = "Failed"
+			elif not doc.resolution_date:
-				else:
+				doc.agreement_status = "Resolution Due"
+			elif get_datetime(doc.resolution_date) <= get_datetime(doc.resolution_by):
 				doc.agreement_status = "Fulfilled"
 			else:
-			if doc.meta.has_field("response_by_variance") and doc.response_by_variance < 0:
 				doc.agreement_status = "Failed"
 		else:
+			if not doc.first_responded_on:
+				doc.agreement_status = "First Response Due"
+			elif get_datetime(doc.first_responded_on) <= get_datetime(doc.response_by):
 				doc.agreement_status = "Fulfilled"
+			else:
+				doc.agreement_status = "Failed"
 
 
 def is_holiday(date, holidays):

erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py

@@ -220,42 +220,6 @@ 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 test_service_level_agreement_filters(self):
 		doctype = "Lead"
 		lead_sla = create_service_level_agreement(
@@ -295,7 +259,8 @@ def get_service_level_agreement(default_service_level_agreement=None, entity_typ
 	return service_level_agreement
 
 def create_service_level_agreement(default_service_level_agreement, holiday_list, response_time, entity_type,
-	entity, resolution_time=0, doctype="Issue", condition="", sla_fulfilled_on=[], pause_sla_on=[], apply_sla_for_resolution=1):
+	entity, resolution_time=0, doctype="Issue", condition="", sla_fulfilled_on=[], pause_sla_on=[], apply_sla_for_resolution=1,
+	service_level=None, start_time="10:00:00", end_time="18:00:00"):
 
 	make_holiday_list()
 	make_priorities()
@@ -312,7 +277,7 @@ def create_service_level_agreement(default_service_level_agreement, holiday_list
 		"doctype": "Service Level Agreement",
 		"enabled": 1,
 		"document_type": doctype,
-		"service_level": "__Test {} SLA".format(entity_type if entity_type else "Default"),
+		"service_level": service_level or "__Test {} SLA".format(entity_type if entity_type else "Default"),
 		"default_service_level_agreement": default_service_level_agreement,
 		"condition": condition,
 		"default_priority": "Medium",
@@ -345,28 +310,28 @@ def create_service_level_agreement(default_service_level_agreement, holiday_list
 		"support_and_resolution": [
 			{
 				"workday": "Monday",
-				"start_time": "10:00:00",
+				"start_time": start_time,
-				"end_time": "18:00:00",
+				"end_time": end_time,
 			},
 			{
 				"workday": "Tuesday",
-				"start_time": "10:00:00",
+				"start_time": start_time,
-				"end_time": "18:00:00",
+				"end_time": end_time,
 			},
 			{
 				"workday": "Wednesday",
-				"start_time": "10:00:00",
+				"start_time": start_time,
-				"end_time": "18:00:00",
+				"end_time": end_time,
 			},
 			{
 				"workday": "Thursday",
-				"start_time": "10:00:00",
+				"start_time": start_time,
-				"end_time": "18:00:00",
+				"end_time": end_time,
 			},
 			{
 				"workday": "Friday",
-				"start_time": "10:00:00",
+				"start_time": start_time,
-				"end_time": "18:00:00",
+				"end_time": end_time,
 			}
 		]
 	})
@@ -443,6 +408,13 @@ def create_service_level_agreements_for_issues():
 	create_service_level_agreement(default_service_level_agreement=0, holiday_list="__Test Holiday List",
 		entity_type="Territory", entity="_Test SLA Territory", response_time=7200, resolution_time=10800)
 
+	create_service_level_agreement(
+		default_service_level_agreement=0, holiday_list="__Test Holiday List",
+		entity_type=None, entity=None, response_time=14400, resolution_time=21600,
+		service_level="24-hour-SLA", start_time="00:00:00", end_time="23:59:59",
+		condition="doc.issue_type == 'Critical'"
+	)
+
 def make_holiday_list():
 	holiday_list = frappe.db.exists("Holiday List", "__Test Holiday List")
 	if not holiday_list: