From 13e0a211ae70766ffddbcd7d13db49704ed62311 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 20 May 2026 00:48:29 +0530 Subject: [PATCH] fix: prevent negative amounts in common party JE on return invoices (#55034) Co-authored-by: Claude Sonnet 4.6 --- .../purchase_invoice/test_purchase_invoice.py | 46 ++++++++++++++++ .../sales_invoice/test_sales_invoice.py | 46 ++++++++++++++++ erpnext/controllers/accounts_controller.py | 54 ++++++++++--------- 3 files changed, 121 insertions(+), 25 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index 5cf3c6be879..509120acce3 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -2962,6 +2962,52 @@ class TestPurchaseInvoice(ERPNextTestSuite, StockTestMixin): pr = make_purchase_receipt_from_pi(pi.name) self.assertFalse(pr.items) + @ERPNextTestSuite.change_settings("Accounts Settings", {"enable_common_party_accounting": True}) + def test_purchase_invoice_return_common_party_je_has_no_negative_amounts(self): + from erpnext.accounts.doctype.opening_invoice_creation_tool.test_opening_invoice_creation_tool import ( + make_customer, + ) + from erpnext.accounts.doctype.party_link.party_link import create_party_link + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + customer = make_customer(customer="_Test Common Party Return PI") + supplier = create_supplier(supplier_name="_Test Common Party Return PI").name + # Supplier must be secondary so get_common_party_link finds it via the PI's party_type + party_link = create_party_link("Customer", customer, supplier) + + pi = make_purchase_invoice(supplier=supplier, parent_cost_center="_Test Cost Center - _TC") + + return_pi = make_return_doc(pi.doctype, pi.name) + return_pi.submit() + + # JE for the return should credit the supplier (secondary/reconciliation) account + # and debit the customer (primary) account — all positive amounts + jv_accounts = frappe.get_all( + "Journal Entry Account", + filters={"reference_type": return_pi.doctype, "reference_name": return_pi.name, "docstatus": 1}, + fields=["debit_in_account_currency", "credit_in_account_currency", "account"], + ) + + self.assertTrue(jv_accounts, "Expected a Journal Entry for the return invoice") + for row in jv_accounts: + self.assertGreaterEqual( + row.debit_in_account_currency, + 0, + f"Negative debit on account {row.account}", + ) + self.assertGreaterEqual( + row.credit_in_account_currency, + 0, + f"Negative credit on account {row.account}", + ) + + # Supplier (secondary) account must be credited, not debited + supplier_row = next(r for r in jv_accounts if r.account == pi.credit_to) + self.assertGreater(supplier_row.credit_in_account_currency, 0) + self.assertEqual(supplier_row.debit_in_account_currency, 0) + + party_link.delete() + def set_advance_flag(company, flag, default_account): frappe.db.set_value( diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 504f354522c..b2e4ea875d0 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3319,6 +3319,52 @@ class TestSalesInvoice(ERPNextTestSuite): party_link.delete() frappe.db.set_single_value("Accounts Settings", "enable_common_party_accounting", 0) + @ERPNextTestSuite.change_settings("Accounts Settings", {"enable_common_party_accounting": True}) + def test_sales_invoice_return_common_party_je_has_no_negative_amounts(self): + from erpnext.accounts.doctype.opening_invoice_creation_tool.test_opening_invoice_creation_tool import ( + make_customer, + ) + from erpnext.accounts.doctype.party_link.party_link import create_party_link + from erpnext.buying.doctype.supplier.test_supplier import create_supplier + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + customer = make_customer(customer="_Test Common Party Return SI") + supplier = create_supplier(supplier_name="_Test Common Party Return SI").name + party_link = create_party_link("Supplier", supplier, customer) + + si = create_sales_invoice(customer=customer, parent_cost_center="_Test Cost Center - _TC") + + return_si = make_return_doc(si.doctype, si.name) + return_si.submit() + + # JE for the return should credit the supplier (primary/advance) account + # and debit the customer (secondary/reconciliation) account — all positive amounts + jv_accounts = frappe.get_all( + "Journal Entry Account", + filters={"reference_type": return_si.doctype, "reference_name": return_si.name, "docstatus": 1}, + fields=["debit_in_account_currency", "credit_in_account_currency", "account"], + ) + + self.assertTrue(jv_accounts, "Expected a Journal Entry for the return invoice") + for row in jv_accounts: + self.assertGreaterEqual( + row.debit_in_account_currency, + 0, + f"Negative debit on account {row.account}", + ) + self.assertGreaterEqual( + row.credit_in_account_currency, + 0, + f"Negative credit on account {row.account}", + ) + + # Customer (secondary) account must be debited, not credited + customer_row = next(r for r in jv_accounts if r.account == return_si.debit_to) + self.assertGreater(customer_row.debit_in_account_currency, 0) + self.assertEqual(customer_row.credit_in_account_currency, 0) + + party_link.delete() + def test_payment_statuses(self): from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 6ab1c54fea8..36dd0ddb88c 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -2899,7 +2899,9 @@ class AccountsController(TransactionBase): advance_entry.party_type = primary_party_type advance_entry.party = primary_party advance_entry.cost_center = self.cost_center or erpnext.get_default_cost_center(self.company) - advance_entry.is_advance = "Yes" + # For returns the direction is reversed, so this entry cannot be an advance + # (JE validation: Supplier advance must be debit, Customer advance must be credit) + advance_entry.is_advance = "No" if self.is_return else "Yes" # Update dimensions dimensions_dict = frappe._dict() @@ -2931,35 +2933,26 @@ class AccountsController(TransactionBase): ) ) - # Convert outstanding amount from secondary to primary account currency, if needed + outstanding_amount = abs(self.outstanding_amount) + os_in_default_currency = outstanding_amount * exc_rate_secondary_to_default + os_in_primary_currency = outstanding_amount * exc_rate_secondary_to_primary - os_in_default_currency = self.outstanding_amount * exc_rate_secondary_to_default - os_in_primary_currency = self.outstanding_amount * exc_rate_secondary_to_primary + # SI normal and PI return → reconciliation is credit; SI return and PI normal → debit + reconciliation_is_credit = (self.doctype == "Sales Invoice") != bool(self.is_return) + _set_je_amounts( + reconcilation_entry, outstanding_amount, os_in_default_currency, reconciliation_is_credit + ) + _set_je_amounts( + advance_entry, os_in_primary_currency, os_in_default_currency, not reconciliation_is_credit + ) - if self.doctype == "Sales Invoice": - # Calculate credit and debit values for reconciliation and advance entries - reconcilation_entry.credit_in_account_currency = self.outstanding_amount - reconcilation_entry.credit = os_in_default_currency - - advance_entry.debit_in_account_currency = os_in_primary_currency - advance_entry.debit = os_in_default_currency - else: - advance_entry.credit_in_account_currency = os_in_primary_currency - advance_entry.credit = os_in_default_currency - - reconcilation_entry.debit_in_account_currency = self.outstanding_amount - reconcilation_entry.debit = os_in_default_currency - - # Set exchange rates for entries reconcilation_entry.exchange_rate = exc_rate_secondary_to_default advance_entry.exchange_rate = exc_rate_primary_to_default else: - if self.doctype == "Sales Invoice": - reconcilation_entry.credit_in_account_currency = self.outstanding_amount - advance_entry.debit_in_account_currency = self.outstanding_amount - else: - advance_entry.credit_in_account_currency = self.outstanding_amount - reconcilation_entry.debit_in_account_currency = self.outstanding_amount + outstanding_amount = abs(self.outstanding_amount) + reconciliation_is_credit = (self.doctype == "Sales Invoice") != bool(self.is_return) + _set_je_amounts(reconcilation_entry, outstanding_amount, is_credit=reconciliation_is_credit) + _set_je_amounts(advance_entry, outstanding_amount, is_credit=not reconciliation_is_credit) jv.multi_currency = multi_currency jv.append("accounts", reconcilation_entry) @@ -3699,6 +3692,17 @@ def set_child_tax_template_and_map(item, child_item, parent_doc): ) +def _set_je_amounts(entry, amount, default_amount=None, is_credit=True): + if is_credit: + entry.credit_in_account_currency = amount + if default_amount is not None: + entry.credit = default_amount + else: + entry.debit_in_account_currency = amount + if default_amount is not None: + entry.debit = default_amount + + def add_taxes_from_tax_template(child_item, parent_doc, db_insert=True): add_taxes_from_item_tax_template = frappe.get_single_value( "Accounts Settings", "add_taxes_from_item_tax_template"