From 4f13f4779a470a1e22aa5dfc4873bec68d3ee8b7 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 4 Apr 2024 16:13:07 +0530 Subject: [PATCH 1/8] fix(test): Advance Received should be under liability account (cherry picked from commit a75e095d007f4ab198d7fc44d884b42b6f05942a) --- 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 2b226e1b241..d271e76a7f2 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1559,7 +1559,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 f3eb559a71459245e107eb88ded77d2cf51d94a3 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 4 Apr 2024 21:08:43 +0530 Subject: [PATCH 2/8] 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. (cherry picked from commit 9fd2dddfde609bd3d8adaac4aade8a0bed88deb2) # Conflicts: # erpnext/accounts/doctype/payment_entry/payment_entry.py --- .../doctype/payment_entry/payment_entry.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index f070b5ee270..94874a4e2bc 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1205,6 +1205,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, @@ -1224,10 +1244,15 @@ class PaymentEntry(AccountsController): if getdate(posting_date) < getdate(self.posting_date): posting_date = self.posting_date +<<<<<<< HEAD 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 +>>>>>>> 9fd2dddfde (fix: invalid ledger entries on payment against reverse payments) args_dict[dr_or_cr] = invoice.allocated_amount args_dict[dr_or_cr + "_in_account_currency"] = invoice.allocated_amount args_dict.update( From d9826e3dfc24ce54e88de952f00436ff0f486698 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 09:56:15 +0530 Subject: [PATCH 3/8] refactor: supplementary field to better handle reverse payments (cherry picked from commit bdd36b00018224700a979fb2eebfefc1c86977f1) # Conflicts: # erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json --- .../doctype/payment_entry/payment_entry.py | 28 +++++++++++++------ .../payment_entry_reference.json | 16 +++++++++++ .../payment_entry_reference.py | 2 ++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 94874a4e2bc..cb8ed88ec59 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1210,15 +1210,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 @@ -2114,6 +2106,10 @@ def get_reference_details(reference_doctype, reference_name, party_account_curre 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 @@ -2128,6 +2124,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: @@ -2172,6 +2180,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 12aa0b520ec..dfbe273809c 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,26 @@ "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": [], +<<<<<<< HEAD "modified": "2023-06-08 07:40:38.487874", +======= + "modified": "2024-04-05 09:44:08.310593", +>>>>>>> bdd36b0001 (refactor: supplementary field to better handle reverse payments) "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 3668de7cb770ad54b9f102ca9af36539f4e729c8 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 11:08:12 +0530 Subject: [PATCH 4/8] fix(test): for reverse payments, only advance acc should have effect This applies for Payments against Reverse-Payment reconcilition (cherry picked from commit 74bc38e0af77126213035e7ecd47564a10af640d) --- 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 d271e76a7f2..6934a635a46 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1615,9 +1615,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 = [ @@ -1628,7 +1628,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 4ad423b1b94fc52e48e721f77db725da683643e5 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 14:06:11 +0530 Subject: [PATCH 5/8] test: reverse payment against payment for supplier (cherry picked from commit 248cc6105b6c3a88c89fceb6ff0558408d64e2f2) --- .../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 1d20a5b954d..47cda95880d 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -1332,6 +1332,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 7c0d46409f896f4714a8947496b18f7675ba8349 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 14:34:36 +0530 Subject: [PATCH 6/8] test: advance payment against reverse payment reconciliation (cherry picked from commit 141f462368b2db4fd02f5ee571b60961bab1d2ae) --- .../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 47cda95880d..adf57fa50fb 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py @@ -102,6 +102,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: @@ -1385,6 +1393,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 67df5ae0ec0c414309f9fcd8d484a914a8ca229f Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Fri, 5 Apr 2024 14:59:17 +0530 Subject: [PATCH 7/8] refactor(test): include new fields in assertion (cherry picked from commit 64b7f624a5433578eba90a2f8e3854ff9c79af99) --- 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 6934a635a46..1f9404d910b 100644 --- a/erpnext/accounts/doctype/payment_entry/test_payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/test_payment_entry.py @@ -1087,6 +1087,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, From 2373dd9b955a51ad578f767ba89a9d71e3618eb6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Mon, 8 Apr 2024 08:51:58 +0530 Subject: [PATCH 8/8] chore: resolve conflicts --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 7 ------- .../payment_entry_reference/payment_entry_reference.json | 4 ---- 2 files changed, 11 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index cb8ed88ec59..4b10c8ec769 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1236,15 +1236,8 @@ class PaymentEntry(AccountsController): if getdate(posting_date) < getdate(self.posting_date): posting_date = self.posting_date -<<<<<<< HEAD - 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 ->>>>>>> 9fd2dddfde (fix: invalid ledger entries on payment against reverse payments) args_dict[dr_or_cr] = invoice.allocated_amount args_dict[dr_or_cr + "_in_account_currency"] = invoice.allocated_amount args_dict.update( 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 dfbe273809c..23ed8252333 100644 --- a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json +++ b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json @@ -125,11 +125,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], -<<<<<<< HEAD - "modified": "2023-06-08 07:40:38.487874", -======= "modified": "2024-04-05 09:44:08.310593", ->>>>>>> bdd36b0001 (refactor: supplementary field to better handle reverse payments) "modified_by": "Administrator", "module": "Accounts", "name": "Payment Entry Reference",