From a75e095d007f4ab198d7fc44d884b42b6f05942a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 4 Apr 2024 16:13:07 +0530 Subject: [PATCH 1/7] fix(test): Advance Received should be under liability account --- erpnext/accounts/doctype/payment_entry/test_payment_entry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 0b3037a5524..a719cb0d42a 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1543,7 +1543,7 @@ class TestPaymentEntry(FrappeTestCase): company = "_Test Company" customer = create_customer(frappe.generate_hash(length=10), "INR") advance_account = create_account( - parent_account="Current Assets - _TC", + parent_account="Current Liabilities - _TC", account_name="Advances Received", company=company, account_type="Receivable", From 9fd2dddfde609bd3d8adaac4aade8a0bed88deb2 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 4 Apr 2024 21:08:43 +0530 Subject: [PATCH 2/7] fix: invalid ledger entries on payment against reverse payments On Advance payments booked on separate party account, reconciliation against a reverse payment posted invalid ledger entries. --- .../doctype/payment_entry/payment_entry.py | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 1ff1cb7ec48..b990260e391 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1316,6 +1316,26 @@ class PaymentEntry(AccountsController): ): self.add_advance_gl_for_reference(gl_entries, ref) + def get_dr_and_account_for_advances(self, reference): + if reference.reference_doctype == "Sales Invoice": + return "credit", reference.account + + if reference.reference_doctype == "Payment Entry": + reverse_payment_details = frappe.db.get_all( + "Payment Entry", + filters={"name": reference.reference_name}, + fields=["payment_type", "party_type"], + )[0] + if ( + reverse_payment_details.payment_type == "Pay" + and reverse_payment_details.party_type == "Customer" + ): + return "credit", self.party_account + else: + return "debit", self.party_account + + return "debit", reference.account + def add_advance_gl_for_reference(self, gl_entries, invoice): args_dict = { "party_type": self.party_type, @@ -1335,8 +1355,8 @@ class PaymentEntry(AccountsController): if getdate(posting_date) < getdate(self.posting_date): posting_date = self.posting_date - dr_or_cr = "credit" if invoice.reference_doctype in ["Sales Invoice", "Payment Entry"] else "debit" - args_dict["account"] = invoice.account + dr_or_cr, account = self.get_dr_and_account_for_advances(invoice) + args_dict["account"] = account args_dict[dr_or_cr] = invoice.allocated_amount args_dict[dr_or_cr + "_in_account_currency"] = invoice.allocated_amount args_dict.update( From bdd36b00018224700a979fb2eebfefc1c86977f1 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 09:56:15 +0530 Subject: [PATCH 3/7] refactor: supplementary field to better handle reverse payments --- .../doctype/payment_entry/payment_entry.py | 28 +++++++++++++------ .../payment_entry_reference.json | 14 +++++++++- .../payment_entry_reference.py | 2 ++ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index b990260e391..faba0be9c5c 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1321,15 +1321,7 @@ class PaymentEntry(AccountsController): return "credit", reference.account if reference.reference_doctype == "Payment Entry": - reverse_payment_details = frappe.db.get_all( - "Payment Entry", - filters={"name": reference.reference_name}, - fields=["payment_type", "party_type"], - )[0] - if ( - reverse_payment_details.payment_type == "Pay" - and reverse_payment_details.party_type == "Customer" - ): + if reference.account_type == "Receivable" and reference.payment_type == "Pay": return "credit", self.party_account else: return "debit", self.party_account @@ -2214,6 +2206,10 @@ def get_reference_details(reference_doctype, reference_name, party_account_curre ref_doc = frappe.get_doc(reference_doctype, reference_name) company_currency = ref_doc.get("company_currency") or erpnext.get_company_currency(ref_doc.company) + # Only applies for Reverse Payment Entries + account_type = None + payment_type = None + if reference_doctype == "Dunning": total_amount = outstanding_amount = ref_doc.get("dunning_amount") exchange_rate = 1 @@ -2226,6 +2222,18 @@ def get_reference_details(reference_doctype, reference_name, party_account_curre exchange_rate = 1 outstanding_amount = get_outstanding_on_journal_entry(reference_name) + elif reference_doctype == "Payment Entry": + if reverse_payment_details := frappe.db.get_all( + "Payment Entry", + filters={"name": reference_name}, + fields=["payment_type", "party_type"], + )[0]: + payment_type = reverse_payment_details.payment_type + account_type = frappe.db.get_value( + "Party Type", reverse_payment_details.party_type, "account_type" + ) + exchange_rate = 1 + elif reference_doctype != "Journal Entry": if not total_amount: if party_account_currency == company_currency: @@ -2270,6 +2278,8 @@ def get_reference_details(reference_doctype, reference_name, party_account_curre "outstanding_amount": flt(outstanding_amount), "exchange_rate": flt(exchange_rate), "bill_no": ref_doc.get("bill_no"), + "account_type": account_type, + "payment_type": payment_type, } ) if account: diff --git a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json index e518009b061..352ece24f06 100644 --- a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json +++ b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json @@ -10,6 +10,8 @@ "due_date", "bill_no", "payment_term", + "account_type", + "payment_type", "column_break_4", "total_amount", "outstanding_amount", @@ -108,12 +110,22 @@ "fieldtype": "Link", "label": "Account", "options": "Account" + }, + { + "fieldname": "account_type", + "fieldtype": "Data", + "label": "Account Type" + }, + { + "fieldname": "payment_type", + "fieldtype": "Data", + "label": "Payment Type" } ], "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2024-03-27 13:10:09.578312", + "modified": "2024-04-05 09:44:08.310593", "modified_by": "Administrator", "module": "Accounts", "name": "Payment Entry Reference", diff --git a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py index 13707e53efc..4a027b4ee32 100644 --- a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py +++ b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py @@ -15,6 +15,7 @@ class PaymentEntryReference(Document): from frappe.types import DF account: DF.Link | None + account_type: DF.Data | None allocated_amount: DF.Float bill_no: DF.Data | None due_date: DF.Date | None @@ -25,6 +26,7 @@ class PaymentEntryReference(Document): parentfield: DF.Data parenttype: DF.Data payment_term: DF.Link | None + payment_type: DF.Data | None reference_doctype: DF.Link reference_name: DF.DynamicLink total_amount: DF.Float From 74bc38e0af77126213035e7ecd47564a10af640d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 11:08:12 +0530 Subject: [PATCH 4/7] fix(test): for reverse payments, only advance acc should have effect This applies for Payments against Reverse-Payment reconcilition --- erpnext/accounts/doctype/payment_entry/test_payment_entry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index a719cb0d42a..57d026f09a4 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1599,9 +1599,9 @@ class TestPaymentEntry(FrappeTestCase): # assert General and Payment Ledger entries post partial reconciliation self.expected_gle = [ - {"account": "Debtors - _TC", "debit": 0.0, "credit": 400.0}, {"account": advance_account, "debit": 400.0, "credit": 0.0}, {"account": advance_account, "debit": 0.0, "credit": 1000.0}, + {"account": advance_account, "debit": 0.0, "credit": 400.0}, {"account": "_Test Cash - _TC", "debit": 1000.0, "credit": 0.0}, ] self.expected_ple = [ @@ -1612,7 +1612,7 @@ class TestPaymentEntry(FrappeTestCase): "amount": -1000.0, }, { - "account": "Debtors - _TC", + "account": advance_account, "voucher_no": pe.name, "against_voucher_no": reverse_pe.name, "amount": -400.0, From 248cc6105b6c3a88c89fceb6ff0558408d64e2f2 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 14:06:11 +0530 Subject: [PATCH 5/7] test: reverse payment against payment for supplier --- .../test_payment_reconciliation.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py index 8b55c117f0d..5c8050b4c9c 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -1335,6 +1335,59 @@ class TestPaymentReconciliation(FrappeTestCase): # Should not raise frappe.exceptions.ValidationError: Payment Entry has been modified after you pulled it. Please pull it again. pr.reconcile() + def test_reverse_payment_against_payment_for_supplier(self): + """ + Reconcile a payment against a reverse payment, for a supplier. + """ + self.supplier = "_Test Supplier" + amount = 4000 + + pe = self.create_payment_entry(amount=amount) + pe.party_type = "Supplier" + pe.party = self.supplier + pe.payment_type = "Pay" + pe.paid_from = self.cash + pe.paid_to = self.creditors + pe.save().submit() + + reverse_pe = self.create_payment_entry(amount=amount) + reverse_pe.party_type = "Supplier" + reverse_pe.party = self.supplier + reverse_pe.payment_type = "Receive" + reverse_pe.paid_from = self.creditors + reverse_pe.paid_to = self.cash + reverse_pe.save().submit() + + pr = self.create_payment_reconciliation(party_is_customer=False) + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + self.assertEqual(pr.invoices[0].invoice_number, reverse_pe.name) + self.assertEqual(pr.payments[0].reference_name, pe.name) + + 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() + + pe.reload() + self.assertEqual(len(pe.references), 1) + self.assertEqual(pe.references[0].exchange_rate, 1) + # There should not be any Exc Gain/Loss + self.assertEqual(pe.references[0].exchange_gain_loss, 0) + self.assertEqual(pe.references[0].reference_name, reverse_pe.name) + + journals = frappe.db.get_all( + "Journal Entry", + filters={ + "voucher_type": "Exchange Gain Or Loss", + "reference_type": "Payment Entry", + "reference_name": ("in", [pe.name, reverse_pe.name]), + }, + ) + # There should be no Exchange Gain/Loss created + self.assertEqual(journals, []) + def make_customer(customer_name, currency=None): if not frappe.db.exists("Customer", customer_name): From 141f462368b2db4fd02f5ee571b60961bab1d2ae Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 14:34:36 +0530 Subject: [PATCH 6/7] test: advance payment against reverse payment reconciliation --- .../test_payment_reconciliation.py | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py index 5c8050b4c9c..0a3a0678084 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -101,6 +101,14 @@ class TestPaymentReconciliation(FrappeTestCase): "account_currency": "USD", "account_type": "Payable", }, + # 'Payable' account for capturing advance paid, under 'Assets' group + { + "attribute": "advance_payable_account", + "account_name": "Advance Paid", + "parent_account": "Current Assets - _PR", + "account_currency": "INR", + "account_type": "Payable", + }, ] for x in accounts: @@ -1388,6 +1396,135 @@ class TestPaymentReconciliation(FrappeTestCase): # There should be no Exchange Gain/Loss created self.assertEqual(journals, []) + def test_advance_reverse_payment_against_payment_for_supplier(self): + """ + Reconcile an Advance payment against reverse payment, for a supplier. + """ + frappe.db.set_value( + "Company", + self.company, + { + "book_advance_payments_in_separate_party_account": 1, + "default_advance_paid_account": self.advance_payable_account, + }, + ) + + self.supplier = "_Test Supplier" + amount = 4000 + + pe = self.create_payment_entry(amount=amount) + pe.party_type = "Supplier" + pe.party = self.supplier + pe.payment_type = "Pay" + pe.paid_from = self.cash + pe.paid_to = self.advance_payable_account + pe.save().submit() + + reverse_pe = self.create_payment_entry(amount=amount) + reverse_pe.party_type = "Supplier" + reverse_pe.party = self.supplier + reverse_pe.payment_type = "Receive" + reverse_pe.paid_from = self.advance_payable_account + reverse_pe.paid_to = self.cash + reverse_pe.save().submit() + + pr = self.create_payment_reconciliation(party_is_customer=False) + pr.default_advance_account = self.advance_payable_account + pr.get_unreconciled_entries() + self.assertEqual(len(pr.invoices), 1) + self.assertEqual(len(pr.payments), 1) + self.assertEqual(pr.invoices[0].invoice_number, reverse_pe.name) + self.assertEqual(pr.payments[0].reference_name, pe.name) + + 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() + + pe.reload() + self.assertEqual(len(pe.references), 1) + self.assertEqual(pe.references[0].exchange_rate, 1) + # There should not be any Exc Gain/Loss + self.assertEqual(pe.references[0].exchange_gain_loss, 0) + self.assertEqual(pe.references[0].reference_name, reverse_pe.name) + + journals = frappe.db.get_all( + "Journal Entry", + filters={ + "voucher_type": "Exchange Gain Or Loss", + "reference_type": "Payment Entry", + "reference_name": ("in", [pe.name, reverse_pe.name]), + }, + ) + # There should be no Exchange Gain/Loss created + self.assertEqual(journals, []) + + # Assert Ledger Entries + gl_entries = frappe.db.get_all( + "GL Entry", + filters={"voucher_no": pe.name}, + fields=["account", "voucher_no", "against_voucher", "debit", "credit"], + order_by="account, against_voucher, debit", + ) + expected_gle = [ + { + "account": self.advance_payable_account, + "voucher_no": pe.name, + "against_voucher": pe.name, + "debit": 0.0, + "credit": amount, + }, + { + "account": self.advance_payable_account, + "voucher_no": pe.name, + "against_voucher": pe.name, + "debit": amount, + "credit": 0.0, + }, + { + "account": self.advance_payable_account, + "voucher_no": pe.name, + "against_voucher": reverse_pe.name, + "debit": amount, + "credit": 0.0, + }, + { + "account": "Cash - _PR", + "voucher_no": pe.name, + "against_voucher": None, + "debit": 0.0, + "credit": amount, + }, + ] + self.assertEqual(gl_entries, expected_gle) + pl_entries = frappe.db.get_all( + "Payment Ledger Entry", + filters={"voucher_no": pe.name}, + fields=["account", "voucher_no", "against_voucher_no", "amount"], + order_by="account, against_voucher_no, amount", + ) + expected_ple = [ + { + "account": self.advance_payable_account, + "voucher_no": pe.name, + "against_voucher_no": pe.name, + "amount": -amount, + }, + { + "account": self.advance_payable_account, + "voucher_no": pe.name, + "against_voucher_no": pe.name, + "amount": amount, + }, + { + "account": self.advance_payable_account, + "voucher_no": pe.name, + "against_voucher_no": reverse_pe.name, + "amount": -amount, + }, + ] + self.assertEqual(pl_entries, expected_ple) + def make_customer(customer_name, currency=None): if not frappe.db.exists("Customer", customer_name): From 64b7f624a5433578eba90a2f8e3854ff9c79af99 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 14:59:17 +0530 Subject: [PATCH 7/7] refactor(test): include new fields in assertion --- erpnext/accounts/doctype/payment_entry/test_payment_entry.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py index 57d026f09a4..03bd21f3765 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1077,6 +1077,8 @@ class TestPaymentEntry(FrappeTestCase): ref_details = get_reference_details(so.doctype, so.name, pe.paid_from_account_currency) expected_response = { "account": get_party_account("Customer", so.customer, so.company), + "account_type": None, # only applies for Reverse Payment Entry + "payment_type": None, # only applies for Reverse Payment Entry "total_amount": 5000.0, "outstanding_amount": 5000.0, "exchange_rate": 1.0,