From abe238552c3c74c2e34548ae0e0cb4174068f892 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 18 Nov 2023 18:05:43 +0100 Subject: [PATCH] refactor: bank transaction --- .../bank_transaction/bank_transaction.json | 10 +- .../bank_transaction/bank_transaction.py | 132 ++++++++---------- 2 files changed, 64 insertions(+), 78 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index b32022e6fd8e..36db222346f4 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", @@ -28,7 +29,6 @@ "payment_entries", "section_break_18", "allocated_amount", - "amended_from", "column_break_17", "unallocated_amount", "party_section", @@ -141,7 +141,8 @@ "fieldname": "allocated_amount", "fieldtype": "Currency", "label": "Allocated Amount", - "options": "currency" + "options": "currency", + "read_only": 1 }, { "fieldname": "amended_from", @@ -160,7 +161,8 @@ "fieldname": "unallocated_amount", "fieldtype": "Currency", "label": "Unallocated Amount", - "options": "currency" + "options": "currency", + "read_only": 1 }, { "fieldname": "party_section", @@ -229,7 +231,7 @@ ], "is_submittable": 1, "links": [], - "modified": "2023-06-06 13:58:12.821411", + "modified": "2023-11-18 17:35:37.828408", "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..98728b054e6c 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -2,78 +2,78 @@ # 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.update_allocated_amount() + self.validate_duplicate_references() + self.allocate_payment_entries() 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 + }, + ) + added = True - # runs on_update_after_submit if added: - self.save() + self.save() # runs before_update_after_submit def allocate_payment_entries(self): """Refactored from bank reconciliation tool. @@ -99,49 +99,37 @@ 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) + self.remove(payment_entry) elif remaining_amount <= 0.0: - self.db_delete_payment_entry(payment_entry) + self.remove(payment_entry) - elif 0.0 < unallocated_amount and unallocated_amount <= remaining_amount: + elif 0.0 < unallocated_amount <= remaining_amount: payment_entry.db_set("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: + elif 0.0 < unallocated_amount: payment_entry.db_set("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() - - def db_delete_payment_entry(self, payment_entry): - frappe.db.delete("Bank Transaction Payments", {"name": payment_entry.name}) + self.remove(payment_entry) + frappe.throw(_("Voucher {0} is over-allocated by {1}").format(unallocated_amount)) @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 +150,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 +185,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 +206,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 +224,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):