From 20eacd4f89e994f401b827562dbe4d0dbbb505fa Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 20 Sep 2023 09:36:18 +0530 Subject: [PATCH 1/4] test: assert ledger entries on partial reconciliation with `Advance as Liability`, partial reconciliation should not post duplicate ledger entries for same reference --- .../purchase_invoice/test_purchase_invoice.py | 4 +- .../sales_invoice/test_sales_invoice.py | 95 ++++++++++++++++++- 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index 13593bcf9b41..64dde6bcec13 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -1769,10 +1769,10 @@ def test_advance_entries_as_asset(self): # Check GL Entry against payment doctype expected_gle = [ - ["Advances Paid - _TC", 0.0, 500, nowdate()], + ["Advances Paid - _TC", 500.0, 0.0, nowdate()], + ["Advances Paid - _TC", 0.0, 500.0, nowdate()], ["Cash - _TC", 0.0, 500, nowdate()], ["Creditors - _TC", 500, 0.0, nowdate()], - ["Creditors - _TC", 500, 0.0, nowdate()], ] check_gl_entries(self, pe.name, expected_gle, nowdate(), voucher_type="Payment Entry") diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 21cc25395973..700bb1512a8c 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3351,21 +3351,21 @@ def test_sales_invoice_with_payable_tax_account(self): def test_advance_entries_as_liability(self): from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry - account = create_account( + advance_account = create_account( parent_account="Current Liabilities - _TC", account_name="Advances Received", company="_Test Company", account_type="Receivable", ) - set_advance_flag(company="_Test Company", flag=1, default_account=account) + set_advance_flag(company="_Test Company", flag=1, default_account=advance_account) pe = create_payment_entry( company="_Test Company", payment_type="Receive", party_type="Customer", party="_Test Customer", - paid_from="Debtors - _TC", + paid_from=advance_account, paid_to="Cash - _TC", paid_amount=1000, ) @@ -3391,9 +3391,9 @@ def test_advance_entries_as_liability(self): # Check GL Entry against payment doctype expected_gle = [ + ["Advances Received - _TC", 0.0, 1000.0, nowdate()], ["Advances Received - _TC", 500, 0.0, nowdate()], ["Cash - _TC", 1000, 0.0, nowdate()], - ["Debtors - _TC", 0.0, 1000, nowdate()], ["Debtors - _TC", 0.0, 500, nowdate()], ] @@ -3430,6 +3430,93 @@ def test_sales_return_negative_rate(self): si.items[0].rate = 10 si.save() + def test_partial_allocation_on_advance_as_liability(self): + from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry + + company = "_Test Company" + customer = "_Test Customer" + debtors_acc = "Debtors - _TC" + advance_account = create_account( + parent_account="Current Liabilities - _TC", + account_name="Advances Received", + company="_Test Company", + account_type="Receivable", + ) + + set_advance_flag(company="_Test Company", flag=1, default_account=advance_account) + + pe = create_payment_entry( + company=company, + payment_type="Receive", + party_type="Customer", + party=customer, + paid_from=advance_account, + paid_to="Cash - _TC", + paid_amount=1000, + ) + pe.submit() + + si = create_sales_invoice( + company=company, + customer=customer, + do_not_save=True, + do_not_submit=True, + rate=1000, + price_list_rate=1000, + ) + si.base_grand_total = 1000 + si.grand_total = 1000 + si.set_advances() + for advance in si.advances: + advance.allocated_amount = 200 if advance.reference_name == pe.name else 0 + si.save() + si.submit() + + self.assertEqual(si.advances[0].allocated_amount, 200) + + # Check GL Entry against partial from advance + expected_gle = [ + [advance_account, 0.0, 1000.0, nowdate()], + [advance_account, 200.0, 0.0, nowdate()], + ["Cash - _TC", 1000.0, 0.0, nowdate()], + [debtors_acc, 0.0, 200.0, nowdate()], + ] + check_gl_entries(self, pe.name, expected_gle, nowdate(), voucher_type="Payment Entry") + si.reload() + self.assertEqual(si.outstanding_amount, 800.0) + + pr = frappe.get_doc("Payment Reconciliation") + pr.company = company + pr.party_type = "Customer" + pr.party = customer + pr.receivable_payable_account = debtors_acc + pr.default_advance_account = advance_account + pr.get_unreconciled_entries() + + # allocate some more of the same advance + # self.assertEqual(len(pr.invoices), 1) + # self.assertEqual(len(pr.payments), 1) + invoices = [x.as_dict() for x in pr.invoices if x.get("invoice_number") == si.name] + payments = [x.as_dict() for x in pr.payments if x.get("reference_name") == pe.name] + pr.allocate_entries(frappe._dict({"invoices": invoices, "payments": payments})) + pr.allocation[0].allocated_amount = 300 + pr.reconcile() + + si.reload() + self.assertEqual(si.outstanding_amount, 500.0) + + # Check GL Entry against multi partial allocations from advance + expected_gle = [ + [advance_account, 0.0, 1000.0, nowdate()], + [advance_account, 200.0, 0.0, nowdate()], + [advance_account, 300.0, 0.0, nowdate()], + ["Cash - _TC", 1000.0, 0.0, nowdate()], + [debtors_acc, 0.0, 200.0, nowdate()], + [debtors_acc, 0.0, 300.0, nowdate()], + ] + check_gl_entries(self, pe.name, expected_gle, nowdate(), voucher_type="Payment Entry") + set_advance_flag(company="_Test Company", flag=0, default_account="") + def set_advance_flag(company, flag, default_account): frappe.db.set_value( From 5f462e4c98c40e67f5b8cb1c0fb306886a7c8a8e Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 20 Sep 2023 13:05:04 +0530 Subject: [PATCH 2/4] refactor: post to GL and Payment Ledger on advance as liability --- erpnext/accounts/utils.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 1c7052f8ffb6..f8d5324707e1 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -468,6 +468,9 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n frappe.flags.ignore_party_validation = True _delete_pl_entries(voucher_type, voucher_no) + if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account: + _delete_gl_entries(voucher_type, voucher_no) + for entry in entries: check_if_advance_entry_modified(entry) validate_allocated_amount(entry) @@ -488,16 +491,19 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n doc.save(ignore_permissions=True) # re-submit advance entry doc = frappe.get_doc(entry.voucher_type, entry.voucher_no) - gl_map = doc.build_gl_map() - create_payment_ledger_entry(gl_map, update_outstanding="No", cancel=0, adv_adj=1) + + if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account: + # both ledgers must be posted to for `Advance as Liability` + doc.make_gl_entries() + else: + gl_map = doc.build_gl_map() + create_payment_ledger_entry(gl_map, update_outstanding="No", cancel=0, adv_adj=1) # Only update outstanding for newly linked vouchers for entry in entries: update_voucher_outstanding( entry.against_voucher_type, entry.against_voucher, entry.account, entry.party_type, entry.party ) - if voucher_type == "Payment Entry": - doc.make_advance_gl_entries(entry.against_voucher_type, entry.against_voucher) frappe.flags.ignore_party_validation = False From 4bda00e234b8f62af156956abe4328ee7a264b79 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Sun, 24 Sep 2023 10:55:04 +0530 Subject: [PATCH 3/4] refactor: don't split based on reference while reconciling advance --- .../doctype/payment_entry/payment_entry.py | 69 +++++++++++-------- erpnext/accounts/utils.py | 8 +-- 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index e6403fddefe3..9b0c25fd5592 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1087,46 +1087,57 @@ def add_party_gl_entries(self, gl_entries): "credit" if erpnext.get_party_account_type(self.party_type) == "Receivable" else "debit" ) - for d in self.get("references"): - cost_center = self.cost_center - if d.reference_doctype == "Sales Invoice" and not cost_center: - cost_center = frappe.db.get_value(d.reference_doctype, d.reference_name, "cost_center") - + if self.book_advance_payments_in_separate_party_account: gle = party_gl_dict.copy() - - allocated_amount_in_company_currency = self.calculate_base_allocated_amount_for_reference(d) - - if self.book_advance_payments_in_separate_party_account: - against_voucher_type = "Payment Entry" - against_voucher = self.name + if self.payment_type == "Receive": + amount = self.base_paid_amount else: - against_voucher_type = d.reference_doctype - against_voucher = d.reference_name + amount = self.base_received_amount gle.update( { - dr_or_cr: allocated_amount_in_company_currency, - dr_or_cr + "_in_account_currency": d.allocated_amount, - "against_voucher_type": against_voucher_type, - "against_voucher": against_voucher, - "cost_center": cost_center, + dr_or_cr: amount, + dr_or_cr + "_in_account_currency": amount, + "against_voucher_type": "Payment Entry", + "against_voucher": self.name, + "cost_center": self.cost_center, } ) gl_entries.append(gle) + else: + for d in self.get("references"): + cost_center = self.cost_center + if d.reference_doctype == "Sales Invoice" and not cost_center: + cost_center = frappe.db.get_value(d.reference_doctype, d.reference_name, "cost_center") - if self.unallocated_amount: - exchange_rate = self.get_exchange_rate() - base_unallocated_amount = self.unallocated_amount * exchange_rate + gle = party_gl_dict.copy() - gle = party_gl_dict.copy() - gle.update( - { - dr_or_cr + "_in_account_currency": self.unallocated_amount, - dr_or_cr: base_unallocated_amount, - } - ) + allocated_amount_in_company_currency = self.calculate_base_allocated_amount_for_reference(d) - gl_entries.append(gle) + gle.update( + { + 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, + } + ) + gl_entries.append(gle) + + if self.unallocated_amount: + exchange_rate = self.get_exchange_rate() + base_unallocated_amount = self.unallocated_amount * exchange_rate + + gle = party_gl_dict.copy() + gle.update( + { + dr_or_cr + "_in_account_currency": self.unallocated_amount, + dr_or_cr: base_unallocated_amount, + } + ) + + gl_entries.append(gle) def make_advance_gl_entries(self, against_voucher_type=None, against_voucher=None, cancel=0): if self.book_advance_payments_in_separate_party_account: diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index f8d5324707e1..d64bb6b7f885 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -466,10 +466,8 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n # cancel advance entry doc = frappe.get_doc(voucher_type, voucher_no) frappe.flags.ignore_party_validation = True - _delete_pl_entries(voucher_type, voucher_no) - - if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account: - _delete_gl_entries(voucher_type, voucher_no) + if not (voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account): + _delete_pl_entries(voucher_type, voucher_no) for entry in entries: check_if_advance_entry_modified(entry) @@ -494,7 +492,7 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account: # both ledgers must be posted to for `Advance as Liability` - doc.make_gl_entries() + doc.make_advance_gl_entries() else: gl_map = doc.build_gl_map() create_payment_ledger_entry(gl_map, update_outstanding="No", cancel=0, adv_adj=1) From 9221e3591d9ff79f83b2bab6cb2ff988c4d000bc Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 28 Sep 2023 13:55:21 +0530 Subject: [PATCH 4/4] refactor: deduplicate using child table `name` --- .../doctype/payment_entry/payment_entry.py | 77 +++++++------------ erpnext/accounts/general_ledger.py | 26 ++++++- erpnext/accounts/utils.py | 20 +++-- 3 files changed, 65 insertions(+), 58 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 9b0c25fd5592..73437c406583 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -1062,7 +1062,8 @@ def make_gl_entries(self, cancel=0, adv_adj=0): else: self.make_exchange_gain_loss_journal() - self.make_advance_gl_entries(cancel=cancel) + if self.book_advance_payments_in_separate_party_account: + self.make_advance_gl_entries(cancel=cancel) def add_party_gl_entries(self, gl_entries): if self.party_account: @@ -1089,15 +1090,14 @@ def add_party_gl_entries(self, gl_entries): if self.book_advance_payments_in_separate_party_account: gle = party_gl_dict.copy() - if self.payment_type == "Receive": - amount = self.base_paid_amount - else: - amount = self.base_received_amount - gle.update( { - dr_or_cr: amount, - dr_or_cr + "_in_account_currency": amount, + dr_or_cr: self.base_paid_amount + if self.payment_type == "Receive" + else self.base_received_amount, + dr_or_cr + "_in_account_currency": self.paid_amount + if self.payment_type == "Receive" + else self.received_amount, "against_voucher_type": "Payment Entry", "against_voucher": self.name, "cost_center": self.cost_center, @@ -1139,53 +1139,28 @@ def add_party_gl_entries(self, gl_entries): gl_entries.append(gle) - def make_advance_gl_entries(self, against_voucher_type=None, against_voucher=None, cancel=0): - if self.book_advance_payments_in_separate_party_account: - gl_entries = [] - for d in self.get("references"): - if d.reference_doctype in ("Sales Invoice", "Purchase Invoice", "Journal Entry"): - if not (against_voucher_type and against_voucher) or ( - d.reference_doctype == against_voucher_type and d.reference_name == against_voucher - ): - self.make_invoice_liability_entry(gl_entries, d) - - if cancel: - for entry in gl_entries: - frappe.db.set_value( - "GL Entry", - { - "voucher_no": self.name, - "voucher_type": self.doctype, - "voucher_detail_no": entry.voucher_detail_no, - "against_voucher_type": entry.against_voucher_type, - "against_voucher": entry.against_voucher, - }, - "is_cancelled", - 1, - ) + def make_advance_gl_entries( + self, entry: object | dict = None, cancel: bool = 0, update_outstanding: str = "Yes" + ): + gl_entries = [] + self.add_advance_gl_entries(gl_entries, entry) - make_reverse_gl_entries(gl_entries=gl_entries, partial_cancel=True) - return + if cancel: + make_reverse_gl_entries(gl_entries, partial_cancel=True) + else: + make_gl_entries(gl_entries, update_outstanding=update_outstanding) - # same reference added to payment entry - for gl_entry in gl_entries.copy(): - if frappe.db.exists( - "GL Entry", - { - "account": gl_entry.account, - "voucher_type": gl_entry.voucher_type, - "voucher_no": gl_entry.voucher_no, - "voucher_detail_no": gl_entry.voucher_detail_no, - "debit": gl_entry.debit, - "credit": gl_entry.credit, - "is_cancelled": 0, - }, - ): - gl_entries.remove(gl_entry) + def add_advance_gl_entries(self, gl_entries: list, entry: object | dict): + if self.book_advance_payments_in_separate_party_account: + references = [x for x in self.get("references")] + if entry: + references = [x for x in self.get("references") if x.name == entry.name] - make_gl_entries(gl_entries) + for ref in references: + if ref.reference_doctype in ("Sales Invoice", "Purchase Invoice", "Journal Entry"): + self.add_advance_gl_for_reference(gl_entries, ref) - def make_invoice_liability_entry(self, gl_entries, invoice): + def add_advance_gl_for_reference(self, gl_entries, invoice): args_dict = { "party_type": self.party_type, "party": self.party, diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 70a847061476..33773bb441da 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -597,7 +597,31 @@ def make_reverse_gl_entries( is_opening = any(d.get("is_opening") == "Yes" for d in gl_entries) validate_against_pcv(is_opening, gl_entries[0]["posting_date"], gl_entries[0]["company"]) - if not partial_cancel: + if partial_cancel: + # Partial cancel is only used by `Advance` in separate account feature. + # Only cancel GL entries for unlinked reference using `voucher_detail_no` + gle = frappe.qb.DocType("GL Entry") + for x in gl_entries: + query = ( + frappe.qb.update(gle) + .set(gle.is_cancelled, True) + .set(gle.modified, now()) + .set(gle.modified_by, frappe.session.user) + .where( + (gle.company == x.company) + & (gle.account == x.account) + & (gle.party_type == x.party_type) + & (gle.party == x.party) + & (gle.voucher_type == x.voucher_type) + & (gle.voucher_no == x.voucher_no) + & (gle.against_voucher_type == x.against_voucher_type) + & (gle.against_voucher == x.against_voucher) + & (gle.voucher_detail_no == x.voucher_detail_no) + ) + ) + query.run() + + else: set_as_cancel(gl_entries[0]["voucher_type"], gl_entries[0]["voucher_no"]) for entry in gl_entries: diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index d64bb6b7f885..68d97e1dabdc 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -466,8 +466,10 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n # cancel advance entry 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 if not (voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account): - _delete_pl_entries(voucher_type, voucher_no) + _delete_gl_entries(voucher_type, voucher_no) for entry in entries: check_if_advance_entry_modified(entry) @@ -482,7 +484,7 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n entry.update({"referenced_row": referenced_row}) doc.make_exchange_gain_loss_journal([entry]) else: - update_reference_in_payment_entry( + referenced_row = update_reference_in_payment_entry( entry, doc, do_not_save=True, skip_ref_details_update_for_pe=skip_ref_details_update_for_pe ) @@ -491,8 +493,8 @@ def reconcile_against_document(args, skip_ref_details_update_for_pe=False): # n 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 as Liability` - doc.make_advance_gl_entries() + # both ledgers must be posted to for `Advance` in separate account feature + doc.make_advance_gl_entries(referenced_row, update_outstanding="No") else: gl_map = doc.build_gl_map() create_payment_ledger_entry(gl_map, update_outstanding="No", cancel=0, adv_adj=1) @@ -669,11 +671,12 @@ def update_reference_in_payment_entry( new_row.docstatus = 1 for field in list(reference_details): new_row.set(field, reference_details[field]) - + row = new_row else: new_row = payment_entry.append("references") new_row.docstatus = 1 new_row.update(reference_details) + row = new_row payment_entry.flags.ignore_validate_update_after_submit = True payment_entry.clear_unallocated_reference_document_rows() @@ -689,6 +692,8 @@ def update_reference_in_payment_entry( if not do_not_save: payment_entry.save(ignore_permissions=True) + return row + def cancel_exchange_gain_loss_journal( parent_doc: dict | object, referenced_dt: str = None, referenced_dn: str = None @@ -864,7 +869,10 @@ def remove_ref_doc_link_from_pe( try: pe_doc = frappe.get_doc("Payment Entry", pe) pe_doc.set_amounts() - pe_doc.make_advance_gl_entries(against_voucher_type=ref_type, against_voucher=ref_no, cancel=1) + references = [ + x for x in pe_doc.references if x.reference_doctype == ref_type and x.reference_name == ref_no + ] + [pe_doc.make_advance_gl_entries(x, cancel=1) for x in references] pe_doc.clear_unallocated_reference_document_rows() pe_doc.validate_payment_type_with_outstanding() except Exception as e: