From cda5ff40f18c406e232a34e83706b492ea1515ac Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sun, 26 Nov 2023 18:57:01 +0530 Subject: [PATCH] refactor: bank transaction (#38182) refactor: bank transaction (#38182) (cherry picked from commit 5426b93387c8a4599b11d5e064d4d8b37078aca1) Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com> --- .../bank_reconciliation_tool.py | 4 +- .../bank_transaction/bank_transaction.json | 17 ++- .../bank_transaction/bank_transaction.py | 139 ++++++++---------- 3 files changed, 77 insertions(+), 83 deletions(-) diff --git a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py index 9a7a9a31d536..f01ae2e8d4d7 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -355,7 +355,9 @@ def reconcile_vouchers(bank_transaction_name, vouchers): vouchers = json.loads(vouchers) transaction = frappe.get_doc("Bank Transaction", bank_transaction_name) transaction.add_payment_entries(vouchers) - return frappe.get_doc("Bank Transaction", bank_transaction_name) + transaction.save() + + return transaction @frappe.whitelist() diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index b32022e6fd8e..0328d51b8920 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json @@ -13,6 +13,7 @@ "status", "bank_account", "company", + "amended_from", "section_break_4", "deposit", "withdrawal", @@ -25,10 +26,10 @@ "transaction_id", "transaction_type", "section_break_14", + "column_break_oufv", "payment_entries", "section_break_18", "allocated_amount", - "amended_from", "column_break_17", "unallocated_amount", "party_section", @@ -138,10 +139,12 @@ "fieldtype": "Section Break" }, { + "allow_on_submit": 1, "fieldname": "allocated_amount", "fieldtype": "Currency", "label": "Allocated Amount", - "options": "currency" + "options": "currency", + "read_only": 1 }, { "fieldname": "amended_from", @@ -157,10 +160,12 @@ "fieldtype": "Column Break" }, { + "allow_on_submit": 1, "fieldname": "unallocated_amount", "fieldtype": "Currency", "label": "Unallocated Amount", - "options": "currency" + "options": "currency", + "read_only": 1 }, { "fieldname": "party_section", @@ -225,11 +230,15 @@ "fieldname": "bank_party_account_number", "fieldtype": "Data", "label": "Party Account No. (Bank Statement)" + }, + { + "fieldname": "column_break_oufv", + "fieldtype": "Column Break" } ], "is_submittable": 1, "links": [], - "modified": "2023-06-06 13:58:12.821411", + "modified": "2023-11-18 18:32:47.203694", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Transaction", diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 4649d2316286..51c823a45929 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -2,78 +2,73 @@ # For license information, please see license.txt import frappe +from frappe import _ from frappe.utils import flt from erpnext.controllers.status_updater import StatusUpdater class BankTransaction(StatusUpdater): - def after_insert(self): - self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) + def before_validate(self): + self.update_allocated_amount() - def on_submit(self): - self.clear_linked_payment_entries() + def validate(self): + self.validate_duplicate_references() + + def validate_duplicate_references(self): + """Make sure the same voucher is not allocated twice within the same Bank Transaction""" + if not self.payment_entries: + return + + pe = [] + for row in self.payment_entries: + reference = (row.payment_document, row.payment_entry) + if reference in pe: + frappe.throw( + _("{0} {1} is allocated twice in this Bank Transaction").format( + row.payment_document, row.payment_entry + ) + ) + pe.append(reference) + + def update_allocated_amount(self): + self.allocated_amount = ( + sum(p.allocated_amount for p in self.payment_entries) if self.payment_entries else 0.0 + ) + self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) - self.allocated_amount + + def before_submit(self): + self.allocate_payment_entries() self.set_status() if frappe.db.get_single_value("Accounts Settings", "enable_party_matching"): self.auto_set_party() - _saving_flag = False - - # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting - def on_update_after_submit(self): - "Run on save(). Avoid recursion caused by multiple saves" - if not self._saving_flag: - self._saving_flag = True - self.clear_linked_payment_entries() - self.update_allocations() - self._saving_flag = False + def before_update_after_submit(self): + self.validate_duplicate_references() + self.allocate_payment_entries() + self.update_allocated_amount() def on_cancel(self): - self.clear_linked_payment_entries(for_cancel=True) - self.set_status(update=True) - - def update_allocations(self): - "The doctype does not allow modifications after submission, so write to the db direct" - if self.payment_entries: - allocated_amount = sum(p.allocated_amount for p in self.payment_entries) - else: - allocated_amount = 0.0 + for payment_entry in self.payment_entries: + self.clear_linked_payment_entry(payment_entry, for_cancel=True) - amount = abs(flt(self.withdrawal) - flt(self.deposit)) - self.db_set("allocated_amount", flt(allocated_amount)) - self.db_set("unallocated_amount", amount - flt(allocated_amount)) - self.reload() self.set_status(update=True) def add_payment_entries(self, vouchers): "Add the vouchers with zero allocation. Save() will perform the allocations and clearance" if 0.0 >= self.unallocated_amount: - frappe.throw(frappe._("Bank Transaction {0} is already fully reconciled").format(self.name)) + frappe.throw(_("Bank Transaction {0} is already fully reconciled").format(self.name)) - added = False for voucher in vouchers: - # Can't add same voucher twice - found = False - for pe in self.payment_entries: - if ( - pe.payment_document == voucher["payment_doctype"] - and pe.payment_entry == voucher["payment_name"] - ): - found = True - - if not found: - pe = { + self.append( + "payment_entries", + { "payment_document": voucher["payment_doctype"], "payment_entry": voucher["payment_name"], "allocated_amount": 0.0, # Temporary - } - child = self.append("payment_entries", pe) - added = True - - # runs on_update_after_submit - if added: - self.save() + }, + ) def allocate_payment_entries(self): """Refactored from bank reconciliation tool. @@ -90,6 +85,7 @@ def allocate_payment_entries(self): - clear means: set the latest transaction date as clearance date """ remaining_amount = self.unallocated_amount + to_remove = [] for payment_entry in self.payment_entries: if payment_entry.allocated_amount == 0.0: unallocated_amount, should_clear, latest_transaction = get_clearance_details( @@ -99,49 +95,39 @@ def allocate_payment_entries(self): if 0.0 == unallocated_amount: if should_clear: latest_transaction.clear_linked_payment_entry(payment_entry) - self.db_delete_payment_entry(payment_entry) + to_remove.append(payment_entry) elif remaining_amount <= 0.0: - self.db_delete_payment_entry(payment_entry) + to_remove.append(payment_entry) - elif 0.0 < unallocated_amount and unallocated_amount <= remaining_amount: - payment_entry.db_set("allocated_amount", unallocated_amount) + elif 0.0 < unallocated_amount <= remaining_amount: + payment_entry.allocated_amount = unallocated_amount remaining_amount -= unallocated_amount if should_clear: latest_transaction.clear_linked_payment_entry(payment_entry) - elif 0.0 < unallocated_amount and unallocated_amount > remaining_amount: - payment_entry.db_set("allocated_amount", remaining_amount) + elif 0.0 < unallocated_amount: + payment_entry.allocated_amount = remaining_amount remaining_amount = 0.0 elif 0.0 > unallocated_amount: - self.db_delete_payment_entry(payment_entry) - frappe.throw(frappe._("Voucher {0} is over-allocated by {1}").format(unallocated_amount)) - - self.reload() + frappe.throw(_("Voucher {0} is over-allocated by {1}").format(unallocated_amount)) - def db_delete_payment_entry(self, payment_entry): - frappe.db.delete("Bank Transaction Payments", {"name": payment_entry.name}) + for payment_entry in to_remove: + self.remove(to_remove) @frappe.whitelist() def remove_payment_entries(self): for payment_entry in self.payment_entries: self.remove_payment_entry(payment_entry) - # runs on_update_after_submit - self.save() + + self.save() # runs before_update_after_submit def remove_payment_entry(self, payment_entry): "Clear payment entry and clearance" self.clear_linked_payment_entry(payment_entry, for_cancel=True) self.remove(payment_entry) - def clear_linked_payment_entries(self, for_cancel=False): - if for_cancel: - for payment_entry in self.payment_entries: - self.clear_linked_payment_entry(payment_entry, for_cancel) - else: - self.allocate_payment_entries() - def clear_linked_payment_entry(self, payment_entry, for_cancel=False): clearance_date = None if for_cancel else self.date set_voucher_clearance( @@ -162,11 +148,10 @@ def auto_set_party(self): deposit=self.deposit, ).match() - if result: - party_type, party = result - frappe.db.set_value( - "Bank Transaction", self.name, field={"party_type": party_type, "party": party} - ) + if not result: + return + + self.party_type, self.party = result @frappe.whitelist() @@ -198,9 +183,7 @@ def get_clearance_details(transaction, payment_entry): if gle["gl_account"] == gl_bank_account: if gle["amount"] <= 0.0: frappe.throw( - frappe._("Voucher {0} value is broken: {1}").format( - payment_entry.payment_entry, gle["amount"] - ) + _("Voucher {0} value is broken: {1}").format(payment_entry.payment_entry, gle["amount"]) ) unmatched_gles -= 1 @@ -221,7 +204,7 @@ def get_clearance_details(transaction, payment_entry): def get_related_bank_gl_entries(doctype, docname): # nosemgrep: frappe-semgrep-rules.rules.frappe-using-db-sql - result = frappe.db.sql( + return frappe.db.sql( """ SELECT ABS(gle.credit_in_account_currency - gle.debit_in_account_currency) AS amount, @@ -239,7 +222,6 @@ def get_related_bank_gl_entries(doctype, docname): dict(doctype=doctype, docname=docname), as_dict=True, ) - return result def get_total_allocated_amount(doctype, docname): @@ -365,6 +347,7 @@ def set_voucher_clearance(doctype, docname, clearance_date, self): if clearance_date: vouchers = [{"payment_doctype": "Bank Transaction", "payment_name": self.name}] bt.add_payment_entries(vouchers) + bt.save() else: for pe in bt.payment_entries: if pe.payment_document == self.doctype and pe.payment_entry == self.name: