Merge pull request #27040 from marination/cart-pl-exchange-rate

fix: Shopping cart Exchange rate validation

erpnext/hooks.py

@@ -251,7 +251,7 @@ doc_events = {
 			"erpnext.support.doctype.issue.issue.set_first_response_time"
 		]
 	},
-	("Sales Taxes and Charges Template", 'Price List'): {
+	"Sales Taxes and Charges Template": {
 		"on_update": "erpnext.shopping_cart.doctype.shopping_cart_settings.shopping_cart_settings.validate_cart_settings"
 	},
 	"Website Settings": {

erpnext/shopping_cart/doctype/shopping_cart_settings/shopping_cart_settings.py

@@ -6,7 +6,7 @@
 from __future__ import unicode_literals
 import frappe
 from frappe import _, msgprint
-from frappe.utils import comma_and
+from frappe.utils import flt
 from frappe.model.document import Document
 from frappe.utils import get_datetime, get_datetime_str, now_datetime
 
@@ -18,46 +18,35 @@ class ShoppingCartSettings(Document):
 
 	def validate(self):
 		if self.enabled:
-			self.validate_exchange_rates_exist()
+			self.validate_price_list_exchange_rate()
+
+	def validate_price_list_exchange_rate(self):
+		"Check if exchange rate exists for Price List currency (to Company's currency)."
+		from erpnext.setup.utils import get_exchange_rate
+
+		if not self.enabled or not self.company or not self.price_list:
+			return # this function is also called from hooks, check values again
+
+		company_currency = frappe.get_cached_value("Company", self.company, "default_currency")
+		price_list_currency = frappe.db.get_value("Price List", self.price_list, "currency")
 
-	def validate_exchange_rates_exist(self):
-		"""check if exchange rates exist for all Price List currencies (to company's currency)"""
-		company_currency = frappe.get_cached_value('Company',  self.company,  "default_currency")
 		if not company_currency:
-			msgprint(_("Please specify currency in Company") + ": " + self.company,
-				raise_exception=ShoppingCartSetupError)
+			msg = f"Please specify currency in Company {self.company}"
+			frappe.throw(_(msg), title=_("Missing Currency"), exc=ShoppingCartSetupError)
 
-		price_list_currency_map = frappe.db.get_values("Price List",
-			[self.price_list], "currency")
+		if not price_list_currency:
+			msg = f"Please specify currency in Price List {frappe.bold(self.price_list)}"
+			frappe.throw(_(msg), title=_("Missing Currency"), exc=ShoppingCartSetupError)
 
-		price_list_currency_map = dict(price_list_currency_map)
+		if price_list_currency != company_currency:
+			from_currency, to_currency = price_list_currency, company_currency
 
-		# check if all price lists have a currency
-		for price_list, currency in price_list_currency_map.items():
-			if not currency:
-				frappe.throw(_("Currency is required for Price List {0}").format(price_list))
+			# Get exchange rate checks Currency Exchange Records too
+			exchange_rate = get_exchange_rate(from_currency, to_currency, args="for_selling")
 
-		expected_to_exist = [currency + "-" + company_currency
-			for currency in price_list_currency_map.values()
-			if currency != company_currency]
-
-		# manqala 20/09/2016: set up selection parameters for query from tabCurrency Exchange
-		from_currency = [currency for currency in price_list_currency_map.values() if currency != company_currency]
-		to_currency = company_currency
-		# manqala end
-
-		if expected_to_exist:
-			# manqala 20/09/2016: modify query so that it uses date in the selection from Currency Exchange.
-			# exchange rates defined with date less than the date on which this document is being saved will be selected
-			exists = frappe.db.sql_list("""select CONCAT(from_currency,'-',to_currency) from `tabCurrency Exchange`
-				where from_currency in (%s) and to_currency = "%s" and date <= curdate()""" % (", ".join(["%s"]*len(from_currency)), to_currency), tuple(from_currency))
-			# manqala end
-
-			missing = list(set(expected_to_exist).difference(exists))
-
-			if missing:
-				msgprint(_("Missing Currency Exchange Rates for {0}").format(comma_and(missing)),
-					raise_exception=ShoppingCartSetupError)
+			if not flt(exchange_rate):
+				msg = f"Missing Currency Exchange Rates for {from_currency}-{to_currency}"
+				frappe.throw(_(msg), title=_("Missing"), exc=ShoppingCartSetupError)
 
 	def validate_tax_rule(self):
 		if not frappe.db.get_value("Tax Rule", {"use_for_shopping_cart" : 1}, "name"):
@@ -71,7 +60,7 @@ class ShoppingCartSettings(Document):
 	def get_shipping_rules(self, shipping_territory):
 		return self.get_name_from_territory(shipping_territory, "shipping_rules", "shipping_rule")
 
-def validate_cart_settings(doc, method):
+def validate_cart_settings(doc=None, method=None):
 	frappe.get_doc("Shopping Cart Settings", "Shopping Cart Settings").run_method("validate")
 
 def get_shopping_cart_settings():

erpnext/shopping_cart/doctype/shopping_cart_settings/test_shopping_cart_settings.py

@@ -16,17 +16,25 @@ class TestShoppingCartSettings(unittest.TestCase):
 		return frappe.get_doc({"doctype": "Shopping Cart Settings",
 			"company": "_Test Company"})
 
-	def test_exchange_rate_exists(self):
-		frappe.db.sql("""delete from `tabCurrency Exchange`""")
+	# NOTE: Exchangrate API has all enabled currencies that ERPNext supports.
+	# We aren't checking just currency exchange record anymore
+	# while validating price list currency exchange rate to that of company.
+	# The API is being used to fetch the rate which again almost always
+	# gives back a valid value (for valid currencies).
+	# This makes the test obsolete.
+	# Commenting because im not sure if there's a better test we can write
 
-		cart_settings = self.get_cart_settings()
-		cart_settings.price_list = "_Test Price List Rest of the World"
-		self.assertRaises(ShoppingCartSetupError, cart_settings.validate_exchange_rates_exist)
+	# def test_exchange_rate_exists(self):
+	# 	frappe.db.sql("""delete from `tabCurrency Exchange`""")
 
-		from erpnext.setup.doctype.currency_exchange.test_currency_exchange import test_records as \
-			currency_exchange_records
-		frappe.get_doc(currency_exchange_records[0]).insert()
-		cart_settings.validate_exchange_rates_exist()
+	# 	cart_settings = self.get_cart_settings()
+	# 	cart_settings.price_list = "_Test Price List Rest of the World"
+	# 	self.assertRaises(ShoppingCartSetupError, cart_settings.validate_price_list_exchange_rate)
+
+	# 	from erpnext.setup.doctype.currency_exchange.test_currency_exchange import test_records as \
+	# 		currency_exchange_records
+	# 	frappe.get_doc(currency_exchange_records[0]).insert()
+	# 	cart_settings.validate_price_list_exchange_rate()
 
 	def test_tax_rule_validation(self):
 		frappe.db.sql("update `tabTax Rule` set use_for_shopping_cart = 0")

erpnext/stock/doctype/price_list/price_list.py

@@ -13,6 +13,9 @@ class PriceList(Document):
 		if not cint(self.buying) and not cint(self.selling):
 			throw(_("Price List must be applicable for Buying or Selling"))
 
+		if not self.is_new():
+			self.check_impact_on_shopping_cart()
+
 	def on_update(self):
 		self.set_default_if_missing()
 		self.update_item_price()
@@ -32,6 +35,17 @@ class PriceList(Document):
 			buying=%s, selling=%s, modified=NOW() where price_list=%s""",
 			(self.currency, cint(self.buying), cint(self.selling), self.name))
 
+	def check_impact_on_shopping_cart(self):
+		"Check if Price List currency change impacts Shopping Cart."
+		from erpnext.shopping_cart.doctype.shopping_cart_settings.shopping_cart_settings import validate_cart_settings
+
+		doc_before_save = self.get_doc_before_save()
+		currency_changed = self.currency != doc_before_save.currency
+		affects_cart = self.name == frappe.get_cached_value("Shopping Cart Settings", None, "price_list")
+
+		if currency_changed and affects_cart:
+			validate_cart_settings()
+
 	def on_trash(self):
 		self.delete_price_list_details_key()