From f996ed7c16e281c8d3d5b20666488ce777ca42f5 Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Thu, 3 Jul 2025 11:32:59 +0500 Subject: [PATCH] feat: skip saving non_editable in db notification preferences (#36968) --- .../notifications/base_notification.py | 14 ++---- .../djangoapps/notifications/email/utils.py | 3 ++ .../djangoapps/notifications/serializers.py | 48 ++++++++++++++++++- .../tests/test_base_notification.py | 36 -------------- .../notifications/tests/test_views.py | 33 ++++++++++++- .../core/djangoapps/notifications/views.py | 5 +- 6 files changed, 89 insertions(+), 50 deletions(-) diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index d345eb1263..4c7afa73f5 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -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 diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py index 43ba900022..d40024adad 100644 --- a/openedx/core/djangoapps/notifications/email/utils.py +++ b/openedx/core/djangoapps/notifications/email/utils.py @@ -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'] diff --git a/openedx/core/djangoapps/notifications/serializers.py b/openedx/core/djangoapps/notifications/serializers.py index c87b0b9805..582b33a1c5 100644 --- a/openedx/core/djangoapps/notifications/serializers.py +++ b/openedx/core/djangoapps/notifications/serializers.py @@ -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': ( diff --git a/openedx/core/djangoapps/notifications/tests/test_base_notification.py b/openedx/core/djangoapps/notifications/tests/test_base_notification.py index 9a49fa987e..ff9b0c9ed6 100644 --- a/openedx/core/djangoapps/notifications/tests/test_base_notification.py +++ b/openedx/core/djangoapps/notifications/tests/test_base_notification.py @@ -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 diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index 2e92a4d572..740824b780 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -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']}) diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index bece28da1f..190c115fa2 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -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)