feat: skip saving non_editable in db notification preferences (#36968)

This commit is contained in:
Muhammad Adeel Tajamul
2025-07-03 11:32:59 +05:00
committed by GitHub
parent 1e2a2d9653
commit f996ed7c16
6 changed files with 89 additions and 50 deletions

View File

@@ -451,9 +451,8 @@ class NotificationTypeManager:
non_core_notification_types_preferences = self.get_non_core_notification_type_preferences(
non_core_notification_types, email_opt_out
)
non_editable_notification_channels = self.get_non_editable_notification_channels(non_core_notification_types)
core_notification_types_name = [notification_type.get('name') for notification_type in core_notification_types]
return non_core_notification_types_preferences, core_notification_types_name, non_editable_notification_channels
return non_core_notification_types_preferences, core_notification_types_name
class NotificationAppManager:
@@ -486,18 +485,15 @@ class NotificationAppManager:
course_notification_preference_config = {}
for notification_app_key, notification_app_attrs in COURSE_NOTIFICATION_APPS.items():
notification_app_preferences = {}
notification_types, core_notifications, \
non_editable_channels = NotificationTypeManager().get_notification_app_preference(
notification_app_key,
email_opt_out
)
notification_types, core_notifications = NotificationTypeManager().get_notification_app_preference(
notification_app_key,
email_opt_out
)
self.add_core_notification_preference(notification_app_attrs, notification_types, email_opt_out)
self.add_core_notification_non_editable(notification_app_attrs, non_editable_channels)
notification_app_preferences['enabled'] = notification_app_attrs.get('enabled', False)
notification_app_preferences['core_notification_types'] = core_notifications
notification_app_preferences['notification_types'] = notification_types
notification_app_preferences['non_editable'] = non_editable_channels
course_notification_preference_config[notification_app_key] = notification_app_preferences
return course_notification_preference_config

View File

@@ -444,6 +444,9 @@ def is_notification_type_channel_editable(app_name, notification_type, channel):
"""
Returns if notification type channel is editable
"""
notification_type = 'core'\
if COURSE_NOTIFICATION_TYPES.get(notification_type, {}).get("is_core", False)\
else notification_type
if notification_type == 'core':
return channel not in COURSE_NOTIFICATION_APPS[app_name]['non_editable']
return channel not in COURSE_NOTIFICATION_TYPES[notification_type]['non_editable']

View File

@@ -116,6 +116,9 @@ class UserCourseNotificationPreferenceSerializer(serializers.ModelSerializer):
user = self.context['user']
preferences = add_info_to_notification_config(preferences)
preferences = remove_preferences_with_no_access(preferences, user)
preferences['notification_preference_config'] = add_non_editable_in_preference(
preferences.get('notification_preference_config', {})
)
return preferences
def get_course_name(self, obj):
@@ -184,6 +187,19 @@ class UserNotificationPreferenceUpdateSerializer(serializers.Serializer):
f'{notification_channel} is not a valid notification channel setting.'
)
if notification_app and notification_type and notification_channel:
if not is_notification_type_channel_editable(
notification_app,
notification_type,
'email' if notification_channel == 'email_cadence' else notification_channel
):
raise ValidationError({
'notification_channel': (
f'{notification_channel} is not editable for notification type '
f'{notification_type}.'
)
})
return attrs
def update(self, instance, validated_data):
@@ -213,7 +229,7 @@ class UserNotificationPreferenceUpdateSerializer(serializers.Serializer):
elif notification_channel and not notification_type:
app_prefs = user_notification_preference_config[notification_app]
for notification_type_name, notification_type_preferences in app_prefs['notification_types'].items():
non_editable_channels = app_prefs['non_editable'].get(notification_type_name, [])
non_editable_channels = get_non_editable_channels(notification_app).get(notification_type_name, [])
if notification_channel not in non_editable_channels:
app_prefs['notification_types'][notification_type_name][notification_channel] = value
@@ -294,6 +310,34 @@ def validate_notification_channel(notification_channel: str) -> str:
return notification_channel
def get_non_editable_channels(app_name):
"""
Returns a dict of notification: [non-editable channels] for the given app name.
"""
non_editable = {"core": COURSE_NOTIFICATION_APPS[app_name].get("non_editable", [])}
for type_name, type_dict in COURSE_NOTIFICATION_TYPES.items():
if type_dict.get("non_editable") and not type_dict["is_core"]:
non_editable[type_name] = type_dict["non_editable"]
return non_editable
def add_non_editable_in_preference(preference):
"""
Add non_editable preferences to the preference dict
"""
for app_name, app_dict in preference.items():
non_editable = {}
for type_name in app_dict.get('notification_types', {}).keys():
if type_name == "core":
non_editable_channels = COURSE_NOTIFICATION_APPS.get(app_name, {}).get('non_editable', [])
else:
non_editable_channels = COURSE_NOTIFICATION_TYPES.get(type_name, {}).get('non_editable', [])
if non_editable_channels:
non_editable[type_name] = non_editable_channels
app_dict['non_editable'] = non_editable
return preference
class UserNotificationPreferenceUpdateAllSerializer(serializers.Serializer):
"""
Serializer for user notification preferences update with custom field validators.
@@ -345,7 +389,7 @@ class UserNotificationPreferenceUpdateAllSerializer(serializers.Serializer):
if not is_notification_type_channel_editable(
notification_app,
notification_type,
notification_channel
"email" if notification_channel == "email_cadence" else notification_channel
):
raise ValidationError({
'notification_channel': (

View File

@@ -186,42 +186,6 @@ class NotificationPreferenceSyncManagerTest(ModuleStoreTestCase):
assert preference_type['email'] == email_value
assert preference_type['push'] == push_value
def test_non_editable_addition_and_removal(self):
"""
Tests if non_editable updates on existing preferences
"""
current_config_version = get_course_notification_preference_config_version()
base_notification.COURSE_NOTIFICATION_TYPES[self.default_type_name]['non_editable'] = ['web']
self._set_notification_config_version(current_config_version + 1)
new_config = CourseNotificationPreference.get_updated_user_course_preferences(self.user, self.course.id)
preferences = new_config.notification_preference_config
preference_non_editable = preferences[self.default_app_name]['non_editable'][self.default_type_name]
assert 'web' in preference_non_editable
base_notification.COURSE_NOTIFICATION_TYPES[self.default_type_name]['non_editable'] = []
self._set_notification_config_version(current_config_version + 2)
new_config = CourseNotificationPreference.get_updated_user_course_preferences(self.user, self.course.id)
preferences = new_config.notification_preference_config
preference_non_editable = preferences[self.default_app_name]['non_editable'].get(self.default_type_name, [])
assert preference_non_editable == []
def test_non_editable_addition_and_removal_for_core_notification(self):
"""
Tests if non_editable updates on existing preferences of core notification
"""
current_config_version = get_course_notification_preference_config_version()
base_notification.COURSE_NOTIFICATION_APPS[self.default_app_name]['non_editable'] = ['web']
self._set_notification_config_version(current_config_version + 1)
new_config = CourseNotificationPreference.get_updated_user_course_preferences(self.user, self.course.id)
preferences = new_config.notification_preference_config
preference_non_editable = preferences[self.default_app_name]['non_editable']['core']
assert 'web' in preference_non_editable
base_notification.COURSE_NOTIFICATION_APPS[self.default_app_name]['non_editable'] = []
self._set_notification_config_version(current_config_version + 2)
new_config = CourseNotificationPreference.get_updated_user_course_preferences(self.user, self.course.id)
preferences = new_config.notification_preference_config
preference_non_editable = preferences[self.default_app_name]['non_editable'].get('core', [])
assert preference_non_editable == []
def test_notification_type_in_core(self):
"""
Tests addition/removal of core in notification type

View File

@@ -539,6 +539,11 @@ class UserNotificationPreferenceAPITest(ModuleStoreTestCase):
for _, type_prefs in app_prefs.get('notification_types', {}).items():
assert 'info' not in type_prefs.keys()
def test_non_editable_is_not_saved_in_json(self):
default_prefs = NotificationAppManager().get_notification_app_preferences()
for app_prefs in default_prefs.values():
assert 'non_editable' not in app_prefs.keys()
@ddt.data(*itertools.product(('email', 'web'), (True, False)))
@ddt.unpack
def test_unsub_user_preferences_removal_on_email_enabled(self, channel, value):
@@ -1373,7 +1378,7 @@ class GetAggregateNotificationPreferencesTest(APITestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data['status'], 'success')
self.assertEqual(response.data['message'], 'Notification preferences retrieved')
self.assertEqual(response.data['data'], {'mocked': {'notification_types': {}}})
self.assertDictEqual(response.data['data'], {'mocked': {'notification_types': {}, 'non_editable': {}}})
def test_unauthenticated_user(self):
"""
@@ -1383,3 +1388,29 @@ class GetAggregateNotificationPreferencesTest(APITestCase):
self.client.logout() # Remove authentication
response = self.client.get(self.url)
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
@mock.patch.dict(COURSE_NOTIFICATION_APPS, {
**COURSE_NOTIFICATION_APPS,
**{
'discussion': {
'name': 'content_reported',
'non_editable': ["web"]
}
}
})
@mock.patch.dict(COURSE_NOTIFICATION_TYPES, {
**COURSE_NOTIFICATION_TYPES,
**{
'course_updates': {
**COURSE_NOTIFICATION_TYPES['course_updates'],
'non_editable': ["email"]
}
}
})
def test_non_editable_is_added_in_api_response(self):
CourseNotificationPreference.objects.create(user=self.user, is_active=True)
response = self.client.get(self.url)
self.assertEqual(response.status_code, status.HTTP_200_OK)
prefs = response.data['data']
self.assertDictEqual(prefs['updates']['non_editable'], {'course_updates': ['email']})
self.assertDictEqual(prefs['discussion']['non_editable'], {'core': ['web']})

View File

@@ -40,7 +40,8 @@ from .serializers import (
NotificationSerializer,
UserCourseNotificationPreferenceSerializer,
UserNotificationPreferenceUpdateAllSerializer,
UserNotificationPreferenceUpdateSerializer
UserNotificationPreferenceUpdateSerializer,
add_non_editable_in_preference
)
from .utils import (
aggregate_notification_configs,
@@ -623,5 +624,5 @@ class AggregatedNotificationPreferences(APIView):
return Response({
'status': 'success',
'message': 'Notification preferences retrieved',
'data': notification_configs
'data': add_non_editable_in_preference(notification_configs)
}, status=status.HTTP_200_OK)