diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index 2a3766d41c..7038d35990 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -7,13 +7,17 @@ from typing import List 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. diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks.py b/openedx/core/djangoapps/notifications/tests/test_tasks.py index 054f445cd9..bc3aa6a2e4 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks.py @@ -5,6 +5,7 @@ Tests for notifications tasks. 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 @@ class SendNotificationsTest(ModuleStoreTestCase): 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 @@ class SendBatchNotificationsTest(ModuleStoreTestCase): 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)