From 90b039273dcea6ed52f3b5c8dc9830fd609fc38e Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Fri, 13 Oct 2023 11:23:30 +0500 Subject: [PATCH] feat: added audit track expiry filter in notifications (#33381) * feat: added audit track expiry filter in notifications --- .../notifications/base_notification.py | 14 ++- .../djangoapps/notifications/config/waffle.py | 10 ++ .../core/djangoapps/notifications/filters.py | 67 +++++++++++++ .../core/djangoapps/notifications/tasks.py | 45 +++++---- .../notifications/tests/test_filters.py | 96 +++++++++++++++++++ .../notifications/tests/test_tasks.py | 1 + 6 files changed, 211 insertions(+), 22 deletions(-) create mode 100644 openedx/core/djangoapps/notifications/filters.py create mode 100644 openedx/core/djangoapps/notifications/tests/test_filters.py diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index b7ebe67416ae..5541b542f8af 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -3,10 +3,9 @@ """ from django.utils.translation import gettext_lazy as _ -from .utils import ( - find_app_in_normalized_apps, - find_pref_in_normalized_prefs, -) +from .utils import find_app_in_normalized_apps, find_pref_in_normalized_prefs + +FILTER_AUDIT_EXPIRED = 'filter_audit_expired' COURSE_NOTIFICATION_TYPES = { 'new_comment_on_response': { @@ -20,6 +19,7 @@ 'replier_name': 'replier name', }, 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED] }, 'new_comment': { 'notification_app': 'discussion', @@ -33,6 +33,7 @@ 'replier_name': 'replier name', }, 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED] }, 'new_response': { 'notification_app': 'discussion', @@ -45,6 +46,7 @@ 'replier_name': 'replier name', }, 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED] }, 'new_discussion_post': { 'notification_app': 'discussion', @@ -61,6 +63,7 @@ 'username': 'Post author name', }, 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED] }, 'new_question_post': { 'notification_app': 'discussion', @@ -77,6 +80,7 @@ 'username': 'Post author name', }, 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED] }, 'response_on_followed_post': { 'notification_app': 'discussion', @@ -91,6 +95,7 @@ 'replier_name': 'replier name', }, 'email_template': '', + 'filter': [FILTER_AUDIT_EXPIRED] }, 'comment_on_followed_post': { 'notification_app': 'discussion', @@ -106,6 +111,7 @@ 'replier_name': 'replier name', }, 'email_template': '', + 'filter': [FILTER_AUDIT_EXPIRED] }, } diff --git a/openedx/core/djangoapps/notifications/config/waffle.py b/openedx/core/djangoapps/notifications/config/waffle.py index fdfd74571c47..e05487b11bcf 100644 --- a/openedx/core/djangoapps/notifications/config/waffle.py +++ b/openedx/core/djangoapps/notifications/config/waffle.py @@ -27,3 +27,13 @@ # .. toggle_target_removal_date: 2023-12-07 # .. toggle_tickets: INF-902 SHOW_NOTIFICATIONS_TRAY = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.show_notifications_tray", __name__) + +# .. toggle_name: notifications.enable_notifications_filters +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable filters in notifications task +# .. toggle_use_cases: temporary, open_edx +# .. toggle_creation_date: 2023-06-07 +# .. toggle_target_removal_date: 2024-06-01 +# .. toggle_tickets: INF-902 +ENABLE_NOTIFICATIONS_FILTERS = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_notifications_filters", __name__) diff --git a/openedx/core/djangoapps/notifications/filters.py b/openedx/core/djangoapps/notifications/filters.py new file mode 100644 index 000000000000..9176faa6ee5e --- /dev/null +++ b/openedx/core/djangoapps/notifications/filters.py @@ -0,0 +1,67 @@ +""" +Notification filters +""" +import logging + +from django.utils import timezone + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.models import CourseEnrollment +from openedx.core.djangoapps.course_date_signals.utils import get_expected_duration +from openedx.core.djangoapps.notifications.base_notification import COURSE_NOTIFICATION_TYPES +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig +from xmodule.modulestore.django import modulestore + +logger = logging.getLogger(__name__) + + +class NotificationFilter: + """ + Filter notifications based on their type + """ + + @staticmethod + def filter_audit_expired(user_ids, course) -> list: + """ + Check if the user has access to the course + """ + verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True) + access_duration = get_expected_duration(course.id) + course_time_limit = CourseDurationLimitConfig.current(course_key=course.id) + if not verified_mode: + logger.info( + "Course %s does not have a verified mode, so no users will be filtered out", + course.id, + ) + return user_ids + enrollments = CourseEnrollment.objects.filter( + user_id__in=user_ids, + course_id=course.id, + mode=CourseMode.AUDIT, + ) + if course_time_limit.enabled_for_course(course.id): + enrollments = enrollments.filter(created__gte=course_time_limit.enabled_as_of) + + for enrollment in enrollments: + content_availability_date = max(enrollment.created, course.start) + expiration_date = content_availability_date + access_duration + if expiration_date and timezone.now() > expiration_date: + logger.info("User %s has expired audit access to course %s", enrollment.user_id, course.id) + user_ids.remove(enrollment.user_id) + return user_ids + + def apply_filters(self, user_ids, course_key, notification_type) -> list: + """ + Apply all the filters + """ + notification_config = COURSE_NOTIFICATION_TYPES.get(notification_type, {}) + applicable_filters = notification_config.get('filters', []) + course = modulestore().get_course(course_key) + for filter_name in applicable_filters: + logger.info( + "Applying filter %s for notification type %s", + filter_name, + notification_type, + ) + user_ids = getattr(self, filter_name)(user_ids, course) + return user_ids diff --git a/openedx/core/djangoapps/notifications/tasks.py b/openedx/core/djangoapps/notifications/tasks.py index 1401a2d9ab17..4d9a240095fa 100644 --- a/openedx/core/djangoapps/notifications/tasks.py +++ b/openedx/core/djangoapps/notifications/tasks.py @@ -14,8 +14,9 @@ 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.config.waffle import ENABLE_NOTIFICATIONS +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 from openedx.core.djangoapps.notifications.models import ( CourseNotificationPreference, Notification, @@ -94,7 +95,6 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c user_ids = list(set(user_ids)) batch_size = settings.NOTIFICATION_CREATION_BATCH_SIZE - audience = [] notifications_generated = False notification_content = '' default_web_config = get_default_values_of_preference(app_name, notification_type).get('web', True) @@ -109,35 +109,44 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c if default_web_config: preferences = create_notification_pref_if_not_exists(batch_user_ids, preferences, course_key) - notifications = [] + if not preferences: + return + for preference in preferences: preference = update_user_preference(preference, preference.user_id, course_key) - if ( + if not ( preference and preference.get_web_config(app_name, notification_type) and preference.get_app_config(app_name).get('enabled', False) ): - notifications.append( - Notification( - user_id=preference.user_id, - app_name=app_name, - notification_type=notification_type, - content_context=context, - content_url=content_url, - course_id=course_key, - ) + batch_user_ids.remove(preference.user_id) + if ENABLE_NOTIFICATIONS_FILTERS.is_enabled(course_key): + logger.info(f'Sending notifications to {len(batch_user_ids)} users.') + batch_user_ids = NotificationFilter().apply_filters(batch_user_ids, course_key, notification_type) + logger.info(f'After applying filters, sending notifications to {len(batch_user_ids)} users.') + + notifications = [] + for user_id in batch_user_ids: + notifications.append( + Notification( + user_id=user_id, + app_name=app_name, + notification_type=notification_type, + content_context=context, + content_url=content_url, + course_id=course_key, ) - audience.append(preference.user_id) + ) # send notification to users but use bulk_create notification_objects = Notification.objects.bulk_create(notifications) if notification_objects and not notifications_generated: notifications_generated = True notification_content = notification_objects[0].content - if notifications_generated: - notification_generated_event( - audience, app_name, notification_type, course_key, content_url, notification_content, - ) + if notifications_generated: + notification_generated_event( + batch_user_ids, app_name, notification_type, course_key, content_url, notification_content, + ) def update_user_preference(preference: CourseNotificationPreference, user_id, course_id): diff --git a/openedx/core/djangoapps/notifications/tests/test_filters.py b/openedx/core/djangoapps/notifications/tests/test_filters.py new file mode 100644 index 000000000000..d1aff28de702 --- /dev/null +++ b/openedx/core/djangoapps/notifications/tests/test_filters.py @@ -0,0 +1,96 @@ +""" +Test for the NotificationFilter class. +""" +from datetime import timedelta +from unittest import mock + +import ddt +from django.utils.timezone import now + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.notifications.filters import NotificationFilter +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig +from openedx.features.course_experience.tests.views.helpers import add_course_mode +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@ddt.ddt +class CourseExpirationTestCase(ModuleStoreTestCase): + """Tests to verify the get_user_course_expiration_date function is working correctly""" + + def setUp(self): + super().setUp() # lint-amnesty, pylint: disable=super-with-arguments + self.course = CourseFactory( + start=now() - timedelta(weeks=10), + ) + + self.user = UserFactory() + self.user_1 = UserFactory() + + # Make this a verified course, so we can test expiration date + add_course_mode(self.course, mode_slug=CourseMode.AUDIT) + add_course_mode(self.course) + CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) + expired_audit = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) + expired_audit.created = now() - timedelta(weeks=6) + expired_audit.save() + + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") + def test_audit_expired_filter( + self, + mock_get_course_run_details, + ): + """ + Test if filter_audit_expired function is working correctly + """ + + mock_get_course_run_details.return_value = {'weeks_to_complete': 4} + result = NotificationFilter.filter_audit_expired( + [self.user.id, self.user_1.id], + self.course, + ) + self.assertEqual([self.user_1.id], result) + + mock_get_course_run_details.return_value = {'weeks_to_complete': 7} + result = NotificationFilter.filter_audit_expired( + [self.user.id, self.user_1.id], + self.course, + ) + self.assertEqual([self.user.id, self.user_1.id], result) + + CourseDurationLimitConfig.objects.create( + enabled=True, + course=CourseOverview.get_from_id(self.course.id), + enabled_as_of=now(), + ) + # weeks_to_complete is set to 4 because we want to test if CourseDurationLimitConfig is working correctly. + mock_get_course_run_details.return_value = {'weeks_to_complete': 4} + result = NotificationFilter.filter_audit_expired( + [self.user.id, self.user_1.id], + self.course, + ) + self.assertEqual([self.user.id, self.user_1.id], result) + + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") + @mock.patch("openedx.core.djangoapps.notifications.filters.NotificationFilter.filter_audit_expired") + def test_apply_filter( + self, + mock_filter_audit_expired, + mock_get_course_run_details, + ): + """ + Test if apply_filter function is working correctly + """ + mock_get_course_run_details.return_value = {'weeks_to_complete': 4} + mock_filter_audit_expired.return_value = [self.user.id, self.user_1.id] + result = NotificationFilter().apply_filters( + [self.user.id, self.user_1.id], + self.course.id, + 'new_comment_on_response' + ) + self.assertEqual([self.user.id, self.user_1.id], result) + mock_filter_audit_expired.assert_called_once() diff --git a/openedx/core/djangoapps/notifications/tests/test_tasks.py b/openedx/core/djangoapps/notifications/tests/test_tasks.py index 3164f9a9bae2..339ce0e03f35 100644 --- a/openedx/core/djangoapps/notifications/tests/test_tasks.py +++ b/openedx/core/djangoapps/notifications/tests/test_tasks.py @@ -164,6 +164,7 @@ def test_send_with_app_disabled_notifications(self, app_name, notification_type) @ddt.ddt +@patch('openedx.core.djangoapps.notifications.tasks.ENABLE_NOTIFICATIONS_FILTERS.is_enabled', lambda x: False) class SendBatchNotificationsTest(ModuleStoreTestCase): """ Test that notification and notification preferences are created in batches