Skip to content

Commit

Permalink
Merge pull request #42173 from frappe/mergify/bp/version-15-hotfix/pr…
Browse files Browse the repository at this point in the history
…-42143

refactor: validation to prevent recursion with mixed conditions (backport #42143)
  • Loading branch information
ruthra-kumar authored Jul 4, 2024
2 parents fcfe78b + 12bec3b commit cf937ed
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
5 changes: 5 additions & 0 deletions erpnext/accounts/doctype/pricing_rule/pricing_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def validate(self):
self.validate_price_list_with_currency()
self.validate_dates()
self.validate_condition()
self.validate_mixed_with_recursion()

if not self.margin_type:
self.margin_rate_or_amount = 0.0
Expand Down Expand Up @@ -308,6 +309,10 @@ def validate_condition(self):
):
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"))


# --------------------------------------------------------------------------------

Expand Down
12 changes: 12 additions & 0 deletions erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -1299,6 +1299,18 @@ def test_pricing_rules_with_and_without_apply_multiple(self):
item_group_rule.delete()
item_code_rule.delete()

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"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ def validate(self):

self.validate_applicable_for()
self.validate_pricing_rules()
self.validate_mixed_with_recursion()

def validate_applicable_for(self):
if self.applicable_for:
Expand All @@ -163,7 +164,7 @@ def validate_pricing_rules(self):
docnames = []

# 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

docnames = frappe.get_all("Pricing Rule", filters={"promotional_scheme": self.name})
Expand All @@ -177,6 +178,7 @@ def validate_pricing_rules(self):
frappe.delete_doc("Pricing Rule", docname.name)

def on_update(self):
self.validate()
pricing_rules = (
frappe.get_all(
"Pricing Rule",
Expand All @@ -188,6 +190,15 @@ def on_update(self):
)
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):
rules = {}
count = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,25 @@ def test_pricing_rule_for_product_discount_slabs(self):
[pr.min_qty, pr.free_item, pr.free_qty, pr.recurse_for], [12, "_Test Item 2", 1, 12]
)

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):
args = frappe._dict(args)
Expand Down

0 comments on commit cf937ed

Please sign in to comment.