From 027080980832a2e03b23d19b24018c1164ae1c4b Mon Sep 17 00:00:00 2001 From: Saad Yousaf Date: Thu, 15 Feb 2024 17:26:43 +0500 Subject: [PATCH] feat: add email cadence setting in notification preferences for emails --- .../notifications/base_notification.py | 16 ++++- .../notifications/email_notifications.py | 35 ++++++++++ .../core/djangoapps/notifications/events.py | 5 +- .../core/djangoapps/notifications/models.py | 24 ++++++- .../djangoapps/notifications/serializers.py | 49 +++++++++++--- .../tests/test_base_notification.py | 4 ++ .../notifications/tests/test_views.py | 67 ++++++++++++++++--- openedx/core/djangoapps/notifications/urls.py | 3 +- .../core/djangoapps/notifications/views.py | 10 +-- 9 files changed, 178 insertions(+), 35 deletions(-) create mode 100644 openedx/core/djangoapps/notifications/email_notifications.py diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index 1942da5f5c2a..5f498a951d2e 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -3,11 +3,13 @@ """ from django.utils.translation import gettext_lazy as _ +from .email_notifications import EmailCadence from .utils import find_app_in_normalized_apps, find_pref_in_normalized_prefs from ..django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE = 'filter_audit_expired_users_with_no_role' + COURSE_NOTIFICATION_TYPES = { 'new_comment_on_response': { 'notification_app': 'discussion', @@ -56,6 +58,7 @@ 'info': '', 'web': False, 'email': False, + 'email_cadence': EmailCadence.DAILY, 'push': False, 'non_editable': [], 'content_template': _('<{p}><{strong}>{username} posted <{strong}>{post_title}'), @@ -73,6 +76,7 @@ 'info': '', 'web': False, 'email': False, + 'email_cadence': EmailCadence.DAILY, 'push': False, 'non_editable': [], 'content_template': _('<{p}><{strong}>{username} asked <{strong}>{post_title}'), @@ -121,6 +125,7 @@ 'info': '', 'web': True, 'email': True, + 'email_cadence': EmailCadence.DAILY, 'push': True, 'non_editable': [], 'content_template': _('

{username}’s {content_type} has been reported {' @@ -170,6 +175,7 @@ 'web': True, 'email': True, 'push': True, + 'email_cadence': EmailCadence.DAILY, 'non_editable': [], 'content_template': _('<{p}>You have a new course update: ' '<{strong}>{course_update_content}'), @@ -189,6 +195,7 @@ 'core_web': True, 'core_email': True, 'core_push': True, + 'core_email_cadence': EmailCadence.DAILY, 'non_editable': ['web'] }, 'updates': { @@ -197,6 +204,7 @@ 'core_web': True, 'core_email': True, 'core_push': True, + 'core_email_cadence': EmailCadence.DAILY, 'non_editable': [] }, } @@ -263,7 +271,7 @@ def denormalize_preferences(normalized_preferences): 'web': preference.get('web'), 'push': preference.get('push'), 'email': preference.get('email'), - 'info': preference.get('info'), + 'email_cadence': preference.get('email_cadence'), } return denormalized_preferences @@ -298,8 +306,8 @@ def update_preferences(preferences): app_name = preference.get('app_name') pref = find_pref_in_normalized_prefs(pref_name, app_name, old_preferences.get('preferences')) if pref: - for channel in ['web', 'email', 'push']: - preference[channel] = pref[channel] + for channel in ['web', 'email', 'push', 'email_cadence']: + preference[channel] = pref.get(channel, preference.get(channel)) return NotificationPreferenceSyncManager.denormalize_preferences(new_prefs) @@ -357,6 +365,7 @@ def get_non_core_notification_type_preferences(non_core_notification_types): 'web': notification_type.get('web', False), 'email': notification_type.get('email', False), 'push': notification_type.get('push', False), + 'email_cadence': notification_type.get('email_cadence', 'Daily'), } return non_core_notification_type_preferences @@ -388,6 +397,7 @@ def add_core_notification_preference(self, notification_app_attrs, notification_ 'web': notification_app_attrs.get('core_web', False), 'email': notification_app_attrs.get('core_email', False), 'push': notification_app_attrs.get('core_push', False), + 'email_cadence': notification_app_attrs.get('core_email_cadence', 'Daily'), } def add_core_notification_non_editable(self, notification_app_attrs, non_editable_channels): diff --git a/openedx/core/djangoapps/notifications/email_notifications.py b/openedx/core/djangoapps/notifications/email_notifications.py new file mode 100644 index 000000000000..9c0bd2406b49 --- /dev/null +++ b/openedx/core/djangoapps/notifications/email_notifications.py @@ -0,0 +1,35 @@ +""" +Email notifications module. +""" +from django.utils.translation import gettext_lazy as _ + + +class EmailCadence: + """ + Email cadence class + """ + DAILY = 'Daily' + WEEKLY = 'Weekly' + INSTANTLY = 'Instantly' + NEVER = 'Never' + EMAIL_CADENCE_CHOICES = [ + (DAILY, _('Daily')), + (WEEKLY, _('Weekly')), + (INSTANTLY, _('Instantly')), + (NEVER, _('Never')), + ] + EMAIL_CADENCE_CHOICES_DICT = dict(EMAIL_CADENCE_CHOICES) + + @classmethod + def get_email_cadence_choices(cls): + """ + Returns email cadence choices. + """ + return cls.EMAIL_CADENCE_CHOICES + + @classmethod + def get_email_cadence_value(cls, email_cadence): + """ + Returns email cadence display for the given email cadence. + """ + return cls.EMAIL_CADENCE_CHOICES_DICT.get(email_cadence, None) diff --git a/openedx/core/djangoapps/notifications/events.py b/openedx/core/djangoapps/notifications/events.py index e76149f1c475..fb50e134941b 100644 --- a/openedx/core/djangoapps/notifications/events.py +++ b/openedx/core/djangoapps/notifications/events.py @@ -126,6 +126,9 @@ def notification_preference_update_event(user, course_id, updated_preference): """ context = contexts.course_context_from_course_id(course_id) with tracker.get_tracker().context(NOTIFICATION_PREFERENCES_UPDATED, context): + value = updated_preference.get('value', '') + if updated_preference.get('notification_channel', '') == 'email_cadence': + value = updated_preference.get('email_cadence', '') tracker.emit( NOTIFICATION_PREFERENCES_UPDATED, { @@ -136,7 +139,7 @@ def notification_preference_update_event(user, course_id, updated_preference): 'notification_app': updated_preference.get('notification_app', ''), 'notification_type': updated_preference.get('notification_type', ''), 'notification_channel': updated_preference.get('notification_channel', ''), - 'value': updated_preference.get('value', ''), + 'value': value } ) diff --git a/openedx/core/djangoapps/notifications/models.py b/openedx/core/djangoapps/notifications/models.py index e0724fbe9b53..1336b3fcdb93 100644 --- a/openedx/core/djangoapps/notifications/models.py +++ b/openedx/core/djangoapps/notifications/models.py @@ -20,6 +20,8 @@ NOTIFICATION_CHANNELS = ['web', 'push', 'email'] +ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS = ['email_cadence'] + # Update this version when there is a change to any course specific notification type or app. COURSE_NOTIFICATION_CONFIG_VERSION = 7 @@ -84,6 +86,13 @@ def get_notification_channels(): return NOTIFICATION_CHANNELS +def get_additional_notification_channel_settings(): + """ + Returns the additional notification channel settings. + """ + return ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS + + class Notification(TimeStampedModel): """ Model to store notifications for users @@ -224,9 +233,18 @@ def get_channels_for_notification_type(self, app_name, notification_type) -> lis ['web', 'push'] """ if self.is_core(app_name, notification_type): - return [channel for channel in NOTIFICATION_CHANNELS if self.get_core_config(app_name).get(channel, False)] - return [channel for channel in NOTIFICATION_CHANNELS if - self.get_notification_type_config(app_name, notification_type).get(channel, False)] + notification_channels = [channel for channel in NOTIFICATION_CHANNELS if + self.get_core_config(app_name).get(channel, False)] + additional_channel_settings = [channel for channel in ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS if + self.get_core_config(app_name).get(channel, False)] + else: + notification_channels = [channel for channel in NOTIFICATION_CHANNELS if + self.get_notification_type_config(app_name, notification_type).get(channel, False)] + additional_channel_settings = [channel for channel in ADDITIONAL_NOTIFICATION_CHANNEL_SETTINGS if + self.get_notification_type_config(app_name, notification_type).get(channel, + False)] + + return notification_channels + additional_channel_settings def is_core(self, app_name, notification_type) -> bool: """ diff --git a/openedx/core/djangoapps/notifications/serializers.py b/openedx/core/djangoapps/notifications/serializers.py index 8f44c5a29003..3b6877cc29f7 100644 --- a/openedx/core/djangoapps/notifications/serializers.py +++ b/openedx/core/djangoapps/notifications/serializers.py @@ -9,9 +9,9 @@ from openedx.core.djangoapps.notifications.models import ( CourseNotificationPreference, Notification, - get_notification_channels + get_notification_channels, get_additional_notification_channel_settings ) -from .base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES +from .base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES, EmailCadence from .utils import filter_course_wide_preferences, remove_preferences_with_no_access @@ -90,9 +90,10 @@ class UserNotificationPreferenceUpdateSerializer(serializers.Serializer): """ notification_app = serializers.CharField() - value = serializers.BooleanField() + value = serializers.BooleanField(required=False) notification_type = serializers.CharField(required=False) notification_channel = serializers.CharField(required=False) + email_cadence = serializers.CharField(required=False) def validate(self, attrs): """ @@ -101,17 +102,24 @@ def validate(self, attrs): notification_app = attrs.get('notification_app') notification_type = attrs.get('notification_type') notification_channel = attrs.get('notification_channel') + notification_email_cadence = attrs.get('email_cadence') notification_app_config = self.instance.notification_preference_config + if notification_email_cadence: + if not notification_type: + raise ValidationError( + 'notification_type is required for email_cadence.' + ) + if EmailCadence.get_email_cadence_value(notification_email_cadence) is None: + raise ValidationError( + f'{attrs.get("value")} is not a valid email cadence.' + ) + if notification_type and not notification_channel: raise ValidationError( 'notification_channel is required for notification_type.' ) - if notification_channel and not notification_type: - raise ValidationError( - 'notification_type is required for notification_channel.' - ) if not notification_app_config.get(notification_app, None): raise ValidationError( @@ -126,9 +134,13 @@ def validate(self, attrs): f'{notification_type} is not a valid notification type.' ) - if notification_channel and notification_channel not in get_notification_channels(): + if ( + notification_channel and + notification_channel not in get_notification_channels() + and notification_channel not in get_additional_notification_channel_settings() + ): raise ValidationError( - f'{notification_channel} is not a valid notification channel.' + f'{notification_channel} is not a valid notification channel setting.' ) return attrs @@ -141,13 +153,30 @@ def update(self, instance, validated_data): notification_type = validated_data.get('notification_type') notification_channel = validated_data.get('notification_channel') value = validated_data.get('value') + notification_email_cadence = validated_data.get('email_cadence') + user_notification_preference_config = instance.notification_preference_config - if notification_type and notification_channel: + # Notification email cadence update + if notification_email_cadence and notification_type: + user_notification_preference_config[notification_app]['notification_types'][notification_type][ + 'email_cadence'] = notification_email_cadence + + # Notification type channel update + elif notification_type and notification_channel: # Update the notification preference for specific notification type user_notification_preference_config[ notification_app]['notification_types'][notification_type][notification_channel] = value + # Notification app-wide channel update + 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, []) + if notification_channel not in non_editable_channels: + app_prefs['notification_types'][notification_type_name][notification_channel] = value + + # Notification app update else: # Update the notification preference for notification_app user_notification_preference_config[notification_app]['enabled'] = value diff --git a/openedx/core/djangoapps/notifications/tests/test_base_notification.py b/openedx/core/djangoapps/notifications/tests/test_base_notification.py index 3130b4375cf2..9a49fa987eb4 100644 --- a/openedx/core/djangoapps/notifications/tests/test_base_notification.py +++ b/openedx/core/djangoapps/notifications/tests/test_base_notification.py @@ -88,6 +88,7 @@ def _create_notification_app(self, overrides=None): 'core_web': True, 'core_email': True, 'core_push': True, + 'core_email_cadence': 'Daily', } if overrides is not None: notification_app.update(overrides) @@ -104,6 +105,7 @@ def _create_notification_type(self, name, overrides=None): 'web': True, 'email': True, 'push': True, + 'email_cadence': 'Daily', 'info': '', 'non_editable': [], 'content_template': '', @@ -255,6 +257,7 @@ def test_validate_notification_apps(self): for app_data in notification_apps.values(): assert 'core_info' in app_data.keys() assert isinstance(app_data['non_editable'], list) + assert isinstance(app_data['core_email_cadence'], str) for key in bool_keys: assert isinstance(app_data[key], bool) @@ -290,6 +293,7 @@ def test_validate_non_core_notification_types(self): assert 'content_template' in notification_type.keys() assert isinstance(notification_type['content_context'], dict) assert isinstance(notification_type['non_editable'], list) + assert isinstance(notification_type['email_cadence'], str) for key in str_keys: assert isinstance(notification_type[key], str) for key in bool_keys: diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index 0ac589a40a6c..a34408ed75f2 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -242,30 +242,34 @@ def _expected_api_response(self, course=None): 'response_endorsed' ], 'notification_types': { - 'core': { - 'web': True, - 'email': True, - 'push': True, - 'info': 'Notifications for responses and comments on your posts, and the ones you’re ' - 'following, including endorsements to your responses and on your posts.' - }, 'new_discussion_post': { 'web': False, 'email': False, 'push': False, + 'email_cadence': 'Daily', 'info': '' }, 'new_question_post': { 'web': False, 'email': False, 'push': False, + 'email_cadence': 'Daily', 'info': '' }, + 'core': { + 'web': True, + 'email': True, + 'push': True, + 'email_cadence': 'Daily', + 'info': 'Notifications for responses and comments on your posts, and the ones you’re ' + 'following, including endorsements to your responses and on your posts.' + }, 'content_reported': { 'web': True, 'email': True, 'push': True, - 'info': '' + 'info': '', + 'email_cadence': 'Daily', }, }, 'non_editable': { @@ -280,12 +284,14 @@ def _expected_api_response(self, course=None): 'web': True, 'email': True, 'push': True, + 'email_cadence': 'Daily', 'info': '' }, 'core': { 'web': True, 'email': True, 'push': True, + 'email_cadence': 'Daily', 'info': 'Notifications for new announcements and updates from the course team.' } }, @@ -372,6 +378,14 @@ def test_get_user_notification_preference_with_visibility_settings(self, role, m ('discussion', 'core', 'email', True, status.HTTP_200_OK, 'type_update'), ('discussion', 'core', 'email', False, status.HTTP_200_OK, 'type_update'), + # Test for email cadence update + ('discussion', 'core', 'email_cadence', 'Daily', status.HTTP_200_OK, 'type_update'), + ('discussion', 'core', 'email_cadence', 'Weekly', status.HTTP_200_OK, 'type_update'), + + # Test for app-wide channel update + ('discussion', None, 'email', True, status.HTTP_200_OK, 'app-wide-channel-update'), + ('discussion', None, 'email', False, status.HTTP_200_OK, 'app-wide-channel-update'), + ('discussion', 'invalid_notification_type', 'email', True, status.HTTP_400_BAD_REQUEST, None), ('discussion', 'new_comment', 'invalid_notification_channel', False, status.HTTP_400_BAD_REQUEST, None), ) @@ -395,6 +409,7 @@ def test_patch_user_notification_preference( response = self.client.patch(self.path, json.dumps(payload), content_type='application/json') self.assertEqual(response.status_code, expected_status) + expected_data = self._expected_api_response() if update_type == 'app_update': expected_data = self._expected_api_response() @@ -409,6 +424,15 @@ def test_patch_user_notification_preference( 'notification_types'][notification_type][notification_channel] = value self.assertEqual(response.data, expected_data) + elif update_type == 'app-wide-channel-update': + expected_data = remove_notifications_with_visibility_settings(expected_data) + app_prefs = expected_data['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, []) + if notification_channel not in non_editable_channels: + app_prefs['notification_types'][notification_type_name][notification_channel] = value + self.assertEqual(response.data, expected_data) + if expected_status == status.HTTP_200_OK: event_name, event_data = mock_emit.call_args[0] self.assertEqual(event_name, 'edx.notifications.preferences.updated') @@ -500,12 +524,31 @@ def _expected_api_response(self, course=None): 'web': True, 'email': True, 'push': True, + 'email_cadence': 'Daily', 'info': 'Notifications for responses and comments on your posts, and the ones you’re ' 'following, including endorsements to your responses and on your posts.' }, - 'new_discussion_post': {'web': False, 'email': False, 'push': False, 'info': ''}, - 'new_question_post': {'web': False, 'email': False, 'push': False, 'info': ''}, - 'content_reported': {'web': True, 'email': True, 'push': True, 'info': ''}, + 'new_discussion_post': { + 'web': False, + 'email': False, + 'push': False, + 'email_cadence': 'Daily', + 'info': '' + }, + 'new_question_post': { + 'web': False, + 'email': False, + 'push': False, + 'email_cadence': 'Daily', + 'info': '' + }, + 'content_reported': { + 'web': True, + 'email': True, + 'push': True, + 'email_cadence': 'Daily', + 'info': '' + }, }, 'non_editable': { 'core': ['web'] @@ -521,12 +564,14 @@ def _expected_api_response(self, course=None): 'web': True, 'email': True, 'push': True, + 'email_cadence': 'Daily', 'info': '' }, 'core': { 'web': True, 'email': True, 'push': True, + 'email_cadence': 'Daily', 'info': 'Notifications for new announcements and updates from the course team.' } }, diff --git a/openedx/core/djangoapps/notifications/urls.py b/openedx/core/djangoapps/notifications/urls.py index cef8d1b5491f..89b04443a581 100644 --- a/openedx/core/djangoapps/notifications/urls.py +++ b/openedx/core/djangoapps/notifications/urls.py @@ -11,8 +11,7 @@ NotificationCountView, NotificationListAPIView, NotificationReadAPIView, - UserNotificationPreferenceView, - UserNotificationChannelPreferenceView + UserNotificationPreferenceView, UserNotificationChannelPreferenceView, ) router = routers.DefaultRouter() diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index 4bdbda5eb6ca..cc830dbb8d7f 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -35,8 +35,7 @@ NotificationCourseEnrollmentSerializer, NotificationSerializer, UserCourseNotificationPreferenceSerializer, - UserNotificationPreferenceUpdateSerializer, - UserNotificationChannelPreferenceUpdateSerializer, + UserNotificationPreferenceUpdateSerializer, UserNotificationChannelPreferenceUpdateSerializer, ) from .utils import get_show_notifications_tray @@ -217,6 +216,10 @@ def patch(self, request, course_key_string): status=status.HTTP_409_CONFLICT, ) + if request.data.get('notification_channel', '') == 'email_cadence': + request.data['email_cadence'] = request.data['value'] + del request.data['value'] + preference_update = UserNotificationPreferenceUpdateSerializer( user_course_notification_preference, data=request.data, partial=True ) @@ -238,7 +241,6 @@ class UserNotificationChannelPreferenceView(APIView): """ Supports retrieving and patching the UserNotificationPreference model. - **Example Requests** PATCH /api/notifications/configurations/{course_id} """ @@ -250,7 +252,6 @@ def patch(self, request, course_key_string): Parameters: request (Request): The request object course_key_string (int): The ID of the course of the notification preference to be updated. - Returns: 200: The updated preference, serialized using the UserNotificationPreferenceSerializer 404: If the preference does not exist @@ -275,7 +276,6 @@ def patch(self, request, course_key_string): preference_update.is_valid(raise_exception=True) updated_notification_preferences = preference_update.save() notification_preference_update_event(request.user, course_id, preference_update.validated_data) - serializer_context = { 'course_id': course_id, 'user': request.user