mirror of
https://github.com/frappe/erpnext.git
synced 2026-05-22 14:39:19 +00:00
fix: prevent negative amounts in common party JE on return invoices (#55034)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -2962,6 +2962,52 @@ class TestPurchaseInvoice(ERPNextTestSuite, StockTestMixin):
|
|||||||
pr = make_purchase_receipt_from_pi(pi.name)
|
pr = make_purchase_receipt_from_pi(pi.name)
|
||||||
self.assertFalse(pr.items)
|
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):
|
def set_advance_flag(company, flag, default_account):
|
||||||
frappe.db.set_value(
|
frappe.db.set_value(
|
||||||
|
|||||||
@@ -3319,6 +3319,52 @@ class TestSalesInvoice(ERPNextTestSuite):
|
|||||||
party_link.delete()
|
party_link.delete()
|
||||||
frappe.db.set_single_value("Accounts Settings", "enable_common_party_accounting", 0)
|
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):
|
def test_payment_statuses(self):
|
||||||
from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry
|
from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry
|
||||||
|
|
||||||
|
|||||||
@@ -2899,7 +2899,9 @@ class AccountsController(TransactionBase):
|
|||||||
advance_entry.party_type = primary_party_type
|
advance_entry.party_type = primary_party_type
|
||||||
advance_entry.party = primary_party
|
advance_entry.party = primary_party
|
||||||
advance_entry.cost_center = self.cost_center or erpnext.get_default_cost_center(self.company)
|
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
|
# Update dimensions
|
||||||
dimensions_dict = frappe._dict()
|
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
|
# SI normal and PI return → reconciliation is credit; SI return and PI normal → debit
|
||||||
os_in_primary_currency = self.outstanding_amount * exc_rate_secondary_to_primary
|
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
|
reconcilation_entry.exchange_rate = exc_rate_secondary_to_default
|
||||||
advance_entry.exchange_rate = exc_rate_primary_to_default
|
advance_entry.exchange_rate = exc_rate_primary_to_default
|
||||||
else:
|
else:
|
||||||
if self.doctype == "Sales Invoice":
|
outstanding_amount = abs(self.outstanding_amount)
|
||||||
reconcilation_entry.credit_in_account_currency = self.outstanding_amount
|
reconciliation_is_credit = (self.doctype == "Sales Invoice") != bool(self.is_return)
|
||||||
advance_entry.debit_in_account_currency = self.outstanding_amount
|
_set_je_amounts(reconcilation_entry, outstanding_amount, is_credit=reconciliation_is_credit)
|
||||||
else:
|
_set_je_amounts(advance_entry, outstanding_amount, is_credit=not reconciliation_is_credit)
|
||||||
advance_entry.credit_in_account_currency = self.outstanding_amount
|
|
||||||
reconcilation_entry.debit_in_account_currency = self.outstanding_amount
|
|
||||||
|
|
||||||
jv.multi_currency = multi_currency
|
jv.multi_currency = multi_currency
|
||||||
jv.append("accounts", reconcilation_entry)
|
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):
|
def add_taxes_from_tax_template(child_item, parent_doc, db_insert=True):
|
||||||
add_taxes_from_item_tax_template = frappe.get_single_value(
|
add_taxes_from_item_tax_template = frappe.get_single_value(
|
||||||
"Accounts Settings", "add_taxes_from_item_tax_template"
|
"Accounts Settings", "add_taxes_from_item_tax_template"
|
||||||
|
|||||||
Reference in New Issue
Block a user