Merge pull request #61620 from aerele/fix/dunning-payment-validation

fix(accounts): validate dunning payments and lifecycle

erpnext/accounts/doctype/dunning/dunning.json

@@ -234,7 +234,7 @@
   },
   {
    "allow_on_submit": 1,
-   "default": "Unresolved",
+   "default": "Draft",
    "fieldname": "status",
    "fieldtype": "Select",
    "in_standard_filter": 1,
@@ -400,7 +400,7 @@
  "icon": "bell-ring",
  "is_submittable": 1,
  "links": [],
- "modified": "2026-10-22 11:40:47.166878",
+ "modified": "2026-10-26 11:25:50.077318",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Dunning",

erpnext/accounts/doctype/dunning/dunning.py

@@ -21,6 +21,7 @@ from frappe.query_builder.functions import Sum
 from frappe.utils import flt, getdate
 
 from erpnext.controllers.accounts_controller import AccountsController
+from erpnext.setup.utils import get_exchange_rate
 
 
 class Dunning(AccountsController):
@@ -71,13 +72,41 @@ class Dunning(AccountsController):
 	# end: auto-generated types
 
 	def validate(self):
+		self.validate_currency()
 		self.set_dunning_fee()
 		self.validate_overdue_payment_references()
 		self.validate_overdue_payments()
 		self.validate_totals()
+		if self.docstatus == 0:
+			self.status = "Draft"
 		self.set_party_details()
 		self.set_dunning_level()
 
+	def validate_currency(self):
+		self.currency = self.currency or self.company_currency
+		if self.currency == self.company_currency:
+			self.conversion_rate = 1
+		elif not self.conversion_rate or (
+			self.get_doc_before_save()
+			and not self.has_value_changed("conversion_rate")
+			and any(self.has_value_changed(field) for field in ("currency", "company", "posting_date"))
+		):
+			self.conversion_rate = get_exchange_rate(
+				self.currency, self.company_currency, self.posting_date, "for_selling"
+			)
+
+		if flt(self.conversion_rate) <= 0:
+			frappe.throw(
+				_("Conversion Rate from {0} to {1} must be greater than zero.").format(
+					self.currency, self.company_currency
+				)
+			)
+
+	def before_submit(self):
+		if self.dunning_amount and not self.income_account:
+			frappe.throw(_("Income Account is required to collect dunning fees or interest."))
+		self.status = "Unresolved"
+
 	def set_dunning_fee(self):
 		if not self.dunning_type:
 			self.dunning_fee = flt(self.dunning_fee)
@@ -302,6 +331,10 @@ class Dunning(AccountsController):
 			if (outstanding := get_current_outstanding(row)) > 0
 		]
 
+	def before_cancel(self):
+		super().before_cancel()
+		self.status = "Cancelled"
+
 	def on_cancel(self):
 		super().on_cancel()
 		self.ignore_linked_doctypes = [
@@ -316,7 +349,6 @@ class Dunning(AccountsController):
 			"Unreconcile Payment Entries",
 			"Payment Ledger Entry",
 			"Serial and Batch Bundle",
-			"Payment Entry",
 		]
 
 	@frappe.whitelist()
@@ -462,13 +494,25 @@ def get_paid_dunning_amount(dunning: str) -> float:
 	"""Interest and fee collected for this dunning, in company currency."""
 	deduction = frappe.qb.DocType("Payment Entry Deduction")
 	payment_entry = frappe.qb.DocType("Payment Entry")
+	dunning_doc = frappe.qb.DocType("Dunning")
 
 	paid = (
 		frappe.qb.from_(deduction)
 		.join(payment_entry)
 		.on(payment_entry.name == deduction.parent)
+		.join(dunning_doc)
+		.on(dunning_doc.name == deduction.dunning)
 		.select(Sum(deduction.amount))
-		.where((deduction.dunning == dunning) & (payment_entry.docstatus == 1))
+		.where(
+			(deduction.dunning == dunning)
+			& (payment_entry.docstatus == 1)
+			& (payment_entry.payment_type == "Receive")
+			& (payment_entry.party_type == "Customer")
+			& (payment_entry.party == dunning_doc.customer)
+			& (payment_entry.company == dunning_doc.company)
+			& (deduction.account == dunning_doc.income_account)
+			& (deduction.amount < 0)
+		)
 	).run()
 
 	# the dunning amount is booked as a negative deduction, against the income account
@@ -511,7 +555,7 @@ def get_linked_dunnings_as_per_state(sales_invoice, state):
 		.distinct()
 		.where(
 			(dunning.status == state)
-			& (dunning.docstatus != 2)
+			& (dunning.docstatus == 1)
 			& (overdue_payment.sales_invoice == sales_invoice)
 		)
 	).run(as_dict=True)

erpnext/accounts/doctype/dunning/test_dunning.py

@@ -1,6 +1,7 @@
 # Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors
 # See license.txt
 import json
+from unittest.mock import patch
 
 import frappe
 from frappe.model import mapper
@@ -48,15 +49,39 @@ class TestDunning(ERPNextTestSuite):
 				self.assertEqual(dunning.dunning_fee, 1 if fee is None else fee)
 				self.assertEqual(dunning.base_dunning_amount, dunning.dunning_amount * 100)
 
-	def test_missing_fee_requires_positive_conversion_rate(self):
+	def test_missing_fee_uses_company_currency_conversion_rate(self):
 		dunning = create_dunning(overdue_days=15, dunning_type_name="Second Notice - _TC")
-		for rate in (0, -1):
+		for rate in (None, 0, -1):
 			with self.subTest(rate=rate):
 				dunning.reload()
 				dunning.dunning_fee = None
 				dunning.conversion_rate = rate
-				with self.assertRaisesRegex(frappe.ValidationError, "Conversion Rate must be greater"):
-					dunning.save()
+				dunning.save()
+				self.assertEqual(dunning.conversion_rate, 1)
+				self.assertEqual(dunning.dunning_fee, 10)
+				self.assertEqual(dunning.base_dunning_amount, dunning.dunning_amount)
+
+	@ERPNextTestSuite.change_settings(
+		"Accounts Settings", {"allow_multi_currency_invoices_against_single_party_account": 1}
+	)
+	def test_missing_fee_requires_positive_foreign_conversion_rate(self):
+		create_dunning_type("API Fee Notice", 100, 0, 0)
+		invoice = create_sales_invoice(
+			posting_date=add_days(today(), -15),
+			currency="USD",
+			conversion_rate=80,
+			rate=100,
+			debit_to="Debtors - _TC",
+		)
+		for rate in (None, 0, -1):
+			with self.subTest(rate=rate):
+				dunning = create_dunning_from_sales_invoice(invoice.name)
+				dunning.dunning_type = "API Fee Notice - _TC"
+				dunning.dunning_fee = None
+				dunning.conversion_rate = rate
+				with patch("erpnext.accounts.doctype.dunning.dunning.get_exchange_rate", return_value=0):
+					with self.assertRaisesRegex(frappe.ValidationError, "must be greater than zero"):
+						dunning.save()
 
 	def test_unallocated_payments_reduce_oldest_installments(self):
 		create_payment_terms_template_for_dunning()
@@ -196,11 +221,8 @@ class TestDunning(ERPNextTestSuite):
 		self.assertEqual(dunning.status, "Unresolved")
 		self.assertEqual(round(dunning.get_unpaid_dunning_amount(), 2), 10.41)
 
-	def test_dunning_can_be_cancelled_after_its_interest_was_paid(self):
-		"""
-		The payment collecting the interest links back to the dunning, which must not stand in
-		the way of cancelling it.
-		"""
+	def test_paid_dunning_requires_payment_cancellation(self):
+		"""Cancel the linked payment before cancelling a Dunning with paid charges."""
 		dunning = create_dunning(overdue_days=15, dunning_type_name="Second Notice - _TC")
 		dunning.submit()
 
@@ -212,8 +234,18 @@ class TestDunning(ERPNextTestSuite):
 		dunning.reload()
 		self.assertEqual(dunning.status, "Resolved")
 
+		frappe.db.savepoint("dunning_cancellation")
+		with self.assertRaises(frappe.LinkExistsError):
+			dunning.cancel()
+		frappe.db.rollback(save_point="dunning_cancellation")
+
+		pe.reload()
+		pe.cancel()
+		dunning.reload()
 		dunning.cancel()
+		dunning.reload()
 		self.assertEqual(dunning.docstatus, 2)
+		self.assertEqual(dunning.status, "Cancelled")
 
 	def test_waived_interest_keeps_a_manually_resolved_dunning_resolved(self):
 		"""

erpnext/accounts/doctype/payment_entry/payment_entry.py

@@ -186,6 +186,7 @@ class PaymentEntry(AccountsController):
 		self.set_exchange_rate()
 		self.validate_mandatory()
 		self.validate_reference_documents()
+		self.validate_dunning_deductions()
 		self.set_amounts()
 		self.validate_amounts()
 		self.apply_taxes()
@@ -203,6 +204,54 @@ class PaymentEntry(AccountsController):
 		self.set_status()
 		self.set_total_in_words()
 
+	def validate_dunning_deductions(self):
+		dunning_names = {row.dunning for row in self.deductions if row.dunning}
+		if not dunning_names:
+			return
+
+		dunnings = {
+			row.name: row
+			for row in frappe.get_all(
+				"Dunning",
+				filters={"name": ("in", list(dunning_names))},
+				fields=["name", "docstatus", "customer", "company", "income_account"],
+			)
+		}
+		for row in self.deductions:
+			if not row.dunning:
+				continue
+
+			dunning = dunnings.get(row.dunning)
+			if not dunning or dunning.docstatus != 1:
+				frappe.throw(_("Row #{0}: Dunning {1} must be submitted.").format(row.idx, row.dunning))
+
+			if self.payment_type != "Receive" or self.party_type != "Customer":
+				frappe.throw(
+					_("Row #{0}: Dunning charges require a Receive payment from a Customer.").format(row.idx)
+				)
+
+			if self.party != dunning.customer:
+				frappe.throw(
+					_("Row #{0}: Dunning {1} belongs to a different customer.").format(row.idx, row.dunning)
+				)
+
+			if self.company != dunning.company:
+				frappe.throw(
+					_("Row #{0}: Dunning {1} belongs to a different company.").format(row.idx, row.dunning)
+				)
+
+			if row.account != dunning.income_account:
+				frappe.throw(
+					_(
+						"Row #{0}: Dunning charges must use the income account {1} configured on Dunning {2}."
+					).format(row.idx, dunning.income_account, row.dunning)
+				)
+
+			if flt(row.amount) >= 0:
+				frappe.throw(
+					_("Row #{0}: Dunning charges must have a negative deduction amount.").format(row.idx)
+				)
+
 	def before_save(self):
 		self.set_matched_unset_payment_requests_to_response()
 

erpnext/patches.txt

@@ -556,3 +556,5 @@ erpnext.patches.v16_0.credit_job_card_transfers_to_their_operation
 erpnext.patches.v16_0.set_lost_on_in_opportunity
 erpnext.patches.v16_0.apply_selling_settings_section_toggles
 erpnext.patches.v16_0.set_output_qty_in_work_order_operations
+execute:frappe.db.set_value("Dunning", {"docstatus": 0}, "status", "Draft", update_modified=False)
+execute:frappe.db.set_value("Dunning", {"docstatus": 2}, "status", "Cancelled", update_modified=False)