From 1714e13b398d96a2e32be77df8a7b1c3a5614f71 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 23:42:05 +0530 Subject: [PATCH 1/5] refactor(journal_entry): tidy reference-validator and GL-composer services Add return type hints and option-A docstrings to JournalEntryReferenceValidator, and split JournalEntryGLComposer.compose into _set_transaction_currency and _gl_row helpers. Behaviour preserved. --- .../journal_entry/services/gl_composer.py | 162 +++++++++--------- .../services/reference_validator.py | 44 +++-- 2 files changed, 108 insertions(+), 98 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/services/gl_composer.py b/erpnext/accounts/doctype/journal_entry/services/gl_composer.py index 16b78eae3b9..277276b4c51 100644 --- a/erpnext/accounts/doctype/journal_entry/services/gl_composer.py +++ b/erpnext/accounts/doctype/journal_entry/services/gl_composer.py @@ -18,86 +18,88 @@ class JournalEntryGLComposer(BaseGLComposer): from the first foreign-currency row (mirroring the former build_gl_map). """ - def compose(self): - doc = self.doc - gl_map = [] - - company_currency = erpnext.get_company_currency(doc.company) - doc.transaction_currency = company_currency - doc.transaction_exchange_rate = 1 - if doc.multi_currency: - for row in doc.get("accounts"): - if row.account_currency != company_currency: - # Journal assumes the first foreign currency as transaction currency - doc.transaction_currency = row.account_currency - doc.transaction_exchange_rate = row.exchange_rate - break - + def compose(self) -> list: + """Project the Journal Entry's non-zero account rows into GL dicts.""" + self._set_transaction_currency() advance_doctypes = get_advance_payment_doctypes() - for d in doc.get("accounts"): - if d.debit or d.credit or (doc.voucher_type == "Exchange Gain Or Loss"): - r = [d.user_remark, doc.remark] - r = [x for x in r if x] - remarks = "\n".join(r) - - row = { - "account": d.account, - "party_type": d.party_type, - "due_date": doc.due_date, - "party": d.party, - "against": d.against_account, - "debit": flt(d.debit, d.precision("debit")), - "credit": flt(d.credit, d.precision("credit")), - "account_currency": d.account_currency, - "debit_in_account_currency": flt( - d.debit_in_account_currency, d.precision("debit_in_account_currency") - ), - "credit_in_account_currency": flt( - d.credit_in_account_currency, d.precision("credit_in_account_currency") - ), - "transaction_currency": doc.transaction_currency, - "transaction_exchange_rate": doc.transaction_exchange_rate, - "debit_in_transaction_currency": flt( - d.debit_in_account_currency, d.precision("debit_in_account_currency") - ) - if doc.transaction_currency == d.account_currency - else flt(d.debit, d.precision("debit")) / doc.transaction_exchange_rate, - "credit_in_transaction_currency": flt( - d.credit_in_account_currency, d.precision("credit_in_account_currency") - ) - if doc.transaction_currency == d.account_currency - else flt(d.credit, d.precision("credit")) / doc.transaction_exchange_rate, - "against_voucher_type": d.reference_type, - "against_voucher": d.reference_name, - "remarks": remarks, - "voucher_detail_no": d.reference_detail_no, - "cost_center": d.cost_center, - "project": d.project, - "finance_book": doc.finance_book, - "advance_voucher_type": d.advance_voucher_type, - "advance_voucher_no": d.advance_voucher_no, - } - - if d.reference_type in advance_doctypes: - row.update( - { - "against_voucher_type": doc.doctype, - "against_voucher": doc.name, - "advance_voucher_type": d.reference_type, - "advance_voucher_no": d.reference_name, - } - ) - - # set flag to skip party validation - account_type = frappe.get_cached_value("Account", d.account, "account_type") - if account_type in ["Receivable", "Payable"] and doc.party_not_required: - frappe.flags.party_not_required = True - - gl_map.append( - self.get_gl_dict( - row, - item=d, - ) - ) + gl_map = [] + for d in self.doc.get("accounts"): + if d.debit or d.credit or self.doc.voucher_type == "Exchange Gain Or Loss": + gl_map.append(self.get_gl_dict(self._gl_row(d, advance_doctypes), item=d)) return gl_map + + def _set_transaction_currency(self) -> None: + """Company currency, or the first foreign-currency row, becomes the transaction currency.""" + doc = self.doc + doc.transaction_currency = erpnext.get_company_currency(doc.company) + doc.transaction_exchange_rate = 1 + if not doc.multi_currency: + return + + for row in doc.get("accounts"): + if row.account_currency != doc.transaction_currency: + # Journal assumes the first foreign currency as transaction currency + doc.transaction_currency = row.account_currency + doc.transaction_exchange_rate = row.exchange_rate + break + + def _gl_row(self, d, advance_doctypes: list) -> dict: + """Build the GL dict for a single account row.""" + doc = self.doc + remarks = "\n".join(x for x in [d.user_remark, doc.remark] if x) + + row = { + "account": d.account, + "party_type": d.party_type, + "due_date": doc.due_date, + "party": d.party, + "against": d.against_account, + "debit": flt(d.debit, d.precision("debit")), + "credit": flt(d.credit, d.precision("credit")), + "account_currency": d.account_currency, + "debit_in_account_currency": flt( + d.debit_in_account_currency, d.precision("debit_in_account_currency") + ), + "credit_in_account_currency": flt( + d.credit_in_account_currency, d.precision("credit_in_account_currency") + ), + "transaction_currency": doc.transaction_currency, + "transaction_exchange_rate": doc.transaction_exchange_rate, + "debit_in_transaction_currency": flt( + d.debit_in_account_currency, d.precision("debit_in_account_currency") + ) + if doc.transaction_currency == d.account_currency + else flt(d.debit, d.precision("debit")) / doc.transaction_exchange_rate, + "credit_in_transaction_currency": flt( + d.credit_in_account_currency, d.precision("credit_in_account_currency") + ) + if doc.transaction_currency == d.account_currency + else flt(d.credit, d.precision("credit")) / doc.transaction_exchange_rate, + "against_voucher_type": d.reference_type, + "against_voucher": d.reference_name, + "remarks": remarks, + "voucher_detail_no": d.reference_detail_no, + "cost_center": d.cost_center, + "project": d.project, + "finance_book": doc.finance_book, + "advance_voucher_type": d.advance_voucher_type, + "advance_voucher_no": d.advance_voucher_no, + } + + if d.reference_type in advance_doctypes: + row.update( + { + "against_voucher_type": doc.doctype, + "against_voucher": doc.name, + "advance_voucher_type": d.reference_type, + "advance_voucher_no": d.reference_name, + } + ) + + # set flag to skip party validation + account_type = frappe.get_cached_value("Account", d.account, "account_type") + if account_type in ["Receivable", "Payable"] and doc.party_not_required: + frappe.flags.party_not_required = True + + return row diff --git a/erpnext/accounts/doctype/journal_entry/services/reference_validator.py b/erpnext/accounts/doctype/journal_entry/services/reference_validator.py index b6b25d4f4aa..1d75b171d08 100644 --- a/erpnext/accounts/doctype/journal_entry/services/reference_validator.py +++ b/erpnext/accounts/doctype/journal_entry/services/reference_validator.py @@ -29,10 +29,11 @@ class JournalEntryReferenceValidator: orders and invoices. """ - def __init__(self, doc): + def __init__(self, doc) -> None: self.doc = doc - def validate(self): + def validate(self) -> None: + """Validate every reference-bearing row, then the referenced orders and invoices.""" self.doc.reference_totals = {} self.doc.reference_types = {} self.doc.reference_accounts = {} @@ -47,23 +48,24 @@ class JournalEntryReferenceValidator: self._validate_orders() self._validate_invoices() - def _normalize_reference_fields(self, row): + def _normalize_reference_fields(self, row) -> None: if not row.reference_type: row.reference_name = None if not row.reference_name: row.reference_type = None - def _has_party_reference(self, row): + def _has_party_reference(self, row) -> bool: return bool( row.reference_type and row.reference_name and row.reference_type in REFERENCE_PARTY_ACCOUNT_FIELDS ) - def _reference_amount_field(self, row): + def _reference_amount_field(self, row) -> str: if row.reference_type in ("Sales Order", "Sales Invoice"): return "credit_in_account_currency" return "debit_in_account_currency" - def _validate_order_direction(self, row): + def _validate_order_direction(self, row) -> None: + """An order can only be linked on the side that records an advance.""" if row.reference_type == "Sales Order" and flt(row.debit) > 0: frappe.throw( _("Row {0}: Debit entry can not be linked with a {1}").format(row.idx, row.reference_type) @@ -73,7 +75,8 @@ class JournalEntryReferenceValidator: _("Row {0}: Credit entry can not be linked with a {1}").format(row.idx, row.reference_type) ) - def _register_reference(self, row): + def _register_reference(self, row) -> None: + """Aggregate the row's amount, type and account onto the per-reference lookups.""" if row.reference_name not in self.doc.reference_totals: self.doc.reference_totals[row.reference_name] = 0.0 if self.doc.voucher_type not in ("Deferred Revenue", "Deferred Expense"): @@ -81,7 +84,8 @@ class JournalEntryReferenceValidator: self.doc.reference_types[row.reference_name] = row.reference_type self.doc.reference_accounts[row.reference_name] = row.account - def _validate_reference_party_and_account(self, row): + def _validate_reference_party_and_account(self, row) -> None: + """Reject a missing reference, then check party/account against the linked document.""" party_fields = REFERENCE_PARTY_ACCOUNT_FIELDS[row.reference_type] against_voucher = frappe.db.get_value( row.reference_type, row.reference_name, [scrub(f) for f in party_fields] @@ -94,7 +98,7 @@ class JournalEntryReferenceValidator: elif row.reference_type in ("Sales Order", "Purchase Order"): self._validate_order_party(row, against_voucher) - def _validate_invoice_party_and_account(self, row, against_voucher, party_fields): + def _validate_invoice_party_and_account(self, row, against_voucher, party_fields) -> None: party_account, against_party = self._resolve_invoice_party_account(row, against_voucher) if self.doc.voucher_type == "Exchange Gain Or Loss": return @@ -105,7 +109,9 @@ class JournalEntryReferenceValidator: ) ) - def _resolve_invoice_party_account(self, row, against_voucher): + def _resolve_invoice_party_account(self, row, against_voucher) -> tuple: + """Expected (party_account, party) for an invoice row, honouring deferred booking + and invoice-discounting accounts.""" if self.doc.voucher_type in ("Deferred Revenue", "Deferred Expense") and row.reference_detail_no: debit_or_credit = "Debit" if row.debit else "Credit" party_account = get_deferred_booking_accounts( @@ -120,7 +126,7 @@ class JournalEntryReferenceValidator: party_account = against_voucher[1] return party_account, against_voucher[0] - def _validate_order_party(self, row, against_voucher): + def _validate_order_party(self, row, against_voucher) -> None: if against_voucher != row.party: frappe.throw( _("Row {0}: {1} {2} does not match with {3}").format( @@ -128,8 +134,8 @@ class JournalEntryReferenceValidator: ) ) - def _validate_orders(self): - """Validate totals, closed and docstatus for orders""" + def _validate_orders(self) -> None: + """Validate totals, closed and docstatus for referenced orders.""" for reference_name, total in self.doc.reference_totals.items(): reference_type = self.doc.reference_types[reference_name] account = self.doc.reference_accounts[reference_name] @@ -140,7 +146,7 @@ class JournalEntryReferenceValidator: self._validate_order_status(order, reference_type, reference_name) self._validate_order_advance_total(order, account, total, reference_type, reference_name) - def _validate_order_status(self, order, reference_type, reference_name): + def _validate_order_status(self, order, reference_type, reference_name) -> None: if order.docstatus != 1: frappe.throw(_("{0} {1} is not submitted").format(reference_type, reference_name)) if flt(order.per_billed) >= 100: @@ -148,7 +154,8 @@ class JournalEntryReferenceValidator: if cstr(order.status) == "Closed": frappe.throw(_("{0} {1} is closed").format(reference_type, reference_name)) - def _validate_order_advance_total(self, order, account, total, reference_type, reference_name): + def _validate_order_advance_total(self, order, account, total, reference_type, reference_name) -> None: + """The advance paid against an order cannot exceed its grand total.""" account_currency = get_account_currency(account) if account_currency == self.doc.company_currency: voucher_total = order.base_grand_total @@ -167,8 +174,8 @@ class JournalEntryReferenceValidator: ) ) - def _validate_invoices(self): - """Validate totals and docstatus for invoices""" + def _validate_invoices(self) -> None: + """Validate totals and docstatus for referenced invoices.""" if self.doc.voucher_type in ("Debit Note", "Credit Note"): return for reference_name, total in self.doc.reference_totals.items(): @@ -178,7 +185,8 @@ class JournalEntryReferenceValidator: invoice = frappe.get_doc(reference_type, reference_name) self._validate_invoice_outstanding(invoice, total, reference_type, reference_name) - def _validate_invoice_outstanding(self, invoice, total, reference_type, reference_name): + def _validate_invoice_outstanding(self, invoice, total, reference_type, reference_name) -> None: + """Payment booked against an invoice cannot exceed its outstanding amount.""" if invoice.docstatus != 1: frappe.throw(_("{0} {1} is not submitted").format(reference_type, reference_name)) From 34955380eefa187d7516a54d4a11d55e91a236fb Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 23:45:04 +0530 Subject: [PATCH 2/5] refactor(journal_entry): break up get_payment_entry and add types/docstrings to mapper Split get_payment_entry into _reference_exchange_rate / _append_party_row / _append_bank_row, and add return type hints and docstrings to all mapper document builders. Behaviour preserved. --- .../accounts/doctype/journal_entry/mapper.py | 101 +++++++++++------- 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/mapper.py b/erpnext/accounts/doctype/journal_entry/mapper.py index e9ac678987f..0d75e340ff2 100644 --- a/erpnext/accounts/doctype/journal_entry/mapper.py +++ b/erpnext/accounts/doctype/journal_entry/mapper.py @@ -24,7 +24,8 @@ def get_payment_entry_against_order( debit_in_account_currency: str | float | None = None, journal_entry: bool = False, bank_account: str | None = None, -): +) -> dict | Document: + """Build an advance-payment Journal Entry against an unbilled Sales/Purchase Order.""" ref_doc = frappe.get_doc(dt, dn) if flt(ref_doc.per_billed, 2) > 0: @@ -74,7 +75,8 @@ def get_payment_entry_against_invoice( debit_in_account_currency: str | None = None, journal_entry: bool = False, bank_account: str | None = None, -): +) -> dict | Document: + """Build a payment Journal Entry against a Sales/Purchase Invoice's outstanding amount.""" ref_doc = frappe.get_doc(dt, dn) if dt == "Sales Invoice": party_type = "Customer" @@ -110,32 +112,54 @@ def get_payment_entry_against_invoice( ) -def get_payment_entry(ref_doc, args): - from erpnext.accounts.doctype.journal_entry.journal_entry import ( - get_default_bank_cash_account, - get_exchange_rate, - ) +def get_payment_entry(ref_doc, args: dict) -> dict | Document: + """Build a Bank Entry Journal Entry paying `ref_doc`, with a party row and a bank row. + + Returns the Journal Entry document when `args["journal_entry"]` is truthy, otherwise its + dict (for client calls). + """ + je = frappe.new_doc("Journal Entry") + je.update({"voucher_type": "Bank Entry", "company": ref_doc.company, "remark": args.get("remarks")}) cost_center = ref_doc.get("cost_center") or frappe.get_cached_value( "Company", ref_doc.company, "cost_center" ) - exchange_rate = 1 - if args.get("party_account"): - # Modified to include the posting date for which the exchange rate is required. - # Assumed to be the posting date in the reference document - exchange_rate = get_exchange_rate( - ref_doc.get("posting_date") or ref_doc.get("transaction_date"), - args.get("party_account"), - args.get("party_account_currency"), - ref_doc.company, - ref_doc.doctype, - ref_doc.name, - ) + exchange_rate = _reference_exchange_rate(ref_doc, args) - je = frappe.new_doc("Journal Entry") - je.update({"voucher_type": "Bank Entry", "company": ref_doc.company, "remark": args.get("remarks")}) + party_row = _append_party_row(je, ref_doc, args, cost_center, exchange_rate) + bank_row = _append_bank_row(je, ref_doc, args, cost_center, exchange_rate) - party_row = je.append( + if party_row.account_currency != ref_doc.company_currency or ( + bank_row.account_currency and bank_row.account_currency != ref_doc.company_currency + ): + je.multi_currency = 1 + + je.set_amounts_in_company_currency() + je.set_total_debit_credit() + + return je if args.get("journal_entry") else je.as_dict() + + +def _reference_exchange_rate(ref_doc, args: dict) -> float: + """Exchange rate of the party account on the reference document's posting date.""" + if not args.get("party_account"): + return 1 + + from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate + + return get_exchange_rate( + ref_doc.get("posting_date") or ref_doc.get("transaction_date"), + args.get("party_account"), + args.get("party_account_currency"), + ref_doc.company, + ref_doc.doctype, + ref_doc.name, + ) + + +def _append_party_row(je, ref_doc, args: dict, cost_center, exchange_rate: float): + """Append the party (debtor/creditor) row that records the advance/payment.""" + return je.append( "accounts", { "account": args.get("party_account"), @@ -153,14 +177,19 @@ def get_payment_entry(ref_doc, args): }, ) - bank_row = je.append("accounts") - # Make it bank_details +def _append_bank_row(je, ref_doc, args: dict, cost_center, exchange_rate: float): + """Append the bank/cash row, defaulting the account and converting the amount to it.""" + from erpnext.accounts.doctype.journal_entry.journal_entry import ( + get_default_bank_cash_account, + get_exchange_rate, + ) + + bank_row = je.append("accounts") bank_account = get_default_bank_cash_account(ref_doc.company, "Bank", account=args.get("bank_account")) if bank_account: bank_row.update(bank_account) - # Modified to include the posting date for which the exchange rate is required. - # Assumed to be the posting date of the reference date + # posting date assumed to be the reference document's posting/transaction date bank_row.exchange_rate = get_exchange_rate( ref_doc.get("posting_date") or ref_doc.get("transaction_date"), bank_account["account"], @@ -171,26 +200,17 @@ def get_payment_entry(ref_doc, args): bank_row.cost_center = cost_center amount = args.get("debit_in_account_currency") or args.get("amount") - if bank_row.account_currency == args.get("party_account_currency"): bank_row.set(args.get("amount_field_bank"), amount) else: bank_row.set(args.get("amount_field_bank"), amount * exchange_rate) - # Multi currency check again - if party_row.account_currency != ref_doc.company_currency or ( - bank_row.account_currency and bank_row.account_currency != ref_doc.company_currency - ): - je.multi_currency = 1 - - je.set_amounts_in_company_currency() - je.set_total_debit_credit() - - return je if args.get("journal_entry") else je.as_dict() + return bank_row @frappe.whitelist() -def make_inter_company_journal_entry(name: str, voucher_type: str, company: str): +def make_inter_company_journal_entry(name: str, voucher_type: str, company: str) -> dict: + """Build the counterpart Journal Entry in another company, linked back to `name`.""" journal_entry = frappe.new_doc("Journal Entry") journal_entry.voucher_type = voucher_type journal_entry.company = company @@ -200,7 +220,8 @@ def make_inter_company_journal_entry(name: str, voucher_type: str, company: str) @frappe.whitelist() -def make_reverse_journal_entry(source_name: str, target_doc: str | Document | None = None): +def make_reverse_journal_entry(source_name: str, target_doc: str | Document | None = None) -> Document: + """Map a submitted Journal Entry to a reversing one (debits and credits swapped).""" existing_reverse = frappe.db.exists("Journal Entry", {"reversal_of": source_name, "docstatus": 1}) if existing_reverse: frappe.throw( @@ -211,7 +232,7 @@ def make_reverse_journal_entry(source_name: str, target_doc: str | Document | No from frappe.model.mapper import get_mapped_doc - def post_process(source, target): + def post_process(source, target) -> None: target.reversal_of = source.name doclist = get_mapped_doc( From 43bb6c5a42d8aa77610c561077c36756bc92e353 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 23:50:59 +0530 Subject: [PATCH 3/5] refactor(journal_entry): break up unlink_asset_reference and type/document asset service Split AssetService.unlink_asset_reference into _is_depreciation_asset_row / _reverse_asset_depreciation / _restore_scheduled_depreciation / _restore_finance_book_value / _block_scrap_journal_cancel, and add return type hints and docstrings across the service. Behaviour preserved (netted by the asset suite). --- .../journal_entry/services/asset_service.py | 135 ++++++++++-------- 1 file changed, 77 insertions(+), 58 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/services/asset_service.py b/erpnext/accounts/doctype/journal_entry/services/asset_service.py index a0a55060394..9e76c05f168 100644 --- a/erpnext/accounts/doctype/journal_entry/services/asset_service.py +++ b/erpnext/accounts/doctype/journal_entry/services/asset_service.py @@ -20,10 +20,11 @@ class AssetService: Journal Entries tied to asset scrapping or value adjustments. """ - def __init__(self, doc): + def __init__(self, doc) -> None: self.doc = doc - def validate_depr_account_and_depr_entry_voucher_type(self): + def validate_depr_account_and_depr_entry_voucher_type(self) -> None: + """A depreciation account requires voucher type Depreciation Entry and an Expense account.""" for d in self.doc.get("accounts"): if d.account_type == "Depreciation": if self.doc.voucher_type != "Depreciation Entry": @@ -34,7 +35,8 @@ class AssetService: if frappe.get_cached_value("Account", d.account, "root_type") != "Expense": frappe.throw(_("Account {0} should be of type Expense").format(d.account)) - def has_asset_adjustment_entry(self): + def has_asset_adjustment_entry(self) -> None: + """Block cancellation while a submitted Asset Value Adjustment links to this entry.""" if self.doc.flags.get("via_asset_value_adjustment"): return @@ -48,11 +50,13 @@ class AssetService: ).format(frappe.utils.get_link_to_form("Asset Value Adjustment", asset_value_adjustment)) ) - def update_asset_value(self): + def update_asset_value(self) -> None: + """Apply the entry's effect to its linked assets on submit (depreciation or disposal).""" self.update_asset_on_depreciation() self.update_asset_on_disposal() - def update_asset_on_depreciation(self): + def update_asset_on_depreciation(self) -> None: + """Reduce each depreciated asset's value and link the depreciation schedule row.""" if self.doc.voucher_type != "Depreciation Entry": return @@ -73,7 +77,8 @@ class AssetService: asset.set_status() asset.set_total_booked_depreciations() - def update_value_after_depreciation(self, asset, depr_amount): + def update_value_after_depreciation(self, asset, depr_amount: float) -> None: + """Subtract the depreciation amount from the asset's relevant finance book.""" fb_idx = 1 if self.doc.finance_book: for fb_row in asset.get("finance_books"): @@ -86,7 +91,8 @@ class AssetService: "Asset Finance Book", fb_row.name, "value_after_depreciation", fb_row.value_after_depreciation ) - def update_journal_entry_link_on_depr_schedule(self, asset, je_row): + def update_journal_entry_link_on_depr_schedule(self, asset, je_row) -> None: + """Stamp this entry onto the matching (date + amount) depreciation schedule row.""" depr_schedule = get_depr_schedule(asset.name, "Active", self.doc.finance_book) for d in depr_schedule or []: if ( @@ -96,7 +102,8 @@ class AssetService: ): frappe.db.set_value("Depreciation Schedule", d.name, "journal_entry", self.doc.name) - def update_asset_on_disposal(self): + def update_asset_on_disposal(self) -> None: + """Mark each referenced asset disposed (date + scrap entry) on an Asset Disposal.""" if self.doc.voucher_type == "Asset Disposal": disposed_assets = [] for d in self.doc.get("accounts"): @@ -117,62 +124,74 @@ class AssetService: asset_doc.set_status() disposed_assets.append(d.reference_name) - def unlink_asset_reference(self): + def unlink_asset_reference(self) -> None: + """On cancel, reverse depreciation links and block cancelling an asset-scrap entry.""" for d in self.doc.get("accounts"): - if ( - self.doc.voucher_type == "Depreciation Entry" - and d.reference_type == "Asset" - and d.reference_name - and frappe.get_cached_value("Account", d.account, "root_type") == "Expense" - and d.debit - ): - asset = frappe.get_doc("Asset", d.reference_name) - - if asset.calculate_depreciation: - je_found = False - - for fb_row in asset.get("finance_books"): - if je_found: - break - - depr_schedule = get_depr_schedule(asset.name, "Active", fb_row.finance_book) - - for s in depr_schedule or []: - if s.journal_entry == self.doc.name: - s.db_set("journal_entry", None) - - fb_row.value_after_depreciation += d.debit - fb_row.db_update() - - je_found = True - break - if not je_found: - fb_idx = 1 - if self.doc.finance_book: - for fb_row in asset.get("finance_books"): - if fb_row.finance_book == self.doc.finance_book: - fb_idx = fb_row.idx - break - - fb_row = asset.get("finance_books")[fb_idx - 1] - fb_row.value_after_depreciation += d.debit - fb_row.db_update() - asset.db_set("value_after_depreciation", asset.value_after_depreciation + d.debit) - asset.set_status() - asset.set_total_booked_depreciations() + if self._is_depreciation_asset_row(d): + self._reverse_asset_depreciation(d) elif ( self.doc.voucher_type == "Journal Entry" and d.reference_type == "Asset" and d.reference_name ): - journal_entry_for_scrap = frappe.db.get_value( - "Asset", d.reference_name, "journal_entry_for_scrap" - ) + self._block_scrap_journal_cancel(d) - if journal_entry_for_scrap == self.doc.name: - frappe.throw( - _("Journal Entry for Asset scrapping cannot be cancelled. Please restore the Asset.") - ) + def _is_depreciation_asset_row(self, d) -> bool: + return bool( + self.doc.voucher_type == "Depreciation Entry" + and d.reference_type == "Asset" + and d.reference_name + and frappe.get_cached_value("Account", d.account, "root_type") == "Expense" + and d.debit + ) - def unlink_asset_adjustment_entry(self): + def _reverse_asset_depreciation(self, d) -> None: + """Add the depreciation amount back to the asset and unlink its schedule row.""" + asset = frappe.get_doc("Asset", d.reference_name) + + if asset.calculate_depreciation and not self._restore_scheduled_depreciation(asset, d.debit): + self._restore_finance_book_value(asset, d.debit) + + asset.db_set("value_after_depreciation", asset.value_after_depreciation + d.debit) + asset.set_status() + asset.set_total_booked_depreciations() + + def _restore_scheduled_depreciation(self, asset, debit: float) -> bool: + """Unlink this entry from the depreciation schedule and credit back its finance book. + + Returns True if a matching scheduled depreciation was found. + """ + for fb_row in asset.get("finance_books"): + depr_schedule = get_depr_schedule(asset.name, "Active", fb_row.finance_book) + for s in depr_schedule or []: + if s.journal_entry == self.doc.name: + s.db_set("journal_entry", None) + fb_row.value_after_depreciation += debit + fb_row.db_update() + return True + return False + + def _restore_finance_book_value(self, asset, debit: float) -> None: + """Credit the depreciation amount back to the relevant finance book when no schedule matched.""" + fb_idx = 1 + if self.doc.finance_book: + for fb_row in asset.get("finance_books"): + if fb_row.finance_book == self.doc.finance_book: + fb_idx = fb_row.idx + break + + fb_row = asset.get("finance_books")[fb_idx - 1] + fb_row.value_after_depreciation += debit + fb_row.db_update() + + def _block_scrap_journal_cancel(self, d) -> None: + """Prevent cancelling a plain Journal Entry that is an asset's scrap voucher.""" + journal_entry_for_scrap = frappe.db.get_value("Asset", d.reference_name, "journal_entry_for_scrap") + if journal_entry_for_scrap == self.doc.name: + frappe.throw( + _("Journal Entry for Asset scrapping cannot be cancelled. Please restore the Asset.") + ) + + def unlink_asset_adjustment_entry(self) -> None: + """Detach this entry from any Asset Value Adjustment that referenced it.""" AssetValueAdjustment = frappe.qb.DocType("Asset Value Adjustment") ( frappe.qb.update(AssetValueAdjustment) From 4f55071eda5951e8bb1ca080425b3942202a7707 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 10 Jun 2026 00:03:26 +0530 Subject: [PATCH 4/5] test(journal_entry): cover the untested mapper builders Add characterization tests for get_payment_entry_against_order (the Sales/ Purchase Order advance path, previously untested) and make_inter_company_journal_entry (previously fully uncovered). --- .../journal_entry/test_journal_entry.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index b6e9805316a..19acb249fbb 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -749,6 +749,34 @@ class TestJournalEntry(ERPNextTestSuite): self.assertFalse(jv.accounts[1].reference_type) self.assertFalse(jv.accounts[1].reference_name) + def test_get_payment_entry_against_order_builds_advance_je(self): + """Characterize the mapper: an advance Bank Entry JE is built against an unbilled order.""" + from erpnext.accounts.doctype.journal_entry.mapper import get_payment_entry_against_order + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + sales_order = make_sales_order() + je = get_payment_entry_against_order("Sales Order", sales_order.name, journal_entry=True) + + self.assertEqual(je.voucher_type, "Bank Entry") + party_rows = [row for row in je.accounts if row.party_type == "Customer"] + self.assertTrue(party_rows) + self.assertEqual(party_rows[0].reference_type, "Sales Order") + self.assertEqual(party_rows[0].reference_name, sales_order.name) + self.assertEqual(party_rows[0].is_advance, "Yes") + + def test_make_inter_company_journal_entry_builds_linked_draft(self): + """Characterize the mapper: the counterpart JE carries the company and back-reference.""" + from erpnext.accounts.doctype.journal_entry.mapper import make_inter_company_journal_entry + + source = make_journal_entry("_Test Cash - _TC", "_Test Bank - _TC", 100, submit=True) + result = make_inter_company_journal_entry( + source.name, "Inter Company Journal Entry", "_Test Company 1" + ) + + self.assertEqual(result.get("voucher_type"), "Inter Company Journal Entry") + self.assertEqual(result.get("company"), "_Test Company 1") + self.assertEqual(result.get("inter_company_journal_entry_reference"), source.name) + def make_journal_entry( account1, From a4cff805f10ea20e9112cfa42e53c3389e7d71ce Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 10 Jun 2026 09:57:12 +0530 Subject: [PATCH 5/5] test(journal_entry): pin transaction-currency conversion in GL entries Mutation testing on gl_composer surfaced that the foreign-row debit/credit_in_transaction_currency conversion (amount / exchange_rate) was unverified -- a / vs * bug survived. Assert those fields in test_multi_currency and add a foreign-debit case so both conversion directions are now caught. --- .../journal_entry/test_journal_entry.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index 19acb249fbb..a646ae64d61 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -169,8 +169,11 @@ class TestJournalEntry(ERPNextTestSuite): "debit_in_account_currency", "credit", "credit_in_account_currency", + "debit_in_transaction_currency", + "credit_in_transaction_currency", ] + # Transaction currency is USD (first foreign row); the INR row is converted at 1/50. self.expected_gle = [ { "account": "_Test Bank - _TC", @@ -179,6 +182,8 @@ class TestJournalEntry(ERPNextTestSuite): "debit_in_account_currency": 0, "credit": 5000, "credit_in_account_currency": 5000, + "debit_in_transaction_currency": 0, + "credit_in_transaction_currency": 100, }, { "account": "_Test Bank USD - _TC", @@ -187,6 +192,8 @@ class TestJournalEntry(ERPNextTestSuite): "debit_in_account_currency": 100, "credit": 0, "credit_in_account_currency": 0, + "debit_in_transaction_currency": 100, + "credit_in_transaction_currency": 0, }, ] @@ -203,6 +210,52 @@ class TestJournalEntry(ERPNextTestSuite): self.assertFalse(gle) + def test_multi_currency_transaction_currency_on_foreign_debit(self): + """Pin debit_in_transaction_currency for a foreign-currency debit row. + + Transaction currency is USD (the first foreign row); the INR debit row must be + converted at 1/exchange_rate, so 5000 INR -> 100 USD. Guards the / vs * direction. + """ + jv = frappe.new_doc("Journal Entry") + jv.company = "_Test Company" + jv.posting_date = nowdate() + jv.multi_currency = 1 + jv.append( + "accounts", + { + "account": "_Test Bank USD - _TC", + "cost_center": "_Test Cost Center - _TC", + "credit_in_account_currency": 100, + "exchange_rate": 50, + }, + ) + jv.append( + "accounts", + { + "account": "_Test Bank - _TC", + "cost_center": "_Test Cost Center - _TC", + "debit_in_account_currency": 5000, + "exchange_rate": 1, + }, + ) + jv.submit() + + self.voucher_no = jv.name + self.fields = ["account", "debit_in_transaction_currency", "credit_in_transaction_currency"] + self.expected_gle = [ + { + "account": "_Test Bank - _TC", + "debit_in_transaction_currency": 100, + "credit_in_transaction_currency": 0, + }, + { + "account": "_Test Bank USD - _TC", + "debit_in_transaction_currency": 0, + "credit_in_transaction_currency": 100, + }, + ] + self.check_gl_entries() + def test_reverse_journal_entry(self): from erpnext.accounts.doctype.journal_entry.mapper import make_reverse_journal_entry