chore: Exclude course staff preferences from general user notifications (#37040)

This commit is contained in:
Hassan Raza
2025-07-21 15:07:00 +05:00
committed by GitHub
parent 692caf0f46
commit cc2126749a
3 changed files with 99 additions and 26 deletions

View File

@@ -21,7 +21,7 @@ from rest_framework import status
from rest_framework.test import APIClient, APITestCase
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseStaffRole
from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
@@ -38,7 +38,8 @@ from openedx.core.djangoapps.notifications.models import (
Notification,
get_course_notification_preference_config_version, NotificationPreference
)
from openedx.core.djangoapps.notifications.serializers import NotificationCourseEnrollmentSerializer
from openedx.core.djangoapps.notifications.serializers import NotificationCourseEnrollmentSerializer, \
add_non_editable_in_preference
from openedx.core.djangoapps.user_api.models import UserPreference
from openedx.core.djangoapps.notifications.email.utils import update_user_preferences_from_patch
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -46,7 +47,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
from ..base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES, NotificationAppManager, \
NotificationTypeManager
from ..utils import get_notification_types_with_visibility_settings
from ..utils import get_notification_types_with_visibility_settings, exclude_inaccessible_preferences
User = get_user_model()
@@ -1453,13 +1454,15 @@ class GetAggregateNotificationPreferencesTest(APITestCase):
})
class TestNotificationPreferencesView(APITestCase):
@ddt.ddt
class TestNotificationPreferencesView(ModuleStoreTestCase):
"""
Tests for the NotificationPreferencesView API view.
"""
def setUp(self):
# Set up a user and API client
super().setUp()
self.default_data = {
"status": "success",
"message": "Notification preferences retrieved successfully.",
@@ -1584,20 +1587,67 @@ class TestNotificationPreferencesView(APITestCase):
}
}
}
self.user = User.objects.create_user(username='testuser', password='testpass')
self.TEST_PASSWORD = 'testpass'
self.user = UserFactory(password=self.TEST_PASSWORD)
self.client = APIClient()
self.client.force_authenticate(user=self.user)
self.url = reverse('notification-preferences-aggregated-v2') # Adjust with the actual name
self.course = CourseFactory.create(display_name='test course 1', run="Testing_course_1")
def test_get_notification_preferences(self):
@ddt.data(
("forum", FORUM_ROLE_ADMINISTRATOR, ['content_reported'], ['ora_staff_notifications']),
("forum", FORUM_ROLE_MODERATOR, ['content_reported'], ['ora_staff_notifications']),
("forum", FORUM_ROLE_COMMUNITY_TA, ['content_reported'], ['ora_staff_notifications']),
("course", CourseStaffRole.ROLE, ['ora_staff_notifications'], ['content_reported']),
("course", CourseInstructorRole.ROLE, ['ora_staff_notifications'], ['content_reported']),
(None, None, [], ['ora_staff_notifications', 'content_reported']),
)
@ddt.unpack
def test_get_notification_preferences(self, role_type, role, visible_apps, hidden_apps):
"""
Test case: Get notification preferences for the authenticated user
Test: Notification preferences visibility for users with forum, course, or no role.
"""
role_instance = None
if role_type == "course":
if role == CourseInstructorRole.ROLE:
CourseStaffRole(self.course.id).add_users(self.user)
else:
CourseInstructorRole(self.course.id).add_users(self.user)
self.client.login(username=self.user.username, password='testpass')
elif role_type == "forum":
role_instance = RoleFactory(name=role, course_id=self.course.id)
role_instance.users.add(self.user)
response = self.client.get(self.url)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data['status'], 'success')
self.assertIn('data', response.data)
self.assertEqual(response.data['data'], self.default_data['data'])
expected_data = exclude_inaccessible_preferences(self.default_data['data'], self.user)
expected_data = add_non_editable_in_preference(expected_data)
self.assertEqual(response.data['data'], expected_data)
notification_apps = {}
for app in ['discussion', 'grading']:
notification_apps.update(response.data['data'][app]['notification_types'])
for app in visible_apps:
self.assertIn(app, notification_apps, msg=f"{app} should be visible for role: {role_type}")
for app in hidden_apps:
self.assertNotIn(app, notification_apps, msg=f"{app} should NOT be visible for role: {role_type}")
if role_type == "forum":
role_instance.users.clear()
elif role_type == "course":
if role == CourseInstructorRole.ROLE:
CourseStaffRole(self.course.id).remove_users(self.user)
else:
CourseInstructorRole(self.course.id).remove_users(self.user)
def test_if_data_is_correctly_aggregated(self):
"""
@@ -1642,12 +1692,6 @@ class TestNotificationPreferencesView(APITestCase):
"push": False,
"email_cadence": "Daily"
},
"content_reported": {
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
},
"new_instructor_all_learners_post": {
"web": False,
"email": False,
@@ -1664,7 +1708,6 @@ class TestNotificationPreferencesView(APITestCase):
"non_editable": {
"new_discussion_post": ["push"],
"new_question_post": ["push"],
"content_reported": ["push"],
"new_instructor_all_learners_post": ["push"]
}
},
@@ -1693,12 +1736,6 @@ class TestNotificationPreferencesView(APITestCase):
"enabled": True,
"core_notification_types": [],
"notification_types": {
"ora_staff_notifications": {
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
},
"ora_grade_assigned": {
"web": False,
"email": False,
@@ -1713,8 +1750,7 @@ class TestNotificationPreferencesView(APITestCase):
}
},
"non_editable": {
"ora_grade_assigned": ["push"],
"ora_staff_notifications": ["push"]
"ora_grade_assigned": ["push"]
}
},
"enrollments": {
@@ -1769,7 +1805,9 @@ class TestNotificationPreferencesView(APITestCase):
response = self.client.put(self.url, update_data, format='json')
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data['status'], 'success')
cadence_set = NotificationPreference.objects.filter(type__in=core_types).values_list('email_cadence', flat=True)
cadence_set = NotificationPreference.objects.filter(user=self.user, type__in=core_types).values_list(
'email_cadence', flat=True
)
self.assertEqual(len(set(cadence_set)), 1)
self.assertIn('Weekly', set(cadence_set))

View File

@@ -295,3 +295,37 @@ def filter_out_visible_preferences_by_course_ids(user, preferences: Dict, course
forum_roles,
course_roles
)
def get_user_forum_access_roles(user_id: int) -> List[str]:
"""
Get forum roles for the given user in all course.
:param user_id: User ID
:return: List of forum roles
"""
return list(Role.objects.filter(users__id=user_id).values_list('name', flat=True))
def exclude_inaccessible_preferences(user_preferences: dict, user):
"""
Exclude notifications from user preferences that the user has no access to,
based on forum and course roles.
:param user_preferences: Dictionary of user notification preferences
:param user: Django User object
:return: Updated user_preferences dictionary (modified in-place)
"""
forum_roles = get_user_forum_access_roles(user.id)
visible_notifications = get_notification_types_with_visibility_settings()
course_roles = CourseAccessRole.objects.filter(
user=user
).values_list('role', flat=True)
filter_out_visible_notifications(
user_preferences,
visible_notifications,
forum_roles,
course_roles
)
return user_preferences

View File

@@ -49,7 +49,8 @@ from .tasks import create_notification_preference
from .utils import (
aggregate_notification_configs,
filter_out_visible_preferences_by_course_ids,
get_show_notifications_tray
get_show_notifications_tray,
exclude_inaccessible_preferences
)
@@ -702,7 +703,7 @@ class NotificationPreferencesView(APIView):
type_details['email'] = user_pref.email
type_details['push'] = user_pref.push
type_details['email_cadence'] = user_pref.email_cadence
exclude_inaccessible_preferences(structured_preferences, request.user)
return Response({
'status': 'success',
'message': 'Notification preferences retrieved successfully.',