fix: extra notification triggered on value change (#17364)

Since value in DB and value on object can be in different type the
notification gets triggered unnecessarily

e.g. data(2012, 10, 10) != "2012-10-10"

frappe/email/doctype/notification/notification.py

@@ -13,7 +13,7 @@ from frappe.desk.doctype.notification_log.notification_log import enqueue_create
 from frappe.integrations.doctype.slack_webhook_url.slack_webhook_url import send_slack_message
 from frappe.model.document import Document
 from frappe.modules.utils import export_module_json, get_doc_module
-from frappe.utils import add_to_date, is_html, nowdate, parse_val, validate_email_address
+from frappe.utils import add_to_date, cast, is_html, nowdate, validate_email_address
 from frappe.utils.jinja import validate_template
 from frappe.utils.safe_exec import get_safe_globals
 
@@ -417,7 +417,7 @@ def trigger_notifications(doc, method=None):
 				frappe.db.commit()
 
 
-def evaluate_alert(doc, alert, event):
+def evaluate_alert(doc: Document, alert, event):
 	from jinja2 import TemplateError
 
 	try:
@@ -439,8 +439,8 @@ def evaluate_alert(doc, alert, event):
 			doc_before_save = doc.get_doc_before_save()
 			field_value_before_save = doc_before_save.get(alert.value_changed) if doc_before_save else None
 
-			field_value_before_save = parse_val(field_value_before_save)
-			if doc.get(alert.value_changed) == field_value_before_save:
+			fieldtype = doc.meta.get_field(alert.value_changed).fieldtype
+			if cast(fieldtype, doc.get(alert.value_changed)) == cast(fieldtype, field_value_before_save):
 				# value not changed
 				return
 

frappe/email/doctype/notification/test_notification.py

@@ -2,6 +2,7 @@
 # Copyright (c) 2018, Frappe Technologies and Contributors
 # License: MIT. See LICENSE
 import unittest
+from contextlib import contextmanager
 
 import frappe
 import frappe.utils
@@ -11,6 +12,15 @@ from frappe.desk.form import assign_to
 test_dependencies = ["User", "Notification"]
 
 
+@contextmanager
+def get_test_notification(config):
+	try:
+		notification = frappe.get_doc(doctype="Notification", **config).insert()
+		yield notification
+	finally:
+		notification.delete()
+
+
 class TestNotification(unittest.TestCase):
 	def setUp(self):
 		frappe.db.delete("Email Queue")
@@ -345,6 +355,31 @@ class TestNotification(unittest.TestCase):
 		self.assertTrue("test2@example.com" in recipients)
 		self.assertTrue("test1@example.com" in recipients)
 
+	def test_notification_value_change_casted_types(self):
+		"""Make sure value change event dont fire because of incorrect type comparisons."""
+		frappe.set_user("Administrator")
+
+		notification = {
+			"document_type": "User",
+			"subject": "User changed birthdate",
+			"event": "Value Change",
+			"channel": "System Notification",
+			"value_changed": "birth_date",
+			"recipients": [{"receiver_by_document_field": "email"}],
+		}
+
+		with get_test_notification(notification) as n:
+			frappe.db.delete("Notification Log", {"subject": n.subject})
+
+			user = frappe.get_doc("User", "test@example.com")
+			user.birth_date = frappe.utils.add_days(user.birth_date, 1)
+			user.save()
+
+			user.reload()
+			user.birth_date = frappe.utils.getdate(user.birth_date)
+			user.save()
+			self.assertEqual(1, frappe.db.count("Notification Log", {"subject": n.subject}))
+
 	@classmethod
 	def tearDownClass(cls):
 		frappe.delete_doc_if_exists("Notification", "ToDo Status Update")

frappe/utils/data.py

@@ -801,7 +801,7 @@ def has_common(l1: typing.Hashable, l2: typing.Hashable) -> bool:
 def cast_fieldtype(fieldtype, value, show_warning=True):
 	if show_warning:
 		message = (
-			"Function `frappe.utils.data.cast` has been deprecated in favour"
+			"Function `frappe.utils.data.cast_fieldtype` has been deprecated in favour"
 			" of `frappe.utils.data.cast`. Use the newer util for safer type casting."
 		)
 		secho(message, fg="yellow")