mirror of
https://github.com/frappe/erpnext.git
synced 2026-05-27 08:54:45 +00:00
feat: pre-submit validation error for packed quantity mismatch
This commit is contained in:
@@ -708,7 +708,7 @@
|
|||||||
"label": "Fetch Payment Schedule In Payment Request"
|
"label": "Fetch Payment Schedule In Payment Request"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"fieldname": "repost_section",
|
"fieldname": "repost_section",
|
||||||
"fieldtype": "Section Break",
|
"fieldtype": "Section Break",
|
||||||
"label": "Repost"
|
"label": "Repost"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1,37 +1,31 @@
|
|||||||
# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors
|
# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors
|
||||||
# License: GNU General Public License v3. See license.txt
|
# License: GNU General Public License v3. See license.txt
|
||||||
|
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
import frappe
|
import frappe
|
||||||
|
|
||||||
from erpnext.accounts.utils import _check_credit_limit_warn
|
from erpnext.accounts.utils import (
|
||||||
from erpnext.selling.doctype.customer.test_customer import (
|
_check_credit_limit_warn,
|
||||||
get_customer_dict,
|
_check_packed_qty_warn,
|
||||||
set_credit_limit,
|
|
||||||
)
|
)
|
||||||
|
from erpnext.selling.doctype.customer.test_customer import set_credit_limit
|
||||||
from erpnext.tests.utils import ERPNextTestSuite
|
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():
|
def _get_orange_warnings():
|
||||||
return [m for m in frappe.message_log if m.get("indicator") == "orange"]
|
return [m for m in frappe.message_log if m.get("indicator") == "orange"]
|
||||||
|
|
||||||
|
|
||||||
class _CreditLimitBase(ERPNextTestSuite):
|
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):
|
def setUp(self):
|
||||||
_make_customer(self.CUSTOMER)
|
set_credit_limit(self.CUSTOMER, self.COMPANY, self.CREDIT_LIMIT)
|
||||||
set_credit_limit(self.CUSTOMER, COMPANY, CREDIT_LIMIT)
|
|
||||||
frappe.message_log.clear()
|
frappe.message_log.clear()
|
||||||
|
|
||||||
|
|
||||||
@@ -39,7 +33,7 @@ class TestCreditLimitWarnSalesInvoice(_CreditLimitBase):
|
|||||||
def _make_si(self, amount, is_return=0):
|
def _make_si(self, amount, is_return=0):
|
||||||
"""Build an in-memory (unsaved) draft SI."""
|
"""Build an in-memory (unsaved) draft SI."""
|
||||||
si = frappe.new_doc("Sales Invoice")
|
si = frappe.new_doc("Sales Invoice")
|
||||||
si.company = COMPANY
|
si.company = self.COMPANY
|
||||||
si.customer = self.CUSTOMER
|
si.customer = self.CUSTOMER
|
||||||
si.is_return = is_return
|
si.is_return = is_return
|
||||||
si.base_grand_total = amount
|
si.base_grand_total = amount
|
||||||
@@ -48,26 +42,26 @@ class TestCreditLimitWarnSalesInvoice(_CreditLimitBase):
|
|||||||
|
|
||||||
def test_warns_when_amount_exceeds_credit_limit(self):
|
def test_warns_when_amount_exceeds_credit_limit(self):
|
||||||
"""Orange warning must appear when base_grand_total > credit_limit."""
|
"""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)
|
_check_credit_limit_warn(si)
|
||||||
self.assertTrue(_get_orange_warnings(), "Expected an orange credit-limit warning")
|
self.assertTrue(_get_orange_warnings(), "Expected an orange credit-limit warning")
|
||||||
|
|
||||||
def test_no_warning_when_amount_within_credit_limit(self):
|
def test_no_warning_when_amount_within_credit_limit(self):
|
||||||
"""No warning when base_grand_total is safely within the credit limit."""
|
"""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)
|
_check_credit_limit_warn(si)
|
||||||
self.assertFalse(_get_orange_warnings())
|
self.assertFalse(_get_orange_warnings())
|
||||||
|
|
||||||
def test_no_warning_for_return_invoices(self):
|
def test_no_warning_for_return_invoices(self):
|
||||||
"""Credit limit check is skipped entirely for return transactions."""
|
"""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)
|
_check_credit_limit_warn(si)
|
||||||
self.assertFalse(_get_orange_warnings())
|
self.assertFalse(_get_orange_warnings())
|
||||||
|
|
||||||
def test_no_warning_when_customer_has_no_credit_limit(self):
|
def test_no_warning_when_customer_has_no_credit_limit(self):
|
||||||
"""If the customer has no credit limit configured, no warning is shown."""
|
"""If the customer has no credit limit configured, no warning is shown."""
|
||||||
frappe.db.delete("Customer Credit Limit", {"parent": self.CUSTOMER})
|
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)
|
_check_credit_limit_warn(si)
|
||||||
self.assertFalse(_get_orange_warnings())
|
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
|
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).
|
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"
|
si.items[0].sales_order = "SO-TEST-0001"
|
||||||
_check_credit_limit_warn(si)
|
_check_credit_limit_warn(si)
|
||||||
self.assertFalse(_get_orange_warnings())
|
self.assertFalse(_get_orange_warnings())
|
||||||
@@ -86,25 +80,25 @@ class TestCreditLimitWarnSalesOrder(_CreditLimitBase):
|
|||||||
def _make_so(self, amount):
|
def _make_so(self, amount):
|
||||||
"""Build an in-memory (unsaved) draft SO."""
|
"""Build an in-memory (unsaved) draft SO."""
|
||||||
so = frappe.new_doc("Sales Order")
|
so = frappe.new_doc("Sales Order")
|
||||||
so.company = COMPANY
|
so.company = self.COMPANY
|
||||||
so.customer = self.CUSTOMER
|
so.customer = self.CUSTOMER
|
||||||
so.base_grand_total = amount
|
so.base_grand_total = amount
|
||||||
so.append("items", {"item_code": "_Test Item", "qty": 1, "rate": amount})
|
so.append("items", {"item_code": "_Test Item", "qty": 1, "rate": amount})
|
||||||
return so
|
return so
|
||||||
|
|
||||||
def test_warns_on_first_save_when_limit_exceeded(self):
|
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)")
|
self.assertTrue(so.is_new(), "Doc should be new (not yet in DB)")
|
||||||
_check_credit_limit_warn(so)
|
_check_credit_limit_warn(so)
|
||||||
self.assertTrue(_get_orange_warnings())
|
self.assertTrue(_get_orange_warnings())
|
||||||
|
|
||||||
def test_warns_when_amount_exceeds_credit_limit(self):
|
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)
|
_check_credit_limit_warn(so)
|
||||||
self.assertTrue(_get_orange_warnings())
|
self.assertTrue(_get_orange_warnings())
|
||||||
|
|
||||||
def test_no_warning_when_amount_within_credit_limit(self):
|
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)
|
_check_credit_limit_warn(so)
|
||||||
self.assertFalse(_get_orange_warnings())
|
self.assertFalse(_get_orange_warnings())
|
||||||
|
|
||||||
@@ -115,11 +109,11 @@ class TestCreditLimitWarnSalesOrder(_CreditLimitBase):
|
|||||||
"""
|
"""
|
||||||
frappe.db.set_value(
|
frappe.db.set_value(
|
||||||
"Customer Credit Limit",
|
"Customer Credit Limit",
|
||||||
{"parent": self.CUSTOMER, "company": COMPANY},
|
{"parent": self.CUSTOMER, "company": self.COMPANY},
|
||||||
"bypass_credit_limit_check",
|
"bypass_credit_limit_check",
|
||||||
1,
|
1,
|
||||||
)
|
)
|
||||||
so = self._make_so(OVER)
|
so = self._make_so(self.OVER)
|
||||||
_check_credit_limit_warn(so)
|
_check_credit_limit_warn(so)
|
||||||
self.assertFalse(_get_orange_warnings())
|
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):
|
def _make_dn(self, amount, bypass=False, against_sales_order=None, against_sales_invoice=None):
|
||||||
"""Build an in-memory (unsaved) draft DN."""
|
"""Build an in-memory (unsaved) draft DN."""
|
||||||
dn = frappe.new_doc("Delivery Note")
|
dn = frappe.new_doc("Delivery Note")
|
||||||
dn.company = COMPANY
|
dn.company = self.COMPANY
|
||||||
dn.customer = self.CUSTOMER
|
dn.customer = self.CUSTOMER
|
||||||
dn.base_grand_total = amount
|
dn.base_grand_total = amount
|
||||||
dn.base_net_total = amount
|
dn.base_net_total = amount
|
||||||
@@ -148,23 +142,21 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase):
|
|||||||
if bypass:
|
if bypass:
|
||||||
frappe.db.set_value(
|
frappe.db.set_value(
|
||||||
"Customer Credit Limit",
|
"Customer Credit Limit",
|
||||||
{"parent": self.CUSTOMER, "company": COMPANY},
|
{"parent": self.CUSTOMER, "company": self.COMPANY},
|
||||||
"bypass_credit_limit_check",
|
"bypass_credit_limit_check",
|
||||||
1,
|
1,
|
||||||
)
|
)
|
||||||
|
|
||||||
return dn
|
return dn
|
||||||
|
|
||||||
# bypass=False (default) ------------------------------------------------
|
|
||||||
|
|
||||||
def test_bypass_false_warns_for_existing_draft(self):
|
def test_bypass_false_warns_for_existing_draft(self):
|
||||||
"""bypass=False, existing draft: proportional extra_amount path still applies."""
|
"""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)
|
_check_credit_limit_warn(dn)
|
||||||
self.assertTrue(_get_orange_warnings())
|
self.assertTrue(_get_orange_warnings())
|
||||||
|
|
||||||
def test_bypass_false_no_warning_when_under_limit(self):
|
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)
|
_check_credit_limit_warn(dn)
|
||||||
self.assertFalse(_get_orange_warnings())
|
self.assertFalse(_get_orange_warnings())
|
||||||
|
|
||||||
@@ -173,7 +165,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase):
|
|||||||
Items fully linked to a SO are excluded from unlinked_net.
|
Items fully linked to a SO are excluded from unlinked_net.
|
||||||
extra_amount becomes 0 → check is skipped.
|
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)
|
_check_credit_limit_warn(dn)
|
||||||
self.assertFalse(_get_orange_warnings())
|
self.assertFalse(_get_orange_warnings())
|
||||||
|
|
||||||
@@ -183,7 +175,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase):
|
|||||||
Only the unlinked portion should count toward the credit limit check.
|
Only the unlinked portion should count toward the credit limit check.
|
||||||
"""
|
"""
|
||||||
dn = frappe.new_doc("Delivery Note")
|
dn = frappe.new_doc("Delivery Note")
|
||||||
dn.company = COMPANY
|
dn.company = self.COMPANY
|
||||||
dn.customer = self.CUSTOMER
|
dn.customer = self.CUSTOMER
|
||||||
dn.append("items", {"item_code": "_Test Item", "qty": 1, "rate": 60, "amount": 60, "base_amount": 60})
|
dn.append("items", {"item_code": "_Test Item", "qty": 1, "rate": 60, "amount": 60, "base_amount": 60})
|
||||||
dn.append(
|
dn.append(
|
||||||
@@ -210,7 +202,7 @@ class TestCreditLimitWarnDeliveryNote(_CreditLimitBase):
|
|||||||
bypass=True: existing doc.check_credit_limit() handles extra_amount
|
bypass=True: existing doc.check_credit_limit() handles extra_amount
|
||||||
internally (base_grand_total for items not against SI).
|
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())
|
self.assertTrue(dn.is_new())
|
||||||
_check_credit_limit_warn(dn)
|
_check_credit_limit_warn(dn)
|
||||||
self.assertTrue(_get_orange_warnings())
|
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.
|
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.
|
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)
|
_check_credit_limit_warn(dn)
|
||||||
self.assertFalse(_get_orange_warnings())
|
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())
|
||||||
|
|||||||
@@ -2737,25 +2737,25 @@ PRE_SUBMIT_DOCTYPE_CONFIG = {
|
|||||||
"Delivery Note": {
|
"Delivery Note": {
|
||||||
"check_prev_docstatus": True,
|
"check_prev_docstatus": True,
|
||||||
"check_credit_limit": True,
|
"check_credit_limit": True,
|
||||||
|
"check_packed_qty": True,
|
||||||
},
|
},
|
||||||
"Purchase Receipt": {
|
"Purchase Receipt": {
|
||||||
"check_prev_docstatus": True,
|
"check_prev_docstatus": True,
|
||||||
},
|
},
|
||||||
"Sales Order": {
|
"Sales Order": {
|
||||||
"check_prev_docstatus": True,
|
|
||||||
"check_credit_limit": True,
|
"check_credit_limit": True,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def pre_submit_validation(doc, method=None):
|
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)
|
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
|
return
|
||||||
_run_pre_submit_checks(doc, cfg)
|
_run_pre_submit_checks(doc, cfg)
|
||||||
|
|
||||||
@@ -2767,19 +2767,20 @@ def _run_pre_submit_checks(doc, cfg):
|
|||||||
if cfg.get("check_credit_limit"):
|
if cfg.get("check_credit_limit"):
|
||||||
_check_credit_limit_warn(doc)
|
_check_credit_limit_warn(doc)
|
||||||
|
|
||||||
|
if cfg.get("check_packed_qty"):
|
||||||
|
_check_packed_qty_warn(doc)
|
||||||
|
|
||||||
|
|
||||||
def _check_prev_docstatus(doc):
|
def _check_prev_docstatus(doc):
|
||||||
try:
|
try:
|
||||||
if doc.get("check_prev_docstatus"):
|
if hasattr(doc, "check_prev_docstatus"):
|
||||||
doc.check_prev_docstatus()
|
doc.check_prev_docstatus()
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
frappe.msgprint(str(e), title=_("Pre-Submit Warning"), indicator="orange")
|
frappe.msgprint(str(e), title=_("Pre-Submit Warning"), indicator="orange")
|
||||||
|
|
||||||
|
|
||||||
def _check_credit_limit_warn(doc):
|
def _check_credit_limit_warn(doc):
|
||||||
if doc.get("is_return"):
|
if doc.get("is_return") or not doc.get("customer"):
|
||||||
return
|
|
||||||
if not doc.get("customer"):
|
|
||||||
return
|
return
|
||||||
|
|
||||||
from erpnext.selling.doctype.customer.customer import check_credit_limit
|
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"),
|
title=_("Pre-Submit Warning: Credit Limit"),
|
||||||
indicator="orange",
|
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",
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user