Skip to content

Commit

Permalink
feat: added audit track expiry filter in notifications (#33381)
Browse files Browse the repository at this point in the history
* feat: added audit track expiry filter in notifications
  • Loading branch information
AhtishamShahid authored Oct 13, 2023
1 parent 4acd6c2 commit 90b0392
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 22 deletions.
14 changes: 10 additions & 4 deletions openedx/core/djangoapps/notifications/base_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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': {
Expand All @@ -20,6 +19,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
},
'new_comment': {
'notification_app': 'discussion',
Expand All @@ -33,6 +33,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
},
'new_response': {
'notification_app': 'discussion',
Expand All @@ -45,6 +46,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
},
'new_discussion_post': {
'notification_app': 'discussion',
Expand All @@ -61,6 +63,7 @@
'username': 'Post author name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
},
'new_question_post': {
'notification_app': 'discussion',
Expand All @@ -77,6 +80,7 @@
'username': 'Post author name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
},
'response_on_followed_post': {
'notification_app': 'discussion',
Expand All @@ -91,6 +95,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filter': [FILTER_AUDIT_EXPIRED]
},
'comment_on_followed_post': {
'notification_app': 'discussion',
Expand All @@ -106,6 +111,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filter': [FILTER_AUDIT_EXPIRED]
},
}

Expand Down
10 changes: 10 additions & 0 deletions openedx/core/djangoapps/notifications/config/waffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
67 changes: 67 additions & 0 deletions openedx/core/djangoapps/notifications/filters.py
Original file line number Diff line number Diff line change
@@ -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
45 changes: 27 additions & 18 deletions openedx/core/djangoapps/notifications/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down
96 changes: 96 additions & 0 deletions openedx/core/djangoapps/notifications/tests/test_filters.py
Original file line number Diff line number Diff line change
@@ -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()
1 change: 1 addition & 0 deletions openedx/core/djangoapps/notifications/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 90b0392

Please sign in to comment.