From fdfab03c11dac62073987c0de495e423c0ecfa21 Mon Sep 17 00:00:00 2001 From: Abhishek Balam Date: Wed, 12 Aug 2020 20:18:31 +0530 Subject: [PATCH 01/10] feat: add condition field in pricing rule --- .../doctype/pricing_rule/pricing_rule.json | 11 ++++++++++- .../doctype/pricing_rule/pricing_rule.py | 16 +++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.json b/erpnext/accounts/doctype/pricing_rule/pricing_rule.json index 29d83783d0..87084c7560 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.json +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.json @@ -1,4 +1,5 @@ { + "actions": [], "allow_import": 1, "allow_rename": 1, "autoname": "field:title", @@ -12,6 +13,7 @@ "apply_on", "price_or_product_discount", "warehouse", + "condition", "column_break_7", "items", "item_groups", @@ -550,11 +552,18 @@ "fieldtype": "Link", "label": "Promotional Scheme", "options": "Promotional Scheme" + }, + { + "description": "Simple Python Expression, Example: territory != 'All Territories'", + "fieldname": "condition", + "fieldtype": "Code", + "label": "Condition" } ], "icon": "fa fa-gift", "idx": 1, - "modified": "2019-12-18 17:29:22.957077", + "links": [], + "modified": "2020-08-12 20:15:32.279592", "modified_by": "Administrator", "module": "Accounts", "name": "Pricing Rule", diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index cff7d5ba22..d88ce222d6 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -7,9 +7,10 @@ from __future__ import unicode_literals import frappe import json import copy +import re + from frappe import throw, _ from frappe.utils import flt, cint, getdate - from frappe.model.document import Document from six import string_types @@ -31,6 +32,7 @@ class PricingRule(Document): self.validate_max_discount() self.validate_price_list_with_currency() self.validate_dates() + validate_condition(self) if not self.margin_type: self.margin_rate_or_amount = 0.0 @@ -141,6 +143,16 @@ class PricingRule(Document): if self.valid_from and self.valid_upto and getdate(self.valid_from) > getdate(self.valid_upto): frappe.throw(_("Valid from date must be less than valid upto date")) +def validate_condition(doc, args=None): + if doc.condition and ("=" in doc.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", doc.condition): + frappe.throw(_("Invalid condition in Pricing Rule: {0}").format(doc.name), frappe.ValidationError) + else: + try: + return frappe.safe_eval(doc.condition, None, args) if bool(args) else True + except Exception as e: + frappe.throw(doc.name + " Pricing Rule 'Condition' field error:
" + str(e).capitalize() ) + return False + #-------------------------------------------------------------------------------- @frappe.whitelist() @@ -252,6 +264,8 @@ def get_pricing_rule_for_item(args, price_list_rate=0, doc=None, for_validate=Fa if pricing_rule.get('suggestion'): continue + if not validate_condition(pricing_rule, args): continue + item_details.validate_applied_rule = pricing_rule.get("validate_applied_rule", 0) item_details.price_or_product_discount = pricing_rule.get("price_or_product_discount") From cbde04d3daf0245a2db0d99cfb64a413aaa25b25 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 13 Aug 2020 11:47:41 +0530 Subject: [PATCH 02/10] fix: Pass doc instead of args --- .../accounts/doctype/pricing_rule/pricing_rule.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index d88ce222d6..98abadbff6 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -32,7 +32,6 @@ class PricingRule(Document): self.validate_max_discount() self.validate_price_list_with_currency() self.validate_dates() - validate_condition(self) if not self.margin_type: self.margin_rate_or_amount = 0.0 @@ -143,16 +142,17 @@ class PricingRule(Document): if self.valid_from and self.valid_upto and getdate(self.valid_from) > getdate(self.valid_upto): frappe.throw(_("Valid from date must be less than valid upto date")) -def validate_condition(doc, args=None): - if doc.condition and ("=" in doc.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", doc.condition): - frappe.throw(_("Invalid condition in Pricing Rule: {0}").format(doc.name), frappe.ValidationError) +def validate_condition(pricing_rule, doc=None): + if pricing_rule.condition and ("=" in pricing_rule.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", pricing_rule.condition): + frappe.throw(_("Invalid condition in Pricing Rule: {0}").format(pricing_rule.name), frappe.ValidationError) else: try: - return frappe.safe_eval(doc.condition, None, args) if bool(args) else True + doc = doc.as_dict() + return frappe.safe_eval(pricing_rule.condition, None, doc) except Exception as e: frappe.throw(doc.name + " Pricing Rule 'Condition' field error:
" + str(e).capitalize() ) return False - + return True #-------------------------------------------------------------------------------- @frappe.whitelist() @@ -264,7 +264,7 @@ def get_pricing_rule_for_item(args, price_list_rate=0, doc=None, for_validate=Fa if pricing_rule.get('suggestion'): continue - if not validate_condition(pricing_rule, args): continue + if not validate_condition(pricing_rule, doc): continue item_details.validate_applied_rule = pricing_rule.get("validate_applied_rule", 0) item_details.price_or_product_discount = pricing_rule.get("price_or_product_discount") From ca898c6da3d63f78883e1b0a6cbf090076e51617 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 13 Aug 2020 11:54:18 +0530 Subject: [PATCH 03/10] fix: Condition fix --- erpnext/accounts/doctype/pricing_rule/pricing_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index 98abadbff6..a4a95e1c20 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -145,7 +145,7 @@ class PricingRule(Document): def validate_condition(pricing_rule, doc=None): if pricing_rule.condition and ("=" in pricing_rule.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", pricing_rule.condition): frappe.throw(_("Invalid condition in Pricing Rule: {0}").format(pricing_rule.name), frappe.ValidationError) - else: + elif pricing_rule.condition: try: doc = doc.as_dict() return frappe.safe_eval(pricing_rule.condition, None, doc) From 487aa30aef20b3689d7853ff73cda59d7d381c7a Mon Sep 17 00:00:00 2001 From: Abhishek Balam Date: Thu, 13 Aug 2020 12:13:08 +0530 Subject: [PATCH 04/10] fix: eval fail message fix --- erpnext/accounts/doctype/pricing_rule/pricing_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index a4a95e1c20..53f900fbf2 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -150,7 +150,7 @@ def validate_condition(pricing_rule, doc=None): doc = doc.as_dict() return frappe.safe_eval(pricing_rule.condition, None, doc) except Exception as e: - frappe.throw(doc.name + " Pricing Rule 'Condition' field error:
" + str(e).capitalize() ) + frappe.throw(" Pricing Rule - " + pricing_rule.name + " - 'Condition' field error:
" + str(e).capitalize() ) return False return True #-------------------------------------------------------------------------------- From a25760046fb4ad5d91bb7ed5ed028b3eea3222de Mon Sep 17 00:00:00 2001 From: Abhishek Balam Date: Thu, 13 Aug 2020 13:22:58 +0530 Subject: [PATCH 05/10] fix: condition syntax validation readded, fetch item details if condition not met ignoring rule --- erpnext/accounts/doctype/pricing_rule/pricing_rule.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index 53f900fbf2..e915618f4c 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -32,6 +32,7 @@ class PricingRule(Document): self.validate_max_discount() self.validate_price_list_with_currency() self.validate_dates() + validate_condition(self) if not self.margin_type: self.margin_rate_or_amount = 0.0 @@ -144,13 +145,13 @@ class PricingRule(Document): def validate_condition(pricing_rule, doc=None): if pricing_rule.condition and ("=" in pricing_rule.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", pricing_rule.condition): - frappe.throw(_("Invalid condition in Pricing Rule: {0}").format(pricing_rule.name), frappe.ValidationError) - elif pricing_rule.condition: + frappe.throw(_("Invalid condition in Pricing Rule - {0}").format(pricing_rule.name), frappe.ValidationError) + elif doc: try: - doc = doc.as_dict() - return frappe.safe_eval(pricing_rule.condition, None, doc) + return frappe.safe_eval(pricing_rule.condition, None, doc.as_dict()) except Exception as e: - frappe.throw(" Pricing Rule - " + pricing_rule.name + " - 'Condition' field error:
" + str(e).capitalize() ) + frappe.msgprint(_("Pricing Rule - " + pricing_rule.name + " - condition field error:
" + \ + str(e).capitalize() + "

Ignoring Pricing Rule"), indicator="orange", title=_("Warning")) return False return True #-------------------------------------------------------------------------------- From d9accededa0615598adcd80886d5cec9deb80d5e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 14 Aug 2020 22:36:01 +0530 Subject: [PATCH 06/10] fix: filter pricing rules based on condition --- .../doctype/pricing_rule/pricing_rule.py | 19 +++++-------------- .../accounts/doctype/pricing_rule/utils.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index e915618f4c..611c2fca69 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -32,7 +32,7 @@ class PricingRule(Document): self.validate_max_discount() self.validate_price_list_with_currency() self.validate_dates() - validate_condition(self) + self.validate_condition() if not self.margin_type: self.margin_rate_or_amount = 0.0 @@ -143,17 +143,10 @@ class PricingRule(Document): if self.valid_from and self.valid_upto and getdate(self.valid_from) > getdate(self.valid_upto): frappe.throw(_("Valid from date must be less than valid upto date")) -def validate_condition(pricing_rule, doc=None): - if pricing_rule.condition and ("=" in pricing_rule.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", pricing_rule.condition): - frappe.throw(_("Invalid condition in Pricing Rule - {0}").format(pricing_rule.name), frappe.ValidationError) - elif doc: - try: - return frappe.safe_eval(pricing_rule.condition, None, doc.as_dict()) - except Exception as e: - frappe.msgprint(_("Pricing Rule - " + pricing_rule.name + " - condition field error:
" + \ - str(e).capitalize() + "

Ignoring Pricing Rule"), indicator="orange", title=_("Warning")) - return False - return True + def validate_condition(self): + if self.condition and ("=" in self.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", self.condition): + frappe.throw(_("Invalid condition in Pricing Rule - {0}").format(pricing_rule.name), frappe.ValidationError) + #-------------------------------------------------------------------------------- @frappe.whitelist() @@ -265,8 +258,6 @@ def get_pricing_rule_for_item(args, price_list_rate=0, doc=None, for_validate=Fa if pricing_rule.get('suggestion'): continue - if not validate_condition(pricing_rule, doc): continue - item_details.validate_applied_rule = pricing_rule.get("validate_applied_rule", 0) item_details.price_or_product_discount = pricing_rule.get("price_or_product_discount") diff --git a/erpnext/accounts/doctype/pricing_rule/utils.py b/erpnext/accounts/doctype/pricing_rule/utils.py index 3fd316f75e..59903a70b6 100644 --- a/erpnext/accounts/doctype/pricing_rule/utils.py +++ b/erpnext/accounts/doctype/pricing_rule/utils.py @@ -37,6 +37,8 @@ def get_pricing_rules(args, doc=None): rules = [] + pricing_rules = filter_pricing_rule_based_on_condition(pricing_rules, doc) + if not pricing_rules: return [] if apply_multiple_pricing_rules(pricing_rules): @@ -51,6 +53,22 @@ def get_pricing_rules(args, doc=None): return rules +def filter_pricing_rule_based_on_condition(pricing_rules, doc=None): + filtered_pricing_rules = [] + if doc: + for pricing_rule in pricing_rules: + if pricing_rule.condition: + try: + if frappe.safe_eval(pricing_rule.condition, None, doc.as_dict()): + filtered_pricing_rules.append(pricing_rule) + except Exception as e: + frappe.msgprint(_("Pricing Rule - " + pricing_rule.name + " - condition field error:
" + \ + str(e).capitalize() + "

Ignoring Pricing Rule"), indicator="orange", title=_("Warning")) + else: + filtered_pricing_rules.append(pricing_rule) + + return filtered_pricing_rules + def _get_pricing_rules(apply_on, args, values): apply_on_field = frappe.scrub(apply_on) From 1067f7ed3ea1b7c402c8a8dc9867da1ce0fd1485 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 17 Aug 2020 23:47:58 +0530 Subject: [PATCH 07/10] fix: Price rule filtering in case of no doc --- erpnext/accounts/doctype/pricing_rule/pricing_rule.py | 2 +- erpnext/accounts/doctype/pricing_rule/utils.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index 611c2fca69..5a93006184 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -145,7 +145,7 @@ class PricingRule(Document): def validate_condition(self): if self.condition and ("=" in self.condition) and re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", self.condition): - frappe.throw(_("Invalid condition in Pricing Rule - {0}").format(pricing_rule.name), frappe.ValidationError) + frappe.throw(_("Invalid condition expression")) #-------------------------------------------------------------------------------- diff --git a/erpnext/accounts/doctype/pricing_rule/utils.py b/erpnext/accounts/doctype/pricing_rule/utils.py index 59903a70b6..fab533ebd2 100644 --- a/erpnext/accounts/doctype/pricing_rule/utils.py +++ b/erpnext/accounts/doctype/pricing_rule/utils.py @@ -66,6 +66,8 @@ def filter_pricing_rule_based_on_condition(pricing_rules, doc=None): str(e).capitalize() + "

Ignoring Pricing Rule"), indicator="orange", title=_("Warning")) else: filtered_pricing_rules.append(pricing_rule) + else: + filtered_pricing_rules = pricing_rules return filtered_pricing_rules From 7d0bb25aca00b7758d1faa674c268658e4f1eb48 Mon Sep 17 00:00:00 2001 From: Abhishek Balam Date: Wed, 26 Aug 2020 12:22:31 +0530 Subject: [PATCH 08/10] fix: dont show message on condition syntax error --- erpnext/accounts/doctype/pricing_rule/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/utils.py b/erpnext/accounts/doctype/pricing_rule/utils.py index fab533ebd2..482abb81a3 100644 --- a/erpnext/accounts/doctype/pricing_rule/utils.py +++ b/erpnext/accounts/doctype/pricing_rule/utils.py @@ -61,9 +61,8 @@ def filter_pricing_rule_based_on_condition(pricing_rules, doc=None): try: if frappe.safe_eval(pricing_rule.condition, None, doc.as_dict()): filtered_pricing_rules.append(pricing_rule) - except Exception as e: - frappe.msgprint(_("Pricing Rule - " + pricing_rule.name + " - condition field error:
" + \ - str(e).capitalize() + "

Ignoring Pricing Rule"), indicator="orange", title=_("Warning")) + except: + pass else: filtered_pricing_rules.append(pricing_rule) else: From dc9af639a7bfffe032c6650206875ec5f5c13443 Mon Sep 17 00:00:00 2001 From: Abhishek Balam Date: Wed, 26 Aug 2020 12:26:15 +0530 Subject: [PATCH 09/10] fix: move condition field --- erpnext/accounts/doctype/pricing_rule/pricing_rule.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.json b/erpnext/accounts/doctype/pricing_rule/pricing_rule.json index 87084c7560..c681c897fc 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.json +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.json @@ -13,7 +13,6 @@ "apply_on", "price_or_product_discount", "warehouse", - "condition", "column_break_7", "items", "item_groups", @@ -73,6 +72,7 @@ "section_break_13", "threshold_percentage", "priority", + "condition", "column_break_66", "apply_multiple_pricing_rules", "apply_discount_on_rate", @@ -563,7 +563,7 @@ "icon": "fa fa-gift", "idx": 1, "links": [], - "modified": "2020-08-12 20:15:32.279592", + "modified": "2020-08-26 12:24:44.740734", "modified_by": "Administrator", "module": "Accounts", "name": "Pricing Rule", From 71c24c8a90a5112bffd558befa0b6514839cc257 Mon Sep 17 00:00:00 2001 From: Abhishek Balam Date: Thu, 1 Oct 2020 14:54:11 +0530 Subject: [PATCH 10/10] feat: add tests --- .../doctype/pricing_rule/test_pricing_rule.py | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py index 2bf0b72563..3555ca895f 100644 --- a/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py @@ -429,7 +429,34 @@ class TestPricingRule(unittest.TestCase): details = get_item_details(args) self.assertTrue(details) - + + def test_pricing_rule_for_condition(self): + frappe.delete_doc_if_exists("Pricing Rule", "_Test Pricing Rule") + + make_pricing_rule(selling=1, margin_type="Percentage", \ + condition="customer=='_Test Customer 1' and is_return==0", discount_percentage=10) + + # Incorrect Customer and Correct is_return value + si = create_sales_invoice(do_not_submit=True, customer="_Test Customer 2", is_return=0) + si.items[0].price_list_rate = 1000 + si.submit() + item = si.items[0] + self.assertEquals(item.rate, 100) + + # Correct Customer and Incorrect is_return value + si = create_sales_invoice(do_not_submit=True, customer="_Test Customer 1", is_return=1, qty=-1) + si.items[0].price_list_rate = 1000 + si.submit() + item = si.items[0] + self.assertEquals(item.rate, 100) + + # Correct Customer and correct is_return value + si = create_sales_invoice(do_not_submit=True, customer="_Test Customer 1", is_return=0) + si.items[0].price_list_rate = 1000 + si.submit() + item = si.items[0] + self.assertEquals(item.rate, 900) + def make_pricing_rule(**args): args = frappe._dict(args) @@ -448,7 +475,8 @@ def make_pricing_rule(**args): "discount_percentage": args.discount_percentage or 0.0, "rate": args.rate or 0.0, "margin_type": args.margin_type, - "margin_rate_or_amount": args.margin_rate_or_amount or 0.0 + "margin_rate_or_amount": args.margin_rate_or_amount or 0.0, + "condition": args.condition or '' }) apply_on = doc.apply_on.replace(' ', '_').lower()