From c89c082f34efa874db27b49e3b2bd655da52ad1d Mon Sep 17 00:00:00 2001 From: shariquerik Date: Fri, 22 Jan 2021 19:33:35 +0530 Subject: [PATCH 01/23] fix: Average Chart compute inaccurate average --- .../dashboard_chart/dashboard_chart.py | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index b19f6cf9f0..33eebe0721 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -184,6 +184,7 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): fields = [ '{} as _unit'.format(datefield), '{aggregate_function}({value_field})'.format(aggregate_function=aggregate_function, value_field=value_field), + 'COUNT({value_field})'.format(value_field=value_field) ], filters = filters, group_by = '_unit', @@ -192,7 +193,10 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): ignore_ifnull = True ) - result = get_result(data, timegrain, from_date, to_date) + if chart.chart_type == 'Average': + result = get_avg_result(data, timegrain, from_date, to_date) + else: + result = get_result(data, timegrain, from_date, to_date) chart_config = { "labels": [get_period(r[0], timegrain) for r in result], @@ -300,6 +304,23 @@ def get_result(data, timegrain, from_date, to_date): return result +def get_avg_result(data, timegrain, from_date, to_date): + dates = get_dates_from_timegrain(from_date, to_date, timegrain) + result = [[date, 0] for date in dates] + data_index, dividend, divisor = 0, 0, 0 + if data: + for i, d in enumerate(result): + while data_index < len(data) and getdate(data[data_index][0]) <= d[0]: + dividend += data[data_index][1] * data[data_index][2] + divisor += data[data_index][2] + data_index += 1 + if dividend == 0: + d[1] = dividend + else: + d[1] = dividend / divisor + + return result + @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_charts_for_user(doctype, txt, searchfield, start, page_len, filters): From 6b5cb1b2c25e5bb9255f40d163c5a3750ab67eec Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 1 Feb 2021 14:01:47 +0530 Subject: [PATCH 02/23] fix: Handled code duplication --- .../dashboard_chart/dashboard_chart.py | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index 33eebe0721..d858402442 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -171,7 +171,6 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): doctype = chart.document_type datefield = chart.based_on - aggregate_function = get_aggregate_function(chart.chart_type) value_field = chart.value_based_on or '1' from_date = from_date.strftime('%Y-%m-%d') to_date = to_date @@ -183,7 +182,7 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): doctype, fields = [ '{} as _unit'.format(datefield), - '{aggregate_function}({value_field})'.format(aggregate_function=aggregate_function, value_field=value_field), + 'SUM({value_field})'.format(value_field=value_field), 'COUNT({value_field})'.format(value_field=value_field) ], filters = filters, @@ -193,10 +192,7 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): ignore_ifnull = True ) - if chart.chart_type == 'Average': - result = get_avg_result(data, timegrain, from_date, to_date) - else: - result = get_result(data, timegrain, from_date, to_date) + result = get_result(data, timegrain, from_date, to_date, chart.chart_type) chart_config = { "labels": [get_period(r[0], timegrain) for r in result], @@ -292,32 +288,20 @@ def get_aggregate_function(chart_type): }[chart_type] -def get_result(data, timegrain, from_date, to_date): +def get_result(data, timegrain, from_date, to_date, chart_type): dates = get_dates_from_timegrain(from_date, to_date, timegrain) result = [[date, 0] for date in dates] - data_index = 0 + data_index, count = 0, 0 if data: for i, d in enumerate(result): while data_index < len(data) and getdate(data[data_index][0]) <= d[0]: d[1] += data[data_index][1] + count += data[data_index][2] data_index += 1 - - return result - -def get_avg_result(data, timegrain, from_date, to_date): - dates = get_dates_from_timegrain(from_date, to_date, timegrain) - result = [[date, 0] for date in dates] - data_index, dividend, divisor = 0, 0, 0 - if data: - for i, d in enumerate(result): - while data_index < len(data) and getdate(data[data_index][0]) <= d[0]: - dividend += data[data_index][1] * data[data_index][2] - divisor += data[data_index][2] - data_index += 1 - if dividend == 0: - d[1] = dividend - else: - d[1] = dividend / divisor + if chart_type == 'Average' and not count == 0: + d[1] = d[1]/count + if chart_type == 'Count': + d[1] = count return result From c8849a642299118d3774aaeb6c341a0c718b1f7f Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Fri, 5 Feb 2021 12:28:23 +0530 Subject: [PATCH 03/23] Implemented Suggestion Co-authored-by: Prssanna Desai --- frappe/desk/doctype/dashboard_chart/dashboard_chart.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index d858402442..47047a100a 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -291,9 +291,10 @@ def get_aggregate_function(chart_type): def get_result(data, timegrain, from_date, to_date, chart_type): dates = get_dates_from_timegrain(from_date, to_date, timegrain) result = [[date, 0] for date in dates] - data_index, count = 0, 0 + data_index = 0 if data: for i, d in enumerate(result): + count = 0 while data_index < len(data) and getdate(data[data_index][0]) <= d[0]: d[1] += data[data_index][1] count += data[data_index][2] From 640b33450ecbe63de218392b4e1b5fbe351dbd3a Mon Sep 17 00:00:00 2001 From: shariquerik Date: Wed, 10 Feb 2021 19:48:39 +0530 Subject: [PATCH 04/23] fix: added test for avg calculation --- .../dashboard_chart/test_dashboard_chart.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py index 3c37ad4a09..fbab33bdb6 100644 --- a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py @@ -212,6 +212,37 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() + def test_avg_dashboard_chart(self): + insert_test_records() + + if frappe.db.exists('Dashboard Chart', 'Test Average Dashboard Chart'): + frappe.delete_doc('Dashboard Chart', 'Test Average Dashboard Chart') + + frappe.get_doc(dict( + doctype = 'Dashboard Chart', + chart_name = 'Test Average Dashboard Chart', + chart_type = 'Average', + document_type = 'Communication', + based_on = 'communication_date', + value_based_on = 'rating', + timespan = 'Select Date Range', + time_interval = 'Weekly', + from_date = datetime(2018, 12, 30), + to_date = datetime(2019, 1, 15), + filters_json = '[]', + timeseries = 1 + )).insert() + + result = get(chart_name='Test Average Dashboard Chart', refresh = 1) + + self.assertEqual(result.get('datasets')[0].get('values'), [50.0, 150.0, 266.6666666666667, 0.0]) + self.assertEqual( + result.get('labels'), + ['30-12-18', '06-01-19', '13-01-19', '20-01-19'] + ) + + frappe.db.rollback() + def insert_test_records(): create_new_communication(datetime(2018, 12, 30), 50) create_new_communication(datetime(2019, 1, 4), 100) From 9fa4d8d39af2a544dd4c1d17b0e2722ace04c1de Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 8 Mar 2021 11:49:27 +0530 Subject: [PATCH 05/23] refactor: removed unnecessary code --- frappe/desk/doctype/dashboard_chart/dashboard_chart.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index 47047a100a..9f48e13c77 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -182,8 +182,8 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): doctype, fields = [ '{} as _unit'.format(datefield), - 'SUM({value_field})'.format(value_field=value_field), - 'COUNT({value_field})'.format(value_field=value_field) + 'SUM({})'.format(value_field), + 'COUNT({})'.format(value_field) ], filters = filters, group_by = '_unit', From 00f6c05c73fe8a9216db29f407f2b0d88a191b94 Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 9 Mar 2021 16:29:58 +0530 Subject: [PATCH 06/23] fix: use count(*) --- frappe/desk/doctype/dashboard_chart/dashboard_chart.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index 9f48e13c77..48b34e6cd9 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -183,7 +183,7 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): fields = [ '{} as _unit'.format(datefield), 'SUM({})'.format(value_field), - 'COUNT({})'.format(value_field) + 'COUNT(*)' ], filters = filters, group_by = '_unit', From 36923b530b1f44e4eb600ddeb0338ad395b7e04d Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 10 Mar 2021 14:17:32 +0530 Subject: [PATCH 07/23] test: Explicitly begin transaction before test to allow rollback --- frappe/test_runner.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index b66a96595d..e620b46e4a 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -67,6 +67,8 @@ def main(app=None, module=None, doctype=None, verbose=False, tests=(), for fn in frappe.get_hooks("before_tests", app_name=app): frappe.get_attr(fn)() + frappe.db.begin() + if doctype: ret = run_tests_for_doctype(doctype, verbose, tests, force, profile, junit_xml_output=junit_xml_output) elif module: From 459f65ba189ef36e611dc175bb19a2efda35a9dd Mon Sep 17 00:00:00 2001 From: prssanna Date: Wed, 10 Mar 2021 16:17:59 +0530 Subject: [PATCH 08/23] test: explicitly start transaction before each test --- frappe/test_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index e620b46e4a..0e81d69593 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -67,8 +67,6 @@ def main(app=None, module=None, doctype=None, verbose=False, tests=(), for fn in frappe.get_hooks("before_tests", app_name=app): frappe.get_attr(fn)() - frappe.db.begin() - if doctype: ret = run_tests_for_doctype(doctype, verbose, tests, force, profile, junit_xml_output=junit_xml_output) elif module: @@ -181,6 +179,8 @@ def run_tests_for_module(module, verbose=False, tests=(), profile=False, junit_x return _run_unittest(module, verbose=verbose, tests=tests, profile=profile, junit_xml_output=junit_xml_output) def _run_unittest(modules, verbose=False, tests=(), profile=False, junit_xml_output=False): + frappe.db.begin() + test_suite = unittest.TestSuite() if not isinstance(modules, (list, tuple)): From 30706a6690098d440a478fe5edac1fd265190fa8 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 16 Mar 2021 11:30:31 +0530 Subject: [PATCH 09/23] fix: check if communication exist before creating --- .../dashboard_chart/test_dashboard_chart.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py index fbab33bdb6..b127a7c912 100644 --- a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py @@ -244,18 +244,20 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def insert_test_records(): - create_new_communication(datetime(2018, 12, 30), 50) - create_new_communication(datetime(2019, 1, 4), 100) - create_new_communication(datetime(2019, 1, 6), 200) - create_new_communication(datetime(2019, 1, 7), 400) - create_new_communication(datetime(2019, 1, 8), 300) - create_new_communication(datetime(2019, 1, 10), 100) + create_new_communication('Communication 1', datetime(2018, 12, 30), 50) + create_new_communication('Communication 2', datetime(2019, 1, 4), 100) + create_new_communication('Communication 3', datetime(2019, 1, 6), 200) + create_new_communication('Communication 4', datetime(2019, 1, 7), 400) + create_new_communication('Communication 5', datetime(2019, 1, 8), 300) + create_new_communication('Communication 6', datetime(2019, 1, 10), 100) -def create_new_communication(date, rating): +def create_new_communication(subject, date, rating): communication = { 'doctype': 'Communication', - 'subject': 'Test Communication', + 'subject': subject, 'rating': rating, 'communication_date': date } - frappe.get_doc(communication).insert() + comm = frappe.get_doc(communication) + if not frappe.db.exists("Communication", {'subject' : comm.subject }): + comm.insert() From fb1eac23c5b95dfdace1291ac640181f0cb93e6e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Mar 2021 11:49:37 +0530 Subject: [PATCH 10/23] docs: update docs for linked_with functions --- frappe/desk/form/linked_with.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 733ee1774c..767b414696 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -79,17 +79,18 @@ def get_submitted_linked_docs(doctype, name, docs=None, visited=None): @frappe.whitelist() def cancel_all_linked_docs(docs, ignore_doctypes_on_cancel_all=[]): """ - Cancel all linked doctype + Cancel all linked doctype, optionally ignore doctypes specified in a list. Arguments: - docs (str) - It contains all list of dictionaries of a linked documents. + docs (json str) - It contains list of dictionaries of a linked documents. + ignore_doctypes_on_cancel_all (list) - List of doctypes to ignore while cancelling. """ docs = json.loads(docs) if isinstance(ignore_doctypes_on_cancel_all, string_types): ignore_doctypes_on_cancel_all = json.loads(ignore_doctypes_on_cancel_all) for i, doc in enumerate(docs, 1): - if validate_linked_doc(doc, ignore_doctypes_on_cancel_all) is True: + if validate_linked_doc(doc, ignore_doctypes_on_cancel_all): frappe.publish_progress(percent=i * 100 / ((len(docs) - len(ignore_doctypes_on_cancel_all))), title=_("Cancelling documents")) linked_doc = frappe.get_doc(doc.get("doctype"), doc.get("name")) linked_doc.cancel() @@ -99,8 +100,9 @@ def validate_linked_doc(docinfo, ignore_doctypes_on_cancel_all=[]): """ Validate a document to be submitted and non-exempted from auto-cancel. - Args: - docs (dict): The document to check for submitted and non-exempt from auto-cancel + Arguments: + docinfo (dict): The document to check for submitted and non-exempt from auto-cancel + ignore_doctypes_on_cancel_all (list) - List of doctypes to ignore while cancelling. Returns: bool: True if linked document passes all validations, else False From e1e27144b7afc927a40a5f844d5b10d4b5f18c11 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 16 Mar 2021 12:19:49 +0530 Subject: [PATCH 11/23] fix: using test_records.json to create communication records --- .../doctype/communication/test_records.json | 42 +++++++++++++++++++ .../dashboard_chart/test_dashboard_chart.py | 24 +---------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/frappe/core/doctype/communication/test_records.json b/frappe/core/doctype/communication/test_records.json index a69d3e9570..914febb225 100644 --- a/frappe/core/doctype/communication/test_records.json +++ b/frappe/core/doctype/communication/test_records.json @@ -6,5 +6,47 @@ "sent_or_received": "Received", "parenttype": "User", "parent": "Administrator" + }, + { + "doctype": "Communication", + "name": "_Test Communication 2", + "subject": "Test Communication", + "rating": 50, + "communication_date": "2018-12-30" + }, + { + "doctype": "Communication", + "name": "_Test Communication 3", + "subject": "Test Communication", + "rating": 100, + "communication_date": "2019-1-4" + }, + { + "doctype": "Communication", + "name": "_Test Communication 4", + "subject": "Test Communication", + "rating": 200, + "communication_date": "2019-1-6" + }, + { + "doctype": "Communication", + "name": "_Test Communication 5", + "subject": "Test Communication", + "rating": 400, + "communication_date": "2019-1-7" + }, + { + "doctype": "Communication", + "name": "_Test Communication 6", + "subject": "Test Communication", + "rating": 300, + "communication_date": "2019-1-8" + }, + { + "doctype": "Communication", + "name": "_Test Communication 7", + "subject": "Test Communication", + "rating": 100, + "communication_date": "2019-1-10" } ] diff --git a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py index b127a7c912..a1b460328f 100644 --- a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py @@ -11,6 +11,8 @@ from frappe.desk.doctype.dashboard_chart.dashboard_chart import get from datetime import datetime from dateutil.relativedelta import relativedelta +test_dependencies = ["Communication"] + class TestDashboardChart(unittest.TestCase): def test_period_ending(self): self.assertEqual(get_period_ending('2019-04-10', 'Daily'), @@ -151,7 +153,6 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def test_daily_dashboard_chart(self): - insert_test_records() if frappe.db.exists('Dashboard Chart', 'Test Daily Dashboard Chart'): frappe.delete_doc('Dashboard Chart', 'Test Daily Dashboard Chart') @@ -182,7 +183,6 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def test_weekly_dashboard_chart(self): - insert_test_records() if frappe.db.exists('Dashboard Chart', 'Test Weekly Dashboard Chart'): frappe.delete_doc('Dashboard Chart', 'Test Weekly Dashboard Chart') @@ -213,7 +213,6 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def test_avg_dashboard_chart(self): - insert_test_records() if frappe.db.exists('Dashboard Chart', 'Test Average Dashboard Chart'): frappe.delete_doc('Dashboard Chart', 'Test Average Dashboard Chart') @@ -242,22 +241,3 @@ class TestDashboardChart(unittest.TestCase): ) frappe.db.rollback() - -def insert_test_records(): - create_new_communication('Communication 1', datetime(2018, 12, 30), 50) - create_new_communication('Communication 2', datetime(2019, 1, 4), 100) - create_new_communication('Communication 3', datetime(2019, 1, 6), 200) - create_new_communication('Communication 4', datetime(2019, 1, 7), 400) - create_new_communication('Communication 5', datetime(2019, 1, 8), 300) - create_new_communication('Communication 6', datetime(2019, 1, 10), 100) - -def create_new_communication(subject, date, rating): - communication = { - 'doctype': 'Communication', - 'subject': subject, - 'rating': rating, - 'communication_date': date - } - comm = frappe.get_doc(communication) - if not frappe.db.exists("Communication", {'subject' : comm.subject }): - comm.insert() From 4a47720801b284ab95916ab2bda4d2be7b6cedee Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 16 Mar 2021 14:17:21 +0530 Subject: [PATCH 12/23] fix: check if communication exist before creating --- .../doctype/communication/test_records.json | 42 ------------------- .../dashboard_chart/test_dashboard_chart.py | 24 ++++++++++- 2 files changed, 22 insertions(+), 44 deletions(-) diff --git a/frappe/core/doctype/communication/test_records.json b/frappe/core/doctype/communication/test_records.json index 914febb225..a69d3e9570 100644 --- a/frappe/core/doctype/communication/test_records.json +++ b/frappe/core/doctype/communication/test_records.json @@ -6,47 +6,5 @@ "sent_or_received": "Received", "parenttype": "User", "parent": "Administrator" - }, - { - "doctype": "Communication", - "name": "_Test Communication 2", - "subject": "Test Communication", - "rating": 50, - "communication_date": "2018-12-30" - }, - { - "doctype": "Communication", - "name": "_Test Communication 3", - "subject": "Test Communication", - "rating": 100, - "communication_date": "2019-1-4" - }, - { - "doctype": "Communication", - "name": "_Test Communication 4", - "subject": "Test Communication", - "rating": 200, - "communication_date": "2019-1-6" - }, - { - "doctype": "Communication", - "name": "_Test Communication 5", - "subject": "Test Communication", - "rating": 400, - "communication_date": "2019-1-7" - }, - { - "doctype": "Communication", - "name": "_Test Communication 6", - "subject": "Test Communication", - "rating": 300, - "communication_date": "2019-1-8" - }, - { - "doctype": "Communication", - "name": "_Test Communication 7", - "subject": "Test Communication", - "rating": 100, - "communication_date": "2019-1-10" } ] diff --git a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py index a1b460328f..72ab18385d 100644 --- a/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py @@ -11,8 +11,6 @@ from frappe.desk.doctype.dashboard_chart.dashboard_chart import get from datetime import datetime from dateutil.relativedelta import relativedelta -test_dependencies = ["Communication"] - class TestDashboardChart(unittest.TestCase): def test_period_ending(self): self.assertEqual(get_period_ending('2019-04-10', 'Daily'), @@ -153,6 +151,7 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def test_daily_dashboard_chart(self): + insert_test_records() if frappe.db.exists('Dashboard Chart', 'Test Daily Dashboard Chart'): frappe.delete_doc('Dashboard Chart', 'Test Daily Dashboard Chart') @@ -183,6 +182,7 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def test_weekly_dashboard_chart(self): + insert_test_records() if frappe.db.exists('Dashboard Chart', 'Test Weekly Dashboard Chart'): frappe.delete_doc('Dashboard Chart', 'Test Weekly Dashboard Chart') @@ -213,6 +213,7 @@ class TestDashboardChart(unittest.TestCase): frappe.db.rollback() def test_avg_dashboard_chart(self): + insert_test_records() if frappe.db.exists('Dashboard Chart', 'Test Average Dashboard Chart'): frappe.delete_doc('Dashboard Chart', 'Test Average Dashboard Chart') @@ -241,3 +242,22 @@ class TestDashboardChart(unittest.TestCase): ) frappe.db.rollback() + +def insert_test_records(): + create_new_communication('Communication 1', datetime(2018, 12, 30), 50) + create_new_communication('Communication 2', datetime(2019, 1, 4), 100) + create_new_communication('Communication 3', datetime(2019, 1, 6), 200) + create_new_communication('Communication 4', datetime(2019, 1, 7), 400) + create_new_communication('Communication 5', datetime(2019, 1, 8), 300) + create_new_communication('Communication 6', datetime(2019, 1, 10), 100) + +def create_new_communication(subject, date, rating): + communication = { + 'doctype': 'Communication', + 'subject': subject, + 'rating': rating, + 'communication_date': date + } + comm = frappe.get_doc(communication) + if not frappe.db.exists("Communication", {'subject' : comm.subject}): + comm.insert() From 41a811af8c14c99fb7e987fa344d688fe2ca17b4 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 16 Mar 2021 17:33:05 +0530 Subject: [PATCH 13/23] feat: Client Script for List views --- .../doctype/client_script/client_script.json | 10 ++++++++- .../doctype/client_script/client_script.py | 18 +++++++++++++-- frappe/desk/form/meta.py | 22 ++++++++++++++++--- frappe/public/js/frappe/model/model.js | 3 +++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/frappe/custom/doctype/client_script/client_script.json b/frappe/custom/doctype/client_script/client_script.json index 57e6c68094..eca4fdb4b7 100644 --- a/frappe/custom/doctype/client_script/client_script.json +++ b/frappe/custom/doctype/client_script/client_script.json @@ -8,6 +8,7 @@ "engine": "InnoDB", "field_order": [ "dt", + "apply_to_view", "enabled", "script", "sample" @@ -43,13 +44,20 @@ "fieldname": "enabled", "fieldtype": "Check", "label": "Enabled" + }, + { + "default": "Form", + "fieldname": "apply_to_view", + "fieldtype": "Select", + "label": "Apply To", + "options": "List\nForm" } ], "icon": "fa fa-glass", "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-02-04 13:57:56.509437", + "modified": "2021-03-16 17:09:25.918859", "modified_by": "Administrator", "module": "Custom", "name": "Client Script", diff --git a/frappe/custom/doctype/client_script/client_script.py b/frappe/custom/doctype/client_script/client_script.py index e252e2a750..a84b9ce6e4 100644 --- a/frappe/custom/doctype/client_script/client_script.py +++ b/frappe/custom/doctype/client_script/client_script.py @@ -3,15 +3,29 @@ from __future__ import unicode_literals import frappe +from frappe import _ from frappe.model.document import Document + class ClientScript(Document): def autoname(self): - self.name = self.dt + self.name = f"{self.dt}-{self.apply_to_view}" + + def validate(self): + if not self.is_new(): + return + + exists = frappe.db.exists( + "Client Script", {"dt": self.dt, "apply_to_view": self.apply_to_view} + ) + if exists: + frappe.throw( + _("Client Script for {0} {1} already exists").format(frappe.bold(self.dt), self.apply_to_view), + frappe.DuplicateEntryError, + ) def on_update(self): frappe.clear_cache(doctype=self.dt) def on_trash(self): frappe.clear_cache(doctype=self.dt) - diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index c63da93a33..fc741b0f64 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -63,7 +63,7 @@ class FormMeta(Meta): "__linked_with", "__messages", "__print_formats", "__workflow_docs", "__form_grid_templates", "__listview_template", "__tree_js", "__dashboard", "__kanban_column_fields", '__templates', - '__custom_js'): + '__custom_js', '__custom_list_js'): d[k] = self.get(k) # d['fields'] = d.get('fields', []) @@ -130,9 +130,25 @@ class FormMeta(Meta): def add_custom_script(self): """embed all require files""" # custom script - custom = frappe.db.get_value("Client Script", {"dt": self.name, "enabled": 1}, "script") or "" + client_scripts = frappe.db.get_all("Client Script", + filters={"dt": self.name, "enabled": 1}, + fields=["script", "apply_to_view"], + order_by="creation asc" + ) or "" - self.set("__custom_js", custom) + print(client_scripts) + + list_script = '' + form_script = '' + for script in client_scripts: + if script.apply_to_view == 'List': + list_script += script.script + + if script.apply_to_view == 'Form': + form_script += script.script + + self.set("__custom_js", form_script) + self.set("__custom_list_js", list_script) def add_search_fields(self): """add search fields found in the doctypes indicated by link fields' options""" diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index 9ec7b0e931..22a5180a2b 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -181,6 +181,9 @@ $.extend(frappe.model, { if(meta.__list_js) { eval(meta.__list_js); } + if(meta.__custom_list_js) { + eval(meta.__custom_list_js); + } if(meta.__calendar_js) { eval(meta.__calendar_js); } From cf160fa1250c515cd0f7ab0217a51e0475054c94 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 16 Mar 2021 17:42:40 +0530 Subject: [PATCH 14/23] fix: remove print statement --- frappe/desk/form/meta.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index fc741b0f64..cd0e1b6fa1 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -136,8 +136,6 @@ class FormMeta(Meta): order_by="creation asc" ) or "" - print(client_scripts) - list_script = '' form_script = '' for script in client_scripts: From 887c92c5d949409317dfcbd176f876576183ff2c Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 16 Mar 2021 17:48:12 +0530 Subject: [PATCH 15/23] fix: rename field to simply "view" --- frappe/custom/doctype/client_script/client_script.json | 6 +++--- frappe/custom/doctype/client_script/client_script.py | 6 +++--- frappe/desk/form/meta.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frappe/custom/doctype/client_script/client_script.json b/frappe/custom/doctype/client_script/client_script.json index eca4fdb4b7..782f2a1185 100644 --- a/frappe/custom/doctype/client_script/client_script.json +++ b/frappe/custom/doctype/client_script/client_script.json @@ -8,7 +8,7 @@ "engine": "InnoDB", "field_order": [ "dt", - "apply_to_view", + "view", "enabled", "script", "sample" @@ -47,7 +47,7 @@ }, { "default": "Form", - "fieldname": "apply_to_view", + "fieldname": "view", "fieldtype": "Select", "label": "Apply To", "options": "List\nForm" @@ -57,7 +57,7 @@ "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-03-16 17:09:25.918859", + "modified": "2021-03-16 17:47:02.758919", "modified_by": "Administrator", "module": "Custom", "name": "Client Script", diff --git a/frappe/custom/doctype/client_script/client_script.py b/frappe/custom/doctype/client_script/client_script.py index a84b9ce6e4..049f979263 100644 --- a/frappe/custom/doctype/client_script/client_script.py +++ b/frappe/custom/doctype/client_script/client_script.py @@ -9,18 +9,18 @@ from frappe.model.document import Document class ClientScript(Document): def autoname(self): - self.name = f"{self.dt}-{self.apply_to_view}" + self.name = f"{self.dt}-{self.view}" def validate(self): if not self.is_new(): return exists = frappe.db.exists( - "Client Script", {"dt": self.dt, "apply_to_view": self.apply_to_view} + "Client Script", {"dt": self.dt, "view": self.view} ) if exists: frappe.throw( - _("Client Script for {0} {1} already exists").format(frappe.bold(self.dt), self.apply_to_view), + _("Client Script for {0} {1} already exists").format(frappe.bold(self.dt), self.view), frappe.DuplicateEntryError, ) diff --git a/frappe/desk/form/meta.py b/frappe/desk/form/meta.py index cd0e1b6fa1..e637f4969a 100644 --- a/frappe/desk/form/meta.py +++ b/frappe/desk/form/meta.py @@ -132,17 +132,17 @@ class FormMeta(Meta): # custom script client_scripts = frappe.db.get_all("Client Script", filters={"dt": self.name, "enabled": 1}, - fields=["script", "apply_to_view"], + fields=["script", "view"], order_by="creation asc" ) or "" list_script = '' form_script = '' for script in client_scripts: - if script.apply_to_view == 'List': + if script.view == 'List': list_script += script.script - if script.apply_to_view == 'Form': + if script.view == 'Form': form_script += script.script self.set("__custom_js", form_script) From 9bdf21ad90b17de528fc471e31b89d316dd2c8f8 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 16 Mar 2021 18:25:35 +0530 Subject: [PATCH 16/23] fix: List view formatters should override standard formatters --- frappe/public/js/frappe/list/list_view.js | 48 +++++++++++++---------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 396cd983fb..6e6635caf6 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -707,25 +707,18 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { const field_html = () => { let html; let _value; - // listview_setting formatter - if ( - this.settings.formatters && - this.settings.formatters[fieldname] - ) { - _value = this.settings.formatters[fieldname](value, df, doc); + let strip_html_required = + df.fieldtype == "Text Editor" || + (df.fetch_from && + ["Text", "Small Text"].includes(df.fieldtype)); + + if (strip_html_required) { + _value = strip_html(value); } else { - let strip_html_required = - df.fieldtype == "Text Editor" || - (df.fetch_from && - ["Text", "Small Text"].includes(df.fieldtype)); - if (strip_html_required) { - _value = strip_html(value); - } else { - _value = - typeof value === "string" - ? frappe.utils.escape_html(value) - : value; - } + _value = + typeof value === "string" + ? frappe.utils.escape_html(value) + : value; } if (df.fieldtype === "Image") { @@ -781,7 +774,15 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { Subject: this.get_subject_html(doc), Field: field_html(), }; - const column_html = html_map[col.type]; + let column_html = html_map[col.type]; + + // listview_setting formatter + if ( + this.settings.formatters && + this.settings.formatters[fieldname] + ) { + column_html = this.settings.formatters[fieldname](value, df, doc); + } return `
@@ -912,7 +913,14 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { get_subject_html(doc) { let subject_field = this.columns[0].df; - let value = doc[subject_field.fieldname] || doc.name; + let value = doc[subject_field.fieldname]; + if (this.settings.formatters && this.settings.formatters[subject_field.fieldname]) { + let formatter = this.settings.formatters[subject_field.fieldname]; + value = formatter(value, subject_field, doc); + } + if (!value) { + value = doc.name; + } let subject = strip_html(value.toString()); let escaped_subject = frappe.utils.escape_html(subject); From 989ac17c79931b1760d744a1401b1cc36a48eaef Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 16 Mar 2021 18:36:39 +0530 Subject: [PATCH 17/23] fix: Don't add child table button for List script --- .../doctype/client_script/client_script.js | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/frappe/custom/doctype/client_script/client_script.js b/frappe/custom/doctype/client_script/client_script.js index 21e7334b82..3ef1932ff3 100644 --- a/frappe/custom/doctype/client_script/client_script.js +++ b/frappe/custom/doctype/client_script/client_script.js @@ -8,40 +8,42 @@ frappe.ui.form.on('Client Script', { () => frappe.set_route('List', frm.doc.dt, 'List')); } - frm.add_custom_button(__('Add script for Child Table'), () => { - frappe.model.with_doctype(frm.doc.dt, () => { - const child_tables = frappe.meta.get_docfields(frm.doc.dt, null, { - fieldtype: 'Table' - }).map(df => df.options); + if (frm.doc.view == 'Form') { + frm.add_custom_button(__('Add script for Child Table'), () => { + frappe.model.with_doctype(frm.doc.dt, () => { + const child_tables = frappe.meta.get_docfields(frm.doc.dt, null, { + fieldtype: 'Table' + }).map(df => df.options); - const d = new frappe.ui.Dialog({ - title: __('Select Child Table'), - fields: [ - { - label: __('Select Child Table'), - fieldtype: 'Link', - fieldname: 'cdt', - options: 'DocType', - get_query: () => { - return { - filters: { - istable: 1, - name: ['in', child_tables] - } - }; + const d = new frappe.ui.Dialog({ + title: __('Select Child Table'), + fields: [ + { + label: __('Select Child Table'), + fieldtype: 'Link', + fieldname: 'cdt', + options: 'DocType', + get_query: () => { + return { + filters: { + istable: 1, + name: ['in', child_tables] + } + }; + } } + ], + primary_action: ({ cdt }) => { + cdt = d.get_field('cdt').value; + frm.events.add_script_for_doctype(frm, cdt); + d.hide(); } - ], - primary_action: ({ cdt }) => { - cdt = d.get_field('cdt').value; - frm.events.add_script_for_doctype(frm, cdt); - d.hide(); - } - }); + }); - d.show(); + d.show(); + }); }); - }); + } frm.set_query('dt', { filters: { From c505f0a04c9e8de7f6d51db06ccb658d1b04e744 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 16 Mar 2021 20:34:39 +0530 Subject: [PATCH 18/23] fix: DocType and Apply To should be set only once --- frappe/custom/doctype/client_script/client_script.json | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/custom/doctype/client_script/client_script.json b/frappe/custom/doctype/client_script/client_script.json index 782f2a1185..db02d8d4bc 100644 --- a/frappe/custom/doctype/client_script/client_script.json +++ b/frappe/custom/doctype/client_script/client_script.json @@ -23,7 +23,8 @@ "oldfieldname": "dt", "oldfieldtype": "Link", "options": "DocType", - "reqd": 1 + "reqd": 1, + "set_only_once": 1 }, { "fieldname": "script", @@ -50,14 +51,15 @@ "fieldname": "view", "fieldtype": "Select", "label": "Apply To", - "options": "List\nForm" + "options": "List\nForm", + "set_only_once": 1 } ], "icon": "fa fa-glass", "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-03-16 17:47:02.758919", + "modified": "2021-03-16 20:33:51.400191", "modified_by": "Administrator", "module": "Custom", "name": "Client Script", From d25909c799635949780af56d856d0e00501bd3e5 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Mar 2021 11:53:17 +0530 Subject: [PATCH 19/23] fix: ZeroDivision error in progress When list of docs is of same length as list of doctypes to ignore, the formula would throw ZeroDivision error. It's meaningless to subtract these two quantities hence removed it. Showing accurate progress bar would require pre-computing number of docs to be cancelled, which is unnecessary. Related issue: ISS-20-21-10442 --- frappe/desk/form/linked_with.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 767b414696..a62e2837d5 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -91,9 +91,9 @@ def cancel_all_linked_docs(docs, ignore_doctypes_on_cancel_all=[]): ignore_doctypes_on_cancel_all = json.loads(ignore_doctypes_on_cancel_all) for i, doc in enumerate(docs, 1): if validate_linked_doc(doc, ignore_doctypes_on_cancel_all): - frappe.publish_progress(percent=i * 100 / ((len(docs) - len(ignore_doctypes_on_cancel_all))), title=_("Cancelling documents")) linked_doc = frappe.get_doc(doc.get("doctype"), doc.get("name")) linked_doc.cancel() + frappe.publish_progress(percent=i/len(docs) * 100, title=_("Cancelling documents")) def validate_linked_doc(docinfo, ignore_doctypes_on_cancel_all=[]): From 9eae89e1c95285b7eba2756b01954bc85c8548cd Mon Sep 17 00:00:00 2001 From: leela Date: Mon, 15 Mar 2021 13:21:32 +0530 Subject: [PATCH 20/23] fix: Remove the starting new line from send email popup body New line is needed at start of the email body only when sending a reply to exisintg emails but not in any other cases. --- frappe/public/js/frappe/views/communication.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index a5f078fc7d..ec944d5bcf 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -775,8 +775,8 @@ frappe.views.CommunicationComposer = Class.extend({ let communication_date = last_email.communication_date || last_email.creation; content = ` -

${reply} +

${frappe.separator_element || ''}

${__("On {0}, {1} wrote:", [frappe.datetime.global_date_format(communication_date) , last_email.sender])}

@@ -784,7 +784,7 @@ frappe.views.CommunicationComposer = Class.extend({
`; } else { - content = "

" + reply; + content = reply; } fields.content.set_value(content); }, From 2e6531bbe733285409ddcf7f3efa470304a3868f Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 17 Mar 2021 11:30:42 +0530 Subject: [PATCH 21/23] fix: Hide Apply To for Single doctypes --- frappe/custom/doctype/client_script/client_script.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/custom/doctype/client_script/client_script.js b/frappe/custom/doctype/client_script/client_script.js index 3ef1932ff3..c9de85e449 100644 --- a/frappe/custom/doctype/client_script/client_script.js +++ b/frappe/custom/doctype/client_script/client_script.js @@ -53,6 +53,8 @@ frappe.ui.form.on('Client Script', { }, dt(frm) { + frm.toggle_display('view', !frappe.boot.single_types.includes(frm.doc.dt)); + if (!frm.doc.script) { frm.events.add_script_for_doctype(frm, frm.doc.dt); } From 38a7383e0ebbd4f452ca0479a7f6df2fdd8cb3c7 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Wed, 17 Mar 2021 12:06:54 +0530 Subject: [PATCH 22/23] fix: Reset form boilerplate if view is List --- frappe/custom/doctype/client_script/client_script.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frappe/custom/doctype/client_script/client_script.js b/frappe/custom/doctype/client_script/client_script.js index c9de85e449..27d11af4d1 100644 --- a/frappe/custom/doctype/client_script/client_script.js +++ b/frappe/custom/doctype/client_script/client_script.js @@ -65,7 +65,18 @@ frappe.ui.form.on('Client Script', { } }, + view(frm) { + let has_form_boilerplate = frm.doc.script.includes('frappe.ui.form.on') + if (frm.doc.view === 'List' && has_form_boilerplate) { + frm.set_value('script', ''); + } + if (frm.doc.view === 'Form' && !has_form_boilerplate) { + frm.trigger('dt'); + } + }, + add_script_for_doctype(frm, doctype) { + if (!doctype) return; let boilerplate = ` frappe.ui.form.on('${doctype}', { refresh(frm) { From 82b4e6cf21f99b979eba0f844745bea9f1fd69af Mon Sep 17 00:00:00 2001 From: Leela vadlamudi Date: Wed, 17 Mar 2021 12:27:06 +0530 Subject: [PATCH 23/23] fix: custom module's query report (#12596) --- frappe/desk/query_report.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 3008cf0e61..22d47d1120 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -164,10 +164,14 @@ def get_script(report_name): module = report.module or frappe.db.get_value( "DocType", report.ref_doctype, "module" ) - module_path = get_module_path(module) - report_folder = os.path.join(module_path, "report", scrub(report.name)) - script_path = os.path.join(report_folder, scrub(report.name) + ".js") - print_path = os.path.join(report_folder, scrub(report.name) + ".html") + + is_custom_module = frappe.get_cached_value("Module Def", module, "custom") + + # custom modules are virtual modules those exists in DB but not in disk. + module_path = '' if is_custom_module else get_module_path(module) + report_folder = module_path and os.path.join(module_path, "report", scrub(report.name)) + script_path = report_folder and os.path.join(report_folder, scrub(report.name) + ".js") + print_path = report_folder and os.path.join(report_folder, scrub(report.name) + ".html") script = None if os.path.exists(script_path):