Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: move new forum content creation notifications to use course wide notification event

Browse Source
This commit is contained in:
SaadYousaf
2023-12-11 14:43:32 +05:00
committed by Saad Yousaf
parent 17a58cc9a0
commit 2d353f5594
5 changed files with 304 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
lms/djangoapps/discussion/rest_api/discussions_notifications.py
View File

@@ -3,12 +3,10 @@ Discussion notifications sender util.
"""
from django.conf import settings
from lms.djangoapps.discussion.django_comment_client.permissions import get_team
from openedx_events.learning.data import UserNotificationData
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED
from openedx_events.learning.data import UserNotificationData, CourseNotificationData
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED, COURSE_NOTIFICATION_REQUESTED
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
from openedx.core.djangoapps.course_groups.models import CourseCohortsSettings, CourseUserGroup
from openedx.core.djangoapps.course_groups.models import CourseCohortsSettings
from openedx.core.djangoapps.discussions.utils import get_divided_discussions
from django.utils.translation import gettext_lazy as _
@@ -19,7 +17,6 @@ from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_MODERATOR,
CourseDiscussionSettings,
Role
)
@@ -62,6 +59,29 @@ class DiscussionNotificationSender:
)
USER_NOTIFICATION_REQUESTED.send_event(notification_data=notification_data)
def _send_course_wide_notification(self, notification_type, audience_filters=None, extra_context=None):
"""
Send notification to all users in the course
"""
if not extra_context:
extra_context = {}
notification_data = CourseNotificationData(
course_key=self.course.id,
content_context={
"replier_name": self.creator.username,
"post_title": self.thread.title,
"course_name": self.course.display_name,
"sender_id": self.creator.id,
**extra_context,
},
notification_type=notification_type,
content_url=f"{settings.DISCUSSIONS_MICROFRONTEND_URL}/{str(self.course.id)}/posts/{self.thread.id}",
app_name="discussion",
audience_filters=audience_filters,
)
COURSE_NOTIFICATION_REQUESTED.send_event(course_notification_data=notification_data)
def _get_parent_response(self):
"""
Get parent response object
@@ -175,7 +195,7 @@ class DiscussionNotificationSender:
def _create_cohort_course_audience(self):
"""
Creates audience based on user cohort and role
Creates audience filter based on user cohort and role
"""
course_key_str = str(self.course.id)
discussion_cohorted = is_discussion_cohorted(course_key_str)
@@ -193,43 +213,21 @@ class DiscussionNotificationSender:
group_id = int(group_id)
# Course wide topics
topic_id = self.thread.attributes['commentable_id']
all_topics = divided_inline_discussions + divided_course_wide_discussions
topic_id = self.thread.attributes['commentable_id']
topic_divided = topic_id in all_topics or discussion_settings.always_divide_inline_discussions
# Team object from topic id
team = get_team(topic_id)
user_ids = []
if team:
user_ids = team.users.all().values_list('id', flat=True)
elif discussion_cohorted and topic_divided and group_id is not None:
users_in_cohort = CourseUserGroup.objects.filter(
course_id=course_key_str, id=group_id
).values_list('users__id', flat=True)
user_ids.extend(users_in_cohort)
privileged_roles = [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]
privileged_users = Role.objects.filter(
name__in=privileged_roles,
course_id=course_key_str
).values_list('users__id', flat=True)
user_ids.extend(privileged_users)
staff_users = CourseStaffRole(self.course.id).users_with_role().values_list('id', flat=True)
user_ids.extend(staff_users)
admin_users = CourseInstructorRole(self.course.id).users_with_role().values_list('id', flat=True)
user_ids.extend(admin_users)
else:
user_ids = CourseEnrollment.objects.filter(
course__id=course_key_str, is_active=True
).values_list('user__id', flat=True)
unique_user_ids = list(set(user_ids))
if self.creator.id in unique_user_ids:
unique_user_ids.remove(self.creator.id)
return unique_user_ids
return {
'teams': [team.team_id],
}
if discussion_cohorted and topic_divided and group_id is not None:
return {
'cohorts': [group_id],
}
return {}
def send_new_thread_created_notification(self):
"""
@@ -244,12 +242,22 @@ class DiscussionNotificationSender:
if notification_type not in ['new_discussion_post', 'new_question_post']:
raise ValueError(f'Invalid notification type {notification_type}')
user_ids = self._create_cohort_course_audience()
audience_filters = self._create_cohort_course_audience()
if audience_filters:
# If the audience is cohorted/teamed, we add the course and forum roles to the audience.
# Include course staff and instructors for course wide discussion notifications.
audience_filters['course_roles'] = ['staff', 'instructor']
# Include privileged forum roles for course wide discussion notifications.
audience_filters['discussion_roles'] = [
FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA
]
context = {
'username': self.creator.username,
'post_title': self.thread.title
}
self._send_notification(user_ids, notification_type, context)
self._send_course_wide_notification(notification_type, audience_filters, context)
def is_discussion_cohorted(course_key_str):

45
lms/djangoapps/discussion/rest_api/tests/test_tasks.py
View File

@@ -8,7 +8,7 @@ import ddt
import httpretty
from django.conf import settings
from edx_toggles.toggles.testutils import override_waffle_flag
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED, COURSE_NOTIFICATION_REQUESTED
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import StaffFactory, UserFactory
@@ -158,14 +158,6 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
self.register_get_thread_response(thread)
return thread
def assert_users_id_list(self, user_ids_1, user_ids_2):
"""
Assert whether the user ids in two lists are same
"""
assert len(user_ids_1) == len(user_ids_2)
for user_id in user_ids_1:
assert user_id in user_ids_2
def test_basic(self):
"""
Left empty intentionally. This test case is inherited from DiscussionAPIViewTestMixin
@@ -176,15 +168,6 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
Left empty intentionally. This test case is inherited from DiscussionAPIViewTestMixin
"""
def test_no_notification_if_course_has_no_enrollments(self):
"""
Tests no notification is send if course has no enrollments
"""
handler = mock.Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)
send_thread_created_notification(self.thread['id'], str(self.course.id), self.author.id)
self.assertEqual(handler.call_count, 0)
@ddt.data(
('new_question_post',),
('new_discussion_post',),
@@ -192,7 +175,7 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
@ddt.unpack
def test_notification_is_send_to_all_enrollments(self, notification_type):
"""
Tests notification is send to all users if course is not cohorted
Tests notification is sent to all users if course is not cohorted
"""
self._assign_enrollments()
thread_type = (
@@ -202,12 +185,13 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
)
thread = self._create_thread(thread_type=thread_type)
handler = mock.Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)
COURSE_NOTIFICATION_REQUESTED.connect(handler)
send_thread_created_notification(thread['id'], str(self.course.id), self.author.id)
self.assertEqual(handler.call_count, 1)
assert notification_type == handler.call_args[1]['notification_data'].notification_type
user_ids_list = [user.id for user in self.notification_to_all_users]
self.assert_users_id_list(user_ids_list, handler.call_args[1]['notification_data'].user_ids)
course_notification_data = handler.call_args[1]['course_notification_data']
assert notification_type == course_notification_data.notification_type
notification_audience_filters = {}
assert notification_audience_filters == course_notification_data.audience_filters
@ddt.data(
('cohort_1', 'new_question_post'),
@@ -218,7 +202,7 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
@ddt.unpack
def test_notification_is_send_to_cohort_ids(self, cohort_text, notification_type):
"""
Tests if notification is send only to privileged users and cohort members if the
Tests if notification is sent only to privileged users and cohort members if the
course is cohorted
"""
self._assign_enrollments()
@@ -238,12 +222,17 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe
cohort_id = cohort.id
thread = self._create_thread(group_id=cohort_id, thread_type=thread_type)
handler = mock.Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)
COURSE_NOTIFICATION_REQUESTED.connect(handler)
send_thread_created_notification(thread['id'], str(self.course.id), self.author.id)
assert notification_type == handler.call_args[1]['notification_data'].notification_type
course_notification_data = handler.call_args[1]['course_notification_data']
assert notification_type == course_notification_data.notification_type
notification_audience_filters = {
'cohorts': [cohort_id],
'course_roles': ['staff', 'instructor'],
'discussion_roles': ['Administrator', 'Moderator', 'Community TA'],
}
assert notification_audience_filters == handler.call_args[1]['course_notification_data'].audience_filters
self.assertEqual(handler.call_count, 1)
user_ids_list = [user.id for user in audience]
self.assert_users_id_list(user_ids_list, handler.call_args[1]['notification_data'].user_ids)
@ddt.ddt

74
openedx/core/djangoapps/notifications/audience_filters.py
View File

@@ -4,9 +4,14 @@ Audience based filters for notifications
from abc import abstractmethod
from opaque_keys.edx.keys import CourseKey
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole
from lms.djangoapps.discussion.django_comment_client.utils import get_users_with_roles
from lms.djangoapps.teams.models import CourseTeam
from openedx.core.djangoapps.course_groups.models import CourseUserGroup
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
@@ -33,11 +38,10 @@ class NotificationAudienceFilterBase:
pass
class RoleAudienceFilter(NotificationAudienceFilterBase):
class ForumRoleAudienceFilter(NotificationAudienceFilterBase):
"""
Filter class for roles
"""
# TODO: Add course roles to this. We currently support only forum roles
allowed_filters = [
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
@@ -55,6 +59,36 @@ class RoleAudienceFilter(NotificationAudienceFilterBase):
return [user.id for user in get_users_with_roles(roles, self.course_key)]
class CourseRoleAudienceFilter(NotificationAudienceFilterBase):
"""
Filter class for course roles
"""
allowed_filters = ['staff', 'instructor']
def filter(self, course_roles):
"""
Filter users based on their course roles
"""
if not self.is_valid_filter(course_roles):
raise ValueError(f'Invalid roles {course_roles} passed to CourseRoleAudienceFilter')
user_ids = []
course_key = self.course_key
if not isinstance(course_key, CourseKey):
course_key = CourseKey.from_string(course_key)
if 'staff' in course_roles:
staff_users = CourseStaffRole(course_key).users_with_role().values_list('id', flat=True)
user_ids.extend(staff_users)
if 'instructor' in course_roles:
instructor_users = CourseInstructorRole(course_key).users_with_role().values_list('id', flat=True)
user_ids.extend(instructor_users)
return user_ids
class EnrollmentAudienceFilter(NotificationAudienceFilterBase):
"""
Filter class for enrollment modes
@@ -71,3 +105,39 @@ class EnrollmentAudienceFilter(NotificationAudienceFilterBase):
course_id=self.course_key,
mode__in=enrollment_modes,
).values_list('user_id', flat=True)
class TeamAudienceFilter(NotificationAudienceFilterBase):
"""
Filter class for team roles
"""
def filter(self, team_ids):
"""
Filter users based on team id
"""
teams = CourseTeam.objects.filter(team_id__in=team_ids, course_id=self.course_key)
if not teams: # invalid team ids passed
raise ValueError(f'Invalid Team ids {team_ids} passed to TeamAudienceFilter for course {self.course_key}')
user_ids = []
for team in teams:
user_ids.extend(team.users.all().values_list('id', flat=True))
return user_ids
class CohortAudienceFilter(NotificationAudienceFilterBase):
"""
Filter class for cohort roles
"""
def filter(self, group_ids):
"""
Filter users based on their cohort ids
"""
users_in_cohort = CourseUserGroup.objects.filter(
course_id=self.course_key, id__in=group_ids
).values_list('users__id', flat=True)
return users_in_cohort

51
openedx/core/djangoapps/notifications/handlers.py
View File

@@ -14,17 +14,24 @@ from openedx_events.learning.signals import (
)
from common.djangoapps.student.models import CourseEnrollment
from openedx.core.djangoapps.notifications.audience_filters import RoleAudienceFilter, EnrollmentAudienceFilter
from openedx.core.djangoapps.notifications.audience_filters import (
ForumRoleAudienceFilter,
EnrollmentAudienceFilter,
TeamAudienceFilter,
CohortAudienceFilter,
CourseRoleAudienceFilter,
)
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
from openedx.core.djangoapps.notifications.models import CourseNotificationPreference
log = logging.getLogger(__name__)
AUDIENCE_FILTER_TYPES = ['role', 'enrollment']
AUDIENCE_FILTER_CLASSES = {
'role': RoleAudienceFilter,
'enrollment': EnrollmentAudienceFilter,
'discussion_roles': ForumRoleAudienceFilter,
'course_roles': CourseRoleAudienceFilter,
'enrollments': EnrollmentAudienceFilter,
'teams': TeamAudienceFilter,
'cohorts': CohortAudienceFilter,
}
@@ -76,11 +83,15 @@ def calculate_course_wide_notification_audience(course_key, audience_filters):
Calculate the audience for a course-wide notification based on the audience filters
"""
if not audience_filters:
return CourseEnrollment.objects.filter(course_id=course_key, is_active=True).values_list('user_id', flat=True)
active_enrollments = CourseEnrollment.objects.filter(
course_id=course_key,
is_active=True
).values_list('user_id', flat=True)
return list(active_enrollments)
audience_user_ids = []
for filter_type, filter_values in audience_filters.items():
if filter_type in AUDIENCE_FILTER_TYPES:
if filter_type in AUDIENCE_FILTER_CLASSES.keys(): # lint-amnesty, pylint: disable=consider-iterating-dictionary
filter_class = AUDIENCE_FILTER_CLASSES.get(filter_type)
if filter_class:
filter_instance = filter_class(course_key)
@@ -93,20 +104,24 @@ def calculate_course_wide_notification_audience(course_key, audience_filters):
@receiver(COURSE_NOTIFICATION_REQUESTED)
def generate_course_notifications(signal, sender, notification_data, metadata, **kwargs):
def generate_course_notifications(signal, sender, course_notification_data, metadata, **kwargs):
"""
Watches for COURSE_NOTIFICATION_REQUESTED signal and calls send_notifications task
"""
from openedx.core.djangoapps.notifications.tasks import send_notifications
notification_data = notification_data.__dict__
notification_data['course_key'] = str(notification_data['course_key'])
audience_filters = notification_data.pop('audience_filters')
user_ids = calculate_course_wide_notification_audience(
notification_data['course_key'],
audience_filters,
)
notification_data['user_ids'] = user_ids
notification_data['context'] = notification_data.pop('content_context')
from openedx.core.djangoapps.notifications.tasks import send_notifications
course_notification_data = course_notification_data.__dict__
notification_data = {
'course_key': str(course_notification_data['course_key']),
'user_ids': calculate_course_wide_notification_audience(
str(course_notification_data['course_key']),
course_notification_data['audience_filters'],
),
'context': course_notification_data.get('content_context'),
'app_name': course_notification_data.get('app_name'),
'notification_type': course_notification_data.get('notification_type'),
'content_url': course_notification_data.get('content_url'),
}
send_notifications.delay(**notification_data)

146
openedx/core/djangoapps/notifications/tests/test_filters.py
View File

@@ -9,9 +9,11 @@ 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.roles import CourseInstructorRole
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
from common.djangoapps.student.tests.factories import UserFactory, CourseEnrollmentFactory
from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
@@ -22,7 +24,10 @@ from openedx.core.djangoapps.django_comment_common.models import (
)
from openedx.core.djangoapps.notifications.audience_filters import (
EnrollmentAudienceFilter,
RoleAudienceFilter,
ForumRoleAudienceFilter,
CourseRoleAudienceFilter,
CohortAudienceFilter,
TeamAudienceFilter,
)
from openedx.core.djangoapps.notifications.filters import NotificationFilter
from openedx.core.djangoapps.notifications.handlers import calculate_course_wide_notification_audience
@@ -224,9 +229,9 @@ class TestEnrollmentAudienceFilter(ModuleStoreTestCase):
@ddt.ddt
class TestRoleAudienceFilter(ModuleStoreTestCase):
class TestForumRoleAudienceFilter(ModuleStoreTestCase):
"""
Tests for the RoleAudienceFilter.
Tests for the ForumRoleAudienceFilter.
"""
def setUp(self):
super().setUp() # lint-amnesty, pylint: disable=super-with-arguments
@@ -263,17 +268,134 @@ class TestRoleAudienceFilter(ModuleStoreTestCase):
)
@ddt.unpack
def test_valid_role_filter(self, role_names, expected_count):
role_filter = RoleAudienceFilter(self.course.id)
role_filter = ForumRoleAudienceFilter(self.course.id)
filtered_users = role_filter.filter(role_names)
self.assertEqual(len(filtered_users), expected_count)
def test_invalid_role_filter(self):
role_filter = RoleAudienceFilter(self.course.id)
role_filter = ForumRoleAudienceFilter(self.course.id)
role_names = ["INVALID_MODE"]
with self.assertRaises(ValueError):
role_filter.filter(role_names)
# TODO: Cleanup this test class
@ddt.ddt
class TestCourseRoleAudienceFilter(ModuleStoreTestCase):
"""
Tests for the CourseRoleAudienceFilter.
"""
def setUp(self):
super().setUp() # lint-amnesty, pylint: disable=super-with-arguments
self.course = CourseFactory()
self.students = [UserFactory() for _ in range(10)]
# Assign 5 users with course staff role
for student in self.students[:5]:
CourseStaffRole(self.course.id).add_users(student)
# Assign 5 users with course instructor role
for student in self.students[5:10]:
CourseInstructorRole(self.course.id).add_users(student)
@ddt.data(
(["instructor"], 5),
(["staff"], 5),
(["instructor", "staff"], 10),
)
@ddt.unpack
def test_valid_role_filter(self, role_names, expected_count):
course_role_filter = CourseRoleAudienceFilter(self.course.id)
filtered_users = course_role_filter.filter(role_names)
self.assertEqual(len(filtered_users), expected_count)
def test_invalid_role_filter(self):
course_role_filter = CourseRoleAudienceFilter(self.course.id)
role_names = ["INVALID_MODE"]
with self.assertRaises(ValueError):
course_role_filter.filter(role_names)
@ddt.ddt
class TestCohortAudienceFilter(ModuleStoreTestCase):
"""
Tests for the CohortAudienceFilter.
"""
def setUp(self):
super().setUp() # lint-amnesty, pylint: disable=super-with-arguments
self.course = CourseFactory()
self.cohort1_users = [UserFactory() for _ in range(3)]
self.cohort2_users = [UserFactory() for _ in range(2)]
users = self.cohort1_users + self.cohort2_users
for user in users:
CourseEnrollment.enroll(user, self.course.id)
self.cohort1 = CohortFactory(course_id=self.course.id, users=self.cohort1_users)
self.cohort2 = CohortFactory(course_id=self.course.id, users=self.cohort2_users)
@ddt.data(
([1], 3),
([2], 2),
([1, 2], 5),
)
@ddt.unpack
def test_valid_cohort_filter(self, cohort_ids, expected_count):
cohort_filter = CohortAudienceFilter(self.course.id)
filtered_users = cohort_filter.filter(cohort_ids)
self.assertEqual(len(filtered_users), expected_count)
def test_invalid_cohort_filter(self):
cohort_filter = CohortAudienceFilter(self.course.id)
cohort_ids = ["INVALID_MODE"]
with self.assertRaises(ValueError):
cohort_filter.filter(cohort_ids)
@ddt.ddt
class TestTeamAudienceFilter(ModuleStoreTestCase):
"""
Tests for the TeamAudienceFilter.
"""
def setUp(self):
super().setUp() # lint-amnesty, pylint: disable=super-with-arguments
self.course = CourseFactory()
self.teams = [CourseTeamFactory(course_id=self.course.id, team_id=f"team-{i}") for i in range(2)]
self.team1_users = [UserFactory() for _ in range(3)]
self.team2_users = [UserFactory() for _ in range(2)]
users = self.team1_users + self.team2_users
for user in users:
CourseEnrollment.enroll(user, self.course.id)
for user in self.team1_users:
CourseTeamMembershipFactory.create(team=self.teams[0], user=user)
for user in self.team2_users:
CourseTeamMembershipFactory.create(team=self.teams[1], user=user)
@ddt.data(
(["team-0"], 3),
(["team-1"], 2),
(["team-0", "team-1"], 5),
)
@ddt.unpack
def test_valid_team_filter(self, team_ids, expected_count):
team_filter = TeamAudienceFilter(self.course.id)
filtered_users = team_filter.filter(team_ids)
self.assertEqual(len(filtered_users), expected_count)
def test_invalid_team_filter(self):
team_filter = TeamAudienceFilter(self.course.id)
team_ids = ["INVALID_MODE"]
with self.assertRaises(ValueError):
team_filter.filter(team_ids)
@ddt.ddt
class TestAudienceFilter(ModuleStoreTestCase):
"""
@@ -310,16 +432,16 @@ class TestAudienceFilter(ModuleStoreTestCase):
@ddt.data(
({
"enrollment": ["verified"],
"role": ["Moderator"],
"enrollments": ["verified"],
"discussion_roles": ["Moderator"],
}, 15),
({
"enrollment": ["audit", "verified"],
"role": ["Administrator", "Student"],
"enrollments": ["audit", "verified"],
"discussion_roles": ["Administrator", "Student"],
}, 30),
({
"enrollment": ["audit", "honor", "verified"],
"role": ["Administrator", "Moderator", "Student", "Community TA"],
"enrollments": ["audit", "honor", "verified"],
"discussion_roles": ["Administrator", "Moderator", "Student", "Community TA"],
}, 30),
)
@ddt.unpack