From 9047e2b9dd8b6d8b1819404d98480d86b598d562 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 24 Nov 2021 13:52:20 +0530 Subject: [PATCH 01/23] refactor: do not set default priority in isssues --- erpnext/support/doctype/issue/issue.json | 4 ++-- erpnext/support/doctype/issue/issue_list.js | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/support/doctype/issue/issue.json b/erpnext/support/doctype/issue/issue.json index 14712f89feb1..75b6d0f4f921 100644 --- a/erpnext/support/doctype/issue/issue.json +++ b/erpnext/support/doctype/issue/issue.json @@ -123,7 +123,6 @@ "search_index": 1 }, { - "default": "Medium", "fieldname": "priority", "fieldtype": "Link", "in_list_view": 1, @@ -410,10 +409,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": [ { diff --git a/erpnext/support/doctype/issue/issue_list.js b/erpnext/support/doctype/issue/issue_list.js index e04498e29ee0..5bfecb019cca 100644 --- a/erpnext/support/doctype/issue/issue_list.js +++ b/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', From 1f060c0b0a2d1925ebfda8b269517462d428ef35 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 24 Nov 2021 13:53:39 +0530 Subject: [PATCH 02/23] feat: find SLA based on customer group's ancestors --- .../service_level_agreement.py | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 5f8f83d89ba8..9dfe339c5192 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -22,6 +22,7 @@ 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 +249,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 +276,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 +312,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 { @@ -343,6 +356,8 @@ def apply(doc, method=None): service_level_agreement = get_active_service_level_agreement_for(doc) + print(service_level_agreement) + if not service_level_agreement: return From c46c8dd6c57a1d12df977b02c14affaf479e1bb3 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 24 Nov 2021 19:23:31 +0530 Subject: [PATCH 03/23] feat: do not change variance if response or resolution is set --- .../service_level_agreement.py | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 9dfe339c5192..a25608b27597 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -515,17 +515,21 @@ def set_service_level_agreement_variance(doctype, doc=None): 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) + else: + variance = round(time_diff_in_seconds(current_doc.response_by, current_doc.first_responded_on), 2) - if variance < 0: - frappe.db.set_value(current_doc.doctype, current_doc.name, "agreement_status", "Failed", update_modified=False) + 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) + elif apply_sla_for_resolution and current_doc.get("resolution_date"): + variance = round(time_diff_in_seconds(current_doc.resolution_by, current_doc.get("resolution_date")), 2) - if variance < 0: - frappe.db.set_value(current_doc.doctype, current_doc.name, "agreement_status", "Failed", update_modified=False) + 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): @@ -808,10 +812,18 @@ def update_agreement_status_on_custom_status(doc): # 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("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 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("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 meta.has_field("agreement_status"): doc.agreement_status = "Fulfilled" if doc.response_by_variance > 0 and doc.resolution_by_variance > 0 else "Failed" @@ -857,6 +869,8 @@ def set_response_by_and_variance(doc, meta, start_date_time, priority): 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) + elif meta.has_field("response_by_variance") and doc.get('first_responded_on'): + 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, meta, start_date_time, priority): if meta.has_field("resolution_by"): From 210f593a492ba4332c526fac6579df9adc3afc1c Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 26 Nov 2021 15:49:17 +0530 Subject: [PATCH 04/23] refactor: SLA form fields --- .../service_level_agreement.js | 31 ++++++++++ .../service_level_agreement.json | 59 ++++++++----------- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.js b/erpnext/support/doctype/service_level_agreement/service_level_agreement.js index ae2080c3b530..7e260dbbf564 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.js +++ b/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) { diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.json b/erpnext/support/doctype/service_level_agreement/service_level_agreement.json index 5f470aad672b..1698e2380f7c 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.json +++ b/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", - "entity_section", + "enabled", + "filters_section", + "default_service_level_agreement", "entity_type", - "column_break_10", "entity", - "filters_section", - "condition", "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", - "fieldtype": "Column Break" - }, - { + "depends_on": "eval: !doc.default_service_level_agreement", "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", - "fieldtype": "HTML", - "options": "

Condition Examples:

\n
doc.status==\"Open\"
doc.due_date==nowdate()
doc.total > 40000\n
" + "fieldname": "column_break_22", + "fieldtype": "Column Break" } ], "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", From a8c75b68628d29cb354ae3cb62424d3a01923586 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 26 Nov 2021 20:16:21 +0530 Subject: [PATCH 05/23] refactor: application of SLA and its metrics --- erpnext/hooks.py | 3 +- .../service_level_agreement.py | 337 +++++++----------- 2 files changed, 136 insertions(+), 204 deletions(-) diff --git a/erpnext/hooks.py b/erpnext/hooks.py index 2a277ee0350c..186dbb5847d1 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -341,8 +341,7 @@ "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.support.doctype.service_level_agreement.service_level_agreement.set_service_level_agreement_variance" + "erpnext.hr.doctype.shift_type.shift_type.process_auto_attendance_for_all_shifts" ], "hourly_long": [ "erpnext.stock.doctype.repost_item_valuation.repost_item_valuation.repost_entries" diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index a25608b27597..864640204b35 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -10,7 +10,6 @@ from frappe.model.document import Document from frappe.utils import ( add_to_date, - cint, get_datetime, get_datetime_str, get_link_to_form, @@ -350,86 +349,113 @@ 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 \ - doc.doctype not in get_documents_with_active_service_level_agreement(): + if ( + 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) - print(service_level_agreement) - if not service_level_agreement: return - set_sla_properties(doc, service_level_agreement) - + process_sla(doc, service_level_agreement) -def set_sla_properties(doc, service_level_agreement): - 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) +def process_sla(doc, service_level_agreement): if not doc.creation: doc.creation = now_datetime(doc.get("owner")) - - if meta.has_field("service_level_agreement_creation"): + if doc.meta.has_field("service_level_agreement_creation"): doc.service_level_agreement_creation = now_datetime(doc.get("owner")) - start_date_time = get_datetime(doc.get("service_level_agreement_creation") or doc.creation) + doc.service_level_agreement = service_level_agreement.name + doc.priority = doc.get("priority") or service_level_agreement.default_priority - set_response_by_and_variance(doc, meta, start_date_time, priority) - if service_level_agreement.apply_sla_for_resolution: - set_resolution_by_and_variance(doc, meta, start_date_time, 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) + update_response_and_resolution_metrics(doc, service_level_agreement.apply_sla_for_resolution) + update_agreement_status(doc, service_level_agreement.apply_sla_for_resolution) - update_status(doc, from_db, meta) +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_and_variance(doc, start_date_time, priority) + if apply_sla_for_resolution: + set_resolution_by_and_variance(doc, start_date_time, priority) -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"])] +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"])] - 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")) +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"])] - 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) +def handle_status_change(doc, prev_status, apply_sla_for_resolution): - 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) + if doc.status != "Open" and prev_status == "Open": + # status changed from Open to something else + if doc.meta.has_field("first_responded_on") and not doc.first_responded_on: + # status changed to something other than Open + doc.first_responded_on = frappe.flags.current_time or now_datetime(doc.get("owner")) - handle_hold_time(doc, meta, from_db.status) + 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): @@ -500,41 +526,9 @@ def get_support_days(service_level): return support_days -def set_service_level_agreement_variance(doctype, doc=None): - - 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) - else: - variance = round(time_diff_in_seconds(current_doc.response_by, current_doc.first_responded_on), 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) - elif apply_sla_for_resolution and current_doc.get("resolution_date"): - variance = round(time_diff_in_seconds(current_doc.resolution_by, current_doc.get("resolution_date")), 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): +def set_user_resolution_time(doc): # 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={ @@ -567,7 +561,7 @@ 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) priority = service_level_agreement.get_service_level_agreement_priority(doc.priority) priority.update({ @@ -596,115 +590,81 @@ def reset_service_level_agreement(doc, reason, user): doc.save() -def reset_metrics(doc, meta): - if meta.has_field("resolution_date"): +def reset_resolution_metrics(doc): + 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" -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): + if doc.communication_type == "Comment" or doc.sent_or_received != "Received": + 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") - if status_field: - options = (status_field.options or "").splitlines() + apply_sla_for_resolution = frappe.db.get_value('Service Level Agreement', parent.service_level_agreement, 'apply_sla_for_resolution') - # if status has a "Replied" option, then handle hold time - if ("Replied" in options) and doc.sent_or_received == "Received": - meta = frappe.get_meta(parent.doctype) - handle_hold_time(parent, meta, 'Replied') + handle_status_change(parent, 'Replied', apply_sla_for_resolution) + update_response_and_resolution_metrics(parent, apply_sla_for_resolution) + update_agreement_status(parent, apply_sla_for_resolution) + parent.save() -def handle_hold_time(doc, meta, status): - 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"))} +def reset_expected_response_and_resolution(doc): + update_values = {} - if meta.has_field("first_responded_on") and not doc.first_responded_on: + 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 - update_values['resolution_by'] = None - update_values['resolution_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 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 meta.has_field("on_hold_since") and doc.on_hold_since: - # last_hold_time will be added to the sla variables - last_hold_time = time_diff_in_seconds(now_time, doc.on_hold_since) - update_values['total_hold_time'] = hold_time + last_hold_time +def set_response_by_and_variance(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.total_hold_time: + doc.response_by = add_to_date(doc.response_by, seconds=round(doc.total_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")) + 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) - # 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)) + if doc.meta.has_field("response_by_variance") and doc.get('first_responded_on'): + doc.response_by_variance = round(time_diff_in_seconds(doc.response_by, doc.get('first_responded_on')), 2) - 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)) +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.total_hold_time: + doc.resolution_by = add_to_date(doc.resolution_by, seconds=round(doc.total_hold_time)) - update_values['resolution_by'] = resolution_by - update_values['resolution_by_variance'] = resolution_by_variance + last_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) - update_values['on_hold_since'] = None - - doc.db_set(update_values) + 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(): @@ -808,45 +768,37 @@ def update_agreement_status_on_custom_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: + 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 meta.has_field("first_responded_on") and doc.first_responded_on: + 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 meta.has_field("resolution_date") and not doc.resolution_date: + 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 meta.has_field("resolution_date") and doc.resolution_date: + 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 meta.has_field("agreement_status"): + 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, meta): - if meta.has_field("service_level_agreement") and meta.has_field("agreement_status") and \ - 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") - +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 meta.has_field("response_by_variance") and meta.has_field("resolution_by_variance"): - if cint(frappe.db.get_value(doc.doctype, doc.name, "response_by_variance")) < 0 or \ - cint(frappe.db.get_value(doc.doctype, doc.name, "resolution_by_variance")) < 0: - + if doc.meta.has_field("response_by_variance") and doc.meta.has_field("resolution_by_variance"): + if doc.response_by_variance < 0 or doc.resolution_by_variance < 0: doc.agreement_status = "Failed" 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: + if doc.meta.has_field("response_by_variance") and doc.response_by_variance < 0: doc.agreement_status = "Failed" else: doc.agreement_status = "Fulfilled" @@ -862,25 +814,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) - elif meta.has_field("response_by_variance") and doc.get('first_responded_on'): - 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, 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) From 214d0e367f5290dd82bc2f1dc7cdbd8e02951283 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 26 Nov 2021 20:20:38 +0530 Subject: [PATCH 06/23] fix: remove leading whitespace --- .../doctype/service_level_agreement/service_level_agreement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.js b/erpnext/support/doctype/service_level_agreement/service_level_agreement.js index 7e260dbbf564..bfbffe22ad78 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.js +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.js @@ -47,7 +47,7 @@ frappe.ui.form.on('Service Level Agreement', { entity: function(frm) { const field = frm.get_field('entity'); if (frm.doc.entity) { - const and_descendants = frm.doc.entity_type != 'Customer' ? __(' or its descendants') : ''; + 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, From 267cc3585013c5a9454df0c25b65d90bf8fef171 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Sat, 27 Nov 2021 16:47:45 +0530 Subject: [PATCH 07/23] fix: failing tests --- .../service_level_agreement/service_level_agreement.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 864640204b35..565f05083b1c 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -642,8 +642,8 @@ def reset_expected_response_and_resolution(doc): def set_response_by_and_variance(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.total_hold_time: - doc.response_by = add_to_date(doc.response_by, seconds=round(doc.total_hold_time)) + if doc.meta.has_field("total_hold_time") and doc.get('total_hold_time'): + 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")) @@ -656,8 +656,8 @@ def set_response_by_and_variance(doc, start_date_time, priority): 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.total_hold_time: - doc.resolution_by = add_to_date(doc.resolution_by, seconds=round(doc.total_hold_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")) From 79f8159ab95e357aa7b094901a9da36a44281c1c Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Sat, 27 Nov 2021 17:14:58 +0530 Subject: [PATCH 08/23] test: issue closing after being on hold --- erpnext/support/doctype/issue/test_issue.py | 26 +++++++++++++++++++ .../service_level_agreement.py | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/erpnext/support/doctype/issue/test_issue.py b/erpnext/support/doctype/issue/test_issue.py index ab9a444bc34d..0559b15649d9 100644 --- a/erpnext/support/doctype/issue/test_issue.py +++ b/erpnext/support/doctype/issue/test_issue.py @@ -142,6 +142,32 @@ def test_hold_time_on_replied(self): issue.reload() 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") + + issue = make_issue(creation, index=1) + create_communication(issue.name, "test@example.com", "Received", creation) + + # send a reply within SLA + creation = get_datetime("2021-11-02 11:00") + create_communication(issue.name, "test@admin.com", "Sent", creation) + + frappe.flags.current_time = creation + 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') + class TestFirstResponseTime(TestSetUp): # working hours used in all cases: Mon-Fri, 10am to 6pm # all dates are in the mm-dd-yyyy format diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 565f05083b1c..9c1e5360786f 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -642,7 +642,7 @@ def reset_expected_response_and_resolution(doc): def set_response_by_and_variance(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'): + 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'): From 6f67bbca69a1dcd7a9dd9918f0c656704ecc14a7 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 2 Dec 2021 13:39:05 +0530 Subject: [PATCH 09/23] refactor(SLA): remove response_by_variance & resolution_by_variance --- erpnext/support/doctype/issue/issue.json | 22 +- erpnext/support/doctype/issue/issue.py | 4 +- erpnext/support/doctype/issue/test_issue.py | 109 +++++--- .../service_level_agreement.py | 259 ++++++++---------- .../test_service_level_agreement.py | 68 ++--- 5 files changed, 216 insertions(+), 246 deletions(-) diff --git a/erpnext/support/doctype/issue/issue.json b/erpnext/support/doctype/issue/issue.json index 75b6d0f4f921..1da22fd58f98 100644 --- a/erpnext/support/doctype/issue/issue.json +++ b/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 }, { diff --git a/erpnext/support/doctype/issue/issue.py b/erpnext/support/doctype/issue/issue.py index 0dc3639f1eb3..d5e5b7828809 100644 --- a/erpnext/support/doctype/issue/issue.py +++ b/erpnext/support/doctype/issue/issue.py @@ -87,11 +87,9 @@ def split_issue(self, subject, communication_id): 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() diff --git a/erpnext/support/doctype/issue/test_issue.py b/erpnext/support/doctype/issue/test_issue.py index 0559b15649d9..da9953df5524 100644 --- a/erpnext/support/doctype/issue/test_issue.py +++ b/erpnext/support/doctype/issue/test_issue.py @@ -83,30 +83,6 @@ def test_response_time_and_resolution_time_based_on_different_sla(self): 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 @@ def test_hold_time_on_replied(self): 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) - create_communication(issue.name, "test@example.com", "Received", creation) + 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 - creation = get_datetime("2021-11-02 11:00") - create_communication(issue.name, "test@admin.com", "Sent", creation) + frappe.flags.current_time = get_datetime("2021-11-02 11:00") + 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 @@ def test_issue_close_after_on_hold(self): 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), diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 9c1e5360786f..e2326ac2df1a 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -358,104 +358,122 @@ 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.priority = doc.get("priority") or service_level_agreement.default_priority + doc.service_level_agreement = sla.name + 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) - update_response_and_resolution_metrics(doc, service_level_agreement.apply_sla_for_resolution) - update_agreement_status(doc, service_level_agreement.apply_sla_for_resolution) + handle_status_change(doc, prev_status, 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 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_and_variance(doc, start_date_time, priority) - if apply_sla_for_resolution: - set_resolution_by_and_variance(doc, start_date_time, priority) +def handle_status_change(doc, prev_status, apply_sla_for_resolution): + now_time = frappe.flags.current_time or now_datetime(doc.get("owner")) + hold_statuses = get_hold_statuses(doc.service_level_agreement) + fulfillment_statuses = get_fulfillment_statuses(doc.service_level_agreement) -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 is_hold_status(status): + return status in hold_statuses + def is_fulfilled_status(status): + return status in fulfillment_statuses -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 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 -def handle_status_change(doc, prev_status, apply_sla_for_resolution): + 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 doc.status != "Open" and prev_status == "Open": + 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: - # status changed to something other than Open - 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 + 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) - handle_fulfillment_status(doc, prev_status, apply_sla_for_resolution) - handle_hold_status(doc, prev_status) + # 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 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 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 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 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 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 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) + if apply_sla_for_resolution: + set_resolution_by(doc, start_date_time, priority) 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) - priority = service_level_agreement.get_service_level_agreement_priority(doc.priority) + sla = frappe.get_doc("Service Level Agreement", doc.service_level_agreement) + priority = sla.get_service_level_agreement_priority(doc.priority) priority.update({ - "support_and_resolution": service_level_agreement.support_and_resolution, - "holiday_list": service_level_agreement.holiday_list + "support_and_resolution": sla.support_and_resolution, + "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) - update_response_and_resolution_metrics(parent, apply_sla_for_resolution) - update_agreement_status(parent, apply_sla_for_resolution) + handle_status_change(parent, 'Replied', for_resolution) + update_response_and_resolution_metrics(parent, 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'): - 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): +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'))) - 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 - - 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" + update_agreement_status(doc) 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 doc.response_by_variance < 0 or doc.resolution_by_variance < 0: - doc.agreement_status = "Failed" - else: - doc.agreement_status = "Fulfilled" - else: - if doc.meta.has_field("response_by_variance") and doc.response_by_variance < 0: - doc.agreement_status = "Failed" + if not doc.first_responded_on: + doc.agreement_status = "First Response Due" + elif not doc.resolution_date: + doc.agreement_status = "Resolution Due" + elif get_datetime(doc.resolution_date) <= get_datetime(doc.resolution_by): + doc.agreement_status = "Fulfilled" else: + 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): diff --git a/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py index cfbe7446c0b1..ce564c4dae73 100644 --- a/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py @@ -220,42 +220,6 @@ def test_fulfilled_sla_for_response_only(self): 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", - "end_time": "18:00:00", + "start_time": start_time, + "end_time": end_time, }, { "workday": "Tuesday", - "start_time": "10:00:00", - "end_time": "18:00:00", + "start_time": start_time, + "end_time": end_time, }, { "workday": "Wednesday", - "start_time": "10:00:00", - "end_time": "18:00:00", + "start_time": start_time, + "end_time": end_time, }, { "workday": "Thursday", - "start_time": "10:00:00", - "end_time": "18:00:00", + "start_time": start_time, + "end_time": end_time, }, { "workday": "Friday", - "start_time": "10:00:00", - "end_time": "18:00:00", + "start_time": start_time, + "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: From a1cedc3ea027820f185c163333b69e5669cccd93 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 2 Dec 2021 14:48:24 +0530 Subject: [PATCH 10/23] fix: failing tests --- .../service_level_agreement/service_level_agreement.py | 8 ++++---- .../test_service_level_agreement.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index e2326ac2df1a..62b21474f03c 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -647,9 +647,9 @@ def update_hold_time(doc, status): def reset_expected_response_and_resolution(doc): update_values = {} - if doc.meta.has_field("first_responded_on") and not doc.first_responded_on: + if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'): update_values['response_by'] = None - if doc.meta.has_field("resolution_by") and not doc.resolution_date: + if doc.meta.has_field("resolution_by") and not doc.get('resolution_date'): update_values['resolution_by'] = None doc.db_set(update_values) @@ -760,7 +760,7 @@ 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 not doc.first_responded_on: + if doc.meta.has_field("first_responded_on") and not doc.first_responded_on: doc.agreement_status = "First Response Due" elif not doc.resolution_date: doc.agreement_status = "Resolution Due" @@ -769,7 +769,7 @@ def update_agreement_status(doc, apply_sla_for_resolution): else: doc.agreement_status = "Failed" else: - if not doc.first_responded_on: + if doc.meta.has_field("first_responded_on") and 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" diff --git a/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py index ce564c4dae73..b07c862c7b0d 100644 --- a/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py @@ -351,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(): From 6736a89b4ee9b2f9125a29378700dd9a547ac19e Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 2 Dec 2021 15:54:02 +0530 Subject: [PATCH 11/23] fix: failing tests --- .../service_level_agreement.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 62b21474f03c..662e42cc9f59 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -409,7 +409,7 @@ def calculate_hold_hours(): 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: + if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'): doc.first_responded_on = now_time # Open to Replied @@ -760,18 +760,18 @@ 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("first_responded_on") and not doc.first_responded_on: + if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'): doc.agreement_status = "First Response Due" - elif not doc.resolution_date: + elif doc.meta.has_field("resolution_date") and not doc.get('resolution_date'): doc.agreement_status = "Resolution Due" - elif get_datetime(doc.resolution_date) <= get_datetime(doc.resolution_by): + elif get_datetime(doc.get('resolution_date')) <= get_datetime(doc.get('resolution_by')): doc.agreement_status = "Fulfilled" else: doc.agreement_status = "Failed" else: - if doc.meta.has_field("first_responded_on") and not doc.first_responded_on: + 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.first_responded_on) <= get_datetime(doc.response_by): + elif get_datetime(doc.get('first_responded_on')) <= get_datetime(doc.get('response_by')): doc.agreement_status = "Fulfilled" else: doc.agreement_status = "Failed" From 1e550d3e46c633e24ba6f49f2883a506aa96b471 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 3 Dec 2021 11:36:53 +0530 Subject: [PATCH 12/23] fix: failing tests --- erpnext/support/doctype/issue/test_issue.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/support/doctype/issue/test_issue.py b/erpnext/support/doctype/issue/test_issue.py index da9953df5524..d566f33f24ab 100644 --- a/erpnext/support/doctype/issue/test_issue.py +++ b/erpnext/support/doctype/issue/test_issue.py @@ -430,9 +430,11 @@ 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, do_not_insert=False): - if not frappe.db.exists('Issue Type', issue_type): - frappe.get_doc(dict(doctype='Issue Type', name=issue_type)).insert() +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", From defa01edac2ebb52047f4fe060f23a9e81db0d3c Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 3 Dec 2021 16:22:10 +0530 Subject: [PATCH 13/23] fix: undo removing of resolution_time fields --- erpnext/support/doctype/issue/issue.json | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/erpnext/support/doctype/issue/issue.json b/erpnext/support/doctype/issue/issue.json index 1da22fd58f98..38b5395adfd7 100644 --- a/erpnext/support/doctype/issue/issue.json +++ b/erpnext/support/doctype/issue/issue.json @@ -42,6 +42,8 @@ "opening_date", "opening_time", "resolution_date", + "resolution_time", + "user_resolution_time", "additional_info", "lead", "contact", @@ -345,16 +347,16 @@ "read_only": 1 }, { - "fieldname": "resolution_time", - "fieldtype": "Duration", - "label": "Resolution Time", - "read_only": 1 + "fieldname": "resolution_time", + "fieldtype": "Duration", + "label": "Resolution Time", + "read_only": 1 }, { - "fieldname": "user_resolution_time", - "fieldtype": "Duration", - "label": "User Resolution Time", - "read_only": 1 + "fieldname": "user_resolution_time", + "fieldtype": "Duration", + "label": "User Resolution Time", + "read_only": 1 }, { "fieldname": "on_hold_since", From f9408d170a8f7e863144003e5966f7b5a3219d3d Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 12:38:25 +0530 Subject: [PATCH 14/23] patch: split 'ongoing' sla status --- erpnext/patches.txt | 1 + .../rename_ongoing_status_in_sla_documents.py | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 897e70ce256a..d85f2339ae6e 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -312,3 +312,4 @@ 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 \ No newline at end of file diff --git a/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py b/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py new file mode 100644 index 000000000000..dddf4a3778d2 --- /dev/null +++ b/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: + query = ( + frappe.qb + .update(doctype) + .set(doctype.agreement_status, 'First Response Due') + .where( + (doctype.first_responded_on.isnull()) | (doctype.first_responded_on == '') + ) + ) + query.run() + query = ( + frappe.qb + .update(doctype) + .set(doctype.agreement_status, 'Resolution Due') + .where(doctype.agreement_status == 'Ongoing') + ) + query.run() + except Exception as e: + frappe.log_error('Failed to Patch SLA Status') \ No newline at end of file From c1d8877a838775ed04fb210ae629c34371ffe17b Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 14:34:59 +0530 Subject: [PATCH 15/23] refactor: handle special cases on communication creation --- erpnext/hooks.py | 2 +- .../service_level_agreement.py | 34 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/erpnext/hooks.py b/erpnext/hooks.py index 546166fe1927..09037f0f6c88 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -234,7 +234,7 @@ }, "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" ] }, diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 662e42cc9f59..9ae2d64d6f4b 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -376,14 +376,14 @@ def process_sla(doc, sla): doc.service_level_agreement = sla.name 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, sla.apply_sla_for_resolution) + 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, prev_status, 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) @@ -407,7 +407,7 @@ def calculate_hold_hours(): 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): + if ((is_open_status(prev_status) and not is_open_status(doc.status)) or doc.flags.on_first_reply): # status changed from Open to something else if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'): doc.first_responded_on = now_time @@ -625,8 +625,8 @@ def reset_resolution_metrics(doc): # called via hooks on communication update -def update_hold_time(doc, status): - if doc.communication_type == "Comment" or doc.sent_or_received != "Received": +def on_communication_update(doc, status): + if doc.communication_type == "Comment": return parent = get_parent_doc(doc) @@ -638,7 +638,27 @@ def update_hold_time(doc, status): for_resolution = frappe.db.get_value('Service Level Agreement', parent.service_level_agreement, 'apply_sla_for_resolution') - handle_status_change(parent, 'Replied', for_resolution) + if ( + doc.sent_or_received == "Received" # a reply is received + and parent.get('status') == 'Open' # issue status is set as open from communication.py + 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) From 812572d250e5dd18ebdd9e043826ba7616e504fe Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 14:35:35 +0530 Subject: [PATCH 16/23] feat: record assignment on first response failure --- erpnext/support/doctype/issue/test_issue.py | 38 +++++++++++++++++++ .../service_level_agreement.py | 22 +++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/erpnext/support/doctype/issue/test_issue.py b/erpnext/support/doctype/issue/test_issue.py index d566f33f24ab..477ac7260ce1 100644 --- a/erpnext/support/doctype/issue/test_issue.py +++ b/erpnext/support/doctype/issue/test_issue.py @@ -211,6 +211,43 @@ def test_issue_open_after_closed(self): 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.get_last_doc('Comment') + self.assertTrue('First Response SLA Failed' in comment.content) + + 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 @@ -425,6 +462,7 @@ def test_first_response_time_case29(self): 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() diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 9ae2d64d6f4b..19aa5783f790 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -397,6 +397,12 @@ def is_fulfilled_status(status): 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 @@ -408,9 +414,7 @@ def calculate_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): - # status changed from Open to something else - if doc.meta.has_field("first_responded_on") and not doc.get('first_responded_on'): - doc.first_responded_on = now_time + set_first_response() # Open to Replied if is_open_status(prev_status) and is_hold_status(doc.status): @@ -688,6 +692,18 @@ def set_resolution_by(doc, start_date_time, priority): 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 = _(f'First Response SLA Failed by {assigned_users}') + doc.add_comment( + comment_type='Assigned', + text=message + ) + + def get_service_level_agreement_fields(): return [ { From 32c81818f619c25878ab5620b3f5e505ef43c1e7 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 15:38:01 +0530 Subject: [PATCH 17/23] fix: transalations --- .../doctype/service_level_agreement/service_level_agreement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 19aa5783f790..7527adf2bf8b 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -697,7 +697,7 @@ def record_assigned_users_on_failure(doc): if assigned_users: from frappe.utils import get_fullname assigned_users = ', '.join((get_fullname(user) for user in assigned_users)) - message = _(f'First Response SLA Failed by {assigned_users}') + message = _('First Response SLA Failed by {}').format(assigned_users) doc.add_comment( comment_type='Assigned', text=message From 10b87e081cd0523fe63f7c6254d683ea4fb48439 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 15:38:19 +0530 Subject: [PATCH 18/23] fix: sider issues --- .../rename_ongoing_status_in_sla_documents.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py b/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py index dddf4a3778d2..b5296fbdb0cc 100644 --- a/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py +++ b/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py @@ -7,21 +7,21 @@ def execute(): for doctype in active_sla_documents: doctype = frappe.qb.DocType(doctype) try: - query = ( - frappe.qb - .update(doctype) - .set(doctype.agreement_status, 'First Response Due') - .where( - (doctype.first_responded_on.isnull()) | (doctype.first_responded_on == '') - ) - ) - query.run() - query = ( - frappe.qb - .update(doctype) - .set(doctype.agreement_status, 'Resolution Due') - .where(doctype.agreement_status == 'Ongoing') - ) - query.run() - except Exception as e: + frappe.qb.update( + doctype + ).set( + doctype.agreement_status, 'First Response Due' + ).where( + (doctype.first_responded_on.isnull()) | (doctype.first_responded_on == '') + ).run() + + frappe.qb.update( + doctype + ).set( + doctype.agreement_status, 'Resolution Due' + ).where( + doctype.agreement_status == 'Ongoing' + ).run() + + except Exception: frappe.log_error('Failed to Patch SLA Status') \ No newline at end of file From 1a76b3801ac1e933087b2cf14a1a3e74c6be8383 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 16:16:13 +0530 Subject: [PATCH 19/23] fix: test --- erpnext/support/doctype/issue/test_issue.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/erpnext/support/doctype/issue/test_issue.py b/erpnext/support/doctype/issue/test_issue.py index 477ac7260ce1..14cec46ad4fc 100644 --- a/erpnext/support/doctype/issue/test_issue.py +++ b/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 @@ -231,8 +231,13 @@ def test_recording_of_assignment_on_first_reponse_failure(self): # assert if a new timeline item has been added # to record the assignment - comment = frappe.get_last_doc('Comment') - self.assertTrue('First Response SLA Failed' in comment.content) + 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") From 476e81a6312f9d38ae8b525b52f47317cbceefe5 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 18:55:30 +0530 Subject: [PATCH 20/23] fix: time to respond & resolve indicators --- .../rename_ongoing_status_in_sla_documents.py | 4 ++-- erpnext/public/js/utils.js | 15 ++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py b/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py index b5296fbdb0cc..1cc5f38f4272 100644 --- a/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py +++ b/erpnext/patches/v14_0/rename_ongoing_status_in_sla_documents.py @@ -12,7 +12,7 @@ def execute(): ).set( doctype.agreement_status, 'First Response Due' ).where( - (doctype.first_responded_on.isnull()) | (doctype.first_responded_on == '') + doctype.first_responded_on.isnull() ).run() frappe.qb.update( @@ -24,4 +24,4 @@ def execute(): ).run() except Exception: - frappe.log_error('Failed to Patch SLA Status') \ No newline at end of file + frappe.log_error(title='Failed to Patch SLA Status') \ No newline at end of file diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js index f0facdd3a102..2d8b1be93f09 100755 --- a/erpnext/public/js/utils.js +++ b/erpnext/public/js/utils.js @@ -831,7 +831,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: { @@ -884,8 +884,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); - if (!frm.doc.first_responded_on && frm.doc.agreement_status === 'Ongoing') { + let time_to_respond = get_status(frm.doc.response_by); + if (!frm.doc.first_responded_on) { time_to_respond = get_time_left(frm.doc.response_by, frm.doc.agreement_status); } @@ -899,8 +899,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); - if (!frm.doc.resolution_date && frm.doc.agreement_status === 'Ongoing') { + let time_to_resolve = get_status(frm.doc.resolution_by); + if (!frm.doc.resolution_date) { time_to_resolve = get_time_left(frm.doc.resolution_by, frm.doc.agreement_status); } @@ -924,8 +924,9 @@ function get_time_left(timestamp, agreement_status) { return {'diff_display': diff_display, 'indicator': indicator}; } -function get_status(variance) { - if (variance > 0) { +function get_status(timestamp) { + const time_left = moment(timestamp).diff(moment()); + if (time_left >= 0) { return {'diff_display': 'Fulfilled', 'indicator': 'green'}; } else { return {'diff_display': 'Failed', 'indicator': 'red'}; From 91aa78707c471f688acb1147956f5aac3b84af99 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 19:13:31 +0530 Subject: [PATCH 21/23] fix: remove missed 'ongoing' references --- erpnext/support/doctype/issue/issue.json | 2 +- .../doctype/service_level_agreement/service_level_agreement.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/support/doctype/issue/issue.json b/erpnext/support/doctype/issue/issue.json index 38b5395adfd7..30aa7319dfc5 100644 --- a/erpnext/support/doctype/issue/issue.json +++ b/erpnext/support/doctype/issue/issue.json @@ -372,7 +372,7 @@ "read_only": 1 }, { - "default": "Ongoing", + "default": "First Response Due", "depends_on": "eval: doc.service_level_agreement", "fieldname": "agreement_status", "fieldtype": "Select", diff --git a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py index 7527adf2bf8b..50f31fde2d64 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -625,7 +625,7 @@ def reset_resolution_metrics(doc): doc.user_resolution_time = None if doc.meta.has_field("agreement_status"): - doc.agreement_status = "Ongoing" + doc.agreement_status = "First Response Due" # called via hooks on communication update From 3446f7545ff8178999a93fd2042c188905de1db2 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 19:17:38 +0530 Subject: [PATCH 22/23] fix: remove 'ongoing' status from issue summary report --- erpnext/support/report/issue_summary/issue_summary.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/support/report/issue_summary/issue_summary.py b/erpnext/support/report/issue_summary/issue_summary.py index 39a5c407cd4e..67fe345d5fe6 100644 --- a/erpnext/support/report/issue_summary/issue_summary.py +++ b/erpnext/support/report/issue_summary/issue_summary.py @@ -82,7 +82,8 @@ def get_columns(self): 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(): From 2f7d8ac29e58898e7fe7bd05b460abdcc1ea7a5a Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 6 Dec 2021 19:19:34 +0530 Subject: [PATCH 23/23] fix: indentation --- erpnext/support/doctype/issue/issue.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/erpnext/support/doctype/issue/issue.json b/erpnext/support/doctype/issue/issue.json index 30aa7319dfc5..3ff7d02f1aef 100644 --- a/erpnext/support/doctype/issue/issue.json +++ b/erpnext/support/doctype/issue/issue.json @@ -347,16 +347,16 @@ "read_only": 1 }, { - "fieldname": "resolution_time", - "fieldtype": "Duration", - "label": "Resolution Time", - "read_only": 1 + "fieldname": "resolution_time", + "fieldtype": "Duration", + "label": "Resolution Time", + "read_only": 1 }, { - "fieldname": "user_resolution_time", - "fieldtype": "Duration", - "label": "User Resolution Time", - "read_only": 1 + "fieldname": "user_resolution_time", + "fieldtype": "Duration", + "label": "User Resolution Time", + "read_only": 1 }, { "fieldname": "on_hold_since",