From 8bb4ffc6b1f72902b40c95656595bb263558cfdf Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 22:17:49 +0530 Subject: [PATCH 1/7] refactor(journal_entry): replace raw SQL with Query Builder Convert the five raw frappe.db.sql calls to Query Builder / ORM: the against-JV lookup, the write-off invoice listing (get_values, now a single query), the JV outstanding aggregate (get_outstanding), and the bill-no lookup (get_value). Behaviour preserved. --- .../doctype/journal_entry/journal_entry.py | 102 ++++++++++-------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index b7de5891781..059f96367df 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -8,6 +8,7 @@ import frappe from frappe import _, msgprint, scrub from frappe.core.doctype.submission_queue.submission_queue import queue_submission from frappe.model.document import Document +from frappe.query_builder.functions import Sum from frappe.utils import comma_and, cstr, flt, fmt_money, formatdate, get_link_to_form, nowdate import erpnext @@ -571,13 +572,20 @@ class JournalEntry(AccountsController): if d.reference_name == self.name: frappe.throw(_("You can not enter current voucher in 'Against Journal Entry' column")) - against_entries = frappe.db.sql( - """select * from `tabJournal Entry Account` - where account = %s and docstatus = 1 and parent = %s - and (reference_type is null or reference_type in ('', 'Sales Order', 'Purchase Order')) - """, - (d.account, d.reference_name), - as_dict=True, + jea = frappe.qb.DocType("Journal Entry Account") + against_entries = ( + frappe.qb.from_(jea) + .select(jea.star) + .where( + (jea.account == d.account) + & (jea.docstatus == 1) + & (jea.parent == d.reference_name) + & ( + jea.reference_type.isnull() + | jea.reference_type.isin(["", "Sales Order", "Purchase Order"]) + ) + ) + .run(as_dict=True) ) if not against_entries: @@ -751,21 +759,15 @@ class JournalEntry(AccountsController): ) if d.reference_type == "Purchase Invoice" and d.debit: - bill_no = frappe.db.sql( - """select bill_no, bill_date - from `tabPurchase Invoice` where name=%s""", - d.reference_name, - ) - if ( - bill_no - and bill_no[0][0] - and bill_no[0][0].lower().strip() not in ["na", "not applicable", "none"] - ): + bill_no, bill_date = frappe.db.get_value( + "Purchase Invoice", d.reference_name, ["bill_no", "bill_date"] + ) or (None, None) + if bill_no and bill_no.lower().strip() not in ["na", "not applicable", "none"]: r.append( _("{0} against Bill {1} dated {2}").format( fmt_money(flt(d.debit), currency=self.company_currency), - bill_no[0][0], - bill_no[0][1] and formatdate(bill_no[0][1].strftime("%Y-%m-%d")), + bill_no, + bill_date and formatdate(bill_date.strftime("%Y-%m-%d")), ) ) @@ -912,28 +914,32 @@ class JournalEntry(AccountsController): self.validate_total_debit_and_credit() def get_values(self): - cond = ( - f" and outstanding_amount <= {flt(self.write_off_amount)}" - if flt(self.write_off_amount) > 0 - else "" - ) - if self.write_off_based_on == "Accounts Receivable": - return frappe.db.sql( - """select name, debit_to as account, customer as party, outstanding_amount - from `tabSales Invoice` where docstatus = 1 and company = {} - and outstanding_amount > 0 {}""".format("%s", cond), - self.company, - as_dict=True, - ) + doctype, account_field, party_field = "Sales Invoice", "debit_to", "customer" elif self.write_off_based_on == "Accounts Payable": - return frappe.db.sql( - """select name, credit_to as account, supplier as party, outstanding_amount - from `tabPurchase Invoice` where docstatus = 1 and company = {} - and outstanding_amount > 0 {}""".format("%s", cond), - self.company, - as_dict=True, + doctype, account_field, party_field = "Purchase Invoice", "credit_to", "supplier" + else: + return + + invoice = frappe.qb.DocType(doctype) + query = ( + frappe.qb.from_(invoice) + .select( + invoice.name, + invoice[account_field].as_("account"), + invoice[party_field].as_("party"), + invoice.outstanding_amount, ) + .where( + (invoice.docstatus == 1) + & (invoice.company == self.company) + & (invoice.outstanding_amount > 0) + ) + ) + if flt(self.write_off_amount) > 0: + query = query.where(invoice.outstanding_amount <= flt(self.write_off_amount)) + + return query.run(as_dict=True) def validate_credit_debit_note(self): if self.stock_entry: @@ -1059,16 +1065,20 @@ def get_outstanding(args: str | dict): due_date = None if args.get("doctype") == "Journal Entry": - condition = " and party=%(party)s" if args.get("party") else "" - - against_jv_amount = frappe.db.sql( - f""" - select sum(debit_in_account_currency) - sum(credit_in_account_currency) - from `tabJournal Entry Account` where parent=%(docname)s and account=%(account)s {condition} - and (reference_type is null or reference_type = '')""", - args, + jea = frappe.qb.DocType("Journal Entry Account") + query = ( + frappe.qb.from_(jea) + .select(Sum(jea.debit_in_account_currency) - Sum(jea.credit_in_account_currency)) + .where( + (jea.parent == args.get("docname")) + & (jea.account == args.get("account")) + & (jea.reference_type.isnull() | (jea.reference_type == "")) + ) ) + if args.get("party"): + query = query.where(jea.party == args.get("party")) + against_jv_amount = query.run() against_jv_amount = flt(against_jv_amount[0][0]) if against_jv_amount else 0 amount_field = "credit_in_account_currency" if against_jv_amount > 0 else "debit_in_account_currency" return {amount_field: abs(against_jv_amount)} From 1105cb8ddfd92b8daf27fe3f42dd39e53515ce60 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 22:24:44 +0530 Subject: [PATCH 2/7] refactor(journal_entry): add missing type hints Add return annotations to the module-level helpers and to make_gl_entries, get_balance and set_total_amount, plus parameter types for set_total_amount and make_gl_entries. --- .../doctype/journal_entry/journal_entry.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 059f96367df..b2f4a026953 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -812,7 +812,7 @@ class JournalEntry(AccountsController): self.set_total_amount(total_amount, currency) - def set_total_amount(self, amt, currency): + def set_total_amount(self, amt: float, currency: str) -> None: self.total_amount = amt self.total_amount_currency = currency from frappe.utils import money_in_words @@ -824,7 +824,7 @@ class JournalEntry(AccountsController): return JournalEntryGLComposer(self).compose() - def make_gl_entries(self, cancel=0, adv_adj=0): + def make_gl_entries(self, cancel: int = 0, adv_adj: int = 0) -> None: from erpnext.accounts.general_ledger import make_gl_entries merge_entries = frappe.get_single_value("Accounts Settings", "merge_similar_account_heads") @@ -848,7 +848,7 @@ class JournalEntry(AccountsController): cancel_exchange_gain_loss_journal(frappe._dict(doctype=self.doctype, name=self.name)) @frappe.whitelist() - def get_balance(self, difference_account: str | None = None): + def get_balance(self, difference_account: str | None = None) -> None: if not self.get("accounts"): msgprint(_("'Entries' cannot be empty"), raise_exception=True) else: @@ -968,7 +968,7 @@ def get_default_bank_cash_account( account: str | None = None, *, fetch_balance: bool = True, -): +) -> dict: from erpnext.accounts.doctype.sales_invoice.sales_invoice import get_bank_cash_account if mode_of_payment: @@ -1023,7 +1023,7 @@ def get_against_jv( start: int, page_len: int, filters: dict, -): +) -> list: if not frappe.db.has_column("Journal Entry", searchfield): return [] @@ -1054,7 +1054,7 @@ def get_against_jv( @frappe.whitelist() -def get_outstanding(args: str | dict): +def get_outstanding(args: str | dict) -> dict: if not frappe.has_permission("Account"): frappe.msgprint(_("No Permission"), raise_exception=1) @@ -1118,7 +1118,7 @@ def get_outstanding(args: str | dict): @frappe.whitelist() -def get_party_account_and_currency(company: str, party_type: str, party: str): +def get_party_account_and_currency(company: str, party_type: str, party: str) -> dict: if not frappe.has_permission("Account"): frappe.msgprint(_("No Permission"), raise_exception=1) @@ -1138,7 +1138,7 @@ def get_account_details_and_party_type( debit: float | str | None = None, credit: float | str | None = None, exchange_rate: float | str | None = None, -): +) -> dict: """Returns dict of account details and party type to be set in Journal Entry on selection of account.""" if not frappe.has_permission("Account"): frappe.msgprint(_("No Permission"), raise_exception=1) @@ -1196,7 +1196,7 @@ def get_exchange_rate( debit: float | str | None = None, credit: float | str | None = None, exchange_rate: str | float | None = None, -): +) -> float: # Ensure exchange_rate is always numeric to avoid calculation errors if isinstance(exchange_rate, str): exchange_rate = flt(exchange_rate) or 1 @@ -1232,7 +1232,7 @@ def get_exchange_rate( @frappe.whitelist() -def get_average_exchange_rate(account: str): +def get_average_exchange_rate(account: str) -> float: exchange_rate = 0 bank_balance_in_account_currency = get_balance_on(account) if bank_balance_in_account_currency: From 4c1cabb53e9d173a7a9bbdcb241acbacd0505564 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 22:30:51 +0530 Subject: [PATCH 3/7] refactor(journal_entry): break up create_remarks and validate_against_jv Split create_remarks into _cheque_remark / _reference_remark / _bill_remark helpers, and validate_against_jv into _validate_jv_reference, _validate_jv_reference_direction and _against_jv_entries. Add docstrings. Behaviour preserved. --- .../doctype/journal_entry/journal_entry.py | 218 +++++++++--------- 1 file changed, 114 insertions(+), 104 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index b2f4a026953..6499d6c9ae2 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -544,69 +544,76 @@ class JournalEntry(AccountsController): self.voucher_type == "Exchange Gain Or Loss" and self.multi_currency and self.is_system_generated ) - def validate_against_jv(self): - for d in self.get("accounts"): - if d.reference_type == "Journal Entry": - account_root_type = frappe.get_cached_value("Account", d.account, "root_type") - if ( - account_root_type == "Asset" - and flt(d.debit) > 0 - and not self.system_generated_gain_loss() - ): - frappe.throw( - _( - "Row #{0}: For {1}, you can select reference document only if account gets credited" - ).format(d.idx, d.account) - ) - elif ( - account_root_type == "Liability" - and flt(d.credit) > 0 - and not self.system_generated_gain_loss() - ): - frappe.throw( - _( - "Row #{0}: For {1}, you can select reference document only if account gets debited" - ).format(d.idx, d.account) - ) + def validate_against_jv(self) -> None: + """Validate every account row that references another Journal Entry.""" + for row in self.get("accounts"): + if row.reference_type == "Journal Entry": + self._validate_jv_reference(row) - if d.reference_name == self.name: - frappe.throw(_("You can not enter current voucher in 'Against Journal Entry' column")) + def _validate_jv_reference(self, row) -> None: + """Validate a single 'Against Journal Entry' row: direction, no self-reference, + and the presence of an unmatched entry on the referenced Journal Entry.""" + self._validate_jv_reference_direction(row) - jea = frappe.qb.DocType("Journal Entry Account") - against_entries = ( - frappe.qb.from_(jea) - .select(jea.star) - .where( - (jea.account == d.account) - & (jea.docstatus == 1) - & (jea.parent == d.reference_name) - & ( - jea.reference_type.isnull() - | jea.reference_type.isin(["", "Sales Order", "Purchase Order"]) - ) - ) - .run(as_dict=True) + if row.reference_name == self.name: + frappe.throw(_("You can not enter current voucher in 'Against Journal Entry' column")) + + against_entries = self._against_jv_entries(row) + if not against_entries: + if self.voucher_type != "Exchange Gain Or Loss": + frappe.throw( + _( + "Journal Entry {0} does not have account {1} or already matched against other voucher" + ).format(row.reference_name, row.account) ) + return - if not against_entries: - if self.voucher_type != "Exchange Gain Or Loss": - frappe.throw( - _( - "Journal Entry {0} does not have account {1} or already matched against other voucher" - ).format(d.reference_name, d.account) - ) - else: - dr_or_cr = "debit" if flt(d.credit) > 0 else "credit" - valid = False - for jvd in against_entries: - if flt(jvd[dr_or_cr]) > 0: - valid = True - if not valid and not self.system_generated_gain_loss(): - frappe.throw( - _("Against Journal Entry {0} does not have any unmatched {1} entry").format( - d.reference_name, dr_or_cr - ) - ) + dr_or_cr = "debit" if flt(row.credit) > 0 else "credit" + has_unmatched_entry = any(flt(entry[dr_or_cr]) > 0 for entry in against_entries) + if not has_unmatched_entry and not self.system_generated_gain_loss(): + frappe.throw( + _("Against Journal Entry {0} does not have any unmatched {1} entry").format( + row.reference_name, dr_or_cr + ) + ) + + def _validate_jv_reference_direction(self, row) -> None: + """An asset account can reference a JE only when credited, a liability only when debited.""" + if self.system_generated_gain_loss(): + return + + account_root_type = frappe.get_cached_value("Account", row.account, "root_type") + if account_root_type == "Asset" and flt(row.debit) > 0: + frappe.throw( + _( + "Row #{0}: For {1}, you can select reference document only if account gets credited" + ).format(row.idx, row.account) + ) + if account_root_type == "Liability" and flt(row.credit) > 0: + frappe.throw( + _("Row #{0}: For {1}, you can select reference document only if account gets debited").format( + row.idx, row.account + ) + ) + + def _against_jv_entries(self, row) -> list[dict]: + """Submitted Journal Entry Account rows on the referenced JE for the same account + that are not themselves linked to an order.""" + jea = frappe.qb.DocType("Journal Entry Account") + return ( + frappe.qb.from_(jea) + .select(jea.star) + .where( + (jea.account == row.account) + & (jea.docstatus == 1) + & (jea.parent == row.reference_name) + & ( + jea.reference_type.isnull() + | jea.reference_type.isin(["", "Sales Order", "Purchase Order"]) + ) + ) + .run(as_dict=True) + ) def set_against_account(self): accounts_debited, accounts_credited = [], [] @@ -728,58 +735,61 @@ class JournalEntry(AccountsController): if not d.exchange_rate: frappe.throw(_("Row {0}: Exchange Rate is mandatory").format(d.idx)) - def create_remarks(self): - r = [] - - if self.flags.skip_remarks_creation: + def create_remarks(self) -> None: + """Build the auto remark from the cheque reference and each account row's linked + document, unless remark creation is skipped or a custom remark is set.""" + if self.flags.skip_remarks_creation or self.get("custom_remark"): return - if self.get("custom_remark"): - return + remarks = [] + if cheque_remark := self._cheque_remark(): + remarks.append(cheque_remark) - if self.cheque_no: - if self.cheque_date: - r.append(_("Reference #{0} dated {1}").format(self.cheque_no, formatdate(self.cheque_date))) - else: - msgprint(_("Please enter Reference date"), raise_exception=frappe.MandatoryError) + for row in self.get("accounts"): + if reference_remark := self._reference_remark(row): + remarks.append(reference_remark) - for d in self.get("accounts"): - if d.reference_type == "Sales Invoice" and d.credit: - r.append( - _("{0} against Sales Invoice {1}").format( - fmt_money(flt(d.credit), currency=self.company_currency), d.reference_name - ) - ) + if remarks: + self.remark = "\n".join(remarks) # User Remarks is not mandatory - if d.reference_type == "Sales Order" and d.credit: - r.append( - _("{0} against Sales Order {1}").format( - fmt_money(flt(d.credit), currency=self.company_currency), d.reference_name - ) - ) + def _cheque_remark(self) -> str | None: + """Remark line for the cheque reference; raises if the cheque date is missing.""" + if not self.cheque_no: + return None + if not self.cheque_date: + msgprint(_("Please enter Reference date"), raise_exception=frappe.MandatoryError) + return _("Reference #{0} dated {1}").format(self.cheque_no, formatdate(self.cheque_date)) - if d.reference_type == "Purchase Invoice" and d.debit: - bill_no, bill_date = frappe.db.get_value( - "Purchase Invoice", d.reference_name, ["bill_no", "bill_date"] - ) or (None, None) - if bill_no and bill_no.lower().strip() not in ["na", "not applicable", "none"]: - r.append( - _("{0} against Bill {1} dated {2}").format( - fmt_money(flt(d.debit), currency=self.company_currency), - bill_no, - bill_date and formatdate(bill_date.strftime("%Y-%m-%d")), - ) - ) + def _reference_remark(self, row) -> str | None: + """Remark line for a single account row's linked Invoice/Order, or None.""" + if row.reference_type == "Sales Invoice" and row.credit: + return _("{0} against Sales Invoice {1}").format( + fmt_money(flt(row.credit), currency=self.company_currency), row.reference_name + ) + if row.reference_type == "Sales Order" and row.credit: + return _("{0} against Sales Order {1}").format( + fmt_money(flt(row.credit), currency=self.company_currency), row.reference_name + ) + if row.reference_type == "Purchase Invoice" and row.debit: + return self._bill_remark(row) + if row.reference_type == "Purchase Order" and row.debit: + return _("{0} against Purchase Order {1}").format( + fmt_money(flt(row.credit), currency=self.company_currency), row.reference_name + ) + return None - if d.reference_type == "Purchase Order" and d.debit: - r.append( - _("{0} against Purchase Order {1}").format( - fmt_money(flt(d.credit), currency=self.company_currency), d.reference_name - ) - ) - - if r: - self.remark = ("\n").join(r) # User Remarks is not mandatory + def _bill_remark(self, row) -> str | None: + """Remark line referencing the supplier bill number/date of a Purchase Invoice row.""" + bill_no, bill_date = frappe.db.get_value( + "Purchase Invoice", row.reference_name, ["bill_no", "bill_date"] + ) or (None, None) + if not bill_no or bill_no.lower().strip() in ["na", "not applicable", "none"]: + return None + return _("{0} against Bill {1} dated {2}").format( + fmt_money(flt(row.debit), currency=self.company_currency), + bill_no, + bill_date and formatdate(bill_date.strftime("%Y-%m-%d")), + ) def set_print_format_fields(self): bank_amount = party_amount = total_amount = 0.0 From 32d7250946049b9b0e06cbf34cfdd956bcca3225 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 22:35:43 +0530 Subject: [PATCH 4/7] refactor(journal_entry): break up reporting, exchange-rate and balance methods Decompose update_invoice_discounting, set_print_format_fields, get_balance_for_periodic_accounting, set_exchange_rate, get_balance and get_outstanding_invoices into focused per-row / row-building helpers (verb prefixed, with docstrings). The nested closure in update_invoice_discounting that ignored its row id is dropped. Behaviour preserved. --- .../doctype/journal_entry/journal_entry.py | 371 +++++++++--------- 1 file changed, 196 insertions(+), 175 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 6499d6c9ae2..ae3be120ce6 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -222,18 +222,16 @@ class JournalEntry(AccountsController): JournalTaxWithholding(self).on_submit() @frappe.whitelist() - def get_balance_for_periodic_accounting(self): + def get_balance_for_periodic_accounting(self) -> None: + """Rebuild the entry rows from the stock-vs-ledger difference of each stock account.""" self.validate_company_for_periodic_accounting() - stock_accounts = self.get_stock_accounts_for_periodic_accounting() self.set("accounts", []) - for account in stock_accounts: - account_bal, stock_bal, warehouse_list = get_stock_and_account_balance( + for account in self.get_stock_accounts_for_periodic_accounting(): + account_bal, stock_bal, _warehouse_list = get_stock_and_account_balance( account, self.posting_date, self.company ) - difference_value = flt(stock_bal - account_bal, self.precision("difference")) - if difference_value == 0: frappe.msgprint( _("No difference found for stock account {0}").format(frappe.bold(account)), @@ -241,23 +239,26 @@ class JournalEntry(AccountsController): ) continue - self.append( - "accounts", - { - "account": account, - "debit_in_account_currency": difference_value if difference_value > 0 else 0, - "credit_in_account_currency": abs(difference_value) if difference_value < 0 else 0, - }, - ) + self._append_periodic_difference_rows(account, difference_value) - self.append( - "accounts", - { - "account": self.periodic_entry_difference_account, - "credit_in_account_currency": difference_value if difference_value > 0 else 0, - "debit_in_account_currency": abs(difference_value) if difference_value < 0 else 0, - }, - ) + def _append_periodic_difference_rows(self, account: str, difference_value: float) -> None: + """Append the stock account row and its offsetting difference-account row.""" + self.append( + "accounts", + { + "account": account, + "debit_in_account_currency": difference_value if difference_value > 0 else 0, + "credit_in_account_currency": abs(difference_value) if difference_value < 0 else 0, + }, + ) + self.append( + "accounts", + { + "account": self.periodic_entry_difference_account, + "credit_in_account_currency": difference_value if difference_value > 0 else 0, + "debit_in_account_currency": abs(difference_value) if difference_value < 0 else 0, + }, + ) def validate_company_for_periodic_accounting(self): if erpnext.is_perpetual_inventory_enabled(self.company): @@ -386,49 +387,44 @@ class JournalEntry(AccountsController): self.name, ) - def update_invoice_discounting(self): - def _validate_invoice_discounting_status(inv_disc, id_status, expected_status, row_id): - id_link = get_link_to_form("Invoice Discounting", inv_disc) - if id_status != expected_status: - frappe.throw( - _("Row #{0}: Status must be {1} for Invoice Discounting {2}").format( - d.idx, expected_status, id_link - ) - ) + def update_invoice_discounting(self) -> None: + """Advance each linked Invoice Discounting to its next status on submit/cancel.""" + discounting_names = { + row.reference_name for row in self.accounts if row.reference_type == "Invoice Discounting" + } + for name in discounting_names: + inv_disc = frappe.get_doc("Invoice Discounting", name) + if status := self._get_next_invoice_discounting_status(inv_disc): + inv_disc.set_status(status=status) - invoice_discounting_list = list( - set([d.reference_name for d in self.accounts if d.reference_type == "Invoice Discounting"]) - ) - for inv_disc in invoice_discounting_list: - inv_disc_doc = frappe.get_doc("Invoice Discounting", inv_disc) - status = None - for d in self.accounts: - if d.account == inv_disc_doc.short_term_loan and d.reference_name == inv_disc: - if self.docstatus == 1: - if d.credit > 0: - _validate_invoice_discounting_status( - inv_disc, inv_disc_doc.status, "Sanctioned", d.idx - ) - status = "Disbursed" - elif d.debit > 0: - _validate_invoice_discounting_status( - inv_disc, inv_disc_doc.status, "Disbursed", d.idx - ) - status = "Settled" - else: - if d.credit > 0: - _validate_invoice_discounting_status( - inv_disc, inv_disc_doc.status, "Disbursed", d.idx - ) - status = "Sanctioned" - elif d.debit > 0: - _validate_invoice_discounting_status( - inv_disc, inv_disc_doc.status, "Settled", d.idx - ) - status = "Disbursed" - break - if status: - inv_disc_doc.set_status(status=status) + def _get_next_invoice_discounting_status(self, inv_disc) -> str | None: + """Validate the current status and return the next one from the loan account row.""" + for row in self.accounts: + if row.account != inv_disc.short_term_loan or row.reference_name != inv_disc.name: + continue + + submitting = self.docstatus == 1 + if row.credit > 0: + expected, next_status = ( + ("Sanctioned", "Disbursed") if submitting else ("Disbursed", "Sanctioned") + ) + elif row.debit > 0: + expected, next_status = ("Disbursed", "Settled") if submitting else ("Settled", "Disbursed") + else: + return None + + self._validate_invoice_discounting_status(inv_disc, expected, row.idx) + return next_status + return None + + def _validate_invoice_discounting_status(self, inv_disc, expected_status: str, row_idx: int) -> None: + """Throw unless the Invoice Discounting is in the status expected for this transition.""" + if inv_disc.status != expected_status: + frappe.throw( + _("Row #{0}: Status must be {1} for Invoice Discounting {2}").format( + row_idx, expected_status, get_link_to_form("Invoice Discounting", inv_disc.name) + ) + ) def unlink_advance_entry_reference(self): for d in self.get("accounts"): @@ -558,7 +554,7 @@ class JournalEntry(AccountsController): if row.reference_name == self.name: frappe.throw(_("You can not enter current voucher in 'Against Journal Entry' column")) - against_entries = self._against_jv_entries(row) + against_entries = self._get_against_jv_entries(row) if not against_entries: if self.voucher_type != "Exchange Gain Or Loss": frappe.throw( @@ -596,7 +592,7 @@ class JournalEntry(AccountsController): ) ) - def _against_jv_entries(self, row) -> list[dict]: + def _get_against_jv_entries(self, row) -> list[dict]: """Submitted Journal Entry Account rows on the referenced JE for the same account that are not themselves linked to an order.""" jea = frappe.qb.DocType("Journal Entry Account") @@ -701,39 +697,43 @@ class JournalEntry(AccountsController): d.debit = flt(d.debit_in_account_currency * flt(d.exchange_rate), d.precision("debit")) d.credit = flt(d.credit_in_account_currency * flt(d.exchange_rate), d.precision("credit")) - def set_exchange_rate(self): - for d in self.get("accounts"): - if d.account_currency == self.company_currency: - d.exchange_rate = 1 - elif ( - not d.exchange_rate - or d.exchange_rate == 1 - or ( - d.reference_type in ("Sales Invoice", "Purchase Invoice") - and d.reference_name - and self.posting_date - ) - ): - ignore_exchange_rate = False - if self.get("flags") and self.flags.get("ignore_exchange_rate"): - ignore_exchange_rate = True + def set_exchange_rate(self) -> None: + """Resolve a mandatory exchange rate for every account row.""" + for row in self.get("accounts"): + self._set_row_exchange_rate(row) + if not row.exchange_rate: + frappe.throw(_("Row {0}: Exchange Rate is mandatory").format(row.idx)) - if not ignore_exchange_rate: - # Modified to include the posting date for which to retreive the exchange rate - d.exchange_rate = get_exchange_rate( - self.posting_date, - d.account, - d.account_currency, - self.company, - d.reference_type, - d.reference_name, - d.debit, - d.credit, - d.exchange_rate, - ) + def _set_row_exchange_rate(self, row) -> None: + """Set a row's exchange rate: 1 for company currency, otherwise fetched when stale.""" + if row.account_currency == self.company_currency: + row.exchange_rate = 1 + return - if not d.exchange_rate: - frappe.throw(_("Row {0}: Exchange Rate is mandatory").format(d.idx)) + needs_refresh = ( + not row.exchange_rate + or row.exchange_rate == 1 + or ( + row.reference_type in ("Sales Invoice", "Purchase Invoice") + and row.reference_name + and self.posting_date + ) + ) + if not needs_refresh or self.flags.get("ignore_exchange_rate"): + return + + # Includes the posting date for which to retrieve the exchange rate + row.exchange_rate = get_exchange_rate( + self.posting_date, + row.account, + row.account_currency, + self.company, + row.reference_type, + row.reference_name, + row.debit, + row.credit, + row.exchange_rate, + ) def create_remarks(self) -> None: """Build the auto remark from the cheque reference and each account row's linked @@ -742,17 +742,17 @@ class JournalEntry(AccountsController): return remarks = [] - if cheque_remark := self._cheque_remark(): + if cheque_remark := self._get_cheque_remark(): remarks.append(cheque_remark) for row in self.get("accounts"): - if reference_remark := self._reference_remark(row): + if reference_remark := self._get_reference_remark(row): remarks.append(reference_remark) if remarks: self.remark = "\n".join(remarks) # User Remarks is not mandatory - def _cheque_remark(self) -> str | None: + def _get_cheque_remark(self) -> str | None: """Remark line for the cheque reference; raises if the cheque date is missing.""" if not self.cheque_no: return None @@ -760,7 +760,7 @@ class JournalEntry(AccountsController): msgprint(_("Please enter Reference date"), raise_exception=frappe.MandatoryError) return _("Reference #{0} dated {1}").format(self.cheque_no, formatdate(self.cheque_date)) - def _reference_remark(self, row) -> str | None: + def _get_reference_remark(self, row) -> str | None: """Remark line for a single account row's linked Invoice/Order, or None.""" if row.reference_type == "Sales Invoice" and row.credit: return _("{0} against Sales Invoice {1}").format( @@ -771,14 +771,14 @@ class JournalEntry(AccountsController): fmt_money(flt(row.credit), currency=self.company_currency), row.reference_name ) if row.reference_type == "Purchase Invoice" and row.debit: - return self._bill_remark(row) + return self._get_bill_remark(row) if row.reference_type == "Purchase Order" and row.debit: return _("{0} against Purchase Order {1}").format( fmt_money(flt(row.credit), currency=self.company_currency), row.reference_name ) return None - def _bill_remark(self, row) -> str | None: + def _get_bill_remark(self, row) -> str | None: """Remark line referencing the supplier bill number/date of a Purchase Invoice row.""" bill_no, bill_date = frappe.db.get_value( "Purchase Invoice", row.reference_name, ["bill_no", "bill_date"] @@ -791,37 +791,47 @@ class JournalEntry(AccountsController): bill_date and formatdate(bill_date.strftime("%Y-%m-%d")), ) - def set_print_format_fields(self): - bank_amount = party_amount = total_amount = 0.0 - currency = bank_account_currency = party_account_currency = pay_to_recd_from = None - party_type = None - for d in self.get("accounts"): - if d.party_type in ["Customer", "Supplier"] and d.party: - party_type = d.party_type - if not pay_to_recd_from: - pay_to_recd_from = d.party + def set_print_format_fields(self) -> None: + """Populate pay_to_recd_from and the total amount/currency shown on the print format.""" + amounts = self._get_party_and_bank_amounts() - if pay_to_recd_from and pay_to_recd_from == d.party: - party_amount += flt(d.debit_in_account_currency) or flt(d.credit_in_account_currency) - party_account_currency = d.account_currency - - elif frappe.get_cached_value("Account", d.account, "account_type") in ["Bank", "Cash"]: - bank_amount += flt(d.debit_in_account_currency) or flt(d.credit_in_account_currency) - bank_account_currency = d.account_currency - - if party_type and pay_to_recd_from: + total_amount, currency = 0.0, None + if amounts.party_type and amounts.pay_to_recd_from: self.pay_to_recd_from = frappe.db.get_value( - party_type, pay_to_recd_from, "customer_name" if party_type == "Customer" else "supplier_name" + amounts.party_type, + amounts.pay_to_recd_from, + "customer_name" if amounts.party_type == "Customer" else "supplier_name", ) - if bank_amount: - total_amount = bank_amount - currency = bank_account_currency + if amounts.bank_amount: + total_amount, currency = amounts.bank_amount, amounts.bank_account_currency else: - total_amount = party_amount - currency = party_account_currency + total_amount, currency = amounts.party_amount, amounts.party_account_currency self.set_total_amount(total_amount, currency) + def _get_party_and_bank_amounts(self) -> frappe._dict: + """Sum the party and bank/cash amounts, with their currencies, across the account rows.""" + totals = frappe._dict( + bank_amount=0.0, + party_amount=0.0, + bank_account_currency=None, + party_account_currency=None, + pay_to_recd_from=None, + party_type=None, + ) + for row in self.get("accounts"): + amount = flt(row.debit_in_account_currency) or flt(row.credit_in_account_currency) + if row.party_type in ["Customer", "Supplier"] and row.party: + totals.party_type = row.party_type + totals.pay_to_recd_from = totals.pay_to_recd_from or row.party + if totals.pay_to_recd_from == row.party: + totals.party_amount += amount + totals.party_account_currency = row.account_currency + elif frappe.get_cached_value("Account", row.account, "account_type") in ["Bank", "Cash"]: + totals.bank_amount += amount + totals.bank_account_currency = row.account_currency + return totals + def set_total_amount(self, amt: float, currency: str) -> None: self.total_amount = amt self.total_amount_currency = currency @@ -859,70 +869,81 @@ class JournalEntry(AccountsController): @frappe.whitelist() def get_balance(self, difference_account: str | None = None) -> None: + """Balance the entry by placing any difference on a blank (or newly added) row.""" if not self.get("accounts"): msgprint(_("'Entries' cannot be empty"), raise_exception=True) - else: - self.total_debit, self.total_credit = 0, 0 - diff = flt(self.difference, self.precision("difference")) + return - # If any row without amount, set the diff on that row - if diff: - blank_row = None - for d in self.get("accounts"): - if not d.credit_in_account_currency and not d.debit_in_account_currency and diff != 0: - blank_row = d + self.total_debit, self.total_credit = 0, 0 + diff = flt(self.difference, self.precision("difference")) + if diff: + self._apply_difference_to_blank_row(diff, difference_account) - if not blank_row: - blank_row = self.append( - "accounts", - { - "account": difference_account, - "cost_center": erpnext.get_default_cost_center(self.company), - }, - ) + self.set_total_debit_credit() + self.validate_total_debit_and_credit() - blank_row.exchange_rate = 1 - if diff > 0: - blank_row.credit_in_account_currency = diff - blank_row.credit = diff - elif diff < 0: - blank_row.debit_in_account_currency = abs(diff) - blank_row.debit = abs(diff) + def _apply_difference_to_blank_row(self, diff: float, difference_account: str | None) -> None: + """Set the balancing difference on the last amountless row, adding one if none exists.""" + blank_row = None + for row in self.get("accounts"): + if not row.credit_in_account_currency and not row.debit_in_account_currency: + blank_row = row - self.set_total_debit_credit() - self.validate_total_debit_and_credit() + if not blank_row: + blank_row = self.append( + "accounts", + { + "account": difference_account, + "cost_center": erpnext.get_default_cost_center(self.company), + }, + ) + + blank_row.exchange_rate = 1 + if diff > 0: + blank_row.credit_in_account_currency = diff + blank_row.credit = diff + elif diff < 0: + blank_row.debit_in_account_currency = abs(diff) + blank_row.debit = abs(diff) @frappe.whitelist() - def get_outstanding_invoices(self): + def get_outstanding_invoices(self) -> None: + """Populate the entry with a write-off row per outstanding invoice plus a balancing row.""" self.set("accounts", []) total = 0 - for d in self.get_values(): - total += flt(d.outstanding_amount, self.precision("credit", "accounts")) - jd1 = self.append("accounts", {}) - jd1.account = d.account - jd1.party = d.party + for invoice in self.get_values(): + total += flt(invoice.outstanding_amount, self.precision("credit", "accounts")) + self._append_outstanding_invoice_row(invoice) - if self.write_off_based_on == "Accounts Receivable": - jd1.party_type = "Customer" - jd1.credit_in_account_currency = flt( - d.outstanding_amount, self.precision("credit", "accounts") - ) - jd1.reference_type = "Sales Invoice" - jd1.reference_name = cstr(d.name) - elif self.write_off_based_on == "Accounts Payable": - jd1.party_type = "Supplier" - jd1.debit_in_account_currency = flt(d.outstanding_amount, self.precision("debit", "accounts")) - jd1.reference_type = "Purchase Invoice" - jd1.reference_name = cstr(d.name) - - jd2 = self.append("accounts", {}) + balancing_row = self.append("accounts", {}) if self.write_off_based_on == "Accounts Receivable": - jd2.debit_in_account_currency = total + balancing_row.debit_in_account_currency = total elif self.write_off_based_on == "Accounts Payable": - jd2.credit_in_account_currency = total + balancing_row.credit_in_account_currency = total self.validate_total_debit_and_credit() + def _append_outstanding_invoice_row(self, invoice) -> None: + """Append a party row for a single outstanding invoice per the write-off basis.""" + row = self.append("accounts", {}) + row.account = invoice.account + row.party = invoice.party + + if self.write_off_based_on == "Accounts Receivable": + row.party_type = "Customer" + row.credit_in_account_currency = flt( + invoice.outstanding_amount, self.precision("credit", "accounts") + ) + row.reference_type = "Sales Invoice" + row.reference_name = cstr(invoice.name) + elif self.write_off_based_on == "Accounts Payable": + row.party_type = "Supplier" + row.debit_in_account_currency = flt( + invoice.outstanding_amount, self.precision("debit", "accounts") + ) + row.reference_type = "Purchase Invoice" + row.reference_name = cstr(invoice.name) + def get_values(self): if self.write_off_based_on == "Accounts Receivable": doctype, account_field, party_field = "Sales Invoice", "debit_to", "customer" From bcc1e7396231bdd64c71f3daefa18e6c30aac470 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 22:44:19 +0530 Subject: [PATCH 5/7] docs(journal_entry): add class and public-method docstrings Add a class docstring plus docstrings for the lifecycle hooks and the public API helpers (get_outstanding, get_against_jv, get_exchange_rate, etc.). Self-evident one-line methods are intentionally left undocumented. --- .../doctype/journal_entry/journal_entry.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index ae3be120ce6..a18657d6b31 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -44,6 +44,14 @@ class StockAccountInvalidTransaction(frappe.ValidationError): class JournalEntry(AccountsController): + """Double-entry accounting voucher for manual and system-generated postings. + + Besides plain journal entries it also backs depreciation, asset disposal, + exchange gain/loss, deferred revenue/expense, inter-company and periodic + accounting entries: it validates the account rows (party, references, + currency) and posts the corresponding GL entries on submit. + """ + # begin: auto-generated types # This code is auto-generated. Do not modify anything in this block. @@ -129,6 +137,7 @@ class JournalEntry(AccountsController): super().__init__(*args, **kwargs) def validate(self): + """Validate the account rows (party, references, currency, stock) and build derived fields.""" from erpnext.accounts.doctype.journal_entry.services.asset_service import AssetService from erpnext.accounts.doctype.journal_entry.services.reference_validator import ( JournalEntryReferenceValidator, @@ -189,28 +198,33 @@ class JournalEntry(AccountsController): validate_docs_for_deferred_accounting([self.name], []) def submit(self): + """Submit inline, or queue submission in the background for large entries.""" if len(self.accounts) > 100 and not self.meta.queue_in_background: queue_submission(self, "_submit") else: return self._submit() def before_cancel(self): + """Block cancellation when a submitted Asset Value Adjustment is linked to this entry.""" from erpnext.accounts.doctype.journal_entry.services.asset_service import AssetService AssetService(self).has_asset_adjustment_entry() def cancel(self): + """Cancel inline, or queue cancellation in the background for large entries.""" if len(self.accounts) > 100: queue_submission(self, "_cancel") else: return self._cancel() def before_submit(self): + """Ensure total debit equals total credit before submission (skipped on data import).""" # Do not validate while importing via data import if not frappe.flags.in_import: self.validate_total_debit_and_credit() def on_submit(self): + """Post GL entries and propagate the submission to assets, inter-company JE and invoice discounting.""" from erpnext.accounts.doctype.journal_entry.services.asset_service import AssetService self.validate_cheque_info() @@ -304,6 +318,7 @@ class JournalEntry(AccountsController): self.repost_accounting_entries() def on_cancel(self): + """Reverse GL entries and unlink asset, inter-company and advance references on cancel.""" # Cancel tax withholding entries from erpnext.accounts.doctype.journal_entry.services.asset_service import AssetService @@ -1055,6 +1070,7 @@ def get_against_jv( page_len: int, filters: dict, ) -> list: + """Link-field search for submitted Journal Entries having an unreferenced row on an account.""" if not frappe.db.has_column("Journal Entry", searchfield): return [] @@ -1086,6 +1102,7 @@ def get_against_jv( @frappe.whitelist() def get_outstanding(args: str | dict) -> dict: + """Return the outstanding amount and side to set when referencing a JV / Invoice.""" if not frappe.has_permission("Account"): frappe.msgprint(_("No Permission"), raise_exception=1) @@ -1150,6 +1167,7 @@ def get_outstanding(args: str | dict) -> dict: @frappe.whitelist() def get_party_account_and_currency(company: str, party_type: str, party: str) -> dict: + """Return the receivable/payable account for a party and its account currency.""" if not frappe.has_permission("Account"): frappe.msgprint(_("No Permission"), raise_exception=1) @@ -1228,6 +1246,7 @@ def get_exchange_rate( credit: float | str | None = None, exchange_rate: str | float | None = None, ) -> float: + """Resolve the exchange rate for an account row, by reference, balance or settings.""" # Ensure exchange_rate is always numeric to avoid calculation errors if isinstance(exchange_rate, str): exchange_rate = flt(exchange_rate) or 1 @@ -1264,6 +1283,7 @@ def get_exchange_rate( @frappe.whitelist() def get_average_exchange_rate(account: str) -> float: + """Implied exchange rate from an account's company-currency vs account-currency balance.""" exchange_rate = 0 bank_balance_in_account_currency = get_balance_on(account) if bank_balance_in_account_currency: From cc8ce03232422c40a030fbc20bd1452dc472e84a Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 23:07:03 +0530 Subject: [PATCH 6/7] test(journal_entry): cover write-off, balance and advance-unlink flows; drop dead code Add characterization tests for the previously untested get_balance (difference on a blank row), get_outstanding_invoices (write-off rows) and unlink_advance_entry_reference (reference cleared on cancel). Remove the unused get_average_exchange_rate, which has no callers in erpnext. --- .../doctype/journal_entry/journal_entry.py | 12 ---- .../journal_entry/test_journal_entry.py | 61 +++++++++++++++++++ 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index a18657d6b31..dde87dcbf36 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -1279,15 +1279,3 @@ def get_exchange_rate( # don't return None or 0 as it is multipled with a value and that value could be lost return exchange_rate or 1 - - -@frappe.whitelist() -def get_average_exchange_rate(account: str) -> float: - """Implied exchange rate from an account's company-currency vs account-currency balance.""" - exchange_rate = 0 - bank_balance_in_account_currency = get_balance_on(account) - if bank_balance_in_account_currency: - bank_balance_in_company_currency = get_balance_on(account, in_account_currency=False) - exchange_rate = bank_balance_in_company_currency / bank_balance_in_account_currency - - return exchange_rate diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index 4590c5cd0b4..b6e9805316a 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -688,6 +688,67 @@ class TestJournalEntry(ERPNextTestSuite): self.assertEqual(jv.reference_types[invoice.name], "Sales Invoice") self.assertEqual(jv.reference_accounts[invoice.name], "Debtors - _TC") + def test_get_balance_places_difference_on_blank_row(self): + """Characterize: get_balance puts the unbalanced difference on an amountless row.""" + jv = frappe.new_doc("Journal Entry") + jv.company = "_Test Company" + jv.posting_date = nowdate() + jv.append( + "accounts", + { + "account": "_Test Cash - _TC", + "debit_in_account_currency": 100, + "debit": 100, + "exchange_rate": 1, + }, + ) + jv.append("accounts", {"account": "_Test Bank - _TC", "exchange_rate": 1}) # amountless row + jv.set_total_debit_credit() + self.assertEqual(jv.difference, 100) + + jv.get_balance() + blank_row = jv.accounts[1] + self.assertEqual(blank_row.credit_in_account_currency, 100) + self.assertEqual(jv.total_debit, jv.total_credit) + + def test_get_outstanding_invoices_builds_write_off_rows(self): + """Characterize: get_outstanding_invoices adds a party row for each outstanding invoice.""" + from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice + + invoice = create_sales_invoice(rate=700) + jv = frappe.new_doc("Journal Entry") + jv.company = "_Test Company" + jv.posting_date = nowdate() + jv.voucher_type = "Write Off Entry" + jv.write_off_based_on = "Accounts Receivable" + jv.write_off_amount = 1000 + jv.get_outstanding_invoices() + + invoice_rows = [row for row in jv.accounts if row.reference_name == invoice.name] + self.assertTrue(invoice_rows) + self.assertEqual(invoice_rows[0].party_type, "Customer") + self.assertEqual(invoice_rows[0].reference_type, "Sales Invoice") + self.assertEqual(flt(invoice_rows[0].credit_in_account_currency), 700) + + def test_unlink_advance_entry_reference_on_cancel(self): + """Characterize: cancelling an advance JE against an invoice clears the row's reference.""" + from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice + + invoice = create_sales_invoice(rate=700) + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + advance_row = jv.accounts[1] + advance_row.party_type = "Customer" + advance_row.party = "_Test Customer" + advance_row.is_advance = "Yes" + advance_row.reference_type = "Sales Invoice" + advance_row.reference_name = invoice.name + jv.submit() + + jv.cancel() + jv.reload() + self.assertFalse(jv.accounts[1].reference_type) + self.assertFalse(jv.accounts[1].reference_name) + def make_journal_entry( account1, From f099dbad353661ef38cbffbf23ab46b36eddc8e0 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 23:28:12 +0530 Subject: [PATCH 7/7] refactor(journal_entry): give get_outstanding an explicit parameter list Replace the single opaque `args` parameter of the whitelisted get_outstanding with explicit named parameters (the supported interface), splitting the body into _get_journal_entry_outstanding / _get_invoice_outstanding. The legacy `args` payload is still accepted via kwargs for backward compatibility with custom apps. Resolves the overusing-args semgrep finding. --- .../doctype/journal_entry/journal_entry.js | 18 ++- .../doctype/journal_entry/journal_entry.py | 130 +++++++++++------- 2 files changed, 85 insertions(+), 63 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.js b/erpnext/accounts/doctype/journal_entry/journal_entry.js index 1293a18ca0b..472f35fdc79 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.js +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.js @@ -409,18 +409,16 @@ erpnext.accounts.JournalEntry = class JournalEntry extends frappe.ui.form.Contro } get_outstanding(doctype, docname, company, child) { - var args = { - doctype: doctype, - docname: docname, - party: child.party, - account: child.account, - account_currency: child.account_currency, - company: company, - }; - return frappe.call({ method: "erpnext.accounts.doctype.journal_entry.journal_entry.get_outstanding", - args: { args: args }, + args: { + doctype: doctype, + docname: docname, + company: company, + account: child.account, + party: child.party, + account_currency: child.account_currency, + }, callback: function (r) { if (r.message) { $.each(r.message, function (field, value) { diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index dde87dcbf36..70e6164beb2 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -1101,68 +1101,92 @@ def get_against_jv( @frappe.whitelist() -def get_outstanding(args: str | dict) -> dict: - """Return the outstanding amount and side to set when referencing a JV / Invoice.""" +def get_outstanding( + doctype: str | None = None, + docname: str | None = None, + company: str | None = None, + account: str | None = None, + party: str | None = None, + account_currency: str | None = None, + **kwargs, +) -> dict | None: + """Return the outstanding amount and side to set when referencing a JV / Invoice. + + The named parameters are the supported interface. The legacy `args` payload dict + (captured via kwargs) is still accepted for backward compatibility with callers, + including custom apps, and is unpacked into the named parameters below. + """ if not frappe.has_permission("Account"): frappe.msgprint(_("No Permission"), raise_exception=1) - if isinstance(args, str): - args = json.loads(args) + if legacy_payload := kwargs.get("args"): + if isinstance(legacy_payload, str): + legacy_payload = json.loads(legacy_payload) + doctype = legacy_payload.get("doctype") + docname = legacy_payload.get("docname") + company = legacy_payload.get("company") + account = legacy_payload.get("account") + party = legacy_payload.get("party") + account_currency = legacy_payload.get("account_currency") - company_currency = erpnext.get_company_currency(args.get("company")) - due_date = None + if doctype == "Journal Entry": + return _get_journal_entry_outstanding(docname, account, party) - if args.get("doctype") == "Journal Entry": - jea = frappe.qb.DocType("Journal Entry Account") - query = ( - frappe.qb.from_(jea) - .select(Sum(jea.debit_in_account_currency) - Sum(jea.credit_in_account_currency)) - .where( - (jea.parent == args.get("docname")) - & (jea.account == args.get("account")) - & (jea.reference_type.isnull() | (jea.reference_type == "")) - ) + if doctype in ("Sales Invoice", "Purchase Invoice"): + return _get_invoice_outstanding(doctype, docname, company, account_currency) + + +def _get_journal_entry_outstanding(docname: str, account: str | None, party: str | None) -> dict: + """Unreferenced debit-minus-credit balance for an account on a Journal Entry.""" + jea = frappe.qb.DocType("Journal Entry Account") + query = ( + frappe.qb.from_(jea) + .select(Sum(jea.debit_in_account_currency) - Sum(jea.credit_in_account_currency)) + .where( + (jea.parent == docname) + & (jea.account == account) + & (jea.reference_type.isnull() | (jea.reference_type == "")) ) - if args.get("party"): - query = query.where(jea.party == args.get("party")) + ) + if party: + query = query.where(jea.party == party) - against_jv_amount = query.run() - against_jv_amount = flt(against_jv_amount[0][0]) if against_jv_amount else 0 - amount_field = "credit_in_account_currency" if against_jv_amount > 0 else "debit_in_account_currency" - return {amount_field: abs(against_jv_amount)} - elif args.get("doctype") in ("Sales Invoice", "Purchase Invoice"): - party_type = "Customer" if args.get("doctype") == "Sales Invoice" else "Supplier" - invoice = frappe.db.get_value( - args["doctype"], - args["docname"], - ["outstanding_amount", "conversion_rate", scrub(party_type), "due_date"], - as_dict=1, + result = query.run() + balance = flt(result[0][0]) if result else 0 + amount_field = "credit_in_account_currency" if balance > 0 else "debit_in_account_currency" + return {amount_field: abs(balance)} + + +def _get_invoice_outstanding(doctype: str, docname: str, company: str, account_currency: str | None) -> dict: + """Outstanding amount, side, party and exchange rate for a Sales/Purchase Invoice.""" + party_type = "Customer" if doctype == "Sales Invoice" else "Supplier" + invoice = frappe.db.get_value( + doctype, + docname, + ["outstanding_amount", "conversion_rate", scrub(party_type), "due_date"], + as_dict=1, + ) + + company_currency = erpnext.get_company_currency(company) + exchange_rate = invoice.conversion_rate if account_currency != company_currency else 1 + + outstanding_is_positive = flt(invoice.outstanding_amount) > 0 + if doctype == "Sales Invoice": + amount_field = ( + "credit_in_account_currency" if outstanding_is_positive else "debit_in_account_currency" + ) + else: + amount_field = ( + "debit_in_account_currency" if outstanding_is_positive else "credit_in_account_currency" ) - due_date = invoice.get("due_date") - - exchange_rate = invoice.conversion_rate if (args.get("account_currency") != company_currency) else 1 - - if args["doctype"] == "Sales Invoice": - amount_field = ( - "credit_in_account_currency" - if flt(invoice.outstanding_amount) > 0 - else "debit_in_account_currency" - ) - else: - amount_field = ( - "debit_in_account_currency" - if flt(invoice.outstanding_amount) > 0 - else "credit_in_account_currency" - ) - - return { - amount_field: abs(flt(invoice.outstanding_amount)), - "exchange_rate": exchange_rate, - "party_type": party_type, - "party": invoice.get(scrub(party_type)), - "reference_due_date": due_date, - } + return { + amount_field: abs(flt(invoice.outstanding_amount)), + "exchange_rate": exchange_rate, + "party_type": party_type, + "party": invoice.get(scrub(party_type)), + "reference_due_date": invoice.get("due_date"), + } @frappe.whitelist()