From 38a3ddacf5c00637502eb7b357ac66504fd34d1a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 31 Jul 2024 14:01:27 +0530 Subject: [PATCH 1/6] refactor: date filters should be explicit --- .../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 a5e0d73270..d6051bbc5e 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -149,6 +149,10 @@ class SalesPipelineAnalytics(object): 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 -- GitLab From 238c97d7fac4569ff0cbfea3d7e6e4ff0cbf14cc Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 10:10:25 +0530 Subject: [PATCH 2/6] refactor: make 'from_date' and 'to_date' mandatory --- .../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 d6051bbc5e..fbf4296436 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(object): 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() @@ -149,10 +157,6 @@ class SalesPipelineAnalytics(object): 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 -- GitLab From 67ae5c1f3b6e7ff592e757def43d94e78c6249c8 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 11:31:13 +0530 Subject: [PATCH 3/6] refactor: report columns should be based on from and to dates --- .../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 fbf4296436..205664afdf 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 @@ -237,10 +237,9 @@ class SalesPipelineAnalytics(object): 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) -- GitLab From 5118ef936d18d76cd33817c3a44f24a078101eac Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 14:08:08 +0530 Subject: [PATCH 4/6] refactor: consider empty-string as Not Assigned --- .../sales_pipeline_analytics/sales_pipeline_analytics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 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 205664afdf..97f5fa6735 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -195,8 +195,8 @@ class SalesPipelineAnalytics(object): count_or_amount = info.get(based_on) if self.filters.get("pipeline_by") == "Owner": - if value == _("Not Assigned") or value == "[]" or value is None: - assigned_to = [_("Not Assigned")] + if value == "Not Assigned" or value == "[]" or value is None or not value: + assigned_to = ["Not Assigned"] else: assigned_to = json.loads(value) self.check_for_assigned_to(period, value, count_or_amount, assigned_to, info) -- GitLab From 91deb222387608b48b85201360de5a6296818d2c Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 14:47:09 +0530 Subject: [PATCH 5/6] refactor(test): use test fixture and supply from and to dates --- .../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 837fbcb5cb..18309a64eb 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) -- GitLab From 72bd01d74eb4b7e28b2e7a796a879df6533507a7 Mon Sep 17 00:00:00 2001 From: Charles-Henri Decultot Date: Tue, 6 Aug 2024 08:05:23 +0000 Subject: [PATCH 6/6] fix: missing translations --- .../sales_pipeline_analytics/sales_pipeline_analytics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 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 97f5fa6735..832ba81f9f 100644 --- a/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py +++ b/erpnext/crm/report/sales_pipeline_analytics/sales_pipeline_analytics.py @@ -195,8 +195,8 @@ class SalesPipelineAnalytics(object): count_or_amount = info.get(based_on) if self.filters.get("pipeline_by") == "Owner": - if value == "Not Assigned" or value == "[]" or value is None or not value: - assigned_to = ["Not Assigned"] + if value == _("Not Assigned") or value == "[]" or value is None or not value: + assigned_to = [_("Not Assigned")] else: assigned_to = json.loads(value) self.check_for_assigned_to(period, value, count_or_amount, assigned_to, info) -- GitLab