From e9bcb1bbb2a16a713627f06539f1c29e70421fb9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 2 Aug 2024 15:43:04 +0530 Subject: [PATCH 01/12] refactor: ignore filter in general ledger for cr / dr notes (cherry picked from commit 59d5beee20f4a3837b1bcd74e6826d4b7290c0fb) --- erpnext/accounts/report/general_ledger/general_ledger.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/erpnext/accounts/report/general_ledger/general_ledger.js b/erpnext/accounts/report/general_ledger/general_ledger.js index 1894e958552..e202d5dfffd 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.js +++ b/erpnext/accounts/report/general_ledger/general_ledger.js @@ -209,6 +209,11 @@ frappe.query_reports["General Ledger"] = { label: __("Ignore Exchange Rate Revaluation Journals"), fieldtype: "Check", }, + { + fieldname: "ignore_cr_dr_notes", + label: __("Ignore System Generated Credit / Debit Notes"), + fieldtype: "Check", + }, ], }; From 17a6224a3f13a434ee954ba7f6b3b53cf7e4a892 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 2 Aug 2024 15:47:27 +0530 Subject: [PATCH 02/12] refactor: ignore system generated cr / dr notes on general ledger (cherry picked from commit bb8c9b5a5845d2db3cd96f0264132c0e9b4a1e6e) --- .../report/general_ledger/general_ledger.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/erpnext/accounts/report/general_ledger/general_ledger.py b/erpnext/accounts/report/general_ledger/general_ledger.py index 4b20746286a..63e57438fca 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.py +++ b/erpnext/accounts/report/general_ledger/general_ledger.py @@ -236,6 +236,20 @@ def get_conditions(filters): if err_journals: filters.update({"voucher_no_not_in": [x[0] for x in err_journals]}) + if filters.get("ignore_cr_dr_notes"): + system_generated_cr_dr_journals = frappe.db.get_all( + "Journal Entry", + filters={ + "company": filters.get("company"), + "docstatus": 1, + "voucher_type": ("in", ["Credit Note", "Debit Note"]), + "is_system_generated": 1, + }, + as_list=True, + ) + if system_generated_cr_dr_journals: + filters.update({"voucher_no_not_in": [x[0] for x in system_generated_cr_dr_journals]}) + if filters.get("voucher_no_not_in"): conditions.append("voucher_no not in %(voucher_no_not_in)s") From 7ec7e11634fea52be14e68b279f9fef38b078c0e Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 09:56:16 +0530 Subject: [PATCH 03/12] test: ignore filter for system generated cr / dr note journals (cherry picked from commit 3ffac735985a1b0d55b3265d5299ff6d0cf6e93a) --- .../general_ledger/test_general_ledger.py | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/erpnext/accounts/report/general_ledger/test_general_ledger.py b/erpnext/accounts/report/general_ledger/test_general_ledger.py index 33b356feb33..682eef8f419 100644 --- a/erpnext/accounts/report/general_ledger/test_general_ledger.py +++ b/erpnext/accounts/report/general_ledger/test_general_ledger.py @@ -5,7 +5,9 @@ import frappe from frappe.tests.utils import FrappeTestCase from frappe.utils import flt, today +from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice from erpnext.accounts.report.general_ledger.general_ledger import execute +from erpnext.controllers.sales_and_purchase_return import make_return_doc class TestGeneralLedger(FrappeTestCase): @@ -248,3 +250,73 @@ class TestGeneralLedger(FrappeTestCase): ) ) self.assertIn(revaluation_jv.name, set([x.voucher_no for x in data])) + + def test_ignore_cr_dr_notes_filter(self): + si = create_sales_invoice() + + cr_note = make_return_doc(si.doctype, si.name) + cr_note.submit() + + pr = frappe.get_doc("Payment Reconciliation") + pr.company = si.company + pr.party_type = "Customer" + pr.party = si.customer + pr.receivable_payable_account = si.debit_to + + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + + invoices = [invoice.as_dict() for invoice in pr.invoices] + payments = [payment.as_dict() for payment in pr.payments] + pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments})) + pr.reconcile() + + self.assertEqual(len(pr.invoices), 0) + self.assertEqual(len(pr.payments), 0) + + system_generated_journal = frappe.db.get_all( + "Journal Entry", + filters={ + "docstatus": 1, + "reference_type": si.doctype, + "reference_name": si.name, + "voucher_type": "Credit Note", + "is_system_generated": True, + }, + fields=["name"], + ) + self.assertEqual(len(system_generated_journal), 1) + expected = set([si.name, cr_note.name, system_generated_journal[0].name]) + # Without ignore_cr_dr_notes + columns, data = execute( + frappe._dict( + { + "company": si.company, + "from_date": si.posting_date, + "to_date": si.posting_date, + "account": [si.debit_to], + "group_by": "Group by Voucher (Consolidated)", + "ignore_cr_dr_notes": False, + } + ) + ) + actual = set([x.voucher_no for x in data if x.voucher_no]) + self.assertEqual(expected, actual) + + # Without ignore_cr_dr_notes + expected = set([si.name, cr_note.name]) + columns, data = execute( + frappe._dict( + { + "company": si.company, + "from_date": si.posting_date, + "to_date": si.posting_date, + "account": [si.debit_to], + "group_by": "Group by Voucher (Consolidated)", + "ignore_cr_dr_notes": True, + } + ) + ) + actual = set([x.voucher_no for x in data if x.voucher_no]) + self.assertEqual(expected, actual) From 3ee59918965f301a5d3d74c084e05729eb420963 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 09:58:50 +0530 Subject: [PATCH 04/12] refactor: make use of date filters on ignore filterss (cherry picked from commit 03f3ab522f54612f557a7d2f715367daf8bc370e) --- erpnext/accounts/report/general_ledger/general_ledger.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/accounts/report/general_ledger/general_ledger.py b/erpnext/accounts/report/general_ledger/general_ledger.py index 63e57438fca..02ecc24dea1 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.py +++ b/erpnext/accounts/report/general_ledger/general_ledger.py @@ -230,6 +230,7 @@ def get_conditions(filters): "company": filters.get("company"), "docstatus": 1, "voucher_type": ("in", ["Exchange Rate Revaluation", "Exchange Gain Or Loss"]), + "posting_date": ["between", [filters.get("from_date"), filters.get("to_date")]], }, as_list=True, ) @@ -244,6 +245,7 @@ def get_conditions(filters): "docstatus": 1, "voucher_type": ("in", ["Credit Note", "Debit Note"]), "is_system_generated": 1, + "posting_date": ["between", [filters.get("from_date"), filters.get("to_date")]], }, as_list=True, ) From 155ca14b19568c62ae1ee3d9964d3f900b62c9b9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 10:23:52 +0530 Subject: [PATCH 05/12] test: clear old data (cherry picked from commit 991069bfbcfba4ef46bae028dcdc908e785f693f) --- .../accounts/report/general_ledger/test_general_ledger.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/erpnext/accounts/report/general_ledger/test_general_ledger.py b/erpnext/accounts/report/general_ledger/test_general_ledger.py index 682eef8f419..094da52e68a 100644 --- a/erpnext/accounts/report/general_ledger/test_general_ledger.py +++ b/erpnext/accounts/report/general_ledger/test_general_ledger.py @@ -2,6 +2,7 @@ # MIT License. See license.txt import frappe +from frappe import qb from frappe.tests.utils import FrappeTestCase from frappe.utils import flt, today @@ -252,6 +253,10 @@ class TestGeneralLedger(FrappeTestCase): self.assertIn(revaluation_jv.name, set([x.voucher_no for x in data])) def test_ignore_cr_dr_notes_filter(self): + # Clear old data + sinv_doctype = qb.DocType("Sales Invoice") + qb.from_(sinv_doctype).delete().where(sinv_doctype.company.eq("_Test Company")).run() + si = create_sales_invoice() cr_note = make_return_doc(si.doctype, si.name) From c1bfa0a31a1ecd2256060f5cb5bbef5e946117c9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 10:46:35 +0530 Subject: [PATCH 06/12] refactor(test): filter and reconcile concerned vouchers (cherry picked from commit 9ade269b7a66a72a7858aad47704da6038a26094) --- .../report/general_ledger/test_general_ledger.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/erpnext/accounts/report/general_ledger/test_general_ledger.py b/erpnext/accounts/report/general_ledger/test_general_ledger.py index 094da52e68a..1a935bd05d7 100644 --- a/erpnext/accounts/report/general_ledger/test_general_ledger.py +++ b/erpnext/accounts/report/general_ledger/test_general_ledger.py @@ -253,10 +253,6 @@ class TestGeneralLedger(FrappeTestCase): self.assertIn(revaluation_jv.name, set([x.voucher_no for x in data])) def test_ignore_cr_dr_notes_filter(self): - # Clear old data - sinv_doctype = qb.DocType("Sales Invoice") - qb.from_(sinv_doctype).delete().where(sinv_doctype.company.eq("_Test Company")).run() - si = create_sales_invoice() cr_note = make_return_doc(si.doctype, si.name) @@ -269,17 +265,12 @@ class TestGeneralLedger(FrappeTestCase): pr.receivable_payable_account = si.debit_to pr.get_unreconciled_entries() - self.assertEqual(len(pr.invoices), 1) - self.assertEqual(len(pr.payments), 1) - invoices = [invoice.as_dict() for invoice in pr.invoices] - payments = [payment.as_dict() for payment in pr.payments] + invoices = [invoice.as_dict() for invoice in pr.invoices if invoice.invoice_number == si.name] + payments = [payment.as_dict() for payment in pr.payments if payment.reference_name == cr_note.name] pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments})) pr.reconcile() - self.assertEqual(len(pr.invoices), 0) - self.assertEqual(len(pr.payments), 0) - system_generated_journal = frappe.db.get_all( "Journal Entry", filters={ From 531410820f5295e68ad1fe06fd3173665b226b63 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 13:25:43 +0530 Subject: [PATCH 07/12] refactor(test): clear old records on GL report tests (cherry picked from commit 56620785a0da8d36c315128c26e0403594d21275) --- .../report/general_ledger/test_general_ledger.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/erpnext/accounts/report/general_ledger/test_general_ledger.py b/erpnext/accounts/report/general_ledger/test_general_ledger.py index 1a935bd05d7..af2c569949a 100644 --- a/erpnext/accounts/report/general_ledger/test_general_ledger.py +++ b/erpnext/accounts/report/general_ledger/test_general_ledger.py @@ -12,6 +12,22 @@ from erpnext.controllers.sales_and_purchase_return import make_return_doc class TestGeneralLedger(FrappeTestCase): + def setUp(self): + self.company = "_Test Company" + self.clear_old_entries() + + def clear_old_entries(self): + doctype_list = [ + "GL Entry", + "Payment Ledger Entry", + "Sales Invoice", + "Purchase Invoice", + "Payment Entry", + "Journal Entry", + ] + for doctype in doctype_list: + qb.from_(qb.DocType(doctype)).delete().where(qb.DocType(doctype).company == self.company).run() + def test_foreign_account_balance_after_exchange_rate_revaluation(self): """ Checks the correctness of balance after exchange rate revaluation From 48e05b61102c1355dfe724e38675298230e6ed93 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 31 Jul 2024 14:01:27 +0530 Subject: [PATCH 08/12] 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 b44598cb7a61a2a2dd80848ac7525280568de035 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 10:10:25 +0530 Subject: [PATCH 09/12] 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 8978a54479773859c43b73f026284fe02099470a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 11:31:13 +0530 Subject: [PATCH 10/12] 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 52578b03b417939e49009fe86c08299152b66505 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 14:08:08 +0530 Subject: [PATCH 11/12] 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 05c606b65d6742022cef3f433a33d7dfd54655ed Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 5 Aug 2024 14:47:09 +0530 Subject: [PATCH 12/12] 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)