From 9503dd0c7f3464cc0777342c383b6d21be36d6c3 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 23:18:22 +0530 Subject: [PATCH 1/3] test(journal_entry): characterize validate_reference_doc branches Pin every branch of validate_reference_doc before refactoring: Sales Order debit / Purchase Order credit rejection, non-existent reference handling, Sales/Purchase Invoice and Order party mismatches, and population of the reference_totals/types/accounts side effects. --- .../journal_entry/test_journal_entry.py | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index b823a44391d..b53a2ce353e 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -609,6 +609,85 @@ class TestJournalEntry(ERPNextTestSuite): jv.save() self.assertRaises(frappe.ValidationError, jv.submit) + def test_validate_reference_doc_debit_against_sales_order_throws(self): + """Characterize: a debit entry linked to a Sales Order is rejected.""" + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + sales_order = make_sales_order() + jv = make_journal_entry("Debtors - _TC", "_Test Cash - _TC", 100, save=False) + jv.accounts[0].party_type = "Customer" + jv.accounts[0].party = "_Test Customer" + jv.accounts[0].reference_type = "Sales Order" + jv.accounts[0].reference_name = sales_order.name + self.assertRaisesRegex(frappe.ValidationError, "Debit entry can not be linked", jv.insert) + + def test_validate_reference_doc_credit_against_purchase_order_throws(self): + """Characterize: a credit entry linked to a Purchase Order is rejected.""" + from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order + + purchase_order = create_purchase_order() + jv = make_journal_entry("_Test Cash - _TC", "Creditors - _TC", 100, save=False) + jv.accounts[1].party_type = "Supplier" + jv.accounts[1].party = "_Test Supplier" + jv.accounts[1].reference_type = "Purchase Order" + jv.accounts[1].reference_name = purchase_order.name + self.assertRaisesRegex(frappe.ValidationError, "Credit entry can not be linked", jv.insert) + + def test_validate_reference_doc_nonexistent_reference_rejected(self): + """Characterize: a JE referencing a non-existent invoice is rejected by link validation. + + Note: the controller's own "Invalid reference" branch is unreachable in normal flow + because Frappe link validation rejects the missing reference before validate_reference_doc. + """ + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + jv.accounts[1].party_type = "Customer" + jv.accounts[1].party = "_Test Customer" + jv.accounts[1].reference_type = "Sales Invoice" + jv.accounts[1].reference_name = "NON-EXISTENT-SI" + self.assertRaises(frappe.LinkValidationError, jv.insert) + + def test_validate_reference_doc_invoice_party_mismatch_throws(self): + """Characterize: an invoice reference whose party differs from the row party is rejected.""" + from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice + + invoice = create_sales_invoice(rate=500) + other_customer = make_customer("_Test JE Mismatch Customer") + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + jv.accounts[1].party_type = "Customer" + jv.accounts[1].party = other_customer + jv.accounts[1].reference_type = "Sales Invoice" + jv.accounts[1].reference_name = invoice.name + self.assertRaisesRegex(frappe.ValidationError, "Party / Account does not match", jv.insert) + + def test_validate_reference_doc_order_party_mismatch_throws(self): + """Characterize: a Sales Order reference whose party differs from the row party is rejected.""" + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + sales_order = make_sales_order() + other_customer = make_customer("_Test JE Mismatch Customer") + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + jv.accounts[1].party_type = "Customer" + jv.accounts[1].party = other_customer + jv.accounts[1].is_advance = "Yes" + jv.accounts[1].reference_type = "Sales Order" + jv.accounts[1].reference_name = sales_order.name + self.assertRaisesRegex(frappe.ValidationError, "does not match", jv.insert) + + def test_validate_reference_doc_populates_reference_side_effects(self): + """Characterize: a valid invoice reference populates reference_totals/types/accounts.""" + from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice + + invoice = create_sales_invoice(rate=500) + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + jv.accounts[1].party_type = "Customer" + jv.accounts[1].party = "_Test Customer" + jv.accounts[1].reference_type = "Sales Invoice" + jv.accounts[1].reference_name = invoice.name + jv.insert() + self.assertEqual(jv.reference_totals[invoice.name], 100.0) + self.assertEqual(jv.reference_types[invoice.name], "Sales Invoice") + self.assertEqual(jv.reference_accounts[invoice.name], "Debtors - _TC") + def make_journal_entry( account1, From 49093b326e77ff0f17d015f9ea52179e30d85826 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 8 Jun 2026 23:18:52 +0530 Subject: [PATCH 2/3] refactor(journal_entry): split validate_reference_doc into per-row methods Extract the 100-line, CC-27 validate_reference_doc into a thin orchestrator loop plus focused per-row private methods, and lift the inline reference field map to a module constant. Behaviour preserved; complexity drops from 27 to 3 and no extracted function exceeds 15 lines. --- .../doctype/journal_entry/journal_entry.py | 185 +++++++++--------- 1 file changed, 94 insertions(+), 91 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 24cce464be8..a44cf28e9e2 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -34,6 +34,13 @@ from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_sched from erpnext.controllers.accounts_controller import AccountsController from erpnext.setup.utils import get_exchange_rate as _get_exchange_rate +REFERENCE_PARTY_ACCOUNT_FIELDS = { + "Sales Invoice": ["Customer", "Debit To"], + "Purchase Invoice": ["Supplier", "Credit To"], + "Sales Order": ["Customer"], + "Purchase Order": ["Supplier"], +} + class StockAccountInvalidTransaction(frappe.ValidationError): pass @@ -743,105 +750,101 @@ class JournalEntry(AccountsController): def validate_reference_doc(self): """Validates reference document""" - field_dict = { - "Sales Invoice": ["Customer", "Debit To"], - "Purchase Invoice": ["Supplier", "Credit To"], - "Sales Order": ["Customer"], - "Purchase Order": ["Supplier"], - } - self.reference_totals = {} self.reference_types = {} self.reference_accounts = {} - for d in self.get("accounts"): - if not d.reference_type: - d.reference_name = None - if not d.reference_name: - d.reference_type = None - if d.reference_type and d.reference_name and (d.reference_type in list(field_dict)): - dr_or_cr = ( - "credit_in_account_currency" - if d.reference_type in ("Sales Order", "Sales Invoice") - else "debit_in_account_currency" - ) - - # check debit or credit type Sales / Purchase Order - if d.reference_type == "Sales Order" and flt(d.debit) > 0: - frappe.throw( - _("Row {0}: Debit entry can not be linked with a {1}").format(d.idx, d.reference_type) - ) - - if d.reference_type == "Purchase Order" and flt(d.credit) > 0: - frappe.throw( - _("Row {0}: Credit entry can not be linked with a {1}").format( - d.idx, d.reference_type - ) - ) - - # set totals - if d.reference_name not in self.reference_totals: - self.reference_totals[d.reference_name] = 0.0 - - if self.voucher_type not in ("Deferred Revenue", "Deferred Expense"): - self.reference_totals[d.reference_name] += flt(d.get(dr_or_cr)) - - self.reference_types[d.reference_name] = d.reference_type - self.reference_accounts[d.reference_name] = d.account - - against_voucher = frappe.db.get_value( - d.reference_type, d.reference_name, [scrub(dt) for dt in field_dict.get(d.reference_type)] - ) - - if not against_voucher: - frappe.throw(_("Row {0}: Invalid reference {1}").format(d.idx, d.reference_name)) - - # check if party and account match - if d.reference_type in ("Sales Invoice", "Purchase Invoice"): - if ( - self.voucher_type in ("Deferred Revenue", "Deferred Expense") - and d.reference_detail_no - ): - debit_or_credit = "Debit" if d.debit else "Credit" - party_account = get_deferred_booking_accounts( - d.reference_type, d.reference_detail_no, debit_or_credit - ) - against_voucher = ["", against_voucher[1]] - else: - if d.reference_type == "Sales Invoice": - party_account = ( - get_party_account_based_on_invoice_discounting(d.reference_name) - or against_voucher[1] - ) - else: - party_account = against_voucher[1] - - if ( - against_voucher[0] != cstr(d.party) or party_account != d.account - ) and self.voucher_type != "Exchange Gain Or Loss": - frappe.throw( - _("Row {0}: Party / Account does not match with {1} / {2} in {3} {4}").format( - d.idx, - field_dict.get(d.reference_type)[0], - field_dict.get(d.reference_type)[1], - d.reference_type, - d.reference_name, - ) - ) - - # check if party matches for Sales / Purchase Order - if d.reference_type in ("Sales Order", "Purchase Order"): - # set totals - if against_voucher != d.party: - frappe.throw( - _("Row {0}: {1} {2} does not match with {3}").format( - d.idx, d.party_type, d.party, d.reference_type - ) - ) + self._normalize_reference_fields(d) + if not self._is_validatable_reference(d): + continue + self._validate_order_direction(d) + self._accumulate_reference(d) + self._validate_reference_party_and_account(d) self.validate_orders() self.validate_invoices() + def _normalize_reference_fields(self, row): + if not row.reference_type: + row.reference_name = None + if not row.reference_name: + row.reference_type = None + + def _is_validatable_reference(self, row): + return bool( + row.reference_type and row.reference_name and row.reference_type in REFERENCE_PARTY_ACCOUNT_FIELDS + ) + + def _reference_dr_or_cr(self, row): + 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): + 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) + ) + if row.reference_type == "Purchase Order" and flt(row.credit) > 0: + frappe.throw( + _("Row {0}: Credit entry can not be linked with a {1}").format(row.idx, row.reference_type) + ) + + def _accumulate_reference(self, row): + if row.reference_name not in self.reference_totals: + self.reference_totals[row.reference_name] = 0.0 + if self.voucher_type not in ("Deferred Revenue", "Deferred Expense"): + self.reference_totals[row.reference_name] += flt(row.get(self._reference_dr_or_cr(row))) + self.reference_types[row.reference_name] = row.reference_type + self.reference_accounts[row.reference_name] = row.account + + def _validate_reference_party_and_account(self, row): + 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] + ) + if not against_voucher: + frappe.throw(_("Row {0}: Invalid reference {1}").format(row.idx, row.reference_name)) + + if row.reference_type in ("Sales Invoice", "Purchase Invoice"): + self._validate_invoice_party_and_account(row, against_voucher, party_fields) + 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): + party_account, against_party = self._resolve_invoice_party_account(row, against_voucher) + if self.voucher_type == "Exchange Gain Or Loss": + return + if against_party != cstr(row.party) or party_account != row.account: + frappe.throw( + _("Row {0}: Party / Account does not match with {1} / {2} in {3} {4}").format( + row.idx, party_fields[0], party_fields[1], row.reference_type, row.reference_name + ) + ) + + def _resolve_invoice_party_account(self, row, against_voucher): + if self.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( + row.reference_type, row.reference_detail_no, debit_or_credit + ) + return party_account, "" + if row.reference_type == "Sales Invoice": + party_account = ( + get_party_account_based_on_invoice_discounting(row.reference_name) or against_voucher[1] + ) + else: + party_account = against_voucher[1] + return party_account, against_voucher[0] + + def _validate_order_party(self, row, against_voucher): + if against_voucher != row.party: + frappe.throw( + _("Row {0}: {1} {2} does not match with {3}").format( + row.idx, row.party_type, row.party, row.reference_type + ) + ) + def validate_orders(self): """Validate totals, closed and docstatus for orders""" for reference_name, total in self.reference_totals.items(): From 5753c23ccf8af4a332aaa74120588b8e228a10dd Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 9 Jun 2026 15:28:12 +0530 Subject: [PATCH 3/3] refactor(journal_entry): clarify reference helper names Rename three private helpers for intent and to drop an abbreviation: _is_validatable_reference -> _has_party_reference, _accumulate_reference -> _register_reference, _reference_dr_or_cr -> _reference_amount_field. --- .../accounts/doctype/journal_entry/journal_entry.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index a44cf28e9e2..6b9b21dd0ba 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -755,10 +755,10 @@ class JournalEntry(AccountsController): self.reference_accounts = {} for d in self.get("accounts"): self._normalize_reference_fields(d) - if not self._is_validatable_reference(d): + if not self._has_party_reference(d): continue self._validate_order_direction(d) - self._accumulate_reference(d) + self._register_reference(d) self._validate_reference_party_and_account(d) self.validate_orders() @@ -770,12 +770,12 @@ class JournalEntry(AccountsController): if not row.reference_name: row.reference_type = None - def _is_validatable_reference(self, row): + def _has_party_reference(self, row): return bool( row.reference_type and row.reference_name and row.reference_type in REFERENCE_PARTY_ACCOUNT_FIELDS ) - def _reference_dr_or_cr(self, row): + def _reference_amount_field(self, row): if row.reference_type in ("Sales Order", "Sales Invoice"): return "credit_in_account_currency" return "debit_in_account_currency" @@ -790,11 +790,11 @@ class JournalEntry(AccountsController): _("Row {0}: Credit entry can not be linked with a {1}").format(row.idx, row.reference_type) ) - def _accumulate_reference(self, row): + def _register_reference(self, row): if row.reference_name not in self.reference_totals: self.reference_totals[row.reference_name] = 0.0 if self.voucher_type not in ("Deferred Revenue", "Deferred Expense"): - self.reference_totals[row.reference_name] += flt(row.get(self._reference_dr_or_cr(row))) + self.reference_totals[row.reference_name] += flt(row.get(self._reference_amount_field(row))) self.reference_types[row.reference_name] = row.reference_type self.reference_accounts[row.reference_name] = row.account