From 8bf8bcf7390296f79432d7a257fa85e095cc6277 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 14:52:35 +0530 Subject: [PATCH 1/9] fix: don't update reference to SI / PI on advances (cherry picked from commit b409f7462070f301469e958be1764b90352298b6) --- .../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 9424d722cf5..34179b0b104 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1211,11 +1211,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 8e47ddc3652..cf25b201129 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -476,8 +476,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) @@ -513,7 +517,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 6267ab994c79b3ab8828069c760e81538ff73887 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:29:48 +0530 Subject: [PATCH 2/9] refactor: reference update logic in advance (cherry picked from commit a112581acd77b41f9140b0cee082db3b00cd1e22) --- .../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 34179b0b104..f031bda045e 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1215,18 +1215,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 8c115e146b65cebbbf09b3c34090ef37a93ce530 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:54:21 +0530 Subject: [PATCH 3/9] refactor: use hooks to identify advance doctypes (cherry picked from commit e7bb960bb557a0cc05570b248962ba519a3278db) --- 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 f031bda045e..eddcdf56fb2 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1166,6 +1166,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: @@ -1216,10 +1220,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 e0477cf59f7612276e3d9549522c8fafa53557a1 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:55:51 +0530 Subject: [PATCH 4/9] refactor(test): utility methods for enabling advance in separate acc (cherry picked from commit a21a406d0409c28c0880b91577343f24db6e510a) # Conflicts: # erpnext/accounts/test/accounts_mixin.py --- erpnext/accounts/test/accounts_mixin.py | 41 +++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/erpnext/accounts/test/accounts_mixin.py b/erpnext/accounts/test/accounts_mixin.py index d503f7bc4af..b67e190f431 100644 --- a/erpnext/accounts/test/accounts_mixin.py +++ b/erpnext/accounts/test/accounts_mixin.py @@ -87,6 +87,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 @@ -101,9 +117,34 @@ 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) +<<<<<<< HEAD +======= + 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"] + ): + setattr(self, "warehouse_" + w.warehouse_name.lower().strip().replace(" ", "_"), w.name) + +>>>>>>> a21a406d04 (refactor(test): utility methods for enabling advance in separate acc) def create_usd_receivable_account(self): account_name = "Debtors USD" if not frappe.db.get_value( From e56dd8268b6aa05fc9c6d52924baa0be1747adab Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 15:56:53 +0530 Subject: [PATCH 5/9] test: unreconciliation of individual SO from Advance Payment (cherry picked from commit 8a6978e55000efda874931dcf4a11403bf795e84) --- .../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 43dfbfaef60..eb5530706b6 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, FrappeTestCase): 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 4752ed2483d7697129c2af245222977bdcb1b750 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sat, 12 Oct 2024 18:12:03 +0530 Subject: [PATCH 6/9] test: reconciled Invoice should not showup in tool Scenario should be tested on 'Advance in separate party account' (cherry picked from commit f1ec61c19ec8275804958e4eaf131906a2cffc90) --- .../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 eb5530706b6..13e5294aa78 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, FrappeTestCase): 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 4c78a682ad38121bd3b9135db9acca68f30028a9 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 13 Oct 2024 08:08:54 +0530 Subject: [PATCH 7/9] chore: better comments for context (cherry picked from commit e7505e92c9b4200d52214d2dc97f7052b2333095) --- .../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 eddcdf56fb2..7e3aa25f358 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1221,6 +1221,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, @@ -1228,6 +1229,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 cf25b201129..1d75fafc1a9 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -474,8 +474,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: @@ -515,11 +515,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() From cf1eabe049c4ee3b7c8f6e978fd7788b4bc7e566 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 13 Oct 2024 10:35:14 +0530 Subject: [PATCH 8/9] chore: resolve conflict --- erpnext/accounts/test/accounts_mixin.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/erpnext/accounts/test/accounts_mixin.py b/erpnext/accounts/test/accounts_mixin.py index b67e190f431..e526e07c734 100644 --- a/erpnext/accounts/test/accounts_mixin.py +++ b/erpnext/accounts/test/accounts_mixin.py @@ -121,8 +121,6 @@ class AccountsTestMixin: new_acc.save() setattr(self, acc.attribute_name, new_acc.name) -<<<<<<< HEAD -======= self.identify_default_warehouses() def enable_advance_as_liability(self): @@ -144,7 +142,6 @@ class AccountsTestMixin: ): setattr(self, "warehouse_" + w.warehouse_name.lower().strip().replace(" ", "_"), w.name) ->>>>>>> a21a406d04 (refactor(test): utility methods for enabling advance in separate acc) def create_usd_receivable_account(self): account_name = "Debtors USD" if not frappe.db.get_value( From a329003f7fc49ad1e534bcff6ce0bfa0fc76bbfe Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 13 Oct 2024 10:35:48 +0530 Subject: [PATCH 9/9] chore: use correct hook for advance payment doctypes --- erpnext/accounts/doctype/payment_entry/payment_entry.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 7e3aa25f358..7885d825129 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1166,9 +1166,7 @@ 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" - ) + advance_payment_doctypes = frappe.get_hooks("advance_payment_doctypes") if self.payment_type == "Receive": against_account = self.paid_to