From 0683337f146b583425be369b5e0035439f601b76 Mon Sep 17 00:00:00 2001 From: Saqib Date: Mon, 12 Jul 2021 20:21:07 +0530 Subject: [PATCH] fix: incorrect response by variance & resolution by variance (#26173) --- erpnext/patches.txt | 1 + .../v13_0/update_response_by_variance.py | 31 +++++++++++++++ .../service_level_agreement.js | 4 +- .../service_level_agreement.json | 2 +- .../service_level_agreement.py | 4 +- .../test_service_level_agreement.py | 38 ++++++++++++++++++- .../service_level_priority.json | 8 ++-- 7 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 erpnext/patches/v13_0/update_response_by_variance.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 986b0c5711..c93f7a7ed9 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -290,5 +290,6 @@ erpnext.patches.v13_0.add_doctype_to_sla #14-06-2021 erpnext.patches.v13_0.set_training_event_attendance erpnext.patches.v13_0.bill_for_rejected_quantity_in_purchase_invoice erpnext.patches.v13_0.rename_issue_status_hold_to_on_hold +erpnext.patches.v13_0.update_response_by_variance erpnext.patches.v13_0.bill_for_rejected_quantity_in_purchase_invoice erpnext.patches.v13_0.update_job_card_details diff --git a/erpnext/patches/v13_0/update_response_by_variance.py b/erpnext/patches/v13_0/update_response_by_variance.py new file mode 100644 index 0000000000..ef4d976383 --- /dev/null +++ b/erpnext/patches/v13_0/update_response_by_variance.py @@ -0,0 +1,31 @@ +# Copyright (c) 2020, Frappe and Contributors +# License: GNU General Public License v3. See license.txt + +from __future__ import unicode_literals +import frappe + +def execute(): + if frappe.db.exists('DocType', 'Issue') and frappe.db.count('Issue'): + invalid_issues = frappe.get_all('Issue', { + 'first_responded_on': ['is', 'set'], + 'response_by_variance': ['<', 0] + }, ["name", "response_by_variance", "timestampdiff(Second, `first_responded_on`, `response_by`) as variance"]) + + # issues which has response_by_variance set as -ve + # but diff between first_responded_on & response_by is +ve i.e SLA isn't failed + invalid_issues = [d for d in invalid_issues if d.get('variance') > 0] + + for issue in invalid_issues: + frappe.db.set_value('Issue', issue.get('name'), 'response_by_variance', issue.get('variance'), update_modified=False) + + invalid_issues = frappe.get_all('Issue', { + 'resolution_date': ['is', 'set'], + 'resolution_by_variance': ['<', 0] + }, ["name", "resolution_by_variance", "timestampdiff(Second, `resolution_date`, `resolution_by`) as variance"]) + + # issues which has resolution_by_variance set as -ve + # but diff between resolution_date & resolution_by is +ve i.e SLA isn't failed + invalid_issues = [d for d in invalid_issues if d.get('variance') > 0] + + for issue in invalid_issues: + frappe.db.set_value('Issue', issue.get('name'), 'resolution_by_variance', issue.get('variance'), update_modified=False) 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 308bce48df..ae2080c3b5 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.js +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.js @@ -5,15 +5,15 @@ frappe.ui.form.on('Service Level Agreement', { setup: function(frm) { if (cint(frm.doc.apply_sla_for_resolution) === 1) { frm.get_field('priorities').grid.editable_fields = [ - {fieldname: 'priority', columns: 1}, {fieldname: 'default_priority', columns: 1}, + {fieldname: 'priority', columns: 2}, {fieldname: 'response_time', columns: 2}, {fieldname: 'resolution_time', columns: 2} ]; } else { frm.get_field('priorities').grid.editable_fields = [ - {fieldname: 'priority', columns: 1}, {fieldname: 'default_priority', columns: 1}, + {fieldname: 'priority', columns: 2}, {fieldname: 'response_time', columns: 3}, ]; } 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 de3389aa42..ef14b29896 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.json +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.json @@ -1,6 +1,6 @@ { "actions": [], - "autoname": "format:SLA-{document_type}-{service_level}-{####}", + "autoname": "format:SLA-{document_type}-{service_level}", "creation": "2018-12-26 21:08:15.448812", "doctype": "DocType", "editable_grid": 1, 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 60e5fbe80e..8739cb2364 100644 --- a/erpnext/support/doctype/service_level_agreement/service_level_agreement.py +++ b/erpnext/support/doctype/service_level_agreement/service_level_agreement.py @@ -797,7 +797,7 @@ def set_response_by_and_variance(doc, meta, start_date_time, priority): if meta.has_field("response_by"): doc.response_by = get_expected_time_for(parameter="response", service_level=priority, start_date_time=start_date_time) - if meta.has_field("response_by_variance"): + if meta.has_field("response_by_variance") and not doc.get('first_responded_on'): now_time = frappe.flags.current_time or now_datetime(doc.get("owner")) doc.response_by_variance = round(time_diff_in_seconds(doc.response_by, now_time), 2) @@ -805,7 +805,7 @@ def set_resolution_by_and_variance(doc, meta, start_date_time, priority): if meta.has_field("resolution_by"): doc.resolution_by = get_expected_time_for(parameter="resolution", service_level=priority, start_date_time=start_date_time) - if meta.has_field("resolution_by_variance"): + if meta.has_field("resolution_by_variance") and not doc.get("resolution_date"): now_time = frappe.flags.current_time or now_datetime(doc.get("owner")) doc.resolution_by_variance = round(time_diff_in_seconds(doc.resolution_by, now_time), 2) 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 7c18a6577f..865fadc97c 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 @@ -217,6 +217,42 @@ class TestServiceLevelAgreement(unittest.TestCase): lead.reload() self.assertEqual(lead.agreement_status, 'Fulfilled') + def test_changing_of_variance_after_response(self): + # create lead + doctype = "Lead" + lead_sla = create_service_level_agreement( + default_service_level_agreement=1, + holiday_list="__Test Holiday List", + entity_type=None, entity=None, + response_time=14400, + doctype=doctype, + sla_fulfilled_on=[{"status": "Replied"}], + apply_sla_for_resolution=0 + ) + creation = datetime.datetime(2019, 3, 4, 12, 0) + lead = make_lead(creation=creation, index=2) + self.assertEqual(lead.service_level_agreement, lead_sla.name) + + # set lead as replied to set first responded on + frappe.flags.current_time = datetime.datetime(2019, 3, 4, 15, 30) + lead.reload() + lead.status = 'Replied' + lead.save() + lead.reload() + self.assertEqual(lead.agreement_status, 'Fulfilled') + + # check response_by_variance + self.assertEqual(lead.first_responded_on, frappe.flags.current_time) + self.assertEqual(lead.response_by_variance, 1800.0) + + # make a change on the document & + # check response_by_variance is unchanged + frappe.flags.current_time = datetime.datetime(2019, 3, 4, 18, 30) + lead.status = 'Open' + lead.save() + lead.reload() + self.assertEqual(lead.response_by_variance, 1800.0) + def tearDown(self): for d in frappe.get_all("Service Level Agreement"): frappe.delete_doc("Service Level Agreement", d.name, force=1) @@ -249,7 +285,7 @@ def create_service_level_agreement(default_service_level_agreement, holiday_list "doctype": "Service Level Agreement", "enabled": 1, "document_type": doctype, - "service_level": "__Test Service Level", + "service_level": "__Test {} SLA".format(entity_type if entity_type else "Default"), "default_service_level_agreement": default_service_level_agreement, "default_priority": "Medium", "holiday_list": holiday_list, diff --git a/erpnext/support/doctype/service_level_priority/service_level_priority.json b/erpnext/support/doctype/service_level_priority/service_level_priority.json index 0367fc6d88..b410fe6660 100644 --- a/erpnext/support/doctype/service_level_priority/service_level_priority.json +++ b/erpnext/support/doctype/service_level_priority/service_level_priority.json @@ -5,9 +5,9 @@ "editable_grid": 1, "engine": "InnoDB", "field_order": [ - "priority", - "cb_01", "default_priority", + "cb_01", + "priority", "sb_00", "response_time", "cb_00", @@ -15,7 +15,7 @@ ], "fields": [ { - "columns": 1, + "columns": 2, "fieldname": "priority", "fieldtype": "Link", "in_list_view": 1, @@ -64,7 +64,7 @@ ], "istable": 1, "links": [], - "modified": "2021-05-29 19:52:51.733248", + "modified": "2021-06-21 12:00:58.089962", "modified_by": "Administrator", "module": "Support", "name": "Service Level Priority",