From b409f7462070f301469e958be1764b90352298b6 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 14:52:35 +0530 Subject: [PATCH 1/7] fix: don't update reference to SI / PI on advances --- .../doctype/payment_entry/payment_entry.py | 16 ++++++++++++++-- erpnext/accounts/utils.py | 11 +++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 59cac538cdb..2e23aa283f2 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1308,11 +1308,23 @@ 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 and d.reference_doctype in [ + "Sales Order", + "Purchase Order", + ]: + gle.update( + { + "against_voucher_type": d.reference_doctype, + "against_voucher": d.reference_name, + } + ) + else: + gle.update({"against_voucher_type": self.doctype, "against_voucher": self.name}) + gl_entries.append(gle) if self.unallocated_amount: diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index d4dfb19eb25..671e46c559c 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -488,8 +488,12 @@ def reconcile_against_document( # For payments with `Advance` in separate account feature enabled, only new ledger entries are posted for each reference. # No need to cancel/delete payment ledger entries + 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) @@ -525,7 +529,10 @@ def reconcile_against_document( 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 # TODO: find a more efficient way post only for the new linked vouchers - doc.make_advance_gl_entries() + if repost_whole_ledger: + doc.make_gl_entries() + else: + doc.make_advance_gl_entries() else: gl_map = doc.build_gl_map() # Make sure there is no overallocation From a112581acd77b41f9140b0cee082db3b00cd1e22 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:29:48 +0530 Subject: [PATCH 2/7] refactor: reference update logic in advance --- .../doctype/payment_entry/payment_entry.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 2e23aa283f2..12c55576d00 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1312,18 +1312,26 @@ class PaymentEntry(AccountsController): } ) - if self.book_advance_payments_in_separate_party_account and d.reference_doctype in [ - "Sales Order", - "Purchase Order", - ]: + if self.book_advance_payments_in_separate_party_account: + if d.reference_doctype in [ + "Sales Order", + "Purchase Order", + ]: + gle.update( + { + "against_voucher_type": d.reference_doctype, + "against_voucher": d.reference_name, + } + ) + else: + 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, } ) - else: - gle.update({"against_voucher_type": self.doctype, "against_voucher": self.name}) gl_entries.append(gle) From e7bb960bb557a0cc05570b248962ba519a3278db Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:54:21 +0530 Subject: [PATCH 3/7] refactor: use hooks to identify advance doctypes --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 12c55576d00..e5678a62105 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/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: @@ -1313,10 +1317,7 @@ class PaymentEntry(AccountsController): ) if self.book_advance_payments_in_separate_party_account: - if d.reference_doctype in [ - "Sales Order", - "Purchase Order", - ]: + if d.reference_doctype in advance_payment_doctypes: gle.update( { "against_voucher_type": d.reference_doctype, From a21a406d0409c28c0880b91577343f24db6e510a Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:55:51 +0530 Subject: [PATCH 4/7] refactor(test): utility methods for enabling advance in separate acc --- erpnext/accounts/test/accounts_mixin.py | 30 +++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/erpnext/accounts/test/accounts_mixin.py b/erpnext/accounts/test/accounts_mixin.py index a3685a2ddb9..f2550f738e2 100644 --- a/erpnext/accounts/test/accounts_mixin.py +++ b/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"] From 8a6978e55000efda874931dcf4a11403bf795e84 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:56:53 +0530 Subject: [PATCH 5/7] test: unreconciliation of individual SO from Advance Payment --- .../test_unreconcile_payment.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py b/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py index 73c589708bb..4abbb4a820e 100644 --- a/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py +++ b/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py @@ -363,3 +363,54 @@ 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() From f1ec61c19ec8275804958e4eaf131906a2cffc90 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 18:12:03 +0530 Subject: [PATCH 6/7] test: reconciled Invoice should not showup in tool Scenario should be tested on 'Advance in separate party account' --- .../test_unreconcile_payment.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py b/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py index 4abbb4a820e..e0e06aa6c01 100644 --- a/erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py +++ b/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 @@ -414,3 +416,49 @@ class TestUnreconcilePayment(AccountsTestMixin, IntegrationTestCase): 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() From e7505e92c9b4200d52214d2dc97f7052b2333095 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 13 Oct 2024 08:08:54 +0530 Subject: [PATCH 7/7] chore: better comments for context --- .../accounts/doctype/payment_entry/payment_entry.py | 2 ++ erpnext/accounts/utils.py | 10 ++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index e5678a62105..58c65aeaa52 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1318,6 +1318,7 @@ class PaymentEntry(AccountsController): 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, @@ -1325,6 +1326,7 @@ class PaymentEntry(AccountsController): } ) 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( diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 671e46c559c..c7de27c2700 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -486,8 +486,8 @@ 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. - # No need to cancel/delete payment ledger entries + # When Advance is allocated from an Order to an Invoice + # 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: if repost_whole_ledger: @@ -527,11 +527,13 @@ 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 - # TODO: find a more efficient way post only for the new linked vouchers + # When Advance is allocated from an Order to an Invoice + # whole ledger must be reposted 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()