Merge pull request #43627 from ruthra-kumar/fix_approach2_advance_order_to_invoice

fix: reconciled advance from reported in reconciliation tool

erpnext/accounts/doctype/payment_entry/payment_entry.py

@@ -1263,6 +1263,10 @@ class PaymentEntry(AccountsController):
 		if not self.party_account:
 			return
 
+		advance_payment_doctypes = frappe.get_hooks("advance_payment_receivable_doctypes") + frappe.get_hooks(
+			"advance_payment_payable_doctypes"
+		)
+
 		if self.payment_type == "Receive":
 			against_account = self.paid_to
 		else:
@@ -1308,11 +1312,30 @@ class PaymentEntry(AccountsController):
 				{
 					dr_or_cr: allocated_amount_in_company_currency,
 					dr_or_cr + "_in_account_currency": d.allocated_amount,
-					"against_voucher_type": d.reference_doctype,
-					"against_voucher": d.reference_name,
 					"cost_center": cost_center,
 				}
 			)
+
+			if self.book_advance_payments_in_separate_party_account:
+				if d.reference_doctype in advance_payment_doctypes:
+					# Upon reconciliation, whole ledger will be reposted. So, reference to SO/PO is fine
+					gle.update(
+						{
+							"against_voucher_type": d.reference_doctype,
+							"against_voucher": d.reference_name,
+						}
+					)
+				else:
+					# Do not reference Invoices while Advance is in separate party account
+					gle.update({"against_voucher_type": self.doctype, "against_voucher": self.name})
+			else:
+				gle.update(
+					{
+						"against_voucher_type": d.reference_doctype,
+						"against_voucher": d.reference_name,
+					}
+				)
+
 			gl_entries.append(gle)
 
 		if self.unallocated_amount:

erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py

@@ -7,7 +7,9 @@ from frappe.utils import today
 
 from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry
 from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
+from erpnext.accounts.party import get_party_account
 from erpnext.accounts.test.accounts_mixin import AccountsTestMixin
+from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice
 from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
 
 
@@ -363,3 +365,100 @@ class TestUnreconcilePayment(AccountsTestMixin, IntegrationTestCase):
 		self.assertEqual(so.advance_paid, 0)
 		self.assertEqual(len(pe.references), 0)
 		self.assertEqual(pe.unallocated_amount, 100)
+
+	def test_06_unreconcile_advance_from_payment_entry(self):
+		self.enable_advance_as_liability()
+		so1 = self.create_sales_order()
+		so2 = self.create_sales_order()
+
+		pe = self.create_payment_entry()
+		# Allocation payment against Sales Order
+		pe.paid_amount = 260
+		pe.append(
+			"references",
+			{"reference_doctype": so1.doctype, "reference_name": so1.name, "allocated_amount": 150},
+		)
+		pe.append(
+			"references",
+			{"reference_doctype": so2.doctype, "reference_name": so2.name, "allocated_amount": 110},
+		)
+		pe.save().submit()
+
+		# Assert 'Advance Paid'
+		so1.reload()
+		self.assertEqual(so1.advance_paid, 150)
+		so2.reload()
+		self.assertEqual(so2.advance_paid, 110)
+
+		unreconcile = frappe.get_doc(
+			{
+				"doctype": "Unreconcile Payment",
+				"company": self.company,
+				"voucher_type": pe.doctype,
+				"voucher_no": pe.name,
+			}
+		)
+		unreconcile.add_references()
+		self.assertEqual(len(unreconcile.allocations), 2)
+		allocations = [(x.reference_name, x.allocated_amount) for x in unreconcile.allocations]
+		self.assertListEqual(allocations, [(so1.name, 150), (so2.name, 110)])
+		# unreconcile so2
+		unreconcile.remove(unreconcile.allocations[0])
+		unreconcile.save().submit()
+
+		# Assert 'Advance Paid'
+		so1.reload()
+		so2.reload()
+		pe.reload()
+		self.assertEqual(so1.advance_paid, 150)
+		self.assertEqual(so2.advance_paid, 0)
+		self.assertEqual(len(pe.references), 1)
+		self.assertEqual(pe.unallocated_amount, 110)
+
+		self.disable_advance_as_liability()
+
+	def test_07_adv_from_so_to_invoice(self):
+		self.enable_advance_as_liability()
+		so = self.create_sales_order()
+		pe = self.create_payment_entry()
+		pe.paid_amount = 1000
+		pe.append(
+			"references",
+			{"reference_doctype": so.doctype, "reference_name": so.name, "allocated_amount": 1000},
+		)
+		pe.save().submit()
+
+		# Assert 'Advance Paid'
+		so.reload()
+		self.assertEqual(so.advance_paid, 1000)
+
+		si = make_sales_invoice(so.name)
+		si.insert().submit()
+
+		pr = frappe.get_doc(
+			{
+				"doctype": "Payment Reconciliation",
+				"company": self.company,
+				"party_type": "Customer",
+				"party": so.customer,
+			}
+		)
+		accounts = get_party_account("Customer", so.customer, so.company, True)
+		pr.receivable_payable_account = accounts[0]
+		pr.default_advance_account = accounts[1]
+		pr.get_unreconciled_entries()
+		self.assertEqual(len(pr.get("invoices")), 1)
+		self.assertEqual(len(pr.get("payments")), 1)
+		invoices = [x.as_dict() for x in pr.get("invoices")]
+		payments = [x.as_dict() for x in pr.get("payments")]
+		pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments}))
+		pr.reconcile()
+
+		self.assertEqual(len(pr.get("invoices")), 0)
+		self.assertEqual(len(pr.get("payments")), 0)
+
+		# Assert 'Advance Paid'
+		so.reload()
+		self.assertEqual(so.advance_paid, 0)
+
+		self.disable_advance_as_liability()

erpnext/accounts/test/accounts_mixin.py

@@ -93,6 +93,22 @@ class AccountsTestMixin:
 					"parent_account": "Bank Accounts - " + abbr,
 				}
 			),
+			frappe._dict(
+				{
+					"attribute_name": "advance_received",
+					"account_name": "Advance Received",
+					"parent_account": "Current Liabilities - " + abbr,
+					"account_type": "Receivable",
+				}
+			),
+			frappe._dict(
+				{
+					"attribute_name": "advance_paid",
+					"account_name": "Advance Paid",
+					"parent_account": "Current Assets - " + abbr,
+					"account_type": "Payable",
+				}
+			),
 		]
 		for acc in other_accounts:
 			acc_name = acc.account_name + " - " + abbr
@@ -107,11 +123,25 @@ class AccountsTestMixin:
 						"company": self.company,
 					}
 				)
+				new_acc.account_type = acc.get("account_type", None)
 				new_acc.save()
 				setattr(self, acc.attribute_name, new_acc.name)
 
 		self.identify_default_warehouses()
 
+	def enable_advance_as_liability(self):
+		company = frappe.get_doc("Company", self.company)
+		company.book_advance_payments_in_separate_party_account = True
+		company.default_advance_received_account = self.advance_received
+		company.default_advance_paid_account = self.advance_paid
+		company.save()
+
+	def disable_advance_as_liability(self):
+		company = frappe.get_doc("Company", self.company)
+		company.book_advance_payments_in_separate_party_account = False
+		company.default_advance_paid_account = company.default_advance_received_account = None
+		company.save()
+
 	def identify_default_warehouses(self):
 		for w in frappe.db.get_all(
 			"Warehouse", filters={"company": self.company}, fields=["name", "warehouse_name"]

erpnext/accounts/utils.py

@@ -486,10 +486,14 @@ def reconcile_against_document(
 		doc = frappe.get_doc(voucher_type, voucher_no)
 		frappe.flags.ignore_party_validation = True
 
-		# For payments with `Advance` in separate account feature enabled, only new ledger entries are posted for each reference.
+		# When Advance is allocated from an Order to an Invoice
-		# No need to cancel/delete payment ledger entries
+		# whole ledger must be reposted
+		repost_whole_ledger = any([x.voucher_detail_no for x in entries])
 		if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account:
-			doc.make_advance_gl_entries(cancel=1)
+			if repost_whole_ledger:
+				doc.make_gl_entries(cancel=1)
+			else:
+				doc.make_advance_gl_entries(cancel=1)
 		else:
 			_delete_pl_entries(voucher_type, voucher_no)
 
@@ -523,9 +527,14 @@ def reconcile_against_document(
 		doc = frappe.get_doc(entry.voucher_type, entry.voucher_no)
 
 		if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account:
-			# both ledgers must be posted to for `Advance` in separate account feature
+			# When Advance is allocated from an Order to an Invoice
-			# TODO: find a more efficient way post only for the new linked vouchers
+			# whole ledger must be reposted
-			doc.make_advance_gl_entries()
+			if repost_whole_ledger:
+				doc.make_gl_entries()
+			else:
+				# both ledgers must be posted to for `Advance` in separate account feature
+				# TODO: find a more efficient way post only for the new linked vouchers
+				doc.make_advance_gl_entries()
 		else:
 			gl_map = doc.build_gl_map()
 			# Make sure there is no overallocation