From 703f58ceac1dc5fe15ef3c4f9023400420a27f39 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 31 Jul 2024 14:01:27 +0530 Subject: [PATCH 1/5] refactor: date filters should be explicit (cherry picked from commit 40c166a0a05bf8ff39ba55de0aa36b6a37afb890) --- .../sales_pipeline_analytics/sales_pipeline_analytics.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py index 9cc69d24a2b..0c540f75df0 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -147,6 +147,10 @@ class SalesPipelineAnalytics: conditions.append( ["expected_closing", "between", [self.filters.get("from_date"), self.filters.get("to_date")]] ) + elif self.filters.get("from_date") and not self.filters.get("to_date"): + conditions.append(["expected_closing", ">=", self.filters.get("from_date")]) + elif not self.filters.get("from_date") and self.filters.get("to_date"): + conditions.append(["expected_closing", "<=", self.filters.get("to_date")]) return conditions From f7f191fe50d5ed003fb07846743cbcc98633834b Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 10:10:25 +0530 Subject: [PATCH 2/5] refactor: make 'from_date' and 'to_date' mandatory (cherry picked from commit 3617b41b9559e264ccdefd31faad99a8bc70f44b) --- .../sales_pipeline_analytics.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py index 0c540f75df0..4baa09195d5 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -21,7 +21,15 @@ class SalesPipelineAnalytics: def __init__(self, filters=None): self.filters = frappe._dict(filters or {}) + def validate_filters(self): + if not self.filters.from_date: + frappe.throw(_("From Date is mandatory")) + + if not self.filters.to_date: + frappe.throw(_("To Date is mandatory")) + def run(self): + self.validate_filters() self.get_columns() self.get_data() self.get_chart_data() @@ -147,10 +155,6 @@ class SalesPipelineAnalytics: conditions.append( ["expected_closing", "between", [self.filters.get("from_date"), self.filters.get("to_date")]] ) - elif self.filters.get("from_date") and not self.filters.get("to_date"): - conditions.append(["expected_closing", ">=", self.filters.get("from_date")]) - elif not self.filters.get("from_date") and self.filters.get("to_date"): - conditions.append(["expected_closing", "<=", self.filters.get("to_date")]) return conditions From baa36c6d5e33fa53580bbbf29440cb311a4c9d51 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 11:31:13 +0530 Subject: [PATCH 3/5] refactor: report columns should be based on from and to dates (cherry picked from commit 751a25c4b74d8c9bd739a982c163caf32f124921) --- .../sales_pipeline_analytics/sales_pipeline_analytics.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py index 4baa09195d5..8460457c5ee 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -8,7 +8,7 @@ from itertools import groupby import frappe from dateutil.relativedelta import relativedelta from frappe import _ -from frappe.utils import cint, flt +from frappe.utils import cint, flt, getdate from erpnext.setup.utils import get_exchange_rate @@ -235,10 +235,9 @@ class SalesPipelineAnalytics: def get_month_list(self): month_list = [] - current_date = date.today() - month_number = date.today().month + current_date = getdate(self.filters.get("from_date")) - for _month in range(month_number, 13): + while current_date < getdate(self.filters.get("to_date")): month_list.append(current_date.strftime("%B")) current_date = current_date + relativedelta(months=1) From 989ef52f59d85edaaa9edf5d51a74b9e0659ee26 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 14:08:08 +0530 Subject: [PATCH 4/5] refactor: consider empty-string as Not Assigned (cherry picked from commit 213b2ba94254655a07ed3df35b1158c35bf02469) --- .../report/sales_pipeline_analytics/sales_pipeline_analytics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py index 8460457c5ee..e5d34231a87 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -193,7 +193,7 @@ class SalesPipelineAnalytics: count_or_amount = info.get(based_on) if self.filters.get("pipeline_by") == "Owner": - if value == "Not Assigned" or value == "[]" or value is None: + if value == "Not Assigned" or value == "[]" or value is None or not value: assigned_to = ["Not Assigned"] else: assigned_to = json.loads(value) From ffd3aea07e69c0479fa4b254036612d1d7e28bf6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 14:47:09 +0530 Subject: [PATCH 5/5] refactor(test): use test fixture and supply from and to dates (cherry picked from commit 4253caf910bdc47a3f7ee0b13cbf711f4a8560b1) --- .../test_sales_pipeline_analytics.py | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/erpnext/crm/report/sales_pipeline_analytics/test_sales_pipeline_analytics.py b/erpnext/crm/report/sales_pipeline_analytics/test_sales_pipeline_analytics.py index 02d82b637e8..bf3f946d6ab 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/test_sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/test_sales_pipeline_analytics.py @@ -1,19 +1,21 @@ import unittest import frappe +from frappe.tests.utils import FrappeTestCase from erpnext.crm.report.sales_pipeline_analytics.sales_pipeline_analytics import execute -class TestSalesPipelineAnalytics(unittest.TestCase): - @classmethod - def setUpClass(self): +class TestSalesPipelineAnalytics(FrappeTestCase): + def setUp(self): frappe.db.delete("Opportunity") create_company() create_customer() create_opportunity() def test_sales_pipeline_analytics(self): + self.from_date = "2021-01-01" + self.to_date = "2021-12-31" self.check_for_monthly_and_number() self.check_for_monthly_and_amount() self.check_for_quarterly_and_number() @@ -28,6 +30,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -43,6 +47,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -59,6 +65,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -74,6 +82,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -90,6 +100,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -105,6 +117,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -121,6 +135,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -136,6 +152,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "status": "Open", "opportunity_type": "Sales", "company": "Best Test", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters) @@ -153,8 +171,8 @@ class TestSalesPipelineAnalytics(unittest.TestCase): "opportunity_type": "Sales", "company": "Best Test", "opportunity_source": "Cold Calling", - "from_date": "2021-08-01", - "to_date": "2021-08-31", + "from_date": self.from_date, + "to_date": self.to_date, } report = execute(filters)