From 55619be732f0e6b03af995e7256fbf76e265c3c0 Mon Sep 17 00:00:00 2001 From: Jatin3128 Date: Mon, 27 Apr 2026 11:19:00 +0530 Subject: [PATCH] feat: pre-submit validation error for packed quantity mismatch --- .../accounts_settings/accounts_settings.json | 2 +- .../test/test_pre_submit_validation.py | 119 ++++++++++++------ erpnext/accounts/utils.py | 35 ++++-- 3 files changed, 104 insertions(+), 52 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index 7bbe5e9f9de..6501c716aa2 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -708,7 +708,7 @@ "label": "Fetch Payment Schedule In Payment Request" }, { - "fieldname": "repost_section", + "fieldname": "repost_section", "fieldtype": "Section Break", "label": "Repost" }, diff --git a/erpnext/accounts/test/test_pre_submit_validation.py b/erpnext/accounts/test/test_pre_submit_validation.py index 25ee3fc6279..9ade1318302 100644 --- a/erpnext/accounts/test/test_pre_submit_validation.py +++ b/erpnext/accounts/test/test_pre_submit_validation.py @@ -1,37 +1,31 @@ # Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +from unittest.mock import patch + import frappe -from erpnext.accounts.utils import _check_credit_limit_warn -from erpnext.selling.doctype.customer.test_customer import ( - get_customer_dict, - set_credit_limit, +from erpnext.accounts.utils import ( + _check_credit_limit_warn, + _check_packed_qty_warn, ) +from erpnext.selling.doctype.customer.test_customer import set_credit_limit from erpnext.tests.utils import ERPNextTestSuite -COMPANY = "_Test Company" -CREDIT_LIMIT = 100.0 -OVER = 200.0 -UNDER = 50.0 - - -def _make_customer(name): - if not frappe.db.exists("Customer", name): - frappe.get_doc({**get_customer_dict(name), "customer_name": name}).insert() - return name - def _get_orange_warnings(): return [m for m in frappe.message_log if m.get("indicator") == "orange"] class _CreditLimitBase(ERPNextTestSuite): - CUSTOMER = "_Pre Submit Test Customer" + COMPANY = "_Test Company" + CUSTOMER = "_Test Customer" + CREDIT_LIMIT = 100.0 + OVER = 200.0 + UNDER = 50.0 def setUp(self): - _make_customer(self.CUSTOMER) - set_credit_limit(self.CUSTOMER, COMPANY, CREDIT_LIMIT) + set_credit_limit(self.CUSTOMER, self.COMPANY, self.CREDIT_LIMIT) frappe.message_log.clear() @@ -39,7 +33,7 @@ class TestCreditLimitWarnSalesInvoice(_CreditLimitBase): def _make_si(self, amount, is_return=0): """Build an in-memory (unsaved) draft SI.""" si = frappe.new_doc("Sales Invoice") - si.company = COMPANY + si.company = self.COMPANY si.customer = self.CUSTOMER si.is_return = is_return si.base_grand_total = amount @@ -48,26 +42,26 @@ class TestCreditLimitWarnSalesInvoice(_CreditLimitBase): def test_warns_when_amount_exceeds_credit_limit(self): """Orange warning must appear when base_grand_total > credit_limit.""" - si = self._make_si(OVER) + si = self._make_si(self.OVER) _check_credit_limit_warn(si) self.assertTrue(_get_orange_warnings(), "Expected an orange credit-limit warning") def test_no_warning_when_amount_within_credit_limit(self): """No warning when base_grand_total is safely within the credit limit.""" - si = self._make_si(UNDER) + si = self._make_si(self.UNDER) _check_credit_limit_warn(si) self.assertFalse(_get_orange_warnings()) def test_no_warning_for_return_invoices(self): """Credit limit check is skipped entirely for return transactions.""" - si = self._make_si(OVER, is_return=1) + si = self._make_si(self.OVER, is_return=1) _check_credit_limit_warn(si) self.assertFalse(_get_orange_warnings()) def test_no_warning_when_customer_has_no_credit_limit(self): """If the customer has no credit limit configured, no warning is shown.""" frappe.db.delete("Customer Credit Limit", {"parent": self.CUSTOMER}) - si = self._make_si(OVER) + si = self._make_si(self.OVER) _check_credit_limit_warn(si) self.assertFalse(_get_orange_warnings()) @@ -76,7 +70,7 @@ class TestCreditLimitWarnSalesInvoice(_CreditLimitBase): When every item on the SI already has a sales_order or delivery_note reference, the check is skipped (the SO/DN already counted this amount). """ - si = self._make_si(OVER) + si = self._make_si(self.OVER) si.items[0].sales_order = "SO-TEST-0001" _check_credit_limit_warn(si) self.assertFalse(_get_orange_warnings()) @@ -86,25 +80,25 @@ class TestCreditLimitWarnSalesOrder(_CreditLimitBase): def _make_so(self, amount): """Build an in-memory (unsaved) draft SO.""" so = frappe.new_doc("Sales Order") - so.company = COMPANY + so.company = self.COMPANY so.customer = self.CUSTOMER so.base_grand_total = amount so.append("items", {"item_code": "_Test Item", "qty": 1, "rate": amount}) return so def test_warns_on_first_save_when_limit_exceeded(self): - so = self._make_so(OVER) + so = self._make_so(self.OVER) self.assertTrue(so.is_new(), "Doc should be new (not yet in DB)") _check_credit_limit_warn(so) self.assertTrue(_get_orange_warnings()) def test_warns_when_amount_exceeds_credit_limit(self): - so = self._make_so(OVER) + so = self._make_so(self.OVER) _check_credit_limit_warn(so) self.assertTrue(_get_orange_warnings()) def test_no_warning_when_amount_within_credit_limit(self): - so = self._make_so(UNDER) + so = self._make_so(self.UNDER) _check_credit_limit_warn(so) self.assertFalse(_get_orange_warnings()) @@ -115,11 +109,11 @@ class TestCreditLimitWarnSalesOrder(_CreditLimitBase): """ frappe.db.set_value( "Customer Credit Limit", - {"parent": self.CUSTOMER, "company": COMPANY}, + {"parent": self.CUSTOMER, "company": self.COMPANY}, "bypass_credit_limit_check", 1, ) - so = self._make_so(OVER) + so = self._make_so(self.OVER) _check_credit_limit_warn(so) self.assertFalse(_get_orange_warnings()) @@ -128,7 +122,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase): def _make_dn(self, amount, bypass=False, against_sales_order=None, against_sales_invoice=None): """Build an in-memory (unsaved) draft DN.""" dn = frappe.new_doc("Delivery Note") - dn.company = COMPANY + dn.company = self.COMPANY dn.customer = self.CUSTOMER dn.base_grand_total = amount dn.base_net_total = amount @@ -148,23 +142,21 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase): if bypass: frappe.db.set_value( "Customer Credit Limit", - {"parent": self.CUSTOMER, "company": COMPANY}, + {"parent": self.CUSTOMER, "company": self.COMPANY}, "bypass_credit_limit_check", 1, ) return dn - # bypass=False (default) ------------------------------------------------ - def test_bypass_false_warns_for_existing_draft(self): """bypass=False, existing draft: proportional extra_amount path still applies.""" - dn = self._make_dn(OVER) + dn = self._make_dn(self.OVER) _check_credit_limit_warn(dn) self.assertTrue(_get_orange_warnings()) def test_bypass_false_no_warning_when_under_limit(self): - dn = self._make_dn(UNDER) + dn = self._make_dn(self.UNDER) _check_credit_limit_warn(dn) self.assertFalse(_get_orange_warnings()) @@ -173,7 +165,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase): Items fully linked to a SO are excluded from unlinked_net. extra_amount becomes 0 → check is skipped. """ - dn = self._make_dn(OVER, against_sales_order="SO-TEST-0001") + dn = self._make_dn(self.OVER, against_sales_order="SO-TEST-0001") _check_credit_limit_warn(dn) self.assertFalse(_get_orange_warnings()) @@ -183,7 +175,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase): Only the unlinked portion should count toward the credit limit check. """ dn = frappe.new_doc("Delivery Note") - dn.company = COMPANY + dn.company = self.COMPANY dn.customer = self.CUSTOMER dn.append("items", {"item_code": "_Test Item", "qty": 1, "rate": 60, "amount": 60, "base_amount": 60}) dn.append( @@ -210,7 +202,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase): bypass=True: existing doc.check_credit_limit() handles extra_amount internally (base_grand_total for items not against SI). """ - dn = self._make_dn(OVER, bypass=True) + dn = self._make_dn(self.OVER, bypass=True) self.assertTrue(dn.is_new()) _check_credit_limit_warn(dn) self.assertTrue(_get_orange_warnings()) @@ -220,6 +212,53 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase): bypass=True: items already linked to a SI are excluded from extra_amount. If all items have against_sales_invoice set, extra_amount=0 → no check. """ - dn = self._make_dn(OVER, bypass=True, against_sales_invoice="SINV-TEST-0001") + dn = self._make_dn(self.OVER, bypass=True, against_sales_invoice="SINV-TEST-0001") _check_credit_limit_warn(dn) self.assertFalse(_get_orange_warnings()) + + +# --------------------------------------------------------------------------- +# Packed Qty +# --------------------------------------------------------------------------- + + +class TestPackedQtyWarn(ERPNextTestSuite): + COMPANY = "_Test Company" + CUSTOMER = "_Test Customer" + + def setUp(self): + frappe.message_log.clear() + + def _make_dn(self): + dn = frappe.new_doc("Delivery Note") + dn.company = self.COMPANY + dn.customer = self.CUSTOMER + dn.append( + "items", + {"item_code": "_Test Item", "qty": 2, "rate": 100, "amount": 200, "base_amount": 200}, + ) + return dn + + def test_no_warning_for_new_doc(self): + """New doc has no packing slip in DB, so validate_packed_qty is skipped.""" + dn = self._make_dn() + _check_packed_qty_warn(dn) + self.assertFalse(_get_orange_warnings()) + + def test_warns_when_packed_qty_mismatches(self): + """When validate_packed_qty raises, an orange warning is produced.""" + dn = self._make_dn() + with patch.object( + dn, + "validate_packed_qty", + side_effect=frappe.ValidationError("Packed Qty must be equal to qty"), + ): + _check_packed_qty_warn(dn) + self.assertTrue(_get_orange_warnings()) + + def test_no_warning_when_packed_qty_matches(self): + """When validate_packed_qty passes silently, no warning is produced.""" + dn = self._make_dn() + with patch.object(dn, "validate_packed_qty", return_value=None): + _check_packed_qty_warn(dn) + self.assertFalse(_get_orange_warnings()) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 1547f80bd6a..cfe9cff29ef 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -2737,25 +2737,25 @@ PRE_SUBMIT_DOCTYPE_CONFIG = { "Delivery Note": { "check_prev_docstatus": True, "check_credit_limit": True, + "check_packed_qty": True, }, "Purchase Receipt": { "check_prev_docstatus": True, }, "Sales Order": { - "check_prev_docstatus": True, "check_credit_limit": True, }, } def pre_submit_validation(doc, method=None): - if doc.docstatus != 0: - return - - if not frappe.get_cached_value("Accounts Settings", None, "preview_mode"): - return cfg = PRE_SUBMIT_DOCTYPE_CONFIG.get(doc.doctype) - if not cfg or not doc.company: + if ( + doc.docstatus != 0 + or not frappe.get_cached_value("Accounts Settings", None, "preview_mode") + or not cfg + or not doc.company + ): return _run_pre_submit_checks(doc, cfg) @@ -2767,19 +2767,20 @@ def _run_pre_submit_checks(doc, cfg): if cfg.get("check_credit_limit"): _check_credit_limit_warn(doc) + if cfg.get("check_packed_qty"): + _check_packed_qty_warn(doc) + def _check_prev_docstatus(doc): try: - if doc.get("check_prev_docstatus"): + if hasattr(doc, "check_prev_docstatus"): doc.check_prev_docstatus() except Exception as e: frappe.msgprint(str(e), title=_("Pre-Submit Warning"), indicator="orange") def _check_credit_limit_warn(doc): - if doc.get("is_return"): - return - if not doc.get("customer"): + if doc.get("is_return") or not doc.get("customer"): return from erpnext.selling.doctype.customer.customer import check_credit_limit @@ -2827,3 +2828,15 @@ def _check_credit_limit_warn(doc): title=_("Pre-Submit Warning: Credit Limit"), indicator="orange", ) + + +def _check_packed_qty_warn(doc): + try: + if hasattr(doc, "validate_packed_qty"): + doc.validate_packed_qty() + except frappe.ValidationError as e: + frappe.msgprint( + str(e), + title=_("Pre-Submit Warning: Packed Qty"), + indicator="orange", + )