Merge pull request #46640 from ljain112/fix-ledger-summary

perf: refactored customer ledger summary for performance

erpnext/accounts/report/customer_ledger_summary/customer_ledger_summary.js

@@ -9,6 +9,7 @@ frappe.query_reports["Customer Ledger Summary"] = {
 			fieldtype: "Link",
 			options: "Company",
 			default: frappe.defaults.get_user_default("Company"),
+			reqd: 1,
 		},
 		{
 			fieldname: "from_date",

erpnext/accounts/report/customer_ledger_summary/customer_ledger_summary.py

@@ -4,14 +4,19 @@
 
 import frappe
 from frappe import _, qb, scrub
+from frappe.query_builder import Criterion, Tuple
 from frappe.query_builder.functions import IfNull
 from frappe.utils import getdate, nowdate
+from frappe.utils.nestedset import get_descendants_of
 
 from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import (
 	get_accounting_dimensions,
 	get_dimension_with_children,
 )
-from erpnext.accounts.report.financial_statements import get_cost_centers_with_children
+
+TREE_DOCTYPES = frozenset(
+	["Customer Group", "Terrirtory", "Supplier Group", "Sales Partner", "Sales Person", "Cost Center"]
+)
 
 
 class PartyLedgerSummaryReport:
@@ -20,59 +25,110 @@ class PartyLedgerSummaryReport:
 		self.filters.from_date = getdate(self.filters.from_date or nowdate())
 		self.filters.to_date = getdate(self.filters.to_date or nowdate())
 
-		if not self.filters.get("company"):
-			self.filters["company"] = frappe.db.get_single_value("Global Defaults", "default_company")
-
 	def run(self, args):
-		if self.filters.from_date > self.filters.to_date:
-			frappe.throw(_("From Date must be before To Date"))
-
 		self.filters.party_type = args.get("party_type")
-		self.party_naming_by = frappe.db.get_single_value(args.get("naming_by")[0], args.get("naming_by")[1])
+
+		self.validate_filters()
+		self.get_party_details()
+
+		if not self.parties:
+			return [], []
 
 		self.get_gl_entries()
-		self.get_additional_columns()
 		self.get_return_invoices()
 		self.get_party_adjustment_amounts()
 
+		self.party_naming_by = frappe.db.get_single_value(args.get("naming_by")[0], args.get("naming_by")[1])
 		columns = self.get_columns()
 		data = self.get_data()
+
 		return columns, data
 
-	def get_additional_columns(self):
+	def validate_filters(self):
+		if not self.filters.get("company"):
+			frappe.throw(_("{0} is mandatory").format(_("Company")))
+
+		if self.filters.from_date > self.filters.to_date:
+			frappe.throw(_("From Date must be before To Date"))
+
+		self.update_hierarchical_filters()
+
+	def update_hierarchical_filters(self):
+		for doctype in TREE_DOCTYPES:
+			key = scrub(doctype)
+			if self.filters.get(key):
+				self.filters[key] = get_children(doctype, self.filters[key])
+
+	def get_party_details(self):
 		"""
 		Additional Columns for 'User Permission' based access control
 		"""
+		self.parties = []
+		self.party_details = frappe._dict()
+		party_type = self.filters.party_type
 
-		if self.filters.party_type == "Customer":
-			self.territories = frappe._dict({})
-			self.customer_group = frappe._dict({})
+		doctype = qb.DocType(party_type)
+		conditions = self.get_party_conditions(doctype)
+		query = (
+			qb.from_(doctype)
+			.select(doctype.name.as_("party"), f"{scrub(party_type)}_name")
+			.where(Criterion.all(conditions))
+		)
 
-			customer = qb.DocType("Customer")
-			result = (
-				frappe.qb.from_(customer)
-				.select(
-					customer.name, customer.territory, customer.customer_group, customer.default_sales_partner
-				)
-				.where(customer.disabled == 0)
-				.run(as_dict=True)
+		from frappe.desk.reportview import build_match_conditions
+
+		query, params = query.walk()
+		match_conditions = build_match_conditions(party_type)
+
+		if match_conditions:
+			query += "and" + match_conditions
+
+		party_details = frappe.db.sql(query, params, as_dict=True)
+
+		for row in party_details:
+			self.parties.append(row.party)
+			self.party_details[row.party] = row
+
+	def get_party_conditions(self, doctype):
+		conditions = []
+		group_field = "customer_group" if self.filters.party_type == "Customer" else "supplier_group"
+
+		if self.filters.party:
+			conditions.append(doctype.name == self.filters.party)
+
+		if self.filters.territory:
+			conditions.append(doctype.territory.isin(self.filters.territory))
+
+		if self.filters.get(group_field):
+			conditions.append(doctype.get(group_field).isin(self.filters.get(group_field)))
+
+		if self.filters.payment_terms_template:
+			conditions.append(doctype.payment_terms == self.filters.payment_terms_template)
+
+		if self.filters.sales_partner:
+			conditions.append(doctype.default_sales_partner.isin(self.filters.sales_partner))
+
+		if self.filters.sales_person:
+			sales_team = qb.DocType("Sales Team")
+			sales_invoice = qb.DocType("Sales Invoice")
+
+			customers = (
+				qb.from_(sales_team)
+				.select(sales_team.parent)
+				.where(sales_team.sales_person.isin(self.filters.sales_person))
+				.where(sales_team.parenttype == "Customer")
+			) + (
+				qb.from_(sales_team)
+				.join(sales_invoice)
+				.on(sales_team.parent == sales_invoice.name)
+				.select(sales_invoice.customer)
+				.where(sales_team.sales_person.isin(self.filters.sales_person))
+				.where(sales_team.parenttype == "Sales Invoice")
 			)
 
-			for x in result:
-				self.territories[x.name] = x.territory
-				self.customer_group[x.name] = x.customer_group
-		else:
-			self.supplier_group = frappe._dict({})
-			supplier = qb.DocType("Supplier")
-			result = (
-				frappe.qb.from_(supplier)
-				.select(supplier.name, supplier.supplier_group)
-				.where(supplier.disabled == 0)
-				.run(as_dict=True)
-			)
+			conditions.append(doctype.name.isin(customers))
 
-			for x in result:
-				self.supplier_group[x.name] = x.supplier_group
+		return conditions
 
 	def get_columns(self):
 		columns = [
@@ -195,12 +251,13 @@ class PartyLedgerSummaryReport:
 
 		self.party_data = frappe._dict({})
 		for gle in self.gl_entries:
+			party_details = self.party_details.get(gle.party)
 			self.party_data.setdefault(
 				gle.party,
 				frappe._dict(
 					{
-						"party": gle.party,
-						"party_name": gle.party_name,
+						**party_details,
+						"party_name": gle.party,
 						"opening_balance": 0,
 						"invoiced_amount": 0,
 						"paid_amount": 0,
@@ -211,12 +268,6 @@ class PartyLedgerSummaryReport:
 				),
 			)
 
-			if self.filters.party_type == "Customer":
-				self.party_data[gle.party].update({"territory": self.territories.get(gle.party)})
-				self.party_data[gle.party].update({"customer_group": self.customer_group.get(gle.party)})
-			else:
-				self.party_data[gle.party].update({"supplier_group": self.supplier_group.get(gle.party)})
-
 			amount = gle.get(invoice_dr_or_cr) - gle.get(reverse_dr_or_cr)
 			self.party_data[gle.party].closing_balance += amount
 
@@ -261,8 +312,6 @@ class PartyLedgerSummaryReport:
 				gle.party,
 				gle.voucher_type,
 				gle.voucher_no,
-				gle.against_voucher_type,
-				gle.against_voucher,
 				gle.debit,
 				gle.credit,
 				gle.is_opening,
@@ -273,26 +322,12 @@ class PartyLedgerSummaryReport:
 				& (gle.party_type == self.filters.party_type)
 				& (IfNull(gle.party, "") != "")
 				& (gle.posting_date <= self.filters.to_date)
+				& (gle.party.isin(self.parties))
 			)
-			.orderby(gle.posting_date)
 		)
 
-		if self.filters.party_type == "Customer":
-			customer = qb.DocType("Customer")
-			query = (
-				query.select(customer.customer_name.as_("party_name"))
-				.left_join(customer)
-				.on(customer.name == gle.party)
-			)
-		elif self.filters.party_type == "Supplier":
-			supplier = qb.DocType("Supplier")
-			query = (
-				query.select(supplier.supplier_name.as_("party_name"))
-				.left_join(supplier)
-				.on(supplier.name == gle.party)
-			)
-
 		query = self.prepare_conditions(query)
+
 		self.gl_entries = query.run(as_dict=True)
 
 	def prepare_conditions(self, query):
@@ -303,70 +338,7 @@ class PartyLedgerSummaryReport:
 		if self.filters.finance_book:
 			query = query.where(IfNull(gle.finance_book, "") == self.filters.finance_book)
 
-		if self.filters.party:
-			query = query.where(gle.party == self.filters.party)
-
-		if self.filters.party_type == "Customer":
-			customer = qb.DocType("Customer")
-			if self.filters.customer_group:
-				query = query.where(
-					(gle.party).isin(
-						qb.from_(customer)
-						.select(customer.name)
-						.where(customer.customer_group == self.filters.customer_group)
-					)
-				)
-
-			if self.filters.territory:
-				query = query.where(
-					(gle.party).isin(
-						qb.from_(customer)
-						.select(customer.name)
-						.where(customer.territory == self.filters.territory)
-					)
-				)
-
-			if self.filters.payment_terms_template:
-				query = query.where(
-					(gle.party).isin(
-						qb.from_(customer)
-						.select(customer.name)
-						.where(customer.payment_terms == self.filters.payment_terms_template)
-					)
-				)
-
-			if self.filters.sales_partner:
-				query = query.where(
-					(gle.party).isin(
-						qb.from_(customer)
-						.select(customer.name)
-						.where(customer.default_sales_partner == self.filters.sales_partner)
-					)
-				)
-
-			if self.filters.sales_person:
-				sales_team = qb.DocType("Sales Team")
-				query = query.where(
-					(gle.party).isin(
-						qb.from_(sales_team)
-						.select(sales_team.parent)
-						.where(sales_team.sales_person == self.filters.sales_person)
-					)
-				)
-
-		if self.filters.party_type == "Supplier":
-			if self.filters.supplier_group:
-				supplier = qb.DocType("Supplier")
-				query = query.where(
-					(gle.party).isin(
-						qb.from_(supplier)
-						.select(supplier.name)
-						.where(supplier.supplier_group == self.filters.supplier_group)
-					)
-				)
-
 		if self.filters.cost_center:
-			self.filters.cost_center = get_cost_centers_with_children(self.filters.cost_center)
 			query = query.where((gle.cost_center).isin(self.filters.cost_center))
 
 		if self.filters.project:
@@ -393,45 +365,44 @@ class PartyLedgerSummaryReport:
 
 	def get_return_invoices(self):
 		doctype = "Sales Invoice" if self.filters.party_type == "Customer" else "Purchase Invoice"
-		self.return_invoices = [
-			d.name
-			for d in frappe.get_all(
-				doctype,
-				filters={
-					"is_return": 1,
-					"docstatus": 1,
-					"posting_date": ["between", [self.filters.from_date, self.filters.to_date]],
-				},
-			)
-		]
+		filters = (
+			{
+				"is_return": 1,
+				"docstatus": 1,
+				"posting_date": ["between", [self.filters.from_date, self.filters.to_date]],
+				f"{scrub(self.filters.party_type)}": ["in", self.parties],
+			},
+		)
+
+		self.return_invoices = frappe.get_all(doctype, filters=filters, pluck="name")
 
 	def get_party_adjustment_amounts(self):
 		account_type = "Expense Account" if self.filters.party_type == "Customer" else "Income Account"
-		self.income_or_expense_accounts = frappe.db.get_all(
-			"Account", filters={"account_type": account_type, "company": self.filters.company}, pluck="name"
-		)
+
 		invoice_dr_or_cr = "debit" if self.filters.party_type == "Customer" else "credit"
 		reverse_dr_or_cr = "credit" if self.filters.party_type == "Customer" else "debit"
 		round_off_account = frappe.get_cached_value("Company", self.filters.company, "round_off_account")
 
-		if not self.income_or_expense_accounts:
-			# prevent empty 'in' condition
-			self.income_or_expense_accounts.append("")
-		else:
-			# escape '%' in account name
-			# ignoring frappe.db.escape as it replaces single quotes with double quotes
-			self.income_or_expense_accounts = [x.replace("%", "%%") for x in self.income_or_expense_accounts]
+		current_period_vouchers = set()
+		adjustment_voucher_entries = {}
+
+		self.party_adjustment_details = {}
+		self.party_adjustment_accounts = set()
+
+		for gle in self.gl_entries:
+			if (
+				gle.is_opening != "Yes"
+				and gle.posting_date >= self.filters.from_date
+				and gle.posting_date <= self.filters.to_date
+			):
+				current_period_vouchers.add((gle.voucher_type, gle.voucher_no))
+				adjustment_voucher_entries.setdefault((gle.voucher_type, gle.voucher_no), []).append(gle)
+
+		if not current_period_vouchers:
+			return
 
 		gl = qb.DocType("GL Entry")
-		accounts_query = self.get_base_accounts_query()
-		accounts_query_voucher_no = accounts_query.select(gl.voucher_no)
-		accounts_query_voucher_type = accounts_query.select(gl.voucher_type)
-
-		subquery = self.get_base_subquery()
-		subquery_voucher_no = subquery.select(gl.voucher_no)
-		subquery_voucher_type = subquery.select(gl.voucher_type)
-
-		gl_entries = (
+		query = (
 			qb.from_(gl)
 			.select(
 				gl.posting_date, gl.account, gl.party, gl.voucher_type, gl.voucher_no, gl.debit, gl.credit
@@ -439,18 +410,16 @@ class PartyLedgerSummaryReport:
 			.where(
 				(gl.docstatus < 2)
 				& (gl.is_cancelled == 0)
-				& (gl.voucher_no.isin(accounts_query_voucher_no))
-				& (gl.voucher_type.isin(accounts_query_voucher_type))
-				& (gl.voucher_no.isin(subquery_voucher_no))
-				& (gl.voucher_type.isin(subquery_voucher_type))
+				& (gl.posting_date.gte(self.filters.from_date))
+				& (gl.posting_date.lte(self.filters.to_date))
+				& (Tuple((gl.voucher_type, gl.voucher_no)).isin(current_period_vouchers))
+				& (IfNull(gl.party, "") == "")
 			)
-		).run(as_dict=True)
+		)
+		query = self.prepare_conditions(query)
+		gl_entries = query.run(as_dict=True)
 
-		self.party_adjustment_details = {}
-		self.party_adjustment_accounts = set()
-		adjustment_voucher_entries = {}
 		for gle in gl_entries:
-			adjustment_voucher_entries.setdefault((gle.voucher_type, gle.voucher_no), [])
 			adjustment_voucher_entries[(gle.voucher_type, gle.voucher_no)].append(gle)
 
 		for voucher_gl_entries in adjustment_voucher_entries.values():
@@ -486,25 +455,11 @@ class PartyLedgerSummaryReport:
 						self.party_adjustment_details[party].setdefault(account, 0)
 						self.party_adjustment_details[party][account] += amount
 
-	def get_base_accounts_query(self):
-		gl = qb.DocType("GL Entry")
-		query = qb.from_(gl).where(
-			(gl.account.isin(self.income_or_expense_accounts))
-			& (gl.posting_date.gte(self.filters.from_date))
-			& (gl.posting_date.lte(self.filters.to_date))
-		)
-		return query
 
-	def get_base_subquery(self):
-		gl = qb.DocType("GL Entry")
-		query = qb.from_(gl).where(
-			(gl.docstatus < 2)
-			& (gl.party_type == self.filters.party_type)
-			& (IfNull(gl.party, "") != "")
-			& (gl.posting_date.between(self.filters.from_date, self.filters.to_date))
-		)
-		query = self.prepare_conditions(query)
-		return query
+def get_children(doctype, value):
+	children = get_descendants_of(doctype, value)
+
+	return [value, *children]
 
 
 def execute(filters=None):
@@ -512,4 +467,5 @@ def execute(filters=None):
 		"party_type": "Customer",
 		"naming_by": ["Selling Settings", "cust_master_name"],
 	}
+
 	return PartyLedgerSummaryReport(filters).run(args)

erpnext/accounts/report/customer_ledger_summary/test_customer_ledger_summary.py

@@ -0,0 +1,152 @@
+import frappe
+from frappe import qb
+from frappe.tests import IntegrationTestCase
+from frappe.utils import add_days, flt, getdate, today
+
+from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry
+from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
+from erpnext.accounts.report.customer_ledger_summary.customer_ledger_summary import execute
+from erpnext.accounts.test.accounts_mixin import AccountsTestMixin
+
+
+class TestCustomerLedgerSummary(AccountsTestMixin, IntegrationTestCase):
+	def setUp(self):
+		self.create_company()
+		self.create_customer()
+		self.create_item()
+		self.clear_old_entries()
+
+	def tearDown(self):
+		frappe.db.rollback()
+
+	def create_sales_invoice(self, do_not_submit=False, **args):
+		si = create_sales_invoice(
+			item=self.item,
+			company=self.company,
+			customer=self.customer,
+			debit_to=self.debit_to,
+			posting_date=today(),
+			parent_cost_center=self.cost_center,
+			cost_center=self.cost_center,
+			rate=100,
+			qty=10,
+			price_list_rate=100,
+			do_not_save=1,
+			**args,
+		)
+		si = si.save()
+		if not do_not_submit:
+			si = si.submit()
+		return si
+
+	def create_payment_entry(self, docname, do_not_submit=False):
+		pe = get_payment_entry("Sales Invoice", docname, bank_account=self.cash, party_amount=40)
+		pe.paid_from = self.debit_to
+		pe.insert()
+		if not do_not_submit:
+			pe.submit()
+		return pe
+
+	def create_credit_note(self, docname, do_not_submit=False):
+		credit_note = create_sales_invoice(
+			company=self.company,
+			customer=self.customer,
+			item=self.item,
+			qty=-1,
+			debit_to=self.debit_to,
+			cost_center=self.cost_center,
+			is_return=1,
+			return_against=docname,
+			do_not_submit=do_not_submit,
+		)
+
+		return credit_note
+
+	def test_ledger_summary_basic_output(self):
+		filters = {"company": self.company, "from_date": today(), "to_date": today()}
+
+		si = self.create_sales_invoice(do_not_submit=True)
+		si.save().submit()
+
+		expected = {
+			"party": "_Test Customer",
+			"party_name": "_Test Customer",
+			"opening_balance": 0,
+			"invoiced_amount": 1000.0,
+			"paid_amount": 0,
+			"return_amount": 0,
+			"closing_balance": 1000.0,
+			"currency": "INR",
+			"customer_name": "_Test Customer",
+		}
+
+		report = execute(filters)[1]
+		self.assertEqual(len(report), 1)
+		for field in expected:
+			with self.subTest(field=field):
+				self.assertEqual(report[0].get(field), expected.get(field))
+
+	def test_summary_with_return_and_payment(self):
+		filters = {"company": self.company, "from_date": today(), "to_date": today()}
+
+		si = self.create_sales_invoice(do_not_submit=True)
+		si.save().submit()
+
+		expected = {
+			"party": "_Test Customer",
+			"party_name": "_Test Customer",
+			"opening_balance": 0,
+			"invoiced_amount": 1000.0,
+			"paid_amount": 0,
+			"return_amount": 0,
+			"closing_balance": 1000.0,
+			"currency": "INR",
+			"customer_name": "_Test Customer",
+		}
+
+		report = execute(filters)[1]
+		self.assertEqual(len(report), 1)
+		for field in expected:
+			with self.subTest(field=field):
+				self.assertEqual(report[0].get(field), expected.get(field))
+
+		cr_note = self.create_credit_note(si.name, True)
+		cr_note.items[0].qty = -2
+		cr_note.save().submit()
+
+		expected_after_cr_note = {
+			"party": "_Test Customer",
+			"party_name": "_Test Customer",
+			"opening_balance": 0,
+			"invoiced_amount": 1000.0,
+			"paid_amount": 0,
+			"return_amount": 200.0,
+			"closing_balance": 800.0,
+			"currency": "INR",
+		}
+		report = execute(filters)[1]
+		self.assertEqual(len(report), 1)
+		for field in expected_after_cr_note:
+			with self.subTest(field=field):
+				self.assertEqual(report[0].get(field), expected_after_cr_note.get(field))
+
+		pe = self.create_payment_entry(si.name, True)
+		pe.paid_amount = 500
+		pe.save().submit()
+
+		expected_after_cr_and_payment = {
+			"party": "_Test Customer",
+			"party_name": "_Test Customer",
+			"opening_balance": 0,
+			"invoiced_amount": 1000.0,
+			"paid_amount": 500.0,
+			"return_amount": 200.0,
+			"closing_balance": 300.0,
+			"currency": "INR",
+		}
+
+		report = execute(filters)[1]
+		self.assertEqual(len(report), 1)
+		for field in expected_after_cr_and_payment:
+			with self.subTest(field=field):
+				self.assertEqual(report[0].get(field), expected_after_cr_and_payment.get(field))

erpnext/accounts/report/supplier_ledger_summary/test_supplier_ledger_summary.py

@@ -0,0 +1,61 @@
+import frappe
+from frappe.tests import IntegrationTestCase
+from frappe.utils import today
+
+from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice
+from erpnext.accounts.report.supplier_ledger_summary.supplier_ledger_summary import execute
+from erpnext.accounts.test.accounts_mixin import AccountsTestMixin
+
+
+class TestSupplierLedgerSummary(AccountsTestMixin, IntegrationTestCase):
+	def setUp(self):
+		self.create_company()
+		self.create_supplier()
+		self.create_item()
+		self.clear_old_entries()
+
+	def tearDown(self):
+		frappe.db.rollback()
+
+	def create_purchase_invoice(self, do_not_submit=False):
+		frappe.set_user("Administrator")
+		pi = make_purchase_invoice(
+			item=self.item,
+			company=self.company,
+			supplier=self.supplier,
+			is_return=False,
+			update_stock=False,
+			posting_date=frappe.utils.datetime.date(2021, 5, 1),
+			do_not_save=1,
+			rate=300,
+			price_list_rate=300,
+			qty=1,
+		)
+
+		pi = pi.save()
+		if not do_not_submit:
+			pi = pi.submit()
+		return pi
+
+	def test_basic_supplier_ledger_summary(self):
+		self.create_purchase_invoice()
+
+		filters = {"company": self.company, "from_date": today(), "to_date": today()}
+
+		expected = {
+			"party": "_Test Supplier",
+			"party_name": "_Test Supplier",
+			"opening_balance": 0,
+			"invoiced_amount": 300.0,
+			"paid_amount": 0,
+			"return_amount": 0,
+			"closing_balance": 300.0,
+			"currency": "INR",
+			"supplier_name": "_Test Supplier",
+		}
+
+		report_output = execute(filters)[1]
+		self.assertEqual(len(report_output), 1)
+		for field in expected:
+			with self.subTest(field=field):
+				self.assertEqual(report_output[0].get(field), expected.get(field))