Merge pull request #28550 from nextchamp-saqib/fix-sla

refactor(SLA): Application of SLA and its fields

erpnext/hooks.py

@@ -234,7 +234,7 @@ doc_events = {
 	},
 	"Communication": {
 		"on_update": [
-			"erpnext.support.doctype.service_level_agreement.service_level_agreement.update_hold_time",
+			"erpnext.support.doctype.service_level_agreement.service_level_agreement.on_communication_update",
 			"erpnext.support.doctype.issue.issue.set_first_response_time"
 		]
 	},
@@ -343,8 +343,7 @@ scheduler_events = {
 		"erpnext.erpnext_integrations.doctype.plaid_settings.plaid_settings.automatic_synchronization",
 		"erpnext.projects.doctype.project.project.hourly_reminder",
 		"erpnext.projects.doctype.project.project.collect_project_status",
-		"erpnext.hr.doctype.shift_type.shift_type.process_auto_attendance_for_all_shifts",
+		"erpnext.hr.doctype.shift_type.shift_type.process_auto_attendance_for_all_shifts"
-		"erpnext.support.doctype.service_level_agreement.service_level_agreement.set_service_level_agreement_variance"
 	],
 	"hourly_long": [
 		"erpnext.stock.doctype.repost_item_valuation.repost_item_valuation.repost_entries"

erpnext/patches.txt

@@ -313,6 +313,7 @@ erpnext.patches.v13_0.update_category_in_ltds_certificate
 erpnext.patches.v13_0.create_pan_field_for_india #2
 erpnext.patches.v14_0.delete_hub_doctypes
 erpnext.patches.v13_0.create_ksa_vat_custom_fields
+erpnext.patches.v14_0.rename_ongoing_status_in_sla_documents
 erpnext.patches.v14_0.migrate_crm_settings
 erpnext.patches.v13_0.rename_ksa_qr_field
 erpnext.patches.v13_0.disable_ksa_print_format_for_others

erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py

@@ -0,0 +1,27 @@
+import frappe
+
+
+def execute():
+	active_sla_documents = [sla.document_type for sla in frappe.get_all("Service Level Agreement", fields=["document_type"])]
+
+	for doctype in active_sla_documents:
+		doctype = frappe.qb.DocType(doctype)
+		try:
+			frappe.qb.update(
+				doctype
+			).set(
+				doctype.agreement_status, 'First Response Due'
+			).where(
+				doctype.first_responded_on.isnull()
+			).run()
+
+			frappe.qb.update(
+				doctype
+			).set(
+				doctype.agreement_status, 'Resolution Due'
+			).where(
+				doctype.agreement_status == 'Ongoing'
+			).run()
+
+		except Exception:
+			frappe.log_error(title='Failed to Patch SLA Status')

erpnext/public/js/utils.js

@@ -835,7 +835,7 @@ $(document).on('app_ready', function() {
 
 					refresh: function(frm) {
 						if (frm.doc.status !== 'Closed' && frm.doc.service_level_agreement
-							&& frm.doc.agreement_status === 'Ongoing') {
+							&& ['First Response Due', 'Resolution Due'].includes(frm.doc.agreement_status)) {
 							frappe.call({
 								'method': 'frappe.client.get',
 								args: {
@@ -888,8 +888,8 @@ $(document).on('app_ready', function() {
 function set_time_to_resolve_and_response(frm, apply_sla_for_resolution) {
 	frm.dashboard.clear_headline();
 
-	let time_to_respond = get_status(frm.doc.response_by_variance);
+	let time_to_respond = get_status(frm.doc.response_by);
-	if (!frm.doc.first_responded_on && frm.doc.agreement_status === 'Ongoing') {
+	if (!frm.doc.first_responded_on) {
 		time_to_respond = get_time_left(frm.doc.response_by, frm.doc.agreement_status);
 	}
 
@@ -903,8 +903,8 @@ function set_time_to_resolve_and_response(frm, apply_sla_for_resolution) {
 
 
 	if (apply_sla_for_resolution) {
-		let time_to_resolve = get_status(frm.doc.resolution_by_variance);
+		let time_to_resolve = get_status(frm.doc.resolution_by);
-		if (!frm.doc.resolution_date && frm.doc.agreement_status === 'Ongoing') {
+		if (!frm.doc.resolution_date) {
 			time_to_resolve = get_time_left(frm.doc.resolution_by, frm.doc.agreement_status);
 		}
 
@@ -928,8 +928,9 @@ function get_time_left(timestamp, agreement_status) {
 	return {'diff_display': diff_display, 'indicator': indicator};
 }
 
-function get_status(variance) {
+function get_status(timestamp) {
-	if (variance > 0) {
+	const time_left = moment(timestamp).diff(moment());
+	if (time_left >= 0) {
 		return {'diff_display': 'Fulfilled', 'indicator': 'green'};
 	} else {
 		return {'diff_display': 'Failed', 'indicator': 'red'};

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",
@@ -123,7 +121,6 @@
    "search_index": 1
   },
   {
-   "default": "Medium",
    "fieldname": "priority",
    "fieldtype": "Link",
    "in_list_view": 1,
@@ -318,22 +315,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",
@@ -391,12 +372,12 @@
    "read_only": 1
   },
   {
-   "default": "Ongoing",
+   "default": "First Response Due",
    "depends_on": "eval: doc.service_level_agreement",
    "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
   },
   {
@@ -410,10 +391,11 @@
  "icon": "fa fa-ticket",
  "idx": 7,
  "links": [],
- "modified": "2021-06-10 03:22:27.098898",
+ "modified": "2021-11-24 13:13:10.276630",
  "modified_by": "Administrator",
  "module": "Support",
  "name": "Issue",
+ "naming_rule": "By \"Naming Series\" field",
  "owner": "Administrator",
  "permissions": [
   {

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/issue_list.js

@@ -18,7 +18,6 @@ frappe.listview_settings['Issue'] = {
 	},
 	get_indicator: function(doc) {
 		if (doc.status === 'Open') {
-			if (!doc.priority) doc.priority = 'Medium';
 			const color = {
 				'Low': 'yellow',
 				'Medium': 'orange',

erpnext/support/doctype/issue/test_issue.py

@@ -1,10 +1,10 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and Contributors
 # See license.txt
 
-import datetime
 import unittest
 
 import frappe
+from frappe import _
 from frappe.core.doctype.user_permission.test_user_permission import create_user
 from frappe.utils import flt, get_datetime
 
@@ -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")
 
@@ -142,6 +118,142 @@ class TestIssue(TestSetUp):
 		issue.reload()
 		self.assertEqual(flt(issue.total_hold_time, 2), 2700)
 
+	def test_issue_close_after_on_hold(self):
+		frappe.flags.current_time = get_datetime("2021-11-01 19:00")
+
+		issue = make_issue(frappe.flags.current_time, index=1)
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
+
+		# send a reply within SLA
+		frappe.flags.current_time = get_datetime("2021-11-02 11:00")
+		create_communication(issue.name, "test@admin.com", "Sent", frappe.flags.current_time)
+
+		issue.reload()
+		issue.status = 'Replied'
+		issue.save()
+
+		self.assertEqual(issue.on_hold_since, frappe.flags.current_time)
+
+		# close the issue after being on hold for 20 days
+		frappe.flags.current_time = get_datetime("2021-11-22 01:00")
+		issue.status = 'Closed'
+		issue.save()
+
+		self.assertEqual(issue.resolution_by, get_datetime('2021-11-22 06:00:00'))
+		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)
+
+	def test_recording_of_assignment_on_first_reponse_failure(self):
+		from frappe.desk.form.assign_to import add as add_assignment
+
+		frappe.flags.current_time = get_datetime("2021-11-01 19:00")
+
+		issue = make_issue(frappe.flags.current_time, index=1)
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
+		add_assignment({
+			'doctype': issue.doctype,
+			'name': issue.name,
+			'assign_to': ['test@admin.com']
+		})
+		issue.reload()
+
+		# send a reply failing response SLA
+		frappe.flags.current_time = get_datetime("2021-11-02 15:00")
+		create_communication(issue.name, "test@admin.com", "Sent", frappe.flags.current_time)
+
+		# assert if a new timeline item has been added
+		# to record the assignment
+		comment = frappe.db.exists('Comment', {
+			'reference_doctype': 'Issue',
+			'reference_name': issue.name,
+			'comment_type': 'Assigned',
+			'content': _('First Response SLA Failed by {}').format('test')
+		})
+		self.assertTrue(comment)
+
+	def test_agreement_status_on_response(self):
+		frappe.flags.current_time = get_datetime("2021-11-01 19:00")
+
+		issue = make_issue(frappe.flags.current_time, index=1)
+		create_communication(issue.name, "test@example.com", "Received", frappe.flags.current_time)
+		self.assertTrue(issue.status == 'Open')
+
+		# send a reply within response SLA
+		frappe.flags.current_time = get_datetime("2021-11-02 11:00")
+		create_communication(issue.name, "test@admin.com", "Sent", frappe.flags.current_time)
+
+		issue.reload()
+		self.assertEquals(issue.first_responded_on, frappe.flags.current_time)
+		self.assertEquals(issue.agreement_status, 'Resolution Due')
+
 class TestFirstResponseTime(TestSetUp):
 	# working hours used in all cases: Mon-Fri, 10am to 6pm
 	# all dates are in the mm-dd-yyyy format
@@ -355,12 +467,18 @@ class TestFirstResponseTime(TestSetUp):
 def create_issue_and_communication(issue_creation, first_responded_on):
 	issue = make_issue(issue_creation, index=1)
 	sender = create_user("test@admin.com")
+	frappe.flags.current_time = first_responded_on
 	create_communication(issue.name, sender.email, "Sent", first_responded_on)
 	issue.reload()
 
 	return issue
 
 def make_issue(creation=None, customer=None, index=0, priority=None, issue_type=None):
+	if issue_type and not frappe.db.exists('Issue Type', issue_type):
+		doc = frappe.new_doc('Issue Type')
+		doc.name = issue_type
+		doc.insert()
+
 	issue = frappe.get_doc({
 		"doctype": "Issue",
 		"subject": "Service Level Agreement Issue {0}".format(index),

erpnext/support/doctype/service_level_agreement/service_level_agreement.js

@@ -22,10 +22,41 @@ frappe.ui.form.on('Service Level Agreement', {
 	refresh: function(frm) {
 		frm.trigger('fetch_status_fields');
 		frm.trigger('toggle_resolution_fields');
+		frm.trigger('default_service_level_agreement');
+		frm.trigger('entity');
+	},
+
+	default_service_level_agreement: function(frm) {
+		const field = frm.get_field('default_service_level_agreement');
+		if (frm.doc.default_service_level_agreement) {
+			field.set_description(__('SLA will be applied on every {0}', [frm.doc.document_type]));
+		} else {
+			field.set_description(__('Enable to apply SLA on every {0}', [frm.doc.document_type]));
+		}
 	},
 
 	document_type: function(frm) {
 		frm.trigger('fetch_status_fields');
+		frm.trigger('default_service_level_agreement');
+	},
+
+	entity_type: function(frm) {
+		frm.set_value('entity', undefined);
+	},
+
+	entity: function(frm) {
+		const field = frm.get_field('entity');
+		if (frm.doc.entity) {
+			const and_descendants = frm.doc.entity_type != 'Customer' ? ' ' + __('or its descendants') : '';
+			field.set_description(
+				__('SLA will be applied if {1} is set as {2}{3}', [
+					frm.doc.document_type, frm.doc.entity_type,
+					frm.doc.entity, and_descendants
+				])
+			);
+		} else {
+			field.set_description('');
+		}
 	},
 
 	fetch_status_fields: function(frm) {

erpnext/support/doctype/service_level_agreement/service_level_agreement.json

@@ -6,22 +6,17 @@
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
-  "enabled",
-  "section_break_2",
   "document_type",
-  "default_service_level_agreement",
   "default_priority",
   "column_break_2",
   "service_level",
-  "holiday_list",
+  "enabled",
-  "entity_section",
-  "entity_type",
-  "column_break_10",
-  "entity",
   "filters_section",
-  "condition",
+  "default_service_level_agreement",
+  "entity_type",
+  "entity",
   "column_break_15",
-  "condition_description",
+  "condition",
   "agreement_details_section",
   "start_date",
   "column_break_7",
@@ -31,8 +26,10 @@
   "priorities",
   "status_details",
   "sla_fulfilled_on",
+  "column_break_22",
   "pause_sla_on",
   "support_and_resolution_section_break",
+  "holiday_list",
   "support_and_resolution"
  ],
  "fields": [
@@ -42,7 +39,8 @@
    "in_list_view": 1,
    "in_standard_filter": 1,
    "label": "Service Level Name",
-   "reqd": 1
+   "reqd": 1,
+   "set_only_once": 1
   },
   {
    "fieldname": "holiday_list",
@@ -56,10 +54,10 @@
    "fieldtype": "Column Break"
   },
   {
-   "depends_on": "eval: !doc.default_service_level_agreement",
+   "depends_on": "eval: doc.document_type",
    "fieldname": "agreement_details_section",
    "fieldtype": "Section Break",
-   "label": "Agreement Details"
+   "label": "Valid From"
   },
   {
    "fieldname": "start_date",
@@ -72,7 +70,6 @@
    "fieldtype": "Column Break"
   },
   {
-   "depends_on": "eval: !doc.default_service_level_agreement",
    "fieldname": "end_date",
    "fieldtype": "Date",
    "label": "End Date"
@@ -80,7 +77,7 @@
   {
    "fieldname": "response_and_resolution_time_section",
    "fieldtype": "Section Break",
-   "label": "Response and Resolution Time"
+   "label": "Response and Resolution"
   },
   {
    "fieldname": "support_and_resolution_section_break",
@@ -90,6 +87,7 @@
   {
    "fieldname": "support_and_resolution",
    "fieldtype": "Table",
+   "label": "Working Hours",
    "options": "Service Day",
    "reqd": 1
   },
@@ -101,10 +99,7 @@
    "reqd": 1
   },
   {
-   "fieldname": "column_break_10",
+   "depends_on": "eval: !doc.default_service_level_agreement",
-   "fieldtype": "Column Break"
-  },
-  {
    "fieldname": "entity",
    "fieldtype": "Dynamic Link",
    "in_list_view": 1,
@@ -114,22 +109,12 @@
   },
   {
    "depends_on": "eval: !doc.default_service_level_agreement",
-   "fieldname": "entity_section",
-   "fieldtype": "Section Break",
-   "label": "Entity"
-  },
-  {
    "fieldname": "entity_type",
    "fieldtype": "Select",
    "in_standard_filter": 1,
    "label": "Entity Type",
    "options": "\nCustomer\nCustomer Group\nTerritory"
   },
-  {
-   "fieldname": "section_break_2",
-   "fieldtype": "Section Break",
-   "hide_border": 1
-  },
   {
    "default": "0",
    "fieldname": "default_service_level_agreement",
@@ -152,7 +137,7 @@
   {
    "fieldname": "document_type",
    "fieldtype": "Link",
-   "label": "Document Type",
+   "label": "Apply On",
    "options": "DocType",
    "reqd": 1,
    "set_only_once": 1
@@ -164,6 +149,7 @@
    "label": "Enabled"
   },
   {
+   "depends_on": "document_type",
    "fieldname": "status_details",
    "fieldtype": "Section Break",
    "label": "Status Details"
@@ -182,28 +168,31 @@
    "label": "Apply SLA for Resolution Time"
   },
   {
+   "depends_on": "document_type",
    "fieldname": "filters_section",
    "fieldtype": "Section Break",
-   "label": "Assignment Condition"
+   "label": "Assignment Conditions"
   },
   {
    "fieldname": "column_break_15",
    "fieldtype": "Column Break"
   },
   {
+   "depends_on": "eval: !doc.default_service_level_agreement",
+   "description": "Simple Python Expression, Example: doc.status == 'Open' and doc.issue_type == 'Bug'",
    "fieldname": "condition",
    "fieldtype": "Code",
    "label": "Condition",
-   "options": "Python"
+   "max_height": "7rem",
+   "options": "PythonExpression"
   },
   {
-   "fieldname": "condition_description",
+   "fieldname": "column_break_22",
-   "fieldtype": "HTML",
+   "fieldtype": "Column Break"
-   "options": "<p><strong>Condition Examples:</strong></p>\n<pre>doc.status==\"Open\"<br>doc.due_date==nowdate()<br>doc.total &gt; 40000\n</pre>"
   }
  ],
  "links": [],
- "modified": "2021-10-02 11:32:55.556024",
+ "modified": "2021-11-26 15:45:33.289911",
  "modified_by": "Administrator",
  "module": "Support",
  "name": "Service Level Agreement",

erpnext/support/doctype/service_level_agreement/service_level_agreement.py

@@ -10,7 +10,6 @@ from frappe.core.utils import get_parent_doc
 from frappe.model.document import Document
 from frappe.utils import (
 	add_to_date,
-	cint,
 	get_datetime,
 	get_datetime_str,
 	get_link_to_form,
@@ -22,6 +21,7 @@ from frappe.utils import (
 	time_diff_in_seconds,
 	to_timedelta,
 )
+from frappe.utils.nestedset import get_ancestors_of
 from frappe.utils.safe_exec import get_safe_globals
 
 from erpnext.support.doctype.issue.issue import get_holidays
@@ -248,7 +248,7 @@ def get_active_service_level_agreement_for(doc):
 
 	customer = doc.get('customer')
 	or_filters.append(
-		["Service Level Agreement", "entity", "in", [customer, get_customer_group(customer), get_customer_territory(customer)]]
+		["Service Level Agreement", "entity", "in", [customer] + get_customer_group(customer) + get_customer_territory(customer)]
 	)
 
 	default_sla_filter = filters + [["Service Level Agreement", "default_service_level_agreement", "=", 1]]
@@ -275,11 +275,23 @@ def get_context(doc):
 	return {"doc": doc.as_dict(), "nowdate": nowdate, "frappe": frappe._dict(utils=get_safe_globals().get("frappe").get("utils"))}
 
 def get_customer_group(customer):
-	return frappe.db.get_value("Customer", customer, "customer_group") if customer else None
+	customer_groups = []
+	customer_group = frappe.db.get_value("Customer", customer, "customer_group") if customer else None
+	if customer_group:
+		ancestors = get_ancestors_of("Customer Group", customer_group)
+		customer_groups = [customer_group] + ancestors
+
+	return customer_groups
 
 
 def get_customer_territory(customer):
-	return frappe.db.get_value("Customer", customer, "territory") if customer else None
+	customer_territories = []
+	customer_territory = frappe.db.get_value("Customer", customer, "territory") if customer else None
+	if customer_territory:
+		ancestors = get_ancestors_of("Territory", customer_territory)
+		customer_territories = [customer_territory] + ancestors
+
+	return customer_territories
 
 
 @frappe.whitelist()
@@ -299,7 +311,7 @@ def get_service_level_agreement_filters(doctype, name, customer=None):
 	if customer:
 		# Include SLA with No Entity and Entity Type
 		or_filters.append(
-			["Service Level Agreement", "entity", "in", [customer, get_customer_group(customer), get_customer_territory(customer), ""]]
+			["Service Level Agreement", "entity", "in", [""] + [customer] + get_customer_group(customer) + get_customer_territory(customer)]
 		)
 
 	return {
@@ -337,84 +349,135 @@ def set_documents_with_active_service_level_agreement():
 
 def apply(doc, method=None):
 	# Applies SLA to document on validate
-	if frappe.flags.in_patch or frappe.flags.in_migrate or frappe.flags.in_install or frappe.flags.in_setup_wizard or \
+	if (
-		doc.doctype not in get_documents_with_active_service_level_agreement():
+		frappe.flags.in_patch
+		or frappe.flags.in_migrate
+		or frappe.flags.in_install
+		or frappe.flags.in_setup_wizard
+		or doc.doctype not in get_documents_with_active_service_level_agreement()
+	):
 		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
 
-	set_sla_properties(doc, service_level_agreement)
+	process_sla(doc, sla)
 
 
-def set_sla_properties(doc, service_level_agreement):
+def process_sla(doc, sla):
-	if frappe.db.exists(doc.doctype, doc.name):
-		from_db = frappe.get_doc(doc.doctype, doc.name)
-	else:
-		from_db = frappe._dict({})
-
-	meta = frappe.get_meta(doc.doctype)
-
-	if meta.has_field("customer") and service_level_agreement.customer and doc.get("customer") and \
-		not service_level_agreement.customer == doc.get("customer"):
-		frappe.throw(_("Service Level Agreement {0} is specific to Customer {1}").format(service_level_agreement.name,
-			service_level_agreement.customer))
-
-	doc.service_level_agreement = service_level_agreement.name
-	doc.priority = doc.get("priority") or service_level_agreement.default_priority
-	priority = get_priority(doc)
 
 	if not doc.creation:
 		doc.creation = now_datetime(doc.get("owner"))
-
+		if doc.meta.has_field("service_level_agreement_creation"):
-		if meta.has_field("service_level_agreement_creation"):
 			doc.service_level_agreement_creation = now_datetime(doc.get("owner"))
 
+	doc.service_level_agreement = sla.name
+	doc.priority = doc.get("priority") or sla.default_priority
+
+	handle_status_change(doc, sla.apply_sla_for_resolution)
+	update_response_and_resolution_metrics(doc, sla.apply_sla_for_resolution)
+	update_agreement_status(doc, sla.apply_sla_for_resolution)
+
+
+def handle_status_change(doc, apply_sla_for_resolution):
+	now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
+	prev_status = frappe.db.get_value(doc.doctype, doc.name, 'status')
+
+	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 set_first_response():
+		if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'):
+			doc.first_responded_on = now_time
+			if get_datetime(doc.get('first_responded_on')) > get_datetime(doc.get('response_by')):
+				record_assigned_users_on_failure(doc)
+
+	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)) or doc.flags.on_first_reply):
+		set_first_response()
+
+	# 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:
+			doc.resolution_date = now_time
+			set_resolution_time(doc)
+
+
+def get_fulfillment_statuses(service_level_agreement):
+	return [entry.status for entry in frappe.db.get_all("SLA Fulfilled On Status", filters={
+		"parent": service_level_agreement
+	}, fields=["status"])]
+
+
+def get_hold_statuses(service_level_agreement):
+	return [entry.status for entry in frappe.db.get_all("Pause SLA On Status", filters={
+		"parent": service_level_agreement
+	}, fields=["status"])]
+
+
+def update_response_and_resolution_metrics(doc, apply_sla_for_resolution):
+	priority = get_response_and_resolution_duration(doc)
 	start_date_time = get_datetime(doc.get("service_level_agreement_creation") or doc.creation)
-
+	set_response_by(doc, start_date_time, priority)
-	set_response_by_and_variance(doc, meta, start_date_time, priority)
+	if apply_sla_for_resolution:
-	if service_level_agreement.apply_sla_for_resolution:
+		set_resolution_by(doc, start_date_time, priority)
-		set_resolution_by_and_variance(doc, meta, start_date_time, priority)
-
-	update_status(doc, from_db, meta)
-
-
-def update_status(doc, from_db, meta):
-	if meta.has_field("status"):
-		if meta.has_field("first_responded_on") and doc.status != "Open" and \
-			from_db.status == "Open" and not doc.first_responded_on:
-			doc.first_responded_on = frappe.flags.current_time or now_datetime(doc.get("owner"))
-
-		if meta.has_field("service_level_agreement") and doc.service_level_agreement:
-			# mark sla status as fulfilled based on the configuration
-			fulfillment_statuses = [entry.status for entry in frappe.db.get_all("SLA Fulfilled On Status", filters={
-				"parent": doc.service_level_agreement
-			}, fields=["status"])]
-
-			if doc.status in fulfillment_statuses and from_db.status not in fulfillment_statuses:
-				apply_sla_for_resolution = frappe.db.get_value("Service Level Agreement", doc.service_level_agreement,
-					"apply_sla_for_resolution")
-
-				if apply_sla_for_resolution and meta.has_field("resolution_date"):
-					doc.resolution_date = frappe.flags.current_time or now_datetime(doc.get("owner"))
-
-				if meta.has_field("agreement_status") and from_db.agreement_status == "Ongoing":
-					set_service_level_agreement_variance(doc.doctype, doc.name)
-					update_agreement_status(doc, meta)
-
-				if apply_sla_for_resolution:
-					set_resolution_time(doc, meta)
-					set_user_resolution_time(doc, meta)
-
-		if doc.status == "Open" and from_db.status != "Open":
-			# if no date, it should be set as None and not a blank string "", as per mysql strict config
-			# enable SLA and variance on Reopen
-			reset_metrics(doc, meta)
-			set_service_level_agreement_variance(doc.doctype, doc.name)
-
-	handle_hold_time(doc, meta, from_db.status)
 
 
 def get_expected_time_for(parameter, service_level, start_date_time):
@@ -485,37 +548,13 @@ def get_support_days(service_level):
 	return support_days
 
 
-def set_service_level_agreement_variance(doctype, doc=None):
+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)
 
-	filters = {"status": "Open", "agreement_status": "Ongoing"}
-
-	if doc:
-		filters = {"name": doc}
-
-	for entry in frappe.get_all(doctype, filters=filters):
-		current_doc = frappe.get_doc(doctype, entry.name)
-		current_time = frappe.flags.current_time or now_datetime(current_doc.get("owner"))
-		apply_sla_for_resolution = frappe.db.get_value("Service Level Agreement", current_doc.service_level_agreement,
-			"apply_sla_for_resolution")
-
-		if not current_doc.first_responded_on: # first_responded_on set when first reply is sent to customer
-			variance = round(time_diff_in_seconds(current_doc.response_by, current_time), 2)
-			frappe.db.set_value(current_doc.doctype, current_doc.name, "response_by_variance", variance, update_modified=False)
-
-			if variance < 0:
-				frappe.db.set_value(current_doc.doctype, current_doc.name, "agreement_status", "Failed", update_modified=False)
-
-		if apply_sla_for_resolution and not current_doc.get("resolution_date"): # resolution_date set when issue has been closed
-			variance = round(time_diff_in_seconds(current_doc.resolution_by, current_time), 2)
-			frappe.db.set_value(current_doc.doctype, current_doc.name, "resolution_by_variance", variance, update_modified=False)
-
-			if variance < 0:
-				frappe.db.set_value(current_doc.doctype, current_doc.name, "agreement_status", "Failed", update_modified=False)
-
-
-def set_user_resolution_time(doc, meta):
 	# total time taken by a user to close the issue apart from wait_time
-	if not meta.has_field("user_resolution_time"):
+	if not doc.meta.has_field("user_resolution_time"):
 		return
 
 	communications = frappe.get_all("Communication", filters={
@@ -531,7 +570,7 @@ def set_user_resolution_time(doc, meta):
 				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
 
 
@@ -548,12 +587,12 @@ def change_service_level_agreement_and_priority(self):
 			frappe.msgprint(_("Service Level Agreement has been changed to {0}.").format(self.service_level_agreement))
 
 
-def get_priority(doc):
+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
 
@@ -572,122 +611,99 @@ 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()
 
 
-def reset_metrics(doc, meta):
+def reset_resolution_metrics(doc):
-	if meta.has_field("resolution_date"):
+	if doc.meta.has_field("resolution_date"):
 		doc.resolution_date = None
 
-	if not meta.has_field("resolution_time"):
+	if doc.meta.has_field("resolution_time"):
 		doc.resolution_time = None
 
-	if not meta.has_field("user_resolution_time"):
+	if doc.meta.has_field("user_resolution_time"):
 		doc.user_resolution_time = None
 
-	if meta.has_field("agreement_status"):
+	if doc.meta.has_field("agreement_status"):
-		doc.agreement_status = "Ongoing"
+		doc.agreement_status = "First Response Due"
-
-
-def set_resolution_time(doc, meta):
-	# total time taken from issue creation to closing
-	if not meta.has_field("resolution_time"):
-		return
-
-	doc.resolution_time = time_diff_in_seconds(doc.resolution_date, doc.creation)
 
 
 # called via hooks on communication update
-def update_hold_time(doc, status):
+def on_communication_update(doc, status):
+	if doc.communication_type == "Comment":
+		return
+
 	parent = get_parent_doc(doc)
 	if not parent:
 		return
 
-	if doc.communication_type == "Comment":
+	if not parent.meta.has_field('service_level_agreement'):
 		return
 
-	status_field = parent.meta.get_field("status")
+	for_resolution = frappe.db.get_value('Service Level Agreement', parent.service_level_agreement, 'apply_sla_for_resolution')
-	if status_field:
-		options = (status_field.options or "").splitlines()
 
-		# if status has a "Replied" option, then handle hold time
+	if (
-		if ("Replied" in options) and doc.sent_or_received == "Received":
+		doc.sent_or_received == "Received" # a reply is received
-			meta = frappe.get_meta(parent.doctype)
+		and parent.get('status') == 'Open' # issue status is set as open from communication.py
-			handle_hold_time(parent, meta, 'Replied')
+		and parent._doc_before_save
+		and parent.get('status') != parent._doc_before_save.get('status') # status changed
+	):
+		# undo the status change in db
+		# since prev status is fetched from db
+		frappe.db.set_value(parent.doctype, parent.name, 'status', parent._doc_before_save.get('status'))
+
+	elif (
+		doc.sent_or_received == "Sent" # a reply is sent
+		and parent.get('first_responded_on') # first_responded_on is set from communication.py
+		and parent._doc_before_save
+		and not parent._doc_before_save.get('first_responded_on') # first_responded_on was not set
+	):
+		# reset first_responded_on since it will be handled/set later on
+		parent.first_responded_on = None
+		parent.flags.on_first_reply = True
+
+	handle_status_change(parent, for_resolution)
+	update_response_and_resolution_metrics(parent, for_resolution)
+	update_agreement_status(parent, for_resolution)
+
+	parent.save()
 
 
-def handle_hold_time(doc, meta, status):
+def reset_expected_response_and_resolution(doc):
-	if meta.has_field("service_level_agreement") and doc.service_level_agreement:
-		# set response and resolution variance as None as the issue is on Hold for status as Replied
-		hold_statuses = [entry.status for entry in frappe.db.get_all("Pause SLA On Status", filters={
-				"parent": doc.service_level_agreement
-			}, fields=["status"])]
-
-		if not hold_statuses:
-			return
-
-		if meta.has_field("status") and doc.status in hold_statuses and status not in hold_statuses:
-			apply_hold_status(doc, meta)
-
-		# calculate hold time when status is changed from any hold status to any non-hold status
-		if meta.has_field("status") and doc.status not in hold_statuses and status in hold_statuses:
-			reset_hold_status_and_update_hold_time(doc, meta)
-
-
-def apply_hold_status(doc, meta):
-	update_values = {'on_hold_since': frappe.flags.current_time or now_datetime(doc.get("owner"))}
-
-	if meta.has_field("first_responded_on") and not doc.first_responded_on:
-		update_values['response_by'] = None
-		update_values['response_by_variance'] = 0
-
-	update_values['resolution_by'] = None
-	update_values['resolution_by_variance'] = 0
-
-	doc.db_set(update_values)
-
-
-def reset_hold_status_and_update_hold_time(doc, meta):
-	hold_time = doc.total_hold_time if meta.has_field("total_hold_time") and doc.total_hold_time else 0
-	now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
-	last_hold_time = 0
 	update_values = {}
-
+	if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'):
-	if meta.has_field("on_hold_since") and doc.on_hold_since:
+		update_values['response_by'] = None
-		# last_hold_time will be added to the sla variables
+	if doc.meta.has_field("resolution_by") and not doc.get('resolution_date'):
-		last_hold_time = time_diff_in_seconds(now_time, doc.on_hold_since)
+		update_values['resolution_by'] = None
-		update_values['total_hold_time'] = hold_time + last_hold_time
-
-	# re-calculate SLA variables after issue changes from any hold status to any non-hold status
-	start_date_time = get_datetime(doc.get("service_level_agreement_creation") or doc.creation)
-	priority = get_priority(doc)
-	now_time = frappe.flags.current_time or now_datetime(doc.get("owner"))
-
-	# add hold time to response by variance
-	if meta.has_field("first_responded_on") and not doc.first_responded_on:
-		response_by = get_expected_time_for(parameter="response", service_level=priority, start_date_time=start_date_time)
-		response_by = add_to_date(response_by, seconds=round(last_hold_time))
-		response_by_variance = round(time_diff_in_seconds(response_by, now_time))
-
-		update_values['response_by'] = response_by
-		update_values['response_by_variance'] = response_by_variance + last_hold_time
-
-	# add hold time to resolution by variance
-	if frappe.db.get_value("Service Level Agreement", doc.service_level_agreement, "apply_sla_for_resolution"):
-		resolution_by = get_expected_time_for(parameter="resolution", service_level=priority, start_date_time=start_date_time)
-		resolution_by = add_to_date(resolution_by, seconds=round(last_hold_time))
-		resolution_by_variance = round(time_diff_in_seconds(resolution_by, now_time))
-
-		update_values['resolution_by'] = resolution_by
-		update_values['resolution_by_variance'] = resolution_by_variance + last_hold_time
-
-	update_values['on_hold_since'] = None
-
 	doc.db_set(update_values)
 
 
+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')))
+
+
+def set_resolution_by(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')))
+
+
+def record_assigned_users_on_failure(doc):
+	assigned_users = doc.get_assigned_users()
+	if assigned_users:
+		from frappe.utils import get_fullname
+		assigned_users = ', '.join((get_fullname(user) for user in assigned_users))
+		message = _('First Response SLA Failed by {}').format(assigned_users)
+		doc.add_comment(
+			comment_type='Assigned',
+			text=message
+		)
+
+
 def get_service_level_agreement_fields():
 	return [
 		{
@@ -714,17 +730,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
 		},
 		{
@@ -746,11 +756,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
 		},
 		{
@@ -759,13 +769,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",
@@ -786,43 +789,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 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 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 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, meta):
+def update_agreement_status(doc, apply_sla_for_resolution):
-	if meta.has_field("service_level_agreement") and meta.has_field("agreement_status") and \
+	if (doc.meta.has_field("agreement_status")):
-		doc.service_level_agreement and doc.agreement_status == "Ongoing":
-
-		apply_sla_for_resolution = frappe.db.get_value("Service Level Agreement", doc.service_level_agreement,
-			"apply_sla_for_resolution")
-
 		# if SLA is applied for resolution check for response and resolution, else only response
 		if apply_sla_for_resolution:
-			if meta.has_field("response_by_variance") and meta.has_field("resolution_by_variance"):
+			if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'):
-				if cint(frappe.db.get_value(doc.doctype, doc.name, "response_by_variance")) < 0 or \
+				doc.agreement_status = "First Response Due"
-					cint(frappe.db.get_value(doc.doctype, doc.name, "resolution_by_variance")) < 0:
+			elif doc.meta.has_field("resolution_date") and not doc.get('resolution_date'):
-
+				doc.agreement_status = "Resolution Due"
-					doc.agreement_status = "Failed"
+			elif get_datetime(doc.get('resolution_date')) <= get_datetime(doc.get('resolution_by')):
-				else:
-					doc.agreement_status = "Fulfilled"
-		else:
-			if meta.has_field("response_by_variance") and \
-				cint(frappe.db.get_value(doc.doctype, doc.name, "response_by_variance")) < 0:
-				doc.agreement_status = "Failed"
-			else:
 				doc.agreement_status = "Fulfilled"
+			else:
+				doc.agreement_status = "Failed"
+		else:
+			if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'):
+				doc.agreement_status = "First Response Due"
+			elif get_datetime(doc.get('first_responded_on')) <= get_datetime(doc.get('response_by')):
+				doc.agreement_status = "Fulfilled"
+			else:
+				doc.agreement_status = "Failed"
 
 
 def is_holiday(date, holidays):
@@ -835,23 +823,6 @@ def get_time_in_timedelta(time):
 	return datetime.timedelta(hours=time.hour, minutes=time.minute, seconds=time.second)
 
 
-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") 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)
-
-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") 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)
-
-
 def now_datetime(user):
 	dt = convert_utc_to_user_timezone(datetime.utcnow(), user)
 	return dt.replace(tzinfo=None)

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,
 			}
 		]
 	})
@@ -386,7 +351,7 @@ def create_service_level_agreement(default_service_level_agreement, holiday_list
 	if sla:
 		frappe.delete_doc("Service Level Agreement", sla, force=1)
 
-	return frappe.get_doc(service_level_agreement).insert(ignore_permissions=True)
+	return frappe.get_doc(service_level_agreement).insert(ignore_permissions=True, ignore_if_duplicate=True)
 
 
 def create_customer():
@@ -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:

erpnext/support/report/issue_summary/issue_summary.py

@@ -82,7 +82,8 @@ class IssueSummary(object):
 		self.sla_status_map = {
 			'SLA Failed': 'failed',
 			'SLA Fulfilled': 'fulfilled',
-			'SLA Ongoing': 'ongoing'
+			'First Response Due': 'first_response_due',
+			'Resolution Due': 'resolution_due'
 		}
 
 		for label, fieldname in self.sla_status_map.items():