Merge branch 'develop' into fix/payment-url

erpnext/accounts/doctype/account/account.js

@@ -137,9 +137,6 @@ frappe.ui.form.on("Account", {
 					args: {
 						old: frm.doc.name,
 						new: data.name,
-						is_group: frm.doc.is_group,
-						root_type: frm.doc.root_type,
-						company: frm.doc.company,
 					},
 					callback: function (r) {
 						if (!r.exc) {

erpnext/accounts/doctype/account/account.py

@@ -18,6 +18,10 @@ class BalanceMismatchError(frappe.ValidationError):
 	pass
 
 
+class InvalidAccountMergeError(frappe.ValidationError):
+	pass
+
+
 class Account(NestedSet):
 	nsm_parent_field = "parent_account"
 
@@ -460,25 +464,34 @@ def update_account_number(name, account_name, account_number=None, from_descenda
 
 
 @frappe.whitelist()
-def merge_account(old, new, is_group, root_type, company):
+def merge_account(old, new):
 	# Validate properties before merging
 	new_account = frappe.get_cached_doc("Account", new)
+	old_account = frappe.get_cached_doc("Account", old)
 
 	if not new_account:
 		throw(_("Account {0} does not exist").format(new))
 
-	if (new_account.is_group, new_account.root_type, new_account.company) != (
+	if (
-		cint(is_group),
+		cint(new_account.is_group),
-		root_type,
+		new_account.root_type,
-		company,
+		new_account.company,
+		cstr(new_account.account_currency),
+	) != (
+		cint(old_account.is_group),
+		old_account.root_type,
+		old_account.company,
+		cstr(old_account.account_currency),
 	):
 		throw(
-			_(
+			msg=_(
-				"""Merging is only possible if following properties are same in both records. Is Group, Root Type, Company"""
+				"""Merging is only possible if following properties are same in both records. Is Group, Root Type, Company and Account Currency"""
-			)
+			),
+			title=("Invalid Accounts"),
+			exc=InvalidAccountMergeError,
 		)
 
-	if is_group and new_account.parent_account == old:
+	if old_account.is_group and new_account.parent_account == old:
 		new_account.db_set("parent_account", frappe.get_cached_value("Account", old, "parent_account"))
 
 	frappe.rename_doc("Account", old, new, merge=1, force=1)

erpnext/accounts/doctype/account/test_account.py

@@ -7,7 +7,11 @@ import unittest
 import frappe
 from frappe.test_runner import make_test_records
 
-from erpnext.accounts.doctype.account.account import merge_account, update_account_number
+from erpnext.accounts.doctype.account.account import (
+	InvalidAccountMergeError,
+	merge_account,
+	update_account_number,
+)
 from erpnext.stock import get_company_default_inventory_account, get_warehouse_account
 
 test_dependencies = ["Company"]
@@ -47,49 +51,53 @@ class TestAccount(unittest.TestCase):
 		frappe.delete_doc("Account", "1211-11-4 - 6 - Debtors 1 - Test - - _TC")
 
 	def test_merge_account(self):
-		if not frappe.db.exists("Account", "Current Assets - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Current Assets",
-			acc.account_name = "Current Assets"
+			is_group=1,
-			acc.is_group = 1
+			parent_account="Application of Funds (Assets) - _TC",
-			acc.parent_account = "Application of Funds (Assets) - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Securities and Deposits - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Securities and Deposits",
-			acc.account_name = "Securities and Deposits"
+			is_group=1,
-			acc.parent_account = "Current Assets - _TC"
+			parent_account="Current Assets - _TC",
-			acc.is_group = 1
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Earnest Money - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Earnest Money",
-			acc.account_name = "Earnest Money"
+			parent_account="Securities and Deposits - _TC",
-			acc.parent_account = "Securities and Deposits - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Cash In Hand - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Cash In Hand",
-			acc.account_name = "Cash In Hand"
+			is_group=1,
-			acc.is_group = 1
+			parent_account="Current Assets - _TC",
-			acc.parent_account = "Current Assets - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Accumulated Depreciation - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Receivable INR",
-			acc.account_name = "Accumulated Depreciation"
+			parent_account="Current Assets - _TC",
-			acc.parent_account = "Fixed Assets - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+			account_currency="INR",
-			acc.account_type = "Accumulated Depreciation"
+		)
-			acc.insert()
+
+		create_account(
+			account_name="Receivable USD",
+			parent_account="Current Assets - _TC",
+			company="_Test Company",
+			account_currency="USD",
+		)
 
-		doc = frappe.get_doc("Account", "Securities and Deposits - _TC")
 		parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
 
 		self.assertEqual(parent, "Securities and Deposits - _TC")
 
-		merge_account(
+		merge_account("Securities and Deposits - _TC", "Cash In Hand - _TC")
-			"Securities and Deposits - _TC", "Cash In Hand - _TC", doc.is_group, doc.root_type, doc.company
+
-		)
 		parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
 
 		# Parent account of the child account changes after merging
@@ -98,30 +106,28 @@ class TestAccount(unittest.TestCase):
 		# Old account doesn't exist after merging
 		self.assertFalse(frappe.db.exists("Account", "Securities and Deposits - _TC"))
 
-		doc = frappe.get_doc("Account", "Current Assets - _TC")
-
 		# Raise error as is_group property doesn't match
 		self.assertRaises(
-			frappe.ValidationError,
+			InvalidAccountMergeError,
 			merge_account,
 			"Current Assets - _TC",
 			"Accumulated Depreciation - _TC",
-			doc.is_group,
-			doc.root_type,
-			doc.company,
 		)
 
-		doc = frappe.get_doc("Account", "Capital Stock - _TC")
-
 		# Raise error as root_type property doesn't match
 		self.assertRaises(
-			frappe.ValidationError,
+			InvalidAccountMergeError,
 			merge_account,
 			"Capital Stock - _TC",
 			"Softwares - _TC",
-			doc.is_group,
+		)
-			doc.root_type,
+
-			doc.company,
+		# Raise error as currency doesn't match
+		self.assertRaises(
+			InvalidAccountMergeError,
+			merge_account,
+			"Receivable INR - _TC",
+			"Receivable USD - _TC",
 		)
 
 	def test_account_sync(self):
@@ -400,11 +406,20 @@ def create_account(**kwargs):
 		"Account", filters={"account_name": kwargs.get("account_name"), "company": kwargs.get("company")}
 	)
 	if account:
-		return account
+		account = frappe.get_doc("Account", account)
+		account.update(
+			dict(
+				is_group=kwargs.get("is_group", 0),
+				parent_account=kwargs.get("parent_account"),
+			)
+		)
+		account.save()
+		return account.name
 	else:
 		account = frappe.get_doc(
 			dict(
 				doctype="Account",
+				is_group=kwargs.get("is_group", 0),
 				account_name=kwargs.get("account_name"),
 				account_type=kwargs.get("account_type"),
 				parent_account=kwargs.get("parent_account"),

erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py

@@ -7,7 +7,9 @@ import json
 import frappe
 from frappe import _
 from frappe.model.document import Document
+from frappe.query_builder.custom import ConstantColumn
 from frappe.utils import cint, flt
+from pypika.terms import Parameter
 
 from erpnext import get_default_cost_center
 from erpnext.accounts.doctype.bank_transaction.bank_transaction import get_total_allocated_amount
@@ -15,7 +17,7 @@ from erpnext.accounts.report.bank_reconciliation_statement.bank_reconciliation_s
 	get_amounts_not_reflected_in_system,
 	get_entries,
 )
-from erpnext.accounts.utils import get_balance_on
+from erpnext.accounts.utils import get_account_currency, get_balance_on
 
 
 class BankReconciliationTool(Document):
@@ -283,68 +285,68 @@ def auto_reconcile_vouchers(
 	to_reference_date=None,
 ):
 	frappe.flags.auto_reconcile_vouchers = True
-	document_types = ["payment_entry", "journal_entry"]
+	reconciled, partially_reconciled = set(), set()
+
 	bank_transactions = get_bank_transactions(bank_account)
-	matched_transaction = []
 	for transaction in bank_transactions:
 		linked_payments = get_linked_payments(
 			transaction.name,
-			document_types,
+			["payment_entry", "journal_entry"],
 			from_date,
 			to_date,
 			filter_by_reference_date,
 			from_reference_date,
 			to_reference_date,
 		)
-		vouchers = []
+
-		for r in linked_payments:
+		if not linked_payments:
-			vouchers.append(
+			continue
-				{
+
-					"payment_doctype": r[1],
+		vouchers = list(
-					"payment_name": r[2],
+			map(
-					"amount": r[4],
+				lambda entry: {
-				}
+					"payment_doctype": entry.get("doctype"),
-			)
+					"payment_name": entry.get("name"),
-		transaction = frappe.get_doc("Bank Transaction", transaction.name)
+					"amount": entry.get("paid_amount"),
-		account = frappe.db.get_value("Bank Account", transaction.bank_account, "account")
-		matched_trans = 0
-		for voucher in vouchers:
-			gl_entry = frappe.db.get_value(
-				"GL Entry",
-				dict(
-					account=account, voucher_type=voucher["payment_doctype"], voucher_no=voucher["payment_name"]
-				),
-				["credit", "debit"],
-				as_dict=1,
-			)
-			gl_amount, transaction_amount = (
-				(gl_entry.credit, transaction.deposit)
-				if gl_entry.credit > 0
-				else (gl_entry.debit, transaction.withdrawal)
-			)
-			allocated_amount = gl_amount if gl_amount >= transaction_amount else transaction_amount
-			transaction.append(
-				"payment_entries",
-				{
-					"payment_document": voucher["payment_doctype"],
-					"payment_entry": voucher["payment_name"],
-					"allocated_amount": allocated_amount,
 				},
+				linked_payments,
 			)
-			matched_transaction.append(str(transaction.name))
+		)
-		transaction.save()
+
-		transaction.update_allocations()
+		updated_transaction = reconcile_vouchers(transaction.name, json.dumps(vouchers))
-	matched_transaction_len = len(set(matched_transaction))
+
-	if matched_transaction_len == 0:
+		if updated_transaction.status == "Reconciled":
-		frappe.msgprint(_("No matching references found for auto reconciliation"))
+			reconciled.add(updated_transaction.name)
-	elif matched_transaction_len == 1:
+		elif flt(transaction.unallocated_amount) != flt(updated_transaction.unallocated_amount):
-		frappe.msgprint(_("{0} transaction is reconcilied").format(matched_transaction_len))
+			# Partially reconciled (status = Unreconciled & unallocated amount changed)
-	else:
+			partially_reconciled.add(updated_transaction.name)
-		frappe.msgprint(_("{0} transactions are reconcilied").format(matched_transaction_len))
+
+	alert_message, indicator = get_auto_reconcile_message(partially_reconciled, reconciled)
+	frappe.msgprint(title=_("Auto Reconciliation"), msg=alert_message, indicator=indicator)
 
 	frappe.flags.auto_reconcile_vouchers = False
+	return reconciled, partially_reconciled
 
-	return frappe.get_doc("Bank Transaction", transaction.name)
+
+def get_auto_reconcile_message(partially_reconciled, reconciled):
+	"""Returns alert message and indicator for auto reconciliation depending on result state."""
+	alert_message, indicator = "", "blue"
+	if not partially_reconciled and not reconciled:
+		alert_message = _("No matches occurred via auto reconciliation")
+		return alert_message, indicator
+
+	indicator = "green"
+	if reconciled:
+		alert_message += _("{0} Transaction(s) Reconciled").format(len(reconciled))
+		alert_message += "<br>"
+
+	if partially_reconciled:
+		alert_message += _("{0} {1} Partially Reconciled").format(
+			len(partially_reconciled),
+			_("Transactions") if len(partially_reconciled) > 1 else _("Transaction"),
+		)
+
+	return alert_message, indicator
 
 
 @frappe.whitelist()
@@ -390,19 +392,13 @@ def subtract_allocations(gl_account, vouchers):
 	"Look up & subtract any existing Bank Transaction allocations"
 	copied = []
 	for voucher in vouchers:
-		rows = get_total_allocated_amount(voucher[1], voucher[2])
+		rows = get_total_allocated_amount(voucher.get("doctype"), voucher.get("name"))
-		amount = None
+		filtered_row = list(filter(lambda row: row.get("gl_account") == gl_account, rows))
-		for row in rows:
-			if row["gl_account"] == gl_account:
-				amount = row["total"]
-				break
 
-		if amount:
+		if amount := None if not filtered_row else filtered_row[0]["total"]:
-			l = list(voucher)
+			voucher["paid_amount"] -= amount
-			l[3] -= amount
+
-			copied.append(tuple(l))
+		copied.append(voucher)
-		else:
-			copied.append(voucher)
 	return copied
 
 
@@ -418,6 +414,18 @@ def check_matching(
 	to_reference_date,
 ):
 	exact_match = True if "exact_match" in document_types else False
+	queries = get_queries(
+		bank_account,
+		company,
+		transaction,
+		document_types,
+		from_date,
+		to_date,
+		filter_by_reference_date,
+		from_reference_date,
+		to_reference_date,
+		exact_match,
+	)
 
 	filters = {
 		"amount": transaction.unallocated_amount,
@@ -429,30 +437,15 @@ def check_matching(
 	}
 
 	matching_vouchers = []
+	for query in queries:
+		matching_vouchers.extend(frappe.db.sql(query, filters, as_dict=True))
 
-	# get matching vouchers from all the apps
+	return (
-	for method_name in frappe.get_hooks("get_matching_vouchers_for_bank_reconciliation"):
+		sorted(matching_vouchers, key=lambda x: x["rank"], reverse=True) if matching_vouchers else []
-		matching_vouchers.extend(
+	)
-			frappe.get_attr(method_name)(
-				bank_account,
-				company,
-				transaction,
-				document_types,
-				from_date,
-				to_date,
-				filter_by_reference_date,
-				from_reference_date,
-				to_reference_date,
-				exact_match,
-				filters,
-			)
-			or []
-		)
-
-	return sorted(matching_vouchers, key=lambda x: x[0], reverse=True) if matching_vouchers else []
 
 
-def get_matching_vouchers_for_bank_reconciliation(
+def get_queries(
 	bank_account,
 	company,
 	transaction,
@@ -463,7 +456,6 @@ def get_matching_vouchers_for_bank_reconciliation(
 	from_reference_date,
 	to_reference_date,
 	exact_match,
-	filters,
 ):
 	# get queries to get matching vouchers
 	account_from_to = "paid_to" if transaction.deposit > 0.0 else "paid_from"
@@ -488,17 +480,7 @@ def get_matching_vouchers_for_bank_reconciliation(
 			or []
 		)
 
-	vouchers = []
+	return queries
-
-	for query in queries:
-		vouchers.extend(
-			frappe.db.sql(
-				query,
-				filters,
-			)
-		)
-
-	return vouchers
 
 
 def get_matching_queries(
@@ -515,6 +497,8 @@ def get_matching_queries(
 	to_reference_date,
 ):
 	queries = []
+	currency = get_account_currency(bank_account)
+
 	if "payment_entry" in document_types:
 		query = get_pe_matching_query(
 			exact_match,
@@ -541,12 +525,12 @@ def get_matching_queries(
 		queries.append(query)
 
 	if transaction.deposit > 0.0 and "sales_invoice" in document_types:
-		query = get_si_matching_query(exact_match)
+		query = get_si_matching_query(exact_match, currency)
 		queries.append(query)
 
 	if transaction.withdrawal > 0.0:
 		if "purchase_invoice" in document_types:
-			query = get_pi_matching_query(exact_match)
+			query = get_pi_matching_query(exact_match, currency)
 			queries.append(query)
 
 	if "bank_transaction" in document_types:
@@ -560,33 +544,48 @@ def get_bt_matching_query(exact_match, transaction):
 	# get matching bank transaction query
 	# find bank transactions in the same bank account with opposite sign
 	# same bank account must have same company and currency
+	bt = frappe.qb.DocType("Bank Transaction")
+
 	field = "deposit" if transaction.withdrawal > 0.0 else "withdrawal"
+	amount_equality = getattr(bt, field) == transaction.unallocated_amount
+	amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
+	amount_condition = amount_equality if exact_match else getattr(bt, field) > 0.0
 
-	return f"""
+	ref_rank = (
+		frappe.qb.terms.Case().when(bt.reference_number == transaction.reference_number, 1).else_(0)
+	)
+	unallocated_rank = (
+		frappe.qb.terms.Case().when(bt.unallocated_amount == transaction.unallocated_amount, 1).else_(0)
+	)
 
-		SELECT
+	party_condition = (
-			(CASE WHEN reference_number = %(reference_no)s THEN 1 ELSE 0 END
+		(bt.party_type == transaction.party_type)
-			+ CASE WHEN {field} = %(amount)s THEN 1 ELSE 0 END
+		& (bt.party == transaction.party)
-			+ CASE WHEN ( party_type = %(party_type)s AND party = %(party)s ) THEN 1 ELSE 0 END
+		& bt.party.isnotnull()
-			+ CASE WHEN unallocated_amount = %(amount)s THEN 1 ELSE 0 END
+	)
-			+ 1) AS rank,
+	party_rank = frappe.qb.terms.Case().when(party_condition, 1).else_(0)
-			'Bank Transaction' AS doctype,
+
-			name,
+	query = (
-			unallocated_amount AS paid_amount,
+		frappe.qb.from_(bt)
-			reference_number AS reference_no,
+		.select(
-			date AS reference_date,
+			(ref_rank + amount_rank + party_rank + unallocated_rank + 1).as_("rank"),
-			party,
+			ConstantColumn("Bank Transaction").as_("doctype"),
-			party_type,
+			bt.name,
-			date AS posting_date,
+			bt.unallocated_amount.as_("paid_amount"),
-			currency
+			bt.reference_number.as_("reference_no"),
-		FROM
+			bt.date.as_("reference_date"),
-			`tabBank Transaction`
+			bt.party,
-		WHERE
+			bt.party_type,
-			status != 'Reconciled'
+			bt.date.as_("posting_date"),
-			AND name != '{transaction.name}'
+			bt.currency,
-			AND bank_account = '{transaction.bank_account}'
+		)
-			AND {field} {'= %(amount)s' if exact_match else '> 0.0'}
+		.where(bt.status != "Reconciled")
-	"""
+		.where(bt.name != transaction.name)
+		.where(bt.bank_account == transaction.bank_account)
+		.where(amount_condition)
+		.where(bt.docstatus == 1)
+	)
+	return str(query)
 
 
 def get_pe_matching_query(
@@ -600,45 +599,56 @@ def get_pe_matching_query(
 	to_reference_date,
 ):
 	# get matching payment entries query
-	if transaction.deposit > 0.0:
+	to_from = "to" if transaction.deposit > 0.0 else "from"
-		currency_field = "paid_to_account_currency as currency"
+	currency_field = f"paid_{to_from}_account_currency"
-	else:
+	payment_type = "Receive" if transaction.deposit > 0.0 else "Pay"
-		currency_field = "paid_from_account_currency as currency"
+	pe = frappe.qb.DocType("Payment Entry")
-	filter_by_date = f"AND posting_date between '{from_date}' and '{to_date}'"
+
-	order_by = " posting_date"
+	ref_condition = pe.reference_no == transaction.reference_number
-	filter_by_reference_no = ""
+	ref_rank = frappe.qb.terms.Case().when(ref_condition, 1).else_(0)
+
+	amount_equality = pe.paid_amount == transaction.unallocated_amount
+	amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
+	amount_condition = amount_equality if exact_match else pe.paid_amount > 0.0
+
+	party_condition = (
+		(pe.party_type == transaction.party_type)
+		& (pe.party == transaction.party)
+		& pe.party.isnotnull()
+	)
+	party_rank = frappe.qb.terms.Case().when(party_condition, 1).else_(0)
+
+	filter_by_date = pe.posting_date.between(from_date, to_date)
 	if cint(filter_by_reference_date):
-		filter_by_date = f"AND reference_date between '{from_reference_date}' and '{to_reference_date}'"
+		filter_by_date = pe.reference_date.between(from_reference_date, to_reference_date)
-		order_by = " reference_date"
+
+	query = (
+		frappe.qb.from_(pe)
+		.select(
+			(ref_rank + amount_rank + party_rank + 1).as_("rank"),
+			ConstantColumn("Payment Entry").as_("doctype"),
+			pe.name,
+			pe.paid_amount,
+			pe.reference_no,
+			pe.reference_date,
+			pe.party,
+			pe.party_type,
+			pe.posting_date,
+			getattr(pe, currency_field).as_("currency"),
+		)
+		.where(pe.docstatus == 1)
+		.where(pe.payment_type.isin([payment_type, "Internal Transfer"]))
+		.where(pe.clearance_date.isnull())
+		.where(getattr(pe, account_from_to) == Parameter("%(bank_account)s"))
+		.where(amount_condition)
+		.where(filter_by_date)
+		.orderby(pe.reference_date if cint(filter_by_reference_date) else pe.posting_date)
+	)
+
 	if frappe.flags.auto_reconcile_vouchers == True:
-		filter_by_reference_no = f"AND reference_no = '{transaction.reference_number}'"
+		query = query.where(ref_condition)
-	return f"""
+
-		SELECT
+	return str(query)
-			(CASE WHEN reference_no=%(reference_no)s THEN 1 ELSE 0 END
-			+ CASE WHEN (party_type = %(party_type)s AND party = %(party)s ) THEN 1 ELSE 0 END
-			+ CASE WHEN paid_amount = %(amount)s THEN 1 ELSE 0 END
-			+ 1 ) AS rank,
-			'Payment Entry' as doctype,
-			name,
-			paid_amount,
-			reference_no,
-			reference_date,
-			party,
-			party_type,
-			posting_date,
-			{currency_field}
-		FROM
-			`tabPayment Entry`
-		WHERE
-			docstatus = 1
-			AND payment_type IN (%(payment_type)s, 'Internal Transfer')
-			AND ifnull(clearance_date, '') = ""
-			AND {account_from_to} = %(bank_account)s
-			AND paid_amount {'= %(amount)s' if exact_match else '> 0.0'}
-			{filter_by_date}
-			{filter_by_reference_no}
-		order by{order_by}
-	"""
 
 
 def get_je_matching_query(
@@ -655,100 +665,121 @@ def get_je_matching_query(
 	# So one bank could have both types of bank accounts like asset and liability
 	# So cr_or_dr should be judged only on basis of withdrawal and deposit and not account type
 	cr_or_dr = "credit" if transaction.withdrawal > 0.0 else "debit"
-	filter_by_date = f"AND je.posting_date between '{from_date}' and '{to_date}'"
+	je = frappe.qb.DocType("Journal Entry")
-	order_by = " je.posting_date"
+	jea = frappe.qb.DocType("Journal Entry Account")
-	filter_by_reference_no = ""
+
+	ref_condition = je.cheque_no == transaction.reference_number
+	ref_rank = frappe.qb.terms.Case().when(ref_condition, 1).else_(0)
+
+	amount_field = f"{cr_or_dr}_in_account_currency"
+	amount_equality = getattr(jea, amount_field) == transaction.unallocated_amount
+	amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
+
+	filter_by_date = je.posting_date.between(from_date, to_date)
 	if cint(filter_by_reference_date):
-		filter_by_date = f"AND je.cheque_date between '{from_reference_date}' and '{to_reference_date}'"
+		filter_by_date = je.cheque_date.between(from_reference_date, to_reference_date)
-		order_by = " je.cheque_date"
+
-	if frappe.flags.auto_reconcile_vouchers == True:
+	query = (
-		filter_by_reference_no = f"AND je.cheque_no = '{transaction.reference_number}'"
+		frappe.qb.from_(jea)
-	return f"""
+		.join(je)
-		SELECT
+		.on(jea.parent == je.name)
-			(CASE WHEN je.cheque_no=%(reference_no)s THEN 1 ELSE 0 END
+		.select(
-			+ CASE WHEN jea.{cr_or_dr}_in_account_currency = %(amount)s THEN 1 ELSE 0 END
+			(ref_rank + amount_rank + 1).as_("rank"),
-			+ 1) AS rank ,
+			ConstantColumn("Journal Entry").as_("doctype"),
-			'Journal Entry' AS doctype,
 			je.name,
-			jea.{cr_or_dr}_in_account_currency AS paid_amount,
+			getattr(jea, amount_field).as_("paid_amount"),
-			je.cheque_no AS reference_no,
+			je.cheque_no.as_("reference_no"),
-			je.cheque_date AS reference_date,
+			je.cheque_date.as_("reference_date"),
-			je.pay_to_recd_from AS party,
+			je.pay_to_recd_from.as_("party"),
 			jea.party_type,
 			je.posting_date,
-			jea.account_currency AS currency
+			jea.account_currency.as_("currency"),
-		FROM
+		)
-			`tabJournal Entry Account` AS jea
+		.where(je.docstatus == 1)
-		JOIN
+		.where(je.voucher_type != "Opening Entry")
-			`tabJournal Entry` AS je
+		.where(je.clearance_date.isnull())
-		ON
+		.where(jea.account == Parameter("%(bank_account)s"))
-			jea.parent = je.name
+		.where(amount_equality if exact_match else getattr(jea, amount_field) > 0.0)
-		WHERE
+		.where(je.docstatus == 1)
-			je.docstatus = 1
+		.where(filter_by_date)
-			AND je.voucher_type NOT IN ('Opening Entry')
+		.orderby(je.cheque_date if cint(filter_by_reference_date) else je.posting_date)
-			AND (je.clearance_date IS NULL OR je.clearance_date='0000-00-00')
+	)
-			AND jea.account = %(bank_account)s
+
-			AND jea.{cr_or_dr}_in_account_currency {'= %(amount)s' if exact_match else '> 0.0'}
+	if frappe.flags.auto_reconcile_vouchers == True:
-			AND je.docstatus = 1
+		query = query.where(ref_condition)
-			{filter_by_date}
+
-			{filter_by_reference_no}
+	return str(query)
-			order by {order_by}
-	"""
 
 
-def get_si_matching_query(exact_match):
+def get_si_matching_query(exact_match, currency):
 	# get matching sales invoice query
-	return f"""
+	si = frappe.qb.DocType("Sales Invoice")
-		SELECT
+	sip = frappe.qb.DocType("Sales Invoice Payment")
-			( CASE WHEN si.customer = %(party)s  THEN 1 ELSE 0 END
+
-			+ CASE WHEN sip.amount = %(amount)s THEN 1 ELSE 0 END
+	amount_equality = sip.amount == Parameter("%(amount)s")
-			+ 1 ) AS rank,
+	amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
-			'Sales Invoice' as doctype,
+	amount_condition = amount_equality if exact_match else sip.amount > 0.0
+
+	party_condition = si.customer == Parameter("%(party)s")
+	party_rank = frappe.qb.terms.Case().when(party_condition, 1).else_(0)
+
+	query = (
+		frappe.qb.from_(sip)
+		.join(si)
+		.on(sip.parent == si.name)
+		.select(
+			(party_rank + amount_rank + 1).as_("rank"),
+			ConstantColumn("Sales Invoice").as_("doctype"),
 			si.name,
-			sip.amount as paid_amount,
+			sip.amount.as_("paid_amount"),
-			'' as reference_no,
+			ConstantColumn("").as_("reference_no"),
-			'' as reference_date,
+			ConstantColumn("").as_("reference_date"),
-			si.customer as party,
+			si.customer.as_("party"),
-			'Customer' as party_type,
+			ConstantColumn("Customer").as_("party_type"),
 			si.posting_date,
-			si.currency
+			si.currency,
+		)
+		.where(si.docstatus == 1)
+		.where(sip.clearance_date.isnull())
+		.where(sip.account == Parameter("%(bank_account)s"))
+		.where(amount_condition)
+		.where(si.currency == currency)
+	)
 
-		FROM
+	return str(query)
-			`tabSales Invoice Payment` as sip
-		JOIN
-			`tabSales Invoice` as si
-		ON
-			sip.parent = si.name
-		WHERE
-			si.docstatus = 1
-			AND (sip.clearance_date is null or sip.clearance_date='0000-00-00')
-			AND sip.account = %(bank_account)s
-			AND sip.amount {'= %(amount)s' if exact_match else '> 0.0'}
-	"""
 
 
-def get_pi_matching_query(exact_match):
+def get_pi_matching_query(exact_match, currency):
 	# get matching purchase invoice query when they are also used as payment entries (is_paid)
-	return f"""
+	purchase_invoice = frappe.qb.DocType("Purchase Invoice")
-		SELECT
+
-			( CASE WHEN supplier = %(party)s THEN 1 ELSE 0 END
+	amount_equality = purchase_invoice.paid_amount == Parameter("%(amount)s")
-			+ CASE WHEN paid_amount = %(amount)s THEN 1 ELSE 0 END
+	amount_rank = frappe.qb.terms.Case().when(amount_equality, 1).else_(0)
-			+ 1 ) AS rank,
+	amount_condition = amount_equality if exact_match else purchase_invoice.paid_amount > 0.0
-			'Purchase Invoice' as doctype,
+
-			name,
+	party_condition = purchase_invoice.supplier == Parameter("%(party)s")
-			paid_amount,
+	party_rank = frappe.qb.terms.Case().when(party_condition, 1).else_(0)
-			'' as reference_no,
+
-			'' as reference_date,
+	query = (
-			supplier as party,
+		frappe.qb.from_(purchase_invoice)
-			'Supplier' as party_type,
+		.select(
-			posting_date,
+			(party_rank + amount_rank + 1).as_("rank"),
-			currency
+			ConstantColumn("Purchase Invoice").as_("doctype"),
-		FROM
+			purchase_invoice.name,
-			`tabPurchase Invoice`
+			purchase_invoice.paid_amount,
-		WHERE
+			ConstantColumn("").as_("reference_no"),
-			docstatus = 1
+			ConstantColumn("").as_("reference_date"),
-			AND is_paid = 1
+			purchase_invoice.supplier.as_("party"),
-			AND ifnull(clearance_date, '') = ""
+			ConstantColumn("Supplier").as_("party_type"),
-			AND cash_bank_account = %(bank_account)s
+			purchase_invoice.posting_date,
-			AND paid_amount {'= %(amount)s' if exact_match else '> 0.0'}
+			purchase_invoice.currency,
-	"""
+		)
+		.where(purchase_invoice.docstatus == 1)
+		.where(purchase_invoice.is_paid == 1)
+		.where(purchase_invoice.clearance_date.isnull())
+		.where(purchase_invoice.cash_bank_account == Parameter("%(bank_account)s"))
+		.where(amount_condition)
+		.where(purchase_invoice.currency == currency)
+	)
+
+	return str(query)

erpnext/accounts/doctype/bank_reconciliation_tool/test_bank_reconciliation_tool.py

@@ -1,9 +1,100 @@
 # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors
 # See license.txt
 
-# import frappe
 import unittest
 
+import frappe
+from frappe import qb
+from frappe.tests.utils import FrappeTestCase, change_settings
+from frappe.utils import add_days, flt, getdate, today
 
-class TestBankReconciliationTool(unittest.TestCase):
+from erpnext.accounts.doctype.bank_reconciliation_tool.bank_reconciliation_tool import (
-	pass
+	auto_reconcile_vouchers,
+	get_bank_transactions,
+)
+from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry
+from erpnext.accounts.test.accounts_mixin import AccountsTestMixin
+
+
+class TestBankReconciliationTool(AccountsTestMixin, FrappeTestCase):
+	def setUp(self):
+		self.create_company()
+		self.create_customer()
+		self.clear_old_entries()
+		bank_dt = qb.DocType("Bank")
+		q = qb.from_(bank_dt).delete().where(bank_dt.name == "HDFC").run()
+		self.create_bank_account()
+
+	def tearDown(self):
+		frappe.db.rollback()
+
+	def create_bank_account(self):
+		bank = frappe.get_doc(
+			{
+				"doctype": "Bank",
+				"bank_name": "HDFC",
+			}
+		).save()
+
+		self.bank_account = (
+			frappe.get_doc(
+				{
+					"doctype": "Bank Account",
+					"account_name": "HDFC _current_",
+					"bank": bank,
+					"is_company_account": True,
+					"account": self.bank,  # account from Chart of Accounts
+				}
+			)
+			.insert()
+			.name
+		)
+
+	def test_auto_reconcile(self):
+		# make payment
+		from_date = add_days(today(), -1)
+		to_date = today()
+		payment = create_payment_entry(
+			company=self.company,
+			posting_date=from_date,
+			payment_type="Receive",
+			party_type="Customer",
+			party=self.customer,
+			paid_from=self.debit_to,
+			paid_to=self.bank,
+			paid_amount=100,
+		).save()
+		payment.reference_no = "123"
+		payment = payment.save().submit()
+
+		# make bank transaction
+		bank_transaction = (
+			frappe.get_doc(
+				{
+					"doctype": "Bank Transaction",
+					"date": to_date,
+					"deposit": 100,
+					"bank_account": self.bank_account,
+					"reference_number": "123",
+				}
+			)
+			.save()
+			.submit()
+		)
+
+		# assert API output pre reconciliation
+		transactions = get_bank_transactions(self.bank_account, from_date, to_date)
+		self.assertEqual(len(transactions), 1)
+		self.assertEqual(transactions[0].name, bank_transaction.name)
+
+		# auto reconcile
+		auto_reconcile_vouchers(
+			bank_account=self.bank_account,
+			from_date=from_date,
+			to_date=to_date,
+			filter_by_reference_date=False,
+		)
+
+		# assert API output post reconciliation
+		transactions = get_bank_transactions(self.bank_account, from_date, to_date)
+		self.assertEqual(len(transactions), 0)

erpnext/accounts/doctype/bank_transaction/test_bank_transaction.py

@@ -47,7 +47,7 @@ class TestBankTransaction(FrappeTestCase):
 			from_date=bank_transaction.date,
 			to_date=utils.today(),
 		)
-		self.assertTrue(linked_payments[0][6] == "Conrad Electronic")
+		self.assertTrue(linked_payments[0]["party"] == "Conrad Electronic")
 
 	# This test validates a simple reconciliation leading to the clearance of the bank transaction and the payment
 	def test_reconcile(self):
@@ -93,7 +93,7 @@ class TestBankTransaction(FrappeTestCase):
 			from_date=bank_transaction.date,
 			to_date=utils.today(),
 		)
-		self.assertTrue(linked_payments[0][3])
+		self.assertTrue(linked_payments[0]["paid_amount"])
 
 	# Check error if already reconciled
 	def test_already_reconciled(self):
@@ -188,7 +188,7 @@ class TestBankTransaction(FrappeTestCase):
 		repayment_entry = create_loan_and_repayment()
 
 		linked_payments = get_linked_payments(bank_transaction.name, ["loan_repayment", "exact_match"])
-		self.assertEqual(linked_payments[0][2], repayment_entry.name)
+		self.assertEqual(linked_payments[0]["name"], repayment_entry.name)
 
 
 @if_lending_app_installed

erpnext/accounts/doctype/ledger_merge/ledger_merge.py

@@ -48,9 +48,6 @@ def start_merge(docname):
 				merge_account(
 					row.account,
 					ledger_merge.account,
-					ledger_merge.is_group,
-					ledger_merge.root_type,
-					ledger_merge.company,
 				)
 				row.db_set("merged", 1)
 				frappe.db.commit()

erpnext/accounts/test/accounts_mixin.py

@@ -158,6 +158,8 @@ class AccountsTestMixin:
 			"Journal Entry",
 			"Sales Order",
 			"Exchange Rate Revaluation",
+			"Bank Account",
+			"Bank Transaction",
 		]
 		for doctype in doctype_list:
 			qb.from_(qb.DocType(doctype)).delete().where(qb.DocType(doctype).company == self.company).run()

erpnext/hooks.py

@@ -555,8 +555,6 @@ get_matching_queries = (
 	"erpnext.accounts.doctype.bank_reconciliation_tool.bank_reconciliation_tool.get_matching_queries"
 )
 
-get_matching_vouchers_for_bank_reconciliation = "erpnext.accounts.doctype.bank_reconciliation_tool.bank_reconciliation_tool.get_matching_vouchers_for_bank_reconciliation"
-
 get_amounts_not_reflected_in_system_for_bank_reconciliation_statement = "erpnext.accounts.report.bank_reconciliation_statement.bank_reconciliation_statement.get_amounts_not_reflected_in_system_for_bank_reconciliation_statement"
 
 get_payment_entries_for_bank_clearance = (

erpnext/public/js/bank_reconciliation_tool/dialog_manager.js

@@ -134,12 +134,12 @@ erpnext.accounts.bank_reconciliation.DialogManager = class DialogManager {
 
 	format_row(row) {
 		return [
-			row[1], // Document Type
+			row["doctype"],
-			row[2], // Document Name
+			row["name"],
-			row[5] || row[8], // Reference Date
+			row["reference_date"] || row["posting_date"],
-			format_currency(row[3], row[9]), // Remaining
+			format_currency(row["paid_amount"], row["currency"]),
-			row[4], // Reference Number
+			row["reference_no"],
-			row[6], // Party
+			row["party"],
 		];
 	}