Skip to content

Commit

Permalink
feat: updated filter logic to send notifications (#33743)
Browse files Browse the repository at this point in the history 
* feat: updated filter logic to send notifications
  • Loading branch information
@AhtishamShahid
AhtishamShahid authored Nov 21, 2023
1 parent 3fc8e5a commit 7165bb5
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 16 deletions.
16 changes: 8 additions & 8 deletions openedx/core/djangoapps/notifications/base_notification.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from .utils import find_app_in_normalized_apps, find_pref_in_normalized_prefs

FILTER_AUDIT_EXPIRED = 'filter_audit_expired'
FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE = 'filter_audit_expired_users_with_no_role'

COURSE_NOTIFICATION_TYPES = {
'new_comment_on_response': {
Expand All @@ -19,7 +19,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
'new_comment': {
'notification_app': 'discussion',
Expand All @@ -33,7 +33,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
'new_response': {
'notification_app': 'discussion',
Expand All @@ -46,7 +46,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
'new_discussion_post': {
'notification_app': 'discussion',
Expand All @@ -63,7 +63,7 @@
'username': 'Post author name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
'new_question_post': {
'notification_app': 'discussion',
Expand All @@ -80,7 +80,7 @@
'username': 'Post author name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
'response_on_followed_post': {
'notification_app': 'discussion',
Expand All @@ -95,7 +95,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
'comment_on_followed_post': {
'notification_app': 'discussion',
Expand All @@ -111,7 +111,7 @@
'replier_name': 'replier name',
},
'email_template': '',
'filters': [FILTER_AUDIT_EXPIRED]
'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE]
},
}

Expand Down
55 changes: 52 additions & 3 deletions openedx/core/djangoapps/notifications/filters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@
Notification filters
"""
import logging
from typing import List

from django.utils import timezone

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment
from openedx.core.djangoapps.course_date_signals.utils import get_expected_duration
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_MODERATOR,
Role
)
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
Expand All @@ -21,9 +29,36 @@ class NotificationFilter:
"""

@staticmethod
def filter_audit_expired(user_ids, course) -> list:
def get_users_with_course_role(user_ids: List[int], course_id: str) -> List[int]:
"""
Check if the user has access to the course
Get users with a course role for the given course.
"""
return CourseAccessRole.objects.filter(
user_id__in=user_ids,
course_id=course_id,
).values_list('user_id', flat=True)

@staticmethod
def get_users_with_forum_roles(user_ids: List[int], course_id: str) -> List[int]:
"""
Get users with forum roles for the given course.
"""
return Role.objects.filter(

course_id=course_id,
users__id__in=user_ids,
name__in=[
FORUM_ROLE_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_GROUP_MODERATOR,
]

).values_list('users__id', flat=True)

def filter_audit_expired_users_with_no_role(self, user_ids, course) -> list:
"""
Check if the user has access to the course this would be true if the user has a course role or a forum role
"""
verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True)
access_duration = get_expected_duration(course.id)
Expand All @@ -34,15 +69,29 @@ def filter_audit_expired(user_ids, course) -> list:
course.id,
)
return user_ids

users_with_course_role = self.get_users_with_course_role(user_ids, course.id)
users_with_forum_roles = self.get_users_with_forum_roles(user_ids, course.id)
enrollments = CourseEnrollment.objects.filter(
user_id__in=user_ids,
course_id=course.id,
mode=CourseMode.AUDIT,
user__is_staff=False,
)

if course_time_limit.enabled_for_course(course.id):
enrollments = enrollments.filter(created__gte=course_time_limit.enabled_as_of)
logger.info("NotificationFilter: Number of audit enrollments for course %s: %s", course.id, enrollments.count())

for enrollment in enrollments:
if enrollment.user_id in users_with_course_role or enrollment.user_id in users_with_forum_roles:
logger.info(
"NotificationFilter: User %s has a course or forum role for course %s, so they will not be "
"filtered out",
enrollment.user_id,
course.id,
)
continue
content_availability_date = max(enrollment.created, course.start)
expiration_date = content_availability_date + access_duration
logger.info("NotificationFilter: content_availability_date: %s and access_duration: %s",
Expand Down
72 changes: 67 additions & 5 deletions openedx/core/djangoapps/notifications/tests/test_filters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,16 @@

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseInstructorRole
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_MODERATOR,
Role
)
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
Expand Down Expand Up @@ -40,7 +48,7 @@ def setUp(self):
expired_audit.save()

@mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details")
def test_audit_expired_filter(
def test_audit_expired_filter_with_no_role(
self,
mock_get_course_run_details,
):
Expand All @@ -49,14 +57,14 @@ def test_audit_expired_filter(
"""

mock_get_course_run_details.return_value = {'weeks_to_complete': 4}
result = NotificationFilter.filter_audit_expired(
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[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(
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[self.user.id, self.user_1.id],
self.course,
)
Expand All @@ -69,14 +77,15 @@ def test_audit_expired_filter(
)
# 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(
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[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")
@mock.patch(
"openedx.core.djangoapps.notifications.filters.NotificationFilter.filter_audit_expired_users_with_no_role")
def test_apply_filter(
self,
mock_filter_audit_expired,
Expand All @@ -94,3 +103,56 @@ def test_apply_filter(
)
self.assertEqual([self.user.id, self.user_1.id], result)
mock_filter_audit_expired.assert_called_once()

@mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details")
def test_audit_expired_for_course_staff(
self,
mock_get_course_run_details,
):
"""
Test if filter_audit_expired function is working correctly for course staff
"""

mock_get_course_run_details.return_value = {'weeks_to_complete': 4}
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[self.user.id, self.user_1.id],
self.course,
)
self.assertEqual([self.user_1.id], result)
CourseInstructorRole(self.course.id).add_users(self.user)
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[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")
@ddt.data(
FORUM_ROLE_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_GROUP_MODERATOR,
)
def test_audit_expired_for_forum_roles(
self,
role_name,
mock_get_course_run_details,

):
"""
Test if filter_audit_expired function is working correctly for forum roles
"""

mock_get_course_run_details.return_value = {'weeks_to_complete': 4}
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[self.user.id, self.user_1.id],
self.course,
)
self.assertEqual([self.user_1.id], result)
role = Role.objects.get_or_create(course_id=self.course.id, name=role_name)[0]
role.users.add(self.user.id)
result = NotificationFilter().filter_audit_expired_users_with_no_role(
[self.user.id, self.user_1.id],
self.course,
)
self.assertEqual([self.user.id, self.user_1.id], result)

0 comments on commit 7165bb5

Please sign in to comment.