Merge pull request #42172 from frappe/mergify/bp/version-14-hotfix/pr-42143
refactor: validation to prevent recursion with mixed conditions (backport #42143)
This commit is contained in:
@@ -31,6 +31,7 @@ class PricingRule(Document):
|
|||||||
self.validate_price_list_with_currency()
|
self.validate_price_list_with_currency()
|
||||||
self.validate_dates()
|
self.validate_dates()
|
||||||
self.validate_condition()
|
self.validate_condition()
|
||||||
|
self.validate_mixed_with_recursion()
|
||||||
|
|
||||||
if not self.margin_type:
|
if not self.margin_type:
|
||||||
self.margin_rate_or_amount = 0.0
|
self.margin_rate_or_amount = 0.0
|
||||||
@@ -201,6 +202,10 @@ class PricingRule(Document):
|
|||||||
):
|
):
|
||||||
frappe.throw(_("Invalid condition expression"))
|
frappe.throw(_("Invalid condition expression"))
|
||||||
|
|
||||||
|
def validate_mixed_with_recursion(self):
|
||||||
|
if self.mixed_conditions and self.is_recursive:
|
||||||
|
frappe.throw(_("Recursive Discounts with Mixed condition is not supported by the system"))
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------------
|
# --------------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|||||||
@@ -1087,6 +1087,18 @@ class TestPricingRule(unittest.TestCase):
|
|||||||
frappe.delete_doc_if_exists("Pricing Rule", "_Test Pricing Rule 1")
|
frappe.delete_doc_if_exists("Pricing Rule", "_Test Pricing Rule 1")
|
||||||
frappe.delete_doc_if_exists("Pricing Rule", "_Test Pricing Rule 2")
|
frappe.delete_doc_if_exists("Pricing Rule", "_Test Pricing Rule 2")
|
||||||
|
|
||||||
|
def test_validation_on_mixed_condition_with_recursion(self):
|
||||||
|
pricing_rule = make_pricing_rule(
|
||||||
|
discount_percentage=10,
|
||||||
|
selling=1,
|
||||||
|
priority=2,
|
||||||
|
min_qty=4,
|
||||||
|
title="_Test Pricing Rule with Min Qty - 2",
|
||||||
|
)
|
||||||
|
pricing_rule.mixed_conditions = True
|
||||||
|
pricing_rule.is_recursive = True
|
||||||
|
self.assertRaises(frappe.ValidationError, pricing_rule.save)
|
||||||
|
|
||||||
|
|
||||||
test_dependencies = ["Campaign"]
|
test_dependencies = ["Campaign"]
|
||||||
|
|
||||||
|
|||||||
@@ -77,6 +77,7 @@ class PromotionalScheme(Document):
|
|||||||
|
|
||||||
self.validate_applicable_for()
|
self.validate_applicable_for()
|
||||||
self.validate_pricing_rules()
|
self.validate_pricing_rules()
|
||||||
|
self.validate_mixed_with_recursion()
|
||||||
|
|
||||||
def validate_applicable_for(self):
|
def validate_applicable_for(self):
|
||||||
if self.applicable_for:
|
if self.applicable_for:
|
||||||
@@ -94,7 +95,7 @@ class PromotionalScheme(Document):
|
|||||||
docnames = []
|
docnames = []
|
||||||
|
|
||||||
# If user has changed applicable for
|
# If user has changed applicable for
|
||||||
if self._doc_before_save.applicable_for == self.applicable_for:
|
if self.get_doc_before_save() and self.get_doc_before_save().applicable_for == self.applicable_for:
|
||||||
return
|
return
|
||||||
|
|
||||||
docnames = frappe.get_all("Pricing Rule", filters={"promotional_scheme": self.name})
|
docnames = frappe.get_all("Pricing Rule", filters={"promotional_scheme": self.name})
|
||||||
@@ -108,6 +109,7 @@ class PromotionalScheme(Document):
|
|||||||
frappe.delete_doc("Pricing Rule", docname.name)
|
frappe.delete_doc("Pricing Rule", docname.name)
|
||||||
|
|
||||||
def on_update(self):
|
def on_update(self):
|
||||||
|
self.validate()
|
||||||
pricing_rules = (
|
pricing_rules = (
|
||||||
frappe.get_all(
|
frappe.get_all(
|
||||||
"Pricing Rule",
|
"Pricing Rule",
|
||||||
@@ -119,6 +121,15 @@ class PromotionalScheme(Document):
|
|||||||
)
|
)
|
||||||
self.update_pricing_rules(pricing_rules)
|
self.update_pricing_rules(pricing_rules)
|
||||||
|
|
||||||
|
def validate_mixed_with_recursion(self):
|
||||||
|
if self.mixed_conditions:
|
||||||
|
if self.product_discount_slabs:
|
||||||
|
for slab in self.product_discount_slabs:
|
||||||
|
if slab.is_recursive:
|
||||||
|
frappe.throw(
|
||||||
|
_("Recursive Discounts with Mixed condition is not supported by the system")
|
||||||
|
)
|
||||||
|
|
||||||
def update_pricing_rules(self, pricing_rules):
|
def update_pricing_rules(self, pricing_rules):
|
||||||
rules = {}
|
rules = {}
|
||||||
count = 0
|
count = 0
|
||||||
|
|||||||
@@ -107,6 +107,25 @@ class TestPromotionalScheme(unittest.TestCase):
|
|||||||
price_rules = frappe.get_all("Pricing Rule", filters={"promotional_scheme": ps.name})
|
price_rules = frappe.get_all("Pricing Rule", filters={"promotional_scheme": ps.name})
|
||||||
self.assertEqual(price_rules, [])
|
self.assertEqual(price_rules, [])
|
||||||
|
|
||||||
|
def test_validation_on_recurse_with_mixed_condition(self):
|
||||||
|
ps = make_promotional_scheme()
|
||||||
|
ps.set("price_discount_slabs", [])
|
||||||
|
ps.set(
|
||||||
|
"product_discount_slabs",
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"rule_description": "12+1",
|
||||||
|
"min_qty": 12,
|
||||||
|
"free_item": "_Test Item 2",
|
||||||
|
"free_qty": 1,
|
||||||
|
"is_recursive": 1,
|
||||||
|
"recurse_for": 12,
|
||||||
|
}
|
||||||
|
],
|
||||||
|
)
|
||||||
|
ps.mixed_conditions = True
|
||||||
|
self.assertRaises(frappe.ValidationError, ps.save)
|
||||||
|
|
||||||
|
|
||||||
def make_promotional_scheme(**args):
|
def make_promotional_scheme(**args):
|
||||||
args = frappe._dict(args)
|
args = frappe._dict(args)
|
||||||
|
|||||||
Reference in New Issue
Block a user