feat: skip notification creation if context is not valid (#34133)

openedx/core/djangoapps/notifications/tasks.py

@@ -7,13 +7,17 @@
 from celery import shared_task
 from celery.utils.log import get_task_logger
 from django.conf import settings
+from django.core.exceptions import ValidationError
 from django.db import transaction
 from edx_django_utils.monitoring import set_code_owner_attribute
 from opaque_keys.edx.keys import CourseKey
 from pytz import UTC
 
 from common.djangoapps.student.models import CourseEnrollment
-from openedx.core.djangoapps.notifications.base_notification import get_default_values_of_preference
+from openedx.core.djangoapps.notifications.base_notification import (
+    get_default_values_of_preference,
+    get_notification_content
+)
 from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFICATIONS_FILTERS
 from openedx.core.djangoapps.notifications.events import notification_generated_event
 from openedx.core.djangoapps.notifications.filters import NotificationFilter
@@ -93,6 +97,9 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
     if not ENABLE_NOTIFICATIONS.is_enabled(course_key):
         return
 
+    if not is_notification_valid(notification_type, context):
+        raise ValidationError(f"Notification is not valid {app_name} {notification_type} {context}")
+
     user_ids = list(set(user_ids))
     batch_size = settings.NOTIFICATION_CREATION_BATCH_SIZE
 
@@ -157,6 +164,17 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
         )
 
 
+def is_notification_valid(notification_type, context):
+    """
+    Validates notification before creation
+    """
+    try:
+        get_notification_content(notification_type, context)
+    except Exception:        # pylint: disable=broad-except
+        return False
+    return True
+
+
 def update_user_preference(preference: CourseNotificationPreference, user_id, course_id):
     """
     Update user preference if config version is changed.

openedx/core/djangoapps/notifications/tests/test_tasks.py

@@ -5,6 +5,7 @@
 from unittest.mock import patch
 
 import ddt
+from django.core.exceptions import ValidationError
 from django.conf import settings
 from edx_toggles.toggles.testutils import override_waffle_flag
 
@@ -206,6 +207,13 @@ def test_send_with_app_disabled_notifications(self, app_name, notification_type)
         notification = Notification.objects.filter(user_id=self.user.id).first()
         self.assertIsNone(notification)
 
+    @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)
+    def test_notification_not_created_when_context_is_incomplete(self):
+        try:
+            send_notifications([self.user.id], str(self.course_1.id), "discussion", "new_comment", {}, "")
+        except Exception as exc:    # pylint: disable=broad-except
+            assert isinstance(exc, ValidationError)
+
 
 @ddt.ddt
 @patch('openedx.core.djangoapps.notifications.tasks.ENABLE_NOTIFICATIONS_FILTERS.is_enabled', lambda x: False)
@@ -345,7 +353,9 @@ def test_preference_enabled_in_batch_audience(self, notification_type,
         app_name = "discussion"
         context = {
             'post_title': 'Post title',
+            'username': 'Username',
             'replier_name': 'replier name',
+            'author_name': 'Authorname'
         }
         content_url = 'https://example.com/'
         send_notifications(user_ids, str(self.course.id), app_name, notification_type, context, content_url)