Merge pull request #39509 from frappe/mergify/bp/version-14-hotfix/pr-39054

refactor: provision to filter on dimensions in reconciliation tool (backport #39054)

erpnext/controllers/accounts_controller.py

@@ -7,6 +7,8 @@ import json
 import frappe
 from frappe import _, bold, qb, throw
 from frappe.model.workflow import get_workflow_name, is_transition_condition_satisfied
+from frappe.query_builder import Criterion
+from frappe.query_builder.custom import ConstantColumn
 from frappe.query_builder.functions import Abs, Sum
 from frappe.utils import (
 	add_days,
@@ -26,6 +28,7 @@ from frappe.utils import (
 import erpnext
 from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import (
 	get_accounting_dimensions,
+	get_dimensions,
 )
 from erpnext.accounts.doctype.pricing_rule.utils import (
 	apply_pricing_rule_for_free_items,
@@ -1118,7 +1121,9 @@ class AccountsController(TransactionBase):
 					return True
 		return False
 
-	def make_exchange_gain_loss_journal(self, args: dict = None) -> None:
+	def make_exchange_gain_loss_journal(
+		self, args: dict = None, dimensions_dict: dict = None
+	) -> None:
 		"""
 		Make Exchange Gain/Loss journal for Invoices and Payments
 		"""
@@ -1173,6 +1178,7 @@ class AccountsController(TransactionBase):
 									self.name,
 									arg.get("referenced_row"),
 									arg.get("cost_center"),
+									dimensions_dict,
 								)
 								frappe.msgprint(
 									_("Exchange Gain/Loss amount has been booked through {0}").format(
@@ -1253,6 +1259,7 @@ class AccountsController(TransactionBase):
 							self.name,
 							d.idx,
 							self.cost_center,
+							dimensions_dict,
 						)
 						frappe.msgprint(
 							_("Exchange Gain/Loss amount has been booked through {0}").format(
@@ -1344,7 +1351,13 @@ class AccountsController(TransactionBase):
 		if lst:
 			from erpnext.accounts.utils import reconcile_against_document
 
-			reconcile_against_document(lst)
+			# pass dimension values to utility method
+			active_dimensions = get_dimensions()[0]
+			for x in lst:
+				for dim in active_dimensions:
+					if self.get(dim.fieldname):
+						x.update({dim.fieldname: self.get(dim.fieldname)})
+			reconcile_against_document(lst, active_dimensions=active_dimensions)
 
 	def on_cancel(self):
 		from erpnext.accounts.doctype.bank_transaction.bank_transaction import (
@@ -2530,6 +2543,9 @@ def get_advance_payment_entries(
 	condition=None,
 	payment_name=None,
 ):
+	pe = qb.DocType("Payment Entry")
+	per = qb.DocType("Payment Entry Reference")
+
 	party_account_field = "paid_from" if party_type == "Customer" else "paid_to"
 	currency_field = (
 		"paid_from_account_currency" if party_type == "Customer" else "paid_to_account_currency"
@@ -2540,76 +2556,79 @@ def get_advance_payment_entries(
 	)
 
 	payment_entries_against_order, unallocated_payment_entries = [], []
-	limit_cond = "limit %s" % limit if limit else ""
+
+	if not condition:
+		condition = []
+
+	if payment_name:
+		condition.append(pe.name.like(f"%%{payment_name}%%"))
 
 	if order_list or against_all_orders:
+		orders_condition = []
 		if order_list:
-			reference_condition = " and t2.reference_name in ({0})".format(
-				", ".join(["%s"] * len(order_list))
+			orders_condition.append(per.reference_name.isin(order_list))
+		payment_entries_query = (
+			qb.from_(pe)
+			.inner_join(per)
+			.on(pe.name == per.parent)
+			.select(
+				ConstantColumn("Payment Entry").as_("reference_type"),
+				pe.name.as_("reference_name"),
+				pe.remarks,
+				per.allocated_amount.as_("amount"),
+				per.name.as_("reference_row"),
+				per.reference_name.as_("against_order"),
+				pe.posting_date,
+				pe[currency_field].as_("currency"),
+				pe[exchange_rate_field].as_("exchange_rate"),
 			)
-		else:
-			reference_condition = ""
-			order_list = []
-
-		payment_name_filter = ""
-		if payment_name:
-			payment_name_filter = " and t1.name like '%%{0}%%'".format(payment_name)
-
-		if not condition:
-			condition = ""
-
-		payment_entries_against_order = frappe.db.sql(
-			"""
-			select
-				'Payment Entry' as reference_type, t1.name as reference_name,
-				t1.remarks, t2.allocated_amount as amount, t2.name as reference_row,
-				t2.reference_name as against_order, t1.posting_date,
-				t1.{0} as currency, t1.{5} as exchange_rate
-			from `tabPayment Entry` t1, `tabPayment Entry Reference` t2
-			where
-				t1.name = t2.parent and t1.{1} = %s and t1.payment_type = %s
-				and t1.party_type = %s and t1.party = %s and t1.docstatus = 1
-				and t2.reference_doctype = %s {2} {3} {6}
-			order by t1.posting_date {4}
-		""".format(
-				currency_field,
-				party_account_field,
-				reference_condition,
-				condition,
-				limit_cond,
-				exchange_rate_field,
-				payment_name_filter,
-			),
-			[party_account, payment_type, party_type, party, order_doctype] + order_list,
-			as_dict=1,
+			.where(
+				(pe[party_account_field] == party_account)
+				& (pe.payment_type == payment_type)
+				& (pe.party_type == party_type)
+				& (pe.party == party)
+				& (pe.docstatus == 1)
+				& (per.reference_doctype == order_doctype)
+			)
+			.where(Criterion.all(condition))
+			.where(Criterion.all(orders_condition))
+			.orderby(pe.posting_date)
 		)
 
+		if limit:
+			payment_entries_query = payment_entries_query.limit(limit)
+
+		payment_entries_against_order = payment_entries_query.run(as_dict=1)
+
 	if include_unallocated:
-		payment_name_filter = ""
-		if payment_name:
-			payment_name_filter = " and name like '%%{0}%%'".format(payment_name)
-
-		unallocated_payment_entries = frappe.db.sql(
-			"""
-				select 'Payment Entry' as reference_type, name as reference_name, posting_date,
-				remarks, unallocated_amount as amount, {2} as exchange_rate, {3} as currency
-				from `tabPayment Entry`
-				where
-					{0} = %s and party_type = %s and party = %s and payment_type = %s
-					and docstatus = 1 and unallocated_amount > 0 {condition} {4}
-				order by posting_date {1}
-			""".format(
-				party_account_field,
-				limit_cond,
-				exchange_rate_field,
-				currency_field,
-				payment_name_filter,
-				condition=condition or "",
-			),
-			(party_account, party_type, party, payment_type),
-			as_dict=1,
+		unallocated_payment_query = (
+			qb.from_(pe)
+			.select(
+				ConstantColumn("Payment Entry").as_("reference_type"),
+				pe.name.as_("reference_name"),
+				pe.posting_date,
+				pe.remarks,
+				pe.unallocated_amount.as_("amount"),
+				pe[exchange_rate_field].as_("exchange_rate"),
+				pe[currency_field].as_("currency"),
+			)
+			.where(
+				(pe[party_account_field] == party_account)
+				& (pe.party_type == party_type)
+				& (pe.party == party)
+				& (pe.payment_type == payment_type)
+				& (pe.docstatus == 1)
+				& (pe.unallocated_amount.gt(0))
+			)
+			.where(Criterion.all(condition))
+			.orderby(pe.posting_date)
 		)
 
+		if limit:
+			unallocated_payment_query = unallocated_payment_query.limit(limit)
+
+		unallocated_payment_entries = unallocated_payment_query.run(as_dict=1)
+
 	return list(payment_entries_against_order) + list(unallocated_payment_entries)
 
 

erpnext/controllers/tests/test_accounts_controller.py

@@ -56,6 +56,7 @@ class TestAccountsController(FrappeTestCase):
 	20 series - Sales Invoice against Journals
 	30 series - Sales Invoice against Credit Notes
 	40 series - Company default Cost center is unset
+	50 series - Dimension inheritence
 	"""
 
 	def setUp(self):
@@ -1255,3 +1256,253 @@ class TestAccountsController(FrappeTestCase):
 				)
 
 		frappe.db.set_value("Company", self.company, "cost_center", cc)
+
+	def setup_dimensions(self):
+		if not frappe.db.exists("Accounting Dimension", {"document_type": "Department"}):
+			frappe.get_doc(
+				{
+					"doctype": "Accounting Dimension",
+					"document_type": "Department",
+				}
+			).insert()
+		else:
+			dimension = frappe.get_doc("Accounting Dimension", "Department")
+			dimension.disabled = 0
+			dimension.save()
+
+		if not frappe.db.exists("Accounting Dimension", {"document_type": "Location"}):
+			dimension1 = frappe.get_doc(
+				{
+					"doctype": "Accounting Dimension",
+					"document_type": "Location",
+				}
+			)
+			dimension1.append(
+				"dimension_defaults",
+				{
+					"company": "_Test Company",
+					"reference_document": "Location",
+					"default_dimension": "Block 1",
+					"mandatory_for_bs": 0,
+					"mandatory_for_pl": 0,
+				},
+			)
+
+			dimension1.insert()
+			dimension1.save()
+		else:
+			dimension1 = frappe.get_doc("Accounting Dimension", "Location")
+			dimension1.disabled = 0
+			dimension1.save()
+
+	def disable_dimensions(self):
+		if frappe.db.exists("Accounting Dimension", {"document_type": "Department"}):
+			dimension = frappe.get_doc("Accounting Dimension", "Department")
+			dimension.disabled = 1
+			dimension.save()
+
+		if frappe.db.exists("Accounting Dimension", {"document_type": "Location"}):
+			dimension1 = frappe.get_doc("Accounting Dimension", "Location")
+			dimension1.disabled = 1
+			dimension1.save()
+
+	def test_50_dimensions_filter(self):
+		"""
+		Test workings of dimension filters
+		"""
+		self.setup_dimensions()
+		rate_in_account_currency = 1
+
+		# Invoices
+		si1 = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_submit=True)
+		si1.department = "Management"
+		si1.save().submit()
+
+		si2 = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_submit=True)
+		si2.department = "Operations"
+		si2.save().submit()
+
+		# Payments
+		cr_note1 = self.create_sales_invoice(qty=-1, conversion_rate=75, rate=1, do_not_save=True)
+		cr_note1.department = "Management"
+		cr_note1.is_return = 1
+		cr_note1.save().submit()
+
+		cr_note2 = self.create_sales_invoice(qty=-1, conversion_rate=75, rate=1, do_not_save=True)
+		cr_note2.department = "Legal"
+		cr_note2.is_return = 1
+		cr_note2.save().submit()
+
+		pe1 = get_payment_entry(si1.doctype, si1.name)
+		pe1.references = []
+		pe1.department = "Research & Development"
+		pe1.save().submit()
+
+		pe2 = get_payment_entry(si1.doctype, si1.name)
+		pe2.references = []
+		pe2.department = "Management"
+		pe2.save().submit()
+
+		je1 = self.create_journal_entry(
+			acc1=self.debit_usd,
+			acc1_exc_rate=75,
+			acc2=self.cash,
+			acc1_amount=-1,
+			acc2_amount=-75,
+			acc2_exc_rate=1,
+		)
+		je1.accounts[0].party_type = "Customer"
+		je1.accounts[0].party = self.customer
+		je1.accounts[0].department = "Management"
+		je1.save().submit()
+
+		# assert dimension filter's result
+		pr = self.create_payment_reconciliation()
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.invoices), 2)
+		self.assertEqual(len(pr.payments), 5)
+
+		pr.department = "Legal"
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.invoices), 0)
+		self.assertEqual(len(pr.payments), 1)
+
+		pr.department = "Management"
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.invoices), 1)
+		self.assertEqual(len(pr.payments), 3)
+
+		pr.department = "Research & Development"
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.invoices), 0)
+		self.assertEqual(len(pr.payments), 1)
+		self.disable_dimensions()
+
+	def test_51_cr_note_should_inherit_dimension(self):
+		self.setup_dimensions()
+		rate_in_account_currency = 1
+
+		# Invoice
+		si = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_submit=True)
+		si.department = "Management"
+		si.save().submit()
+
+		# Payment
+		cr_note = self.create_sales_invoice(qty=-1, conversion_rate=75, rate=1, do_not_save=True)
+		cr_note.department = "Management"
+		cr_note.is_return = 1
+		cr_note.save().submit()
+
+		pr = self.create_payment_reconciliation()
+		pr.department = "Management"
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.invoices), 1)
+		self.assertEqual(len(pr.payments), 1)
+		invoices = [x.as_dict() for x in pr.invoices]
+		payments = [x.as_dict() for x 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)
+
+		# There should be 2 journals, JE(Cr Note) and JE(Exchange Gain/Loss)
+		exc_je_for_si = self.get_journals_for(si.doctype, si.name)
+		exc_je_for_cr_note = self.get_journals_for(cr_note.doctype, cr_note.name)
+		self.assertNotEqual(exc_je_for_si, [])
+		self.assertEqual(len(exc_je_for_si), 2)
+		self.assertEqual(len(exc_je_for_cr_note), 2)
+		self.assertEqual(exc_je_for_si, exc_je_for_cr_note)
+
+		for x in exc_je_for_si + exc_je_for_cr_note:
+			with self.subTest(x=x):
+				self.assertEqual(
+					[cr_note.department, cr_note.department],
+					frappe.db.get_all("Journal Entry Account", filters={"parent": x.parent}, pluck="department"),
+				)
+		self.disable_dimensions()
+
+	def test_52_dimension_inhertiance_exc_gain_loss(self):
+		# Sales Invoice in Foreign Currency
+		self.setup_dimensions()
+		rate = 80
+		rate_in_account_currency = 1
+		dpt = "Research & Development"
+
+		si = self.create_sales_invoice(qty=1, rate=rate_in_account_currency, do_not_save=True)
+		si.department = dpt
+		si.save().submit()
+
+		pe = self.create_payment_entry(amount=1, source_exc_rate=82).save()
+		pe.department = dpt
+		pe = pe.save().submit()
+
+		pr = self.create_payment_reconciliation()
+		pr.department = dpt
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.invoices), 1)
+		self.assertEqual(len(pr.payments), 1)
+		invoices = [x.as_dict() for x in pr.invoices]
+		payments = [x.as_dict() for x 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)
+
+		# Exc Gain/Loss journals should inherit dimension from parent
+		journals = self.get_journals_for(si.doctype, si.name)
+		self.assertEqual(
+			[dpt, dpt],
+			frappe.db.get_all(
+				"Journal Entry Account",
+				filters={"parent": ("in", [x.parent for x in journals])},
+				pluck="department",
+			),
+		)
+		self.disable_dimensions()
+
+	def test_53_dimension_inheritance_on_advance(self):
+		self.setup_dimensions()
+		dpt = "Research & Development"
+
+		adv = self.create_payment_entry(amount=1, source_exc_rate=85)
+		adv.department = dpt
+		adv.save().submit()
+		adv.reload()
+
+		# Sales Invoices in different exchange rates
+		si = self.create_sales_invoice(qty=1, conversion_rate=82, rate=1, do_not_submit=True)
+		si.department = dpt
+		advances = si.get_advance_entries()
+		self.assertEqual(len(advances), 1)
+		self.assertEqual(advances[0].reference_name, adv.name)
+		si.append(
+			"advances",
+			{
+				"doctype": "Sales Invoice Advance",
+				"reference_type": advances[0].reference_type,
+				"reference_name": advances[0].reference_name,
+				"reference_row": advances[0].reference_row,
+				"advance_amount": 1,
+				"allocated_amount": 1,
+				"ref_exchange_rate": advances[0].exchange_rate,
+				"remarks": advances[0].remarks,
+			},
+		)
+		si = si.save().submit()
+
+		# Outstanding in both currencies should be '0'
+		adv.reload()
+		self.assertEqual(si.outstanding_amount, 0)
+		self.assert_ledger_outstanding(si.doctype, si.name, 0.0, 0.0)
+
+		# Exc Gain/Loss journals should inherit dimension from parent
+		journals = self.get_journals_for(si.doctype, si.name)
+		self.assertEqual(
+			[dpt, dpt],
+			frappe.db.get_all(
+				"Journal Entry Account",
+				filters={"parent": ("in", [x.parent for x in journals])},
+				pluck="department",
+			),
+		)
+		self.disable_dimensions()